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

doc: hubble configuration cleanup #17522

Merged

Conversation

kaworu
Copy link
Member

@kaworu kaworu commented Oct 4, 2021

Improve the hubble TLS configuration by rewording, and adding links to external resources.

@kaworu kaworu added kind/enhancement This would improve or streamline existing functionality. area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. release-note/misc This PR makes changes that have no direct user impact. labels Oct 4, 2021
@kaworu kaworu requested a review from a team as a code owner October 4, 2021 10:10
@kaworu kaworu requested a review from a team October 4, 2021 10:10
Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

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

Thanks, this is an improvement on the existing text. I have a couple of minor suggestions / FYIs from glancing through but I'm also fine with just merging this as-is.

@kaworu kaworu force-pushed the pr/kaworu/doc/hubble-configuration-cleanup branch from 234f544 to 6a0ba81 Compare October 5, 2021 09:37
Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

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

Just a few more minor grammar nits, then should be good to go I think.

@kaworu
Copy link
Member Author

kaworu commented Oct 7, 2021

Thanks for the comments @joestringer. Hopefully I've addressed all of them, please take another look!

Copy link
Member

@rolinh rolinh left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM except for some typos.

@kaworu kaworu force-pushed the pr/kaworu/doc/hubble-configuration-cleanup branch from 1015fd7 to 3531d0a Compare October 14, 2021 13:51
Rewording, add links to the cert-manager issuers, avoid shell command
with three-dots.

Signed-off-by: Alexandre Perrin <alex@kaworu.ch>
This patch remove two items from the cert-manager advantage list, so
that the two remaining items are highlighted.

cert-manager not needing a Kubernetes CronJob is arguably
counter-balanced by the fact that ones needs all the cert-manager Pods
running.

Remove the "Renew certificates automatically" item, as the previously
mentioned certgen method in the documentation is able to automatically
renew certificates as well.

Signed-off-by: Alexandre Perrin <alex@kaworu.ch>
@kaworu
Copy link
Member Author

kaworu commented Oct 15, 2021

No need for a full CI run here as this PR only contains documentation changes, marking as ready-to-merge.

@kaworu kaworu added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Oct 15, 2021
@joamaki joamaki merged commit 8ec177c into cilium:master Oct 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. kind/enhancement This would improve or streamline existing functionality. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants