consul/connect: split connect native flag and task in service

api/services.go

@@ -110,6 +110,7 @@ type Service struct {
 	Connect           *ConsulConnect
 	Meta              map[string]string
 	CanaryMeta        map[string]string
+	TaskName          string `mapstructure:"task"`
 }
 
 // Canonicalize the Service by ensuring its name and address mode are set. Task
@@ -140,7 +141,7 @@ func (s *Service) Canonicalize(t *Task, tg *TaskGroup, job *Job) {
 
 // ConsulConnect represents a Consul Connect jobspec stanza.
 type ConsulConnect struct {
-	Native         string
+	Native         bool
 	SidecarService *ConsulSidecarService `mapstructure:"sidecar_service"`
 	SidecarTask    *SidecarTask          `mapstructure:"sidecar_task"`
 }

command/agent/job_endpoint.go

@@ -1177,6 +1177,7 @@ func ApiServicesToStructs(in []*api.Service) []*structs.Service {
 		out[i] = &structs.Service{
 			Name:              s.Name,
 			PortLabel:         s.PortLabel,
+			TaskName:          s.TaskName,
 			Tags:              s.Tags,
 			CanaryTags:        s.CanaryTags,
 			EnableTagOverride: s.EnableTagOverride,

command/agent/job_endpoint_test.go

@@ -1693,7 +1693,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
 							},
 						},
 						Connect: &api.ConsulConnect{
-							Native: "",
+							Native: false,
 							SidecarService: &api.ConsulSidecarService{
 								Tags: []string{"f", "g"},
 								Port: "9000",
@@ -2061,7 +2061,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
 							},
 						},
 						Connect: &structs.ConsulConnect{
-							Native: "",
+							Native: false,
 							SidecarService: &structs.ConsulSidecarService{
 								Tags: []string{"f", "g"},
 								Port: "9000",
@@ -2767,11 +2767,11 @@ func TestConversion_ApiConsulConnectToStructs_legacy(t *testing.T) {
 	t.Parallel()
 	require.Nil(t, ApiConsulConnectToStructs(nil))
 	require.Equal(t, &structs.ConsulConnect{
-		Native:         "",
+		Native:         false,
 		SidecarService: &structs.ConsulSidecarService{Port: "myPort"},
 		SidecarTask:    &structs.SidecarTask{Name: "task"},
 	}, ApiConsulConnectToStructs(&api.ConsulConnect{
-		Native:         "",
+		Native:         false,
 		SidecarService: &api.ConsulSidecarService{Port: "myPort"},
 		SidecarTask:    &api.SidecarTask{Name: "task"},
 	}))
@@ -2781,8 +2781,8 @@ func TestConversion_ApiConsulConnectToStructs_native(t *testing.T) {
 	t.Parallel()
 	require.Nil(t, ApiConsulConnectToStructs(nil))
 	require.Equal(t, &structs.ConsulConnect{
-		Native: "foo",
+		Native: true,
 	}, ApiConsulConnectToStructs(&api.ConsulConnect{
-		Native: "foo",
+		Native: true,
 	}))
 }

jobspec/parse_service.go

@@ -47,6 +47,7 @@ func parseService(o *ast.ObjectItem) (*api.Service, error) {
 		"address_mode",
 		"check_restart",
 		"connect",
+		"task", // todo
 		"meta",
 		"canary_meta",
 	}

jobspec/parse_test.go

@@ -1168,7 +1168,7 @@ func TestParse(t *testing.T) {
 					Services: []*api.Service{{
 						Name: "example",
 						Connect: &api.ConsulConnect{
-							Native:         "",
+							Native:         false,
 							SidecarService: &api.ConsulSidecarService{},
 							SidecarTask: &api.SidecarTask{
 								Name: "my-sidecar",
@@ -1190,7 +1190,7 @@ func TestParse(t *testing.T) {
 					Services: []*api.Service{{
 						Name: "example",
 						Connect: &api.ConsulConnect{
-							Native: "",
+							Native: false,
 							SidecarService: &api.ConsulSidecarService{
 								Proxy: &api.ConsulProxy{
 									LocalServiceAddress: "10.0.1.2",
@@ -1237,7 +1237,7 @@ func TestParse(t *testing.T) {
 					Services: []*api.Service{{
 						Name: "example",
 						Connect: &api.ConsulConnect{
-							Native: "",
+							Native: false,
 							SidecarService: &api.ConsulSidecarService{
 								Proxy: &api.ConsulProxy{
 									LocalServiceAddress: "10.0.1.2",
@@ -1284,9 +1284,10 @@ func TestParse(t *testing.T) {
 				TaskGroups: []*api.TaskGroup{{
 					Name: helper.StringToPtr("group"),
 					Services: []*api.Service{{
-						Name: "example",
+						Name:     "example",
+						TaskName: "task1",
 						Connect: &api.ConsulConnect{
-							Native: "foo",
+							Native: true,
 						},
 					}},
 				}},

jobspec/test-fixtures/tg-service-connect-native.hcl

@@ -2,9 +2,10 @@ job "connect_native_service" {
   group "group" {
     service {
       name = "example"
+      task = "task1"
 
       connect {
-        native = "foo"
+        native = true
       }
     }
   }

nomad/job_endpoint_hook_connect.go

@@ -155,7 +155,7 @@ func groupConnectHook(job *structs.Job, g *structs.TaskGroup) error {
 			// create a port for the sidecar task's proxy port
 			makePort(fmt.Sprintf("%s-%s", structs.ConnectProxyPrefix, service.Name))
 		} else if service.Connect.IsNative() {
-			nativeTaskName := service.Connect.Native
+			nativeTaskName := service.TaskName
 			if t := getNamedTaskForNativeService(g, nativeTaskName); t != nil {
 				t.Kind = structs.NewTaskKind(structs.ConnectNativePrefix, service.Name)
 			} else {
@@ -194,18 +194,42 @@ func newConnectTask(serviceName string) *structs.Task {
 func groupConnectValidate(g *structs.TaskGroup) (warnings []error, err error) {
 	for _, s := range g.Services {
 		if s.Connect.HasSidecar() {
-			if n := len(g.Networks); n != 1 {
-				return nil, fmt.Errorf("Consul Connect sidecars require exactly 1 network, found %d in group %q", n, g.Name)
+			if err := groupConnectSidecarValidate(g); err != nil {
+				return nil, err
 			}
-
-			if g.Networks[0].Mode != "bridge" {
-				return nil, fmt.Errorf("Consul Connect sidecar requires bridge network, found %q in group %q", g.Networks[0].Mode, g.Name)
+		} else if s.Connect.IsNative() {
+			if err := groupConnectNativeValidate(g, s); err != nil {
+				return nil, err
 			}
-
-			// Stopping loop, only need to do the validation once
-			break
 		}
 	}
-
 	return nil, nil
 }
+
+func groupConnectSidecarValidate(g *structs.TaskGroup) error {
+	if n := len(g.Networks); n != 1 {
+		return fmt.Errorf("Consul Connect sidecars require exactly 1 network, found %d in group %q", n, g.Name)
+	}
+
+	if g.Networks[0].Mode != "bridge" {
+		return fmt.Errorf("Consul Connect sidecar requires bridge network, found %q in group %q", g.Networks[0].Mode, g.Name)
+	}
+	return nil
+}
+
+func groupConnectNativeValidate(g *structs.TaskGroup, s *structs.Service) error {
+	// note that network mode is not enforced for connect native services
+
+	// a native service must have the task identified in the service definition.
+	if len(s.TaskName) == 0 {
+		return fmt.Errorf("Consul Connect Native service %q requires task name", s.Name)
+	}
+
+	// also make sure that task actually exists
+	for _, task := range g.Tasks {
+		if s.TaskName == task.Name {
+			return nil
+		}
+	}
+	return fmt.Errorf("Consul Connect Native service %q requires undefined task %q in group %q", s.Name, s.TaskName, g.Name)
+}

nomad/job_endpoint_hook_connect_test.go

@@ -10,7 +10,9 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func Test_isSidecarForService(t *testing.T) {
+func TestJobEndpointConnect_isSidecarForService(t *testing.T) {
+	t.Parallel()
+
 	cases := []struct {
 		t *structs.Task // task
 		s string        // service
@@ -49,7 +51,9 @@ func Test_isSidecarForService(t *testing.T) {
 	}
 }
 
-func Test_groupConnectHook(t *testing.T) {
+func TestJobEndpointConnect_groupConnectHook(t *testing.T) {
+	t.Parallel()
+
 	// Test that connect-proxy task is inserted for backend service
 	job := mock.Job()
 	job.TaskGroups[0] = &structs.TaskGroup{
@@ -110,7 +114,7 @@ func Test_groupConnectHook(t *testing.T) {
 // the service name is interpolated *before the task is created.
 //
 // See https://github.com/hashicorp/nomad/issues/6853
-func TestJobEndpoint_ConnectInterpolation(t *testing.T) {
+func TestJobEndpointConnect_ConnectInterpolation(t *testing.T) {
 	t.Parallel()
 
 	server := &Server{logger: testlog.HCLogger(t)}
@@ -125,3 +129,60 @@ func TestJobEndpoint_ConnectInterpolation(t *testing.T) {
 	require.Len(t, j.TaskGroups[0].Tasks, 2)
 	require.Equal(t, "connect-proxy-my-job-api", j.TaskGroups[0].Tasks[1].Name)
 }
+
+func TestJobEndpointConnect_groupConnectSidecarValidate(t *testing.T) {
+	t.Run("sidecar 0 networks", func(t *testing.T) {
+		require.EqualError(t, groupConnectSidecarValidate(&structs.TaskGroup{
+			Name:     "g1",
+			Networks: nil,
+		}), `Consul Connect sidecars require exactly 1 network, found 0 in group "g1"`)
+	})
+
+	t.Run("sidecar non bridge", func(t *testing.T) {
+		require.EqualError(t, groupConnectSidecarValidate(&structs.TaskGroup{
+			Name: "g2",
+			Networks: structs.Networks{{
+				Mode: "host",
+			}},
+		}), `Consul Connect sidecar requires bridge network, found "host" in group "g2"`)
+	})
+
+	t.Run("sidecar okay", func(t *testing.T) {
+		require.NoError(t, groupConnectSidecarValidate(&structs.TaskGroup{
+			Name: "g3",
+			Networks: structs.Networks{{
+				Mode: "bridge",
+			}},
+		}))
+	})
+}
+
+func TestJobEndpointConnect_groupConnectNativeValidate(t *testing.T) {
+	t.Run("no task in service", func(t *testing.T) {
+		require.EqualError(t, groupConnectNativeValidate(&structs.TaskGroup{
+			Name: "g1",
+		}, &structs.Service{
+			Name:     "s1",
+			TaskName: "",
+		}), `Consul Connect Native service "s1" requires task name`)
+	})
+
+	t.Run("no task for service", func(t *testing.T) {
+		require.EqualError(t, groupConnectNativeValidate(&structs.TaskGroup{
+			Name: "g2",
+		}, &structs.Service{
+			Name:     "s2",
+			TaskName: "t1",
+		}), `Consul Connect Native service "s2" requires undefined task "t1" in group "g2"`)
+	})
+
+	t.Run("native okay", func(t *testing.T) {
+		require.NoError(t, groupConnectNativeValidate(&structs.TaskGroup{
+			Name:  "g2",
+			Tasks: []*structs.Task{{Name: "t0"}, {Name: "t1"}, {Name: "t3"}},
+		}, &structs.Service{
+			Name:     "s2",
+			TaskName: "t1",
+		}))
+	})
+}