-
Notifications
You must be signed in to change notification settings - Fork 6.8k
fix(server): OIDC config via secrets fails (#18269) #26214
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
Conversation
Signed-off-by: Valéry Fouques <[email protected]>
❌ Preview Environment deleted from BunnyshellAvailable commands (reply to this comment):
|
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.
Can you add a unit test to validate that this fixes the issue? :)
By looking at the code, it looks like it does!
Will do, I have to write a unit test which fails the older version, first ;) |
After looking into it, I don't think it is possible to write a test for it, because it would imply rewriting the GetSettings function, and show that the secretRefs are not expanded if the secrets list is empty. updateSettingsFromConfigMap calls a function which expects the secrets to be set, but the function which actually sets them is tupdateSettingsFromSecret. Don't know how to test that. GetSettings() returns an eventually consistent *ArcoCDSettings, this PR just removes the warnings. I don't know how to test, because in the end, the data structure has the expected data, so there is nothing "wrong", except for that warning message which disturbs people like me while trying to debug ArgoCD integration with an OIDC provider ;) The warning is still needed if the referenced secret does not exist, though. |
|
Yep, that is a fair comment! Thanks for looking into it nonetheless :) Feel free to open a refactor PR if you feel like it, but that is totally up to you! |
|
From the issue, it looks like this issue exists in 3.2.6 too. I think we'll be required to cherry-pick this. |
Signed-off-by: Valéry Fouques <[email protected]>
|
🍒 Cherry-pick PR created for 3.2: #26389 |
Closes #18269
Improper ordering of function calls in
(*SettingsManager).GetSettings, which causes warnings related to OIDC secrets not being found.The call to
updateSettingsFromConfigMapwas done before the SettingsManager secrets were set.Checklist: