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: ingress gateway validation for http hosts and wildcards #15749

Merged
merged 3 commits into from Jan 11, 2023

Conversation

gulducat
Copy link
Member

@gulducat gulducat commented Jan 10, 2023

This makes code match the documentation, and reality 😋

Applies to all non-"tcp" protocols: http, http2, and grpc, which support "hosts" and tests now cover all of them as well. I could maybe be convinced to remove the extra test coverage if it seems superfluous, but it's intended to guard against potential future regressions.

per https://developer.hashicorp.com/nomad/docs/job-specification/gateway#service-parameters,

service Parameters

  • hosts (array<string>: nil) - A list of hosts that specify what requests will match this service. This cannot be used with a tcp listener, and cannot be specified alongside a wildcard (*) service name. If not specified, the default domain <service-name>.ingress.* will be used to match services.
e.g. this will now work:
listener {
  port     = 8080
  protocol = "http"
  service {
    name  = "uuid-api"
    # hosts = no longer required
  }
}

and this will no longer work (by "work" I mean pass to consul, which errors less-specifically):

listener {
  port     = 8080
  protocol = "http"
  service {
    name  = "*"
    hosts = ["anything"]
  }
}

error before:

Error submitting job: Unexpected response code: 500 (Unexpected response code: 500 (Associating hosts to a wildcard service is not supported (listener on port 8080)))

error after:

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 with a wildcard "*" service name can not also specify hosts)

Closes #10955

Note: to use these non-"tcp" protocols, users will still need to manually write a service-defaults Consul config entry as described in #8647 (comment)

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
Copy link
Member

@shoenig shoenig left a comment

Choose a reason for hiding this comment

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

LGTM! just the one suggestion. Be sure to set backport labels for 1.4.x, 1.3.x, and 1.2.x

nomad/structs/services_test.go Outdated Show resolved Hide resolved
@gulducat gulducat added backport/1.2.x backport to 1.1.x release line backport/1.3.x backport to 1.3.x release line backport/1.4.x backport to 1.4.x release line labels Jan 10, 2023
@gulducat gulducat merged commit fe0852c into main Jan 11, 2023
@gulducat gulducat deleted the gulducat/consul-ingress-http-no-hosts branch January 11, 2023 17:52
philrenaud pushed a commit that referenced this pull request Jan 23, 2023
)

* 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
@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 May 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/1.2.x backport to 1.1.x release line backport/1.3.x backport to 1.3.x release line backport/1.4.x backport to 1.4.x release line
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consul Ingress Gateway config incorrectly requires Hosts value
2 participants