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

Conversation

shoenig
Copy link
Member

@shoenig shoenig commented Apr 27, 2021

This PR fixes a bug where Nomad was more restrictive on Ingress Gateway Configuration
Entry definitions than Consul. Before, Nomad would not allow for declaring IGCEs with
http listeners with service name "*", which is a special feature enabled by Consul.

Note: to make http protocol work, a service-default must be defined setting the
protocol to http for each service. (Setting protocol on the service definition in Consul is not possible)

Fixes: #9729

… services

This PR fixes a bug where Nomad was more restrictive on Ingress Gateway Configuration
Entry definitions than Consul. Before, Nomad would not allow for declaring IGCEs with
http listeners with service name "*", which is a special feature allowable by Consul.

Note: to make http protocol work, a service-default must be defined setting the
protocol to http for each service.

Fixes: #9729
@shoenig
Copy link
Member Author

shoenig commented Apr 27, 2021

Demo

New behavior

Setup service defaults for our example service

{
    "Kind": "service-defaults",
    "Name": "uuid-api",
    "Protocol": "http",
    "MeshGateway": {},
    "Expose": {},
    "CreateIndex": 170,
    "ModifyIndex": 170
}

Job with ingress gateway with wildcard http service

job "w" {

  datacenters = ["dc1"]
  
  group "ingress-group" {

    network {
      mode = "bridge"

      port "inbound" {
        static = 8080
        to     = 8080
      }
    }

    service {
      name = "my-ingress-service"
      port = "8080"

      connect {
        gateway {
          ingress {
            listener {
              port     = 8080
              protocol = "http"
              service {
		name = "*" # this works now
              }
            }
          }
        }
      }
    }
  }

  group "generator" {
    network {
      mode = "host"
      port "api" {}
    }

    service {
      name = "uuid-api"
      port = "api"

      connect {
        native = true
      }
    }

    task "generate" {
      driver = "docker"

      config {
        image        = "hashicorpnomad/uuid-api:v5"
        network_mode = "host"
      }

      env {
        BIND = "0.0.0.0"
        PORT = "${NOMAD_PORT_api}"
      }
    }
  }
}

Run job

$ nomad job run w.nomad
==> Monitoring evaluation "24acde2e"
    Evaluation triggered by job "w"
==> Monitoring evaluation "24acde2e"
    Evaluation within deployment: "d484e9a1"
    Allocation "943993c6" created: node "99ba54ec", group "ingress-group"
    Allocation "bed3ba0b" created: node "99ba54ec", group "generator"
    Evaluation status changed: "pending" -> "complete"
==> Evaluation "24acde2e" finished with status "complete"

Query to the ingress gateway works

$ curl -H "Host: uuid-api.ingress.dc1.consul:8080" $(dig +short @127.0.0.1 -p 8600 uuid-api.ingress.dc1.consul. ANY):8080
e1bc5cda-d1e6-992f-c617-ee9ef81b5531
Old job-submission rejection
$ nomad job run w.nomad
Error submitting job: Unexpected response code: 500 (1 error occurred:
	* Task group ingress-group validation failed: 1 error occurred:
	* Task group service validation failed: 1 error occurred:
	* Service[0] my-ingress-service validation failed: 1 error occurred:
	* Consul Ingress Service requires one or more hosts when using HTTP protocol

)

@shoenig shoenig marked this pull request as ready for review April 27, 2021 20:04
@shoenig shoenig requested review from tgross and isabeldepapel April 27, 2021 20:04
Copy link
Member

@tgross tgross left a comment

Choose a reason for hiding this comment

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

LGTM!

// 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 🤷‍♂️

@shoenig shoenig merged commit c519c90 into main Apr 27, 2021
@shoenig shoenig deleted the b-igce-wildcard branch April 27, 2021 20:41
schmichael pushed a commit that referenced this pull request May 14, 2021
consul/connect: fix bug where ingress gateways could not use wildcard services
@tgross tgross added this to the 1.1.0 milestone May 17, 2021
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

connect: wildcard services in IGCE more restricted than consul
2 participants