More code review feedback

command/agent/job_endpoint.go

@@ -805,7 +805,7 @@ func ApiTaskToStructsTask(apiTask *api.Task, structsTask *structs.Task) {
 	structsTask.KillTimeout = *apiTask.KillTimeout
 	structsTask.ShutdownDelay = apiTask.ShutdownDelay
 	structsTask.KillSignal = apiTask.KillSignal
-	structsTask.Kind = structs.Kind(apiTask.Kind)
+	structsTask.Kind = structs.TaskKind(apiTask.Kind)
 	structsTask.Constraints = ApiConstraintsToStructs(apiTask.Constraints)
 	structsTask.Affinities = ApiAffinitiesToStructs(apiTask.Affinities)
 

nomad/structs/structs.go

@@ -5202,9 +5202,9 @@ type Task struct {
 	// specification and defaults to SIGINT
 	KillSignal string
 
-	// Used internally to manage tasks according to their Kind. Initial use case
+	// Used internally to manage tasks according to their TaskKind. Initial use case
 	// is for Consul Connect
-	Kind Kind
+	Kind TaskKind
 }
 
 func (t *Task) Copy() *Task {
@@ -5410,8 +5410,7 @@ func (t *Task) Validate(ephemeralDisk *EphemeralDisk, jobType string, tgServices
 		}
 	}
 
-	// Validation for Kind field which is used for Consul Connect integration
-	// TODO better wording for all error messages
+	// Validation for TaskKind field which is used for Consul Connect integration
 	taskKind := t.Kind
 	if taskKind.IsConnect() {
 		// This task is a Connect proxy so it should not have service stanzas
@@ -5421,7 +5420,7 @@ func (t *Task) Validate(ephemeralDisk *EphemeralDisk, jobType string, tgServices
 		if t.Leader {
 			mErr.Errors = append(mErr.Errors, fmt.Errorf("Connect proxy task must not have leader set"))
 		}
-		serviceErr := taskKind.ValidateService(tgServices)
+		serviceErr := taskKind.validateProxyService(tgServices)
 		if serviceErr != nil {
 			mErr.Errors = append(mErr.Errors, serviceErr)
 		}
@@ -5569,20 +5568,23 @@ func (t *Task) Warnings() error {
 	return mErr.ErrorOrNil()
 }
 
-type Kind string
+type TaskKind string
 
-const connect_prefix = "connect:"
+const connectPrefix = "connect-proxy:"
 
-func (k Kind) IsConnect() bool {
-	return strings.HasPrefix(string(k), connect_prefix) && len(k) > len(connect_prefix)
+func (k TaskKind) IsConnect() bool {
+	return strings.HasPrefix(string(k), connectPrefix) && len(k) > len(connectPrefix)
 }
 
-func (k Kind) ValidateService(tgServices []*Service) error {
+// validateProxyService checks that the service that is being
+// proxied by this task exists in the task group and contains
+// valid Connect config.
+func (k TaskKind) validateProxyService(tgServices []*Service) error {
 	var mErr multierror.Error
 	parts := strings.Split(string(k), ":")
 	serviceName := strings.Join(parts[1:], "")
 	if len(parts) > 2 {
-		mErr.Errors = append(mErr.Errors, fmt.Errorf("Connect proxy service kind %q should not contain `:`", k))
+		mErr.Errors = append(mErr.Errors, fmt.Errorf("Connect proxy service kind %q must not contain `:`", k))
 	}
 
 	found := false

nomad/structs/structs_test.go

@@ -1515,7 +1515,7 @@ func TestTask_Validate_Service_Check_CheckRestart(t *testing.T) {
 
 func TestTask_Validate_ConnectKind(t *testing.T) {
 	ephemeralDisk := DefaultEphemeralDisk()
-	getTask := func(kind Kind, leader bool) *Task {
+	getTask := func(kind TaskKind, leader bool) *Task {
 		task := &Task{
 			Name:      "web",
 			Driver:    "docker",
@@ -1540,7 +1540,7 @@ func TestTask_Validate_ConnectKind(t *testing.T) {
 
 	cases := []struct {
 		Desc        string
-		Kind        Kind
+		Kind        TaskKind
 		Leader      bool
 		Service     *Service
 		TgService   []*Service
@@ -1552,15 +1552,15 @@ func TestTask_Validate_ConnectKind(t *testing.T) {
 		},
 		{
 			Desc: "Invalid because of service in task definition",
-			Kind: "connect:redis",
+			Kind: "connect-proxy:redis",
 			Service: &Service{
 				Name: "redis",
 			},
-			ErrContains: "Connect proxy task should not have service stanza in it",
+			ErrContains: "Connect proxy task must not have a service stanza",
 		},
 		{
 			Desc:   "Leader should not be set",
-			Kind:   "connect:redis",
+			Kind:   "connect-proxy:redis",
 			Leader: true,
 			Service: &Service{
 				Name: "redis",
@@ -1569,20 +1569,20 @@ func TestTask_Validate_ConnectKind(t *testing.T) {
 		},
 		{
 			Desc: "Service name invalid",
-			Kind: "connect:redis:test",
+			Kind: "connect-proxy:redis:test",
 			Service: &Service{
 				Name: "redis",
 			},
-			ErrContains: "Connect proxy service kind \"connect:redis:test\" should not contain `:`",
+			ErrContains: "Connect proxy service kind \"connect-proxy:redis:test\" must not contain `:`",
 		},
 		{
 			Desc:        "Service name not found in group",
-			Kind:        "connect:redis",
+			Kind:        "connect-proxy:redis",
 			ErrContains: "Connect proxy service name not found in services from task group",
 		},
 		{
 			Desc: "Connect stanza not configured in group",
-			Kind: "connect:redis",
+			Kind: "connect-proxy:redis",
 			TgService: []*Service{{
 				Name: "redis",
 			}},
@@ -1590,7 +1590,7 @@ func TestTask_Validate_ConnectKind(t *testing.T) {
 		},
 		{
 			Desc: "Valid connect proxy kind",
-			Kind: "connect:redis",
+			Kind: "connect-proxy:redis",
 			TgService: []*Service{{
 				Name: "redis",
 				Connect: &ConsulConnect{