feat!(vpc-import): Reimplement reference to a pre-existing VPC

[akash1810]

Note

Need to consider the impact on these projects:

• Instances of passing publicSubnets to GuEc2App
• Instances of passing privateSubnets to GuEc2App
• Uses of GuVpc.fromIdParameter
• Uses of GuVpc.subnetsFromParameterFixedNumber

What does this change?

The current implementation of importing a VPC, via SSM Parameters, for use in an application makes reference to the AWS CDK method scope.availabilityZones. AWS CDK comments about this:

Returns the list of AZs that are available in the AWS environment (account/region) associated with this stack.

If the stack is environment-agnostic (either account and/or region are tokens), this property will return an array with 2 tokens that will resolve at deploy-time to the first two availability zones returned from CloudFormation's Fn::GetAZs intrinsic function.

If they are not available in the context, returns a set of dummy values and reports them as missing, and let the CLI resolve them by calling EC2 DescribeAvailabilityZones on the target environment.

That is, AWS CDK attempts to dynamically resolve the AZs by interrogating the account and storing values in the context file. If we don't configure AWS CDK with AWS credentials, attempts to reference a subnet results in the error:

Found an encoded list token string in a scalar string context. Use 'Fn.select(0, list)' (not 'list[0]') to extract elements from token lists.

This is very confusing!

We don't need to call scope.availabilityZones as we usually do not explicitly specify the AZ to place our resources. These changes reimplement how we reference a pre-existing VPC. The following have been replaced with GuVpcImport.fromSsmParameters:

• GuVpc.fromIdParameter
• GuVpc.subnetsFromParameter

For the most part, this should not impact any stacks as GuCDK's patterns have been updated. It is a breaking change, however, as this change includes a change to the CloudFormation parameter names.

How to test

See added and updated unit tests.

I've also tested these changes in guardian/cdk-playground#629.

How can we measure success?

More intuitive DX, with a reduction in confusing errors.

Have we considered potential risks?

We'd need to check the compatibility of these changes with those in #1566 and #2477.

Checklist

• I have listed any breaking changes, along with a migration path ¹
• I have updated the documentation as required for the described changes ²

Footnotes

1. Consider whether this is something that will mean changes to projects that have already been migrated, or to the CDK CLI tool. If changes are required, consider adding a checklist here and/or linking to related PRs. ↩

2. If you are adding a new construct or pattern, has new documentation been added? If you are amending defaults or changing behaviour, are the existing docs still valid? ↩

[changeset-bot]

🦋 Changeset detected

Latest commit: e81f459

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@guardian/cdk	Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[akash1810]

This behaviour was introduced in #399. I don't think it makes sense to have these parameters be app aware. The VpcId parameter is not app aware, that is, every app in a CFN stack uses the same VPC. We also don't have a subnet per app.