-
Notifications
You must be signed in to change notification settings - Fork 3.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(core): Fn.findInMap supports default value #26543
Changes from 12 commits
ae65409
2ebdad8
376c505
d03b8f8
7c427cd
7e61955
642a69e
e550c7f
75c6765
3b671b5
9ea3be0
1a0b7f0
8f3366b
08f4b74
159171a
bb555f3
2a48975
b3fc6ae
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
import * as cdk from 'aws-cdk-lib/core'; | ||
import { Bucket } from 'aws-cdk-lib/aws-s3'; | ||
import { IntegTest } from '@aws-cdk/integ-tests-alpha'; | ||
|
||
const app = new cdk.App(); | ||
|
||
const stack = new cdk.Stack(app, 'core-cfn-mapping-1'/*,{ env }*/); | ||
|
||
const backing = { | ||
TopLevelKey1: { | ||
SecondLevelKey1: 'Yes', | ||
SecondLevelKey2: 'No', | ||
}, | ||
}; | ||
|
||
const mapping = new cdk.CfnMapping(stack, 'Regular mapping', { | ||
mapping: backing, | ||
}); | ||
|
||
const lazyMapping = new cdk.CfnMapping(stack, 'Lazy mapping', { | ||
mapping: backing, | ||
lazy: true, | ||
}); | ||
|
||
const defValue = 'foob'; | ||
const defValue2 = 'bart'; | ||
|
||
const mapYes = mapping.findInMap('TopLevelKey1', 'SecondLevelKey1', defValue); // resolve to 'Yes' | ||
const mapDefault = mapping.findInMap('TopLevelKey1', cdk.Aws.REGION, defValue); // resolve to 'foob' | ||
const mapFn = cdk.Fn.findInMap(mapping.logicalId, 'TopLevelKey1', 'SecondLevelKey3', defValue); // resolve to 'foob' | ||
|
||
const lazyNo = lazyMapping.findInMap('TopLevelKey1', 'SecondLevelKey2', defValue2); // short circuit to 'No' | ||
const lazyDefault = lazyMapping.findInMap('TopLevelKey2', 'SecondLevelKey2', defValue2); // short circuit to 'bart' | ||
const lazyResolve = lazyMapping.findInMap(cdk.Aws.REGION, 'SecondLevelKey2', defValue2); // resolve to 'bart' | ||
|
||
new cdk.CfnOutput(stack, 'Output0', { value: mapYes }); | ||
new cdk.CfnOutput(stack, 'Output1', { value: mapDefault }); | ||
new cdk.CfnOutput(stack, 'Output2', { value: mapFn }); | ||
new cdk.CfnOutput(stack, 'Output3', { value: lazyNo }); | ||
new cdk.CfnOutput(stack, 'Output4', { value: lazyDefault }); | ||
new cdk.CfnOutput(stack, 'Output5', { value: lazyResolve }); | ||
|
||
new Bucket(stack, 'CfnMappingFindInMapBucket'); | ||
|
||
new IntegTest(app, 'CfnMappingFindInMapTest', { | ||
testCases: [stack], | ||
}); | ||
|
||
app.synth(); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,8 +40,8 @@ export interface CfnMappingProps { | |
export class CfnMapping extends CfnRefElement { | ||
private mapping: Mapping; | ||
private readonly lazy?: boolean; | ||
private lazyRender = false; | ||
private lazyInformed = false; | ||
private lazyRender = false; // prescribes `_toCloudFormation()` to pass nothing if value from map is returned lazily. | ||
private lazyInformed = false; // keeps track if user has been sent a message informing them of the possibility to use lazy synthesis. | ||
|
||
constructor(scope: Construct, id: string, props: CfnMappingProps = {}) { | ||
super(scope, id); | ||
|
@@ -63,30 +63,41 @@ export class CfnMapping extends CfnRefElement { | |
} | ||
|
||
/** | ||
* @returns A reference to a value in the map based on the two keys. | ||
* @returns A reference to a value in the map based on the two keys. If mapping is lazy, the value from the map or default value is returned instead of the reference. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm pretty sure "lazy" means: only render the actual mapping to the template if there is at least one value that actually needs to be looked up from it at deploy time. Conversely: if no value is ever read from the map, or all values can be resolved at synth time, then there is no need to render the map. The line of documentation you added here doesn't help me understand that :). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Felt that it wasn't accurate to say it returns a reference is sometimes it just passes a value. Also could help debug if the short circuit was not documented. Can add a line about the mapping behavior too |
||
*/ | ||
public findInMap(key1: string, key2: string): string { | ||
public findInMap(key1: string, key2: string, defaultValue?: string): string { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ohmigod. Can this method be simplified if we reorganize it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would be glad to, but not seeing a clear way. Think we ship it and someone enterprising can tidy it up later. |
||
let fullyResolved = false; | ||
let notInMap = false; | ||
if (!Token.isUnresolved(key1)) { | ||
if (!(key1 in this.mapping)) { | ||
throw new Error(`Mapping doesn't contain top-level key '${key1}'`); | ||
} | ||
if (!Token.isUnresolved(key2)) { | ||
if (defaultValue === undefined) { | ||
throw new Error(`Mapping doesn't contain top-level key '${key1}'`); | ||
} else { | ||
notInMap = true; | ||
} | ||
} else if (!Token.isUnresolved(key2)) { | ||
if (!(key2 in this.mapping[key1])) { | ||
throw new Error(`Mapping doesn't contain second-level key '${key2}'`); | ||
if (defaultValue === undefined) { | ||
throw new Error(`Mapping doesn't contain second-level key '${key2}'`); | ||
} else { | ||
notInMap = true; | ||
} | ||
} | ||
fullyResolved = true; | ||
} | ||
} | ||
if (fullyResolved) { | ||
if (this.lazy) { | ||
|
||
if (this.lazy) { | ||
if (notInMap && defaultValue !== undefined) { | ||
return defaultValue; | ||
} else if (fullyResolved) { | ||
return this.mapping[key1][key2]; | ||
} | ||
} else { | ||
this.lazyRender = true; | ||
} | ||
|
||
return new CfnMappingEmbedder(this, this.mapping, key1, key2).toString(); | ||
this.lazyRender = !fullyResolved; | ||
|
||
return new CfnMappingEmbedder(this, this.mapping, key1, key2, defaultValue).toString(); | ||
} | ||
|
||
/** | ||
|
@@ -130,12 +141,16 @@ export class CfnMapping extends CfnRefElement { | |
class CfnMappingEmbedder implements IResolvable { | ||
readonly creationStack: string[] = []; | ||
|
||
constructor(private readonly cfnMapping: CfnMapping, readonly mapping: Mapping, private readonly key1: string, private readonly key2: string) { } | ||
constructor(private readonly cfnMapping: CfnMapping, | ||
readonly mapping: Mapping, | ||
private readonly key1: string, | ||
private readonly key2: string, | ||
private readonly defaultValue?: string) { } | ||
|
||
public resolve(context: IResolveContext): string { | ||
const consumingStack = Stack.of(context.scope); | ||
if (consumingStack === Stack.of(this.cfnMapping)) { | ||
return Fn.findInMap(this.cfnMapping.logicalId, this.key1, this.key2); | ||
return Fn.findInMap(this.cfnMapping.logicalId, this.key1, this.key2, this.defaultValue); | ||
} | ||
|
||
const constructScope = consumingStack; | ||
|
@@ -148,7 +163,7 @@ class CfnMappingEmbedder implements IResolvable { | |
}); | ||
} | ||
|
||
return Fn.findInMap(mappingCopy.logicalId, this.key1, this.key2); | ||
return Fn.findInMap(mappingCopy.logicalId, this.key1, this.key2, this.defaultValue); | ||
} | ||
|
||
public toString() { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this how we like to convey warnings / best practices?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you should explain succinctly why you should not use with lazy maps