-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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: non-login requests shouldn't care about login roles for lease quotas. Also fix a potential deadlock #21110
Conversation
This exposed:
|
…to apply roles to non-login lease quotas.
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.
Looks good! This should make sense, as we still call DetermineRoleFromLoginRequest
in handleLoginRequest
.
Our quota tests in ent are a lot more robust than they are in OSS (e.g. quotas_ent_test.go
), so you might want to make sure they all pass, just to be safe, before merging.
Good idea, did so in https://github.com/hashicorp/vault-enterprise/pull/4164. |
I'm working on some code in ent that messes with how we use these locks, so I turned on deadlock detection, and I found a pre-existing issue. Creating this PR to highlight it.