fix(ec2): can't use imported Subnets in a SubnetSelection

[flemjame-at-amazon]

---

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

[rix0rrr]

The title does not really describe what's wrong.

A simpler fix is to turn this into a getter:

https://github.com/aws/aws-cdk/blob/master/packages/@aws-cdk/aws-ec2/lib/vpc.ts#L348

get availabilityZones(): { return subnets.map(s => s.availabilityZone); },

Which (should) make it so that the evaluation of availabilityZone is only done when it's actually requested by a construct.

[flemjame-at-amazon]

A simpler fix is to turn this into a getter:

https://github.com/aws/aws-cdk/blob/master/packages/@aws-cdk/aws-ec2/lib/vpc.ts#L348

get availabilityZones(): { return subnets.map(s => s.availabilityZone); },

Which (should) make it so that the evaluation of availabilityZone is only done when it's actually requested by a construct.

  public selectSubnets(selection: SubnetSelection = {}): SelectedSubnets {
    const subnets = this.selectSubnetObjects(selection);
    const pubs = new Set(this.publicSubnets);

    return {
      subnetIds: subnets.map(s => s.subnetId),
      get availabilityZones(): string[] { return subnets.map(s => s.availabilityZone); },
      internetConnectivityEstablished: tap(new CompositeDependable(), d => subnets.forEach(s => d.add(s.internetConnectivityEstablished))),
      subnets,
      hasPublic: subnets.some(s => pubs.has(s)),
    };
  }

That seems to have done it.

Any thoughts on returning a CFN reference if the AZ is referenced, but wasn't provided in the subnet lookup? Technically it would work for region-agnostic templates.

[rix0rrr]

Any thoughts on returning a CFN reference if the AZ is referenced, but wasn't provided in the subnet lookup? Technically it would work for region-agnostic templates.

I don't see how that will work though. How will:

{ "Fn::GetAtt": [ "s-12356", "AvailabilityZone"] }

Resolve to the availability zone of subnet s-123456? This goes against everything I know to be true about CloudFormation. Is this a special case?

(The first argument to Fn::GetAtt is typically a Logical ID of a resource that exists in the same template, not an identifier to an arbitrary AWS resource that exists outside of the template)

EDIT: It's definitely not documented here: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/intrinsic-function-reference-getatt.html

[flemjame-at-amazon]

Any thoughts on returning a CFN reference if the AZ is referenced, but wasn't provided in the subnet lookup? Technically it would work for region-agnostic templates.

I don't see how that will work though. How will:

{ "Fn::GetAtt": [ "s-12356", "AvailabilityZone"] }

Resolve to the availability zone of subnet s-123456? This goes against everything I know to be true about CloudFormation. Is this a special case?

(The first argument to Fn::GetAtt is typically a Logical ID of a resource that exists in the same template, not an identifier to an arbitrary AWS resource that exists outside of the template)

EDIT: It's definitely not documented here: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/intrinsic-function-reference-getatt.html

I didn't read the documentation carefully enough -- you're right, we don't get a logical ID from a subnet import. Ok I'll scrap that part of the code.

Pull request has been modified.

[mergify]

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
• Commit ID: 94af631
• Result: SUCCEEDED
• Build Logs (available for 30 days)

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

[mergify]

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).