Skip to content

Commit 1135356

Browse files
authored
fix(core): allow override with cross-stack references (#24920)
closes #18882 The problem is described in the original issue, and below is what I found as the root cause and how it can be fixed. --- Previously when we used a cross-stack reference in override, it was not resolved as an `Fn::ImportValue`. To make it an `Fn::ImportValue`, we need to get every token in an app and find _references_ (tokens that references resources outside of its stack) from them. The related code is here: https://github.com/aws/aws-cdk/blob/810d736a8d20638e778c5773507f0edb12733a49/packages/%40aws-cdk/core/lib/private/refs.ts#L139-L140 To get all the tokens in an app, we use `RememberingTokenResolver`, which _remembers_ every token it has found during resolution. So basically this resolver must be used on every resolution to find all the tokens. https://github.com/aws/aws-cdk/blob/810d736a8d20638e778c5773507f0edb12733a49/packages/%40aws-cdk/core/lib/private/resolve.ts#L270-L276 However, the resolver is not used specifically when we resolve tokens in **raw overrides**. Actually the current interface of `postProcess` function of `PostResolveToken` class makes It difficult to use an external resolver. https://github.com/aws/aws-cdk/blob/810d736a8d20638e778c5773507f0edb12733a49/packages/%40aws-cdk/core/lib/cfn-resource.ts#L374-L380 That is why, in this PR, we move the resolution process outside of the `postProcess`, allowing to resolve tokens in raw overrides with the `RememberingTokenResolver` resolver. This change also simplifies the current implementation of deepMerge as a side product. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 59be881 commit 1135356

File tree

5 files changed

+43
-10
lines changed

5 files changed

+43
-10
lines changed

packages/aws-cdk-lib/core/lib/cfn-resource.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,9 @@ import { CfnCreationPolicy, CfnDeletionPolicy, CfnUpdatePolicy } from './cfn-res
77
import { Construct, IConstruct, Node } from 'constructs';
88
import { addDependency, obtainDependencies, removeDependency } from './deps';
99
import { CfnReference } from './private/cfn-reference';
10-
import { CLOUDFORMATION_TOKEN_RESOLVER } from './private/cloudformation-lang';
1110
import { Reference } from './reference';
1211
import { RemovalPolicy, RemovalPolicyOptions } from './removal-policy';
1312
import { TagManager } from './tag-manager';
14-
import { Tokenization } from './token';
1513
import { capitalizePropertyNames, ignoreEmpty, PostResolveToken } from './util';
1614
import { FeatureFlags } from './feature-flags';
1715
import { ResolutionTypeHint } from './type-hints';
@@ -432,15 +430,13 @@ export class CfnResource extends CfnRefElement {
432430
Description: this.cfnOptions.description,
433431
Metadata: ignoreEmpty(this.cfnOptions.metadata),
434432
Condition: this.cfnOptions.condition && this.cfnOptions.condition.logicalId,
435-
}, resourceDef => {
433+
}, (resourceDef, context) => {
436434
const renderedProps = this.renderProperties(resourceDef.Properties || {});
437435
if (renderedProps) {
438436
const hasDefined = Object.values(renderedProps).find(v => v !== undefined);
439437
resourceDef.Properties = hasDefined !== undefined ? renderedProps : undefined;
440438
}
441-
const resolvedRawOverrides = Tokenization.resolve(this.rawOverrides, {
442-
scope: this,
443-
resolver: CLOUDFORMATION_TOKEN_RESOLVER,
439+
const resolvedRawOverrides = context.resolve(this.rawOverrides, {
444440
// we need to preserve the empty elements here,
445441
// as that's how removing overrides are represented as
446442
removeEmpty: false,

packages/aws-cdk-lib/core/lib/resolvable.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,13 @@ export interface ResolveChangeContextOptions {
4444
* @default - Unchanged
4545
*/
4646
readonly allowIntrinsicKeys?: boolean;
47+
48+
/**
49+
* Whether to remove undefined elements from arrays and objects when resolving.
50+
*
51+
* @default - Unchanged
52+
*/
53+
readonly removeEmpty?: boolean;
4754
}
4855

4956
/**

packages/aws-cdk-lib/core/lib/util.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ export function filterUndefined(obj: any): any {
7979
* A Token that applies a function AFTER resolve resolution
8080
*/
8181
export class PostResolveToken extends Intrinsic implements IPostProcessor {
82-
constructor(value: any, private readonly processor: (x: any) => any) {
82+
constructor(value: any, private readonly processor: (x: any, context: IResolveContext) => any) {
8383
super(value, { stackTrace: false });
8484
}
8585

@@ -88,8 +88,8 @@ export class PostResolveToken extends Intrinsic implements IPostProcessor {
8888
return super.resolve(context);
8989
}
9090

91-
public postProcess(o: any, _context: IResolveContext): any {
92-
return this.processor(o);
91+
public postProcess(o: any, context: IResolveContext): any {
92+
return this.processor(o, context);
9393
}
9494
}
9595

packages/aws-cdk-lib/core/test/resource.test.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -821,6 +821,36 @@ describe('resource', () => {
821821
});
822822
});
823823

824+
test('overrides allow cross-stack references', () => {
825+
// GIVEN
826+
const app = new App();
827+
const stack1 = new Stack(app, 'Stack1');
828+
const stack2 = new Stack(app, 'Stack2');
829+
const res1 = new CfnResource(stack1, 'SomeResource1', {
830+
type: 'Some::Resource1',
831+
});
832+
const res2 = new CfnResource(stack2, 'SomeResource2', {
833+
type: 'Some::Resource2',
834+
});
835+
836+
// WHEN
837+
res2.addPropertyOverride('Key', res1.getAtt('Value'));
838+
839+
// THEN
840+
expect(
841+
app.synth().getStackByName(stack2.stackName).template?.Resources,
842+
).toEqual({
843+
SomeResource2: {
844+
Properties: {
845+
Key: {
846+
'Fn::ImportValue': 'Stack1:ExportsOutputFnGetAttSomeResource1Value50DD3EF0',
847+
},
848+
},
849+
Type: 'Some::Resource2',
850+
},
851+
});
852+
});
853+
824854
describe('using mutable properties', () => {
825855
test('can be used by derived classes to specify overrides before render()', () => {
826856
const stack = new Stack();

packages/aws-cdk-lib/core/test/stack.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1498,7 +1498,7 @@ describe('stack', () => {
14981498
public _toCloudFormation() {
14991499
return new PostResolveToken({
15001500
xoo: 1234,
1501-
}, props => {
1501+
}, (props, _context) => {
15021502
validateString(props).assertSuccess();
15031503
});
15041504
}

0 commit comments

Comments
 (0)