Skip to content

Commit

Permalink
fix(ec2): non-agnostic stack throws error with availability zones def…
Browse files Browse the repository at this point in the history
…ined in VPC (#25468)

## Changes Made

The existing code calls a get availabilityZones method off of the stack. If no context is provided, then two possible return values are possible:
1) The stack is deemed agnostic, i.e., the stack has no defined environment, or the environment only contains just an account or just a region. In this case, the return value will be an array of unresolved tokens.
2) The stack is deemed non-agnostic. In this case, the return value will be ['dummy1a', 'dummy1b', 'dummy1c']

In the event of case 1, the filter method is called off of the array of unresolved tokens and attempts to create a new array of 'resolved stack AZs'. However, since the array that filter is called off of contains unresolved tokens, the result is an empty array. Moving forward, the value 'areGivenAzsSubsetOfStack' will be true only because 'resolvedStackAzs' is of length 0.

In the event of case 2, the filter method is called off of the array containing dummy values - ['dummy1a', 'dummy1b', 'dummy1c']. Since each element in this array will be considered resolved, the resulting array will also just be ['dummy1a', 'dummy1b', 'dummy1c']. Moving forward from this, the value 'areGivenAzsSubsetOfStack' will be false assuming the user is providing actual availability zones, i.e., 'us-east-1a', 'us-east-1b', etc. The previously mentioned error will then be thrown which is unhelpful for the user as the validation done in this case isn't helpful.

As a result, the only useful validation that can occur is in the case that a context has been provided that contains one of two keys:
1) availability-zones:account=${account}:region=${region}
2) aws:cdk:availability-zones:fallback

I've updated the validation logic to check for one of these two keys - it first checks for key 1 and then checks for the availability-zones fallback key (key 2). If either of the two keys is found then 'stackAzs' will be equal to the associated key's value. If neither key is found then 'stackAzs' will be undefined. From here, we check if 'stackAzs' is undefined, and if it is then no useful validation can be done so 'areGivenAzsSubsetOfStackAzs' will be true and we will move on without validation. If 'stackAzs' is not undefined, then we can check that each given availability zone exists within the'stackAzs' array. The error mentioned above will only be thrown if 'areGivenAzsSubsetOfStackAzs' if false.

Closes #21690

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
colifran committed May 26, 2023
1 parent 784cd0e commit c2a22fa
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 6 deletions.
23 changes: 17 additions & 6 deletions packages/aws-cdk-lib/aws-ec2/lib/vpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1451,13 +1451,11 @@ export class Vpc extends VpcBase {

if (props.availabilityZones) {
// If given AZs and stack AZs are both resolved, then validate their compatibility.
const resolvedStackAzs = stack.availabilityZones.filter(az => !Token.isUnresolved(az));
const areGivenAzsSubsetOfStack = resolvedStackAzs.length === 0 // stack AZs are tokenized, so we cannot validate it
|| props.availabilityZones.every(
az => Token.isUnresolved(az) // given AZ is tokenized, such as in integ tests, so we cannot validate it
|| resolvedStackAzs.includes(az));
const resolvedStackAzs = this.resolveStackAvailabilityZones(stack.availabilityZones);
const areGivenAzsSubsetOfStack = resolvedStackAzs.length === 0 ||
props.availabilityZones.every(az => Token.isUnresolved(az) ||resolvedStackAzs.includes(az));
if (!areGivenAzsSubsetOfStack) {
throw new Error(`Given VPC 'availabilityZones' ${props.availabilityZones} must be a subset of the stack's availability zones ${stack.availabilityZones}`);
throw new Error(`Given VPC 'availabilityZones' ${props.availabilityZones} must be a subset of the stack's availability zones ${resolvedStackAzs}`);
}
this.availabilityZones = props.availabilityZones;
} else {
Expand Down Expand Up @@ -1722,6 +1720,19 @@ export class Vpc extends VpcBase {
},
});
}

/**
* Returns the list of resolved availability zones found in the provided stack availability
* zones.
*
* Note: A resolved availability zone refers to an availability zone that is not a token
* and is also not a dummy value.
*/
private resolveStackAvailabilityZones(stackAvailabilityZones: string[]): string[] {
const dummyValues = ['dummy1a', 'dummy1b', 'dummy1c'];
// if an az is resolved and it is not a 'dummy' value, then add it to array as a resolved az
return stackAvailabilityZones.filter(az => !Token.isUnresolved(az) && !dummyValues.includes(az));
}
}

const SUBNETTYPE_TAG = 'aws-cdk:subnet-type';
Expand Down
25 changes: 25 additions & 0 deletions packages/aws-cdk-lib/aws-ec2/test/vpc.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -719,13 +719,38 @@ describe('vpc', () => {

test('with availabilityZones set to zones different from stack', () => {
const stack = getTestStack();
// we need to create a context with availability zones, otherwise we're checking against dummy values
stack.node.setContext(
`availability-zones:account=${stack.account}:region=${stack.region}`,
['us-east-1a', 'us-east1b', 'us-east-1c'],
);
expect(() => {
new Vpc(stack, 'VPC', {
availabilityZones: [stack.availabilityZones[0] + 'invalid'],
});
}).toThrow(/must be a subset of the stack/);
});

test('does not throw with availability zones set without context in non-agnostic stack', () => {
const stack = getTestStack();
expect(() => {
new Vpc(stack, 'VPC', {
availabilityZones: ['us-east-1a'],
});
}).not.toThrow();
});

test('agnostic stack without context with defined vpc AZs', () => {
const stack = new Stack(undefined, 'TestStack');
new Vpc(stack, 'VPC', {
availabilityZones: ['us-east-1a'],
});
Template.fromStack(stack).resourceCountIs('AWS::EC2::Subnet', 2);
Template.fromStack(stack).hasResourceProperties('AWS::EC2::Subnet', {
AvailabilityZone: 'us-east-1a',
});
});

test('with natGateway set to 1', () => {
const stack = getTestStack();
new Vpc(stack, 'VPC', {
Expand Down

0 comments on commit c2a22fa

Please sign in to comment.