feat(assets): Add deploy-time content hash #2334

RomainMuller · 2019-04-19T16:45:58Z

Introduces an IAsset interface that centralizes common aspects about
assets, such as the sourceHash and bundleHash properties.

The sourceHash fingerprints the objects that are used as the source
for the asset bundling logic, and is available at synthesis time (it can
for example be injected in construct IDs when it one wants to ensure a
new logical ID is issued for every new version of the asset).

The bundleHash fingerprints the result of the bundling logic, and is
more accurate than sourceHash (in that, if the same source can produce
different artifacts at different points in time, the sourceHash will
remain un-changed, but the bundleHash will change. The bundleHash is
however a deploy-time value and thus cannot be used in construct IDs.

Fixes #1400

Pull Request Checklist

Testing
- Unit test added (prefer not to modify an existing test, otherwise, it's probably a breaking change)
- CLI change?: coordinate update of integration tests with team
- cdk-init template change?: coordinated update of integration tests with team
Docs
- jsdocs: All public APIs documented
- README: README and/or documentation topic updated
Title and Description
- Change type: title prefixed with fix, feat will appear in changelog
- Title: use lower-case and doesn't end with a period
- Breaking?: last paragraph: "BREAKING CHANGE: <describe what changed + link for details>"
- Issues: Indicate issues fixed via: "Fixes #xxx" or "Closes #xxx"
Sensitive Modules (requires 2 PR approvers)
- IAM Policy Document (in @aws-cdk/aws-iam)
- EC2 Security Groups and ACLs (in @aws-cdk/aws-ec2)
- Grant APIs (only if not based on official documentation with a reference)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.

Introduces an `IAsset` interface that centralizes common aspects about assets, such as the `sourceHash` and `bundleHash` properties. The `sourceHash` fingerprints the objects that are used as the source for the asset bundling logic, and is available at synthesis time (it can for example be injected in construct IDs when it one wants to ensure a new logical ID is issued for every new version of the asset). The `bundleHash` fingerprints the result of the bundling logic, and is more accurate than `sourceHash` (in that, if the same source can produce different artifacts at different points in time, the `sourceHash` will remain un-changed, but the `bundleHash` will change. The `bundleHash` is however a deploy-time value and thus cannot be used in construct IDs. Fixes #1400

packages/@aws-cdk/assets-docker/lib/adopt-repository/handler.js

packages/@aws-cdk/assets/lib/asset.ts

packages/aws-cdk/bin/cdk.ts

packages/aws-cdk/lib/cdk-toolkit.ts

packages/aws-cdk/lib/docker.ts

eladb · 2019-04-22T19:28:36Z

packages/@aws-cdk/assets/lib/fs/fingerprint.ts

-  }
+  const rootDirectory = fs.statSync(fileOrDirectory).isDirectory()
+    ? fileOrDirectory
+    : path.dirname(fileOrDirectory);


missing tests for all the new options

As far as I know, I have not introduced any new options 😅 but yes, some of that surface is untested. I'll se if I can get around to fixing that.

The ability to exclude files has been added (previously these options were ignored)

…nt-hash

jogold · 2019-04-23T16:29:01Z

packages/@aws-cdk/aws-ecs/test/ec2/integ.event-task.lit.expected.json

@@ -692,13 +692,13 @@
                      "RepositoryName"
                    ]
                  },
-                  ":",
+                  "@sha256:",


The mix of Fn::Select and Fn::Join in the Image property looks weird to me (+ it ends up with amazonaws.com in the final string and not AWS::URLSuffix), shouldn't this result in something like:

{ "Image": { "Fn::Join": [ "", [ { "Ref": "AWS::AccountId" }, ".dkr.ecr.", { "Ref": "AWS::Region" }, ".", { "Ref": "AWS::URLSuffix" }, "/", { "Fn::GetAtt": [ "EventImageAdoptRepositoryDFAAC242", "RepositoryName"] }, "@sha256:" { "Ref": "EventImageImageNameE972A8B1" }, ] ] } }

or { "Ref": "AdoptEcrRepositorydbc60defc59544bcaa5c28c95d68f62cCodeArtifactHash8BCBAA49" } for the last element

+after pushing, now that the repo digest is used we have the full "resolved URI" (with region, accountid, etc.), is it still needed to use Refs in this case? A single Ref to the parameter should be possible?

We're passing the repository name, not full URI, in a parameter. This is because the CustomResource that "adopts" the CDK-toolkit-created ECR repository works on the name (for that is the ECR API). I'm torn myself on whether I should cut the chase and just pass the URI around, but it makes parsing the name out of the URI an awkward process (I'd rather recompose the URI than parse it out).

packages/@aws-cdk/assets-docker/lib/image-asset.ts

eladb · 2019-04-30T10:15:48Z

packages/@aws-cdk/assets/lib/fs/fingerprint.ts

-  }
+  const rootDirectory = fs.statSync(fileOrDirectory).isDirectory()
+    ? fileOrDirectory
+    : path.dirname(fileOrDirectory);


The ability to exclude files has been added (previously these options were ignored)

eladb · 2019-04-30T10:16:35Z

packages/@aws-cdk/assets/lib/staging.ts

@@ -53,6 +58,7 @@ export class Staging extends Construct {
    super(scope, id);

    this.sourcePath = props.sourcePath;
+    this.sourceHash = fingerprint(this.sourcePath);


I think we should expose the copy options (exclude, symlinks), but this can be in a subsequent PR.

…nt-hash

RomainMuller requested review from skinny85, SoManyHs and a team as code owners April 19, 2019 16:45

eladb reviewed Apr 22, 2019

View reviewed changes

RomainMuller added 2 commits April 23, 2019 14:51

Merge remote-tracking branch 'origin/master' into rmuller/asset-conte…

e98da5e

…nt-hash

Apply PR feedback

127847a

jogold reviewed Apr 23, 2019

View reviewed changes

Add new unit testing for new feature surface

769f2a3

RomainMuller force-pushed the rmuller/asset-content-hash branch from 729f535 to 769f2a3 Compare April 24, 2019 13:20

eladb approved these changes Apr 30, 2019

View reviewed changes

mitchlloyd mentioned this pull request May 20, 2019

Lambda code asset reuse #2595

Closed

RomainMuller added 10 commits May 23, 2019 09:09

Merge remote-tracking branch 'origin/master' into rmuller/asset-conte…

e95bd78

…nt-hash

Add example in comment

97dd087

Forward CopyOptions everywhere

ce946c8

Merge remote-tracking branch 'origin/master' into rmuller/asset-conte…

7588ad4

…nt-hash

Fix some shit

2fee5cb

Merge remote-tracking branch 'origin/master' into rmuller/asset-conte…

74376df

…nt-hash

Fix linter warning

cd17ab2

Fix broken fs utils test

edbdd37

Merge remote-tracking branch 'origin/master' into rmuller/asset-conte…

d259a1c

…nt-hash

Fix integ test expectations

22b15b0

RomainMuller merged commit 9b4db42 into master May 23, 2019

RomainMuller deleted the rmuller/asset-content-hash branch May 23, 2019 15:48

markusl mentioned this pull request May 25, 2019

CDK 0.32.0 fails to deploy ECR repository: Parameters: [ ... CodeArtifactHash ...] must have values #2638

Closed

nason mentioned this pull request May 29, 2019

[aws-ecr-assets] DockerImageAsset: Specify image tag #2663

Closed

snyk-bot mentioned this pull request Aug 22, 2019

[Snyk] Upgrade @aws-cdk/aws-ecs from 0.18.1 to 0.39.0 udinachmany/trivia-api#5

Open

NGL321 added the contribution/core This is a PR that came from AWS. label Sep 27, 2019

This was referenced Jan 20, 2020

[Snyk] Upgrade @aws-cdk/aws-route53 from 0.22.0 to 0.39.0 MechanicalRock/tech-radar#20

Closed

[Snyk] Upgrade @aws-cdk/aws-s3 from 0.22.0 to 0.39.0 MechanicalRock/tech-radar#21

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(assets): Add deploy-time content hash #2334

feat(assets): Add deploy-time content hash #2334

RomainMuller commented Apr 19, 2019 •

edited by shivlaks

Loading

eladb Apr 22, 2019

RomainMuller Apr 23, 2019

eladb Apr 30, 2019

jogold Apr 23, 2019 •

edited

Loading

jogold Apr 23, 2019 •

edited

Loading

RomainMuller Apr 24, 2019

eladb Apr 30, 2019

eladb Apr 30, 2019

feat(assets): Add deploy-time content hash #2334

feat(assets): Add deploy-time content hash #2334

Conversation

RomainMuller commented Apr 19, 2019 • edited by shivlaks Loading

Pull Request Checklist

eladb Apr 22, 2019

Choose a reason for hiding this comment

RomainMuller Apr 23, 2019

Choose a reason for hiding this comment

eladb Apr 30, 2019

Choose a reason for hiding this comment

jogold Apr 23, 2019 • edited Loading

Choose a reason for hiding this comment

jogold Apr 23, 2019 • edited Loading

Choose a reason for hiding this comment

RomainMuller Apr 24, 2019

Choose a reason for hiding this comment

eladb Apr 30, 2019

Choose a reason for hiding this comment

eladb Apr 30, 2019

Choose a reason for hiding this comment

RomainMuller commented Apr 19, 2019 •

edited by shivlaks

Loading

jogold Apr 23, 2019 •

edited

Loading

jogold Apr 23, 2019 •

edited

Loading