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

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 sidecar 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)]

nomad/job_endpoint_hook_connect.go

@@ -2,6 +2,7 @@ package nomad
 
 import (
 	"fmt"
+	"strings"
 	"time"
 
 	"github.com/hashicorp/nomad/client/taskenv"
@@ -444,7 +445,7 @@ 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 {
+			if err := groupConnectSidecarValidate(g, s); err != nil {
 				return nil, err
 			}
 		case s.Connect.IsNative():
@@ -460,14 +461,27 @@ func groupConnectValidate(g *structs.TaskGroup) (warnings []error, err error) {
 	return nil, nil
 }
 
-func groupConnectSidecarValidate(g *structs.TaskGroup) error {
+func groupConnectSidecarValidate(g *structs.TaskGroup, s *structs.Service) 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)
 	}
+
+	// We must enforce lowercase characters on group and service names for connect
+	// sidecar proxies, because Consul assumes this invariant without validating it.
+	// https://github.com/hashicorp/consul/blob/v1.9.5/command/connect/proxy/proxy.go#L235
+
+	if s.Name != strings.ToLower(s.Name) {
+		return fmt.Errorf("Consul Connect service name %q in group %q must not contain uppercase characters", s.Name, g.Name)
+	}
+
+	if g.Name != strings.ToLower(g.Name) {
+		return fmt.Errorf("Consul Connect group %q with service %q must not contain uppercase characters", g.Name, s.Name)
+	}
+
 	return nil
 }
 

nomad/job_endpoint_hook_connect_test.go

@@ -239,11 +239,19 @@ func TestJobEndpointConnect_ConnectInterpolation(t *testing.T) {
 func TestJobEndpointConnect_groupConnectSidecarValidate(t *testing.T) {
 	t.Parallel()
 
+	// network validation
+
+	makeService := func(name string) *structs.Service {
+		return &structs.Service{Name: name, Connect: &structs.ConsulConnect{
+			SidecarService: new(structs.ConsulSidecarService),
+		}}
+	}
+
 	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"`)
+		}, makeService("connect-service")), `Consul Connect sidecars require exactly 1 network, found 0 in group "g1"`)
 	})
 
 	t.Run("sidecar non bridge", func(t *testing.T) {
@@ -252,7 +260,7 @@ func TestJobEndpointConnect_groupConnectSidecarValidate(t *testing.T) {
 			Networks: structs.Networks{{
 				Mode: "host",
 			}},
-		}), `Consul Connect sidecar requires bridge network, found "host" in group "g2"`)
+		}, makeService("connect-service")), `Consul Connect sidecar requires bridge network, found "host" in group "g2"`)
 	})
 
 	t.Run("sidecar okay", func(t *testing.T) {
@@ -261,7 +269,64 @@ func TestJobEndpointConnect_groupConnectSidecarValidate(t *testing.T) {
 			Networks: structs.Networks{{
 				Mode: "bridge",
 			}},
-		}))
+		}, makeService("connect-service")))
+	})
+
+	// group and service name validation
+
+	t.Run("non-connect service contains uppercase characters", func(t *testing.T) {
+		_, err := groupConnectValidate(&structs.TaskGroup{
+			Name:     "group",
+			Networks: structs.Networks{{Mode: "bridge"}},
+			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",
+			Networks: structs.Networks{{Mode: "bridge"}},
+			Services: []*structs.Service{{
+				Name: "Other-Service",
+			}, makeService("Connect-Service")},
+		})
+		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",
+			Networks: structs.Networks{{Mode: "bridge"}},
+			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",
+			Networks: structs.Networks{{Mode: "bridge"}},
+			Services: []*structs.Service{{
+				Name: "other-service",
+			}, makeService("connect-service")},
+		})
+		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",
+			Networks: structs.Networks{{Mode: "bridge"}},
+			Services: []*structs.Service{{
+				Name: "other-service",
+			}, makeService("connect-service")},
+		})
+		require.NoError(t, err)
 	})
 }