Skip to content

Commit 6f4dcfd

Browse files
authored
fix(core): the string 'undefined' is recognized as a valid partition when looking up for fact values (#23023)
When the partition value is the literal string 'undefined' (because it was stringified from the context), it should be treated as if it is actually `undefined`. When looking up a fact value, the two values are already considered the same in one code path, but are then treated differently in the immediate logic. This is causing a new integration test to fail. This commit makes the logic consistent. ---- ### 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*
1 parent dc8f87a commit 6f4dcfd

File tree

2 files changed

+29
-1
lines changed

2 files changed

+29
-1
lines changed

Diff for: packages/@aws-cdk/core/lib/stack.ts

+4-1
Original file line numberDiff line numberDiff line change
@@ -960,7 +960,10 @@ export class Stack extends Construct implements ITaggable {
960960
throw new Error(`Context value '${cxapi.TARGET_PARTITIONS}' should be a list of strings, got: ${JSON.stringify(partitions)}`);
961961
}
962962

963-
const lookupMap = partitions ? RegionInfo.limitedRegionMap(factName, partitions) : RegionInfo.regionMap(factName);
963+
const lookupMap =
964+
partitions !== undefined && partitions !== 'undefined'
965+
? RegionInfo.limitedRegionMap(factName, partitions)
966+
: RegionInfo.regionMap(factName);
964967

965968
return deployTimeLookup(this, factName, lookupMap, defaultValue);
966969
}

Diff for: packages/@aws-cdk/core/test/stack.test.ts

+25
Original file line numberDiff line numberDiff line change
@@ -1719,6 +1719,31 @@ describe('regionalFact', () => {
17191719
expect(stack.regionalFact('MyFact')).toEqual('x.amazonaws.com');
17201720
});
17211721

1722+
test('regional facts use the global lookup map if partition is the literal string of "undefined"', () => {
1723+
const stack = new Stack();
1724+
Node.of(stack).setContext(cxapi.TARGET_PARTITIONS, 'undefined');
1725+
new CfnOutput(stack, 'TheFact', {
1726+
value: stack.regionalFact('WeirdFact'),
1727+
});
1728+
1729+
expect(toCloudFormation(stack)).toEqual({
1730+
Mappings: {
1731+
WeirdFactMap: {
1732+
'eu-west-1': { value: 'otherformat' },
1733+
'us-east-1': { value: 'oneformat' },
1734+
},
1735+
},
1736+
Outputs: {
1737+
TheFact: {
1738+
Value: {
1739+
'Fn::FindInMap': ['WeirdFactMap', { Ref: 'AWS::Region' }, 'value'],
1740+
},
1741+
},
1742+
},
1743+
});
1744+
1745+
});
1746+
17221747
test('regional facts generate a mapping if necessary', () => {
17231748
const stack = new Stack();
17241749
new CfnOutput(stack, 'TheFact', {

0 commit comments

Comments
 (0)