Skip to content

Commit

Permalink
fix(cfn-diff): allow resources to change types (#19891)
Browse files Browse the repository at this point in the history
When writing the cloudformation-diff module,
it was assumed that resources would never change their CloudFormation types between deployments.
As it turns out, there is a legitimate case for resources changing types,
and that's when you migrate from a template that uses Transforms
(like the Serverless Transform) to one that doesn't use them.

Because of that, handle the case of resources changing types in diff.

Fixes #13921

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
skinny85 committed Apr 14, 2022
1 parent c421a5f commit 4f3a340
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 2 deletions.
7 changes: 5 additions & 2 deletions packages/@aws-cdk/cloudformation-diff/lib/diff/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,10 @@ export class TemplateDiff implements ITemplateDiff {
const ret = new Array<PropertyChange>();

for (const [resourceLogicalId, resourceChange] of Object.entries(this.resources.changes)) {
if (!resourceChange) { continue; }
if (resourceChange.resourceTypeChanged) {
// we ignore resource type changes here, and handle them in scrutinizableResourceChanges()
continue;
}

const props = cfnspec.scrutinizablePropertyNames(resourceChange.newResourceType!, scrutinyTypes);
for (const propertyName of props) {
Expand Down Expand Up @@ -152,7 +155,7 @@ export class TemplateDiff implements ITemplateDiff {
resourceLogicalId,
};

// Even though it's not physically possible in CFN, let's pretend to handle a change of 'Type'.
// 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!)) {
Expand Down
34 changes: 34 additions & 0 deletions packages/@aws-cdk/cloudformation-diff/test/diff-template.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -655,3 +655,37 @@ test('when a property with a number-like format doesn\'t change', () => {
const difference = differences.resources.changes.BucketResource;
expect(difference).toBeUndefined();
});

test('handles a resource changing its Type', () => {
const currentTemplate = {
Resources: {
FunctionApi: {
Type: 'AWS::Serverless::Api',
Properties: {
StageName: 'prod',
},
},
},
};
const newTemplate = {
Resources: {
FunctionApi: {
Type: 'AWS::ApiGateway::RestApi',
},
},
};

const differences = diffTemplate(currentTemplate, newTemplate);
expect(differences.differenceCount).toBe(1);
expect(differences.resources.differenceCount).toBe(1);
const difference = differences.resources.changes.FunctionApi;
expect(difference).toEqual({
isAddition: false,
isRemoval: false,
newValue: { Type: 'AWS::ApiGateway::RestApi' },
oldValue: { Properties: { StageName: 'prod' }, Type: 'AWS::Serverless::Api' },
otherDiffs: {},
propertyDiffs: {},
resourceTypes: { newType: 'AWS::ApiGateway::RestApi', oldType: 'AWS::Serverless::Api' },
});
});

0 comments on commit 4f3a340

Please sign in to comment.