fix(ec2): availabilityZones can be empty for Vpc causing subsequent subnet errors

[gravitylow]

Issue # (if applicable)

Closes #32451

Reason for this change

When creating a Vpc construct and specifying a list of availability zones to use for creating subnets with, passing an empty list causing misleading and confusing errors to happen with other Vpc features. See #32451 for further details and examples.

Description of changes

I added a simple check to throw an error when at least one subnet is going to be created, but the provided availabilityZones to use is empty.

Describe any new or updated permissions being added

N/A

Description of how you validated changes

Added a unit test and confirm it fails with the misleading error message prior to fixing the issue:

% yarn test aws-ec2
...
Summary of all failing tests
 FAIL  aws-ec2/test/vpc.test.ts
  ● vpc › When creating a VPC › throws an error when availabilityZones is empty

    expect(received).toThrow(expected)

    Expected pattern: /At least one availability zone must be configured to create subnets/
    Received message: "There are no 'Private' subnet groups in this VPC. Available types: "

          644 |     if (subnets.length === 0 && !this.incompleteSubnetDefinition) {
          645 |       const availableTypes = Object.entries(allSubnets).filter(([_, subs]) => subs.length > 0).map(([typeName, _]) => typeName);
        > 646 |       throw new Error(`There are no '${subnetType}' subnet groups in this VPC. Available types: ${availableTypes}`);
              |             ^
          647 |     }
          648 |
          649 |     return subnets;

          at Vpc.selectSubnetObjectsByType (aws-ec2/lib/vpc.ts:646:13)
          at Vpc.selectSubnetObjectsByType [as selectSubnetObjects] (aws-ec2/lib/vpc.ts:599:22)
          at Vpc.selectSubnetObjects [as selectSubnets] (aws-ec2/lib/vpc.ts:475:26)
          at selectSubnets (aws-ec2/lib/vpc-endpoint.ts:210:50)
              at Array.map (<anonymous>)
          at new map (aws-ec2/lib/vpc-endpoint.ts:210:31)
          at Vpc.addGatewayEndpoint (aws-ec2/lib/vpc.ts:560:12)
          at new addGatewayEndpoint (aws-ec2/lib/vpc.ts:1692:14)
          at aws-ec2/test/vpc.test.ts:1407:9
          at Object.<anonymous> (../../node_modules/expect/build/toThrowMatchers.js:74:11)
          at Object.throwingMatcher [as toThrow] (../../node_modules/expect/build/index.js:320:21)
          at Object.<anonymous> (aws-ec2/test/vpc.test.ts:1416:10)

      1414 |           },
      1415 |         });
    > 1416 |       }).toThrow(/At least one availability zone must be configured to create subnets/);
           |          ^
      1417 |     });
      1418 |   });
      1419 |

      at Object.<anonymous> (aws-ec2/test/vpc.test.ts:1416:10)


Test Suites: 1 failed, 27 passed, 28 total
Tests:       1 failed, 882 passed, 883 total
Snapshots:   0 total
Time:        6.635 s

Fixed the issue and confirm the test now passes, along with others:

% yarn test aws-ec2
...
Test Suites: 28 passed, 28 total
Tests:       883 passed, 883 total
Snapshots:   0 total
Time:        6.17 s
Ran all test suites matching /aws-ec2/i.
✨  Done in 7.34s.

Checklist

• My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[aws-cdk-automation]

The pull request linter fails with the following errors:

❌ Fixes must contain a change to an integration test file and the resulting snapshot.

If you believe this pull request should receive an exemption, please comment and provide a justification. A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed, add Clarification Request to a comment.

✅ A exemption request has been requested. Please wait for a maintainer's review.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.39%. Comparing base (be383a9) to head (73ae970).

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #33993   +/-   ##
=======================================
  Coverage   82.39%   82.39%           
=======================================
  Files         120      120           
  Lines        6960     6960           
  Branches     1175     1175           
=======================================
  Hits         5735     5735           
  Misses       1120     1120           
  Partials      105      105           

Flag	Coverage Δ
suite.unit	82.39% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
packages/aws-cdk	∅ <ø> (∅)
packages/aws-cdk-lib/core	82.39% <ø> (ø)

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[gravitylow]

Exemption Request for "Fixes must contain a change to an integration test file and the resulting snapshot." -- the fix involves throwing an error at build time and won't end up deploying a stack. A unit test exercising the behavior of the fix was added.

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: 73ae970
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[shikha372]

Thank you @gravitylow for submitting this, while i agree that there should be a validation to check whether the availabilty zones is an empty array or not, i also think it is not a good definition of setting an availability zone as '[]' in the Vpc definition, few concerns that I have to add this validation and if we can verify that this doesn't affect those existing implementations will be good to go:

1. Case where we have a looked up VPC and in DummyVPC we pass this parameter as [] to import the asymmetric subnets, would like to validate that with throwing error here doesn't impact those implementations.
2. Same concern to verify this with fromVpcAttributes.
3. This can break customers who are passing this value as an empty array right now in their stacks and maybe initializing it later, so throwing a warning instead of error would be safest to reduce the impact on such customers. Please let me know if you disagree on any of these concerns.

For the exemption request, we can try running the existing integration test just to confirm that these are working fine along with additional unit tests for the scenario's mentioned above.