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

Document SDS for ingress gateways #11164

Merged
merged 4 commits into from
Dec 14, 2021
Merged

Document SDS for ingress gateways #11164

merged 4 commits into from
Dec 14, 2021

Conversation

banks
Copy link
Member

@banks banks commented Sep 28, 2021

This PR adds documentation for features added in #10903 and #11163.

These are "low-level" features designed to be used by systems integration builders rather than typical-end users but are included to provide a complete example so this functionality can be used. I don't anticipate a Learn guide for this feature as it's more of an integration interface so included a somewhat guide-like complete example of getting the whole feature to work.

If anyone can think of a better place to put the more detailed guide than the ingress gateway overview page I'd be open to suggestions.

Preview: https://consul-o89qrzjyl-hashicorp.vercel.app/docs/connect/config-entries/ingress-gateway#sds

@banks banks requested a review from a team as a code owner September 28, 2021 12:46
@github-actions github-actions bot added theme/envoy/xds Related to Envoy support type/docs Documentation needs to be created/updated/clarified labels Sep 28, 2021
@banks banks added this to the 1.11.0 milestone Sep 28, 2021
Copy link
Contributor

@karl-cardenas-coding karl-cardenas-coding left a comment

Choose a reason for hiding this comment

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

Thanks for adding the docs content @banks . I left some suggestions and questions for you. I also tagged @trujillo-adam to review from a tech writing perspective.

@banks banks force-pushed the feature/ingress-tls-mixed branch 2 times, most recently from 985f93b to d65bb33 Compare September 28, 2021 16:13
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging September 28, 2021 16:14 Inactive
@banks banks changed the base branch from feature/ingress-tls-mixed to docs/mesh-header-manip September 28, 2021 16:15
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging September 28, 2021 16:22 Inactive
Copy link
Contributor

@trujillo-adam trujillo-adam left a comment

Choose a reason for hiding this comment

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

I had a few questions and made some formatting suggestions that you can take or leave.

website/content/docs/connect/gateways/ingress-gateway.mdx Outdated Show resolved Hide resolved
website/content/docs/connect/gateways/ingress-gateway.mdx Outdated Show resolved Hide resolved
website/content/docs/connect/gateways/ingress-gateway.mdx Outdated Show resolved Hide resolved
website/content/docs/connect/gateways/ingress-gateway.mdx Outdated Show resolved Hide resolved
website/content/docs/connect/gateways/ingress-gateway.mdx Outdated Show resolved Hide resolved
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging September 29, 2021 14:55 Inactive
Base automatically changed from docs/mesh-header-manip to main October 8, 2021 12:11
Copy link
Contributor

@freddygv freddygv left a comment

Choose a reason for hiding this comment

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

LGTM, left a couple minor suggestions

website/content/docs/connect/gateways/ingress-gateway.mdx Outdated Show resolved Hide resolved
website/content/docs/connect/gateways/ingress-gateway.mdx Outdated Show resolved Hide resolved
Copy link
Contributor

@trujillo-adam trujillo-adam left a comment

Choose a reason for hiding this comment

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

Left a few suggestions.

@banks
Copy link
Member Author

banks commented Nov 30, 2021

Thanks for all the reviews folks. This fell off my radar a bit - I had some other work locally that didn't get pushed up, but I'll get the feedback all incorporated this week so it's done and merged in time for GA.

banks and others added 4 commits December 1, 2021 15:13
Co-authored-by: mrspanishviking <kcardenas@hashicorp.com>
Co-authored-by: trujillo-adam <47586768+trujillo-adam@users.noreply.github.com>
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging December 1, 2021 15:35 Inactive
@banks
Copy link
Member Author

banks commented Dec 1, 2021

Thanks @trujillo-adam and @freddygv, I finally got around to completing my edits here. I think this addresses all the feedback so far and looks good to me.

If you want to take another pass, feel free otherwise i'll merge this in a few days to make sure it makes it into 1.11 on time!

@banks banks added the pr/no-changelog PR does not need a corresponding .changelog entry label Dec 1, 2021
@banks banks merged commit 131897b into main Dec 14, 2021
@banks banks deleted the docs/ingress-sds branch December 14, 2021 17:32
@hc-github-team-consul-core
Copy link
Contributor

🍒 If backport labels were added before merging, cherry-picking will start automatically.

To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/527813.

@hc-github-team-consul-core
Copy link
Contributor

🍒 If backport labels were added before merging, cherry-picking will start automatically.

To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/527833.

@hc-github-team-consul-core
Copy link
Contributor

🍒✅ Cherry pick of commit 131897b onto release/1.11.x succeeded!

hc-github-team-consul-core pushed a commit that referenced this pull request Dec 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/no-changelog PR does not need a corresponding .changelog entry theme/envoy/xds Related to Envoy support type/docs Documentation needs to be created/updated/clarified
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants