Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(cdk): Resolve cross stack and default parameters for hotswaps #27195

Merged
merged 10 commits into from
Sep 28, 2023
65 changes: 58 additions & 7 deletions packages/aws-cdk/lib/api/evaluate-cloudformation-template.ts
Original file line number Diff line number Diff line change
@@ -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<AWS.CloudFormation.StackResourceSummary[]>;
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need this here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In current CDK, evaluate-cloudformation-template can only evaluate CFN (Intrinsic) Fn references for resources in the same stack (Let's call it A) and hence it is passed a list of A's resources to assist in evaluation.

However the problem comes when a resource in A stack references resources in a different stack (Let's call it B). In that case evaluate-cloudformation-template of A has no knowledge of it.

This PR refactors evaluate-cloudformation-template in a way that it can recursively instantiate another evaluate-cloudformation-template for the referenced stack. And to do that, A's evaluate-cloudformation-template needs to pass in the list of B's resources while creating another instance and hence the sdk needs to be passed in for the A's evaluate-cloudformation-template so that it can call B's listResources

Now, instead of having the caller to get the list of stack's resources and pass it to evaluate-cloudformation-template, it's better for evaluate-cloudformation-template to manage it on it's own, since these resources are specifically being used by the evaluate-cloudformation-template and instead just pass the sdk rather than both sdk and the stackResources

nestedStackNames: this.nestedStackNames,
});
}

Expand Down Expand Up @@ -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<string | undefined> {

// 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.<refName> into 'Outputs' and '<refName>' 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);
Expand Down
20 changes: 10 additions & 10 deletions packages/aws-cdk/lib/api/hotswap-deployments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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,
Expand Down Expand Up @@ -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);
Comment on lines -57 to -60
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this has been moved elsewhere, could you explain why? I don't see why this is necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Answered here #27195 (comment)


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,
Expand Down Expand Up @@ -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(
Expand Down
3 changes: 2 additions & 1 deletion packages/aws-cdk/lib/api/logs/find-cloudwatch-logs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Loading
Loading