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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

connect: update allowed protocols in ingress gateway config #11187

Merged
merged 2 commits into from
Sep 16, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 15 additions & 15 deletions nomad/structs/services.go
Original file line number Diff line number Diff line change
Expand Up @@ -1728,7 +1728,7 @@ func (s *ConsulIngressService) Equals(o *ConsulIngressService) bool {
return helper.CompareSliceSetString(s.Hosts, o.Hosts)
}

func (s *ConsulIngressService) Validate(isHTTP bool) error {
func (s *ConsulIngressService) Validate(protocol string) error {
if s == nil {
return nil
}
Expand All @@ -1737,25 +1737,25 @@ func (s *ConsulIngressService) Validate(isHTTP bool) error {
return errors.New("Consul Ingress Service requires a name")
}

// Validation of wildcard service name and hosts varies on whether the protocol
// for the gateway is HTTP.
// 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 isHTTP {
case true:
switch protocol {
case "tcp":
if s.Name == "*" {
return nil
return errors.New(`Consul Ingress Service doesn't support wildcard name for "tcp" protocol`)
}

if len(s.Hosts) == 0 {
return errors.New("Consul Ingress Service requires one or more hosts when using HTTP protocol")
if len(s.Hosts) != 0 {
return errors.New(`Consul Ingress Service doesn't support associating hosts to a service for the "tcp" protocol`)
}
case false:
default:
if s.Name == "*" {
return errors.New("Consul Ingress Service supports wildcard names only with HTTP protocol")
return nil
}

if len(s.Hosts) > 0 {
return errors.New("Consul Ingress Service supports hosts only when using HTTP protocol")
if len(s.Hosts) == 0 {
return fmt.Errorf("Consul Ingress Service requires one or more hosts when using %q protocol", protocol)
}
}

Expand Down Expand Up @@ -1815,17 +1815,17 @@ func (l *ConsulIngressListener) Validate() error {
return fmt.Errorf("Consul Ingress Listener requires valid Port")
}

protocols := []string{"http", "tcp"}
protocols := []string{"tcp", "http", "http2", "grpc"}
if !helper.SliceStringContains(protocols, l.Protocol) {
return fmt.Errorf(`Consul Ingress Listener requires protocol of "http" or "tcp", got %q`, l.Protocol)
return fmt.Errorf(`Consul Ingress Listener requires protocol of %s, got %q`, strings.Join(protocols, ", "), l.Protocol)
}

if len(l.Services) == 0 {
return fmt.Errorf("Consul Ingress Listener requires one or more services")
}

for _, service := range l.Services {
if err := service.Validate(l.Protocol == "http"); err != nil {
if err := service.Validate(l.Protocol); err != nil {
return err
}
}
Expand Down
22 changes: 11 additions & 11 deletions nomad/structs/services_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1122,52 +1122,52 @@ func TestConsulIngressService_Validate(t *testing.T) {
t.Run("invalid name", func(t *testing.T) {
err := (&ConsulIngressService{
Name: "",
}).Validate(true)
}).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(true)
require.EqualError(t, err, "Consul Ingress Service requires one or more hosts when using HTTP protocol")
}).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",
Hosts: []string{"host1"},
}).Validate(false)
require.EqualError(t, err, "Consul Ingress Service supports hosts only when using HTTP protocol")
}).Validate("tcp")
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) {
err := (&ConsulIngressService{
Name: "service1",
}).Validate(false)
}).Validate("tcp")
require.NoError(t, err)
})

t.Run("ok http", func(t *testing.T) {
err := (&ConsulIngressService{
Name: "service1",
Hosts: []string{"host1"},
}).Validate(true)
}).Validate("http")
require.NoError(t, err)
})

t.Run("http with wildcard service", func(t *testing.T) {
err := (&ConsulIngressService{
Name: "*",
}).Validate(true)
}).Validate("http")
require.NoError(t, err)
})

t.Run("tcp with wildcard service", func(t *testing.T) {
err := (&ConsulIngressService{
Name: "*",
}).Validate(false)
require.EqualError(t, err, "Consul Ingress Service supports wildcard names only with HTTP protocol")
}).Validate("tcp")
require.EqualError(t, err, `Consul Ingress Service doesn't support wildcard name for "tcp" protocol`)
})
}

Expand All @@ -1193,7 +1193,7 @@ func TestConsulIngressListener_Validate(t *testing.T) {
Name: "service1",
}},
}).Validate()
require.EqualError(t, err, `Consul Ingress Listener requires protocol of "http" or "tcp", got "gopher"`)
require.EqualError(t, err, `Consul Ingress Listener requires protocol of tcp, http, http2, grpc, got "gopher"`)
})

t.Run("no services", func(t *testing.T) {
Expand Down