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

Updates docs with ingress Host header changes #8062

Merged
merged 4 commits into from
Jun 17, 2020
Merged

Conversation

crhino
Copy link
Contributor

@crhino crhino commented Jun 9, 2020

Clarify that a Host header is required for L7 protocols, and specify
that the default is to use the Consul DNS ingress subdomain

This comes from merging #7990 and #8083, as well as just better clarity.

@crhino crhino requested review from a team and kyhavlov June 9, 2020 15:49
@crhino crhino added type/docs Documentation needs to be created/updated/clarified type/docs-cherrypick labels Jun 9, 2020
Clarify that a Host header is required for L7 protocols, and specify
that the default is to use the Consul DNS ingress subdomain
@david-yu
Copy link
Contributor

@crhino In running and using an Ingress Gateway section, could we also include that layer7 routing can be used to direct traffic from Ingress Gateway to services? This would be optional but would complete the Ingress Gateway flow in my opinion.

@crhino
Copy link
Contributor Author

crhino commented Jun 17, 2020

@david-yu Pushed a change to add a sentence like that, let me know how you feel?

@crhino
Copy link
Contributor Author

crhino commented Jun 17, 2020

Added a note in the config entry page about port numbers from feedback in #8045

@david-yu
Copy link
Contributor

Ok this LGTM, thanks!

@david-yu david-yu self-requested a review June 17, 2020 19:29
@crhino crhino merged commit bb103f2 into master Jun 17, 2020
@crhino crhino deleted the docs/ingress-host-header branch June 17, 2020 19:44
@hashicorp-ci
Copy link
Contributor

🍒✅ Cherry pick of commit bb103f2 onto stable-website succeeded!

hashicorp-ci pushed a commit that referenced this pull request Jun 17, 2020
* Updates docs with ingress Host header changes

Clarify that a Host header is required for L7 protocols, and specify
that the default is to use the Consul DNS ingress subdomain

* Add sentence about using '*' by itself for testing

* Add optional step for using L7 routing config

* Note that port numbers may need to be added in the Hosts field
@nicholasjackson
Copy link
Contributor

@crhino Bit late on this one since it has already been merged but I suggest the example is updated to reflect the documentation enhancement. If folks cut and paste the sample it will cause confusion when it does not work.

Either the below example should have the port changed to 80 or the port 4567 should be added to the hosts as specified in the documentation.

{
    Port     = 4567
    Protocol = "http"
    Services = [
      {
        Name  = "api"
        Hosts = ["foo.example.com"]
      },
      {
        Name  = "web"
        Hosts = ["website.example.com"]
      }
    ]
  }

@david-yu
Copy link
Contributor

Hmm is it just me @crhino or did the docs not get cherry picked here? https://www.consul.io/docs/connect/ingress-gateway

@crhino
Copy link
Contributor Author

crhino commented Jun 30, 2020

@david-yu These changes are in the config entry documentation: https://www.consul.io/docs/agent/config-entries/ingress-gateway

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/docs Documentation needs to be created/updated/clarified
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants