Allow arbitrary service quota requests #97

autero1 · 2024-02-10T09:26:25Z

Description

Add codegen to store AWS default service quotas
Allow arbitrary quota requests

TODOs

Read the Gruntwork contribution guidelines.

Update the docs.
Run the relevant tests successfully, including pre-commit checks.
Ensure any 3rd party code adheres with our license policy or delete this line if its not applicable.
Include release notes. If this PR is backward incompatible, include a migration guide.

Release Notes (draft)

Added codegen to generate service quotas code
Modified request-quota-increase module to allow arbitrary quota requests

Migration Guide

`request-quota-increase`

The input format for resources_to_increase in request-quota-increase was removed. Change the input values to use quota-specific input variables, e.g.

module "quota_increase" {
  source = "git::git@github.com:gruntwork-io/terraform-aws-utilities.git//modules/quota-increase?ref=<VERSION>"

  vpc_rules_per_network_acl              = 30
  vpc_nat_gateways_per_availability_zone = 30
}

gruntwork-ci · 2024-02-10T09:45:08Z

Upgrade test results for build 732

Modules upgraded from origin/HEAD => feat/generic-service-quotas
Terraform upgraded from latest:^1.2 => latest:^1.3

⚠️ If plan results are not all 0's, review the plan output to ensure backward compatibility.

Example	Modules to upgrade	Plan	Upgraded?	Patched	Upgraded?
`join-path`	`instance-type` `join-path`	0, 0, 0	✅
`instance-type`	`instance-type` `join-path`	0, 0, 0	✅

brikis98 · 2024-02-12T16:25:25Z

examples/request-quota-increase/main.tf

+    NAT_GW_PER_AZ = {
+      service_code  = "vpc"
+      quota_code    = "L-FE5A380F"
+      desired_quota = 5
+    },
+    RULES_PER_ACL = {
+      service_code  = "vpc"
+      quota_code    = "L-2AEEBF1A"
+      desired_quota = 20
+    },


Having to manually look up service_code and quota_code is tedious and error prone.

If you already have a script that can generate a Markdown file with all that data, could you update that script to generate TF code? The code could then give each quota an input var name to enable that quota and the module would internally handle all the service_code and quota_code details.

Issue with having an input var for each quota is that Terraform doesn't allow iterating all input variables. As such (with ~4300 configurable quotas), the resulting Terraform files would be huge, as you would have to manage the mappings per each individual entry.

As an example, consider the following auto generated input var

variable "rekognition_transactions_per_second_per_account_for_the_amazon_rekognition_face_liveness_operation_startfacelivenesssession" { description = "Adjustable quota for Amazon Rekognition - Transactions per second per account for the Amazon Rekognition Face Liveness operation StartFaceLivenessSession" type = number default = 0 }

You would have to have the comparison for non-zero value somewhere - locals, resource declaration,... And you would have to do this for each of the 4000+ other inputs.

If we wanted to generate mappings for the quotas like they are now, we'd end up with a map of maps with thousands of entries:

codes = { .... nacl_rules = { quota_code = "L-2AEEBF1A" service_code = "vpc" }, nat_gateway = { quota_code = "L-FE5A380F" service_code = "vpc" }, rekognition_transactions_per_second_per_account_for_the_amazon_rekognition_face_liveness_operation_startfacelivenesssession = { quota_code = "XXXXXXXX" service_code = "rekognition" }, } ...

and you would still have to search the keys from the source code.

The only thing to gain here is not having to lookup the quota/service tuple, just the internal map key.

No perfect solution here. One approach could be a general purpose module like this, coupled with a wrapper module with a vastly reduced number of inputs/configurable quotas.

I agree there's no perfect solution here.

That said, what is the problem with having 4,000 input vars if it is all auto-generated anyway? Does Terraform not allow that?

The advantage of using input vars is we get auto-complete in IDEs that support it, which will make finding the inputs you want a little easier just by starting to type and hitting CTRL + SPACE. Also, we get some basic type checking, and our auto-generated docs would allow you to find what you need pretty easily as well with CTRL + F.

I understand 4,000 variables is annoying, but that's the nature of this domain, which has 4,000 quotas. It seems to me like 4,000 named inputs, with type safety, typeahead, descriptions, and auto-generated docs is the least bad option here, no?

I'll give it a shot 😄 . To get auto-complete, we have to use input vars and probably default the quotas to null to distinguish which quota requests we actually want to submit.

Also, are we happy with python or should this be like https://github.com/gruntwork-io/terraform-aws-security/tree/main/codegen?

I'll give it a shot 😄 . To get auto-complete, we have to use input vars and probably default the quotas to null to distinguish which quota requests we actually want to submit.

Good point.

Also, are we happy with python or should this be like https://github.com/gruntwork-io/terraform-aws-security/tree/main/codegen?

Whatever is easiest/fastest.

After the latest commit, the quota request module is now auto-generated. However, there were certain issues. Some of the quotas end up as duplicate, so I had to drop a few. Which code is the correct, that I don't know. These are the duplicates:

networkmonitor_number_of_probes_per_monitor: L-F192A8D6 networkmonitor_number_of_monitors_per_account_per_aws_region: L-15A52C9B networkmonitor_number_of_probes_per_subnet_for_each_monitor: L-6A0F5BB1 networkmonitor_number_of_probes_per_monitor: L-4C539B84 networkmonitor_number_of_probes_per_subnet_for_each_monitor: L-417BD1FB networkmonitor_number_of_monitors_per_account_per_aws_region: L-24751C61 transcribe_transactions_per_second_listcallanalyticscategories_operation: L-B4737A9F transcribe_transactions_per_second_getcallanalyticscategory_operation: L-A07285C7 transcribe_transactions_per_second_createcallanalyticscategory_operation: L-020BD1D2 transcribe_maximum_number_of_rules_per_category_for_call_analytics_batch_jobs: L-2E269322 transcribe_the_number_of_concurrent_startcallanalyticsstreamtranscription_http_2_streaming_requests_: L-128229BB transcribe_transactions_per_second_listmedicalvocabularies_operation: L-580FD495 transcribe_transactions_per_second_startmedicaltranscriptionjob_operation: L-BF30671E transcribe_number_of_concurrent_call_analytics_batch_jobs: L-48FC5F8A transcribe_transactions_per_second_startcallanalyticsstreamtranscription_websocket_operation: L-A0A9F543 transcribe_transactions_per_second_updatecallanalyticscategory_operation: L-B24B8474 transcribe_maximum_number_of_categories_for_call_analytics_batch_jobs: L-E9626652 transcribe_transactions_per_second_startcallanalyticsstreamtranscription_operation: L-EF81BED4 transcribe_transactions_per_second_startmedicalstreamtranscription_operation: L-A3F1B3F5 transcribe_transactions_per_second_deletemedicaltranscriptionjob_operation: L-AAFCE0A3 transcribe_transactions_per_second_updatemedicalvocabulary_operation: L-765784F2 transcribe_transactions_per_second_deletecallanalyticsjob_operation: L-E9D8884B transcribe_transactions_per_second_getcallanalyticsjob_operation: L-2F86860B transcribe_transactions_per_second_startcallanalyticsjob_operation: L-4B321684 transcribe_transactions_per_second_getmedicalvocabulary_operation: L-16E31C23 transcribe_number_of_pending_medical_vocabularies: L-836A857F transcribe_number_of_concurrent_medical_batch_transcription_jobs: L-63F366BB transcribe_transactions_per_second_deletecallanalyticscategory_operation: L-40CC32CD transcribe_total_number_of_medical_vocabularies_per_account: L-79BBEFC1 transcribe_number_of_startmedicalstreamtranscription_websocket_requests: L-0FB6DE48 transcribe_transactions_per_second_getmedicaltranscriptionjob_operation: L-A99534C1 transcribe_transactions_per_second_listmedicaltranscriptionjobs_operation: L-2D4ED180 transcribe_transactions_per_second_deletemedicalvocabulary_operation: L-50B57C16 transcribe_transactions_per_second_listcallanalyticsjobs_operation: L-93564E36 transcribe_maximum_number_of_targets_allowed_per_category_for_call_analytics_batch_jobs: L-3533BA28

One final option to consider is adding an include or excludes list and we only generate included or those not in the exclude list.

This one is ready for review... haven't properly cleaned up or documented the code until we determine if this is the way to go.

Huh, the dupes are weird!

As long as the module handles them in a deterministic way, I'm happy with whatever option you've chosen. If we need to change that later, we can. For now, the only thing I'd do is ensure that we document (in the README, Python code, etc) that there are dupes, so future maintainers can see we've thought about it and understand how we've handled it.

gruntwork-ci · 2024-02-19T12:03:34Z

Upgrade test results for build 738

Modules upgraded from origin/HEAD => feat/generic-service-quotas
Terraform upgraded from latest:^1.2 => latest:^1.3

⚠️ If plan results are not all 0's, review the plan output to ensure backward compatibility.

Example	Modules to upgrade	Plan	Upgraded?	Patched	Upgraded?
`join-path`	`instance-type` `join-path`	0, 0, 0	✅
`instance-type`	`instance-type` `join-path`	0, 0, 0	✅

autero1 · 2024-02-19T12:11:15Z

The generated files are quite large:
https://github.com/gruntwork-io/terraform-aws-utilities/blob/feat/generic-service-quotas/modules/request-quota-increase/main.tf
https://github.com/gruntwork-io/terraform-aws-utilities/blob/feat/generic-service-quotas/modules/request-quota-increase/variables.tf

brikis98

LGTM!

gruntwork-ci · 2024-02-20T17:22:50Z

Upgrade test results for build 745

Modules upgraded from origin/HEAD => feat/generic-service-quotas
Terraform upgraded from latest:^1.2 => latest:^1.3

⚠️ If plan results are not all 0's, review the plan output to ensure backward compatibility.

Example	Modules to upgrade	Plan	Upgraded?	Patched	Upgraded?
`join-path`	`instance-type` `join-path`	0, 0, 0	✅
`instance-type`	`instance-type` `join-path`	0, 0, 0	✅

autero1 · 2024-02-20T17:33:01Z

Added improved duplicate handling and documentation. Need a re-review @brikis98

brikis98

Ah, clever way to handle the dupes.

LGTM!

autero1 added 3 commits February 10, 2024 11:05

add quota codegen

3dd9fcb

use defaults vs actuals

04f5883

add generic quota capbility

a801071

autero1 marked this pull request as ready for review February 10, 2024 09:49

autero1 requested review from arsci, infraredgirl and oredavids as code owners February 10, 2024 09:49

autero1 changed the title ~~[WIP] Generic service quota increase~~ Generic service quota increase Feb 10, 2024

autero1 changed the title ~~Generic service quota increase~~ Allow arbitrary service quota requests Feb 10, 2024

brikis98 reviewed Feb 12, 2024

View reviewed changes

quota codegen

ea5c043

brikis98 previously approved these changes Feb 20, 2024

View reviewed changes

update generation script and add notes about duplicates

41d95af

autero1 dismissed brikis98’s stale review via 41d95af February 20, 2024 17:18

autero1 requested a review from brikis98 February 20, 2024 18:44

brikis98 approved these changes Feb 20, 2024

View reviewed changes

autero1 merged commit f95cbb3 into main Feb 21, 2024

autero1 deleted the feat/generic-service-quotas branch February 21, 2024 05:33

Allow arbitrary service quota requests #97

Allow arbitrary service quota requests #97

Uh oh!

Conversation

autero1 commented Feb 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

TODOs

Release Notes (draft)

Migration Guide

request-quota-increase

Uh oh!

gruntwork-ci commented Feb 10, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gruntwork-ci commented Feb 19, 2024

Uh oh!

autero1 commented Feb 19, 2024

Uh oh!

brikis98 left a comment

Choose a reason for hiding this comment

Uh oh!

gruntwork-ci commented Feb 20, 2024

Uh oh!

autero1 commented Feb 20, 2024

Uh oh!

brikis98 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

autero1 commented Feb 10, 2024 •

edited

Loading

`request-quota-increase`