Skip to content
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

VAULT-6371 Fix issue with lease quotas on read requests that generate leases #15735

Merged
merged 3 commits into from
Jun 3, 2022

Conversation

VioletHynes
Copy link
Contributor

@VioletHynes VioletHynes commented Jun 1, 2022

The crux of the issue here was that lease quota checks get routed differently to rate limit checks. In the case of lease quota checks, the MountPoint was not always present on the logical.Request. In essence, when searching for an applicable quota to apply to the request, it would not correctly find the applicable quota as it uses the MountPoint as part of searching for applicable quotas. It seems to apply only to endpoints that generate a lease on Read.

Lease quotas now apply properly on quotas based on mounts that when generating a lease on a Read:

violethynes@violethynes-C02CW1WWMD6R vault-enterprise % vault read mysql-database/creds/my-role
Error reading mysql-database/creds/my-role: Error making API request.

URL: GET http://127.0.0.1:8200/v1/mysql-database/creds/my-role
Code: 429. Errors:

* 1 error occurred:
	* request path "mysql-database/creds/my-role": lease count quota exceeded

Tests added as part of enterprise PR.

@VioletHynes VioletHynes marked this pull request as ready for review June 1, 2022 18:45
@ncabatoff
Copy link
Collaborator

I wasn't sure if there was an appropriate place to put a test for this, as the layers I could find for lease quotas created logical.Requests with the MountPoints included

Any of the tests in quotas_ent_test look like a good basis for a test for this, as they're all blackbox tests using the API, e.g. TestQuotas_LeaseCount_Counting.

@VioletHynes
Copy link
Contributor Author

quotas_ent_test.go seems to only have one test, and it is as I'd described, where the MountPath is provided directly to QueryQuota - it works largely with the Query Manager directly

@VioletHynes VioletHynes changed the title VAULT-6371 Fix issue with lease quotas on non-auth mounts VAULT-6371 Fix issue with lease quotas on read requests that generate leases Jun 3, 2022
@VioletHynes VioletHynes merged commit fbb707c into main Jun 3, 2022
@VioletHynes VioletHynes deleted the violethynes/VAULT-6371 branch June 3, 2022 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants