fix(ec2): looking up a shared VPC has incorrect account ID in ARN (#24486)

This ensures that when a VPC is looked up using the context provider the
resulting VPC has the correct ARN. Previously, the OwnerId field in
the output was ignored; this is now passed as part of the environment of
the resource (along with its region). Additionally, owner-id as added
as a valid field to query on (as defined by the DescribeVpcs
documentation).

Further, previously the ARN did not include the region or account; this
seems to be an error as Arn.format only considers the given component
and the stack region/account, so those parameters are now passed as
well.

If OwnerId is undefined, this will fallback to the AWS::AccountId
pseudoparameter (the parent stack's Account ID); it seems like OwnerId
should always be defined though (even though the documentation
technically says it is not required).

Finally, an interface private to the tests had a typo in the name and
that is corrected (to MockVpcContextResponse).

This follows up on #23982 which was closed by automation after incorrectly
detecting it as abandoned.

Closes #23865.

----

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

Author: laurelmay

packages/@aws-cdk/cx-api/lib/context/vpc.ts

@@ -168,4 +168,11 @@ export interface VpcContextResponse {
    * @default - Region of the parent stack
    */
   readonly region?: string;
+
+  /**
+   * The ID of the AWS account that owns the VPC.
+   *
+   * @default the account id of the parent stack
+   */
+  readonly ownerAccountId?: string;
 }

packages/aws-cdk-lib/aws-ec2/lib/vpc-lookup.ts

@@ -68,4 +68,11 @@ export interface VpcLookupOptions {
    * @default true
    */
   readonly returnVpnGateways?: boolean;
+
+  /**
+   * The ID of the AWS account that owns the VPC
+   *
+   * @default the account id of the parent stack
+   */
+  readonly ownerAccountId?: string;
 }

packages/aws-cdk-lib/aws-ec2/lib/vpc.ts

@@ -1257,6 +1257,7 @@ export class Vpc extends VpcBase {
     // We give special treatment to some tags
     if (options.vpcId) { filter['vpc-id'] = options.vpcId; }
     if (options.vpcName) { filter['tag:Name'] = options.vpcName; }
+    if (options.ownerAccountId) { filter['owner-id'] = options.ownerAccountId; }
     if (options.isDefault !== undefined) {
       filter.isDefault = options.isDefault ? 'true' : 'false';
     }
@@ -2161,13 +2162,16 @@ class LookedUpVpc extends VpcBase {
   constructor(scope: Construct, id: string, props: cxapi.VpcContextResponse, isIncomplete: boolean) {
     super(scope, id, {
       region: props.region,
+      account: props.ownerAccountId,
     });
 
     this.vpcId = props.vpcId;
     this.vpcArn = Arn.format({
       service: 'ec2',
       resource: 'vpc',
       resourceName: this.vpcId,
+      region: this.env.region,
+      account: this.env.account,
     }, Stack.of(this));
     this.cidr = props.vpcCidrBlock;
     this._vpnGatewayId = props.vpnGatewayId;

packages/aws-cdk-lib/aws-ec2/test/vpc.from-lookup.test.ts

@@ -287,17 +287,111 @@ describe('vpc from lookup', () => {
       expect(vpc.env.region).toEqual('region-1234');
       restoreContextProvider(previous);
     });
+
+    test('passes owner account id to LookedUpVpc correctly', () => {
+      const previous = mockVpcContextProviderWith({
+        vpcId: 'vpc-1234',
+        subnetGroups: [],
+        region: 'region-1234',
+        ownerAccountId: '123456789012',
+      });
+
+      const stack = new Stack();
+      const vpc = Vpc.fromLookup(stack, 'Vpc', {
+        vpcId: 'vpc-1234',
+      });
+      expect(vpc.env.account).toEqual('123456789012');
+      restoreContextProvider(previous);
+    });
+
+    test('passes owner account id to context query correctly', () => {
+      const previous = mockVpcContextProviderWith({
+        vpcId: 'vpc-1234',
+        subnetGroups: [],
+        region: 'region-1234',
+        ownerAccountId: '123456789012',
+      }, options => {
+        expect(options.filter['owner-id']).toEqual('123456789012');
+      });
+
+      const stack = new Stack();
+      const vpc = Vpc.fromLookup(stack, 'Vpc', {
+        vpcId: 'vpc-1234',
+        ownerAccountId: '123456789012',
+      });
+      expect(vpc.env.account).toEqual('123456789012');
+      restoreContextProvider(previous);
+    });
+
+    test('a looked up VPC in a different region shared from an account has correct VPC', () => {
+      const previous = mockVpcContextProviderWith({
+        vpcId: 'vpc-1234',
+        subnetGroups: [],
+        region: 'region-1234',
+        ownerAccountId: '123456789012',
+      });
+      const stack = new Stack();
+      const vpc = Vpc.fromLookup(stack, 'Vpc', {
+        vpcId: 'vpc-1234',
+      });
+      expect(stack.resolve(vpc.vpcArn)).toEqual({
+        'Fn::Join': [
+          '',
+          [
+            'arn:',
+            {
+              Ref: 'AWS::Partition',
+            },
+            ':ec2:region-1234:123456789012:vpc/vpc-1234',
+          ],
+        ],
+      });
+      restoreContextProvider(previous);
+    });
+
+    test('a looked up VPC falls back to the parent stack\'s account and region', () => {
+      const previous = mockVpcContextProviderWith({
+        vpcId: 'vpc-1234',
+        subnetGroups: [],
+      });
+      const stack = new Stack();
+      const vpc = Vpc.fromLookup(stack, 'Vpc', {
+        vpcId: 'vpc-1234',
+      });
+      expect(stack.resolve(vpc.vpcArn)).toEqual({
+        'Fn::Join': [
+          '',
+          [
+            'arn:',
+            {
+              Ref: 'AWS::Partition',
+            },
+            ':ec2:',
+            {
+              Ref: 'AWS::Region',
+            },
+            ':',
+            {
+              Ref: 'AWS::AccountId',
+            },
+            ':vpc/vpc-1234',
+          ],
+        ],
+      });
+      restoreContextProvider(previous);
+    });
   });
 });
 
-interface MockVcpContextResponse {
+interface MockVpcContextResponse {
   readonly vpcId: string;
   readonly subnetGroups: cxapi.VpcSubnetGroup[];
+  readonly ownerAccountId?: string;
   readonly region?: string;
 }
 
 function mockVpcContextProviderWith(
-  response: MockVcpContextResponse,
+  response: MockVpcContextResponse,
   paramValidator?: (options: cxschema.VpcContextQuery) => void) {
   const previous = ContextProvider.getValue;
   ContextProvider.getValue = (_scope: Construct, options: GetContextValueOptions) => {

packages/aws-cdk-lib/cx-api/lib/context/vpc.ts

@@ -168,4 +168,11 @@ export interface VpcContextResponse {
    * @default - Region of the parent stack
    */
   readonly region?: string;
+
+  /**
+   * The ID of the AWS account that owns the VPC.
+   *
+   * @default the account id of the parent stack
+   */
+  readonly ownerAccountId?: string;
 }

packages/aws-cdk/lib/context-providers/vpcs.ts

@@ -138,6 +138,7 @@ export class VpcNetworkContextProviderPlugin implements ContextProviderPlugin {
     return {
       vpcId,
       vpcCidrBlock: vpc.CidrBlock!,
+      ownerAccountId: vpc.OwnerId,
       availabilityZones: grouped.azs,
       isolatedSubnetIds: collapse(flatMap(findGroups(SubnetType.Isolated, grouped), group => group.subnets.map(s => s.subnetId))),
       isolatedSubnetNames: collapse(flatMap(findGroups(SubnetType.Isolated, grouped), group => group.name ? [group.name] : [])),

packages/aws-cdk/test/context-providers/vpcs.test.ts

@@ -78,6 +78,7 @@ test('looks up the requested VPC', async () => {
   expect(result).toEqual({
     vpcId: 'vpc-1234567',
     vpcCidrBlock: '1.1.1.1/16',
+    ownerAccountId: '123456789012',
     availabilityZones: ['bermuda-triangle-1337'],
     isolatedSubnetIds: undefined,
     isolatedSubnetNames: undefined,
@@ -234,6 +235,7 @@ test('does not throw when subnet with subnetGroupNameTag is found', async () =>
   expect(result).toEqual({
     vpcId: 'vpc-1234567',
     vpcCidrBlock: '1.1.1.1/16',
+    ownerAccountId: '123456789012',
     availabilityZones: ['bermuda-triangle-1337'],
     isolatedSubnetIds: undefined,
     isolatedSubnetNames: undefined,
@@ -329,6 +331,7 @@ test('uses the VPC main route table when a subnet has no specific association',
   expect(result).toEqual({
     vpcId: 'vpc-1234567',
     vpcCidrBlock: '1.1.1.1/16',
+    ownerAccountId: '123456789012',
     availabilityZones: ['bermuda-triangle-1337'],
     isolatedSubnetIds: undefined,
     isolatedSubnetNames: undefined,
@@ -392,6 +395,7 @@ test('Recognize public subnet by route table', async () => {
   expect(result).toEqual({
     vpcId: 'vpc-1234567',
     vpcCidrBlock: '1.1.1.1/16',
+    ownerAccountId: '123456789012',
     availabilityZones: ['bermuda-triangle-1337'],
     isolatedSubnetIds: undefined,
     isolatedSubnetNames: undefined,
@@ -455,6 +459,7 @@ test('Recognize private subnet by route table', async () => {
   expect(result).toEqual({
     vpcId: 'vpc-1234567',
     vpcCidrBlock: '1.1.1.1/16',
+    ownerAccountId: '123456789012',
     availabilityZones: ['bermuda-triangle-1337'],
     isolatedSubnetIds: undefined,
     isolatedSubnetNames: undefined,
@@ -506,6 +511,7 @@ test('Recognize isolated subnet by route table', async () => {
   expect(result).toEqual({
     vpcId: 'vpc-1234567',
     vpcCidrBlock: '1.1.1.1/16',
+    ownerAccountId: '123456789012',
     availabilityZones: ['bermuda-triangle-1337'],
     isolatedSubnetIds: ['sub-123456'],
     isolatedSubnetNames: ['Isolated'],
@@ -532,7 +538,7 @@ function mockVpcLookup(options: VpcLookupOptions) {
 
   AWS.mock('EC2', 'describeVpcs', (params: aws.EC2.DescribeVpcsRequest, cb: AwsCallback<aws.EC2.DescribeVpcsResult>) => {
     expect(params.Filters).toEqual([{ Name: 'foo', Values: ['bar'] }]);
-    return cb(null, { Vpcs: [{ VpcId, CidrBlock: '1.1.1.1/16' }] });
+    return cb(null, { Vpcs: [{ VpcId, CidrBlock: '1.1.1.1/16', OwnerId: '123456789012' }] });
   });
 
   AWS.mock('EC2', 'describeSubnets', (params: aws.EC2.DescribeSubnetsRequest, cb: AwsCallback<aws.EC2.DescribeSubnetsResult>) => {