diff --git a/aws-cdk.code-workspace b/aws-cdk.code-workspace index ad498ecc8ae71..a5f31caba18b9 100644 --- a/aws-cdk.code-workspace +++ b/aws-cdk.code-workspace @@ -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" diff --git a/packages/@aws-cdk/cloudformation-diff/jest.config.js b/packages/@aws-cdk/cloudformation-diff/jest.config.js index e49f3262c4d91..f022e087fa6d5 100644 --- a/packages/@aws-cdk/cloudformation-diff/jest.config.js +++ b/packages/@aws-cdk/cloudformation-diff/jest.config.js @@ -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: [ + '/**/test/**/?(*.)+(test).ts', + ], + + coverageThreshold: { + global: { + branches: 60, + statements: 55, }, + }, }; diff --git a/packages/@aws-cdk/cloudformation-diff/lib/diff/index.ts b/packages/@aws-cdk/cloudformation-diff/lib/diff/index.ts index c7716be2f6522..8cf4e9ee830b7 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/diff/index.ts +++ b/packages/@aws-cdk/cloudformation-diff/lib/diff/index.ts @@ -1,6 +1,7 @@ -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 { loadResourceModel } from '../util'; export function diffAttribute(oldValue: any, newValue: any): types.Difference { return new types.Difference(_asString(oldValue), _asString(newValue)); @@ -36,8 +37,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)); @@ -50,16 +50,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: diff --git a/packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts b/packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts index ae482173a7229..c6e6328ea1294 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts +++ b/packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts @@ -1,8 +1,9 @@ import { AssertionError } from 'assert'; -import * as cfnspec from '@aws-cdk/cfnspec'; +import { PropertyScrutinyType, ResourceScrutinyType, Resource as ResourceModel } from '@aws-cdk/service-spec-types'; import { deepEqual } from './util'; import { IamChanges } from '../iam/iam-changes'; import { SecurityGroupChanges } from '../network/security-group-changes'; +import { loadResourceModel } from '../util'; export type PropertyMap = {[key: string]: any }; @@ -55,10 +56,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]), }); } @@ -110,7 +111,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(); for (const [resourceLogicalId, resourceChange] of Object.entries(this.resources.changes)) { @@ -119,16 +120,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], + }); + } } } @@ -141,11 +149,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(); - const scrutinizableTypes = new Set(cfnspec.scrutinizableResourceTypes(scrutinyTypes)); - for (const [resourceLogicalId, resourceChange] of Object.entries(this.resources.changes)) { if (!resourceChange) { continue; } @@ -158,28 +164,36 @@ 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!, }); } } @@ -187,6 +201,10 @@ export class TemplateDiff implements ITemplateDiff { return ret; } + + private resourceIsScrutinizable(res: ResourceModel, scrutinyTypes: Array): boolean { + return scrutinyTypes.includes(res.scrutinizable || ResourceScrutinyType.None); + } } /** @@ -211,7 +229,7 @@ export interface PropertyChange { /** * Scrutiny type for this property change */ - scrutinyType: cfnspec.schema.PropertyScrutinyType; + scrutinyType: PropertyScrutinyType; /** * Name of the property that is changing @@ -243,7 +261,7 @@ export interface ResourceChange { /** * Scrutiny type for this resource change */ - scrutinyType: cfnspec.schema.ResourceScrutinyType; + scrutinyType: ResourceScrutinyType; /** * The type of the resource diff --git a/packages/@aws-cdk/cloudformation-diff/lib/format.ts b/packages/@aws-cdk/cloudformation-diff/lib/format.ts index 6cddb3c2415ab..ce84c2e40f31c 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/format.ts +++ b/packages/@aws-cdk/cloudformation-diff/lib/format.ts @@ -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; } diff --git a/packages/@aws-cdk/cloudformation-diff/lib/iam/iam-changes.ts b/packages/@aws-cdk/cloudformation-diff/lib/iam/iam-changes.ts index f18ac6dfcab52..017ed6bcd904f 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/iam/iam-changes.ts +++ b/packages/@aws-cdk/cloudformation-diff/lib/iam/iam-changes.ts @@ -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'; @@ -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(); @@ -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)); @@ -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; diff --git a/packages/@aws-cdk/cloudformation-diff/lib/util.ts b/packages/@aws-cdk/cloudformation-diff/lib/util.ts index 5fcb761b2a284..011b6fa1ae7c1 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/util.ts +++ b/packages/@aws-cdk/cloudformation-diff/lib/util.ts @@ -1,3 +1,6 @@ +import { loadAwsServiceSpecSync } from '@aws-cdk/aws-service-spec'; +import { Resource, SpecDatabase } from '@aws-cdk/service-spec-types'; + /** * Turn a (multi-key) extraction function into a comparator for use in Array.sort() */ @@ -46,4 +49,16 @@ export function flatMap(xs: T[], f: (x: T) => U[]): U[] { ret.push(...f(x)); } return ret; +} + +let DATABASE: SpecDatabase | undefined; +function database(): SpecDatabase { + if (!DATABASE) { + DATABASE = loadAwsServiceSpecSync(); + } + return DATABASE; +} + +export function loadResourceModel(type: string): Resource | undefined { + return database().lookup('resource', 'cloudFormationType', 'equals', type).at(0); } \ No newline at end of file diff --git a/packages/@aws-cdk/cloudformation-diff/package.json b/packages/@aws-cdk/cloudformation-diff/package.json index 7e1f355f08f2e..7501b94961229 100644 --- a/packages/@aws-cdk/cloudformation-diff/package.json +++ b/packages/@aws-cdk/cloudformation-diff/package.json @@ -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", @@ -56,5 +57,10 @@ "maturity": "stable", "publishConfig": { "tag": "latest" + }, + "pkglint": { + "exclude": [ + "dependencies/cdk-point-dependencies" + ] } }