-
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
rate limit: fix initialize defaults #10536
Conversation
Note @vishalnayak, I do not think you can get this to panic unless you go from 1.6.x -> 1.5.x -> 1.6.x. This is because |
Tested using the following script: https://github.com/hashicorp/vault-tools/blob/master/users/vishal/quotas/rateLimitPanic.sh |
Note that we didn't have to restart Vault to see the panic. If we just updated a rate limit quota that existed in 1.5.5, that was enough to trigger it. This is because, even the update operation will walk over the |
@alexanderbez You might want to add changelog file to this PR. |
Added a CL entry @vishalnayak |
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.
This should have a changelog file, not a direct edit to changelog.md
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.
Approving - my approval only covers the changelog though
Backport PR for 1.6.x: #10565 |
When coming from 1.5.x to 1.6.x, rate limit resource quotas are incompletely initialized. In partciular, the underling rate limiter was swapped between these two versions and the current one (1.6.x) requires fields that when coming from 1.5.x, do not exist. Given these fields are never mutated or provided by an operator or client, the solution is to just set them to the default(s).