From 413b64347f333573b2a07150e87244bd4c11d264 Mon Sep 17 00:00:00 2001 From: Romain Marcadier Date: Tue, 21 Mar 2023 13:36:34 +0100 Subject: [PATCH] fix(sfn): stop replacing JsonPath.DISCARD with `null` (#24717) Follow-up to #24593. The `renderJsonPath` function is subsituting a literal `null` for `JsonPath.DISCARD`, which results in the key being dropped if the value is sent across a language boundary, which effectively changes semantics. The `JsonPath.DISCARD` value is a `Token` that ultimately resolves to `null`, and it must be preserved as such so that it is safe to exchange across languages. Thanks to @beck3905 for reporting & diagnosing this. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../aws-stepfunctions/lib/states/state.ts | 3 +- .../aws-stepfunctions/test/state.test.ts | 29 ++++++++++++------- .../core/lib/private/cloudformation-lang.ts | 2 +- packages/@aws-cdk/core/lib/private/resolve.ts | 8 +++++ packages/@aws-cdk/core/lib/token.ts | 10 +++++-- 5 files changed, 36 insertions(+), 16 deletions(-) diff --git a/packages/@aws-cdk/aws-stepfunctions/lib/states/state.ts b/packages/@aws-cdk/aws-stepfunctions/lib/states/state.ts index 66869e282b163..901743caec625 100644 --- a/packages/@aws-cdk/aws-stepfunctions/lib/states/state.ts +++ b/packages/@aws-cdk/aws-stepfunctions/lib/states/state.ts @@ -1,7 +1,7 @@ import { Token } from '@aws-cdk/core'; import { IConstruct, Construct, Node } from 'constructs'; import { Condition } from '../condition'; -import { FieldUtils, JsonPath } from '../fields'; +import { FieldUtils } from '../fields'; import { StateGraph } from '../state-graph'; import { CatchProps, Errors, IChainable, INextable, RetryProps } from '../types'; @@ -578,7 +578,6 @@ export function renderList(xs: T[], mapFn: (x: T) => any, sortFn?: (a: T, b: */ export function renderJsonPath(jsonPath?: string): undefined | null | string { if (jsonPath === undefined) { return undefined; } - if (jsonPath === JsonPath.DISCARD) { return null; } if (!Token.isUnresolved(jsonPath) && !jsonPath.startsWith('$')) { throw new Error(`Expected JSON path to start with '$', got: ${jsonPath}`); diff --git a/packages/@aws-cdk/aws-stepfunctions/test/state.test.ts b/packages/@aws-cdk/aws-stepfunctions/test/state.test.ts index 0b2157f7526ec..d878935dd5056 100644 --- a/packages/@aws-cdk/aws-stepfunctions/test/state.test.ts +++ b/packages/@aws-cdk/aws-stepfunctions/test/state.test.ts @@ -1,26 +1,33 @@ +import * as assert from '@aws-cdk/assertions'; import * as cdk from '@aws-cdk/core'; import { FakeTask } from './fake-task'; -import { renderGraph } from './private/render-util'; -import { JsonPath } from '../lib'; +import { JsonPath, StateMachine } from '../lib'; test('JsonPath.DISCARD can be used to discard a state\'s output', () => { - const stack = new cdk.Stack(); - + // GIVEN + const app = new cdk.App(); + const stack = new cdk.Stack(app, 'TestStack'); const task = new FakeTask(stack, 'my-state', { inputPath: JsonPath.DISCARD, outputPath: JsonPath.DISCARD, resultPath: JsonPath.DISCARD, }); + new StateMachine(stack, 'state-machine', { + definition: task, + }); + + // WHEN + const definitionString = new assert.Capture(); + assert.Template.fromStack(stack).hasResourceProperties('AWS::StepFunctions::StateMachine', { + DefinitionString: definitionString, + }); + + // THEN + const definition = JSON.parse(definitionString.asString()); - expect(renderGraph(task)).toEqual({ - StartAt: 'my-state', + expect(definition).toMatchObject({ States: { 'my-state': { - End: true, - Type: 'Task', - Resource: expect.any(String), - Parameters: expect.any(Object), - // The important bits: InputPath: null, OutputPath: null, ResultPath: null, diff --git a/packages/@aws-cdk/core/lib/private/cloudformation-lang.ts b/packages/@aws-cdk/core/lib/private/cloudformation-lang.ts index c31d097c4f9f9..9e278eebf8a00 100644 --- a/packages/@aws-cdk/core/lib/private/cloudformation-lang.ts +++ b/packages/@aws-cdk/core/lib/private/cloudformation-lang.ts @@ -452,4 +452,4 @@ class ScopedCache { } } -const stringifyCache = new ScopedCache(); \ No newline at end of file +const stringifyCache = new ScopedCache(); diff --git a/packages/@aws-cdk/core/lib/private/resolve.ts b/packages/@aws-cdk/core/lib/private/resolve.ts index 9041ecfc4f15d..b16ab314182f0 100644 --- a/packages/@aws-cdk/core/lib/private/resolve.ts +++ b/packages/@aws-cdk/core/lib/private/resolve.ts @@ -192,6 +192,14 @@ export function resolve(obj: any, options: IResolveOptions): any { return arr; } + // + // literal null -- from JsonNull resolution, preserved as-is (semantically meaningful) + // + + if (obj === null) { + return obj; + } + // // tokens - invoke 'resolve' and continue to resolve recursively // diff --git a/packages/@aws-cdk/core/lib/token.ts b/packages/@aws-cdk/core/lib/token.ts index 0ff003b2c28c5..c6af813dd6da2 100644 --- a/packages/@aws-cdk/core/lib/token.ts +++ b/packages/@aws-cdk/core/lib/token.ts @@ -4,7 +4,7 @@ import { unresolved } from './private/encoding'; import { Intrinsic } from './private/intrinsic'; import { resolve } from './private/resolve'; import { TokenMap } from './private/token-map'; -import { IResolvable, ITokenResolver } from './resolvable'; +import { IResolvable, ITokenResolver, IResolveContext } from './resolvable'; import { TokenizedStringFragments } from './string-fragments'; /** @@ -236,12 +236,18 @@ export class Tokenization { * An object which serializes to the JSON `null` literal, and which can safely * be passed across languages where `undefined` and `null` are not different. */ -export class JsonNull { +export class JsonNull implements IResolvable { /** The canonical instance of `JsonNull`. */ public static readonly INSTANCE = new JsonNull(); + public readonly creationStack: string[] = []; + private constructor() { } + public resolve(_ctx: IResolveContext): any { + return null; + } + /** Obtains the JSON representation of this object (`null`) */ public toJSON(): any { return null;