From 6d315b8539e714143c35762a45e8f8f4ddcc9217 Mon Sep 17 00:00:00 2001 From: Masashi Tomooka Date: Thu, 17 Aug 2023 17:29:10 +0900 Subject: [PATCH 1/4] fix(cli): cannot hotswap ECS task definitions containing certain intrinsics (#26404) ## Reason for this change Currently an ECS task definition cannot be hotswapped in many cases, for example when it contains references to certain AWS resources as environment variables. This PR removes the limitation and make hotswaps possible in much more various situations. Specifically, if there is any token that is not part of the following, we cannot hotswap a task definition: 1. Ref functions, which will be resolved as physical resource ids 2. GetAtt functions for some resources and attributes (manually supported one by one, [code](https://github.com/aws/aws-cdk/blob/5ccc56975c323ea19fd0917def51184e13f440d9/packages/aws-cdk/lib/api/evaluate-cloudformation-template.ts#L352)) 3. several simple CFn functions (join, split, select, sub) 4. parameters like AWS::AccountId, Region, Partition, or UrlSuffix Although it is not supported much, using `GetAtt` function in a task definition is very common (imagine you reference other resource's property as an environment variable). This PR allows to hotswap a task definition even if it contains these tokens. ## Solution To hotswap a task definition, we need to construct a task definition to call `registerTaskDefinition` API. For this, we have to [evaluate](https://github.com/aws/aws-cdk/blob/5ccc56975c323ea19fd0917def51184e13f440d9/packages/aws-cdk/lib/api/evaluate-cloudformation-template.ts#L134) CloudFormation template locally to resolve all the intrinsics in the template. However, some intrinsics such as `Fn::GetAtt` is not fully supported by CDK CLI because we have to manually support them for each AWS resource type. The reason why some task definitions are unhotswappable is that there are such intrinsics in the template and the CDK fails to evaluate it locally. So the basic idea to overcome this limitation in this PR is that we don't try to evaluate it locally, but we fetch the latest task definition already deployed and get the required values from it. Here's how we can implement the idea. ### How we determine if changes to a task definition can be hotswapped In the hotswap process, we have to decide whether the change can be hotswapped or not. Now we can hotswap the task definition if 1. there are only changes in `ContainerDefinitions` field, and all the fields in the task definition can be evaluated locally. (original behavior) OR, 2. there are only changes in `ContainerDefinitions` field, and all the **updated** field can be evaluated locally (added in this PR). The first condition can actually be included in the second condition, but for now I keep it as-is to avoid the possibility of breaking the existing behavior. If the second condition is true, we fetch the latest task definition from AWS account, override the updated fields, and register a new task definition to update a service. By this way, we don't have to evaluate tokens in unchanged fields locally, allowing to use hotswap in much more situations. ### How we compare the old and new task definition Here is an example task definition: ```json { "ContainerDefinitions": [ { "Environment": [ { "Name": "VPC_ID", "Value": { "Fn::GetAtt": [ "Vpc8378EB38", "CidrBlock" ] } } ], "Essential": true, "Image": "nginx:stable", "Name": "EcsApp" } ], "Cpu": "256", "Family": "myteststackTask289798EC", "Memory": "512", "NetworkMode": "awsvpc", "RequiresCompatibilities": [ "FARGATE" ], "TaskRoleArn": { "Fn::GetAtt": [ "TaskTaskRoleE98524A1", "Arn" ] } } ``` We compare the old and new task definition in the following steps: 1. Check if there are only changes in `ContainerDefinitions` field. If not, we cannot hotswap. 2. Try `evaluateCfnExpression` on the containerDefinitons. If it can be evaluated, proceed to hotswap. If not, proceed to step 3. 3. Check if the length of `ContainerDefinitions` is the same. If not, we cannot hotswap. 4. For each container definition, deep-compare each key (e.g. `Environment`, `Image`, `Name`, etc) 5. For each key, if there is any diff in the corresponding value, try `evaluateCfnExpression` on the value. If the evaluation fails, we cannot hotswap. 6. After checking all the keys and there is no field that cannot be hotswapped, proceed to hotswap. Imagine if there is a change only in `Image` field (container image tag) but `Environment` field contains unsupported intrinsics (e.g. `"Fn::GetAtt": ["Vpc8378EB38", "CidrBlock"]`). In the previous CDK CLI we cannot hotswap it due to an evaluation error. We can now hotswap it because we don't have to evaluate the `Environment` field when it has no diffs. Closes #25563 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- packages/aws-cdk/lib/api/hotswap/common.ts | 212 ++++++ .../aws-cdk/lib/api/hotswap/ecs-services.ts | 107 ++- .../ecs-services-hotswap-deployments.test.ts | 608 +++++++++++++----- 3 files changed, 734 insertions(+), 193 deletions(-) diff --git a/packages/aws-cdk/lib/api/hotswap/common.ts b/packages/aws-cdk/lib/api/hotswap/common.ts index d72d7b270fc85..5bbb69535faf1 100644 --- a/packages/aws-cdk/lib/api/hotswap/common.ts +++ b/packages/aws-cdk/lib/api/hotswap/common.ts @@ -1,5 +1,6 @@ import * as cfn_diff from '@aws-cdk/cloudformation-diff'; import { ISDK } from '../aws-auth'; +import { CfnEvaluationException, EvaluateCloudFormationTemplate } from '../evaluate-cloudformation-template'; export const ICON = '✨'; @@ -135,6 +136,13 @@ export function lowerCaseFirstCharacter(str: string): string { return str.length > 0 ? `${str[0].toLowerCase()}${str.slice(1)}` : str; } +/** + * This function upper cases the first character of the string provided. + */ +export function upperCaseFirstCharacter(str: string): string { + return str.length > 0 ? `${str[0].toUpperCase()}${str.slice(1)}` : str; +} + export type PropDiffs = Record>; export class ClassifiedChanges { @@ -213,3 +221,207 @@ export function reportNonHotswappableResource( reason, }]; } + +type ChangedProps = { + /** + * Array to specify the property from an object. + * e.g. Given this object `{ 'a': { 'b': 1 } }`, the key array for the element `1` will be `['a', 'b']` + */ + key: string[]; + + /** + * Whether the property is added (also modified) or removed. + */ + type: 'removed' | 'added'; + + /** + * evaluated value of the property. + * undefined if type == 'removed' + */ + value?: any +}; + +function detectChangedProps(next: any, prev: any): ChangedProps[] { + const changedProps: ChangedProps[] = []; + changedProps.push(...detectAdditions(next, prev)); + changedProps.push(...detectRemovals(next, prev)); + return changedProps; +} + +function detectAdditions(next: any, prev: any, keys: string[] = []): ChangedProps[] { + // Compare each value of two objects, detect additions (added or modified properties) + // If we encounter CFn intrinsic (key.startsWith('Fn::') || key == 'Ref'), stop recursion + + if (typeof next !== 'object') { + if (next !== prev) { + // there is an addition or change to the property + return [{ key: new Array(...keys), type: 'added' }]; + } else { + return []; + } + } + + if (typeof prev !== 'object') { + // there is an addition or change to the property + return [{ key: new Array(...keys), type: 'added' }]; + } + + // If the next is a CFn intrinsic, don't recurse further. + const childKeys = Object.keys(next); + if (childKeys.length === 1 && (childKeys[0].startsWith('Fn::') || childKeys[0] === 'Ref')) { + if (!deepCompareObject(prev, next)) { + // there is an addition or change to the property + return [{ key: new Array(...keys), type: 'added' }]; + } else { + return []; + } + } + + const changedProps: ChangedProps[] = []; + // compare children + for (const key of childKeys) { + keys.push(key); + changedProps.push(...detectAdditions((next as any)[key], (prev as any)[key], keys)); + keys.pop(); + } + return changedProps; +} + +function detectRemovals(next: any, prev: any, keys: string[] = []): ChangedProps[] { + // Compare each value of two objects, detect removed properties + // To do this, find any keys that exist only in prev object. + // If we encounter CFn intrinsic (key.startsWith('Fn::') || key == 'Ref'), stop recursion + if (next === undefined) { + return [{ key: new Array(...keys), type: 'removed' }]; + } + + if (typeof prev !== 'object' || typeof next !== 'object') { + // either prev or next is not an object nor undefined, then the property is not removed + return []; + } + + // If the prev is a CFn intrinsic, don't recurse further. + const childKeys = Object.keys(prev); + if (childKeys.length === 1 && (childKeys[0].startsWith('Fn::') || childKeys[0] === 'Ref')) { + // next is not undefined here, so it is at least not removed + return []; + } + + const changedProps: ChangedProps[] = []; + // compare children + for (const key of childKeys) { + keys.push(key); + changedProps.push(...detectRemovals((next as any)[key], (prev as any)[key], keys)); + keys.pop(); + } + return changedProps; +} + +/** + * return true when two objects are identical + */ +function deepCompareObject(lhs: any, rhs: any): boolean { + if (typeof lhs !== 'object') { + return lhs === rhs; + } + if (typeof rhs !== 'object') { + return false; + } + if (Object.keys(lhs).length != Object.keys(rhs).length) { + return false; + } + for (const key of Object.keys(lhs)) { + if (!deepCompareObject((lhs as any)[key], (rhs as any)[key])) { + return false; + } + } + return true; +} + +interface EvaluatedPropertyUpdates { + readonly updates: ChangedProps[]; + readonly unevaluatableUpdates: ChangedProps[]; +} + +/** + * Diff each property of the changes, and check if each diff can be actually hotswapped (i.e. evaluated by EvaluateCloudFormationTemplate.) + * If any diff cannot be evaluated, they are reported by unevaluatableUpdates. + * This method works on more granular level than HotswappableChangeCandidate.propertyUpdates. + * + * If propertiesToInclude is specified, we only compare properties that are under keys in the argument. + */ +export async function evaluatableProperties( + evaluate: EvaluateCloudFormationTemplate, + change: HotswappableChangeCandidate, + propertiesToInclude?: string[], +): Promise { + const prev = change.oldValue.Properties!; + const next = change.newValue.Properties!; + const changedProps = detectChangedProps(next, prev).filter( + prop => propertiesToInclude?.includes(prop.key[0]) ?? true, + ); + const evaluatedUpdates = await Promise.all( + changedProps + .filter((prop) => prop.type === 'added') + .map(async (prop) => { + const val = getPropertyFromKey(prop.key, next); + try { + const evaluated = await evaluate.evaluateCfnExpression(val); + return { + ...prop, + value: evaluated, + }; + } catch (e) { + if (e instanceof CfnEvaluationException) { + return prop; + } + throw e; + } + })); + const unevaluatableUpdates = evaluatedUpdates.filter(update => update.value === undefined); + evaluatedUpdates.push(...changedProps.filter(prop => prop.type == 'removed')); + + return { + updates: evaluatedUpdates, + unevaluatableUpdates, + }; +} + +function getPropertyFromKey(key: string[], obj: object) { + return key.reduce((prev, cur) => (prev as any)?.[cur], obj); +} + +function overwriteProperty(key: string[], newValue: any, target: object) { + for (const next of key.slice(0, -1)) { + if (next in target) { + target = (target as any)[next]; + } else if (Array.isArray(target)) { + // When an element is added to an array, we need explicitly allocate the new element. + target = {}; + (target as any)[next] = {}; + } else { + // This is an unexpected condition. Perhaps the deployed task definition is modified outside of CFn. + return false; + } + } + if (newValue === undefined) { + delete (target as any)[key[key.length - 1]]; + } else { + (target as any)[key[key.length - 1]] = newValue; + } + return true; +} + +/** + * Take the old template and property updates, and synthesize a new template. + */ +export function applyPropertyUpdates(patches: ChangedProps[], target: any) { + target = JSON.parse(JSON.stringify(target)); + for (const patch of patches) { + const res = overwriteProperty(patch.key, patch.value, target); + if (!res) { + throw new Error(`failed to applying patch to ${patch.key.join('.')}. Please try deploying without hotswap first.`); + } + } + return target; +} diff --git a/packages/aws-cdk/lib/api/hotswap/ecs-services.ts b/packages/aws-cdk/lib/api/hotswap/ecs-services.ts index ad62950fc861b..7f2dea19ea493 100644 --- a/packages/aws-cdk/lib/api/hotswap/ecs-services.ts +++ b/packages/aws-cdk/lib/api/hotswap/ecs-services.ts @@ -1,5 +1,5 @@ import * as AWS from 'aws-sdk'; -import { ChangeHotswapResult, classifyChanges, HotswappableChangeCandidate, lowerCaseFirstCharacter, reportNonHotswappableChange, transformObjectKeys } from './common'; +import { ChangeHotswapResult, classifyChanges, HotswappableChangeCandidate, lowerCaseFirstCharacter, reportNonHotswappableChange, transformObjectKeys, upperCaseFirstCharacter, applyPropertyUpdates, evaluatableProperties } from './common'; import { ISDK } from '../aws-auth'; import { EvaluateCloudFormationTemplate } from '../evaluate-cloudformation-template'; @@ -16,7 +16,8 @@ export async function isHotswappableEcsServiceChange( // We only allow a change in the ContainerDefinitions of the TaskDefinition for now - // it contains the image and environment variables, so seems like a safe bet for now. // We might revisit this decision in the future though! - const classifiedChanges = classifyChanges(change, ['ContainerDefinitions']); + const propertiesToHotswap = ['ContainerDefinitions']; + const classifiedChanges = classifyChanges(change, propertiesToHotswap); classifiedChanges.reportNonHotswappablePropertyChanges(ret); // find all ECS Services that reference the TaskDefinition that changed @@ -33,7 +34,8 @@ export async function isHotswappableEcsServiceChange( // if there are no resources referencing the TaskDefinition, // hotswap is not possible in FALL_BACK mode reportNonHotswappableChange(ret, change, undefined, 'No ECS services reference the changed task definition', false); - } if (resourcesReferencingTaskDef.length > ecsServicesReferencingTaskDef.length) { + } + if (resourcesReferencingTaskDef.length > ecsServicesReferencingTaskDef.length) { // if something besides an ECS Service is referencing the TaskDefinition, // hotswap is not possible in FALL_BACK mode const nonEcsServiceTaskDefRefs = resourcesReferencingTaskDef.filter(r => r.Type !== 'AWS::ECS::Service'); @@ -44,14 +46,32 @@ export async function isHotswappableEcsServiceChange( const namesOfHotswappableChanges = Object.keys(classifiedChanges.hotswappableProps); if (namesOfHotswappableChanges.length > 0) { - const taskDefinitionResource = await prepareTaskDefinitionChange(evaluateCfnTemplate, logicalId, change); + const familyName = await getFamilyName(evaluateCfnTemplate, logicalId, change); + if (familyName === undefined) { + reportNonHotswappableChange(ret, change, undefined, 'Failed to determine family name of the task definition', false); + return ret; + } + const oldTaskDefinitionArn = await evaluateCfnTemplate.findPhysicalNameFor(logicalId); + if (oldTaskDefinitionArn === undefined) { + reportNonHotswappableChange(ret, change, undefined, 'Failed to determine ARN of the task definition', false); + return ret; + } + + const changes = await evaluatableProperties(evaluateCfnTemplate, change, propertiesToHotswap); + if (changes.unevaluatableUpdates.length > 0) { + reportNonHotswappableChange(ret, change, undefined, `Found changes that cannot be evaluated locally in the task definition - ${ + changes.unevaluatableUpdates.map(p => p.key.join('.')).join(', ') + }`, false); + return ret; + } + ret.push({ hotswappable: true, resourceType: change.newValue.Type, propsChanged: namesOfHotswappableChanges, service: 'ecs-service', resourceNames: [ - `ECS Task Definition '${await taskDefinitionResource.Family}'`, + `ECS Task Definition '${familyName}'`, ...ecsServicesReferencingTaskDef.map(ecsService => `ECS Service '${ecsService.serviceArn.split('/')[2]}'`), ], apply: async (sdk: ISDK) => { @@ -59,11 +79,43 @@ export async function isHotswappableEcsServiceChange( // we need to lowercase the evaluated TaskDef from CloudFormation, // as the AWS SDK uses lowercase property names for these - // The SDK requires more properties here than its worth doing explicit typing for - // instead, just use all the old values in the diff to fill them in implicitly - const lowercasedTaskDef = transformObjectKeys(taskDefinitionResource, lowerCaseFirstCharacter, { - // All the properties that take arbitrary string as keys i.e. { "string" : "string" } - // https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_RegisterTaskDefinition.html#API_RegisterTaskDefinition_RequestSyntax + // get the task definition of the family and revision corresponding to the old CFn template + const target = await sdk + .ecs() + .describeTaskDefinition({ + taskDefinition: oldTaskDefinitionArn, + include: ['TAGS'], + }) + .promise(); + if (target.taskDefinition === undefined) { + throw new Error(`Could not find a task definition: ${oldTaskDefinitionArn}. Try deploying without hotswap first.`); + } + + // The describeTaskDefinition response contains several keys that must not exist in a registerTaskDefinition request. + // We remove these keys here, comparing these two structs: + // https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_RegisterTaskDefinition.html#API_RegisterTaskDefinition_RequestSyntax + // https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_DescribeTaskDefinition.html#API_DescribeTaskDefinition_ResponseSyntax + [ + 'compatibilities', + 'taskDefinitionArn', + 'revision', + 'status', + 'requiresAttributes', + 'compatibilities', + 'registeredAt', + 'registeredBy', + ].forEach(key=> delete (target.taskDefinition as any)[key]); + + // the tags field is in a different location in describeTaskDefinition response, + // moving it as intended for registerTaskDefinition request. + if (target.tags !== undefined && target.tags.length > 0) { + (target.taskDefinition as any).tags = target.tags; + delete target.tags; + } + + // Don't transform the properties that take arbitrary string as keys i.e. { "string" : "string" } + // https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_RegisterTaskDefinition.html#API_RegisterTaskDefinition_RequestSyntax + const excludeFromTransform = { ContainerDefinitions: { DockerLabels: true, FirelensConfiguration: { @@ -79,7 +131,14 @@ export async function isHotswappableEcsServiceChange( Labels: true, }, }, - }); + } as const; + // We first uppercase the task definition to properly merge it with the one from CloudFormation template. + const upperCasedTaskDef = transformObjectKeys(target.taskDefinition, upperCaseFirstCharacter, excludeFromTransform); + // merge evaluatable diff from CloudFormation template. + const updatedTaskDef = applyPropertyUpdates(changes.updates, upperCasedTaskDef); + // lowercase the merged task definition to use it in AWS SDK. + const lowercasedTaskDef = transformObjectKeys(updatedTaskDef, lowerCaseFirstCharacter, excludeFromTransform); + const registerTaskDefResponse = await sdk.ecs().registerTaskDefinition(lowercasedTaskDef).promise(); const taskDefRevArn = registerTaskDefResponse.taskDefinition?.taskDefinitionArn; @@ -171,14 +230,15 @@ interface EcsService { readonly serviceArn: string; } -async function prepareTaskDefinitionChange( - evaluateCfnTemplate: EvaluateCloudFormationTemplate, logicalId: string, change: HotswappableChangeCandidate, -) { +async function getFamilyName( + evaluateCfnTemplate: EvaluateCloudFormationTemplate, + logicalId: string, + change: HotswappableChangeCandidate) { const taskDefinitionResource: { [name: string]: any } = { ...change.oldValue.Properties, ContainerDefinitions: change.newValue.Properties?.ContainerDefinitions, }; - // first, let's get the name of the family + // first, let's get the name of the family const familyNameOrArn = await evaluateCfnTemplate.establishResourcePhysicalName(logicalId, taskDefinitionResource?.Family); if (!familyNameOrArn) { // if the Family property has not been provided, and we can't find it in the current Stack, @@ -189,17 +249,12 @@ async function prepareTaskDefinitionChange( // remove it if needed const familyNameOrArnParts = familyNameOrArn.split(':'); const family = familyNameOrArnParts.length > 1 - // familyNameOrArn is actually an ARN, of the format 'arn:aws:ecs:region:account:task-definition/:' - // so, take the 6th element, at index 5, and split it on '/' + // familyNameOrArn is actually an ARN, of the format 'arn:aws:ecs:region:account:task-definition/:' + // so, take the 6th element, at index 5, and split it on '/' ? familyNameOrArnParts[5].split('/')[1] - // otherwise, familyNameOrArn is just the simple name evaluated from the CloudFormation template + // otherwise, familyNameOrArn is just the simple name evaluated from the CloudFormation template : familyNameOrArn; - // then, let's evaluate the body of the remainder of the TaskDef (without the Family property) - return { - ...await evaluateCfnTemplate.evaluateCfnExpression({ - ...(taskDefinitionResource ?? {}), - Family: undefined, - }), - Family: family, - }; + // then, let's evaluate the body of the remainder of the TaskDef (without the Family property) + + return family; } diff --git a/packages/aws-cdk/test/api/hotswap/ecs-services-hotswap-deployments.test.ts b/packages/aws-cdk/test/api/hotswap/ecs-services-hotswap-deployments.test.ts index e0267635a59d8..b9be0725315c8 100644 --- a/packages/aws-cdk/test/api/hotswap/ecs-services-hotswap-deployments.test.ts +++ b/packages/aws-cdk/test/api/hotswap/ecs-services-hotswap-deployments.test.ts @@ -1,19 +1,22 @@ /* eslint-disable import/order */ import * as AWS from 'aws-sdk'; import * as setup from './hotswap-test-setup'; -import { HotswapMode } from '../../../lib/api/hotswap/common'; +import { HotswapMode, lowerCaseFirstCharacter, transformObjectKeys } from '../../../lib/api/hotswap/common'; let hotswapMockSdkProvider: setup.HotswapMockSdkProvider; let mockRegisterTaskDef: jest.Mock; +let mockDescribeTaskDef: jest.Mock; let mockUpdateService: (params: AWS.ECS.UpdateServiceRequest) => AWS.ECS.UpdateServiceResponse; beforeEach(() => { hotswapMockSdkProvider = setup.setupHotswapTests(); mockRegisterTaskDef = jest.fn(); + mockDescribeTaskDef = jest.fn(); mockUpdateService = jest.fn(); hotswapMockSdkProvider.stubEcs({ registerTaskDefinition: mockRegisterTaskDef, + describeTaskDefinition: mockDescribeTaskDef, updateService: mockUpdateService, }, { // these are needed for the waiter API that the ECS service hotswap uses @@ -30,36 +33,71 @@ beforeEach(() => { }); }); -describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hotswapMode) => { - test('should call registerTaskDefinition and updateService for a difference only in the TaskDefinition with a Family property', async () => { - // GIVEN - setup.setCurrentCfnStackTemplate({ - Resources: { - TaskDef: { - Type: 'AWS::ECS::TaskDefinition', - Properties: { - Family: 'my-task-def', - ContainerDefinitions: [ - { Image: 'image1' }, - ], - }, - }, +function setupCurrentTaskDefinition(props: {taskDefinitionProperties: any, includeService: boolean, otherResources?: any}) { + setup.setCurrentCfnStackTemplate({ + Resources: { + TaskDef: { + Type: 'AWS::ECS::TaskDefinition', + Properties: props.taskDefinitionProperties, + }, + ...(props.includeService ? { Service: { Type: 'AWS::ECS::Service', Properties: { TaskDefinition: { Ref: 'TaskDef' }, }, }, - }, - }); + } : {}), + ...(props.otherResources ?? {}), + }, + }); + if (props.includeService) { setup.pushStackResourceSummaries( setup.stackSummaryOf('Service', 'AWS::ECS::Service', 'arn:aws:ecs:region:account:service/my-cluster/my-service'), ); - mockRegisterTaskDef.mockReturnValue({ - taskDefinition: { - taskDefinitionArn: 'arn:aws:ecs:region:account:task-definition/my-task-def:3', + } + setup.pushStackResourceSummaries( + setup.stackSummaryOf('TaskDef', 'AWS::ECS::TaskDefinition', + 'arn:aws:ecs:region:account:task-definition/my-task-def:2'), + ); + mockRegisterTaskDef.mockReturnValue({ + taskDefinition: { + taskDefinitionArn: 'arn:aws:ecs:region:account:task-definition/my-task-def:3', + }, + }); + mockDescribeTaskDef.mockReturnValue({ + taskDefinition: transformObjectKeys(props.taskDefinitionProperties, lowerCaseFirstCharacter, { + ContainerDefinitions: { + DockerLabels: true, + FirelensConfiguration: { + Options: true, + }, + LogConfiguration: { + Options: true, + }, + }, + Volumes: { + DockerVolumeConfiguration: { + DriverOpts: true, + Labels: true, + }, }, + }), + }); +} + +describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hotswapMode) => { + test('should call registerTaskDefinition and updateService for a difference only in the TaskDefinition with a Family property', async () => { + // GIVEN + setupCurrentTaskDefinition({ + taskDefinitionProperties: { + Family: 'my-task-def', + ContainerDefinitions: [ + { Image: 'image1' }, + ], + }, + includeService: true, }); const cdkStackArtifact = setup.cdkStackArtifactOf({ template: { @@ -94,6 +132,10 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot { image: 'image2' }, ], }); + expect(mockDescribeTaskDef).toBeCalledWith({ + taskDefinition: 'arn:aws:ecs:region:account:task-definition/my-task-def:2', + include: ['TAGS'], + }); expect(mockUpdateService).toBeCalledWith({ service: 'arn:aws:ecs:region:account:service/my-cluster/my-service', cluster: 'my-cluster', @@ -107,34 +149,15 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot test('any other TaskDefinition property change besides ContainerDefinition cannot be hotswapped in CLASSIC mode but does not block HOTSWAP_ONLY mode deployments', async () => { // GIVEN - setup.setCurrentCfnStackTemplate({ - Resources: { - TaskDef: { - Type: 'AWS::ECS::TaskDefinition', - Properties: { - Family: 'my-task-def', - ContainerDefinitions: [ - { Image: 'image1' }, - ], - Cpu: '256', - }, - }, - Service: { - Type: 'AWS::ECS::Service', - Properties: { - TaskDefinition: { Ref: 'TaskDef' }, - }, - }, - }, - }); - setup.pushStackResourceSummaries( - setup.stackSummaryOf('Service', 'AWS::ECS::Service', - 'arn:aws:ecs:region:account:service/my-cluster/my-service'), - ); - mockRegisterTaskDef.mockReturnValue({ - taskDefinition: { - taskDefinitionArn: 'arn:aws:ecs:region:account:task-definition/my-task-def:3', + setupCurrentTaskDefinition({ + taskDefinitionProperties: { + Family: 'my-task-def', + ContainerDefinitions: [ + { Image: 'image1' }, + ], + Cpu: '256', }, + includeService: true, }); const cdkStackArtifact = setup.cdkStackArtifactOf({ template: { @@ -166,6 +189,7 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot // THEN expect(deployStackResult).toBeUndefined(); expect(mockRegisterTaskDef).not.toHaveBeenCalled(); + expect(mockDescribeTaskDef).not.toHaveBeenCalled(); expect(mockUpdateService).not.toHaveBeenCalled(); } else if (hotswapMode === HotswapMode.HOTSWAP_ONLY) { // WHEN @@ -180,6 +204,10 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot ], cpu: '256', // this uses the old value because a new value could cause a service replacement }); + expect(mockDescribeTaskDef).toBeCalledWith({ + taskDefinition: 'arn:aws:ecs:region:account:task-definition/my-task-def:2', + include: ['TAGS'], + }); expect(mockUpdateService).toBeCalledWith({ service: 'arn:aws:ecs:region:account:service/my-cluster/my-service', cluster: 'my-cluster', @@ -194,34 +222,15 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot test('deleting any other TaskDefinition property besides ContainerDefinition results in a full deployment in CLASSIC mode and a hotswap deployment in HOTSWAP_ONLY mode', async () => { // GIVEN - setup.setCurrentCfnStackTemplate({ - Resources: { - TaskDef: { - Type: 'AWS::ECS::TaskDefinition', - Properties: { - Family: 'my-task-def', - ContainerDefinitions: [ - { Image: 'image1' }, - ], - Cpu: '256', - }, - }, - Service: { - Type: 'AWS::ECS::Service', - Properties: { - TaskDefinition: { Ref: 'TaskDef' }, - }, - }, - }, - }); - setup.pushStackResourceSummaries( - setup.stackSummaryOf('Service', 'AWS::ECS::Service', - 'arn:aws:ecs:region:account:service/my-cluster/my-service'), - ); - mockRegisterTaskDef.mockReturnValue({ - taskDefinition: { - taskDefinitionArn: 'arn:aws:ecs:region:account:task-definition/my-task-def:3', + setupCurrentTaskDefinition({ + taskDefinitionProperties: { + Family: 'my-task-def', + ContainerDefinitions: [ + { Image: 'image1' }, + ], + Cpu: '256', }, + includeService: true, }); const cdkStackArtifact = setup.cdkStackArtifactOf({ template: { @@ -252,6 +261,7 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot // THEN expect(deployStackResult).toBeUndefined(); expect(mockRegisterTaskDef).not.toHaveBeenCalled(); + expect(mockDescribeTaskDef).not.toHaveBeenCalled(); expect(mockUpdateService).not.toHaveBeenCalled(); } else if (hotswapMode === HotswapMode.HOTSWAP_ONLY) { // WHEN @@ -259,6 +269,10 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot // THEN expect(deployStackResult).not.toBeUndefined(); + expect(mockDescribeTaskDef).toBeCalledWith({ + taskDefinition: 'arn:aws:ecs:region:account:task-definition/my-task-def:2', + include: ['TAGS'], + }); expect(mockRegisterTaskDef).toBeCalledWith({ family: 'my-task-def', containerDefinitions: [ @@ -280,33 +294,23 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot test('should call registerTaskDefinition and updateService for a difference only in the TaskDefinition without a Family property', async () => { // GIVEN - setup.setCurrentCfnStackTemplate({ - Resources: { - TaskDef: { - Type: 'AWS::ECS::TaskDefinition', - Properties: { - ContainerDefinitions: [ - { Image: 'image1' }, - ], - }, - }, - Service: { - Type: 'AWS::ECS::Service', - Properties: { - TaskDefinition: { Ref: 'TaskDef' }, - }, - }, + setupCurrentTaskDefinition({ + taskDefinitionProperties: { + ContainerDefinitions: [ + { Image: 'image1' }, + ], }, + includeService: true, }); - setup.pushStackResourceSummaries( - setup.stackSummaryOf('TaskDef', 'AWS::ECS::TaskDefinition', - 'arn:aws:ecs:region:account:task-definition/my-task-def:2'), - setup.stackSummaryOf('Service', 'AWS::ECS::Service', - 'arn:aws:ecs:region:account:service/my-cluster/my-service'), - ); - mockRegisterTaskDef.mockReturnValue({ + mockDescribeTaskDef.mockReturnValue({ taskDefinition: { - taskDefinitionArn: 'arn:aws:ecs:region:account:task-definition/my-task-def:3', + taskDefinitionArn: 'arn:aws:ecs:region:account:task-definition/my-task-def:2', + family: 'my-task-def', + containerDefinitions: [ + { + image: 'image1', + }, + ], }, }); const cdkStackArtifact = setup.cdkStackArtifactOf({ @@ -335,6 +339,10 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot // THEN expect(deployStackResult).not.toBeUndefined(); + expect(mockDescribeTaskDef).toBeCalledWith({ + taskDefinition: 'arn:aws:ecs:region:account:task-definition/my-task-def:2', + include: ['TAGS'], + }); expect(mockRegisterTaskDef).toBeCalledWith({ family: 'my-task-def', containerDefinitions: [ @@ -354,26 +362,14 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot test('a difference just in a TaskDefinition, without any services using it, is not hotswappable in FALL_BACK mode', async () => { // GIVEN - setup.setCurrentCfnStackTemplate({ - Resources: { - TaskDef: { - Type: 'AWS::ECS::TaskDefinition', - Properties: { - ContainerDefinitions: [ - { Image: 'image1' }, - ], - }, - }, - }, - }); - setup.pushStackResourceSummaries( - setup.stackSummaryOf('TaskDef', 'AWS::ECS::TaskDefinition', - 'arn:aws:ecs:region:account:task-definition/my-task-def:2'), - ); - mockRegisterTaskDef.mockReturnValue({ - taskDefinition: { - taskDefinitionArn: 'arn:aws:ecs:region:account:task-definition/my-task-def:3', + setupCurrentTaskDefinition({ + taskDefinitionProperties: { + Family: 'my-task-def', + ContainerDefinitions: [ + { Image: 'image1' }, + ], }, + includeService: false, }); const cdkStackArtifact = setup.cdkStackArtifactOf({ template: { @@ -396,6 +392,7 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot // THEN expect(deployStackResult).toBeUndefined(); + expect(mockDescribeTaskDef).not.toHaveBeenCalled(); expect(mockRegisterTaskDef).not.toHaveBeenCalled(); expect(mockUpdateService).not.toHaveBeenCalled(); } else if (hotswapMode === HotswapMode.HOTSWAP_ONLY) { @@ -404,6 +401,10 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot // THEN expect(deployStackResult).not.toBeUndefined(); + expect(mockDescribeTaskDef).toBeCalledWith({ + taskDefinition: 'arn:aws:ecs:region:account:task-definition/my-task-def:2', + include: ['TAGS'], + }); expect(mockRegisterTaskDef).toBeCalledWith({ family: 'my-task-def', containerDefinitions: [ @@ -417,23 +418,15 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot test('if anything besides an ECS Service references the changed TaskDefinition, hotswapping is not possible in CLASSIC mode but is possible in HOTSWAP_ONLY', async () => { // GIVEN - setup.setCurrentCfnStackTemplate({ - Resources: { - TaskDef: { - Type: 'AWS::ECS::TaskDefinition', - Properties: { - Family: 'my-task-def', - ContainerDefinitions: [ - { Image: 'image1' }, - ], - }, - }, - Service: { - Type: 'AWS::ECS::Service', - Properties: { - TaskDefinition: { Ref: 'TaskDef' }, - }, - }, + setupCurrentTaskDefinition({ + taskDefinitionProperties: { + Family: 'my-task-def', + ContainerDefinitions: [ + { Image: 'image1' }, + ], + }, + includeService: true, + otherResources: { Function: { Type: 'AWS::Lambda::Function', Properties: { @@ -446,15 +439,6 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot }, }, }); - setup.pushStackResourceSummaries( - setup.stackSummaryOf('Service', 'AWS::ECS::Service', - 'arn:aws:ecs:region:account:service/my-cluster/my-service'), - ); - mockRegisterTaskDef.mockReturnValue({ - taskDefinition: { - taskDefinitionArn: 'arn:aws:ecs:region:account:task-definition/my-task-def:3', - }, - }); const cdkStackArtifact = setup.cdkStackArtifactOf({ template: { Resources: { @@ -493,6 +477,7 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot // THEN expect(deployStackResult).toBeUndefined(); + expect(mockDescribeTaskDef).not.toHaveBeenCalled(); expect(mockRegisterTaskDef).not.toHaveBeenCalled(); expect(mockUpdateService).not.toHaveBeenCalled(); } else if (hotswapMode === HotswapMode.HOTSWAP_ONLY) { @@ -501,6 +486,10 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot // THEN expect(deployStackResult).not.toBeUndefined(); + expect(mockDescribeTaskDef).toBeCalledWith({ + taskDefinition: 'arn:aws:ecs:region:account:task-definition/my-task-def:2', + include: ['TAGS'], + }); expect(mockRegisterTaskDef).toBeCalledWith({ family: 'my-task-def', containerDefinitions: [ @@ -519,43 +508,328 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot } }); - test('should call registerTaskDefinition with certain properties not lowercased', async () => { + test('should call registerTaskDefinition, describeTaskDefinition, and updateService for a difference only in the container image but with environment variables of unsupported intrinsics', async () => { // GIVEN - setup.setCurrentCfnStackTemplate({ - Resources: { - TaskDef: { - Type: 'AWS::ECS::TaskDefinition', - Properties: { - Family: 'my-task-def', - ContainerDefinitions: [ - { Image: 'image1' }, + setupCurrentTaskDefinition({ + taskDefinitionProperties: { + Family: 'my-task-def', + ContainerDefinitions: [ + { + Image: 'image1', + Environment: [ + { + Name: 'FOO', + Value: { 'Fn::GetAtt': ['Bar', 'Baz'] }, + }, ], - Volumes: [ + }, + ], + }, + includeService: true, + }); + mockDescribeTaskDef.mockReturnValue({ + taskDefinition: { + taskDefinitionArn: 'arn:aws:ecs:region:account:task-definition/my-task-def:2', + family: 'my-task-def', + containerDefinitions: [ + { + image: 'image1', + environment: [ { - DockerVolumeConfiguration: { - DriverOpts: { Option1: 'option1' }, - Labels: { Label1: 'label1' }, - }, + name: 'FOO', + value: 'value', }, ], }, + ], + }, + }); + const cdkStackArtifact = setup.cdkStackArtifactOf({ + template: { + Resources: { + TaskDef: { + Type: 'AWS::ECS::TaskDefinition', + Properties: { + Family: 'my-task-def', + ContainerDefinitions: [ + { + Image: 'image2', + Environment: [ + { + Name: 'FOO', + Value: { 'Fn::GetAtt': ['Bar', 'Baz'] }, + }, + ], + }, + ], + }, + }, + Service: { + Type: 'AWS::ECS::Service', + Properties: { + TaskDefinition: { Ref: 'TaskDef' }, + }, + }, }, - Service: { - Type: 'AWS::ECS::Service', - Properties: { - TaskDefinition: { Ref: 'TaskDef' }, + }, + }); + + // WHEN + const deployStackResult = await hotswapMockSdkProvider.tryHotswapDeployment(hotswapMode, cdkStackArtifact); + + // THEN + expect(deployStackResult).not.toBeUndefined(); + expect(mockRegisterTaskDef).toBeCalledWith({ + family: 'my-task-def', + containerDefinitions: [ + { + image: 'image2', + environment: [ + { + name: 'FOO', + value: 'value', + }, + ], + }, + ], + }); + expect(mockUpdateService).toBeCalledWith({ + service: 'arn:aws:ecs:region:account:service/my-cluster/my-service', + cluster: 'my-cluster', + taskDefinition: 'arn:aws:ecs:region:account:task-definition/my-task-def:3', + deploymentConfiguration: { + minimumHealthyPercent: 0, + }, + forceNewDeployment: true, + }); + }); + + test('should call registerTaskDefinition, describeTaskDefinition, and updateService for a simple environment variable addition', async () => { + // GIVEN + setupCurrentTaskDefinition({ + taskDefinitionProperties: { + Family: 'my-task-def', + ContainerDefinitions: [ + { + Image: 'image1', + Environment: [ + { + Name: 'FOO', + Value: { 'Fn::GetAtt': ['Bar', 'Baz'] }, + }, + ], + }, + ], + }, + includeService: true, + }); + mockDescribeTaskDef.mockReturnValue({ + taskDefinition: { + taskDefinitionArn: 'arn:aws:ecs:region:account:task-definition/my-task-def:2', + family: 'my-task-def', + containerDefinitions: [ + { + image: 'image1', + environment: [ + { + name: 'FOO', + value: 'value', + }, + ], + }, + ], + }, + }); + const cdkStackArtifact = setup.cdkStackArtifactOf({ + template: { + Resources: { + TaskDef: { + Type: 'AWS::ECS::TaskDefinition', + Properties: { + Family: 'my-task-def', + ContainerDefinitions: [ + { + Image: 'image2', + Environment: [ + { + Name: 'FOO', + Value: { 'Fn::GetAtt': ['Bar', 'Baz'] }, + }, + { + Name: 'BAR', + Value: '1', + }, + ], + }, + ], + }, + }, + Service: { + Type: 'AWS::ECS::Service', + Properties: { + TaskDefinition: { Ref: 'TaskDef' }, + }, }, }, }, }); - setup.pushStackResourceSummaries( - setup.stackSummaryOf('Service', 'AWS::ECS::Service', - 'arn:aws:ecs:region:account:service/my-cluster/my-service'), - ); - mockRegisterTaskDef.mockReturnValue({ + + // WHEN + const deployStackResult = await hotswapMockSdkProvider.tryHotswapDeployment(hotswapMode, cdkStackArtifact); + + // THEN + expect(deployStackResult).not.toBeUndefined(); + expect(mockRegisterTaskDef).toBeCalledWith({ + family: 'my-task-def', + containerDefinitions: [ + { + image: 'image2', + environment: [ + { + name: 'FOO', + value: 'value', + }, + { + name: 'BAR', + value: '1', + }, + ], + }, + ], + }); + expect(mockUpdateService).toBeCalledWith({ + service: 'arn:aws:ecs:region:account:service/my-cluster/my-service', + cluster: 'my-cluster', + taskDefinition: 'arn:aws:ecs:region:account:task-definition/my-task-def:3', + deploymentConfiguration: { + minimumHealthyPercent: 0, + }, + forceNewDeployment: true, + }); + }); + + test('should call registerTaskDefinition, describeTaskDefinition, and updateService for a environment variable deletion', async () => { + // GIVEN + setupCurrentTaskDefinition({ + taskDefinitionProperties: { + Family: 'my-task-def', + ContainerDefinitions: [ + { + Image: 'image1', + Environment: [ + { + Name: 'FOO', + Value: { 'Fn::GetAtt': ['Bar', 'Baz'] }, + }, + { + Name: 'BAR', + Value: '1', + }, + ], + }, + ], + }, + includeService: true, + }); + mockDescribeTaskDef.mockReturnValue({ taskDefinition: { - taskDefinitionArn: 'arn:aws:ecs:region:account:task-definition/my-task-def:3', + taskDefinitionArn: 'arn:aws:ecs:region:account:task-definition/my-task-def:2', + family: 'my-task-def', + containerDefinitions: [ + { + image: 'image1', + environment: [ + { + name: 'FOO', + value: 'value', + }, + { + name: 'BAR', + value: '1', + }, + ], + }, + ], + }, + }); + const cdkStackArtifact = setup.cdkStackArtifactOf({ + template: { + Resources: { + TaskDef: { + Type: 'AWS::ECS::TaskDefinition', + Properties: { + Family: 'my-task-def', + ContainerDefinitions: [ + { + Image: 'image2', + Environment: [ + { + Name: 'FOO', + Value: { 'Fn::GetAtt': ['Bar', 'Baz'] }, + }, + ], + }, + ], + }, + }, + Service: { + Type: 'AWS::ECS::Service', + Properties: { + TaskDefinition: { Ref: 'TaskDef' }, + }, + }, + }, + }, + }); + + // WHEN + const deployStackResult = await hotswapMockSdkProvider.tryHotswapDeployment(hotswapMode, cdkStackArtifact); + + // THEN + expect(deployStackResult).not.toBeUndefined(); + expect(mockRegisterTaskDef).toBeCalledWith({ + family: 'my-task-def', + containerDefinitions: [ + { + image: 'image2', + environment: [ + { + name: 'FOO', + value: 'value', + }, + ], + }, + ], + }); + expect(mockUpdateService).toBeCalledWith({ + service: 'arn:aws:ecs:region:account:service/my-cluster/my-service', + cluster: 'my-cluster', + taskDefinition: 'arn:aws:ecs:region:account:task-definition/my-task-def:3', + deploymentConfiguration: { + minimumHealthyPercent: 0, + }, + forceNewDeployment: true, + }); + }); + + test('should call registerTaskDefinition with certain properties not lowercased', async () => { + // GIVEN + setupCurrentTaskDefinition({ + taskDefinitionProperties: { + Family: 'my-task-def', + ContainerDefinitions: [ + { Image: 'image1' }, + ], + Volumes: [ + { + DockerVolumeConfiguration: { + DriverOpts: { Option1: 'option1' }, + Labels: { Label1: 'label1' }, + }, + }, + ], }, + includeService: true, }); const cdkStackArtifact = setup.cdkStackArtifactOf({ template: { From 0446e4aae38d61b8e8a7adb1bd56655887880aa1 Mon Sep 17 00:00:00 2001 From: Rico Hermans Date: Thu, 17 Aug 2023 11:11:52 +0200 Subject: [PATCH 2/4] fix(lambda): use of `currentVersion` fails deployment after upgrade (#26777) Between version `2.87.0` and version `2.88.0`, the hash calculation used to make sure that `fn.currentVersion` is automatically updated when a new version of the Lambda Function is deployed changed. This causes a creation of a new Version upon upgrading CDK, but that new Version creation will fail because the underlying Function hasn't changed. The change was due to property ordering used in calculating a unique hash for the Function configuration. This change restores the property ordering to the pre-2.88.0 behavior. Fixes #26739. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../aws-lambda/lib/function-hash.ts | 136 ++++++++++++------ .../aws-lambda/test/function.test.ts | 62 ++++++++ packages/aws-cdk-lib/aws-lambda/test/x.zip | 1 + 3 files changed, 155 insertions(+), 44 deletions(-) create mode 100644 packages/aws-cdk-lib/aws-lambda/test/x.zip diff --git a/packages/aws-cdk-lib/aws-lambda/lib/function-hash.ts b/packages/aws-cdk-lib/aws-lambda/lib/function-hash.ts index f853db60b1d8f..87a224016b052 100644 --- a/packages/aws-cdk-lib/aws-lambda/lib/function-hash.ts +++ b/packages/aws-cdk-lib/aws-lambda/lib/function-hash.ts @@ -8,26 +8,16 @@ export function calculateFunctionHash(fn: LambdaFunction, additional: string = ' const stack = Stack.of(fn); const functionResource = fn.node.defaultChild as CfnResource; - - // render the cloudformation resource from this function - const config = stack.resolve((functionResource as any)._toCloudFormation()); - // config is of the shape: { Resources: { LogicalId: { Type: 'Function', Properties: { ... } }}} - const resources = config.Resources; - const resourceKeys = Object.keys(resources); - if (resourceKeys.length !== 1) { - throw new Error(`Expected one rendered CloudFormation resource but found ${resourceKeys.length}`); - } - const logicalId = resourceKeys[0]; - const properties = resources[logicalId].Properties; + const { properties, template, logicalId } = resolveSingleResourceProperties(stack, functionResource); let stringifiedConfig; if (FeatureFlags.of(fn).isEnabled(LAMBDA_RECOGNIZE_VERSION_PROPS)) { - const updatedProps = sortProperties(filterUsefulKeys(properties)); + const updatedProps = sortFunctionProperties(filterUsefulKeys(properties)); stringifiedConfig = JSON.stringify(updatedProps); } else { - const sorted = sortProperties(properties); - config.Resources[logicalId].Properties = sorted; - stringifiedConfig = JSON.stringify(config); + const sorted = sortFunctionProperties(properties); + template.Resources[logicalId].Properties = sorted; + stringifiedConfig = JSON.stringify(template); } if (FeatureFlags.of(fn).isEnabled(LAMBDA_RECOGNIZE_LAYER_VERSION)) { @@ -103,26 +93,6 @@ function filterUsefulKeys(properties: any) { return ret; } -function sortProperties(properties: any) { - const ret: any = {}; - // We take all required properties in the order that they were historically, - // to make sure the hash we calculate is stable. - // There cannot be more required properties added in the future, - // as that would be a backwards-incompatible change. - const requiredProperties = ['Code', 'Handler', 'Role', 'Runtime']; - for (const requiredProperty of requiredProperties) { - ret[requiredProperty] = properties[requiredProperty]; - } - // then, add all of the non-required properties, - // in the original order - for (const property of Object.keys(properties)) { - if (requiredProperties.indexOf(property) === -1) { - ret[property] = properties[property]; - } - } - return ret; -} - function calculateLayersHash(layers: ILayerVersion[]): string { const layerConfig: {[key: string]: any } = {}; for (const layer of layers) { @@ -143,17 +113,95 @@ function calculateLayersHash(layers: ILayerVersion[]): string { } continue; } - const config = stack.resolve((layerResource as any)._toCloudFormation()); - const resources = config.Resources; - const resourceKeys = Object.keys(resources); - if (resourceKeys.length !== 1) { - throw new Error(`Expected one rendered CloudFormation resource but found ${resourceKeys.length}`); - } - const logicalId = resourceKeys[0]; - const properties = resources[logicalId].Properties; + + const { properties } = resolveSingleResourceProperties(stack, layerResource); + // all properties require replacement, so they are all version locked. - layerConfig[layer.node.id] = properties; + layerConfig[layer.node.id] = sortLayerVersionProperties(properties); } return md5hash(JSON.stringify(layerConfig)); } + +/** + * Sort properties in an object according to a sort order of known keys + * + * Any additional keys are added at the end, but also sorted. + * + * We only sort one level deep, because we rely on the fact that everything + * that needs to be sorted happens to be sorted by the codegen already, and + * we explicitly rely on some objects NOT being sorted. + */ +class PropertySort { + constructor(private readonly knownKeysOrder: string[]) { + } + + public sortObject(properties: any): any { + const ret: any = {}; + + // Scratch-off set for keys we don't know about yet + const unusedKeys = new Set(Object.keys(properties)); + for (const prop of this.knownKeysOrder) { + ret[prop] = properties[prop]; + unusedKeys.delete(prop); + } + + for (const prop of Array.from(unusedKeys).sort()) { + ret[prop] = properties[prop]; + } + + return ret; + } +} + +/** + * Sort properties in a stable order, even as we switch to new codegen + * + * <=2.87.0, we used to generate properties in the order that they occurred in + * the CloudFormation spec. >= 2.88.0, we switched to a new spec source, which + * sorts the properties lexicographically. The order change changed the hash, + * even though the properties themselves have not changed. + * + * We now have a set of properties with the sort order <=2.87.0, and add any + * additional properties later on, but also sort them. + * + * We should be making sure that the orderings for all subobjects + * between 2.87.0 and 2.88.0 are the same, but fortunately all the subobjects + * were already in lexicographic order in <=2.87.0 so we only need to sort some + * top-level properties on the resource. + * + * We also can't deep-sort everything, because for backwards compatibility + * reasons we have a test that ensures that environment variables are not + * lexicographically sorted, but emitted in the order they are added in source + * code, so for now we rely on the codegen being lexicographically sorted. + */ +function sortFunctionProperties(properties: any) { + return new PropertySort([ + // <= 2.87 explicitly fixed order + 'Code', 'Handler', 'Role', 'Runtime', + // <= 2.87 implicitly fixed order + 'Architectures', 'CodeSigningConfigArn', 'DeadLetterConfig', 'Description', 'Environment', + 'EphemeralStorage', 'FileSystemConfigs', 'FunctionName', 'ImageConfig', 'KmsKeyArn', 'Layers', + 'MemorySize', 'PackageType', 'ReservedConcurrentExecutions', 'RuntimeManagementConfig', 'SnapStart', + 'Tags', 'Timeout', 'TracingConfig', 'VpcConfig', + ]).sortObject(properties); +} + +function sortLayerVersionProperties(properties: any) { + return new PropertySort([ + // <=2.87.0 implicit sort order + 'Content', 'CompatibleArchitectures', 'CompatibleRuntimes', 'Description', + 'LayerName', 'LicenseInfo', + ]).sortObject(properties); +} + +function resolveSingleResourceProperties(stack: Stack, res: CfnResource): any { + const template = stack.resolve(res._toCloudFormation()); + const resources = template.Resources; + const resourceKeys = Object.keys(resources); + if (resourceKeys.length !== 1) { + throw new Error(`Expected one rendered CloudFormation resource but found ${resourceKeys.length}`); + } + const logicalId = resourceKeys[0]; + return { properties: resources[logicalId].Properties, template, logicalId }; +} diff --git a/packages/aws-cdk-lib/aws-lambda/test/function.test.ts b/packages/aws-cdk-lib/aws-lambda/test/function.test.ts index 83d25a6c13155..f4364373cdce7 100644 --- a/packages/aws-cdk-lib/aws-lambda/test/function.test.ts +++ b/packages/aws-cdk-lib/aws-lambda/test/function.test.ts @@ -3246,6 +3246,68 @@ test('set SnapStart to desired value', () => { }); }); +test('test 2.87.0 version hash stability', () => { + // GIVEN + const app = new cdk.App({ + context: { + '@aws-cdk/aws-lambda:recognizeLayerVersion': true, + }, + }); + const stack = new cdk.Stack(app, 'Stack'); + + // WHEN + const layer = new lambda.LayerVersion(stack, 'MyLayer', { + code: lambda.Code.fromAsset(path.join(__dirname, 'x.zip')), + compatibleRuntimes: [ + lambda.Runtime.NODEJS_18_X, + ], + }); + + const role = new iam.Role(stack, 'MyRole', { + assumedBy: new iam.ServicePrincipal('lambda.amazonaws.com'), + managedPolicies: [ + iam.ManagedPolicy.fromAwsManagedPolicyName('service-role/AWSLambdaBasicExecutionRole'), + iam.ManagedPolicy.fromAwsManagedPolicyName('AWSXRayDaemonWriteAccess'), + ], + }); + + const lambdaFn = new lambda.Function(stack, 'MyLambda', { + runtime: lambda.Runtime.NODEJS_18_X, + memorySize: 128, + handler: 'index.handler', + timeout: cdk.Duration.seconds(30), + environment: { + VARIABLE_1: 'ONE', + }, + code: lambda.Code.fromAsset(path.join(__dirname, 'x.zip')), + role, + currentVersionOptions: { + removalPolicy: cdk.RemovalPolicy.RETAIN, + }, + layers: [ + layer, + ], + }); + + new lambda.Alias(stack, 'MyAlias', { + aliasName: 'current', + version: lambdaFn.currentVersion, + }); + + // THEN + // Precalculated version hash using 2.87.0 version + Template.fromStack(stack).hasResource('AWS::Lambda::Alias', { + Properties: { + FunctionVersion: { + 'Fn::GetAtt': [ + 'MyLambdaCurrentVersionE7A382CCd55a48b26bd9a860d8842137f2243c37', + 'Version', + ], + }, + }, + }); +}); + function newTestLambda(scope: constructs.Construct) { return new lambda.Function(scope, 'MyLambda', { code: new lambda.InlineCode('foo'), diff --git a/packages/aws-cdk-lib/aws-lambda/test/x.zip b/packages/aws-cdk-lib/aws-lambda/test/x.zip new file mode 100644 index 0000000000000..c1b0730e01334 --- /dev/null +++ b/packages/aws-cdk-lib/aws-lambda/test/x.zip @@ -0,0 +1 @@ +x \ No newline at end of file From 8dc51900e09d82685fb34e199289796504bec248 Mon Sep 17 00:00:00 2001 From: Luca Pizzini Date: Thu, 17 Aug 2023 13:49:04 +0200 Subject: [PATCH 3/4] fix(apigateway): duplicate methodResponses if the same array is reused between addMethod calls (#26636) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adding a new method to an API with `addMethod` and passing `methodOptions.methodResponses` was generating duplicate entries using `StepFunctionsIntegration.startExecution` as integration: For example: ``` const integ = apigw.StepFunctionsIntegration.startExecution(stateMachine, integrationOptions); api.root.addMethod('GET', integ, methodOptions); api.root.addMethod('POST', integ, methodOptions); ``` Would generate (fails on deployment): ``` "MethodResponses": [ { "ResponseParameters": { "method.response.header.Access-Control-Allow-Origin": true }, "StatusCode": "200" }, { "ResponseModels": { "application/json": "Empty" }, "StatusCode": "200" }, { "ResponseModels": { "application/json": "Error" }, "StatusCode": "400" }, { "ResponseModels": { "application/json": "Error" }, "StatusCode": "500" }, { "ResponseModels": { "application/json": "Empty" }, "StatusCode": "200" }, { "ResponseModels": { "application/json": "Error" }, "StatusCode": "400" }, { "ResponseModels": { "application/json": "Error" }, "StatusCode": "500" } ], ``` With this fix, it will keep only the specified `methodResponses`. Also, the `integrationResponses` option in `StepFunctionsIntegration.startExecution` was not used by the corresponding `Integration`. This fix will allow to use the specified option value. Closes #26586. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- ...y-stepfunctions-startexecution.assets.json | 19 + ...stepfunctions-startexecution.template.json | 394 +++++++++++ ...efaultTestDeployAssert3A9ACD49.assets.json | 19 + ...aultTestDeployAssert3A9ACD49.template.json | 36 + .../cdk.out | 1 + .../integ.json | 12 + .../manifest.json | 198 ++++++ .../tree.json | 668 ++++++++++++++++++ .../integ.stepfunctions-startexecution.ts | 51 ++ .../lib/integrations/stepfunctions.ts | 2 +- .../aws-cdk-lib/aws-apigateway/lib/method.ts | 26 +- .../test/integrations/stepfunctions.test.ts | 116 +++ 12 files changed, 1531 insertions(+), 11 deletions(-) create mode 100644 packages/@aws-cdk-testing/framework-integ/test/aws-apigateway/test/integ.stepfunctions-startexecution.js.snapshot/aws-cdk-aws-apigateway-stepfunctions-startexecution.assets.json create mode 100644 packages/@aws-cdk-testing/framework-integ/test/aws-apigateway/test/integ.stepfunctions-startexecution.js.snapshot/aws-cdk-aws-apigateway-stepfunctions-startexecution.template.json create mode 100644 packages/@aws-cdk-testing/framework-integ/test/aws-apigateway/test/integ.stepfunctions-startexecution.js.snapshot/awsapigatewaystepfunctionsstartexecutionDefaultTestDeployAssert3A9ACD49.assets.json create mode 100644 packages/@aws-cdk-testing/framework-integ/test/aws-apigateway/test/integ.stepfunctions-startexecution.js.snapshot/awsapigatewaystepfunctionsstartexecutionDefaultTestDeployAssert3A9ACD49.template.json create mode 100644 packages/@aws-cdk-testing/framework-integ/test/aws-apigateway/test/integ.stepfunctions-startexecution.js.snapshot/cdk.out create mode 100644 packages/@aws-cdk-testing/framework-integ/test/aws-apigateway/test/integ.stepfunctions-startexecution.js.snapshot/integ.json create mode 100644 packages/@aws-cdk-testing/framework-integ/test/aws-apigateway/test/integ.stepfunctions-startexecution.js.snapshot/manifest.json create mode 100644 packages/@aws-cdk-testing/framework-integ/test/aws-apigateway/test/integ.stepfunctions-startexecution.js.snapshot/tree.json create mode 100644 packages/@aws-cdk-testing/framework-integ/test/aws-apigateway/test/integ.stepfunctions-startexecution.ts diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-apigateway/test/integ.stepfunctions-startexecution.js.snapshot/aws-cdk-aws-apigateway-stepfunctions-startexecution.assets.json b/packages/@aws-cdk-testing/framework-integ/test/aws-apigateway/test/integ.stepfunctions-startexecution.js.snapshot/aws-cdk-aws-apigateway-stepfunctions-startexecution.assets.json new file mode 100644 index 0000000000000..b4c64ab60cc31 --- /dev/null +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-apigateway/test/integ.stepfunctions-startexecution.js.snapshot/aws-cdk-aws-apigateway-stepfunctions-startexecution.assets.json @@ -0,0 +1,19 @@ +{ + "version": "33.0.0", + "files": { + "11071ca702d34295486ea4a64c45b64857f4bd23c672fe0895a0b7e753065d77": { + "source": { + "path": "aws-cdk-aws-apigateway-stepfunctions-startexecution.template.json", + "packaging": "file" + }, + "destinations": { + "current_account-current_region": { + "bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}", + "objectKey": "11071ca702d34295486ea4a64c45b64857f4bd23c672fe0895a0b7e753065d77.json", + "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}" + } + } + } + }, + "dockerImages": {} +} \ No newline at end of file diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-apigateway/test/integ.stepfunctions-startexecution.js.snapshot/aws-cdk-aws-apigateway-stepfunctions-startexecution.template.json b/packages/@aws-cdk-testing/framework-integ/test/aws-apigateway/test/integ.stepfunctions-startexecution.js.snapshot/aws-cdk-aws-apigateway-stepfunctions-startexecution.template.json new file mode 100644 index 0000000000000..d117c9978ce39 --- /dev/null +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-apigateway/test/integ.stepfunctions-startexecution.js.snapshot/aws-cdk-aws-apigateway-stepfunctions-startexecution.template.json @@ -0,0 +1,394 @@ +{ + "Resources": { + "myrestapiBAC2BF45": { + "Type": "AWS::ApiGateway::RestApi", + "Properties": { + "Name": "my-rest-api" + } + }, + "myrestapiDeployment010A9D4F8d8391dd5e41be487121f43257df00d7": { + "Type": "AWS::ApiGateway::Deployment", + "Properties": { + "Description": "Automatically created by the RestApi construct", + "RestApiId": { + "Ref": "myrestapiBAC2BF45" + } + }, + "DependsOn": [ + "myrestapiGET3A49A218", + "myrestapiPOST155A9625" + ] + }, + "myrestapiDeploymentStageprod3140E1BE": { + "Type": "AWS::ApiGateway::Stage", + "Properties": { + "DeploymentId": { + "Ref": "myrestapiDeployment010A9D4F8d8391dd5e41be487121f43257df00d7" + }, + "RestApiId": { + "Ref": "myrestapiBAC2BF45" + }, + "StageName": "prod" + } + }, + "myrestapiGETStartSyncExecutionRoleC284C05B": { + "Type": "AWS::IAM::Role", + "Properties": { + "AssumeRolePolicyDocument": { + "Statement": [ + { + "Action": "sts:AssumeRole", + "Effect": "Allow", + "Principal": { + "Service": "apigateway.amazonaws.com" + } + } + ], + "Version": "2012-10-17" + } + } + }, + "myrestapiGETStartSyncExecutionRoleDefaultPolicy8B2F6ADF": { + "Type": "AWS::IAM::Policy", + "Properties": { + "PolicyDocument": { + "Statement": [ + { + "Action": "states:StartSyncExecution", + "Effect": "Allow", + "Resource": { + "Ref": "StateMachine2E01A3A5" + } + } + ], + "Version": "2012-10-17" + }, + "PolicyName": "myrestapiGETStartSyncExecutionRoleDefaultPolicy8B2F6ADF", + "Roles": [ + { + "Ref": "myrestapiGETStartSyncExecutionRoleC284C05B" + } + ] + } + }, + "myrestapiGET3A49A218": { + "Type": "AWS::ApiGateway::Method", + "Properties": { + "AuthorizationType": "NONE", + "HttpMethod": "GET", + "Integration": { + "Credentials": { + "Fn::GetAtt": [ + "myrestapiGETStartSyncExecutionRoleC284C05B", + "Arn" + ] + }, + "IntegrationHttpMethod": "POST", + "IntegrationResponses": [ + { + "ResponseParameters": { + "method.response.header.Access-Control-Allow-Origin": "'*'" + }, + "StatusCode": "200" + } + ], + "PassthroughBehavior": "NEVER", + "RequestTemplates": { + "application/json": { + "Fn::Join": [ + "", + [ + "## Velocity Template used for API Gateway request mapping template\n##\n## This template forwards the request body, header, path, and querystring\n## to the execution input of the state machine.\n##\n## \"@@\" is used here as a placeholder for '\"' to avoid using escape characters.\n\n#set($inputString = '')\n#set($includeHeaders = false)\n#set($includeQueryString = true)\n#set($includePath = true)\n#set($includeAuthorizer = false)\n#set($allParams = $input.params())\n{\n \"stateMachineArn\": \"", + { + "Ref": "StateMachine2E01A3A5" + }, + "\",\n\n #set($inputString = \"$inputString,@@body@@: $input.body\")\n\n #if ($includeHeaders)\n #set($inputString = \"$inputString, @@header@@:{\")\n #foreach($paramName in $allParams.header.keySet())\n #set($inputString = \"$inputString @@$paramName@@: @@$util.escapeJavaScript($allParams.header.get($paramName))@@\")\n #if($foreach.hasNext)\n #set($inputString = \"$inputString,\")\n #end\n #end\n #set($inputString = \"$inputString }\")\n \n #end\n\n #if ($includeQueryString)\n #set($inputString = \"$inputString, @@querystring@@:{\")\n #foreach($paramName in $allParams.querystring.keySet())\n #set($inputString = \"$inputString @@$paramName@@: @@$util.escapeJavaScript($allParams.querystring.get($paramName))@@\")\n #if($foreach.hasNext)\n #set($inputString = \"$inputString,\")\n #end\n #end\n #set($inputString = \"$inputString }\")\n #end\n\n #if ($includePath)\n #set($inputString = \"$inputString, @@path@@:{\")\n #foreach($paramName in $allParams.path.keySet())\n #set($inputString = \"$inputString @@$paramName@@: @@$util.escapeJavaScript($allParams.path.get($paramName))@@\")\n #if($foreach.hasNext)\n #set($inputString = \"$inputString,\")\n #end\n #end\n #set($inputString = \"$inputString }\")\n #end\n \n #if ($includeAuthorizer)\n #set($inputString = \"$inputString, @@authorizer@@:{\")\n #foreach($paramName in $context.authorizer.keySet())\n #set($inputString = \"$inputString @@$paramName@@: @@$util.escapeJavaScript($context.authorizer.get($paramName))@@\")\n #if($foreach.hasNext)\n #set($inputString = \"$inputString,\")\n #end\n #end\n #set($inputString = \"$inputString }\")\n #end\n\n #set($requestContext = \"\")\n ## Check if the request context should be included as part of the execution input\n #if($requestContext && !$requestContext.empty)\n #set($inputString = \"$inputString,\")\n #set($inputString = \"$inputString @@requestContext@@: $requestContext\")\n #end\n\n #set($inputString = \"$inputString}\")\n #set($inputString = $inputString.replaceAll(\"@@\",'\"'))\n #set($len = $inputString.length() - 1)\n \"input\": \"{$util.escapeJavaScript($inputString.substring(1,$len)).replaceAll(\"\\\\'\",\"'\")}\"\n}\n" + ] + ] + } + }, + "Type": "AWS", + "Uri": { + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":apigateway:", + { + "Ref": "AWS::Region" + }, + ":states:action/StartSyncExecution" + ] + ] + } + }, + "MethodResponses": [ + { + "ResponseParameters": { + "method.response.header.Access-Control-Allow-Origin": true + }, + "StatusCode": "200" + }, + { + "ResponseModels": { + "application/json": "Empty" + }, + "StatusCode": "200" + }, + { + "ResponseModels": { + "application/json": "Error" + }, + "StatusCode": "400" + }, + { + "ResponseModels": { + "application/json": "Error" + }, + "StatusCode": "500" + } + ], + "ResourceId": { + "Fn::GetAtt": [ + "myrestapiBAC2BF45", + "RootResourceId" + ] + }, + "RestApiId": { + "Ref": "myrestapiBAC2BF45" + } + } + }, + "myrestapiPOSTStartSyncExecutionRole7AFBE835": { + "Type": "AWS::IAM::Role", + "Properties": { + "AssumeRolePolicyDocument": { + "Statement": [ + { + "Action": "sts:AssumeRole", + "Effect": "Allow", + "Principal": { + "Service": "apigateway.amazonaws.com" + } + } + ], + "Version": "2012-10-17" + } + } + }, + "myrestapiPOSTStartSyncExecutionRoleDefaultPolicy7D411AE9": { + "Type": "AWS::IAM::Policy", + "Properties": { + "PolicyDocument": { + "Statement": [ + { + "Action": "states:StartSyncExecution", + "Effect": "Allow", + "Resource": { + "Ref": "StateMachine2E01A3A5" + } + } + ], + "Version": "2012-10-17" + }, + "PolicyName": "myrestapiPOSTStartSyncExecutionRoleDefaultPolicy7D411AE9", + "Roles": [ + { + "Ref": "myrestapiPOSTStartSyncExecutionRole7AFBE835" + } + ] + } + }, + "myrestapiPOST155A9625": { + "Type": "AWS::ApiGateway::Method", + "Properties": { + "AuthorizationType": "NONE", + "HttpMethod": "POST", + "Integration": { + "Credentials": { + "Fn::GetAtt": [ + "myrestapiPOSTStartSyncExecutionRole7AFBE835", + "Arn" + ] + }, + "IntegrationHttpMethod": "POST", + "IntegrationResponses": [ + { + "ResponseParameters": { + "method.response.header.Access-Control-Allow-Origin": "'*'" + }, + "StatusCode": "200" + } + ], + "PassthroughBehavior": "NEVER", + "RequestTemplates": { + "application/json": { + "Fn::Join": [ + "", + [ + "## Velocity Template used for API Gateway request mapping template\n##\n## This template forwards the request body, header, path, and querystring\n## to the execution input of the state machine.\n##\n## \"@@\" is used here as a placeholder for '\"' to avoid using escape characters.\n\n#set($inputString = '')\n#set($includeHeaders = false)\n#set($includeQueryString = true)\n#set($includePath = true)\n#set($includeAuthorizer = false)\n#set($allParams = $input.params())\n{\n \"stateMachineArn\": \"", + { + "Ref": "StateMachine2E01A3A5" + }, + "\",\n\n #set($inputString = \"$inputString,@@body@@: $input.body\")\n\n #if ($includeHeaders)\n #set($inputString = \"$inputString, @@header@@:{\")\n #foreach($paramName in $allParams.header.keySet())\n #set($inputString = \"$inputString @@$paramName@@: @@$util.escapeJavaScript($allParams.header.get($paramName))@@\")\n #if($foreach.hasNext)\n #set($inputString = \"$inputString,\")\n #end\n #end\n #set($inputString = \"$inputString }\")\n \n #end\n\n #if ($includeQueryString)\n #set($inputString = \"$inputString, @@querystring@@:{\")\n #foreach($paramName in $allParams.querystring.keySet())\n #set($inputString = \"$inputString @@$paramName@@: @@$util.escapeJavaScript($allParams.querystring.get($paramName))@@\")\n #if($foreach.hasNext)\n #set($inputString = \"$inputString,\")\n #end\n #end\n #set($inputString = \"$inputString }\")\n #end\n\n #if ($includePath)\n #set($inputString = \"$inputString, @@path@@:{\")\n #foreach($paramName in $allParams.path.keySet())\n #set($inputString = \"$inputString @@$paramName@@: @@$util.escapeJavaScript($allParams.path.get($paramName))@@\")\n #if($foreach.hasNext)\n #set($inputString = \"$inputString,\")\n #end\n #end\n #set($inputString = \"$inputString }\")\n #end\n \n #if ($includeAuthorizer)\n #set($inputString = \"$inputString, @@authorizer@@:{\")\n #foreach($paramName in $context.authorizer.keySet())\n #set($inputString = \"$inputString @@$paramName@@: @@$util.escapeJavaScript($context.authorizer.get($paramName))@@\")\n #if($foreach.hasNext)\n #set($inputString = \"$inputString,\")\n #end\n #end\n #set($inputString = \"$inputString }\")\n #end\n\n #set($requestContext = \"\")\n ## Check if the request context should be included as part of the execution input\n #if($requestContext && !$requestContext.empty)\n #set($inputString = \"$inputString,\")\n #set($inputString = \"$inputString @@requestContext@@: $requestContext\")\n #end\n\n #set($inputString = \"$inputString}\")\n #set($inputString = $inputString.replaceAll(\"@@\",'\"'))\n #set($len = $inputString.length() - 1)\n \"input\": \"{$util.escapeJavaScript($inputString.substring(1,$len)).replaceAll(\"\\\\'\",\"'\")}\"\n}\n" + ] + ] + } + }, + "Type": "AWS", + "Uri": { + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":apigateway:", + { + "Ref": "AWS::Region" + }, + ":states:action/StartSyncExecution" + ] + ] + } + }, + "MethodResponses": [ + { + "ResponseParameters": { + "method.response.header.Access-Control-Allow-Origin": true + }, + "StatusCode": "200" + }, + { + "ResponseModels": { + "application/json": "Empty" + }, + "StatusCode": "200" + }, + { + "ResponseModels": { + "application/json": "Error" + }, + "StatusCode": "400" + }, + { + "ResponseModels": { + "application/json": "Error" + }, + "StatusCode": "500" + } + ], + "ResourceId": { + "Fn::GetAtt": [ + "myrestapiBAC2BF45", + "RootResourceId" + ] + }, + "RestApiId": { + "Ref": "myrestapiBAC2BF45" + } + } + }, + "StateMachineRoleB840431D": { + "Type": "AWS::IAM::Role", + "Properties": { + "AssumeRolePolicyDocument": { + "Statement": [ + { + "Action": "sts:AssumeRole", + "Effect": "Allow", + "Principal": { + "Service": "states.amazonaws.com" + } + } + ], + "Version": "2012-10-17" + } + } + }, + "StateMachine2E01A3A5": { + "Type": "AWS::StepFunctions::StateMachine", + "Properties": { + "DefinitionString": "{\"StartAt\":\"passTask\",\"States\":{\"passTask\":{\"Type\":\"Pass\",\"InputPath\":\"$.somekey\",\"End\":true}}}", + "RoleArn": { + "Fn::GetAtt": [ + "StateMachineRoleB840431D", + "Arn" + ] + }, + "StateMachineType": "EXPRESS" + }, + "DependsOn": [ + "StateMachineRoleB840431D" + ], + "UpdateReplacePolicy": "Delete", + "DeletionPolicy": "Delete" + } + }, + "Outputs": { + "myrestapiEndpoint0DE8A5DE": { + "Value": { + "Fn::Join": [ + "", + [ + "https://", + { + "Ref": "myrestapiBAC2BF45" + }, + ".execute-api.", + { + "Ref": "AWS::Region" + }, + ".", + { + "Ref": "AWS::URLSuffix" + }, + "/", + { + "Ref": "myrestapiDeploymentStageprod3140E1BE" + }, + "/" + ] + ] + } + } + }, + "Parameters": { + "BootstrapVersion": { + "Type": "AWS::SSM::Parameter::Value", + "Default": "/cdk-bootstrap/hnb659fds/version", + "Description": "Version of the CDK Bootstrap resources in this environment, automatically retrieved from SSM Parameter Store. [cdk:skip]" + } + }, + "Rules": { + "CheckBootstrapVersion": { + "Assertions": [ + { + "Assert": { + "Fn::Not": [ + { + "Fn::Contains": [ + [ + "1", + "2", + "3", + "4", + "5" + ], + { + "Ref": "BootstrapVersion" + } + ] + } + ] + }, + "AssertDescription": "CDK bootstrap stack version 6 required. Please run 'cdk bootstrap' with a recent version of the CDK CLI." + } + ] + } + } +} \ No newline at end of file diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-apigateway/test/integ.stepfunctions-startexecution.js.snapshot/awsapigatewaystepfunctionsstartexecutionDefaultTestDeployAssert3A9ACD49.assets.json b/packages/@aws-cdk-testing/framework-integ/test/aws-apigateway/test/integ.stepfunctions-startexecution.js.snapshot/awsapigatewaystepfunctionsstartexecutionDefaultTestDeployAssert3A9ACD49.assets.json new file mode 100644 index 0000000000000..914ca646ec74f --- /dev/null +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-apigateway/test/integ.stepfunctions-startexecution.js.snapshot/awsapigatewaystepfunctionsstartexecutionDefaultTestDeployAssert3A9ACD49.assets.json @@ -0,0 +1,19 @@ +{ + "version": "33.0.0", + "files": { + "21fbb51d7b23f6a6c262b46a9caee79d744a3ac019fd45422d988b96d44b2a22": { + "source": { + "path": "awsapigatewaystepfunctionsstartexecutionDefaultTestDeployAssert3A9ACD49.template.json", + "packaging": "file" + }, + "destinations": { + "current_account-current_region": { + "bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}", + "objectKey": "21fbb51d7b23f6a6c262b46a9caee79d744a3ac019fd45422d988b96d44b2a22.json", + "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}" + } + } + } + }, + "dockerImages": {} +} \ No newline at end of file diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-apigateway/test/integ.stepfunctions-startexecution.js.snapshot/awsapigatewaystepfunctionsstartexecutionDefaultTestDeployAssert3A9ACD49.template.json b/packages/@aws-cdk-testing/framework-integ/test/aws-apigateway/test/integ.stepfunctions-startexecution.js.snapshot/awsapigatewaystepfunctionsstartexecutionDefaultTestDeployAssert3A9ACD49.template.json new file mode 100644 index 0000000000000..ad9d0fb73d1dd --- /dev/null +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-apigateway/test/integ.stepfunctions-startexecution.js.snapshot/awsapigatewaystepfunctionsstartexecutionDefaultTestDeployAssert3A9ACD49.template.json @@ -0,0 +1,36 @@ +{ + "Parameters": { + "BootstrapVersion": { + "Type": "AWS::SSM::Parameter::Value", + "Default": "/cdk-bootstrap/hnb659fds/version", + "Description": "Version of the CDK Bootstrap resources in this environment, automatically retrieved from SSM Parameter Store. [cdk:skip]" + } + }, + "Rules": { + "CheckBootstrapVersion": { + "Assertions": [ + { + "Assert": { + "Fn::Not": [ + { + "Fn::Contains": [ + [ + "1", + "2", + "3", + "4", + "5" + ], + { + "Ref": "BootstrapVersion" + } + ] + } + ] + }, + "AssertDescription": "CDK bootstrap stack version 6 required. Please run 'cdk bootstrap' with a recent version of the CDK CLI." + } + ] + } + } +} \ No newline at end of file diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-apigateway/test/integ.stepfunctions-startexecution.js.snapshot/cdk.out b/packages/@aws-cdk-testing/framework-integ/test/aws-apigateway/test/integ.stepfunctions-startexecution.js.snapshot/cdk.out new file mode 100644 index 0000000000000..560dae10d018f --- /dev/null +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-apigateway/test/integ.stepfunctions-startexecution.js.snapshot/cdk.out @@ -0,0 +1 @@ +{"version":"33.0.0"} \ No newline at end of file diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-apigateway/test/integ.stepfunctions-startexecution.js.snapshot/integ.json b/packages/@aws-cdk-testing/framework-integ/test/aws-apigateway/test/integ.stepfunctions-startexecution.js.snapshot/integ.json new file mode 100644 index 0000000000000..d0083afa83de3 --- /dev/null +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-apigateway/test/integ.stepfunctions-startexecution.js.snapshot/integ.json @@ -0,0 +1,12 @@ +{ + "version": "33.0.0", + "testCases": { + "aws-apigateway-stepfunctions-startexecution/DefaultTest": { + "stacks": [ + "aws-cdk-aws-apigateway-stepfunctions-startexecution" + ], + "assertionStack": "aws-apigateway-stepfunctions-startexecution/DefaultTest/DeployAssert", + "assertionStackName": "awsapigatewaystepfunctionsstartexecutionDefaultTestDeployAssert3A9ACD49" + } + } +} \ No newline at end of file diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-apigateway/test/integ.stepfunctions-startexecution.js.snapshot/manifest.json b/packages/@aws-cdk-testing/framework-integ/test/aws-apigateway/test/integ.stepfunctions-startexecution.js.snapshot/manifest.json new file mode 100644 index 0000000000000..7f56cd4cd730a --- /dev/null +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-apigateway/test/integ.stepfunctions-startexecution.js.snapshot/manifest.json @@ -0,0 +1,198 @@ +{ + "version": "33.0.0", + "artifacts": { + "aws-cdk-aws-apigateway-stepfunctions-startexecution.assets": { + "type": "cdk:asset-manifest", + "properties": { + "file": "aws-cdk-aws-apigateway-stepfunctions-startexecution.assets.json", + "requiresBootstrapStackVersion": 6, + "bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version" + } + }, + "aws-cdk-aws-apigateway-stepfunctions-startexecution": { + "type": "aws:cloudformation:stack", + "environment": "aws://unknown-account/unknown-region", + "properties": { + "templateFile": "aws-cdk-aws-apigateway-stepfunctions-startexecution.template.json", + "validateOnSynth": false, + "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}", + "cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}", + "stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/11071ca702d34295486ea4a64c45b64857f4bd23c672fe0895a0b7e753065d77.json", + "requiresBootstrapStackVersion": 6, + "bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version", + "additionalDependencies": [ + "aws-cdk-aws-apigateway-stepfunctions-startexecution.assets" + ], + "lookupRole": { + "arn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-lookup-role-${AWS::AccountId}-${AWS::Region}", + "requiresBootstrapStackVersion": 8, + "bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version" + } + }, + "dependencies": [ + "aws-cdk-aws-apigateway-stepfunctions-startexecution.assets" + ], + "metadata": { + "/aws-cdk-aws-apigateway-stepfunctions-startexecution/my-rest-api/Resource": [ + { + "type": "aws:cdk:logicalId", + "data": "myrestapiBAC2BF45" + } + ], + "/aws-cdk-aws-apigateway-stepfunctions-startexecution/my-rest-api/Deployment/Resource": [ + { + "type": "aws:cdk:logicalId", + "data": "myrestapiDeployment010A9D4F8d8391dd5e41be487121f43257df00d7" + } + ], + "/aws-cdk-aws-apigateway-stepfunctions-startexecution/my-rest-api/DeploymentStage.prod/Resource": [ + { + "type": "aws:cdk:logicalId", + "data": "myrestapiDeploymentStageprod3140E1BE" + } + ], + "/aws-cdk-aws-apigateway-stepfunctions-startexecution/my-rest-api/Endpoint": [ + { + "type": "aws:cdk:logicalId", + "data": "myrestapiEndpoint0DE8A5DE" + } + ], + "/aws-cdk-aws-apigateway-stepfunctions-startexecution/my-rest-api/Default/GET": [ + { + "type": "aws:cdk:warning", + "data": "addMethodResponse called multiple times with statusCode=200, deployment will be nondeterministic. Use a single addMethodResponse call to configure the entire response." + } + ], + "/aws-cdk-aws-apigateway-stepfunctions-startexecution/my-rest-api/Default/GET/StartSyncExecutionRole/Resource": [ + { + "type": "aws:cdk:logicalId", + "data": "myrestapiGETStartSyncExecutionRoleC284C05B" + } + ], + "/aws-cdk-aws-apigateway-stepfunctions-startexecution/my-rest-api/Default/GET/StartSyncExecutionRole/DefaultPolicy/Resource": [ + { + "type": "aws:cdk:logicalId", + "data": "myrestapiGETStartSyncExecutionRoleDefaultPolicy8B2F6ADF" + } + ], + "/aws-cdk-aws-apigateway-stepfunctions-startexecution/my-rest-api/Default/GET/Resource": [ + { + "type": "aws:cdk:logicalId", + "data": "myrestapiGET3A49A218" + } + ], + "/aws-cdk-aws-apigateway-stepfunctions-startexecution/my-rest-api/Default/POST": [ + { + "type": "aws:cdk:warning", + "data": "addMethodResponse called multiple times with statusCode=200, deployment will be nondeterministic. Use a single addMethodResponse call to configure the entire response." + } + ], + "/aws-cdk-aws-apigateway-stepfunctions-startexecution/my-rest-api/Default/POST/StartSyncExecutionRole/Resource": [ + { + "type": "aws:cdk:logicalId", + "data": "myrestapiPOSTStartSyncExecutionRole7AFBE835" + } + ], + "/aws-cdk-aws-apigateway-stepfunctions-startexecution/my-rest-api/Default/POST/StartSyncExecutionRole/DefaultPolicy/Resource": [ + { + "type": "aws:cdk:logicalId", + "data": "myrestapiPOSTStartSyncExecutionRoleDefaultPolicy7D411AE9" + } + ], + "/aws-cdk-aws-apigateway-stepfunctions-startexecution/my-rest-api/Default/POST/Resource": [ + { + "type": "aws:cdk:logicalId", + "data": "myrestapiPOST155A9625" + } + ], + "/aws-cdk-aws-apigateway-stepfunctions-startexecution/StateMachine/Role/Resource": [ + { + "type": "aws:cdk:logicalId", + "data": "StateMachineRoleB840431D" + } + ], + "/aws-cdk-aws-apigateway-stepfunctions-startexecution/StateMachine/Resource": [ + { + "type": "aws:cdk:logicalId", + "data": "StateMachine2E01A3A5" + } + ], + "/aws-cdk-aws-apigateway-stepfunctions-startexecution/BootstrapVersion": [ + { + "type": "aws:cdk:logicalId", + "data": "BootstrapVersion" + } + ], + "/aws-cdk-aws-apigateway-stepfunctions-startexecution/CheckBootstrapVersion": [ + { + "type": "aws:cdk:logicalId", + "data": "CheckBootstrapVersion" + } + ], + "myrestapiDeployment010A9D4F59b46296e944eb988c600755d169a00d": [ + { + "type": "aws:cdk:logicalId", + "data": "myrestapiDeployment010A9D4F59b46296e944eb988c600755d169a00d", + "trace": [ + "!!DESTRUCTIVE_CHANGES: WILL_DESTROY" + ] + } + ] + }, + "displayName": "aws-cdk-aws-apigateway-stepfunctions-startexecution" + }, + "awsapigatewaystepfunctionsstartexecutionDefaultTestDeployAssert3A9ACD49.assets": { + "type": "cdk:asset-manifest", + "properties": { + "file": "awsapigatewaystepfunctionsstartexecutionDefaultTestDeployAssert3A9ACD49.assets.json", + "requiresBootstrapStackVersion": 6, + "bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version" + } + }, + "awsapigatewaystepfunctionsstartexecutionDefaultTestDeployAssert3A9ACD49": { + "type": "aws:cloudformation:stack", + "environment": "aws://unknown-account/unknown-region", + "properties": { + "templateFile": "awsapigatewaystepfunctionsstartexecutionDefaultTestDeployAssert3A9ACD49.template.json", + "validateOnSynth": false, + "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}", + "cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}", + "stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/21fbb51d7b23f6a6c262b46a9caee79d744a3ac019fd45422d988b96d44b2a22.json", + "requiresBootstrapStackVersion": 6, + "bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version", + "additionalDependencies": [ + "awsapigatewaystepfunctionsstartexecutionDefaultTestDeployAssert3A9ACD49.assets" + ], + "lookupRole": { + "arn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-lookup-role-${AWS::AccountId}-${AWS::Region}", + "requiresBootstrapStackVersion": 8, + "bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version" + } + }, + "dependencies": [ + "awsapigatewaystepfunctionsstartexecutionDefaultTestDeployAssert3A9ACD49.assets" + ], + "metadata": { + "/aws-apigateway-stepfunctions-startexecution/DefaultTest/DeployAssert/BootstrapVersion": [ + { + "type": "aws:cdk:logicalId", + "data": "BootstrapVersion" + } + ], + "/aws-apigateway-stepfunctions-startexecution/DefaultTest/DeployAssert/CheckBootstrapVersion": [ + { + "type": "aws:cdk:logicalId", + "data": "CheckBootstrapVersion" + } + ] + }, + "displayName": "aws-apigateway-stepfunctions-startexecution/DefaultTest/DeployAssert" + }, + "Tree": { + "type": "cdk:tree", + "properties": { + "file": "tree.json" + } + } + } +} \ No newline at end of file diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-apigateway/test/integ.stepfunctions-startexecution.js.snapshot/tree.json b/packages/@aws-cdk-testing/framework-integ/test/aws-apigateway/test/integ.stepfunctions-startexecution.js.snapshot/tree.json new file mode 100644 index 0000000000000..73b02800aefcc --- /dev/null +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-apigateway/test/integ.stepfunctions-startexecution.js.snapshot/tree.json @@ -0,0 +1,668 @@ +{ + "version": "tree-0.1", + "tree": { + "id": "App", + "path": "", + "children": { + "aws-cdk-aws-apigateway-stepfunctions-startexecution": { + "id": "aws-cdk-aws-apigateway-stepfunctions-startexecution", + "path": "aws-cdk-aws-apigateway-stepfunctions-startexecution", + "children": { + "my-rest-api": { + "id": "my-rest-api", + "path": "aws-cdk-aws-apigateway-stepfunctions-startexecution/my-rest-api", + "children": { + "Resource": { + "id": "Resource", + "path": "aws-cdk-aws-apigateway-stepfunctions-startexecution/my-rest-api/Resource", + "attributes": { + "aws:cdk:cloudformation:type": "AWS::ApiGateway::RestApi", + "aws:cdk:cloudformation:props": { + "name": "my-rest-api" + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" + } + }, + "Deployment": { + "id": "Deployment", + "path": "aws-cdk-aws-apigateway-stepfunctions-startexecution/my-rest-api/Deployment", + "children": { + "Resource": { + "id": "Resource", + "path": "aws-cdk-aws-apigateway-stepfunctions-startexecution/my-rest-api/Deployment/Resource", + "attributes": { + "aws:cdk:cloudformation:type": "AWS::ApiGateway::Deployment", + "aws:cdk:cloudformation:props": { + "description": "Automatically created by the RestApi construct", + "restApiId": { + "Ref": "myrestapiBAC2BF45" + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" + } + }, + "DeploymentStage.prod": { + "id": "DeploymentStage.prod", + "path": "aws-cdk-aws-apigateway-stepfunctions-startexecution/my-rest-api/DeploymentStage.prod", + "children": { + "Resource": { + "id": "Resource", + "path": "aws-cdk-aws-apigateway-stepfunctions-startexecution/my-rest-api/DeploymentStage.prod/Resource", + "attributes": { + "aws:cdk:cloudformation:type": "AWS::ApiGateway::Stage", + "aws:cdk:cloudformation:props": { + "deploymentId": { + "Ref": "myrestapiDeployment010A9D4F8d8391dd5e41be487121f43257df00d7" + }, + "restApiId": { + "Ref": "myrestapiBAC2BF45" + }, + "stageName": "prod" + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" + } + }, + "Endpoint": { + "id": "Endpoint", + "path": "aws-cdk-aws-apigateway-stepfunctions-startexecution/my-rest-api/Endpoint", + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" + } + }, + "Default": { + "id": "Default", + "path": "aws-cdk-aws-apigateway-stepfunctions-startexecution/my-rest-api/Default", + "children": { + "GET": { + "id": "GET", + "path": "aws-cdk-aws-apigateway-stepfunctions-startexecution/my-rest-api/Default/GET", + "children": { + "StartSyncExecutionRole": { + "id": "StartSyncExecutionRole", + "path": "aws-cdk-aws-apigateway-stepfunctions-startexecution/my-rest-api/Default/GET/StartSyncExecutionRole", + "children": { + "ImportStartSyncExecutionRole": { + "id": "ImportStartSyncExecutionRole", + "path": "aws-cdk-aws-apigateway-stepfunctions-startexecution/my-rest-api/Default/GET/StartSyncExecutionRole/ImportStartSyncExecutionRole", + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" + } + }, + "Resource": { + "id": "Resource", + "path": "aws-cdk-aws-apigateway-stepfunctions-startexecution/my-rest-api/Default/GET/StartSyncExecutionRole/Resource", + "attributes": { + "aws:cdk:cloudformation:type": "AWS::IAM::Role", + "aws:cdk:cloudformation:props": { + "assumeRolePolicyDocument": { + "Statement": [ + { + "Action": "sts:AssumeRole", + "Effect": "Allow", + "Principal": { + "Service": "apigateway.amazonaws.com" + } + } + ], + "Version": "2012-10-17" + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" + } + }, + "DefaultPolicy": { + "id": "DefaultPolicy", + "path": "aws-cdk-aws-apigateway-stepfunctions-startexecution/my-rest-api/Default/GET/StartSyncExecutionRole/DefaultPolicy", + "children": { + "Resource": { + "id": "Resource", + "path": "aws-cdk-aws-apigateway-stepfunctions-startexecution/my-rest-api/Default/GET/StartSyncExecutionRole/DefaultPolicy/Resource", + "attributes": { + "aws:cdk:cloudformation:type": "AWS::IAM::Policy", + "aws:cdk:cloudformation:props": { + "policyDocument": { + "Statement": [ + { + "Action": "states:StartSyncExecution", + "Effect": "Allow", + "Resource": { + "Ref": "StateMachine2E01A3A5" + } + } + ], + "Version": "2012-10-17" + }, + "policyName": "myrestapiGETStartSyncExecutionRoleDefaultPolicy8B2F6ADF", + "roles": [ + { + "Ref": "myrestapiGETStartSyncExecutionRoleC284C05B" + } + ] + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" + } + }, + "Resource": { + "id": "Resource", + "path": "aws-cdk-aws-apigateway-stepfunctions-startexecution/my-rest-api/Default/GET/Resource", + "attributes": { + "aws:cdk:cloudformation:type": "AWS::ApiGateway::Method", + "aws:cdk:cloudformation:props": { + "authorizationType": "NONE", + "httpMethod": "GET", + "integration": { + "type": "AWS", + "uri": { + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":apigateway:", + { + "Ref": "AWS::Region" + }, + ":states:action/StartSyncExecution" + ] + ] + }, + "integrationHttpMethod": "POST", + "requestTemplates": { + "application/json": { + "Fn::Join": [ + "", + [ + "## Velocity Template used for API Gateway request mapping template\n##\n## This template forwards the request body, header, path, and querystring\n## to the execution input of the state machine.\n##\n## \"@@\" is used here as a placeholder for '\"' to avoid using escape characters.\n\n#set($inputString = '')\n#set($includeHeaders = false)\n#set($includeQueryString = true)\n#set($includePath = true)\n#set($includeAuthorizer = false)\n#set($allParams = $input.params())\n{\n \"stateMachineArn\": \"", + { + "Ref": "StateMachine2E01A3A5" + }, + "\",\n\n #set($inputString = \"$inputString,@@body@@: $input.body\")\n\n #if ($includeHeaders)\n #set($inputString = \"$inputString, @@header@@:{\")\n #foreach($paramName in $allParams.header.keySet())\n #set($inputString = \"$inputString @@$paramName@@: @@$util.escapeJavaScript($allParams.header.get($paramName))@@\")\n #if($foreach.hasNext)\n #set($inputString = \"$inputString,\")\n #end\n #end\n #set($inputString = \"$inputString }\")\n \n #end\n\n #if ($includeQueryString)\n #set($inputString = \"$inputString, @@querystring@@:{\")\n #foreach($paramName in $allParams.querystring.keySet())\n #set($inputString = \"$inputString @@$paramName@@: @@$util.escapeJavaScript($allParams.querystring.get($paramName))@@\")\n #if($foreach.hasNext)\n #set($inputString = \"$inputString,\")\n #end\n #end\n #set($inputString = \"$inputString }\")\n #end\n\n #if ($includePath)\n #set($inputString = \"$inputString, @@path@@:{\")\n #foreach($paramName in $allParams.path.keySet())\n #set($inputString = \"$inputString @@$paramName@@: @@$util.escapeJavaScript($allParams.path.get($paramName))@@\")\n #if($foreach.hasNext)\n #set($inputString = \"$inputString,\")\n #end\n #end\n #set($inputString = \"$inputString }\")\n #end\n \n #if ($includeAuthorizer)\n #set($inputString = \"$inputString, @@authorizer@@:{\")\n #foreach($paramName in $context.authorizer.keySet())\n #set($inputString = \"$inputString @@$paramName@@: @@$util.escapeJavaScript($context.authorizer.get($paramName))@@\")\n #if($foreach.hasNext)\n #set($inputString = \"$inputString,\")\n #end\n #end\n #set($inputString = \"$inputString }\")\n #end\n\n #set($requestContext = \"\")\n ## Check if the request context should be included as part of the execution input\n #if($requestContext && !$requestContext.empty)\n #set($inputString = \"$inputString,\")\n #set($inputString = \"$inputString @@requestContext@@: $requestContext\")\n #end\n\n #set($inputString = \"$inputString}\")\n #set($inputString = $inputString.replaceAll(\"@@\",'\"'))\n #set($len = $inputString.length() - 1)\n \"input\": \"{$util.escapeJavaScript($inputString.substring(1,$len)).replaceAll(\"\\\\'\",\"'\")}\"\n}\n" + ] + ] + } + }, + "passthroughBehavior": "NEVER", + "integrationResponses": [ + { + "responseParameters": { + "method.response.header.Access-Control-Allow-Origin": "'*'" + }, + "statusCode": "200" + } + ], + "credentials": { + "Fn::GetAtt": [ + "myrestapiGETStartSyncExecutionRoleC284C05B", + "Arn" + ] + } + }, + "methodResponses": [ + { + "statusCode": "200", + "responseParameters": { + "method.response.header.Access-Control-Allow-Origin": true + } + }, + { + "statusCode": "200", + "responseModels": { + "application/json": "Empty" + } + }, + { + "statusCode": "400", + "responseModels": { + "application/json": "Error" + } + }, + { + "statusCode": "500", + "responseModels": { + "application/json": "Error" + } + } + ], + "resourceId": { + "Fn::GetAtt": [ + "myrestapiBAC2BF45", + "RootResourceId" + ] + }, + "restApiId": { + "Ref": "myrestapiBAC2BF45" + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" + } + }, + "POST": { + "id": "POST", + "path": "aws-cdk-aws-apigateway-stepfunctions-startexecution/my-rest-api/Default/POST", + "children": { + "StartSyncExecutionRole": { + "id": "StartSyncExecutionRole", + "path": "aws-cdk-aws-apigateway-stepfunctions-startexecution/my-rest-api/Default/POST/StartSyncExecutionRole", + "children": { + "ImportStartSyncExecutionRole": { + "id": "ImportStartSyncExecutionRole", + "path": "aws-cdk-aws-apigateway-stepfunctions-startexecution/my-rest-api/Default/POST/StartSyncExecutionRole/ImportStartSyncExecutionRole", + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" + } + }, + "Resource": { + "id": "Resource", + "path": "aws-cdk-aws-apigateway-stepfunctions-startexecution/my-rest-api/Default/POST/StartSyncExecutionRole/Resource", + "attributes": { + "aws:cdk:cloudformation:type": "AWS::IAM::Role", + "aws:cdk:cloudformation:props": { + "assumeRolePolicyDocument": { + "Statement": [ + { + "Action": "sts:AssumeRole", + "Effect": "Allow", + "Principal": { + "Service": "apigateway.amazonaws.com" + } + } + ], + "Version": "2012-10-17" + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" + } + }, + "DefaultPolicy": { + "id": "DefaultPolicy", + "path": "aws-cdk-aws-apigateway-stepfunctions-startexecution/my-rest-api/Default/POST/StartSyncExecutionRole/DefaultPolicy", + "children": { + "Resource": { + "id": "Resource", + "path": "aws-cdk-aws-apigateway-stepfunctions-startexecution/my-rest-api/Default/POST/StartSyncExecutionRole/DefaultPolicy/Resource", + "attributes": { + "aws:cdk:cloudformation:type": "AWS::IAM::Policy", + "aws:cdk:cloudformation:props": { + "policyDocument": { + "Statement": [ + { + "Action": "states:StartSyncExecution", + "Effect": "Allow", + "Resource": { + "Ref": "StateMachine2E01A3A5" + } + } + ], + "Version": "2012-10-17" + }, + "policyName": "myrestapiPOSTStartSyncExecutionRoleDefaultPolicy7D411AE9", + "roles": [ + { + "Ref": "myrestapiPOSTStartSyncExecutionRole7AFBE835" + } + ] + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" + } + }, + "Resource": { + "id": "Resource", + "path": "aws-cdk-aws-apigateway-stepfunctions-startexecution/my-rest-api/Default/POST/Resource", + "attributes": { + "aws:cdk:cloudformation:type": "AWS::ApiGateway::Method", + "aws:cdk:cloudformation:props": { + "authorizationType": "NONE", + "httpMethod": "POST", + "integration": { + "type": "AWS", + "uri": { + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":apigateway:", + { + "Ref": "AWS::Region" + }, + ":states:action/StartSyncExecution" + ] + ] + }, + "integrationHttpMethod": "POST", + "requestTemplates": { + "application/json": { + "Fn::Join": [ + "", + [ + "## Velocity Template used for API Gateway request mapping template\n##\n## This template forwards the request body, header, path, and querystring\n## to the execution input of the state machine.\n##\n## \"@@\" is used here as a placeholder for '\"' to avoid using escape characters.\n\n#set($inputString = '')\n#set($includeHeaders = false)\n#set($includeQueryString = true)\n#set($includePath = true)\n#set($includeAuthorizer = false)\n#set($allParams = $input.params())\n{\n \"stateMachineArn\": \"", + { + "Ref": "StateMachine2E01A3A5" + }, + "\",\n\n #set($inputString = \"$inputString,@@body@@: $input.body\")\n\n #if ($includeHeaders)\n #set($inputString = \"$inputString, @@header@@:{\")\n #foreach($paramName in $allParams.header.keySet())\n #set($inputString = \"$inputString @@$paramName@@: @@$util.escapeJavaScript($allParams.header.get($paramName))@@\")\n #if($foreach.hasNext)\n #set($inputString = \"$inputString,\")\n #end\n #end\n #set($inputString = \"$inputString }\")\n \n #end\n\n #if ($includeQueryString)\n #set($inputString = \"$inputString, @@querystring@@:{\")\n #foreach($paramName in $allParams.querystring.keySet())\n #set($inputString = \"$inputString @@$paramName@@: @@$util.escapeJavaScript($allParams.querystring.get($paramName))@@\")\n #if($foreach.hasNext)\n #set($inputString = \"$inputString,\")\n #end\n #end\n #set($inputString = \"$inputString }\")\n #end\n\n #if ($includePath)\n #set($inputString = \"$inputString, @@path@@:{\")\n #foreach($paramName in $allParams.path.keySet())\n #set($inputString = \"$inputString @@$paramName@@: @@$util.escapeJavaScript($allParams.path.get($paramName))@@\")\n #if($foreach.hasNext)\n #set($inputString = \"$inputString,\")\n #end\n #end\n #set($inputString = \"$inputString }\")\n #end\n \n #if ($includeAuthorizer)\n #set($inputString = \"$inputString, @@authorizer@@:{\")\n #foreach($paramName in $context.authorizer.keySet())\n #set($inputString = \"$inputString @@$paramName@@: @@$util.escapeJavaScript($context.authorizer.get($paramName))@@\")\n #if($foreach.hasNext)\n #set($inputString = \"$inputString,\")\n #end\n #end\n #set($inputString = \"$inputString }\")\n #end\n\n #set($requestContext = \"\")\n ## Check if the request context should be included as part of the execution input\n #if($requestContext && !$requestContext.empty)\n #set($inputString = \"$inputString,\")\n #set($inputString = \"$inputString @@requestContext@@: $requestContext\")\n #end\n\n #set($inputString = \"$inputString}\")\n #set($inputString = $inputString.replaceAll(\"@@\",'\"'))\n #set($len = $inputString.length() - 1)\n \"input\": \"{$util.escapeJavaScript($inputString.substring(1,$len)).replaceAll(\"\\\\'\",\"'\")}\"\n}\n" + ] + ] + } + }, + "passthroughBehavior": "NEVER", + "integrationResponses": [ + { + "responseParameters": { + "method.response.header.Access-Control-Allow-Origin": "'*'" + }, + "statusCode": "200" + } + ], + "credentials": { + "Fn::GetAtt": [ + "myrestapiPOSTStartSyncExecutionRole7AFBE835", + "Arn" + ] + } + }, + "methodResponses": [ + { + "statusCode": "200", + "responseParameters": { + "method.response.header.Access-Control-Allow-Origin": true + } + }, + { + "statusCode": "200", + "responseModels": { + "application/json": "Empty" + } + }, + { + "statusCode": "400", + "responseModels": { + "application/json": "Error" + } + }, + { + "statusCode": "500", + "responseModels": { + "application/json": "Error" + } + } + ], + "resourceId": { + "Fn::GetAtt": [ + "myrestapiBAC2BF45", + "RootResourceId" + ] + }, + "restApiId": { + "Ref": "myrestapiBAC2BF45" + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" + } + }, + "passTask": { + "id": "passTask", + "path": "aws-cdk-aws-apigateway-stepfunctions-startexecution/passTask", + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" + } + }, + "StateMachine": { + "id": "StateMachine", + "path": "aws-cdk-aws-apigateway-stepfunctions-startexecution/StateMachine", + "children": { + "Role": { + "id": "Role", + "path": "aws-cdk-aws-apigateway-stepfunctions-startexecution/StateMachine/Role", + "children": { + "ImportRole": { + "id": "ImportRole", + "path": "aws-cdk-aws-apigateway-stepfunctions-startexecution/StateMachine/Role/ImportRole", + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" + } + }, + "Resource": { + "id": "Resource", + "path": "aws-cdk-aws-apigateway-stepfunctions-startexecution/StateMachine/Role/Resource", + "attributes": { + "aws:cdk:cloudformation:type": "AWS::IAM::Role", + "aws:cdk:cloudformation:props": { + "assumeRolePolicyDocument": { + "Statement": [ + { + "Action": "sts:AssumeRole", + "Effect": "Allow", + "Principal": { + "Service": "states.amazonaws.com" + } + } + ], + "Version": "2012-10-17" + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" + } + }, + "Resource": { + "id": "Resource", + "path": "aws-cdk-aws-apigateway-stepfunctions-startexecution/StateMachine/Resource", + "attributes": { + "aws:cdk:cloudformation:type": "AWS::StepFunctions::StateMachine", + "aws:cdk:cloudformation:props": { + "definitionString": "{\"StartAt\":\"passTask\",\"States\":{\"passTask\":{\"Type\":\"Pass\",\"InputPath\":\"$.somekey\",\"End\":true}}}", + "roleArn": { + "Fn::GetAtt": [ + "StateMachineRoleB840431D", + "Arn" + ] + }, + "stateMachineType": "EXPRESS" + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" + } + }, + "BootstrapVersion": { + "id": "BootstrapVersion", + "path": "aws-cdk-aws-apigateway-stepfunctions-startexecution/BootstrapVersion", + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" + } + }, + "CheckBootstrapVersion": { + "id": "CheckBootstrapVersion", + "path": "aws-cdk-aws-apigateway-stepfunctions-startexecution/CheckBootstrapVersion", + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" + } + }, + "aws-apigateway-stepfunctions-startexecution": { + "id": "aws-apigateway-stepfunctions-startexecution", + "path": "aws-apigateway-stepfunctions-startexecution", + "children": { + "DefaultTest": { + "id": "DefaultTest", + "path": "aws-apigateway-stepfunctions-startexecution/DefaultTest", + "children": { + "Default": { + "id": "Default", + "path": "aws-apigateway-stepfunctions-startexecution/DefaultTest/Default", + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" + } + }, + "DeployAssert": { + "id": "DeployAssert", + "path": "aws-apigateway-stepfunctions-startexecution/DefaultTest/DeployAssert", + "children": { + "BootstrapVersion": { + "id": "BootstrapVersion", + "path": "aws-apigateway-stepfunctions-startexecution/DefaultTest/DeployAssert/BootstrapVersion", + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" + } + }, + "CheckBootstrapVersion": { + "id": "CheckBootstrapVersion", + "path": "aws-apigateway-stepfunctions-startexecution/DefaultTest/DeployAssert/CheckBootstrapVersion", + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" + } + } + }, + "constructInfo": { + "fqn": "@aws-cdk/integ-tests-alpha.IntegTestCase", + "version": "0.0.0" + } + } + }, + "constructInfo": { + "fqn": "@aws-cdk/integ-tests-alpha.IntegTest", + "version": "0.0.0" + } + }, + "Tree": { + "id": "Tree", + "path": "Tree", + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.2.69" + } + } +} \ No newline at end of file diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-apigateway/test/integ.stepfunctions-startexecution.ts b/packages/@aws-cdk-testing/framework-integ/test/aws-apigateway/test/integ.stepfunctions-startexecution.ts new file mode 100644 index 0000000000000..86dbe8f9c0974 --- /dev/null +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-apigateway/test/integ.stepfunctions-startexecution.ts @@ -0,0 +1,51 @@ +import * as sfn from 'aws-cdk-lib/aws-stepfunctions'; +import * as cdk from 'aws-cdk-lib'; +import { IntegTest } from '@aws-cdk/integ-tests-alpha'; +import * as apigw from 'aws-cdk-lib/aws-apigateway'; + +const app = new cdk.App(); +const stack = new cdk.Stack(app, 'aws-cdk-aws-apigateway-stepfunctions-startexecution'); + +const api = new apigw.RestApi(stack, 'my-rest-api'); +const passTask = new sfn.Pass(stack, 'passTask', { + inputPath: '$.somekey', +}); + +const stateMachine: sfn.IStateMachine = new sfn.StateMachine(stack, 'StateMachine', { + definitionBody: sfn.DefinitionBody.fromChainable(passTask), + stateMachineType: sfn.StateMachineType.EXPRESS, +}); + +const methodOptions = { + methodResponses: [ + { + statusCode: '200', + responseParameters: { + 'method.response.header.Access-Control-Allow-Origin': true, + }, + }, + ], +}; + +const integrationOptions = { + integrationResponses: [ + { + responseParameters: { + 'method.response.header.Access-Control-Allow-Origin': "'*'", + }, + statusCode: '200', + }, + ], +}; + +const integ = apigw.StepFunctionsIntegration.startExecution(stateMachine, integrationOptions); +api.root.addMethod('GET', integ, methodOptions); +api.root.addMethod('POST', integ, methodOptions); + +new IntegTest(app, 'aws-apigateway-stepfunctions-startexecution', { + testCases: [ + stack, + ], +}); + +app.synth(); \ No newline at end of file diff --git a/packages/aws-cdk-lib/aws-apigateway/lib/integrations/stepfunctions.ts b/packages/aws-cdk-lib/aws-apigateway/lib/integrations/stepfunctions.ts index 4e75eb967afab..252322158d2b3 100644 --- a/packages/aws-cdk-lib/aws-apigateway/lib/integrations/stepfunctions.ts +++ b/packages/aws-cdk-lib/aws-apigateway/lib/integrations/stepfunctions.ts @@ -117,7 +117,7 @@ class StepFunctionsExecutionIntegration extends AwsIntegration { action: 'StartSyncExecution', options: { credentialsRole: options.credentialsRole, - integrationResponses: integrationResponse(), + integrationResponses: options.integrationResponses ?? integrationResponse(), passthroughBehavior: PassthroughBehavior.NEVER, requestTemplates: requestTemplates(stateMachine, options), ...options, diff --git a/packages/aws-cdk-lib/aws-apigateway/lib/method.ts b/packages/aws-cdk-lib/aws-apigateway/lib/method.ts index db93c7ffef611..5b4ca0428f86f 100644 --- a/packages/aws-cdk-lib/aws-apigateway/lib/method.ts +++ b/packages/aws-cdk-lib/aws-apigateway/lib/method.ts @@ -13,7 +13,7 @@ import { IStage } from './stage'; import { validateHttpMethod } from './util'; import * as cloudwatch from '../../aws-cloudwatch'; import * as iam from '../../aws-iam'; -import { ArnFormat, FeatureFlags, Lazy, Names, Resource, Stack } from '../../core'; +import { Annotations, ArnFormat, FeatureFlags, Lazy, Names, Resource, Stack } from '../../core'; import { APIGATEWAY_REQUEST_VALIDATOR_UNIQUE_ID } from '../../cx-api'; export interface MethodOptions { @@ -173,7 +173,7 @@ export class Method extends Resource { */ public readonly api: IRestApi; - private methodResponses: MethodResponse[]; + private readonly methodResponses: MethodResponse[] = []; constructor(scope: Construct, id: string, props: MethodProps) { super(scope, id); @@ -203,7 +203,9 @@ export class Method extends Resource { authorizer._attachToApi(this.api); } - this.methodResponses = options.methodResponses ?? defaultMethodOptions.methodResponses ?? []; + for (const mr of options.methodResponses ?? defaultMethodOptions.methodResponses ?? []) { + this.addMethodResponse(mr); + } const integration = props.integration ?? this.resource.defaultIntegration ?? new MockIntegration(); const bindResult = integration.bind(this); @@ -278,8 +280,16 @@ export class Method extends Resource { /** * Add a method response to this method + * + * You should only add one method reponse for every status code. The API allows it + * for historical reasons, but will add a warning if this happens. If you do, your Method + * will nondeterministically use one of the responses, and ignore the rest. */ public addMethodResponse(methodResponse: MethodResponse): void { + const mr = this.methodResponses.find((x) => x.statusCode === methodResponse.statusCode); + if (mr) { + Annotations.of(this).addWarning(`addMethodResponse called multiple times with statusCode=${methodResponse.statusCode}, deployment will be nondeterministic. Use a single addMethodResponse call to configure the entire response.`); + } this.methodResponses.push(methodResponse); } @@ -322,12 +332,8 @@ export class Method extends Resource { let responseModels: {[contentType: string]: string} | undefined; if (mr.responseModels) { - responseModels = {}; - for (const contentType in mr.responseModels) { - if (mr.responseModels.hasOwnProperty(contentType)) { - responseModels[contentType] = mr.responseModels[contentType].modelId; - } - } + responseModels = Object.fromEntries(Object.entries(mr.responseModels) + .map(([contentType, rm]) => [contentType, rm.modelId])); } const methodResponseProp = { @@ -506,4 +512,4 @@ export enum AuthorizationType { function pathForArn(path: string): string { return path.replace(/\{[^\}]*\}/g, '*'); // replace path parameters (like '{bookId}') with asterisk -} +} \ No newline at end of file diff --git a/packages/aws-cdk-lib/aws-apigateway/test/integrations/stepfunctions.test.ts b/packages/aws-cdk-lib/aws-apigateway/test/integrations/stepfunctions.test.ts index 0f2a5c65bcf9a..26b2a34662b09 100644 --- a/packages/aws-cdk-lib/aws-apigateway/test/integrations/stepfunctions.test.ts +++ b/packages/aws-cdk-lib/aws-apigateway/test/integrations/stepfunctions.test.ts @@ -372,6 +372,122 @@ describe('StepFunctionsIntegration', () => { .toThrow(/State Machine must be of type "EXPRESS". Please use StateMachineType.EXPRESS as the stateMachineType/); }); }); + + test('addMethod is not susceptible to false sharing of arrays', () => { + //GIVEN + const { stack, api, stateMachine } = givenSetup(); + + //WHEN + const methodOptions = { + methodResponses: [ + { + statusCode: '200', + responseParameters: { + 'method.response.header.Access-Control-Allow-Origin': true, + }, + }, + ], + }; + + const integrationOptions = { + integrationResponses: [ + { + responseParameters: { + 'method.response.header.Access-Control-Allow-Origin': "'*'", + }, + statusCode: '200', + }, + ], + }; + + const integ = apigw.StepFunctionsIntegration.startExecution(stateMachine, integrationOptions); + api.root.addMethod('GET', integ, methodOptions); + api.root.addMethod('POST', integ, methodOptions); + + // THEN - the MethodResponses arrays have 4 elements instead of 8 + // (This is still incorrect because 200 occurs multiple times, but that's a separate + // issue with a non-straightforward solution) + Template.fromStack(stack).resourceCountIs('AWS::ApiGateway::Method', 2); + Template.fromStack(stack).hasResourceProperties('AWS::ApiGateway::Method', { + HttpMethod: 'GET', + MethodResponses: [ + { + ResponseParameters: { + 'method.response.header.Access-Control-Allow-Origin': true, + }, + StatusCode: '200', + }, + { + ResponseModels: { + 'application/json': 'Empty', + }, + StatusCode: '200', + }, + { + ResponseModels: { + 'application/json': 'Error', + }, + StatusCode: '400', + }, + { + ResponseModels: { + 'application/json': 'Error', + }, + StatusCode: '500', + }, + ], + Integration: { + IntegrationResponses: [ + { + ResponseParameters: { + 'method.response.header.Access-Control-Allow-Origin': "'*'", + }, + StatusCode: '200', + }, + ], + }, + }); + + Template.fromStack(stack).hasResourceProperties('AWS::ApiGateway::Method', { + HttpMethod: 'POST', + MethodResponses: [ + { + ResponseParameters: { + 'method.response.header.Access-Control-Allow-Origin': true, + }, + StatusCode: '200', + }, + { + ResponseModels: { + 'application/json': 'Empty', + }, + StatusCode: '200', + }, + { + ResponseModels: { + 'application/json': 'Error', + }, + StatusCode: '400', + }, + { + ResponseModels: { + 'application/json': 'Error', + }, + StatusCode: '500', + }, + ], + Integration: { + IntegrationResponses: [ + { + ResponseParameters: { + 'method.response.header.Access-Control-Allow-Origin': "'*'", + }, + StatusCode: '200', + }, + ], + }, + }); + }); }); function givenSetup() { From e264a2f2c95e57e38d77c5fedad4aa06b2ec9ead Mon Sep 17 00:00:00 2001 From: "k.goto" <24818752+go-to-k@users.noreply.github.com> Date: Thu, 17 Aug 2023 21:18:54 +0900 Subject: [PATCH 4/4] fix(ecr): autoDeleteImages fails when repository is renamed (#26742) This PR fixes the bug that ECRAutoDeleteImages fails on repo rename. The customResource depends on the role, and when the repository name changes, the role is updated to match the new repository instead of the old one, before customResource runs and the old repository is deleted. It was difficult to delete the old repo before the role update ran, so I changed the resource of the role to a wildcard. Closes #26711. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../aws-ecr-integ-stack.assets.json | 4 +- .../aws-ecr-integ-stack.template.json | 27 ++++++-- .../manifest.json | 2 +- .../tree.json | 64 +++++++++---------- ...-resourcesmax-ACCOUNT-REGION.template.json | 33 +++++++--- .../aws-cdk-lib/aws-ecr/lib/repository.ts | 15 ++--- .../aws-ecr/test/repository.test.ts | 24 ++++--- 7 files changed, 102 insertions(+), 67 deletions(-) diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-ecr/test/integ.repository-auto-delete-images.js.snapshot/aws-ecr-integ-stack.assets.json b/packages/@aws-cdk-testing/framework-integ/test/aws-ecr/test/integ.repository-auto-delete-images.js.snapshot/aws-ecr-integ-stack.assets.json index 2ed59aaf47afa..e63f36bac2b52 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-ecr/test/integ.repository-auto-delete-images.js.snapshot/aws-ecr-integ-stack.assets.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-ecr/test/integ.repository-auto-delete-images.js.snapshot/aws-ecr-integ-stack.assets.json @@ -14,7 +14,7 @@ } } }, - "32b456cdb8ff646c98c4580ff6d88bd51e84e33af74af927bb882158a53a0d21": { + "adb4722adf5406a51ec51892647c61e7ac61ba38b845cd1163397018822e542e": { "source": { "path": "aws-ecr-integ-stack.template.json", "packaging": "file" @@ -22,7 +22,7 @@ "destinations": { "current_account-current_region": { "bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}", - "objectKey": "32b456cdb8ff646c98c4580ff6d88bd51e84e33af74af927bb882158a53a0d21.json", + "objectKey": "adb4722adf5406a51ec51892647c61e7ac61ba38b845cd1163397018822e542e.json", "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}" } } diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-ecr/test/integ.repository-auto-delete-images.js.snapshot/aws-ecr-integ-stack.template.json b/packages/@aws-cdk-testing/framework-integ/test/aws-ecr/test/integ.repository-auto-delete-images.js.snapshot/aws-ecr-integ-stack.template.json index f4a8953a85a42..b9c3c667b0ed7 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-ecr/test/integ.repository-auto-delete-images.js.snapshot/aws-ecr-integ-stack.template.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-ecr/test/integ.repository-auto-delete-images.js.snapshot/aws-ecr-integ-stack.template.json @@ -69,12 +69,31 @@ ], "Resource": [ { - "Fn::GetAtt": [ - "Repo02AC86CF", - "Arn" + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":ecr:", + { + "Ref": "AWS::Region" + }, + ":", + { + "Ref": "AWS::AccountId" + }, + ":repository/*" + ] ] } - ] + ], + "Condition": { + "StringEquals": { + "ecr:ResourceTag/aws-cdk:auto-delete-images": "true" + } + } } ] } diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-ecr/test/integ.repository-auto-delete-images.js.snapshot/manifest.json b/packages/@aws-cdk-testing/framework-integ/test/aws-ecr/test/integ.repository-auto-delete-images.js.snapshot/manifest.json index 646bcf69283fc..549c403bbf383 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-ecr/test/integ.repository-auto-delete-images.js.snapshot/manifest.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-ecr/test/integ.repository-auto-delete-images.js.snapshot/manifest.json @@ -17,7 +17,7 @@ "validateOnSynth": false, "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}", "cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}", - "stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/32b456cdb8ff646c98c4580ff6d88bd51e84e33af74af927bb882158a53a0d21.json", + "stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/adb4722adf5406a51ec51892647c61e7ac61ba38b845cd1163397018822e542e.json", "requiresBootstrapStackVersion": 6, "bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version", "additionalDependencies": [ diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-ecr/test/integ.repository-auto-delete-images.js.snapshot/tree.json b/packages/@aws-cdk-testing/framework-integ/test/aws-ecr/test/integ.repository-auto-delete-images.js.snapshot/tree.json index 694514e724fea..99ef3b4059be5 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-ecr/test/integ.repository-auto-delete-images.js.snapshot/tree.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-ecr/test/integ.repository-auto-delete-images.js.snapshot/tree.json @@ -28,8 +28,8 @@ } }, "constructInfo": { - "fqn": "aws-cdk-lib.aws_ecr.CfnRepository", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.69" } }, "AutoDeleteImagesCustomResource": { @@ -40,20 +40,20 @@ "id": "Default", "path": "aws-ecr-integ-stack/Repo/AutoDeleteImagesCustomResource/Default", "constructInfo": { - "fqn": "aws-cdk-lib.CfnResource", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.69" } } }, "constructInfo": { - "fqn": "aws-cdk-lib.CustomResource", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.69" } } }, "constructInfo": { - "fqn": "aws-cdk-lib.aws_ecr.Repository", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.69" } }, "Custom::ECRAutoDeleteImagesCustomResourceProvider": { @@ -64,60 +64,60 @@ "id": "Staging", "path": "aws-ecr-integ-stack/Custom::ECRAutoDeleteImagesCustomResourceProvider/Staging", "constructInfo": { - "fqn": "aws-cdk-lib.AssetStaging", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.69" } }, "Role": { "id": "Role", "path": "aws-ecr-integ-stack/Custom::ECRAutoDeleteImagesCustomResourceProvider/Role", "constructInfo": { - "fqn": "aws-cdk-lib.CfnResource", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.69" } }, "Handler": { "id": "Handler", "path": "aws-ecr-integ-stack/Custom::ECRAutoDeleteImagesCustomResourceProvider/Handler", "constructInfo": { - "fqn": "aws-cdk-lib.CfnResource", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.69" } } }, "constructInfo": { - "fqn": "aws-cdk-lib.CustomResourceProvider", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.69" } }, "RepositoryURI": { "id": "RepositoryURI", "path": "aws-ecr-integ-stack/RepositoryURI", "constructInfo": { - "fqn": "aws-cdk-lib.CfnOutput", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.69" } }, "BootstrapVersion": { "id": "BootstrapVersion", "path": "aws-ecr-integ-stack/BootstrapVersion", "constructInfo": { - "fqn": "aws-cdk-lib.CfnParameter", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.69" } }, "CheckBootstrapVersion": { "id": "CheckBootstrapVersion", "path": "aws-ecr-integ-stack/CheckBootstrapVersion", "constructInfo": { - "fqn": "aws-cdk-lib.CfnRule", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.69" } } }, "constructInfo": { - "fqn": "aws-cdk-lib.Stack", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.69" } }, "cdk-integ-auto-delete-images": { @@ -144,22 +144,22 @@ "id": "BootstrapVersion", "path": "cdk-integ-auto-delete-images/DefaultTest/DeployAssert/BootstrapVersion", "constructInfo": { - "fqn": "aws-cdk-lib.CfnParameter", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.69" } }, "CheckBootstrapVersion": { "id": "CheckBootstrapVersion", "path": "cdk-integ-auto-delete-images/DefaultTest/DeployAssert/CheckBootstrapVersion", "constructInfo": { - "fqn": "aws-cdk-lib.CfnRule", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.69" } } }, "constructInfo": { - "fqn": "aws-cdk-lib.Stack", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.69" } } }, @@ -184,8 +184,8 @@ } }, "constructInfo": { - "fqn": "aws-cdk-lib.App", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.2.69" } } } \ No newline at end of file diff --git a/packages/@aws-cdk/app-staging-synthesizer-alpha/test/integ.synth-default-resources.js.snapshot/StagingStack-default-resourcesmax-ACCOUNT-REGION.template.json b/packages/@aws-cdk/app-staging-synthesizer-alpha/test/integ.synth-default-resources.js.snapshot/StagingStack-default-resourcesmax-ACCOUNT-REGION.template.json index bb7b4bebdd950..e414b1a846fa0 100644 --- a/packages/@aws-cdk/app-staging-synthesizer-alpha/test/integ.synth-default-resources.js.snapshot/StagingStack-default-resourcesmax-ACCOUNT-REGION.template.json +++ b/packages/@aws-cdk/app-staging-synthesizer-alpha/test/integ.synth-default-resources.js.snapshot/StagingStack-default-resourcesmax-ACCOUNT-REGION.template.json @@ -636,18 +636,31 @@ ], "Resource": [ { - "Fn::GetAtt": [ - "defaultresourcesmaxecrasset13112F7F9", - "Arn" - ] - }, - { - "Fn::GetAtt": [ - "defaultresourcesmaxecrasset2904B88A7", - "Arn" + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":ecr:", + { + "Ref": "AWS::Region" + }, + ":", + { + "Ref": "AWS::AccountId" + }, + ":repository/*" + ] ] } - ] + ], + "Condition": { + "StringEquals": { + "ecr:ResourceTag/aws-cdk:auto-delete-images": "true" + } + } } ] } diff --git a/packages/aws-cdk-lib/aws-ecr/lib/repository.ts b/packages/aws-cdk-lib/aws-ecr/lib/repository.ts index cefd1407f5851..41b2d452acd81 100644 --- a/packages/aws-cdk-lib/aws-ecr/lib/repository.ts +++ b/packages/aws-cdk-lib/aws-ecr/lib/repository.ts @@ -20,11 +20,11 @@ import { CustomResource, CustomResourceProvider, CustomResourceProviderRuntime, + Aws, } from '../../core'; const AUTO_DELETE_IMAGES_RESOURCE_TYPE = 'Custom::ECRAutoDeleteImages'; const AUTO_DELETE_IMAGES_TAG = 'aws-cdk:auto-delete-images'; -const REPO_ARN_SYMBOL = Symbol.for('@aws-cdk/aws-ecr.RepoArns'); /** * Represents an ECR repository. @@ -867,12 +867,8 @@ export class Repository extends RepositoryBase { }); if (firstTime) { - const repoArns = [this._resource.attrArn]; - (provider as any)[REPO_ARN_SYMBOL] = repoArns; - // Use a iam policy to allow the custom resource to list & delete // images in the repository and the ability to get all repositories to find the arn needed on delete. - // We lazily produce a list of repositories associated with this custom resource provider. provider.addToRolePolicy({ Effect: 'Allow', Action: [ @@ -881,10 +877,13 @@ export class Repository extends RepositoryBase { 'ecr:ListImages', 'ecr:ListTagsForResource', ], - Resource: Lazy.list({ produce: () => repoArns }), + Resource: [`arn:${Aws.PARTITION}:ecr:${Stack.of(this).region}:${Stack.of(this).account}:repository/*`], + Condition: { + StringEquals: { + ['ecr:ResourceTag/' + AUTO_DELETE_IMAGES_TAG]: 'true', + }, + }, }); - } else { - (provider as any)[REPO_ARN_SYMBOL].push(this._resource.attrArn); } const customResource = new CustomResource(this, 'AutoDeleteImagesCustomResource', { diff --git a/packages/aws-cdk-lib/aws-ecr/test/repository.test.ts b/packages/aws-cdk-lib/aws-ecr/test/repository.test.ts index 6b58bbb3e2d88..38e7bdebac650 100644 --- a/packages/aws-cdk-lib/aws-ecr/test/repository.test.ts +++ b/packages/aws-cdk-lib/aws-ecr/test/repository.test.ts @@ -1006,18 +1006,22 @@ describe('repository', () => { ], Resource: [ { - 'Fn::GetAtt': [ - 'Repo1DBD717D9', - 'Arn', - ], - }, - { - 'Fn::GetAtt': [ - 'Repo2730A8200', - 'Arn', - ], + 'Fn::Join': ['', [ + 'arn:', + { Ref: 'AWS::Partition' }, + ':ecr:', + { Ref: 'AWS::Region' }, + ':', + { Ref: 'AWS::AccountId' }, + ':repository/*', + ]], }, ], + Condition: { + StringEquals: { + 'ecr:ResourceTag/aws-cdk:auto-delete-images': 'true', + }, + }, }, ], },