-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Docs: Update Dex config, change misleading error message. #11208
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Elad Leev <eladleev@gmail.com>
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #11208 +/- ##
==========================================
- Coverage 49.73% 49.72% -0.01%
==========================================
Files 261 261
Lines 44705 44705
==========================================
- Hits 22234 22231 -3
- Misses 20284 20286 +2
- Partials 2187 2188 +1
☔ View full report in Codecov by Sentry. |
@@ -182,6 +182,8 @@ Go through the same steps as in [OpenID Connect using Dex](#openid-connect-using | |||
apiVersion: v1 | |||
kind: Secret | |||
metadata: | |||
labels: |
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.
Hi @EladLeev Can you please explain what was the impact of not having this before ?
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.
Thank you for taking a look at this PR @iam-veeramalla!
Without this label, ArgoCD won't consider this secret as part of ArgoCD, and won't read it at all.
Correct me if I'm wrong, but using this label, argo select and parse related secrets.
Without it, Argo will just use the default secret (argocd-secret
).
This also relates to the rest of the changes that I've made - when this label is absent, and you are referring to a different secret (using $<k8s_secret_name>:<key>
) the error that you'll get is -
config referenced '%s', but the key does not exist in secret
Which is a bit misleading as you are not really reading that secret and not finding the key.
side note: I think that it's better to add validations and a custom errors when referring to a secret that is not labelled, but I didn't want to make too many changes to settings.go
on my first contribution 😅
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.
Are you adding this manually today in your setup ?
Signed-off-by: Elad Leev eladleev@gmail.com
Note on DCO:
If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the Details link next to the DCO action for instructions on how to resolve this.
Checklist:
Description:
Hi :)
Creating this PR to update some of the Dex documentation, and to change a misleading error message on
ReplaceStringSecret
.Dex changes:
part-of
label is missing from the Secret example, so it won't be associate it with Argo.Log changes:
Thanks! 💪