-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Documentation fixes & improvements #2464
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2464 +/- ##
==========================================
+ Coverage 41.61% 41.67% +0.05%
==========================================
Files 74 74
Lines 5291 5291
==========================================
+ Hits 2202 2205 +3
+ Misses 2792 2790 -2
+ Partials 297 296 -1
Continue to review full report at Codecov.
|
docs/user-guide/multiple-ingress.md
Outdated
|
||
Setting the annotation `kubernetes.io/ingress.class` to any other value which does not match a valid ingress class will force the NGINX Ingress controller to ignore your Ingress. If you are only running a single NGINX ingress controller, this can be achieved by setting this to any value except "nginx" or an empty string. | ||
!!! important | ||
Deploying multiple Ingress controllers and not specifying a class annotation will result in both or all controllers fighting to satisfy the Ingress. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and updating the Ingress Status field
docs/user-guide/tls.md
Outdated
|
||
## Server-side HTTPS enforcement through redirect | ||
|
||
By default the controller redirects (301) to `HTTPS` if TLS is enabled for that ingress. If you want to disable that behavior globally, you can use `ssl-redirect: "false"` in the NGINX config map. | ||
By default the controller redirects HTTP clients to the HTTPS port | ||
443 using a 301 Moved Permanently response if TLS is enabled for that Ingress. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is now 308
docs/deploy/upgrade.md
Outdated
args: ... | ||
``` | ||
|
||
simply change the `0.9.0` tag to the version you wish to upgrade to. The easiest way to do this is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe
kubectl set image deployment/nginx-ingress-controller \
nginx-ingress-controller=nginx:quay.io/kubernetes-ingress-controller/nginx-ingress-controller:0.14.0
@akx just some comments from my part Thank you very much for doing this! |
@aledbf Thanks for the comments, I've distilled them in! (Also, TIL about And you're welcome – I'm doing this also to learn about |
|
||
## Automated Certificate Management with Kube-Lego | ||
|
||
[Kube-Lego] automatically requests missing or expired certificates from [Let's Encrypt] by monitoring ingress resources and their referenced secrets. To enable this for an ingress resource you have to add an annotation: | ||
!!! tip | ||
Kube-Lego has reached end-of-life and is being |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should refresh that part to only mention cert-manager.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, but I feel that's out of scope for this PR.
docs/user-guide/tls.md
Outdated
|
||
A sample ConfigMap to allow these older clients connect could look something like the following: | ||
To change this default behavior, use a [ConfigMap]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing link.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, thanks. :)
/approve |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: akx, aledbf, antoineco The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What this PR does / why we need it:
This PR improves the documentation especially around TLS/SSL, since that's what I've been working on deploying these last few days :), fixes formatting and adds a short doc page on upgrading ingress-nginx (fixes #2458).