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

Apply the 'ssl-redirect' annotation per-location #919

Merged
merged 1 commit into from
Jun 28, 2017

Conversation

ankon
Copy link
Contributor

@ankon ankon commented Jun 28, 2017

This is needed to avoid ingress definitions with different settings for SSL
redirection conflicting with each other.

NB: This was discussed in the review of #427, but ultimately not addressed.


In my test setup I have regular ingress definitions with enabled SSL-redirect, and kube-lego is adding '/.well-known/acme-challenge/...' ingress definitions that at least for a while need to not use ssl-redirection. In kube-lego's log files one can see the problem:

time="2017-06-27T10:29:01Z" level=debug msg="testing reachability of http://DOMAIN.TLD/.well-known/acme-challenge/_selftest" context=acme domain=DOMAIN.TLD
time="2017-06-27T10:29:01Z" level=debug msg="error while authorizing: reachability test failed: Get https://DOMAIN.TLD/.well-known/acme-challenge/_selftest: x509: certificate signed by unknown authority" context=acme domain=DOMAIN.TLD

Note that the reachability test wants to use 'http' (see https://github.com/jetstack/kube-lego/blob/master/pkg/acme/cert_request.go#L47 for the request, and https://github.com/jetstack/kube-lego/blob/master/pkg/provider/nginx/nginx.go#L119 for the ingress configuration using ssl-redirect=false), but then follows the redirect to 'https' -- which uses the "Kubernetes Ingress Controller Fake Certificate".

This is needed to avoid ingress definitions with different settings for SSL
redirection conflicting with each other.

NB: This was discussed in the review of kubernetes#427, but ultimately not addressed.
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 28, 2017
@k8s-reviewable
Copy link

This change is Reviewable

@ankon ankon changed the title Apply the 'ssl-redirect' redirect per-location Apply the 'ssl-redirect' annotation per-location Jun 28, 2017
@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 44.329% when pulling 04346a8 on Collaborne:pr/redirect-per-location into 005ed52 on kubernetes:master.

@aledbf
Copy link
Member

aledbf commented Jun 28, 2017

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 28, 2017
@aledbf
Copy link
Member

aledbf commented Jun 28, 2017

@ankon thanks!

@aledbf aledbf merged commit 3c9ac43 into kubernetes:master Jun 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants