Skip to content

Commit

Permalink
fix(CLI): sam resources hidden in changeset diffs (#29223)
Browse files Browse the repository at this point in the history
### Issue # (if applicable)

Closes #29185

### Reason for this change

CFN applies the SAM transform before the changeset is created. This means that SAM resources become their underlying CFN types in the template that the changeset operates on. This means that the changeset is operating on resources that we don't see in our template. 

### Description of changes

Before, if we saw properties in our diff that were not in the changeset (like `codeUri` for `Serverless::Function`, which doesn't appear in the changeset, because it becomes `Code` for `Lambda::Function`), we'd filter them out of the diff. We now skip this process for SAM resources.

### Description of how you validated changes

unit test

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
comcalvi authored Feb 23, 2024
1 parent 2e080fe commit aa186ac
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 0 deletions.
4 changes: 4 additions & 0 deletions packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,10 @@ function addImportInformation(diff: types.TemplateDiff, changeSet: CloudFormatio
function filterFalsePositivies(diff: types.TemplateDiff, changeSet: CloudFormation.DescribeChangeSetOutput) {
const replacements = findResourceReplacements(changeSet);
diff.resources.forEachDifference((logicalId: string, change: types.ResourceDifference) => {
if (change.resourceType.includes('AWS::Serverless')) {
// CFN applies the SAM transform before creating the changeset, so the changeset contains no information about SAM resources
return;
}
change.forEachDifference((type: 'Property' | 'Other', name: string, value: types.Difference<any> | types.PropertyDifference<any>) => {
if (type === 'Property') {
if (!replacements[logicalId]) {
Expand Down
49 changes: 49 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 @@ -1118,6 +1118,55 @@ describe('changeset', () => {
expect(differences.resources.differenceCount).toBe(1);
});

test('SAM Resources are rendered with changeset diffs', () => {
// GIVEN
const currentTemplate = {
Resources: {
ServerlessFunction: {
Type: 'AWS::Serverless::Function',
Properties: {
CodeUri: 's3://bermuda-triangle-1337-bucket/old-handler.zip',
},
},
},
};

// WHEN
const newTemplate = {
Resources: {
ServerlessFunction: {
Type: 'AWS::Serverless::Function',
Properties: {
CodeUri: 's3://bermuda-triangle-1337-bucket/new-handler.zip',
},
},
},
};

let differences = fullDiff(currentTemplate, newTemplate, {
Changes: [
{
Type: 'Resource',
ResourceChange: {
Action: 'Modify',
LogicalResourceId: 'ServerlessFunction',
ResourceType: 'AWS::Lambda::Function', // The SAM transform is applied before the changeset is created, so the changeset has a Lambda resource here!
Replacement: 'False',
Details: [{
Evaluation: 'Direct',
Target: {
Attribute: 'Properties',
Name: 'Code',
RequiresRecreation: 'Never',
},
}],
},
},
],
});
expect(differences.resources.differenceCount).toBe(1);
});

test('imports are respected for new stacks', async () => {
// GIVEN
const currentTemplate = {};
Expand Down

0 comments on commit aa186ac

Please sign in to comment.