Skip to content

list-resource/aws_vpc: Reduces API calls when listing#46935

Open
gdavison wants to merge 14 commits intomainfrom
td-list-vpc-batch-api-calls
Open

list-resource/aws_vpc: Reduces API calls when listing#46935
gdavison wants to merge 14 commits intomainfrom
td-list-vpc-batch-api-calls

Conversation

@gdavison
Copy link
Contributor

@gdavison gdavison commented Mar 14, 2026

Rollback Plan

If a change needs to be reverted, we will publish an updated version of the library.

Changes to Security Controls

Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.

Description

Reduces AWS API calls when listing by:

  • Only fetching additional data when resource data is requested
  • Batching API requests where possible

Relations

Closes #46724

Output from Acceptance Testing

% make testacc PKG=ec2 TESTS=TestAccVPC_

--- PASS: TestAccVPC_List_Filtered_isDefault (11.09s)
--- PASS: TestAccVPC_List_filteredVPCIDs (55.33s)
--- PASS: TestAccVPC_List_vpcIDs (55.51s)
--- PASS: TestAccVPC_List_DefaultVPC_exclude (61.68s)
--- PASS: TestAccVPC_Tags_DefaultTags_emptyResourceTag (72.48s)
--- PASS: TestAccVPC_Tags_null (91.95s)
--- PASS: TestAccVPC_Tags_DefaultTags_nullOverlappingResourceTag (91.95s)
--- PASS: TestAccVPC_Tags_DefaultTags_emptyProviderOnlyTag (80.88s)
--- PASS: TestAccVPC_Tags_emptyMap (92.12s)
--- PASS: TestAccVPC_Tags_DefaultTags_nullNonOverlappingResourceTag (92.14s)
--- PASS: TestAccVPC_Tags_ComputedTag_onCreate (92.22s)
--- PASS: TestAccVPC_Identity_basic (106.78s)
--- PASS: TestAccVPC_Tags_EmptyTag_OnUpdate_replace (131.05s)
--- PASS: TestAccVPC_enableNetworkAddressUsageMetrics (76.15s)
--- PASS: TestAccVPC_Tags_EmptyTag_onCreate (131.51s)
--- PASS: TestAccVPC_Tags_addOnUpdate (131.72s)
--- PASS: TestAccVPC_Tags_ComputedTag_OnUpdate_add (164.85s)
--- PASS: TestAccVPC_tags (383.87s)
--- PASS: TestAccVPC_Tags_DefaultTags_providerOnly (395.57s)
--- PASS: TestAccVPC_disappears (64.14s)
--- PASS: TestAccVPC_updateDNSHostnames (117.25s)
--- PASS: TestAccVPC_DynamicResourceTags_ignoreChanges (139.53s)
--- PASS: TestAccVPC_basic (74.38s)
--- PASS: TestAccVPC_assignGeneratedIPv6CIDRBlock (289.90s)
--- PASS: TestAccVPC_tags_ignoreTags (137.89s)
--- PASS: TestAccVPC_Identity_ExistingResource_noRefreshNoChange (106.85s)
--- PASS: TestAccVPC_DynamicResourceTagsMergedWithLocals_ignoreChanges (102.57s)
--- PASS: TestAccVPC_Identity_ExistingResource_basic (119.70s)
--- PASS: TestAccVPC_Identity_regionOverride (97.12s)
--- PASS: TestAccVPC_tags_defaultAndIgnoreTags (125.88s)
--- PASS: TestAccVPC_Tags_ComputedTag_OnUpdate_replace (100.63s)
--- PASS: TestAccVPC_tenancy (163.80s)
--- PASS: TestAccVPC_upgradeFromV5WithDefaultRegionRefreshFalse (88.07s)
--- PASS: TestAccVPC_Tags_IgnoreTags_Overlap_defaultTag (108.21s)
--- PASS: TestAccVPC_Tags_IgnoreTags_Overlap_resourceTag (115.93s)

@github-actions
Copy link
Contributor

Community Guidelines

This comment is added to every new Pull Request to provide quick reference to how the Terraform AWS Provider is maintained. Please review the information below, and thank you for contributing to the community that keeps the provider thriving! 🚀

Voting for Prioritization

  • Please vote on this Pull Request by adding a 👍 reaction to the original post to help the community and maintainers prioritize it.
  • Please see our prioritization guide for additional information on how the maintainers handle prioritization.
  • Please do not leave +1 or other comments that do not add relevant new information or questions; they generate extra noise for others following the Pull Request and do not help prioritize the request.

Pull Request Authors

  • Review the contribution guide relating to the type of change you are making to ensure all of the necessary steps have been taken.
  • Whether or not the branch has been rebased will not impact prioritization, but doing so is always a welcome surprise.

@github-actions github-actions bot added prioritized Part of the maintainer teams immediate focus. To be addressed within the current quarter. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. service/lambda Issues and PRs that pertain to the lambda service. service/iam Issues and PRs that pertain to the iam service. service/ec2 Issues and PRs that pertain to the ec2 service. service/ecr Issues and PRs that pertain to the ecr service. service/batch Issues and PRs that pertain to the batch service. service/secretsmanager Issues and PRs that pertain to the secretsmanager service. service/route53resolver Issues and PRs that pertain to the route53resolver service. service/vpc Issues and PRs that pertain to the vpc service. size/XL Managed by automation to categorize the size of a PR. and removed prioritized Part of the maintainer teams immediate focus. To be addressed within the current quarter. labels Mar 14, 2026
@gdavison gdavison marked this pull request as ready for review March 14, 2026 00:35
@gdavison gdavison requested a review from a team as a code owner March 14, 2026 00:35
@dosubot dosubot bot added the new-list-resource Introduces list resource support. label Mar 14, 2026
@ewbankkit ewbankkit added technical-debt Addresses areas of the codebase that need refactoring or redesign. and removed new-list-resource Introduces list resource support. labels Mar 16, 2026
Copy link
Contributor

@ewbankkit ewbankkit left a comment

Choose a reason for hiding this comment

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

LGTM 🚀.

Comment on lines +2035 to +2038
results := make(map[string]*awstypes.RouteTable, len(output))
for i, v := range output {
results[aws.ToString(v.VpcId)] = &output[i]
}
Copy link
Member

@jar-b jar-b Mar 16, 2026

Choose a reason for hiding this comment

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

This seems like it will overwrite the map entry for each route table in the result. Should this instead check for the key in result and append if already exists?

Edit: I see now the only current call to this function is from batchFindVPCMainRouteTables where we know the result will be limited to a single item per VPC, but it may still be worth future proofing should we reach for this in the future for batch listing all route tables and grouping by VPC.

Comment on lines +2083 to +2086
results := make(map[string]*awstypes.SecurityGroup, len(output))
for i, v := range output {
results[aws.ToString(v.VpcId)] = &output[i]
}
Copy link
Member

Choose a reason for hiding this comment

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

Same for iterating security group results (multiple per VPC).

Comment on lines +1871 to +1874
results := make(map[string]*awstypes.NetworkAcl, len(output))
for i, v := range output {
results[aws.ToString(v.VpcId)] = &output[i]
}
Copy link
Member

Choose a reason for hiding this comment

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

Same for iterating network ACLs (one per subnet, but a VPC may have multiple).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

service/batch Issues and PRs that pertain to the batch service. service/ec2 Issues and PRs that pertain to the ec2 service. service/ecr Issues and PRs that pertain to the ecr service. service/iam Issues and PRs that pertain to the iam service. service/lambda Issues and PRs that pertain to the lambda service. service/route53resolver Issues and PRs that pertain to the route53resolver service. service/secretsmanager Issues and PRs that pertain to the secretsmanager service. service/vpc Issues and PRs that pertain to the vpc service. size/XL Managed by automation to categorize the size of a PR. technical-debt Addresses areas of the codebase that need refactoring or redesign. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

List Resource Efficiency: aws_vpc

3 participants