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

consul/connect: fix bug where ingress gateways could not use wildcard services #10457

Merged
merged 1 commit into from
Apr 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ BUG FIXES:
* cli: Remove extra linefeeds in monitor.log files written by `nomad operator debug`. [[GH-10252](https://github.com/hashicorp/nomad/issues/10252)]
* client: Fixed log formatting when killing tasks. [[GH-10135](https://github.com/hashicorp/nomad/issues/10135)]
* client: Fixed a bug where small files would be assigned the wrong content type. [[GH-10348](https://github.com/hashicorp/nomad/pull/10348)]
* consul/connect: Fixed a bug where HTTP ingress gateways could not use wildcard names. [[GH-10457](https://github.com/hashicorp/nomad/pull/10457)]
* csi: Fixed a bug where volume with IDs that are a substring prefix of another volume could use the wrong volume for feasibility checking. [[GH-10158](https://github.com/hashicorp/nomad/issues/10158)]
* scheduler: Fixed a bug where Nomad reports negative or incorrect running children counts for periodic jobs. [[GH-10145](https://github.com/hashicorp/nomad/issues/10145)]
* scheduler: Fixed a bug where jobs requesting multiple CSI volumes could be incorrectly scheduled if only one of the volumes passed feasibility checking. [[GH-10143](https://github.com/hashicorp/nomad/issues/10143)]
Expand Down
27 changes: 22 additions & 5 deletions nomad/structs/services.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package structs

import (
"crypto/sha1"
"errors"
"fmt"
"hash"
"io"
Expand Down Expand Up @@ -1645,13 +1646,29 @@ func (s *ConsulIngressService) Validate(isHTTP bool) error {
}

if s.Name == "" {
return fmt.Errorf("Consul Ingress Service requires a name")
return errors.New("Consul Ingress Service requires a name")
}

if isHTTP && len(s.Hosts) == 0 {
return fmt.Errorf("Consul Ingress Service requires one or more hosts when using HTTP protocol")
} else if !isHTTP && len(s.Hosts) > 0 {
return fmt.Errorf("Consul Ingress Service supports hosts only when using HTTP protocol")
// Validation of wildcard service name and hosts varies on whether the protocol
// for the gateway is HTTP.
// https://www.consul.io/docs/connect/config-entries/ingress-gateway#hosts
switch isHTTP {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This switch feels like we're longing for better pattern matching in go, but 🤷‍♂️

case true:
if s.Name == "*" {
return nil
}

if len(s.Hosts) == 0 {
return errors.New("Consul Ingress Service requires one or more hosts when using HTTP protocol")
}
case false:
if s.Name == "*" {
return errors.New("Consul Ingress Service supports wildcard names only with HTTP protocol")
}

if len(s.Hosts) > 0 {
return errors.New("Consul Ingress Service supports hosts only when using HTTP protocol")
}
}

return nil
Expand Down
14 changes: 14 additions & 0 deletions nomad/structs/services_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1106,6 +1106,20 @@ func TestConsulIngressService_Validate(t *testing.T) {
}).Validate(true)
require.NoError(t, err)
})

t.Run("http with wildcard service", func(t *testing.T) {
err := (&ConsulIngressService{
Name: "*",
}).Validate(true)
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")
})
}

func TestConsulIngressListener_Validate(t *testing.T) {
Expand Down