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: {