diff --git a/.changelog/15749.txt b/.changelog/15749.txt new file mode 100644 index 000000000..f8dfda285 --- /dev/null +++ b/.changelog/15749.txt @@ -0,0 +1,3 @@ +```release-note:bug +connect: ingress http/2/grpc listeners may exclude hosts +``` diff --git a/nomad/structs/services.go b/nomad/structs/services.go index e6899af75..33f4b056a 100644 --- a/nomad/structs/services.go +++ b/nomad/structs/services.go @@ -1874,13 +1874,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 == "*" { @@ -1891,12 +1891,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 9f81bd744..b43def12a 100644 --- a/nomad/structs/services_test.go +++ b/nomad/structs/services_test.go @@ -1325,14 +1325,7 @@ func TestConsulIngressService_Validate(t *testing.T) { err := (&ConsulIngressService{ Name: "", }).Validate("http") - 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`) + must.EqError(t, err, "Consul Ingress Service requires a name") }) t.Run("tcp extraneous hosts", func(t *testing.T) { @@ -1340,37 +1333,55 @@ func TestConsulIngressService_Validate(t *testing.T) { Name: "service1", Hosts: []string{"host1"}, }).Validate("tcp") - require.EqualError(t, err, `Consul Ingress Service doesn't support associating hosts to a service for the "tcp" protocol`) + must.EqError(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) + must.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`) + must.EqError(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) + must.NoError(t, err) + }) + + t.Run(proto+" without hosts", func(t *testing.T) { + err := (&ConsulIngressService{ + Name: "service1", + }).Validate(proto) + must.NoError(t, err, must.Sprintf(`"%s" protocol should not require hosts`, proto)) + }) + + t.Run(proto+" wildcard service", func(t *testing.T) { + err := (&ConsulIngressService{ + Name: "*", + }).Validate(proto) + must.NoError(t, err, must.Sprintf(`"%s" protocol should allow wildcard service`, proto)) + }) + + t.Run(proto+" wildcard service and host", func(t *testing.T) { + err := (&ConsulIngressService{ + Name: "*", + Hosts: []string{"any"}, + }).Validate(proto) + must.EqError(t, err, `Consul Ingress Service with a wildcard "*" service name can not also specify hosts`) + }) + } } func TestConsulIngressListener_Validate(t *testing.T) {