diff --git a/packages/aws-cdk/lib/api/hotswap/common.ts b/packages/aws-cdk/lib/api/hotswap/common.ts index 5bbb69535faf1..d72d7b270fc85 100644 --- a/packages/aws-cdk/lib/api/hotswap/common.ts +++ b/packages/aws-cdk/lib/api/hotswap/common.ts @@ -1,6 +1,5 @@ import * as cfn_diff from '@aws-cdk/cloudformation-diff'; import { ISDK } from '../aws-auth'; -import { CfnEvaluationException, EvaluateCloudFormationTemplate } from '../evaluate-cloudformation-template'; export const ICON = '✨'; @@ -136,13 +135,6 @@ 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 { @@ -221,207 +213,3 @@ 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 42aa610795b4f..ad62950fc861b 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, upperCaseFirstCharacter, applyPropertyUpdates, evaluatableProperties } from './common'; +import { ChangeHotswapResult, classifyChanges, HotswappableChangeCandidate, lowerCaseFirstCharacter, reportNonHotswappableChange, transformObjectKeys } from './common'; import { ISDK } from '../aws-auth'; import { EvaluateCloudFormationTemplate } from '../evaluate-cloudformation-template'; @@ -16,8 +16,7 @@ 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 propertiesToHotswap = ['ContainerDefinitions']; - const classifiedChanges = classifyChanges(change, propertiesToHotswap); + const classifiedChanges = classifyChanges(change, ['ContainerDefinitions']); classifiedChanges.reportNonHotswappablePropertyChanges(ret); // find all ECS Services that reference the TaskDefinition that changed @@ -34,8 +33,7 @@ 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'); @@ -46,32 +44,14 @@ export async function isHotswappableEcsServiceChange( const namesOfHotswappableChanges = Object.keys(classifiedChanges.hotswappableProps); if (namesOfHotswappableChanges.length > 0) { - 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; - } - + const taskDefinitionResource = await prepareTaskDefinitionChange(evaluateCfnTemplate, logicalId, change); ret.push({ hotswappable: true, resourceType: change.newValue.Type, propsChanged: namesOfHotswappableChanges, service: 'ecs-service', resourceNames: [ - `ECS Task Definition '${familyName}'`, + `ECS Task Definition '${await taskDefinitionResource.Family}'`, ...ecsServicesReferencingTaskDef.map(ecsService => `ECS Service '${ecsService.serviceArn.split('/')[2]}'`), ], apply: async (sdk: ISDK) => { @@ -79,43 +59,11 @@ export async function isHotswappableEcsServiceChange( // we need to lowercase the evaluated TaskDef from CloudFormation, // as the AWS SDK uses lowercase property names for these - // 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 = { + // 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 ContainerDefinitions: { DockerLabels: true, FirelensConfiguration: { @@ -131,15 +79,7 @@ export async function isHotswappableEcsServiceChange( Labels: true, }, }, - } as const; - const excludeFromTransformLowercased = transformObjectKeys(excludeFromTransform, lowerCaseFirstCharacter); - // We first uppercase the task definition to properly merge it with the one from CloudFormation template. - const upperCasedTaskDef = transformObjectKeys(target.taskDefinition, upperCaseFirstCharacter, excludeFromTransformLowercased); - // 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; @@ -231,15 +171,14 @@ interface EcsService { readonly serviceArn: string; } -async function getFamilyName( - evaluateCfnTemplate: EvaluateCloudFormationTemplate, - logicalId: string, - change: HotswappableChangeCandidate) { +async function prepareTaskDefinitionChange( + 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, @@ -250,12 +189,17 @@ async function getFamilyName( // 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 family; + // 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, + }; } 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 2d41d1b13ba58..e0267635a59d8 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,22 +1,19 @@ /* eslint-disable import/order */ import * as AWS from 'aws-sdk'; import * as setup from './hotswap-test-setup'; -import { HotswapMode, lowerCaseFirstCharacter, transformObjectKeys } from '../../../lib/api/hotswap/common'; +import { HotswapMode } 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 @@ -33,71 +30,36 @@ beforeEach(() => { }); }); -function setupCurrentTaskDefinition(props: {taskDefinitionProperties: any, includeService: boolean, otherResources?: any}) { - setup.setCurrentCfnStackTemplate({ - Resources: { - TaskDef: { - Type: 'AWS::ECS::TaskDefinition', - Properties: props.taskDefinitionProperties, - }, - ...(props.includeService ? { +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' }, + ], + }, + }, 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'), ); - } - 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' }, - ], + mockRegisterTaskDef.mockReturnValue({ + taskDefinition: { + taskDefinitionArn: 'arn:aws:ecs:region:account:task-definition/my-task-def:3', }, - includeService: true, }); const cdkStackArtifact = setup.cdkStackArtifactOf({ template: { @@ -132,10 +94,6 @@ 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', @@ -149,15 +107,34 @@ 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 - setupCurrentTaskDefinition({ - taskDefinitionProperties: { - Family: 'my-task-def', - ContainerDefinitions: [ - { Image: 'image1' }, - ], - Cpu: '256', + 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', }, - includeService: true, }); const cdkStackArtifact = setup.cdkStackArtifactOf({ template: { @@ -189,7 +166,6 @@ 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 @@ -204,10 +180,6 @@ 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', @@ -222,15 +194,34 @@ 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 - setupCurrentTaskDefinition({ - taskDefinitionProperties: { - Family: 'my-task-def', - ContainerDefinitions: [ - { Image: 'image1' }, - ], - Cpu: '256', + 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', }, - includeService: true, }); const cdkStackArtifact = setup.cdkStackArtifactOf({ template: { @@ -261,7 +252,6 @@ 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 @@ -269,10 +259,6 @@ 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: [ @@ -294,23 +280,33 @@ 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 - setupCurrentTaskDefinition({ - taskDefinitionProperties: { - ContainerDefinitions: [ - { Image: 'image1' }, - ], + setup.setCurrentCfnStackTemplate({ + Resources: { + TaskDef: { + Type: 'AWS::ECS::TaskDefinition', + Properties: { + ContainerDefinitions: [ + { Image: 'image1' }, + ], + }, + }, + Service: { + Type: 'AWS::ECS::Service', + Properties: { + TaskDefinition: { Ref: 'TaskDef' }, + }, + }, }, - includeService: true, }); - mockDescribeTaskDef.mockReturnValue({ + 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({ taskDefinition: { - taskDefinitionArn: 'arn:aws:ecs:region:account:task-definition/my-task-def:2', - family: 'my-task-def', - containerDefinitions: [ - { - image: 'image1', - }, - ], + taskDefinitionArn: 'arn:aws:ecs:region:account:task-definition/my-task-def:3', }, }); const cdkStackArtifact = setup.cdkStackArtifactOf({ @@ -339,10 +335,6 @@ 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: [ @@ -362,14 +354,26 @@ 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 - setupCurrentTaskDefinition({ - taskDefinitionProperties: { - Family: 'my-task-def', - ContainerDefinitions: [ - { Image: 'image1' }, - ], + 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', }, - includeService: false, }); const cdkStackArtifact = setup.cdkStackArtifactOf({ template: { @@ -392,7 +396,6 @@ 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) { @@ -401,10 +404,6 @@ 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: [ @@ -418,15 +417,23 @@ 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 - setupCurrentTaskDefinition({ - taskDefinitionProperties: { - Family: 'my-task-def', - ContainerDefinitions: [ - { Image: 'image1' }, - ], - }, - includeService: true, - otherResources: { + 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' }, + }, + }, Function: { Type: 'AWS::Lambda::Function', Properties: { @@ -439,6 +446,15 @@ 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: { @@ -477,7 +493,6 @@ 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) { @@ -486,10 +501,6 @@ 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: [ @@ -508,337 +519,43 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot } }); - test('should call registerTaskDefinition, describeTaskDefinition, and updateService for a difference only in the container image but with environment variables of unsupported intrinsics', async () => { + test('should call registerTaskDefinition with certain properties not lowercased', async () => { // GIVEN - setupCurrentTaskDefinition({ - taskDefinitionProperties: { - Family: 'my-task-def', - ContainerDefinitions: [ - { - Image: 'image1', - Environment: [ - { - Name: 'FOO', - Value: { 'Fn::GetAtt': ['Bar', 'Baz'] }, - }, + setup.setCurrentCfnStackTemplate({ + Resources: { + TaskDef: { + Type: 'AWS::ECS::TaskDefinition', + Properties: { + Family: 'my-task-def', + ContainerDefinitions: [ + { Image: 'image1' }, ], - }, - ], - }, - 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: [ + Volumes: [ { - 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'] }, - }, - ], + DockerVolumeConfiguration: { + DriverOpts: { Option1: 'option1' }, + Labels: { Label1: 'label1' }, }, - ], - }, - }, - 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' }, - }, - }, }, - }, - }); - - // 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: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' }, - }, + 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 nor uppercased', async () => { - // GIVEN - setupCurrentTaskDefinition({ - taskDefinitionProperties: { - Family: 'my-task-def', - ContainerDefinitions: [ - { - Image: 'image1', - DockerLabels: { Label1: 'label1' }, - FirelensConfiguration: { - Options: { Name: 'cloudwatch' }, - }, - LogConfiguration: { - Options: { Option1: 'option1', option2: 'option2' }, - }, - }, - ], - Volumes: [ - { - DockerVolumeConfiguration: { - DriverOpts: { Option1: 'option1' }, - Labels: { Label1: 'label1' }, - }, - }, - ], + 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', }, - includeService: true, }); const cdkStackArtifact = setup.cdkStackArtifactOf({ template: { @@ -855,7 +572,7 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot Options: { Name: 'cloudwatch' }, }, LogConfiguration: { - Options: { Option1: 'option1', option2: 'option2' }, + Options: { Option1: 'option1' }, }, }, ], @@ -896,7 +613,7 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot }, }, logConfiguration: { - options: { Option1: 'option1', option2: 'option2' }, + options: { Option1: 'option1' }, }, }, ],