-
Notifications
You must be signed in to change notification settings - Fork 293
[Auth] Change user token auth secret naming convention #8986
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
| ) | ||
|
|
||
| create = False | ||
| secret_data = self._encode_user_token(token_name, token, expiration) |
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.
Encoding here becomes useless if the code doesn't access any of the if conditions below. You can encode only if you're going to create/update
| create = True | ||
|
|
||
| if create: |
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.
why not keep it in the if not k8s_secret: clause?
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.
just for explicitness - thought it make it a bit more readable
quaark
left a 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.
🚀
📝 Description
The current implementation assumes username and token name part of the secret name - which can lead to k8s validation issues when username contains @ or token name has some
_characters which is not k8s valid for secret names.🛠️ Changes Made
✅ Checklist
🧪 Testing
🔗 References
🚨 Breaking Changes?
🔍️ Additional Notes