From 98e51e0648f08339787a6d349e5ce4edd7f20493 Mon Sep 17 00:00:00 2001 From: Daniel Bennett Date: Tue, 10 Jan 2023 17:52:30 +0000 Subject: [PATCH] backport of commit be19d7c5ee0000ec9b0a054edf92e840bfe8bfd5 --- nomad/structs/services.go | 14 +++------ nomad/structs/services_test.go | 57 ++++++++++++++++++++-------------- 2 files changed, 39 insertions(+), 32 deletions(-) diff --git a/nomad/structs/services.go b/nomad/structs/services.go index 9ada78e908a..d452ec5dab0 100644 --- a/nomad/structs/services.go +++ b/nomad/structs/services.go @@ -1728,13 +1728,13 @@ func (s *ConsulIngressService) Validate(protocol string) error { return nil } + // pre-validate service Name and Hosts before passing along to consul: + // https://developer.hashicorp.com/consul/docs/connect/config-entries/ingress-gateway#services + if s.Name == "" { return errors.New("Consul Ingress Service requires a name") } - // Validation of wildcard service name and hosts varies depending on the - // protocol for the gateway. - // https://www.consul.io/docs/connect/config-entries/ingress-gateway#hosts switch protocol { case "tcp": if s.Name == "*" { @@ -1745,12 +1745,8 @@ func (s *ConsulIngressService) Validate(protocol string) error { return errors.New(`Consul Ingress Service doesn't support associating hosts to a service for the "tcp" protocol`) } default: - if s.Name == "*" { - return nil - } - - if len(s.Hosts) == 0 { - return fmt.Errorf("Consul Ingress Service requires one or more hosts when using %q protocol", protocol) + if s.Name == "*" && len(s.Hosts) != 0 { + return errors.New(`Consul Ingress Service with a wildcard "*" service name can not also specify hosts`) } } diff --git a/nomad/structs/services_test.go b/nomad/structs/services_test.go index 4887de23c46..77c86459b01 100644 --- a/nomad/structs/services_test.go +++ b/nomad/structs/services_test.go @@ -1146,13 +1146,6 @@ func TestConsulIngressService_Validate(t *testing.T) { require.EqualError(t, err, "Consul Ingress Service requires a name") }) - t.Run("http missing hosts", func(t *testing.T) { - err := (&ConsulIngressService{ - Name: "service1", - }).Validate("http") - require.EqualError(t, err, `Consul Ingress Service requires one or more hosts when using "http" protocol`) - }) - t.Run("tcp extraneous hosts", func(t *testing.T) { err := (&ConsulIngressService{ Name: "service1", @@ -1161,34 +1154,52 @@ func TestConsulIngressService_Validate(t *testing.T) { require.EqualError(t, err, `Consul Ingress Service doesn't support associating hosts to a service for the "tcp" protocol`) }) - t.Run("ok tcp", func(t *testing.T) { + t.Run("tcp ok", func(t *testing.T) { err := (&ConsulIngressService{ Name: "service1", }).Validate("tcp") require.NoError(t, err) }) - t.Run("ok http", func(t *testing.T) { - err := (&ConsulIngressService{ - Name: "service1", - Hosts: []string{"host1"}, - }).Validate("http") - require.NoError(t, err) - }) - - t.Run("http with wildcard service", func(t *testing.T) { - err := (&ConsulIngressService{ - Name: "*", - }).Validate("http") - require.NoError(t, err) - }) - t.Run("tcp with wildcard service", func(t *testing.T) { err := (&ConsulIngressService{ Name: "*", }).Validate("tcp") require.EqualError(t, err, `Consul Ingress Service doesn't support wildcard name for "tcp" protocol`) }) + + // non-"tcp" protocols should be all treated the same. + for _, proto := range []string{"http", "http2", "grpc"} { + t.Run(proto+" ok", func(t *testing.T) { + err := (&ConsulIngressService{ + Name: "service1", + Hosts: []string{"host1"}, + }).Validate(proto) + require.NoError(t, err) + }) + + t.Run(proto+" without hosts", func(t *testing.T) { + err := (&ConsulIngressService{ + Name: "service1", + }).Validate(proto) + require.NoErrorf(t, err, `should not require hosts with "%s" protocol`, proto) + }) + + t.Run(proto+" wildcard service", func(t *testing.T) { + err := (&ConsulIngressService{ + Name: "*", + }).Validate(proto) + require.NoErrorf(t, err, `should allow wildcard hosts with "%s" protocol`, proto) + }) + + t.Run(proto+" wildcard service and host", func(t *testing.T) { + err := (&ConsulIngressService{ + Name: "*", + Hosts: []string{"any"}, + }).Validate(proto) + require.EqualError(t, err, `Consul Ingress Service with a wildcard "*" service name can not also specify hosts`) + }) + } } func TestConsulIngressListener_Validate(t *testing.T) {