Skip to content

Conversation

@liranbg
Copy link
Member

@liranbg liranbg commented Nov 30, 2025

📝 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

  • Changed secret name to be hashed-base rather than username/token name
  • To allow retrieving user tokens or user secrets, we leverage secret labels and getting those secret by using username/token name labels. this change is opaque to end users.
  • Relaxed some spammy logs

✅ Checklist

  • I updated the documentation (if applicable)
  • I have tested the changes in this PR
  • I confirmed whether my changes are covered by system tests
    • If yes, I ran all relevant system tests and ensured they passed before submitting this PR
    • I updated existing system tests and/or added new ones if needed to cover my changes
  • If I introduced a deprecation:

🧪 Testing

  • Unit testings
  • Manual testings

🔗 References


🚨 Breaking Changes?

  • Yes (explain below)
  • [n] No

🔍️ Additional Notes

)

create = False
secret_data = self._encode_user_token(token_name, token, expiration)
Copy link
Contributor

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

Comment on lines +1211 to +1213
create = True

if create:
Copy link
Member

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?

Copy link
Member Author

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

Copy link
Member

@quaark quaark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@liranbg liranbg merged commit feb28bd into mlrun:development Dec 1, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants