From 11d27e4323c811ca3df29922b344af593ad755b4 Mon Sep 17 00:00:00 2001 From: Stijn Brouwers Date: Mon, 12 Dec 2022 16:47:31 +0100 Subject: [PATCH] fix(opensearch): Don't throw a Az count mismatch for imported VPCs (#22654) Solving issue #22651 Creating a domain fails for imported vpc/subnets when zone awareness is enabled and the cdk context is cleared. When the CDK context is already retrieved and the VPC is in the context, the deployment works. This is due to the fact that when there is no context yet, the subnet count is always 0. That's why I decided to disable it. If it's not correct, it will fail when applying the CloudFormation template. ---- ### All Submissions: * [X] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../aws-opensearchservice/lib/domain.ts | 29 +++++++++++++++++-- .../aws-opensearchservice/test/domain.test.ts | 23 ++++++++++++++- 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/packages/@aws-cdk/aws-opensearchservice/lib/domain.ts b/packages/@aws-cdk/aws-opensearchservice/lib/domain.ts index 06d450320890f..7ea759f7e82d0 100644 --- a/packages/@aws-cdk/aws-opensearchservice/lib/domain.ts +++ b/packages/@aws-cdk/aws-opensearchservice/lib/domain.ts @@ -1235,8 +1235,11 @@ export class Domain extends DomainBase implements IDomain, ec2.IConnectable { let securityGroups: ec2.ISecurityGroup[] | undefined; let subnets: ec2.ISubnet[] | undefined; + let skipZoneAwarenessCheck: boolean = false; if (props.vpc) { - subnets = selectSubnets(props.vpc, props.vpcSubnets ?? [{ subnetType: ec2.SubnetType.PRIVATE_WITH_EGRESS }]); + const subnetSelections = props.vpcSubnets ?? [{ subnetType: ec2.SubnetType.PRIVATE_WITH_EGRESS }]; + subnets = selectSubnets(props.vpc, subnetSelections); + skipZoneAwarenessCheck = zoneAwarenessCheckShouldBeSkipped(props.vpc, subnetSelections); securityGroups = props.securityGroups ?? [new ec2.SecurityGroup(this, 'SecurityGroup', { vpc: props.vpc, description: `Security group for domain ${this.node.id}`, @@ -1248,8 +1251,12 @@ export class Domain extends DomainBase implements IDomain, ec2.IConnectable { } } - // If VPC options are supplied ensure that the number of subnets matches the number AZ - if (subnets && zoneAwarenessEnabled && new Set(subnets.map((subnet) => subnet.availabilityZone)).size < availabilityZoneCount) { + // If VPC options are supplied ensure that the number of subnets matches the number AZ (only if the vpc is not imported from another stack) + if (subnets && + zoneAwarenessEnabled && + !skipZoneAwarenessCheck && + new Set(subnets.map((subnet) => subnet.availabilityZone)).size < availabilityZoneCount + ) { throw new Error('When providing vpc options you need to provide a subnet for each AZ you are using'); } @@ -1776,6 +1783,22 @@ function selectSubnets(vpc: ec2.IVpc, vpcSubnets: ec2.SubnetSelection[]): ec2.IS return selected; } +/** + * Check if any of the subnets are pending lookups. If so, the zone awareness check should be skipped, otherwise it will always throw an error + * + * @param vpc The vpc to which the subnets apply + * @param vpcSubnets The vpc subnets that should be checked + * @returns true if there are pending lookups for the subnets + */ +function zoneAwarenessCheckShouldBeSkipped(vpc: ec2.IVpc, vpcSubnets: ec2.SubnetSelection[]): boolean { + for (const selection of vpcSubnets) { + if (vpc.selectSubnets(selection).isPendingLookup) { + return true; + }; + } + return false; +} + /** * Initializes an instance type. * diff --git a/packages/@aws-cdk/aws-opensearchservice/test/domain.test.ts b/packages/@aws-cdk/aws-opensearchservice/test/domain.test.ts index 6260678513600..6e983d88baf7e 100644 --- a/packages/@aws-cdk/aws-opensearchservice/test/domain.test.ts +++ b/packages/@aws-cdk/aws-opensearchservice/test/domain.test.ts @@ -2,7 +2,7 @@ import { Match, Template } from '@aws-cdk/assertions'; import * as acm from '@aws-cdk/aws-certificatemanager'; import { Metric, Statistic } from '@aws-cdk/aws-cloudwatch'; -import { Vpc, EbsDeviceVolumeType, Port, SecurityGroup } from '@aws-cdk/aws-ec2'; +import { Vpc, EbsDeviceVolumeType, Port, SecurityGroup, SubnetType } from '@aws-cdk/aws-ec2'; import * as iam from '@aws-cdk/aws-iam'; import * as kms from '@aws-cdk/aws-kms'; import * as logs from '@aws-cdk/aws-logs'; @@ -1406,6 +1406,27 @@ describe('custom error responses', () => { })).toThrow(/you need to provide a subnet for each AZ you are using/); }); + test('Imported VPC with subnets that are still pending lookup won\'t throw Az count mismatch', () => { + const vpc = Vpc.fromLookup(stack, 'ExampleVpc', { + vpcId: 'vpc-123', + }); + let subnets = vpc.selectSubnets({ + subnetType: SubnetType.PRIVATE_WITH_EGRESS, + }); + + new Domain(stack, 'Domain1', { + version: defaultVersion, + vpc, + vpcSubnets: [subnets], + zoneAwareness: { + enabled: true, + availabilityZoneCount: 3, + }, + }); + + Template.fromStack(stack).resourceCountIs('AWS::OpenSearchService::Domain', 1); + }); + test('error when master, data or Ultra Warm instance types do not end with .search', () => { const error = /instance types must end with ".search"/; expect(() => new Domain(stack, 'Domain1', {