From 27b0add29332ff30301c9d93e1f3d7266be7fe9a Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Tue, 27 Apr 2021 10:23:57 -0600 Subject: [PATCH] consul/connect: check connect group and service names for uppercase characters 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: https://github.com/hashicorp/consul/issues/6765 Closes #7581 #10450 --- CHANGELOG.md | 1 + nomad/job_endpoint_hook_connect.go | 31 +++++++++---- nomad/job_endpoint_hook_connect_test.go | 61 +++++++++++++++++++++++++ 3 files changed, 83 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d8b505b0d15b..ae99ab011ca6 100644 --- 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 index 1e215f6196aa..3ffe07102196 100644 --- 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 index 4b197aedce81..b36b1dc6b012 100644 --- 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) + }) +}