Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

revert: "fix(cli): cannot hotswap ECS task definitions containing certain intrinsics" #27358

Merged
merged 4 commits into from
Oct 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
212 changes: 0 additions & 212 deletions packages/aws-cdk/lib/api/hotswap/common.ts
Original file line number Diff line number Diff line change
@@ -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 = '✨';

Expand Down Expand Up @@ -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<string, cfn_diff.PropertyDifference<any>>;

export class ClassifiedChanges {
Expand Down Expand Up @@ -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<EvaluatedPropertyUpdates> {
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;
}
Loading
Loading