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

Add DNSName to SAN for go1.15 CN deprecation #9

Merged
merged 1 commit into from
Sep 21, 2020

Conversation

adrianludwin
Copy link
Contributor

This change is identical to
open-policy-agent/gatekeeper#811 and fixes
cert-controller on K8s 1.19+.

Tested: imported cert-controller with this change into HNC and verified
that without this change, HNC won't start on KIND 1.19, but with this
change, HNC's e2e tests work correctly.

This change is identical to
open-policy-agent/gatekeeper#811 and fixes
cert-controller on K8s 1.19+.

Tested: imported cert-controller with this change into HNC and verified
that without this change, HNC won't start on KIND 1.19, but with this
change, HNC's e2e tests work correctly.

Signed-off-by: Adrian Ludwin <aludwin@google.com>
@adrianludwin
Copy link
Contributor Author

Hmm, looking at that source PR, I'm now not sure why I thought I needed to add DNSNames in two locations, not one. I'm certain I didn't come up with it myself but now I'm not sure where I did get it from.

@rbtr, do you know why only one certificate needed this change?

@rbtr
Copy link

rbtr commented Sep 21, 2020

@adrianludwin Ahh, sorry, I think that's my fault for misleading you slightly with my original PR.

With go1.15 both the CA and the certs derived from it need SANs. When I made the change in open-policy-agent/gatekeeper#811, Gatekeeper was not yet built with 1.15, so the CA did not yet need the SAN because it was not being consumed by anything that was built with 1.15. (The serving cert immediately needed the SAN because it was being consumed by k8s 1.19, built with go 1.15.)

I followed that change up with bumping the build to go1.15 in open-policy-agent/gatekeeper#813, where you can see that I also needed to add the SAN to the CA to make the build happy.

So your change here is correct and necessary for building with go1.15, sorry for the confusion! 🙂

@adrianludwin
Copy link
Contributor Author

adrianludwin commented Sep 21, 2020 via email

Copy link
Member

@ritazh ritazh left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Contributor

@maxsmythe maxsmythe left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for doing this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants