From 443251d609b00d7fc2501bfc28d3722720f40caa Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Sat, 22 Mar 2025 15:44:10 +0100 Subject: [PATCH 1/7] fix: context provider's `ignoreErrorOnMissingContext` parameter is misleading 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 `ignoreFailedLookup` instead. Also explain the `dummyValue` field and this one a bit better. --- packages/aws-cdk-lib/aws-kms/lib/key.ts | 2 +- packages/aws-cdk-lib/aws-ssm/lib/parameter.ts | 2 +- .../aws-cdk-lib/core/lib/context-provider.ts | 68 +++++++++++++++++-- 3 files changed, 65 insertions(+), 7 deletions(-) diff --git a/packages/aws-cdk-lib/aws-kms/lib/key.ts b/packages/aws-cdk-lib/aws-kms/lib/key.ts index f8c9b59961ad0..8f33779e41370 100644 --- a/packages/aws-cdk-lib/aws-kms/lib/key.ts +++ b/packages/aws-cdk-lib/aws-kms/lib/key.ts @@ -741,7 +741,7 @@ export class Key extends KeyBase { dummyValue: { keyId: Key.DEFAULT_DUMMY_KEY_ID, }, - ignoreErrorOnMissingContext: options.returnDummyKeyOnMissing, + ignoreFailedLookup: options.returnDummyKeyOnMissing, }).value; return new Import(attributes.keyId, diff --git a/packages/aws-cdk-lib/aws-ssm/lib/parameter.ts b/packages/aws-cdk-lib/aws-ssm/lib/parameter.ts index a7cbb96283f89..c10dfe4e7fefa 100644 --- a/packages/aws-cdk-lib/aws-ssm/lib/parameter.ts +++ b/packages/aws-cdk-lib/aws-ssm/lib/parameter.ts @@ -585,7 +585,7 @@ export class StringParameter extends ParameterBase implements IStringParameter { provider: cxschema.ContextProvider.SSM_PARAMETER_PROVIDER, props: { parameterName }, dummyValue: defaultValue || `dummy-value-for-${parameterName}`, - ignoreErrorOnMissingContext: defaultValue !== undefined, + ignoreFailedLookup: defaultValue !== undefined, }).value; return value; diff --git a/packages/aws-cdk-lib/core/lib/context-provider.ts b/packages/aws-cdk-lib/core/lib/context-provider.ts index b58405098174b..9a5d314e78666 100644 --- a/packages/aws-cdk-lib/core/lib/context-provider.ts +++ b/packages/aws-cdk-lib/core/lib/context-provider.ts @@ -30,16 +30,66 @@ export interface GetContextKeyOptions { */ export interface GetContextValueOptions extends GetContextKeyOptions { /** - * The value to return if the context value was not found and a missing - * context is reported. + * The value to return if the lookup has not yet been performed. + * + * Upon first synthesis, the lookups has not yet been performed. The + * `getValue()` operation returns this value instead, so that synthesis can + * proceed. After synthesis completes the first time, the actual lookup will + * be performed and synthesis will run again with the *real* value. + * + * Dummy values should preferably have valid shapes so that downstream + * consumers of lookup values don't throw validation exceptions if they + * encounter a dummy value (or all possible downstream consumers need to + * effectively check for the well-known shape of the dummy value); throwing an + * exception would error out the synthesis operation and prevent the lookup + * and the second, real, synthesis from happening. + * + * ## Link to ignoreErrorOnMissingContext + * + * `dummyValue` is also used as the official value to return if the lookup has + * failed and `ignoreErrorOnMissingContext` is set. */ readonly dummyValue: any; /** - * When True, the context provider will not throw an error if missing context - * is reported + * Ignore a lookup failure and return the `dummyValue` instead + * + * If this is `true` and the value we tried to look up could not be found, the + * failure is suppressed and `dummyValue` is officially returned instead. + * + * When this happens, `dummyValue` is encoded into cached context and it will + * never be refreshed anymore until the user runs `cdk context --reset `. + * + * Note that it is not possible for the CDK app code to make a distinction + * between "the lookup has not been performed yet" and "the lookup didn't + * find anything and we returned a default value instead". + * + * ## Context providers + * + * This feature must explicitly be supported by context providers. It is + * currently supported by: + * + * - KMS key provider + * - SSM parameter provider + * + * ## Note to implementors + * + * The dummy value should not be returned for all SDK lookup failures. For + * example, "no network" or "no credentials" or "malformed query" should + * not lead to the dummy value being returned. Only the case of "no such + * resource" should. + * + * @default false + */ + readonly ignoreFailedLookup?: boolean; + + /** + * Ignore a lookup failure and return the `dummyValue` instead + * + * Deprecated alias for `ignoredFailedLookup`. * * @default false + * @deprecated Use ignoreFailedLookup instead */ readonly ignoreErrorOnMissingContext?: boolean; } @@ -92,6 +142,10 @@ export class ContextProvider { } public static getValue(scope: Construct, options: GetContextValueOptions): GetContextValueResult { + if ((options.ignoreFailedLookup !== undefined) && (options.ignoreErrorOnMissingContext !== undefined)) { + throw new Error('Only supply one of \'ignoreFailedLookup\' and \'ignoreErrorOnMissingContext\''); + } + const stack = Stack.of(scope); if (Token.isUnresolved(stack.account) || Token.isUnresolved(stack.region)) { @@ -111,7 +165,11 @@ export class ContextProvider { // build a version of the props which includes the dummyValue and ignoreError flag const extendedProps: { [p: string]: any } = { dummyValue: options.dummyValue, - ignoreErrorOnMissingContext: options.ignoreErrorOnMissingContext, + + // Even though we renamed the user-facing property, the field in the + // cloud assembly still has the original name, which is somewhat wrong + // because it's not about missing context. + ignoreErrorOnMissingContext: options.ignoreFailedLookup ?? options.ignoreErrorOnMissingContext, ...props, }; From 100e16b45af29f0cd787239e3bf93a5ece6ef8d7 Mon Sep 17 00:00:00 2001 From: Rico Hermans Date: Mon, 24 Mar 2025 09:59:25 +0100 Subject: [PATCH 2/7] Update packages/aws-cdk-lib/core/lib/context-provider.ts Co-authored-by: Kenta Goto (k.goto) <24818752+go-to-k@users.noreply.github.com> --- packages/aws-cdk-lib/core/lib/context-provider.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/aws-cdk-lib/core/lib/context-provider.ts b/packages/aws-cdk-lib/core/lib/context-provider.ts index 9a5d314e78666..e947c69a63e7e 100644 --- a/packages/aws-cdk-lib/core/lib/context-provider.ts +++ b/packages/aws-cdk-lib/core/lib/context-provider.ts @@ -44,10 +44,10 @@ export interface GetContextValueOptions extends GetContextKeyOptions { * exception would error out the synthesis operation and prevent the lookup * and the second, real, synthesis from happening. * - * ## Link to ignoreErrorOnMissingContext + * ## Link to ignoreFailedLookup * * `dummyValue` is also used as the official value to return if the lookup has - * failed and `ignoreErrorOnMissingContext` is set. + * failed and `ignoreFailedLookup` is set. */ readonly dummyValue: any; From b6098b9893657ec9c37455ac49c1debe0c2247a7 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Mon, 24 Mar 2025 19:44:05 +0100 Subject: [PATCH 3/7] Rename to `mustExist` --- packages/aws-cdk-lib/aws-kms/lib/key.ts | 2 +- packages/aws-cdk-lib/aws-ssm/lib/parameter.ts | 2 +- .../aws-cdk-lib/core/lib/context-provider.ts | 31 ++++++++++++------- 3 files changed, 21 insertions(+), 14 deletions(-) diff --git a/packages/aws-cdk-lib/aws-kms/lib/key.ts b/packages/aws-cdk-lib/aws-kms/lib/key.ts index 8f33779e41370..7702822976948 100644 --- a/packages/aws-cdk-lib/aws-kms/lib/key.ts +++ b/packages/aws-cdk-lib/aws-kms/lib/key.ts @@ -741,7 +741,7 @@ export class Key extends KeyBase { dummyValue: { keyId: Key.DEFAULT_DUMMY_KEY_ID, }, - ignoreFailedLookup: options.returnDummyKeyOnMissing, + mustExist: options.returnDummyKeyOnMissing === undefined, }).value; return new Import(attributes.keyId, diff --git a/packages/aws-cdk-lib/aws-ssm/lib/parameter.ts b/packages/aws-cdk-lib/aws-ssm/lib/parameter.ts index c10dfe4e7fefa..778a4beb921c1 100644 --- a/packages/aws-cdk-lib/aws-ssm/lib/parameter.ts +++ b/packages/aws-cdk-lib/aws-ssm/lib/parameter.ts @@ -585,7 +585,7 @@ export class StringParameter extends ParameterBase implements IStringParameter { provider: cxschema.ContextProvider.SSM_PARAMETER_PROVIDER, props: { parameterName }, dummyValue: defaultValue || `dummy-value-for-${parameterName}`, - ignoreFailedLookup: defaultValue !== undefined, + mustExist: defaultValue === undefined, }).value; return value; diff --git a/packages/aws-cdk-lib/core/lib/context-provider.ts b/packages/aws-cdk-lib/core/lib/context-provider.ts index e947c69a63e7e..612e16e863132 100644 --- a/packages/aws-cdk-lib/core/lib/context-provider.ts +++ b/packages/aws-cdk-lib/core/lib/context-provider.ts @@ -44,17 +44,20 @@ export interface GetContextValueOptions extends GetContextKeyOptions { * exception would error out the synthesis operation and prevent the lookup * and the second, real, synthesis from happening. * - * ## Link to ignoreFailedLookup + * ## Link to mustExist * * `dummyValue` is also used as the official value to return if the lookup has - * failed and `ignoreFailedLookup` is set. + * failed and `mustExist == false`. */ readonly dummyValue: any; /** - * Ignore a lookup failure and return the `dummyValue` instead + * Whether the resource must exist + * + * If this is set (the default), the query fails if the value or resource we + * tried to look up doesn't exist. * - * If this is `true` and the value we tried to look up could not be found, the + * If this is `false` and the value we tried to look up could not be found, the * failure is suppressed and `dummyValue` is officially returned instead. * * When this happens, `dummyValue` is encoded into cached context and it will @@ -79,17 +82,18 @@ export interface GetContextValueOptions extends GetContextKeyOptions { * not lead to the dummy value being returned. Only the case of "no such * resource" should. * - * @default false + * @default true */ - readonly ignoreFailedLookup?: boolean; + readonly mustExist?: boolean; /** * Ignore a lookup failure and return the `dummyValue` instead * - * Deprecated alias for `ignoredFailedLookup`. + * `mustExist` is the recommended alias for this deprecated + * property (note that its value is reversed). * * @default false - * @deprecated Use ignoreFailedLookup instead + * @deprecated Use mustExist instead */ readonly ignoreErrorOnMissingContext?: boolean; } @@ -142,8 +146,8 @@ export class ContextProvider { } public static getValue(scope: Construct, options: GetContextValueOptions): GetContextValueResult { - if ((options.ignoreFailedLookup !== undefined) && (options.ignoreErrorOnMissingContext !== undefined)) { - throw new Error('Only supply one of \'ignoreFailedLookup\' and \'ignoreErrorOnMissingContext\''); + if ((options.mustExist !== undefined) && (options.ignoreErrorOnMissingContext !== undefined)) { + throw new Error('Only supply one of \'mustExist\' and \'ignoreErrorOnMissingContext\''); } const stack = Stack.of(scope); @@ -162,14 +166,17 @@ export class ContextProvider { // if context is missing or an error occurred during context retrieval, // report and return a dummy value. if (value === undefined || providerError !== undefined) { + const ignoreErrorOnMissingContext = options.mustExist !== undefined ? !options.mustExist : options.ignoreErrorOnMissingContext; + // build a version of the props which includes the dummyValue and ignoreError flag const extendedProps: { [p: string]: any } = { dummyValue: options.dummyValue, // Even though we renamed the user-facing property, the field in the // cloud assembly still has the original name, which is somewhat wrong - // because it's not about missing context. - ignoreErrorOnMissingContext: options.ignoreFailedLookup ?? options.ignoreErrorOnMissingContext, + // because it's not about missing context. Only render 'true' or don't + // render at all. + ignoreErrorOnMissingContext: ignoreErrorOnMissingContext ? true : undefined, ...props, }; From 845039a3880eb3048dce1fecf9d9e7e5cd03391d Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Mon, 24 Mar 2025 19:45:49 +0100 Subject: [PATCH 4/7] oops boolean --- packages/aws-cdk-lib/aws-kms/lib/key.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/aws-cdk-lib/aws-kms/lib/key.ts b/packages/aws-cdk-lib/aws-kms/lib/key.ts index 7702822976948..ac6da4b4d09bf 100644 --- a/packages/aws-cdk-lib/aws-kms/lib/key.ts +++ b/packages/aws-cdk-lib/aws-kms/lib/key.ts @@ -741,7 +741,7 @@ export class Key extends KeyBase { dummyValue: { keyId: Key.DEFAULT_DUMMY_KEY_ID, }, - mustExist: options.returnDummyKeyOnMissing === undefined, + mustExist: !options.returnDummyKeyOnMissing, }).value; return new Import(attributes.keyId, From 27c956372d74228e8188df28baabab697b4a9ec4 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Mon, 24 Mar 2025 19:50:05 +0100 Subject: [PATCH 5/7] Link => relationship --- packages/aws-cdk-lib/core/lib/context-provider.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/aws-cdk-lib/core/lib/context-provider.ts b/packages/aws-cdk-lib/core/lib/context-provider.ts index 612e16e863132..205ae88809c20 100644 --- a/packages/aws-cdk-lib/core/lib/context-provider.ts +++ b/packages/aws-cdk-lib/core/lib/context-provider.ts @@ -44,7 +44,7 @@ export interface GetContextValueOptions extends GetContextKeyOptions { * exception would error out the synthesis operation and prevent the lookup * and the second, real, synthesis from happening. * - * ## Link to mustExist + * ## Relationship to mustExist * * `dummyValue` is also used as the official value to return if the lookup has * failed and `mustExist == false`. From c500cfc0d80e298febc107d4dba3ed237f675960 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Mon, 24 Mar 2025 19:51:58 +0100 Subject: [PATCH 6/7] relationship => connection --- packages/aws-cdk-lib/core/lib/context-provider.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/aws-cdk-lib/core/lib/context-provider.ts b/packages/aws-cdk-lib/core/lib/context-provider.ts index 205ae88809c20..614383e22f275 100644 --- a/packages/aws-cdk-lib/core/lib/context-provider.ts +++ b/packages/aws-cdk-lib/core/lib/context-provider.ts @@ -44,7 +44,7 @@ export interface GetContextValueOptions extends GetContextKeyOptions { * exception would error out the synthesis operation and prevent the lookup * and the second, real, synthesis from happening. * - * ## Relationship to mustExist + * ## Connection to mustExist * * `dummyValue` is also used as the official value to return if the lookup has * failed and `mustExist == false`. From 64674548b6a767f0d63436988d9d6720a1f36db4 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Tue, 25 Mar 2025 11:29:05 +0100 Subject: [PATCH 7/7] No changes! --- packages/aws-cdk-lib/core/lib/context-provider.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/aws-cdk-lib/core/lib/context-provider.ts b/packages/aws-cdk-lib/core/lib/context-provider.ts index 614383e22f275..81ea9d9227cde 100644 --- a/packages/aws-cdk-lib/core/lib/context-provider.ts +++ b/packages/aws-cdk-lib/core/lib/context-provider.ts @@ -166,7 +166,10 @@ export class ContextProvider { // if context is missing or an error occurred during context retrieval, // report and return a dummy value. if (value === undefined || providerError !== undefined) { - const ignoreErrorOnMissingContext = options.mustExist !== undefined ? !options.mustExist : options.ignoreErrorOnMissingContext; + // Render 'ignoreErrorOnMissingContext' iff one of the parameters is supplied. + const ignoreErrorOnMissingContext = options.mustExist !== undefined || options.ignoreErrorOnMissingContext !== undefined + ? (options.mustExist !== undefined ? !options.mustExist : options.ignoreErrorOnMissingContext) + : undefined; // build a version of the props which includes the dummyValue and ignoreError flag const extendedProps: { [p: string]: any } = { @@ -174,9 +177,8 @@ export class ContextProvider { // Even though we renamed the user-facing property, the field in the // cloud assembly still has the original name, which is somewhat wrong - // because it's not about missing context. Only render 'true' or don't - // render at all. - ignoreErrorOnMissingContext: ignoreErrorOnMissingContext ? true : undefined, + // because it's not about missing context. + ignoreErrorOnMissingContext, ...props, };