-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
GCE/GKE "pre-shared" TLS cert #291
Conversation
Coverage increased (+0.2%) to 45.762% when pulling a99720feba4bfb63bb079f59e794c9601e0033ff on tonglil:gce-tls into a41ee3f on kubernetes:master. |
Great, thanks for the pr! |
No I did not, still waiting for a time to dig deeper into that. For this one, I ssh into the master of the e2e to change the image pulled. |
@nicksardo this is the pr for #45 (comment) |
Anything I can do to help the review process/bump this? |
Will be looking at this in a day or two @tonglil. Sorry for the delay. |
@tonglil, we want to get this merged within the next few hours. In order for this to happen, can you remove the change to the vendored k8s. Move the annotation to controller/utils.go and set as After performing a final test, ping me. Thanks |
// manage this certificate, it is the users responsibility to create/delete it. | ||
// In GCP, the Ingress controller assigns the SSL certificate with this name | ||
// to the target proxies of the Ingress. | ||
preSharedCertKey = "ingress.gcp.kubernetes.io/pre-shared-cert" |
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.
Just want to raise to light the mismatch in formatting between annotations (if this is still the right way to go).
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.
After discussing annotation naming conventions with Tim, I rather use the correct format moving forward. We can add modify docs later to highlight the disparity.
/lgtm Your final test was successful? |
Yes, the functionality remained the same (minus validation of config). |
Thanks, I'll make a ticket for the documentation! |
What this PR does / why we need it:
Ingress does not use TLS secret if a reference exists(pending validation code)Which issue this PR fixes:
Closes #45.
Related to kubernetes/kubernetes#41593.
Special notes for your reviewer:
TODO
item on naming of this annotation key.Release note: