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 godebug configs #133

Merged
merged 1 commit into from
Dec 1, 2020
Merged

add godebug configs #133

merged 1 commit into from
Dec 1, 2020

Conversation

xtremerui
Copy link
Contributor

fixes #127

alternatively if we want to limit the debug flags option we could do sth like

if enable_godebug_cn_matching
GODEBUG: x509ignoreCN=0

to hardcode the name/value pair. WDYT?

Signed-off-by: Rui Yang <[email protected]>
@xtremerui
Copy link
Contributor Author

bumping @clarafu

@clarafu
Copy link
Contributor

clarafu commented Nov 25, 2020

Hm I'm going to pull in @vito as the PM here for some insight, because my initial reaction is that this additional flag is added just so that someone that has an outdated certificate is trying to continue using it even though it is no longer supported. I'm just hesitant on adding flags to temporarily fix one person's use case, when it can be solved through generating a proper cert. But I also have very little knowledge of certificates, AD servers, etc and if this will be a more widespread problem that many of our users will run into, so I could be totally wrong.

@xtremerui
Copy link
Contributor Author

Totally agree, i came to a PR recetnly concourse/docker-image-resource#317, which also adds the same env var for go 1.15.

We should come up with a more systematic way to allow folks ignore the warning (indeed there is case that the cert is out of their control).

@vito
Copy link
Member

vito commented Nov 25, 2020

This is kind of annoying but it seems reasonable to allow this for now just so folks aren't blocked on things outside of their control. 👍

@xtremerui I wouldn't go as far as proactively adding a param for this everywhere - I think it's better for folks to run into it first so they know that something needs to be done. At least with the Helm chart users won't be blocked because you can always add the env var yourself after generating the template.

I'm not keen to add this in to all the resource types, either - that's a lot of churn for something that's deprecated. I'm more inclined to have folks build their own forks instead until they can fix their certs.

@vito vito merged commit 13ae823 into master Dec 1, 2020
@vito vito deleted the godebug branch December 1, 2020 16:27
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.

Go 1.15 breaks LDAP integration with AD controllers due to CN x509 field deprecation
3 participants