luxolus
/
open-nomad
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

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

* connect: fix non-"tcp" ingress gateway validation

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
This commit is contained in:
Daniel Bennett 2023-01-11 11:52:32 -06:00 committed by GitHub
parent 719eee8112
commit 7d1059b5ae
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 46 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
.changelog/15749.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:bug
connect: ingress http/2/grpc listeners may exclude hosts
```

14
nomad/structs/services.go
View File

@ -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`)
}
}

65
nomad/structs/services_test.go
View File

@ -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) {