From 79a52411762eb30f7aacf93ee8fad3ad81d1e6e9 Mon Sep 17 00:00:00 2001
From: Seth Hoenig <shoenig@hashicorp.com>
Date: Thu, 9 Apr 2020 20:26:26 -0600
Subject: [PATCH] connect: correctly deal with nil sidecar_service task stanza

Before, if the sidecar_service stanza of a connect enabled service
was missing, the job submission would cause a panic in the nomad
agent. Since the panic was happening in the API handler the agent
itself continued running, but this change will the condition more
gracefully.

By fixing the `Copy` method, the API handler now returns the proper
error.

$ nomad job run foo.nomad
Error submitting job: Unexpected response code: 500 (1 error occurred:
	* Task group api validation failed: 2 errors occurred:
	* Missing tasks for task group
	* Task group service validation failed: 1 error occurred:
	* Service[0] count-api validation failed: 1 error occurred:
	* Consul Connect must be native or use a sidecar service
---
 nomad/structs/services.go      |  3 +++
 nomad/structs/services_test.go | 30 ++++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/nomad/structs/services.go b/nomad/structs/services.go
index ee8b12b3b82d..d12bb697f986 100644
--- a/nomad/structs/services.go
+++ b/nomad/structs/services.go
@@ -701,6 +701,9 @@ func (s *ConsulSidecarService) HasUpstreams() bool {
 
 // Copy the stanza recursively. Returns nil if nil.
 func (s *ConsulSidecarService) Copy() *ConsulSidecarService {
+	if s == nil {
+		return nil
+	}
 	return &ConsulSidecarService{
 		Tags:  helper.CopySliceString(s.Tags),
 		Port:  s.Port,
diff --git a/nomad/structs/services_test.go b/nomad/structs/services_test.go
index b889a9ec7c34..f6fed3c64383 100644
--- a/nomad/structs/services_test.go
+++ b/nomad/structs/services_test.go
@@ -174,6 +174,8 @@ func TestConsulConnect_CopyEquals(t *testing.T) {
 }
 
 func TestSidecarTask_MergeIntoTask(t *testing.T) {
+	t.Parallel()
+
 	task := MockJob().TaskGroups[0].Tasks[0]
 	sTask := &SidecarTask{
 		Name:   "sidecar",
@@ -300,6 +302,8 @@ func TestConsulExposePath_exposePathsEqual(t *testing.T) {
 }
 
 func TestConsulExposeConfig_Copy(t *testing.T) {
+	t.Parallel()
+
 	require.Nil(t, (*ConsulExposeConfig)(nil).Copy())
 	require.Equal(t, &ConsulExposeConfig{
 		Paths: []ConsulExposePath{{
@@ -313,6 +317,8 @@ func TestConsulExposeConfig_Copy(t *testing.T) {
 }
 
 func TestConsulExposeConfig_Equals(t *testing.T) {
+	t.Parallel()
+
 	require.True(t, (*ConsulExposeConfig)(nil).Equals(nil))
 	require.True(t, (&ConsulExposeConfig{
 		Paths: []ConsulExposePath{{
@@ -324,3 +330,27 @@ func TestConsulExposeConfig_Equals(t *testing.T) {
 		}},
 	}))
 }
+
+func TestConsulSidecarService_Copy(t *testing.T) {
+	t.Parallel()
+
+	t.Run("nil", func(t *testing.T) {
+		s := (*ConsulSidecarService)(nil)
+		result := s.Copy()
+		require.Nil(t, result)
+	})
+
+	t.Run("not nil", func(t *testing.T) {
+		s := &ConsulSidecarService{
+			Tags:  []string{"foo", "bar"},
+			Port:  "port1",
+			Proxy: &ConsulProxy{LocalServiceAddress: "10.0.0.1"},
+		}
+		result := s.Copy()
+		require.Equal(t, &ConsulSidecarService{
+			Tags:  []string{"foo", "bar"},
+			Port:  "port1",
+			Proxy: &ConsulProxy{LocalServiceAddress: "10.0.0.1"},
+		}, result)
+	})
+}