Skip to content

Commit

Permalink
fix(core): the string 'undefined' is recognized as a valid partition …
Browse files Browse the repository at this point in the history
…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*
  • Loading branch information
huyphan committed Nov 22, 2022
1 parent dc8f87a commit 6f4dcfd
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 1 deletion.
5 changes: 4 additions & 1 deletion packages/@aws-cdk/core/lib/stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -960,7 +960,10 @@ export class Stack extends Construct implements ITaggable {
throw new Error(`Context value '${cxapi.TARGET_PARTITIONS}' should be a list of strings, got: ${JSON.stringify(partitions)}`);
}

const lookupMap = partitions ? RegionInfo.limitedRegionMap(factName, partitions) : RegionInfo.regionMap(factName);
const lookupMap =
partitions !== undefined && partitions !== 'undefined'
? RegionInfo.limitedRegionMap(factName, partitions)
: RegionInfo.regionMap(factName);

return deployTimeLookup(this, factName, lookupMap, defaultValue);
}
Expand Down
25 changes: 25 additions & 0 deletions packages/@aws-cdk/core/test/stack.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1719,6 +1719,31 @@ describe('regionalFact', () => {
expect(stack.regionalFact('MyFact')).toEqual('x.amazonaws.com');
});

test('regional facts use the global lookup map if partition is the literal string of "undefined"', () => {
const stack = new Stack();
Node.of(stack).setContext(cxapi.TARGET_PARTITIONS, 'undefined');
new CfnOutput(stack, 'TheFact', {
value: stack.regionalFact('WeirdFact'),
});

expect(toCloudFormation(stack)).toEqual({
Mappings: {
WeirdFactMap: {
'eu-west-1': { value: 'otherformat' },
'us-east-1': { value: 'oneformat' },
},
},
Outputs: {
TheFact: {
Value: {
'Fn::FindInMap': ['WeirdFactMap', { Ref: 'AWS::Region' }, 'value'],
},
},
},
});

});

test('regional facts generate a mapping if necessary', () => {
const stack = new Stack();
new CfnOutput(stack, 'TheFact', {
Expand Down

0 comments on commit 6f4dcfd

Please sign in to comment.