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

Fix ClientCertFile when using local config #325

Merged
merged 2 commits into from
Aug 6, 2023

Conversation

marcelobartsch-jt
Copy link
Contributor

@marcelobartsch-jt marcelobartsch-jt commented Aug 4, 2023

No description provided.

Fix ClientCertFile parameter
oboukili
oboukili previously approved these changes Aug 4, 2023
@onematchfox
Copy link
Collaborator

@marcelobartsch thanks so much for taking the time to track this down and open this PR; sorry, for all the hassle! Also, great spot on the ClientCertFile 😄. I'll cut a new release with these fixes ASAP.

This is set if `use_local_config` is set to `true` in `setLocalConfigOpts`
@onematchfox
Copy link
Collaborator

onematchfox commented Aug 4, 2023

@marcelobartsch, now back in front of a PC and on closer examination, opts.Context is already being set here (only if use_local_config = true). So, possibly the only underlying issue was the client cert. I've dropped the change you made to set opts.Context (which also missed fetching the value from the relevant env var) from this PR. Would you be able to test again at some point and confirm if just the fix for ClientCertFile works for you?

@onematchfox onematchfox dismissed oboukili’s stale review August 4, 2023 15:15

PR needs further discussion. Should not be merged at this point.

@marcelobartsch-jt
Copy link
Contributor Author

the change for clientCert file works!! and you were totally right on the context stuff, I had a extra env variable causing havoc in my setup.

Thanks you so much for the help on fixing this!!!

@onematchfox onematchfox changed the title Add missing Context to Opts Fix ClientCertFile when using local config Aug 6, 2023
@onematchfox onematchfox self-requested a review August 6, 2023 06:58
@onematchfox onematchfox merged commit bbd315a into argoproj-labs:master Aug 6, 2023
6 checks passed
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.

3 participants