From 3507141bd75022c72df1f8e24605f637deef5179 Mon Sep 17 00:00:00 2001 From: Amplifiyer <51211245+Amplifiyer@users.noreply.github.com> Date: Thu, 28 Sep 2023 19:26:54 +0200 Subject: [PATCH] fix(cdk): Resolve cross stack and default parameters for hotswaps (#27195) This PR solves two problems when doing hotswap deployments with nested stacks. 1. Adding capabilities to evaluate CFN parameters of `AWS::CloudFormation::Stack` type (i.e. a nested stack). In this PR, we are only resolving `Outputs` section of `AWS::CloudFormation::Stack` and it can be expanded to other fields in the future. See https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/quickref-cloudformation.html#w2ab1c17c23c19b5 for CFN documentation around using Outputs ref parameters 2. If a template has parameters with default values and they are not provided (a value) by the parent stack when invoking, then resolve these parameters using the default values in the template. Partially helps https://github.com/aws/aws-cdk/issues/23533 and https://github.com/aws/aws-cdk/issues/25418 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../api/evaluate-cloudformation-template.ts | 65 +++++- .../aws-cdk/lib/api/hotswap-deployments.ts | 20 +- .../lib/api/logs/find-cloudwatch-logs.ts | 3 +- .../api/hotswap/nested-stacks-hotswap.test.ts | 210 ++++++++++++++++++ ...-sibling-stack-output.nested.template.json | 30 +++ .../one-output-stack.nested.template.json | 5 + 6 files changed, 315 insertions(+), 18 deletions(-) create mode 100644 packages/aws-cdk/test/nested-stack-templates/one-lambda-stack-with-dependency-on-sibling-stack-output.nested.template.json create mode 100644 packages/aws-cdk/test/nested-stack-templates/one-output-stack.nested.template.json diff --git a/packages/aws-cdk/lib/api/evaluate-cloudformation-template.ts b/packages/aws-cdk/lib/api/evaluate-cloudformation-template.ts index f4a2576cee55e..03daa7b7b8daf 100644 --- a/packages/aws-cdk/lib/api/evaluate-cloudformation-template.ts +++ b/packages/aws-cdk/lib/api/evaluate-cloudformation-template.ts @@ -1,5 +1,6 @@ import * as AWS from 'aws-sdk'; import { ISDK } from './aws-auth'; +import { NestedStackNames } from './nested-stack-helpers'; export interface ListStackResources { listStackResources(): Promise; @@ -42,27 +43,33 @@ export interface ResourceDefinition { } export interface EvaluateCloudFormationTemplateProps { + readonly stackName: string; readonly template: Template; readonly parameters: { [parameterName: string]: string }; readonly account: string; readonly region: string; readonly partition: string; readonly urlSuffix: (region: string) => string; - readonly listStackResources: ListStackResources; + readonly sdk: ISDK; + readonly nestedStackNames?: { [nestedStackLogicalId: string]: NestedStackNames }; } export class EvaluateCloudFormationTemplate { - private readonly stackResources: ListStackResources; + private readonly stackName: string; private readonly template: Template; private readonly context: { [k: string]: any }; private readonly account: string; private readonly region: string; private readonly partition: string; private readonly urlSuffix: (region: string) => string; + private readonly sdk: ISDK; + private readonly nestedStackNames: { [nestedStackLogicalId: string]: NestedStackNames }; + private readonly stackResources: LazyListStackResources; + private cachedUrlSuffix: string | undefined; constructor(props: EvaluateCloudFormationTemplateProps) { - this.stackResources = props.listStackResources; + this.stackName = props.stackName; this.template = props.template; this.context = { 'AWS::AccountId': props.account, @@ -74,22 +81,34 @@ export class EvaluateCloudFormationTemplate { this.region = props.region; this.partition = props.partition; this.urlSuffix = props.urlSuffix; + this.sdk = props.sdk; + + // We need names of nested stack so we can evaluate cross stack references + this.nestedStackNames = props.nestedStackNames ?? {}; + + // The current resources of the Stack. + // We need them to figure out the physical name of a resource in case it wasn't specified by the user. + // We fetch it lazily, to save a service call, in case all hotswapped resources have their physical names set. + this.stackResources = new LazyListStackResources(this.sdk, this.stackName); } // clones current EvaluateCloudFormationTemplate object, but updates the stack name - public createNestedEvaluateCloudFormationTemplate( - listNestedStackResources: ListStackResources, + public async createNestedEvaluateCloudFormationTemplate( + stackName: string, nestedTemplate: Template, nestedStackParameters: { [parameterName: string]: any }, ) { + const evaluatedParams = await this.evaluateCfnExpression(nestedStackParameters); return new EvaluateCloudFormationTemplate({ + stackName, template: nestedTemplate, - parameters: nestedStackParameters, + parameters: evaluatedParams, account: this.account, region: this.region, partition: this.partition, urlSuffix: this.urlSuffix, - listStackResources: listNestedStackResources, + sdk: this.sdk, + nestedStackNames: this.nestedStackNames, }); } @@ -262,20 +281,52 @@ export class EvaluateCloudFormationTemplate { return this.cachedUrlSuffix; } + // Try finding the ref in the passed in parameters const parameterTarget = this.context[logicalId]; if (parameterTarget) { return parameterTarget; } + + // If not in the passed in parameters, see if there is a default value in the template parameter that was not passed in + const defaultParameterValue = this.template.Parameters?.[logicalId]?.Default; + if (defaultParameterValue) { + return defaultParameterValue; + } + // if it's not a Parameter, we need to search in the current Stack resources return this.findGetAttTarget(logicalId); } private async findGetAttTarget(logicalId: string, attribute?: string): Promise { + + // Handle case where the attribute is referencing a stack output (used in nested stacks to share parameters) + // See https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/quickref-cloudformation.html#w2ab1c17c23c19b5 + if (logicalId === 'Outputs' && attribute) { + return this.evaluateCfnExpression(this.template.Outputs[attribute]?.Value); + } + const stackResources = await this.stackResources.listStackResources(); const foundResource = stackResources.find(sr => sr.LogicalResourceId === logicalId); if (!foundResource) { return undefined; } + + if (foundResource.ResourceType == 'AWS::CloudFormation::Stack' && attribute?.startsWith('Outputs.')) { + // need to resolve attributes from another stack's Output section + const dependantStackName = this.nestedStackNames[logicalId]?.nestedStackPhysicalName; + if (!dependantStackName) { + //this is a newly created nested stack and cannot be hotswapped + return undefined; + } + const dependantStackTemplate = this.template.Resources[logicalId]; + const evaluateCfnTemplate = await this.createNestedEvaluateCloudFormationTemplate( + dependantStackName, + dependantStackTemplate?.Properties?.NestedTemplate, + dependantStackTemplate.newValue?.Properties?.Parameters); + + // Split Outputs. into 'Outputs' and '' and recursively call evaluate + return evaluateCfnTemplate.evaluateCfnExpression({ 'Fn::GetAtt': attribute.split(/\.(.*)/s) }); + } // now, we need to format the appropriate identifier depending on the resource type, // and the requested attribute name return this.formatResourceAttribute(foundResource, attribute); diff --git a/packages/aws-cdk/lib/api/hotswap-deployments.ts b/packages/aws-cdk/lib/api/hotswap-deployments.ts index eb03bdba5a8fb..5006a39963605 100644 --- a/packages/aws-cdk/lib/api/hotswap-deployments.ts +++ b/packages/aws-cdk/lib/api/hotswap-deployments.ts @@ -3,7 +3,7 @@ import * as cxapi from '@aws-cdk/cx-api'; import * as chalk from 'chalk'; import { ISDK, Mode, SdkProvider } from './aws-auth'; import { DeployStackResult } from './deploy-stack'; -import { EvaluateCloudFormationTemplate, LazyListStackResources } from './evaluate-cloudformation-template'; +import { EvaluateCloudFormationTemplate } from './evaluate-cloudformation-template'; import { isHotswappableAppSyncChange } from './hotswap/appsync-mapping-templates'; import { isHotswappableCodeBuildProjectChange } from './hotswap/code-build-projects'; import { ICON, ChangeHotswapResult, HotswapMode, HotswappableChange, NonHotswappableChange, HotswappableChangeCandidate, ClassifiedResourceChanges, reportNonHotswappableChange } from './hotswap/common'; @@ -24,6 +24,7 @@ const RESOURCE_DETECTORS: { [key:string]: HotswapDetector } = { 'AWS::Lambda::Function': isHotswappableLambdaFunctionChange, 'AWS::Lambda::Version': isHotswappableLambdaFunctionChange, 'AWS::Lambda::Alias': isHotswappableLambdaFunctionChange, + // AppSync 'AWS::AppSync::Resolver': isHotswappableAppSyncChange, 'AWS::AppSync::FunctionConfiguration': isHotswappableAppSyncChange, @@ -54,21 +55,21 @@ export async function tryHotswapDeployment( // create a new SDK using the CLI credentials, because the default one will not work for new-style synthesis - // it assumes the bootstrap deploy Role, which doesn't have permissions to update Lambda functions const sdk = (await sdkProvider.forEnvironment(resolvedEnv, Mode.ForWriting)).sdk; - // The current resources of the Stack. - // We need them to figure out the physical name of a resource in case it wasn't specified by the user. - // We fetch it lazily, to save a service call, in case all hotswapped resources have their physical names set. - const listStackResources = new LazyListStackResources(sdk, stackArtifact.stackName); + + const currentTemplate = await loadCurrentTemplateWithNestedStacks(stackArtifact, sdk); + const evaluateCfnTemplate = new EvaluateCloudFormationTemplate({ + stackName: stackArtifact.stackName, template: stackArtifact.template, parameters: assetParams, account: resolvedEnv.account, region: resolvedEnv.region, partition: (await sdk.currentAccount()).partition, urlSuffix: (region) => sdk.getEndpointSuffix(region), - listStackResources, + sdk, + nestedStackNames: currentTemplate.nestedStackNames, }); - const currentTemplate = await loadCurrentTemplateWithNestedStacks(stackArtifact, sdk); const stackChanges = cfn_diff.diffTemplate(currentTemplate.deployedTemplate, stackArtifact.template); const { hotswappableChanges, nonHotswappableChanges } = await classifyResourceChanges( stackChanges, evaluateCfnTemplate, sdk, currentTemplate.nestedStackNames, @@ -231,9 +232,8 @@ async function findNestedHotswappableChanges( }; } - const nestedStackParameters = await evaluateCfnTemplate.evaluateCfnExpression(change.newValue?.Properties?.Parameters); - const evaluateNestedCfnTemplate = evaluateCfnTemplate.createNestedEvaluateCloudFormationTemplate( - new LazyListStackResources(sdk, nestedStackName), change.newValue?.Properties?.NestedTemplate, nestedStackParameters, + const evaluateNestedCfnTemplate = await evaluateCfnTemplate.createNestedEvaluateCloudFormationTemplate( + nestedStackName, change.newValue?.Properties?.NestedTemplate, change.newValue?.Properties?.Parameters, ); const nestedDiff = cfn_diff.diffTemplate( diff --git a/packages/aws-cdk/lib/api/logs/find-cloudwatch-logs.ts b/packages/aws-cdk/lib/api/logs/find-cloudwatch-logs.ts index a54daabc9a0ae..bcee908dcc746 100644 --- a/packages/aws-cdk/lib/api/logs/find-cloudwatch-logs.ts +++ b/packages/aws-cdk/lib/api/logs/find-cloudwatch-logs.ts @@ -56,13 +56,14 @@ export async function findCloudWatchLogGroups( const listStackResources = new LazyListStackResources(sdk, stackArtifact.stackName); const evaluateCfnTemplate = new EvaluateCloudFormationTemplate({ + stackName: stackArtifact.stackName, template: stackArtifact.template, parameters: {}, account: resolvedEnv.account, region: resolvedEnv.region, partition: (await sdk.currentAccount()).partition, urlSuffix: (region) => sdk.getEndpointSuffix(region), - listStackResources, + sdk, }); const stackResources = await listStackResources.listStackResources(); diff --git a/packages/aws-cdk/test/api/hotswap/nested-stacks-hotswap.test.ts b/packages/aws-cdk/test/api/hotswap/nested-stacks-hotswap.test.ts index 747a34fd12c8a..77d159074c8da 100644 --- a/packages/aws-cdk/test/api/hotswap/nested-stacks-hotswap.test.ts +++ b/packages/aws-cdk/test/api/hotswap/nested-stacks-hotswap.test.ts @@ -826,6 +826,216 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot }); }); + test('can hotswap a lambda function in a 1-level nested stack with dependency on a output of sibling stack', async () => { + // GIVEN: RootStack has two child stacks `NestedLambdaStack` and `NestedSiblingStack`. `NestedLambdaStack` + // takes two parameters s3Key and s3Bucket and use them for a Lambda function. + // RootStack resolves s3Bucket from a root template parameter and s3Key through output of `NestedSiblingStack` + hotswapMockSdkProvider = setup.setupHotswapNestedStackTests('RootStack'); + mockUpdateLambdaCode = jest.fn().mockReturnValue({}); + hotswapMockSdkProvider.stubLambda({ + updateFunctionCode: mockUpdateLambdaCode, + }); + + const rootStack = testStack({ + stackName: 'RootStack', + template: { + Resources: { + NestedLambdaStack: { + Type: 'AWS::CloudFormation::Stack', + Properties: { + TemplateURL: 'https://www.magic-url.com', + Parameters: { + referenceToS3BucketParam: { + Ref: 'S3BucketParam', + }, + referenceToS3StackKeyOutput: { + 'Fn::GetAtt': [ + 'NestedSiblingStack', + 'Outputs.NestedOutput', + ], + }, + }, + }, + Metadata: { + 'aws:asset:path': 'one-lambda-stack-with-dependency-on-sibling-stack-output.nested.template.json', + }, + }, + NestedSiblingStack: { + Type: 'AWS::CloudFormation::Stack', + Properties: { + TemplateURL: 'https://www.magic-url.com', + }, + Metadata: { + 'aws:asset:path': 'one-output-stack.nested.template.json', + }, + }, + Parameters: { + S3BucketParam: { + Type: 'String', + Description: 'S3 bucket for asset', + }, + }, + }, + }, + }); + + const nestedLambdaStack = testStack({ + stackName: 'NestedLambdaStack', + template: { + Resources: { + Func: { + Type: 'AWS::Lambda::Function', + Properties: { + Code: { + S3Bucket: 'current-bucket', + S3Key: 'current-key', + }, + FunctionName: 'my-function', + }, + }, + }, + Metadata: { + 'aws:asset:path': 'old-path', + }, + }, + }); + + const nestedSiblingStack = testStack({ + stackName: 'NestedSiblingStack', + template: { + Outputs: { + NestedOutput: { Value: 's3-key-value-from-output' }, + }, + Metadata: { + 'aws:asset:path': 'old-path', + }, + }, + }); + + setup.addTemplateToCloudFormationLookupMock(rootStack); + setup.addTemplateToCloudFormationLookupMock(nestedLambdaStack); + setup.addTemplateToCloudFormationLookupMock(nestedSiblingStack); + + setup.pushNestedStackResourceSummaries('RootStack', + setup.stackSummaryOf('NestedLambdaStack', 'AWS::CloudFormation::Stack', + 'arn:aws:cloudformation:bermuda-triangle-1337:123456789012:stack/NestedLambdaStack/abcd', + ), + setup.stackSummaryOf('NestedSiblingStack', 'AWS::CloudFormation::Stack', + 'arn:aws:cloudformation:bermuda-triangle-1337:123456789012:stack/NestedSiblingStack/abcd', + ), + ); + setup.pushNestedStackResourceSummaries('NestedLambdaStack', + setup.stackSummaryOf('Func', 'AWS::Lambda::Function', 'nested-lambda-function'), + ); + setup.pushNestedStackResourceSummaries('NestedSiblingStack'); + + const cdkStackArtifact = testStack({ stackName: 'RootStack', template: rootStack.template }); + + // WHEN + const deployStackResult = await hotswapMockSdkProvider.tryHotswapDeployment(hotswapMode, cdkStackArtifact, { + S3BucketParam: 'new-bucket', + }); + + // THEN + expect(deployStackResult).not.toBeUndefined(); + expect(mockUpdateLambdaCode).toHaveBeenCalledWith({ + FunctionName: 'my-function', + S3Bucket: 'new-bucket', + S3Key: 's3-key-value-from-output', + }); + }); + + test('can hotswap a lambda function in a 1-level nested stack and read default parameters value if not provided', async () => { + // GIVEN: RootStack has one child stack `NestedStack`. `NestedStack` takes two + // parameters s3Key and s3Bucket and use them for a Lambda function. + // RootStack resolves both parameters from root template parameters. Current/old change + // has hardcoded resolved values and the new change doesn't provide parameters through + // root stack forcing the evaluation of default parameter values. + hotswapMockSdkProvider = setup.setupHotswapNestedStackTests('LambdaRoot'); + mockUpdateLambdaCode = jest.fn().mockReturnValue({}); + hotswapMockSdkProvider.stubLambda({ + updateFunctionCode: mockUpdateLambdaCode, + }); + + const rootStack = testStack({ + stackName: 'LambdaRoot', + template: { + Resources: { + NestedStack: { + Type: 'AWS::CloudFormation::Stack', + Properties: { + TemplateURL: 'https://www.magic-url.com', + Parameters: { + referencetoS3BucketParam: { + Ref: 'S3BucketParam', + }, + referencetoS3KeyParam: { + Ref: 'S3KeyParam', + }, + }, + }, + Metadata: { + 'aws:asset:path': 'one-lambda-stack-with-asset-parameters.nested.template.json', + }, + }, + }, + Parameters: { + S3BucketParam: { + Type: 'String', + Description: 'S3 bucket for asset', + Default: 'default-s3-bucket', + }, + S3KeyParam: { + Type: 'String', + Description: 'S3 bucket for asset', + Default: 'default-s3-key', + }, + }, + }, + }); + + setup.addTemplateToCloudFormationLookupMock(rootStack); + setup.addTemplateToCloudFormationLookupMock(testStack({ + stackName: 'NestedStack', + template: { + Resources: { + Func: { + Type: 'AWS::Lambda::Function', + Properties: { + Code: { + S3Bucket: 'current-bucket', + S3Key: 'current-key', + }, + FunctionName: 'my-function', + }, + Metadata: { + 'aws:asset:path': 'old-path', + }, + }, + }, + }, + })); + + setup.pushNestedStackResourceSummaries('LambdaRoot', + setup.stackSummaryOf('NestedStack', 'AWS::CloudFormation::Stack', + 'arn:aws:cloudformation:bermuda-triangle-1337:123456789012:stack/NestedStack/abcd', + ), + ); + + const cdkStackArtifact = testStack({ stackName: 'LambdaRoot', template: rootStack.template }); + + // WHEN + const deployStackResult = await hotswapMockSdkProvider.tryHotswapDeployment(hotswapMode, cdkStackArtifact); + + // THEN + expect(deployStackResult).not.toBeUndefined(); + expect(mockUpdateLambdaCode).toHaveBeenCalledWith({ + FunctionName: 'my-function', + S3Bucket: 'default-s3-bucket', + S3Key: 'default-s3-key', + }); + }); + test('can hotswap a lambda function in a 2-level nested stack with asset parameters', async () => { // GIVEN hotswapMockSdkProvider = setup.setupHotswapNestedStackTests('LambdaRoot'); diff --git a/packages/aws-cdk/test/nested-stack-templates/one-lambda-stack-with-dependency-on-sibling-stack-output.nested.template.json b/packages/aws-cdk/test/nested-stack-templates/one-lambda-stack-with-dependency-on-sibling-stack-output.nested.template.json new file mode 100644 index 0000000000000..68f18403ab37d --- /dev/null +++ b/packages/aws-cdk/test/nested-stack-templates/one-lambda-stack-with-dependency-on-sibling-stack-output.nested.template.json @@ -0,0 +1,30 @@ +{ + "Type": "AWS::CloudFormation::Stack", + "Resources": { + "Func": { + "Type": "AWS::Lambda::Function", + "Properties": { + "Code": { + "S3Bucket": { + "Ref": "referenceToS3BucketParam" + }, + "S3Key": { + "Ref": "referenceToS3StackKeyOutput" + } + }, + "FunctionName": "my-function" + }, + "Metadata": { + "aws:asset:path": "old-path" + } + } + }, + "Parameters": { + "referenceToS3BucketParam": { + "Type": "String" + }, + "referenceToS3StackKeyOutput": { + "Type": "String" + } + } +} diff --git a/packages/aws-cdk/test/nested-stack-templates/one-output-stack.nested.template.json b/packages/aws-cdk/test/nested-stack-templates/one-output-stack.nested.template.json new file mode 100644 index 0000000000000..345987597d6cb --- /dev/null +++ b/packages/aws-cdk/test/nested-stack-templates/one-output-stack.nested.template.json @@ -0,0 +1,5 @@ +{ + "Outputs": { + "NestedOutput": { "Value": "s3-key-value-from-output" } + } +}