Skip to content

Commit

Permalink
more tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@bergjaak
bergjaak committed May 19, 2024
1 parent f8593d4 commit 05956d6
Show file tree
Hide file tree
Showing 5 changed files with 726 additions and 71 deletions.
9 changes: 2 additions & 7 deletions packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts
View file
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
// The SDK is only used to reference `DescribeChangeSetOutput`, so the SDK is added as a devDependency.
// The SDK should not make network calls here
import type { DescribeChangeSetOutput as DescribeChangeSet } from '@aws-sdk/client-cloudformation';
import type { DescribeChangeSetOutput } from '@aws-sdk/client-cloudformation';
import * as impl from './diff';
import { TemplateAndChangeSetDiffMerger } from './diff/template-and-changeset-diff-merger';
import * as types from './diff/types';
import { deepEqual, diffKeyedEntities, unionOf } from './diff/util';

export * from './diff/types';

export type DescribeChangeSetOutput = DescribeChangeSet;

type DiffHandler = (diff: types.ITemplateDiff, oldValue: any, newValue: any) => void;
type HandlerRegistry = { [section: string]: DiffHandler };

Expand Down Expand Up @@ -56,10 +54,7 @@ export function fullDiff(
normalize(newTemplate);
let theDiff = diffTemplate(currentTemplate, newTemplate);
if (changeSet) {
const changeSetDiff = new TemplateAndChangeSetDiffMerger({
changeSet: changeSet,
currentTemplateResources: currentTemplate.Resources,
});
const changeSetDiff = new TemplateAndChangeSetDiffMerger({ changeSet: changeSet });
changeSetDiff.addChangeSetResourcesToDiff(theDiff.resources);
changeSetDiff.addImportInformation(theDiff.resources);
theDiff = new types.TemplateDiff(theDiff); // do this to propagate security changes.
Expand Down
131 changes: 73 additions & 58 deletions packages/@aws-cdk/cloudformation-diff/lib/diff/template-and-changeset-diff-merger.ts
View file
Original file line number Diff line number Diff line change
@@ -1,35 +1,74 @@
// The SDK is only used to reference `DescribeChangeSetOutput`, so the SDK is added as a devDependency.
// The SDK should not make network calls here
import type { DescribeChangeSetOutput as DescribeChangeSet, ResourceChangeDetail } from '@aws-sdk/client-cloudformation';
import type { DescribeChangeSetOutput, ResourceChangeDetail } from '@aws-sdk/client-cloudformation';
import { diffResource } from '.';
import * as types from '../diff/types';

export type DescribeChangeSetOutput = DescribeChangeSet;

/**
* The purpose of this class is to include differences from the ChangeSet to differences in the TemplateDiff.
*/
export class TemplateAndChangeSetDiffMerger {
// If we somehow cannot find the resourceType, then we'll mark it as UNKNOWN, so that can be seen in the diff.
private UNKNOWN_RESOURCE_TYPE = 'UNKNOWN';
static UNKNOWN_RESOURCE_TYPE = 'UNKNOWN';

changeSet: DescribeChangeSetOutput;
currentTemplateResources: {[logicalId: string]: any};
changeSetResources: types.ChangeSetResources;
/**
* @param parseOldOrNewValues These enum values come from DescribeChangeSetOutput, which indicate if the property resolves to that value after the change or before the change.
*/
static convertResourceFromChangesetToResourceForDiff(
resourceInfoFromChangeset: types.ChangeSetResource,
parseOldOrNewValues: 'BEFORE_VALUES' | 'AFTER_VALUES',
): types.Resource {
const props: { [logicalId: string]: string | undefined } = {};
if (parseOldOrNewValues === 'AFTER_VALUES') {
for (const [propertyName, value] of Object.entries(resourceInfoFromChangeset.properties ?? {})) {
props[propertyName] = value.afterValue;
}
} else {
for (const [propertyName, value] of Object.entries(resourceInfoFromChangeset.properties ?? {})) {
props[propertyName] = value.beforeValue;
}
}

return {
Type: resourceInfoFromChangeset.resourceType ?? TemplateAndChangeSetDiffMerger.UNKNOWN_RESOURCE_TYPE,
Properties: props,
};
}

static determineChangeSetReplacementMode(propertyChange: ResourceChangeDetail): types.ChangeSetReplacementMode {
if (propertyChange.Target?.RequiresRecreation === undefined) {
// We can't determine if the resource will be replaced or not. That's what conditionally means.
return 'Conditionally';
}

if (propertyChange.Target.RequiresRecreation === 'Always') {
switch (propertyChange.Evaluation) {
case 'Static':
return 'Always';
case 'Dynamic':
// If Evaluation is 'Dynamic', then this may cause replacement, or it may not.
// see 'Replacement': https://docs.aws.amazon.com/AWSCloudFormation/latest/APIReference/API_ResourceChange.html
return 'Conditionally';
}
}

return propertyChange.Target.RequiresRecreation as types.ChangeSetReplacementMode;
}

changeSet: DescribeChangeSetOutput | undefined;
changeSetResources: types.ChangeSetResources | undefined;

constructor(
args: {
changeSet: DescribeChangeSetOutput;
currentTemplateResources: {[logicalId: string]: any};
},
) {
this.changeSet = args.changeSet;
this.currentTemplateResources = args.currentTemplateResources;
this.changeSetResources = this.inspectChangeSet(this.changeSet);
}

inspectChangeSet(changeSet: DescribeChangeSetOutput): types.ChangeSetResources {
const replacements: types.ChangeSetResources = {};
const changeSetResources: types.ChangeSetResources = {};
for (const resourceChange of changeSet.Changes ?? []) {
if (resourceChange.ResourceChange?.LogicalResourceId === undefined) {
continue;
Expand All @@ -39,36 +78,33 @@ export class TemplateAndChangeSetDiffMerger {
for (const propertyChange of resourceChange.ResourceChange.Details ?? []) {
if (propertyChange.Target?.Attribute === 'Properties' && propertyChange.Target.Name) {
propertiesReplaced[propertyChange.Target.Name] = {
changeSetReplacementMode: this.determineChangeSetReplacementMode(propertyChange),
beforeValue: propertyChange.Target.BeforeValue,
afterValue: propertyChange.Target.AfterValue,
changeSetReplacementMode: TemplateAndChangeSetDiffMerger.determineChangeSetReplacementMode(propertyChange),
beforeValue: _maybeJsonParse(propertyChange.Target.BeforeValue),
afterValue: _maybeJsonParse(propertyChange.Target.AfterValue),
};
}
}

replacements[resourceChange.ResourceChange.LogicalResourceId] = {
changeSetResources[resourceChange.ResourceChange.LogicalResourceId] = {
resourceWasReplaced: resourceChange.ResourceChange.Replacement === 'True',
resourceType: resourceChange.ResourceChange.ResourceType ?? this.UNKNOWN_RESOURCE_TYPE, // DescribeChanegSet doesn't promise to have the ResourceType...
resourceType: resourceChange.ResourceChange.ResourceType ?? TemplateAndChangeSetDiffMerger.UNKNOWN_RESOURCE_TYPE, // DescribeChanegSet doesn't promise to have the ResourceType...
properties: propertiesReplaced,
};
}

return replacements;
}

determineChangeSetReplacementMode(propertyChange: ResourceChangeDetail): types.ChangeSetReplacementMode {
if (propertyChange.Target!.RequiresRecreation === 'Always') {
switch (propertyChange.Evaluation) {
case 'Static':
return 'Always';
case 'Dynamic':
// If Evaluation is 'Dynamic', then this may cause replacement, or it may not.
// see 'Replacement': https://docs.aws.amazon.com/AWSCloudFormation/latest/APIReference/API_ResourceChange.html
return 'Conditionally';
}
return changeSetResources;

/**
* we will try to parse the afterValue so that downstream processing of the diff can access object properties.
* However, there's not a guarantee that it will work, since clouformation will truncate the afterValue and BeforeValue if they're too long.
*/
function _maybeJsonParse(value: string | undefined): any | undefined {
let result = value;
try {
result = JSON.parse(value ?? '');
} catch (e) {}
return result;
}

return propertyChange.Target!.RequiresRecreation as types.ChangeSetReplacementMode;
}

/**
Expand All @@ -79,18 +115,18 @@ export class TemplateAndChangeSetDiffMerger {
* - Another case is when a resource is changed because the resource is defined by an SSM parameter, and the value of that SSM parameter changes.
*/
addChangeSetResourcesToDiff(resourceDiffs: types.DifferenceCollection<types.Resource, types.ResourceDifference>) {
for (const [logicalId, changeSetResource] of Object.entries(this.changeSetResources)) {
for (const [logicalId, changeSetResource] of Object.entries(this.changeSetResources ?? {})) {
const resourceNotFoundInTemplateDiff = !(resourceDiffs.logicalIds.includes(logicalId));
if (resourceNotFoundInTemplateDiff) {
const resourceDiffFromChangeset = diffResource(
this.convertResourceFromChangesetToResourceForDiff(changeSetResource, 'OLD_VALUES'),
this.convertResourceFromChangesetToResourceForDiff(changeSetResource, 'NEW_VALUES'),
TemplateAndChangeSetDiffMerger.convertResourceFromChangesetToResourceForDiff(changeSetResource, 'BEFORE_VALUES'),
TemplateAndChangeSetDiffMerger.convertResourceFromChangesetToResourceForDiff(changeSetResource, 'AFTER_VALUES'),
);
resourceDiffs.set(logicalId, resourceDiffFromChangeset);
}

const propertyChangesFromTemplate = resourceDiffs.get(logicalId).propertyUpdates;
for (const propertyName of Object.keys(this.changeSetResources[logicalId]?.properties ?? {})) {
for (const propertyName of Object.keys((this.changeSetResources ?? {})[logicalId]?.properties ?? {})) {
if (propertyName in propertyChangesFromTemplate) {
// If the property is already marked to be updated, then we don't need to do anything.
continue;
Expand All @@ -106,27 +142,6 @@ export class TemplateAndChangeSetDiffMerger {
this.enhanceChangeImpacts(resourceDiffs);
}

convertResourceFromChangesetToResourceForDiff(
resourceInfoFromChangeset: types.ChangeSetResource,
parseOldOrNewValues: 'OLD_VALUES' | 'NEW_VALUES',
): types.Resource {
const props: { [logicalId: string]: string | undefined } = {};
if (parseOldOrNewValues === 'NEW_VALUES') {
for (const [propertyName, value] of Object.entries(resourceInfoFromChangeset.properties ?? {})) {
props[propertyName] = value.afterValue;
}
} else {
for (const [propertyName, value] of Object.entries(resourceInfoFromChangeset.properties ?? {})) {
props[propertyName] = value.beforeValue;
}
}

return {
Type: resourceInfoFromChangeset.resourceType ?? this.UNKNOWN_RESOURCE_TYPE,
Properties: props,
};
}

enhanceChangeImpacts(resourceDiffs: types.DifferenceCollection<types.Resource, types.ResourceDifference>) {
resourceDiffs.forEachDifference((logicalId: string, change: types.ResourceDifference) => {
if ((!change.resourceTypeChanged) && change.resourceType?.includes('AWS::Serverless')) {
Expand All @@ -135,13 +150,13 @@ export class TemplateAndChangeSetDiffMerger {
}
change.forEachDifference((type: 'Property' | 'Other', name: string, value: types.Difference<any> | types.PropertyDifference<any>) => {
if (type === 'Property') {
if (!this.changeSetResources[logicalId]) {
if (!(this.changeSetResources ?? {})[logicalId]) {
(value as types.PropertyDifference<any>).changeImpact = types.ResourceImpact.NO_CHANGE;
(value as types.PropertyDifference<any>).isDifferent = false;
return;
}

const changeSetReplacementMode = (this.changeSetResources[logicalId]?.properties ?? {})[name]?.changeSetReplacementMode;
const changeSetReplacementMode = ((this.changeSetResources ?? {})[logicalId]?.properties ?? {})[name]?.changeSetReplacementMode;
switch (changeSetReplacementMode) {
case 'Always':
(value as types.PropertyDifference<any>).changeImpact = types.ResourceImpact.WILL_REPLACE;
Expand Down Expand Up @@ -180,7 +195,7 @@ export class TemplateAndChangeSetDiffMerger {

findResourceImports(): string[] {
const importedResourceLogicalIds = [];
for (const resourceChange of this.changeSet.Changes ?? []) {
for (const resourceChange of this.changeSet?.Changes ?? []) {
if (resourceChange.ResourceChange?.Action === 'Import') {
importedResourceLogicalIds.push(resourceChange.ResourceChange.LogicalResourceId!);
}
Expand Down
8 changes: 5 additions & 3 deletions packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,11 @@ export class TemplateDiff implements ITemplateDiff {
this.resources = args.resources || new DifferenceCollection({});
this.unknown = args.unknown || new DifferenceCollection({});

this.iamChanges = new IamChanges({
propertyChanges: this.scrutinizablePropertyChanges(IamChanges.IamPropertyScrutinies),
resourceChanges: this.scrutinizableResourceChanges(IamChanges.IamResourceScrutinies),
const x = this.scrutinizablePropertyChanges(IamChanges.IamPropertyScrutinies); // these are missing the old and new values
const y = this.scrutinizableResourceChanges(IamChanges.IamResourceScrutinies); // this has the old and new values but is still being ignored in the iam constructor.
this.iamChanges = new IamChanges({ // changes are not added because oldvalue and newvalue are undefined, which causes early return
propertyChanges: x,
resourceChanges: y,
});

this.securityGroupChanges = new SecurityGroupChanges({
Expand Down
Loading

0 comments on commit 05956d6

Please sign in to comment.