diff --git a/CHANGELOG.md b/CHANGELOG.md index e18469b09de..e3ad7718a4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ IMPROVEMENTS: * consul: Allow setting `body` field on service/check Consul health checks. [[GH-10186](https://github.com/hashicorp/nomad/issues/10186)] * consul/connect: Use exponential backoff for consul envoy bootstrap process [[GH-10453](https://github.com/hashicorp/nomad/pull/10453)] * 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)] diff --git a/nomad/job_endpoint_hook_connect.go b/nomad/job_endpoint_hook_connect.go index 1e215f6196a..560a993f863 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" @@ -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,7 +461,7 @@ 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) } @@ -468,6 +469,19 @@ func groupConnectSidecarValidate(g *structs.TaskGroup) error { 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 } diff --git a/nomad/job_endpoint_hook_connect_test.go b/nomad/job_endpoint_hook_connect_test.go index 4b197aedce8..6d37c238f95 100644 --- a/nomad/job_endpoint_hook_connect_test.go +++ b/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) }) }