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

Toolkit: diff shows version bump even if nothing else changed #465

Closed
rix0rrr opened this issue Aug 1, 2018 · 3 comments · Fixed by #1186
Closed

Toolkit: diff shows version bump even if nothing else changed #465

rix0rrr opened this issue Aug 1, 2018 · 3 comments · Fixed by #1186
Labels
package/tools Related to AWS CDK Tools or CLI

Comments

@rix0rrr
Copy link
Contributor

rix0rrr commented Aug 1, 2018

I was making changes, and to verify I didn't break anything I ran cdk diff. This was the output:

huijbers ~/W/P/cdk-ops(master)> cdk diff cdk-master
[~] 🛠 Updating CDKMetadata (type: AWS::CDK::Metadata)
 └─ [~] .Modules:
     ├─ [-] Old value: @aws-cdk/assets=0.7.4-beta,@aws-cdk/aws-cloudformation=0.
7.4-beta,@aws-cdk/aws-cloudwatch=0.7.4-beta,@aws-cdk/aws-codebuild=0.7.4-beta,@a
ws-cdk/aws-codebuild-codepipeline=0.7.4-beta,@aws-cdk/aws-codecommit-codepipelin
e=0.7.4-beta,@aws-cdk/aws-codepipeline=0.7.4-beta,@aws-cdk/aws-custom-resources=
0.7.4-beta,@aws-cdk/aws-events=0.7.4-beta,@aws-cdk/aws-iam=0.7.4-beta,@aws-cdk/a
ws-kms=0.7.4-beta,@aws-cdk/aws-lambda=0.7.4-beta,@aws-cdk/aws-s3=0.7.4-beta,@aws
-cdk/aws-sns=0.7.4-beta,@aws-cdk/cdk=0.7.4-beta,@aws-cdk/cx-api=0.7.4-beta,@aws-
cdk/util=0.7.4-beta,buildable=1.0.0,js-base64=2.4.5
     └─ [+] New value: @aws-cdk/assets=0.8.0,@aws-cdk/aws-cloudformation=0.8.0,@
aws-cdk/aws-cloudwatch=0.8.0,@aws-cdk/aws-codebuild=0.8.0,@aws-cdk/aws-codebuild
-codepipeline=0.8.0,@aws-cdk/aws-codecommit-codepipeline=0.8.0,@aws-cdk/aws-code
pipeline=0.8.0,@aws-cdk/aws-custom-resources=0.8.0,@aws-cdk/aws-events=0.8.0,@aw
s-cdk/aws-iam=0.8.0,@aws-cdk/aws-kms=0.8.0,@aws-cdk/aws-lambda=0.8.0,@aws-cdk/aw
s-s3=0.8.0,@aws-cdk/aws-sns=0.8.0,@aws-cdk/cdk=0.8.0,@aws-cdk/cx-api=0.8.0,@aws-
cdk/util=0.8.0,buildable=1.0.0,js-base64=2.4.5

The tool was answering my question "did anything change" literally correct, but I would still argue that this is not helpful and we shouldn't include the CDKMetadata resource in the diff:

  • This resource is not about the USER's resources. They don't care about it, it doesn't fulfill any function for them.
  • It actively harms--now I need to know what this resource means and need to do reading to convince myself that, no, it's not actually important to my infrastructure. Yes, we are all very aware about what it is and does, but any arbitrary user doesn't know and shouldn't need to know.
  • In fact, it's an implementation detail. This is how we've chosen to communicate the version data back to the server. If it had been in the template's Metadata section, it would not have been part of the diff (even though the same data would have changed). Does that seem right to you?

There is one good counterargument to be made--if cdk diff shows no diff, you would expect cdk deploy to not do anything either, which would be broken by this change.

I am willing to take that hit, as am I willing to udpate the stackset change checker to disregard changes to the CDKMetadata resource.

@RomainMuller
Copy link
Contributor

A possible middle-ground would be to ignore the CDKMetadata resource by default in the cdk diff output (filter it out, basically), but have a cdk diff --pedantic (come up with a better name if you can) that includes everything.

@rix0rrr
Copy link
Contributor Author

rix0rrr commented Aug 2, 2018

Yes, I was thinking something like that as well.

@eladb
Copy link
Contributor

eladb commented Aug 2, 2018

Sounds good (--strict?)

@debora-ito debora-ito added the package/tools Related to AWS CDK Tools or CLI label Nov 7, 2018
eladb pushed a commit that referenced this issue Nov 15, 2018
`DifferenceCollection#applyFilter` can be used to delete any changes
that do not pass the predicate from the collection.

Changed `DifferenceCollection#count` to do a lazy calculation since now
`changes` is mutable.

The toolkit switch `cdk diff --strict` disables this behavior.

Fixes #465
eladb pushed a commit that referenced this issue Nov 18, 2018
)

`DifferenceCollection#filter` returns a new collection with changes filtered.

Changed `DifferenceCollection#count` to do a lazy calculation since now
`changes` is mutable.

The toolkit switch `cdk diff --strict` disables this behavior.

Fixes #465
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package/tools Related to AWS CDK Tools or CLI
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants