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

feat(cloudformation-diff): use awscdk-service-spec as data source #27813

Merged
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
4 changes: 4 additions & 0 deletions aws-cdk.code-workspace
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@
"name": "cli-lib-alpha",
"rootPath": "packages/@aws-cdk/cli-lib-alpha"
},
{
"name": "cloudformation-diff",
"rootPath": "packages/@aws-cdk/cloudformation-diff"
},
{
"name": "custom-resource-handlers",
"rootPath": "packages/@aws-cdk/custom-resource-handlers"
Expand Down
20 changes: 14 additions & 6 deletions packages/@aws-cdk/cloudformation-diff/jest.config.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,18 @@
const baseConfig = require('@aws-cdk/cdk-build-tools/config/jest.config');

/** @type {import('ts-jest').JestConfigWithTsJest} */
module.exports = {
...baseConfig,
coverageThreshold: {
global: {
statements: 60,
branches: 55,
},
...baseConfig,

// Different than usual
testMatch: [
'<rootDir>/**/test/**/?(*.)+(test).ts',
],

coverageThreshold: {
global: {
branches: 60,
statements: 55,
},
},
};
17 changes: 8 additions & 9 deletions packages/@aws-cdk/cloudformation-diff/lib/diff/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as cfnspec from '@aws-cdk/cfnspec';
import { Resource } from '@aws-cdk/service-spec-types';
import * as types from './types';
import { deepEqual, diffKeyedEntities } from './util';
import { deepEqual, diffKeyedEntities, loadResourceModel } from './util';

export function diffAttribute(oldValue: any, newValue: any): types.Difference<string> {
return new types.Difference<string>(_asString(oldValue), _asString(newValue));
Expand Down Expand Up @@ -36,8 +36,7 @@ export function diffResource(oldValue?: types.Resource, newValue?: types.Resourc

if (resourceType.oldType !== undefined && resourceType.oldType === resourceType.newType) {
// Only makes sense to inspect deeper if the types stayed the same
const typeSpec = cfnspec.filteredSpecification(resourceType.oldType);
const impl = typeSpec.ResourceTypes[resourceType.oldType];
const impl = loadResourceModel(resourceType.oldType);
propertyDiffs = diffKeyedEntities(oldValue!.Properties,
newValue!.Properties,
(oldVal, newVal, key) => _diffProperty(oldVal, newVal, key, impl));
Expand All @@ -50,16 +49,16 @@ export function diffResource(oldValue?: types.Resource, newValue?: types.Resourc
resourceType, propertyDiffs, otherDiffs,
});

function _diffProperty(oldV: any, newV: any, key: string, resourceSpec?: cfnspec.schema.ResourceType) {
function _diffProperty(oldV: any, newV: any, key: string, resourceSpec?: Resource) {
let changeImpact = types.ResourceImpact.NO_CHANGE;

const spec = resourceSpec && resourceSpec.Properties && resourceSpec.Properties[key];
const spec = resourceSpec?.properties?.[key];
if (spec && !deepEqual(oldV, newV)) {
switch (spec.UpdateType) {
case cfnspec.schema.UpdateType.Immutable:
switch (spec.causesReplacement) {
case 'yes':
changeImpact = types.ResourceImpact.WILL_REPLACE;
break;
case cfnspec.schema.UpdateType.Conditional:
case 'maybe':
changeImpact = types.ResourceImpact.MAY_REPLACE;
break;
default:
Expand Down
93 changes: 55 additions & 38 deletions packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { AssertionError } from 'assert';
import * as cfnspec from '@aws-cdk/cfnspec';
import { deepEqual } from './util';
import { PropertyScrutinyType, ResourceScrutinyType, Resource as ResourceModel } from '@aws-cdk/service-spec-types';
import { deepEqual, loadResourceModel } from './util';
import { IamChanges } from '../iam/iam-changes';
import { SecurityGroupChanges } from '../network/security-group-changes';

Expand Down Expand Up @@ -55,10 +55,10 @@ export class TemplateDiff implements ITemplateDiff {
});

this.securityGroupChanges = new SecurityGroupChanges({
egressRulePropertyChanges: this.scrutinizablePropertyChanges([cfnspec.schema.PropertyScrutinyType.EgressRules]),
ingressRulePropertyChanges: this.scrutinizablePropertyChanges([cfnspec.schema.PropertyScrutinyType.IngressRules]),
egressRuleResourceChanges: this.scrutinizableResourceChanges([cfnspec.schema.ResourceScrutinyType.EgressRuleResource]),
ingressRuleResourceChanges: this.scrutinizableResourceChanges([cfnspec.schema.ResourceScrutinyType.IngressRuleResource]),
egressRulePropertyChanges: this.scrutinizablePropertyChanges([PropertyScrutinyType.EgressRules]),
ingressRulePropertyChanges: this.scrutinizablePropertyChanges([PropertyScrutinyType.IngressRules]),
egressRuleResourceChanges: this.scrutinizableResourceChanges([ResourceScrutinyType.EgressRuleResource]),
ingressRuleResourceChanges: this.scrutinizableResourceChanges([ResourceScrutinyType.IngressRuleResource]),
});
}

Expand Down Expand Up @@ -110,7 +110,7 @@ export class TemplateDiff implements ITemplateDiff {
* We don't just look at property updates; we also look at resource additions and deletions (in which
* case there is no further detail on property values), and resource type changes.
*/
private scrutinizablePropertyChanges(scrutinyTypes: cfnspec.schema.PropertyScrutinyType[]): PropertyChange[] {
private scrutinizablePropertyChanges(scrutinyTypes: PropertyScrutinyType[]): PropertyChange[] {
const ret = new Array<PropertyChange>();

for (const [resourceLogicalId, resourceChange] of Object.entries(this.resources.changes)) {
Expand All @@ -119,16 +119,23 @@ export class TemplateDiff implements ITemplateDiff {
continue;
}

const props = cfnspec.scrutinizablePropertyNames(resourceChange.newResourceType!, scrutinyTypes);
for (const propertyName of props) {
ret.push({
resourceLogicalId,
propertyName,
resourceType: resourceChange.resourceType,
scrutinyType: cfnspec.propertySpecification(resourceChange.resourceType, propertyName).ScrutinyType!,
oldValue: resourceChange.oldProperties && resourceChange.oldProperties[propertyName],
newValue: resourceChange.newProperties && resourceChange.newProperties[propertyName],
});
if (!resourceChange.newResourceType) {
continue;
}

const newTypeProps = loadResourceModel(resourceChange.newResourceType)?.properties || {};
for (const [propertyName, prop] of Object.entries(newTypeProps)) {
const propScrutinyType = prop.scrutinizable || PropertyScrutinyType.None;
if (scrutinyTypes.includes(propScrutinyType)) {
ret.push({
resourceLogicalId,
propertyName,
resourceType: resourceChange.resourceType,
scrutinyType: propScrutinyType,
oldValue: resourceChange.oldProperties?.[propertyName],
newValue: resourceChange.newProperties?.[propertyName],
});
}
}
}

Expand All @@ -141,11 +148,9 @@ export class TemplateDiff implements ITemplateDiff {
* We don't just look at resource updates; we also look at resource additions and deletions (in which
* case there is no further detail on property values), and resource type changes.
*/
private scrutinizableResourceChanges(scrutinyTypes: cfnspec.schema.ResourceScrutinyType[]): ResourceChange[] {
private scrutinizableResourceChanges(scrutinyTypes: ResourceScrutinyType[]): ResourceChange[] {
const ret = new Array<ResourceChange>();

const scrutinizableTypes = new Set(cfnspec.scrutinizableResourceTypes(scrutinyTypes));

for (const [resourceLogicalId, resourceChange] of Object.entries(this.resources.changes)) {
if (!resourceChange) { continue; }

Expand All @@ -158,35 +163,47 @@ export class TemplateDiff implements ITemplateDiff {
// changes to the Type of resources can happen when migrating from CFN templates that use Transforms
if (resourceChange.resourceTypeChanged) {
// Treat as DELETE+ADD
if (scrutinizableTypes.has(resourceChange.oldResourceType!)) {
ret.push({
...commonProps,
newProperties: undefined,
resourceType: resourceChange.oldResourceType!,
scrutinyType: cfnspec.resourceSpecification(resourceChange.oldResourceType!).ScrutinyType!,
});
if (resourceChange.oldResourceType) {
const oldResourceModel = loadResourceModel(resourceChange.oldResourceType);
if (oldResourceModel && this.resourceIsScrutinizable(oldResourceModel, scrutinyTypes)) {
ret.push({
...commonProps,
newProperties: undefined,
resourceType: resourceChange.oldResourceType!,
scrutinyType: oldResourceModel.scrutinizable!,
});
}
}
if (scrutinizableTypes.has(resourceChange.newResourceType!)) {
ret.push({
...commonProps,
oldProperties: undefined,
resourceType: resourceChange.newResourceType!,
scrutinyType: cfnspec.resourceSpecification(resourceChange.newResourceType!).ScrutinyType!,
});

if (resourceChange.newResourceType) {
const newResourceModel = loadResourceModel(resourceChange.newResourceType);
if (newResourceModel && this.resourceIsScrutinizable(newResourceModel, scrutinyTypes)) {
ret.push({
...commonProps,
oldProperties: undefined,
resourceType: resourceChange.newResourceType!,
scrutinyType: newResourceModel.scrutinizable!,
});
}
}
} else {
if (scrutinizableTypes.has(resourceChange.resourceType)) {
const resourceModel = loadResourceModel(resourceChange.resourceType);
if (resourceModel && this.resourceIsScrutinizable(resourceModel, scrutinyTypes)) {
ret.push({
...commonProps,
resourceType: resourceChange.resourceType,
scrutinyType: cfnspec.resourceSpecification(resourceChange.resourceType).ScrutinyType!,
scrutinyType: resourceModel.scrutinizable!,
});
}
}
}

return ret;
}

private resourceIsScrutinizable(res: ResourceModel, scrutinyTypes: Array<ResourceScrutinyType>): boolean {
return scrutinyTypes.includes(res.scrutinizable || ResourceScrutinyType.None);
}
}

/**
Expand All @@ -211,7 +228,7 @@ export interface PropertyChange {
/**
* Scrutiny type for this property change
*/
scrutinyType: cfnspec.schema.PropertyScrutinyType;
scrutinyType: PropertyScrutinyType;

/**
* Name of the property that is changing
Expand Down Expand Up @@ -243,7 +260,7 @@ export interface ResourceChange {
/**
* Scrutiny type for this resource change
*/
scrutinyType: cfnspec.schema.ResourceScrutinyType;
scrutinyType: ResourceScrutinyType;

/**
* The type of the resource
Expand Down
23 changes: 23 additions & 0 deletions packages/@aws-cdk/cloudformation-diff/lib/diff/util.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import { loadAwsServiceSpecSync } from '@aws-cdk/aws-service-spec';
import { Resource, SpecDatabase } from '@aws-cdk/service-spec-types';

/**
* Compares two objects for equality, deeply. The function handles arguments that are
* +null+, +undefined+, arrays and objects. For objects, the function will not take the
Expand Down Expand Up @@ -163,3 +166,23 @@ export function mangleLikeCloudFormation(payload: string) {
function safeParseFloat(str: string): number {
return Number(str);
}

/**
* Lazily load the service spec database and cache the loaded db
*/
let DATABASE: SpecDatabase | undefined;
function database(): SpecDatabase {
if (!DATABASE) {
DATABASE = loadAwsServiceSpecSync();
}
return DATABASE;
}

/**
* Load a Resource model from the Service Spec Database
*
* The database is loaded lazily and cached across multiple calls to `loadResourceModel`.
*/
export function loadResourceModel(type: string): Resource | undefined {
return database().lookup('resource', 'cloudFormationType', 'equals', type).at(0);
}
4 changes: 2 additions & 2 deletions packages/@aws-cdk/cloudformation-diff/lib/format.ts
Original file line number Diff line number Diff line change
Expand Up @@ -305,12 +305,12 @@ class Formatter {
for (const [logicalId, resourceDiff] of Object.entries(templateDiff.resources)) {
if (!resourceDiff) { continue; }

const oldPathMetadata = resourceDiff.oldValue && resourceDiff.oldValue.Metadata && resourceDiff.oldValue.Metadata[PATH_METADATA_KEY];
const oldPathMetadata = resourceDiff.oldValue?.Metadata?.[PATH_METADATA_KEY];
if (oldPathMetadata && !(logicalId in this.logicalToPathMap)) {
this.logicalToPathMap[logicalId] = oldPathMetadata;
}

const newPathMetadata = resourceDiff.newValue && resourceDiff.newValue.Metadata && resourceDiff.newValue.Metadata[PATH_METADATA_KEY];
const newPathMetadata = resourceDiff.newValue?.Metadata?.[PATH_METADATA_KEY];
if (newPathMetadata && !(logicalId in this.logicalToPathMap)) {
this.logicalToPathMap[logicalId] = newPathMetadata;
}
Expand Down
26 changes: 13 additions & 13 deletions packages/@aws-cdk/cloudformation-diff/lib/iam/iam-changes.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as cfnspec from '@aws-cdk/cfnspec';
import { PropertyScrutinyType, ResourceScrutinyType } from '@aws-cdk/service-spec-types';
import * as chalk from 'chalk';
import { ManagedPolicyAttachment, ManagedPolicyJson } from './managed-policy';
import { parseLambdaPermission, parseStatements, Statement, StatementJson } from './statement';
Expand All @@ -18,15 +18,15 @@ export interface IamChangesProps {
*/
export class IamChanges {
public static IamPropertyScrutinies = [
cfnspec.schema.PropertyScrutinyType.InlineIdentityPolicies,
cfnspec.schema.PropertyScrutinyType.InlineResourcePolicy,
cfnspec.schema.PropertyScrutinyType.ManagedPolicies,
PropertyScrutinyType.InlineIdentityPolicies,
PropertyScrutinyType.InlineResourcePolicy,
PropertyScrutinyType.ManagedPolicies,
];

public static IamResourceScrutinies = [
cfnspec.schema.ResourceScrutinyType.ResourcePolicyResource,
cfnspec.schema.ResourceScrutinyType.IdentityPolicyResource,
cfnspec.schema.ResourceScrutinyType.LambdaPermission,
ResourceScrutinyType.ResourcePolicyResource,
ResourceScrutinyType.IdentityPolicyResource,
ResourceScrutinyType.LambdaPermission,
];

public readonly statements = new DiffableCollection<Statement>();
Expand Down Expand Up @@ -144,17 +144,17 @@ export class IamChanges {

private readPropertyChange(propertyChange: PropertyChange) {
switch (propertyChange.scrutinyType) {
case cfnspec.schema.PropertyScrutinyType.InlineIdentityPolicies:
case PropertyScrutinyType.InlineIdentityPolicies:
// AWS::IAM::{ Role | User | Group }.Policies
this.statements.addOld(...this.readIdentityPolicies(propertyChange.oldValue, propertyChange.resourceLogicalId));
this.statements.addNew(...this.readIdentityPolicies(propertyChange.newValue, propertyChange.resourceLogicalId));
break;
case cfnspec.schema.PropertyScrutinyType.InlineResourcePolicy:
case PropertyScrutinyType.InlineResourcePolicy:
// Any PolicyDocument on a resource (including AssumeRolePolicyDocument)
this.statements.addOld(...this.readResourceStatements(propertyChange.oldValue, propertyChange.resourceLogicalId));
this.statements.addNew(...this.readResourceStatements(propertyChange.newValue, propertyChange.resourceLogicalId));
break;
case cfnspec.schema.PropertyScrutinyType.ManagedPolicies:
case PropertyScrutinyType.ManagedPolicies:
// Just a list of managed policies
this.managedPolicies.addOld(...this.readManagedPolicies(propertyChange.oldValue, propertyChange.resourceLogicalId));
this.managedPolicies.addNew(...this.readManagedPolicies(propertyChange.newValue, propertyChange.resourceLogicalId));
Expand All @@ -164,17 +164,17 @@ export class IamChanges {

private readResourceChange(resourceChange: ResourceChange) {
switch (resourceChange.scrutinyType) {
case cfnspec.schema.ResourceScrutinyType.IdentityPolicyResource:
case ResourceScrutinyType.IdentityPolicyResource:
// AWS::IAM::Policy
this.statements.addOld(...this.readIdentityPolicyResource(resourceChange.oldProperties));
this.statements.addNew(...this.readIdentityPolicyResource(resourceChange.newProperties));
break;
case cfnspec.schema.ResourceScrutinyType.ResourcePolicyResource:
case ResourceScrutinyType.ResourcePolicyResource:
// AWS::*::{Bucket,Queue,Topic}Policy
this.statements.addOld(...this.readResourcePolicyResource(resourceChange.oldProperties));
this.statements.addNew(...this.readResourcePolicyResource(resourceChange.newProperties));
break;
case cfnspec.schema.ResourceScrutinyType.LambdaPermission:
case ResourceScrutinyType.LambdaPermission:
this.statements.addOld(...this.readLambdaStatements(resourceChange.oldProperties));
this.statements.addNew(...this.readLambdaStatements(resourceChange.newProperties));
break;
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/cloudformation-diff/lib/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,4 @@ export function flatMap<T, U>(xs: T[], f: (x: T) => U[]): U[] {
ret.push(...f(x));
}
return ret;
}
}
8 changes: 7 additions & 1 deletion packages/@aws-cdk/cloudformation-diff/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
},
"license": "Apache-2.0",
"dependencies": {
"@aws-cdk/cfnspec": "0.0.0",
"@aws-cdk/aws-service-spec": "^0.0.26",
"@aws-cdk/service-spec-types": "^0.0.26",
"chalk": "^4",
"diff": "^5.1.0",
"fast-deep-equal": "^3.1.3",
Expand Down Expand Up @@ -56,5 +57,10 @@
"maturity": "stable",
"publishConfig": {
"tag": "latest"
},
"pkglint": {
"exclude": [
"dependencies/cdk-point-dependencies"
]
}
}
2 changes: 1 addition & 1 deletion packages/@aws-cdk/integ-runner/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
"BSD-2-Clause",
"0BSD"
],
"dontAttribute": "^@aws-cdk/|^cdk-assets$",
"dontAttribute": "^@aws-cdk/|^@cdklabs/|^cdk-assets$",
"test": "bin/integ-runner --version"
}
},
Expand Down
1 change: 1 addition & 0 deletions packages/aws-cdk/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,4 @@ test/integ/cli/*.d.ts
junit.xml

lib/**/*.wasm
db.json.gz
Loading
Loading