Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

connect: ingress gateway validation for http hosts and wildcards #15749

Merged
merged 3 commits into from
Jan 11, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/15749.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
connect: ingress http/2/grpc listeners may exclude hosts
```
14 changes: 5 additions & 9 deletions nomad/structs/services.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 == "*" {
Expand All @@ -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`)
}
}

Expand Down
57 changes: 34 additions & 23 deletions nomad/structs/services_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1328,13 +1328,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",
Expand All @@ -1343,34 +1336,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)
gulducat marked this conversation as resolved.
Show resolved Hide resolved
})

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) {
Expand Down