From da7525d9f7341d8bd48afca0f0eb084f84ca0260 Mon Sep 17 00:00:00 2001
From: Luiz Aoqui <luiz@hashicorp.com>
Date: Wed, 13 Sep 2023 17:58:34 -0300
Subject: [PATCH] consul: check for warnings on service identity (#18466)

Apply workload identity warnings to group and task level Consul services
that have an identity assigned.
---
 nomad/job_endpoint_hooks_test.go | 29 +++++++++++++++++++++++++++--
 nomad/structs/services.go        | 14 ++++++++++++++
 nomad/structs/structs.go         | 22 +++++++++++++++++++---
 3 files changed, 60 insertions(+), 5 deletions(-)

diff --git a/nomad/job_endpoint_hooks_test.go b/nomad/job_endpoint_hooks_test.go
index 579b00413..825810ead 100644
--- a/nomad/job_endpoint_hooks_test.go
+++ b/nomad/job_endpoint_hooks_test.go
@@ -51,6 +51,7 @@ func Test_jobValidate_Validate_consul_service(t *testing.T) {
 					UseIdentity: pointer.Of(true),
 					ServiceIdentity: &config.WorkloadIdentityConfig{
 						Audience: []string{"consul.io"},
+						TTL:      pointer.Of(time.Hour),
 					},
 				},
 			},
@@ -62,7 +63,27 @@ func Test_jobValidate_Validate_consul_service(t *testing.T) {
 				Name:     "web",
 				Identity: &structs.WorkloadIdentity{
 					Name:        "consul-service/web",
-					Audience:    []string{"consul.io", "nomad.dev"},
+					Audience:    []string{"consul.io"},
+					File:        true,
+					Env:         false,
+					ServiceName: "web",
+					TTL:         time.Hour,
+				},
+			},
+			inputConfig: &Config{
+				ConsulConfig: &config.ConsulConfig{
+					UseIdentity: pointer.Of(true),
+				},
+			},
+		},
+		{
+			name: "warn when service identity has no TTL",
+			inputService: &structs.Service{
+				Provider: "consul",
+				Name:     "web",
+				Identity: &structs.WorkloadIdentity{
+					Name:        "consul-service/web",
+					Audience:    []string{"consul.io"},
 					File:        true,
 					Env:         false,
 					ServiceName: "web",
@@ -73,6 +94,9 @@ func Test_jobValidate_Validate_consul_service(t *testing.T) {
 					UseIdentity: pointer.Of(true),
 				},
 			},
+			expectedWarns: []string{
+				"identities without an expiration are insecure",
+			},
 		},
 		{
 			name: "error when consul identity is disabled and service has identity",
@@ -81,9 +105,10 @@ func Test_jobValidate_Validate_consul_service(t *testing.T) {
 				Name:     "web",
 				Identity: &structs.WorkloadIdentity{
 					Name:     fmt.Sprintf("%s/web", consulServiceIdentityNamePrefix),
-					Audience: []string{"consul.io", "nomad.dev"},
+					Audience: []string{"consul.io"},
 					File:     true,
 					Env:      false,
+					TTL:      time.Hour,
 				},
 			},
 			inputConfig: &Config{
diff --git a/nomad/structs/services.go b/nomad/structs/services.go
index 9cb3dfd6b..cdeeb531e 100644
--- a/nomad/structs/services.go
+++ b/nomad/structs/services.go
@@ -707,6 +707,20 @@ func (s *Service) Canonicalize(job, taskGroup, task, jobNamespace string) {
 	}
 }
 
+// Warnings returns a list of warnings that may be from dubious settings or
+// deprecation warnings.
+func (s *Service) Warnings() error {
+	var mErr *multierror.Error
+
+	if s.Identity != nil {
+		if err := s.Identity.Warnings(); err != nil {
+			mErr = multierror.Append(mErr, err)
+		}
+	}
+
+	return mErr.ErrorOrNil()
+}
+
 // Validate checks if the Service definition is valid
 func (s *Service) Validate() error {
 	var mErr multierror.Error
diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go
index 4703e56d2..77247f950 100644
--- a/nomad/structs/structs.go
+++ b/nomad/structs/structs.go
@@ -7331,10 +7331,18 @@ func (tg *TaskGroup) Warnings(j *Job) error {
 		mErr.Errors = append(mErr.Errors, fmt.Errorf("mbits has been deprecated as of Nomad 0.12.0. Please remove mbits from the network block"))
 	}
 
+	// Validate group-level services.
+	for _, s := range tg.Services {
+		if err := s.Warnings(); err != nil {
+			err = multierror.Prefix(err, fmt.Sprintf("Service %q:", s.Name))
+			mErr = *multierror.Append(&mErr, err)
+		}
+	}
+
 	for _, t := range tg.Tasks {
 		if err := t.Warnings(); err != nil {
-			err = multierror.Prefix(err, fmt.Sprintf("Task %q:", t.Name))
-			mErr.Errors = append(mErr.Errors, err)
+			outer := fmt.Errorf("Task %q has warnings: %v", t.Name, err)
+			mErr.Errors = append(mErr.Errors, outer)
 		}
 	}
 
@@ -8136,7 +8144,15 @@ func (t *Task) Warnings() error {
 	for idx, tmpl := range t.Templates {
 		if err := tmpl.Warnings(); err != nil {
 			err = multierror.Prefix(err, fmt.Sprintf("Template[%d]", idx))
-			mErr.Errors = append(mErr.Errors, err)
+			mErr = *multierror.Append(&mErr, err)
+		}
+	}
+
+	// Validate task-level services.
+	for _, s := range t.Services {
+		if err := s.Warnings(); err != nil {
+			err = multierror.Prefix(err, fmt.Sprintf("Service %q:", s.Name))
+			mErr = *multierror.Append(&mErr, err)
 		}
 	}