From 039e69b0668bf12cb9a1445fa6f36c7b9a0e4d10 Mon Sep 17 00:00:00 2001 From: tmokmss Date: Sun, 16 Jul 2023 14:41:33 +0900 Subject: [PATCH 1/4] fix(cli): hotswap doesn't update SSM parameter environment variables properly --- packages/aws-cdk/lib/api/deploy-stack.ts | 2 +- .../aws-cdk/lib/api/util/cloudformation.ts | 10 +++ .../api/hotswap/hotswap-deployments.test.ts | 77 +++++++++++++++++++ 3 files changed, 88 insertions(+), 1 deletion(-) diff --git a/packages/aws-cdk/lib/api/deploy-stack.ts b/packages/aws-cdk/lib/api/deploy-stack.ts index 416b3e4141d17..28913e263699b 100644 --- a/packages/aws-cdk/lib/api/deploy-stack.ts +++ b/packages/aws-cdk/lib/api/deploy-stack.ts @@ -303,7 +303,7 @@ export async function deployStack(options: DeployStackOptions): Promise!this.formalParams[k]?.Type.startsWith('AWS::SSM::Parameter::Value')), + ); + } + /** * Whether this set of parameter updates will change the actual stack values */ diff --git a/packages/aws-cdk/test/api/hotswap/hotswap-deployments.test.ts b/packages/aws-cdk/test/api/hotswap/hotswap-deployments.test.ts index aab805a441f5b..c89426decb908 100644 --- a/packages/aws-cdk/test/api/hotswap/hotswap-deployments.test.ts +++ b/packages/aws-cdk/test/api/hotswap/hotswap-deployments.test.ts @@ -630,4 +630,81 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot }); } }); + + test('Changes containing an SSM Parameter cannot be hotswapped', async () => { + // GIVEN + setup.setCurrentCfnStackTemplate({ + Parameters: { + SomeParameter: { + Type: 'AWS::SSM::Parameter::Value', + Default: '/some/parameter', + }, + }, + Resources: { + Func: { + Type: 'AWS::Lambda::Function', + Properties: { + Code: { + S3Bucket: 'current-bucket', + S3Key: 'new-key', + }, + Environment: { + Param: { Ref: 'SomeParameter' }, + }, + FunctionName: 'my-function', + }, + Metadata: { + 'aws:asset:path': 'new-path', + }, + }, + }, + }); + const cdkStackArtifact = setup.cdkStackArtifactOf({ + template: { + Parameters: { + SomeParameter: { + Type: 'AWS::SSM::Parameter::Value', + Default: '/some/parameter', + }, + }, + Resources: { + Func: { + Type: 'AWS::Lambda::Function', + Properties: { + Code: { + S3Bucket: 'current-bucket', + S3Key: 'new-key', + }, + Environment: { + Param: { Ref: 'SomeParameter' }, + Key: 'Value', + }, + FunctionName: 'my-function', + }, + Metadata: { + 'aws:asset:path': 'new-path', + }, + }, + }, + }, + }); + + if (hotswapMode === HotswapMode.FALL_BACK) { + // WHEN + const deployStackResult = hotswapMockSdkProvider.tryHotswapDeployment(hotswapMode, cdkStackArtifact); + + // THEN + await expect(deployStackResult).rejects.toThrowError(CfnEvaluationException); + expect(mockUpdateMachineDefinition).not.toHaveBeenCalled(); + expect(mockUpdateLambdaCode).not.toHaveBeenCalled(); + } else if (hotswapMode === HotswapMode.HOTSWAP_ONLY) { + // WHEN + const deployStackResult = hotswapMockSdkProvider.tryHotswapDeployment(hotswapMode, cdkStackArtifact); + + // THEN + await expect(deployStackResult).rejects.toThrowError(CfnEvaluationException); + expect(mockUpdateMachineDefinition).not.toHaveBeenCalled(); + expect(mockUpdateLambdaCode).not.toHaveBeenCalled(); + } + }); }); From a05d2ae1dea47fa3535862a97e6188364e9c5bb5 Mon Sep 17 00:00:00 2001 From: tmokmss Date: Sun, 16 Jul 2023 15:20:47 +0900 Subject: [PATCH 2/4] use resolved value from describeStacks response --- packages/aws-cdk/lib/api/deploy-stack.ts | 2 +- .../aws-cdk/lib/api/util/cloudformation.ts | 12 +---------- .../api/hotswap/hotswap-deployments.test.ts | 21 ++++++++++++------- 3 files changed, 15 insertions(+), 20 deletions(-) diff --git a/packages/aws-cdk/lib/api/deploy-stack.ts b/packages/aws-cdk/lib/api/deploy-stack.ts index 28913e263699b..416b3e4141d17 100644 --- a/packages/aws-cdk/lib/api/deploy-stack.ts +++ b/packages/aws-cdk/lib/api/deploy-stack.ts @@ -303,7 +303,7 @@ export async function deployStack(options: DeployStackOptions): Promise = {}; for (const param of this.stack!.Parameters ?? []) { - ret[param.ParameterKey!] = param.ParameterValue!; + ret[param.ParameterKey!] = param.ResolvedValue ?? param.ParameterValue!; } return ret; } @@ -475,16 +475,6 @@ export class ParameterValues { } } - /** - * The values for SSM parameters cannot be used in hotswap since they are only parameter names, not actual values. - * Excluding those values to be used with hotswap. - */ - public get valuesForHotswap() { - return Object.fromEntries( - Object.entries(this.values).filter(([k, _])=>!this.formalParams[k]?.Type.startsWith('AWS::SSM::Parameter::Value')), - ); - } - /** * Whether this set of parameter updates will change the actual stack values */ diff --git a/packages/aws-cdk/test/api/hotswap/hotswap-deployments.test.ts b/packages/aws-cdk/test/api/hotswap/hotswap-deployments.test.ts index c89426decb908..283f0f1779211 100644 --- a/packages/aws-cdk/test/api/hotswap/hotswap-deployments.test.ts +++ b/packages/aws-cdk/test/api/hotswap/hotswap-deployments.test.ts @@ -6,16 +6,19 @@ import { HotswapMode } from '../../../lib/api/hotswap/common'; let hotswapMockSdkProvider: setup.HotswapMockSdkProvider; let mockUpdateLambdaCode: (params: Lambda.Types.UpdateFunctionCodeRequest) => Lambda.Types.FunctionConfiguration; +let mockUpdateLambdaConfiguration: (params: Lambda.Types.UpdateFunctionConfigurationRequest) => Lambda.Types.FunctionConfiguration; let mockUpdateMachineDefinition: (params: StepFunctions.Types.UpdateStateMachineInput) => StepFunctions.Types.UpdateStateMachineOutput; let mockGetEndpointSuffix: () => string; beforeEach(() => { hotswapMockSdkProvider = setup.setupHotswapTests(); mockUpdateLambdaCode = jest.fn().mockReturnValue({}); + mockUpdateLambdaConfiguration = jest.fn().mockReturnValue({}); mockUpdateMachineDefinition = jest.fn(); mockGetEndpointSuffix = jest.fn(() => 'amazonaws.com'); hotswapMockSdkProvider.stubLambda({ updateFunctionCode: mockUpdateLambdaCode, + updateFunctionConfiguration: mockUpdateLambdaConfiguration, }); hotswapMockSdkProvider.setUpdateStateMachineMock(mockUpdateMachineDefinition); hotswapMockSdkProvider.stubGetEndpointSuffix(mockGetEndpointSuffix); @@ -691,20 +694,22 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot if (hotswapMode === HotswapMode.FALL_BACK) { // WHEN - const deployStackResult = hotswapMockSdkProvider.tryHotswapDeployment(hotswapMode, cdkStackArtifact); + const deployStackResult = await hotswapMockSdkProvider.tryHotswapDeployment(hotswapMode, cdkStackArtifact, { + SomeParameter: 'someValue', + }); // THEN - await expect(deployStackResult).rejects.toThrowError(CfnEvaluationException); - expect(mockUpdateMachineDefinition).not.toHaveBeenCalled(); - expect(mockUpdateLambdaCode).not.toHaveBeenCalled(); + expect(deployStackResult).not.toBeUndefined; + expect(mockUpdateLambdaConfiguration).toHaveBeenCalled(); } else if (hotswapMode === HotswapMode.HOTSWAP_ONLY) { // WHEN - const deployStackResult = hotswapMockSdkProvider.tryHotswapDeployment(hotswapMode, cdkStackArtifact); + const deployStackResult = await hotswapMockSdkProvider.tryHotswapDeployment(hotswapMode, cdkStackArtifact, { + SomeParameter: 'someValue', + }); // THEN - await expect(deployStackResult).rejects.toThrowError(CfnEvaluationException); - expect(mockUpdateMachineDefinition).not.toHaveBeenCalled(); - expect(mockUpdateLambdaCode).not.toHaveBeenCalled(); + expect(deployStackResult).not.toBeUndefined; + expect(mockUpdateLambdaConfiguration).toHaveBeenCalled(); } }); }); From e3d0c537448412e539a000d9d75b1d550d78d120 Mon Sep 17 00:00:00 2001 From: tmokmss Date: Sun, 16 Jul 2023 16:38:09 +0900 Subject: [PATCH 3/4] add unit test --- .../aws-cdk/test/api/deploy-stack.test.ts | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/packages/aws-cdk/test/api/deploy-stack.test.ts b/packages/aws-cdk/test/api/deploy-stack.test.ts index f4c23b00db1c3..e3bda40dfde2c 100644 --- a/packages/aws-cdk/test/api/deploy-stack.test.ts +++ b/packages/aws-cdk/test/api/deploy-stack.test.ts @@ -149,6 +149,36 @@ test('correctly passes CFN parameters when hotswapping', async () => { expect(tryHotswapDeployment).toHaveBeenCalledWith(expect.anything(), { A: 'A-value', B: 'B=value' }, expect.anything(), expect.anything(), HotswapMode.FALL_BACK); }); +test('correctly passes SSM parameters when hotswapping', async () => { + // GIVEN + givenStackExists({ + Parameters: [ + { ParameterKey: 'SomeParameter', ParameterValue: 'ParameterName', ResolvedValue: 'SomeValue' }, + ], + }); + + // WHEN + await deployStack({ + ...standardDeployStackArguments(), + stack: testStack({ + stackName: 'stack', + template: { + Parameters: { + SomeParameter: { + Type: 'AWS::SSM::Parameter::Value', + Default: 'ParameterName', + }, + }, + }, + }), + hotswap: HotswapMode.FALL_BACK, + usePreviousParameters: true, + }); + + // THEN + expect(tryHotswapDeployment).toHaveBeenCalledWith(expect.anything(), { SomeParameter: 'SomeValue' }, expect.anything(), expect.anything(), HotswapMode.FALL_BACK); +}); + test('call CreateStack when method=direct and the stack doesnt exist yet', async () => { // WHEN await deployStack({ From 3920ccff55b72344e757eee7b6db69ec9cbb73fe Mon Sep 17 00:00:00 2001 From: tmokmss Date: Sun, 16 Jul 2023 16:38:40 +0900 Subject: [PATCH 4/4] remove unit test --- .../api/hotswap/hotswap-deployments.test.ts | 82 ------------------- 1 file changed, 82 deletions(-) diff --git a/packages/aws-cdk/test/api/hotswap/hotswap-deployments.test.ts b/packages/aws-cdk/test/api/hotswap/hotswap-deployments.test.ts index 283f0f1779211..aab805a441f5b 100644 --- a/packages/aws-cdk/test/api/hotswap/hotswap-deployments.test.ts +++ b/packages/aws-cdk/test/api/hotswap/hotswap-deployments.test.ts @@ -6,19 +6,16 @@ import { HotswapMode } from '../../../lib/api/hotswap/common'; let hotswapMockSdkProvider: setup.HotswapMockSdkProvider; let mockUpdateLambdaCode: (params: Lambda.Types.UpdateFunctionCodeRequest) => Lambda.Types.FunctionConfiguration; -let mockUpdateLambdaConfiguration: (params: Lambda.Types.UpdateFunctionConfigurationRequest) => Lambda.Types.FunctionConfiguration; let mockUpdateMachineDefinition: (params: StepFunctions.Types.UpdateStateMachineInput) => StepFunctions.Types.UpdateStateMachineOutput; let mockGetEndpointSuffix: () => string; beforeEach(() => { hotswapMockSdkProvider = setup.setupHotswapTests(); mockUpdateLambdaCode = jest.fn().mockReturnValue({}); - mockUpdateLambdaConfiguration = jest.fn().mockReturnValue({}); mockUpdateMachineDefinition = jest.fn(); mockGetEndpointSuffix = jest.fn(() => 'amazonaws.com'); hotswapMockSdkProvider.stubLambda({ updateFunctionCode: mockUpdateLambdaCode, - updateFunctionConfiguration: mockUpdateLambdaConfiguration, }); hotswapMockSdkProvider.setUpdateStateMachineMock(mockUpdateMachineDefinition); hotswapMockSdkProvider.stubGetEndpointSuffix(mockGetEndpointSuffix); @@ -633,83 +630,4 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot }); } }); - - test('Changes containing an SSM Parameter cannot be hotswapped', async () => { - // GIVEN - setup.setCurrentCfnStackTemplate({ - Parameters: { - SomeParameter: { - Type: 'AWS::SSM::Parameter::Value', - Default: '/some/parameter', - }, - }, - Resources: { - Func: { - Type: 'AWS::Lambda::Function', - Properties: { - Code: { - S3Bucket: 'current-bucket', - S3Key: 'new-key', - }, - Environment: { - Param: { Ref: 'SomeParameter' }, - }, - FunctionName: 'my-function', - }, - Metadata: { - 'aws:asset:path': 'new-path', - }, - }, - }, - }); - const cdkStackArtifact = setup.cdkStackArtifactOf({ - template: { - Parameters: { - SomeParameter: { - Type: 'AWS::SSM::Parameter::Value', - Default: '/some/parameter', - }, - }, - Resources: { - Func: { - Type: 'AWS::Lambda::Function', - Properties: { - Code: { - S3Bucket: 'current-bucket', - S3Key: 'new-key', - }, - Environment: { - Param: { Ref: 'SomeParameter' }, - Key: 'Value', - }, - FunctionName: 'my-function', - }, - Metadata: { - 'aws:asset:path': 'new-path', - }, - }, - }, - }, - }); - - if (hotswapMode === HotswapMode.FALL_BACK) { - // WHEN - const deployStackResult = await hotswapMockSdkProvider.tryHotswapDeployment(hotswapMode, cdkStackArtifact, { - SomeParameter: 'someValue', - }); - - // THEN - expect(deployStackResult).not.toBeUndefined; - expect(mockUpdateLambdaConfiguration).toHaveBeenCalled(); - } else if (hotswapMode === HotswapMode.HOTSWAP_ONLY) { - // WHEN - const deployStackResult = await hotswapMockSdkProvider.tryHotswapDeployment(hotswapMode, cdkStackArtifact, { - SomeParameter: 'someValue', - }); - - // THEN - expect(deployStackResult).not.toBeUndefined; - expect(mockUpdateLambdaConfiguration).toHaveBeenCalled(); - } - }); });