Skip to content

Commit

Permalink
connect: fix non-"tcp" ingress gateway validation
Browse files Browse the repository at this point in the history
changes apply to http, http2, and grpc:
* if "hosts" is excluded, consul will use its default domain
  e.g. <service-name>.ingress.dc1.consul
* can't set hosts with "*" service name
* test http2 and grpc too
  • Loading branch information
gulducat committed Jan 10, 2023
1 parent cab35b3 commit be19d7c
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 32 deletions.
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)
})

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

0 comments on commit be19d7c

Please sign in to comment.