Skip to content

Commit b3187b9

Browse files
authored
fix: context provider's ignoreErrorOnMissingContext parameter is misleading (#33875)
In `ContextProvider.getValue()`, `ignoreErrorOnMissingContext` is a request to the CLI's context provider to not fail the lookup, but return the dummy value instead. The operation doesn't have anything to do with missing context, and missing context isn't an error. Deprecate that parameter and add `mustExist` instead (with reversed semantics). Also explain the `dummyValue` field and this one a bit better. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 7f2ed96 commit b3187b9

File tree

3 files changed

+74
-7
lines changed

3 files changed

+74
-7
lines changed

packages/aws-cdk-lib/aws-kms/lib/key.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -741,7 +741,7 @@ export class Key extends KeyBase {
741741
dummyValue: {
742742
keyId: Key.DEFAULT_DUMMY_KEY_ID,
743743
},
744-
ignoreErrorOnMissingContext: options.returnDummyKeyOnMissing,
744+
mustExist: !options.returnDummyKeyOnMissing,
745745
}).value;
746746

747747
return new Import(attributes.keyId,

packages/aws-cdk-lib/aws-ssm/lib/parameter.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -585,7 +585,7 @@ export class StringParameter extends ParameterBase implements IStringParameter {
585585
provider: cxschema.ContextProvider.SSM_PARAMETER_PROVIDER,
586586
props: { parameterName },
587587
dummyValue: defaultValue || `dummy-value-for-${parameterName}`,
588-
ignoreErrorOnMissingContext: defaultValue !== undefined,
588+
mustExist: defaultValue === undefined,
589589
}).value;
590590

591591
return value;

packages/aws-cdk-lib/core/lib/context-provider.ts

+72-5
Original file line numberDiff line numberDiff line change
@@ -30,16 +30,70 @@ export interface GetContextKeyOptions {
3030
*/
3131
export interface GetContextValueOptions extends GetContextKeyOptions {
3232
/**
33-
* The value to return if the context value was not found and a missing
34-
* context is reported.
33+
* The value to return if the lookup has not yet been performed.
34+
*
35+
* Upon first synthesis, the lookups has not yet been performed. The
36+
* `getValue()` operation returns this value instead, so that synthesis can
37+
* proceed. After synthesis completes the first time, the actual lookup will
38+
* be performed and synthesis will run again with the *real* value.
39+
*
40+
* Dummy values should preferably have valid shapes so that downstream
41+
* consumers of lookup values don't throw validation exceptions if they
42+
* encounter a dummy value (or all possible downstream consumers need to
43+
* effectively check for the well-known shape of the dummy value); throwing an
44+
* exception would error out the synthesis operation and prevent the lookup
45+
* and the second, real, synthesis from happening.
46+
*
47+
* ## Connection to mustExist
48+
*
49+
* `dummyValue` is also used as the official value to return if the lookup has
50+
* failed and `mustExist == false`.
3551
*/
3652
readonly dummyValue: any;
3753

3854
/**
39-
* When True, the context provider will not throw an error if missing context
40-
* is reported
55+
* Whether the resource must exist
56+
*
57+
* If this is set (the default), the query fails if the value or resource we
58+
* tried to look up doesn't exist.
59+
*
60+
* If this is `false` and the value we tried to look up could not be found, the
61+
* failure is suppressed and `dummyValue` is officially returned instead.
62+
*
63+
* When this happens, `dummyValue` is encoded into cached context and it will
64+
* never be refreshed anymore until the user runs `cdk context --reset <key>`.
65+
*
66+
* Note that it is not possible for the CDK app code to make a distinction
67+
* between "the lookup has not been performed yet" and "the lookup didn't
68+
* find anything and we returned a default value instead".
69+
*
70+
* ## Context providers
71+
*
72+
* This feature must explicitly be supported by context providers. It is
73+
* currently supported by:
74+
*
75+
* - KMS key provider
76+
* - SSM parameter provider
77+
*
78+
* ## Note to implementors
79+
*
80+
* The dummy value should not be returned for all SDK lookup failures. For
81+
* example, "no network" or "no credentials" or "malformed query" should
82+
* not lead to the dummy value being returned. Only the case of "no such
83+
* resource" should.
84+
*
85+
* @default true
86+
*/
87+
readonly mustExist?: boolean;
88+
89+
/**
90+
* Ignore a lookup failure and return the `dummyValue` instead
91+
*
92+
* `mustExist` is the recommended alias for this deprecated
93+
* property (note that its value is reversed).
4194
*
4295
* @default false
96+
* @deprecated Use mustExist instead
4397
*/
4498
readonly ignoreErrorOnMissingContext?: boolean;
4599
}
@@ -92,6 +146,10 @@ export class ContextProvider {
92146
}
93147

94148
public static getValue(scope: Construct, options: GetContextValueOptions): GetContextValueResult {
149+
if ((options.mustExist !== undefined) && (options.ignoreErrorOnMissingContext !== undefined)) {
150+
throw new Error('Only supply one of \'mustExist\' and \'ignoreErrorOnMissingContext\'');
151+
}
152+
95153
const stack = Stack.of(scope);
96154

97155
if (Token.isUnresolved(stack.account) || Token.isUnresolved(stack.region)) {
@@ -108,10 +166,19 @@ export class ContextProvider {
108166
// if context is missing or an error occurred during context retrieval,
109167
// report and return a dummy value.
110168
if (value === undefined || providerError !== undefined) {
169+
// Render 'ignoreErrorOnMissingContext' iff one of the parameters is supplied.
170+
const ignoreErrorOnMissingContext = options.mustExist !== undefined || options.ignoreErrorOnMissingContext !== undefined
171+
? (options.mustExist !== undefined ? !options.mustExist : options.ignoreErrorOnMissingContext)
172+
: undefined;
173+
111174
// build a version of the props which includes the dummyValue and ignoreError flag
112175
const extendedProps: { [p: string]: any } = {
113176
dummyValue: options.dummyValue,
114-
ignoreErrorOnMissingContext: options.ignoreErrorOnMissingContext,
177+
178+
// Even though we renamed the user-facing property, the field in the
179+
// cloud assembly still has the original name, which is somewhat wrong
180+
// because it's not about missing context.
181+
ignoreErrorOnMissingContext,
115182
...props,
116183
};
117184

0 commit comments

Comments
 (0)