From 3ac396d2b4f8db54faa8a1189792c38b5f5f2e1d Mon Sep 17 00:00:00 2001 From: Momo Kornher Date: Thu, 2 Nov 2023 16:28:11 +0000 Subject: [PATCH] feat(cloudformation-diff): use awscdk-service-spec as data source --- .../cloudformation-diff/lib/diff/index.ts | 16 +++--- .../cloudformation-diff/lib/diff/types.ts | 49 ++++++++++--------- .../lib/iam/iam-changes.ts | 26 +++++----- .../@aws-cdk/cloudformation-diff/lib/model.ts | 15 ++++++ .../@aws-cdk/cloudformation-diff/package.json | 3 +- 5 files changed, 65 insertions(+), 44 deletions(-) create mode 100644 packages/@aws-cdk/cloudformation-diff/lib/model.ts diff --git a/packages/@aws-cdk/cloudformation-diff/lib/diff/index.ts b/packages/@aws-cdk/cloudformation-diff/lib/diff/index.ts index c7716be2f6522..47d4a3a2938ef 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 { database } from '../model'; 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 = database().lookup('resource', 'cloudFormationType', 'equals', resourceType.oldType).only(); 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 && resourceSpec.properties && 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..5487b3072fe80 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts +++ b/packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts @@ -1,7 +1,8 @@ import { AssertionError } from 'assert'; -import * as cfnspec from '@aws-cdk/cfnspec'; +import { PropertyScrutinyType, ResourceScrutinyType } from '@aws-cdk/service-spec-types'; import { deepEqual } from './util'; import { IamChanges } from '../iam/iam-changes'; +import { database } from '../model'; import { SecurityGroupChanges } from '../network/security-group-changes'; 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,19 @@ 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], - }); + const newType = database().lookup('resource', 'cloudFormationType', 'equals', resourceChange.newResourceType!).only(); + for (const [propertyName, prop] of Object.entries(newType.properties)) { + const propScrutinyType = prop.scrutinizable || PropertyScrutinyType.None; + if (scrutinyTypes.includes(propScrutinyType)) { + ret.push({ + resourceLogicalId, + propertyName, + resourceType: resourceChange.resourceType, + scrutinyType: propScrutinyType, + oldValue: resourceChange.oldProperties && resourceChange.oldProperties[propertyName], + newValue: resourceChange.newProperties && resourceChange.newProperties[propertyName], + }); + } } } @@ -141,9 +145,10 @@ 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(); + database().lookup('resource', ''); const scrutinizableTypes = new Set(cfnspec.scrutinizableResourceTypes(scrutinyTypes)); for (const [resourceLogicalId, resourceChange] of Object.entries(this.resources.changes)) { @@ -163,7 +168,7 @@ export class TemplateDiff implements ITemplateDiff { ...commonProps, newProperties: undefined, resourceType: resourceChange.oldResourceType!, - scrutinyType: cfnspec.resourceSpecification(resourceChange.oldResourceType!).ScrutinyType!, + scrutinyType: cfnspec.resourceSpecification(resourceChange.oldResourceType!).scrutinizable!, }); } if (scrutinizableTypes.has(resourceChange.newResourceType!)) { @@ -171,7 +176,7 @@ export class TemplateDiff implements ITemplateDiff { ...commonProps, oldProperties: undefined, resourceType: resourceChange.newResourceType!, - scrutinyType: cfnspec.resourceSpecification(resourceChange.newResourceType!).ScrutinyType!, + scrutinyType: cfnspec.resourceSpecification(resourceChange.newResourceType!).scrutinizable!, }); } } else { @@ -179,7 +184,7 @@ export class TemplateDiff implements ITemplateDiff { ret.push({ ...commonProps, resourceType: resourceChange.resourceType, - scrutinyType: cfnspec.resourceSpecification(resourceChange.resourceType).ScrutinyType!, + scrutinyType: cfnspec.resourceSpecification(resourceChange.resourceType).scrutinizable!, }); } } @@ -211,7 +216,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 +248,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/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/model.ts b/packages/@aws-cdk/cloudformation-diff/lib/model.ts new file mode 100644 index 0000000000000..e9385210b7b0a --- /dev/null +++ b/packages/@aws-cdk/cloudformation-diff/lib/model.ts @@ -0,0 +1,15 @@ +import { readFileSync } from 'node:fs'; +import { gunzipSync } from 'node:zlib'; +import { SpecDatabase, emptyDatabase } from '@aws-cdk/service-spec-types'; + +const DATABASE = emptyDatabase(); +let DB_LOADED = false; + +export function database(): SpecDatabase { + if (!DB_LOADED) { + const spec = readFileSync(require.resolve('@aws-cdk/aws-service-spec/db.json.gz')); + DATABASE.load(JSON.parse(gunzipSync(spec).toString('utf-8'))); + DB_LOADED = true; + } + return DATABASE; +} diff --git a/packages/@aws-cdk/cloudformation-diff/package.json b/packages/@aws-cdk/cloudformation-diff/package.json index 7e1f355f08f2e..797236e3acb9f 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.24", + "@aws-cdk/service-spec-types": "^0.0.24", "chalk": "^4", "diff": "^5.1.0", "fast-deep-equal": "^3.1.3",