Skip to content

Commit

Permalink
fix(core): crossRegionReferences don't work across multiple regions (#…
Browse files Browse the repository at this point in the history
…25384)

The first attempt to fix this in #25190 didn't work because it didn't account for the fact that when exporting to multiple regions, we create multiple `ExportWriter`s that all use the same provider (and provider role).

This PR fixes that by adding the policy cross region ARNs directly to the custom resource provider (1 per stack) rather than the `ExportWriter` (multiple per stack). I also updated the test case to better account for this scenario.

fixes #25377

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
corymhall committed May 4, 2023
1 parent a0ad49d commit 65265e1
Show file tree
Hide file tree
Showing 8 changed files with 352 additions and 327 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,15 @@
}
}
},
"a645bb0b063dc77d60a1af0c1e427999161c49fe9008543153cb48fed642a943": {
"f93ad4a2004b446276ef3fac1ada613b58a45257e6bfd66f18ab8119232f6131": {
"source": {
"path": "cross-region-producer.template.json",
"packaging": "file"
},
"destinations": {
"current_account-us-east-1": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-us-east-1",
"objectKey": "a645bb0b063dc77d60a1af0c1e427999161c49fe9008543153cb48fed642a943.json",
"objectKey": "f93ad4a2004b446276ef3fac1ada613b58a45257e6bfd66f18ab8119232f6131.json",
"region": "us-east-1",
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-us-east-1"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,18 @@
":parameter/cdk/exports/*"
]
]
},
{
"Fn::Join": [
"",
[
"arn:aws:ssm:us-west-2:",
{
"Ref": "AWS::AccountId"
},
":parameter/cdk/exports/*"
]
]
}
],
"Action": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@
}
}
},
"a916bc3a5655b0f9f8fc2ca727c849da1870288a890fc7a778f18859c60dc3b2": {
"abfdad43ef2795ab9e253a1b3cb3bfd72c1d00fa011b42ce99ffdff17b83c947": {
"source": {
"path": "crossregionreferencesDefaultTestDeployAssertAB7415FD.template.json",
"packaging": "file"
},
"destinations": {
"current_account-current_region": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
"objectKey": "a916bc3a5655b0f9f8fc2ca727c849da1870288a890fc7a778f18859c60dc3b2.json",
"objectKey": "abfdad43ef2795ab9e253a1b3cb3bfd72c1d00fa011b42ce99ffdff17b83c947.json",
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"StackName": "cross-region-producer"
},
"flattenResponse": "false",
"salt": "1682378118042"
"salt": "1682942114901"
},
"UpdateReplacePolicy": "Delete",
"DeletionPolicy": "Delete"
Expand Down Expand Up @@ -118,7 +118,7 @@
"StackName": "cross-region-producer"
},
"flattenResponse": "false",
"salt": "1682378118042"
"salt": "1682942114901"
},
"DependsOn": [
"AwsApiCallCloudFormationdeleteStack"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"validateOnSynth": false,
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-us-east-1",
"cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-us-east-1",
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-us-east-1/a645bb0b063dc77d60a1af0c1e427999161c49fe9008543153cb48fed642a943.json",
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-us-east-1/f93ad4a2004b446276ef3fac1ada613b58a45257e6bfd66f18ab8119232f6131.json",
"requiresBootstrapStackVersion": 6,
"bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version",
"additionalDependencies": [
Expand Down Expand Up @@ -328,7 +328,7 @@
"validateOnSynth": false,
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}",
"cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}",
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/a916bc3a5655b0f9f8fc2ca727c849da1870288a890fc7a778f18859c60dc3b2.json",
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/abfdad43ef2795ab9e253a1b3cb3bfd72c1d00fa011b42ce99ffdff17b83c947.json",
"requiresBootstrapStackVersion": 6,
"bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version",
"additionalDependencies": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import { Intrinsic } from '../../private/intrinsic';
import { makeUniqueId } from '../../private/uniqueid';
import { Reference } from '../../reference';
import { Stack } from '../../stack';
import { builtInCustomResourceProviderNodeRuntime, CustomResourceProvider } from '../custom-resource-provider';
import { Token } from '../../token';
import { builtInCustomResourceProviderNodeRuntime, CustomResourceProvider, CustomResourceProviderProps } from '../custom-resource-provider';

/**
* Properties for an ExportReader
Expand All @@ -24,6 +24,43 @@ export interface ExportWriterProps {
readonly region?: string;
}

/**
* Create our own CustomResourceProvider so that we can add a single policy
* with a list of ARNs instead of having to create a separate policy statement per ARN.
*/
class CRProvider extends CustomResourceProvider {
public static getOrCreateProvider(scope: Construct, uniqueid: string, props: CustomResourceProviderProps): CRProvider {
const id = `${uniqueid}CustomResourceProvider`;
const stack = Stack.of(scope);
const provider = stack.node.tryFindChild(id) as CRProvider
?? new CRProvider(stack, id, props);

return provider;
}

private readonly resourceArns = new Set<string>();
constructor(scope: Construct, id: string, props: CustomResourceProviderProps) {
super(scope, id, props);
this.addToRolePolicy({
Effect: 'Allow',
Resource: Lazy.list({ produce: () => Array.from(this.resourceArns) }),
Action: [
'ssm:DeleteParameters',
'ssm:ListTagsForResource',
'ssm:GetParameters',
'ssm:PutParameter',
],
});
}

/**
* Add a resource ARN to the existing policy statement
*/
public addResourceArn(arn: string): void {
this.resourceArns.add(arn);
}
}

/**
* Creates a custom resource that will return a list of stack exports from a given
* AWS region. The export can then be referenced by the export name.
Expand Down Expand Up @@ -55,34 +92,20 @@ export class ExportWriter extends Construct {
});
}
private readonly _references: CrossRegionExports = {};
private readonly provider: CustomResourceProvider;
private readonly resourceArns = new Set<string>;
private readonly provider: CRProvider;
constructor(scope: Construct, id: string, props: ExportWriterProps) {
super(scope, id);
const stack = Stack.of(this);
const region = props.region ?? stack.region;
this.addRegionToPolicy(region);

const resourceType = 'Custom::CrossRegionExportWriter';
this.provider = CustomResourceProvider.getOrCreateProvider(this, resourceType, {
this.provider = CRProvider.getOrCreateProvider(this, resourceType, {
codeDirectory: path.join(__dirname, 'cross-region-ssm-writer-handler'),
runtime: builtInCustomResourceProviderNodeRuntime(this),
policyStatements: [{
Effect: 'Allow',
Resource: Lazy.list({
produce: () => [
...Array.from(this.resourceArns),
],
}),
Action: [
'ssm:DeleteParameters',
'ssm:ListTagsForResource',
'ssm:GetParameters',
'ssm:PutParameter',
],
}],
});

this.addRegionToPolicy(region);

const properties: ExportWriterCRProps = {
region: region,
exports: Lazy.any({ produce: () => this._references }),
Expand Down Expand Up @@ -126,14 +149,12 @@ export class ExportWriter extends Construct {
*/
private addRegionToPolicy(region: string): void {
if (!Token.isUnresolved(region)) {
this.resourceArns.add(
Stack.of(this).formatArn({
service: 'ssm',
resource: 'parameter',
region,
resourceName: `${SSM_EXPORT_PATH_PREFIX}*`,
}),
);
this.provider.addResourceArn(Stack.of(this).formatArn({
service: 'ssm',
resource: 'parameter',
region,
resourceName: `${SSM_EXPORT_PATH_PREFIX}*`,
}));
}
}

Expand Down
6 changes: 3 additions & 3 deletions packages/aws-cdk-lib/core/lib/private/refs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
// CROSS REFERENCES
// ----------------------------------------------------

import * as cxapi from '../../../cx-api';
import { IConstruct } from 'constructs';
import { CfnReference } from './cfn-reference';
import { Intrinsic } from './intrinsic';
import { findTokens } from './resolve';
import { makeUniqueId } from './uniqueid';
import * as cxapi from '../../../cx-api';
import { CfnElement } from '../cfn-element';
import { CfnOutput } from '../cfn-output';
import { CfnParameter } from '../cfn-parameter';
Expand Down Expand Up @@ -233,11 +233,11 @@ function createCrossRegionImportValue(reference: Reference, importStack: Stack):

// get or create the export writer
const writerConstructName = makeUniqueId(['ExportsWriter', importStack.region]);
const exportReader = ExportWriter.getOrCreate(exportingStack, writerConstructName, {
const exportWriter = ExportWriter.getOrCreate(exportingStack, writerConstructName, {
region: importStack.region,
});

const exported = exportReader.exportValue(exportName, reference, importStack);
const exported = exportWriter.exportValue(exportName, reference, importStack);
if (importStack.nestedStackParent) {
return createNestedStackParameter(importStack, (exported as CfnReference), exported);
}
Expand Down
Loading

0 comments on commit 65265e1

Please sign in to comment.