consul/connect: check connect group and service names for uppercase c…

…haracters This PR adds job-submission validation that checks for the use of uppercase characters in group and service names for services that make use of Consul Connect. This prevents attempting to launch services that Consul will not validate correctly, which in turn causes tasks to fail to launch in Nomad. Underlying Consul issue: hashicorp/consul#6765 Closes #7581 #10450
hashicorp · Apr 27, 2021 · 27b0add · 27b0add
1 parent fb5e898
commit 27b0add
Show file tree

Hide file tree

Showing 3 changed files with 83 additions and 10 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -18,6 +18,7 @@ IMPROVEMENTS:
  * client/fingerprint: Added support multiple host network aliases for the same interface. [[GH-10104](https://github.com/hashicorp/nomad/issues/10104)]
  * consul: Allow setting `body` field on service/check Consul health checks. [[GH-10186](https://github.com/hashicorp/nomad/issues/10186)]
  * consul/connect: Enable setting `local_bind_address` field on connect upstreams [[GH-6248](https://github.com/hashicorp/nomad/issues/6248)]
+ * consul/connect: Added job-submission validation for Connect service and group names [[GH-10455](https://github.com/hashicorp/nomad/pull/10455)]
  * consul/connect: Automatically populate `CONSUL_HTTP_ADDR` for connect native tasks in host networking mode. [[GH-10239](https://github.com/hashicorp/nomad/issues/10239)]
  * csi: Added support for jobs to request a unique volume ID per allocation. [[GH-10136](https://github.com/hashicorp/nomad/issues/10136)]
  * driver/docker: Added support for optional extra container labels. [[GH-9885](https://github.com/hashicorp/nomad/issues/9885)]

diff --git a/nomad/job_endpoint_hook_connect.go b/nomad/job_endpoint_hook_connect.go
@@ -2,6 +2,7 @@ package nomad
 
 import (
 	"fmt"
+	"strings"
 	"time"
 
 	"github.com/hashicorp/nomad/client/taskenv"
@@ -442,18 +443,28 @@ func newConnectSidecarTask(service string) *structs.Task {
 
 func groupConnectValidate(g *structs.TaskGroup) (warnings []error, err error) {
 	for _, s := range g.Services {
-		switch {
-		case s.Connect.HasSidecar():
-			if err := groupConnectSidecarValidate(g); err != nil {
-				return nil, err
+		if s.Connect != nil {
+			if s.Name != strings.ToLower(s.Name) {
+				return nil, fmt.Errorf("Consul Connect service name %q in group %q must not contain uppercase characters", s.Name, g.Name)
 			}
-		case s.Connect.IsNative():
-			if err := groupConnectNativeValidate(g, s); err != nil {
-				return nil, err
+
+			if g.Name != strings.ToLower(g.Name) {
+				return nil, fmt.Errorf("Consul Connect group %q with service %q must not contain uppercase characters", g.Name, s.Name)
 			}
-		case s.Connect.IsGateway():
-			if err := groupConnectGatewayValidate(g); err != nil {
-				return nil, err
+
+			switch {
+			case s.Connect.HasSidecar():
+				if err := groupConnectSidecarValidate(g); err != nil {
+					return nil, err
+				}
+			case s.Connect.IsNative():
+				if err := groupConnectNativeValidate(g, s); err != nil {
+					return nil, err
+				}
+			case s.Connect.IsGateway():
+				if err := groupConnectGatewayValidate(g); err != nil {
+					return nil, err
+				}
 			}
 		}
 	}

diff --git a/nomad/job_endpoint_hook_connect_test.go b/nomad/job_endpoint_hook_connect_test.go
@@ -610,3 +610,64 @@ func TestJobEndpointConnect_gatewayProxyForBridge(t *testing.T) {
 		//
 	})
 }
+
+func TestJobEndpointConnect_groupConnectValidate(t *testing.T) {
+	t.Run("non-connect service contains uppercase characters", func(t *testing.T) {
+		_, err := groupConnectValidate(&structs.TaskGroup{
+			Name: "group",
+			Services: []*structs.Service{{
+				Name: "Other-Service",
+			}},
+		})
+		require.NoError(t, err)
+	})
+
+	t.Run("connect service contains uppercase characters", func(t *testing.T) {
+		_, err := groupConnectValidate(&structs.TaskGroup{
+			Name: "group",
+			Services: []*structs.Service{{
+				Name: "Other-Service",
+			}, {
+				Name:    "Connect-Service",
+				Connect: new(structs.ConsulConnect),
+			}},
+		})
+		require.EqualError(t, err, `Consul Connect service name "Connect-Service" in group "group" must not contain uppercase characters`)
+	})
+
+	t.Run("non-connect group contains uppercase characters", func(t *testing.T) {
+		_, err := groupConnectValidate(&structs.TaskGroup{
+			Name: "Other-Group",
+			Services: []*structs.Service{{
+				Name: "other-service",
+			}},
+		})
+		require.NoError(t, err)
+	})
+
+	t.Run("connect-group contains uppercase characters", func(t *testing.T) {
+		_, err := groupConnectValidate(&structs.TaskGroup{
+			Name: "Connect-Group",
+			Services: []*structs.Service{{
+				Name: "other-service",
+			}, {
+				Name:    "connect-service",
+				Connect: new(structs.ConsulConnect),
+			}},
+		})
+		require.EqualError(t, err, `Consul Connect group "Connect-Group" with service "connect-service" must not contain uppercase characters`)
+	})
+
+	t.Run("connect group and service lowercase", func(t *testing.T) {
+		_, err := groupConnectValidate(&structs.TaskGroup{
+			Name: "connect-group",
+			Services: []*structs.Service{{
+				Name: "other-service",
+			}, {
+				Name:    "connect-service",
+				Connect: new(structs.ConsulConnect),
+			}},
+		})
+		require.NoError(t, err)
+	})
+}