Skip to content

fix(ecr-assets): unable to use one Dockerfile to build multiple images#5705

Merged
mergify[bot] merged 8 commits intomasterfrom
benisrae/fix-docker-source-hash-salt
Jan 8, 2020
Merged

fix(ecr-assets): unable to use one Dockerfile to build multiple images#5705
mergify[bot] merged 8 commits intomasterfrom
benisrae/fix-docker-source-hash-salt

Conversation

@eladb
Copy link
Copy Markdown
Contributor

@eladb eladb commented Jan 8, 2020

Since we use the source hash as the image ID, if we use different docker build options such as specifying a different target, buildArgs or a custom docker file name, we still get the same image ID. This means that if this image ID already exists, we skip the build. This makes it impossible to use the same docker build context to build multiple images.

This fix exposes the ability to include extra information to the source hash fingerprint algorithm (in assets.Staging), and utilizes this capability to "salt" the hash with build arguments if they are provided.

Fixes #5683


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

Since we use the source hash as the image ID, if we use different docker build options such as specifying a different `target`, `buildArgs` or a custom docker file name, we still get the same image ID. This means that if this image ID already exists, we skip the build. This makes it impossible to use the same docker build context to build multiple images.

This fix exposes the ability to include extra information to the source hash fingerprint algorithm (in `assets.Staging`), and utilizes this capability to "salt" the hash with build arguments if they are provided.

Fixes #5683
@eladb eladb requested a review from rix0rrr as a code owner January 8, 2020 09:10
@eladb eladb requested a review from iliapolo January 8, 2020 09:10
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Jan 8, 2020
@aws-cdk-automation
Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

Copy link
Copy Markdown
Contributor

@RomainMuller RomainMuller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a nice feature! I have some minor comments, but mostly stylistic (and quite frankly, besides the name being awkward, it's not that important).

Copy link
Copy Markdown
Contributor

@iliapolo iliapolo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eladb few comments

Elad Ben-Israel added 2 commits January 8, 2020 15:32
- rename `extra` to `extraHash` so it makes more sense when extending `FingerprintOptions`.
- build `extraHash` as an object instead of an array.
- include `repositoryName` in fingerprint
@eladb eladb requested review from RomainMuller and iliapolo January 8, 2020 13:34
@aws-cdk-automation
Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Jan 8, 2020

Thank you for contributing! Your pull request is now being automatically merged.

@aws-cdk-automation
Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Jan 8, 2020

Thank you for contributing! Your pull request is now being automatically merged.

@mergify mergify bot merged commit ff3f27f into master Jan 8, 2020
@mergify mergify bot deleted the benisrae/fix-docker-source-hash-salt branch January 8, 2020 15:56
@amirfireeye
Copy link
Copy Markdown
Contributor

Can this behavior be disabled or tuned with some flag? We have the opposite problem where we pass CodeArtifact token as a build arg so every single synth generates a new Docker image.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contribution/core This is a PR that came from AWS.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ecr-assets: unable to use one Dockerfile to build multiple images

5 participants