fix: docker build attestations break cdk-assets (400 Bad Request) #342
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kaizencc
changed the title
kaizencc
changed the title
1 task
iliapolo
reviewed
Feb 11, 2025
|
How come? which two files? Docker default is now to include an 'attestations' file along with the docker image -- the file is basically metadata: https://docs.docker.com/build/metadata/attestations/slsa-provenance/. Attestations attach to images as a manifest in the image index. see https://docs.docker.com/build/metadata/attestations/#storage cdk-assets does not expect 2 files to come out of the docker build step and picks one at random first to upload to ECR, which sometimes will be the attestation file (which can be confirmed in ECR if the image size is 0.0 mb). it will then try to upload the second file which ECR refuses due to the immutability setting. |
iliapolo
approved these changes
Feb 11, 2025
aws-cdk-automation
pushed a commit
that referenced
this pull request
Feb 11, 2025
There are various issues in cdk that can be traced back to attestations in docker: aws/aws-cdk#30258 aws/aws-cdk#31549 aws/aws-cdk#33264 cdk-assets cannot work with docker containerd because it will attempt to upload multiple files to the same hash in ECR, and our ECR repository is immutable (by requirement). docker recently changed their default to turn on containerd which causes this issue to skyrocket. the hotfix here is to add an environment variable when calling `docker` so that the attestation file is not added to the manifest. we can later look into adding support for including [provenance](https://docs.docker.com/build/metadata/attestations/slsa-provenance/) attestations if there is need for it. i've chosen to fix this via environment variable instead of as a command option `--provenance=false` because we must keep docker replacements in mind, and at least finch [does not](https://runfinch.com/docs/cli-reference/finch_build/) have a `provenance` property at the moment. in addition to this unit test that shows the env variable exists when `docker build` is called, i have also ensured that this solves the issue in my local setup + symlinked `cdk-assets`.. (cherry picked from commit 8bdea13) # Conflicts: # lib/private/docker.ts # test/private/docker.test.ts
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation and see the Github Action logs for details |
github-merge-queue bot
pushed a commit
that referenced
this pull request
Feb 11, 2025
…ckport #342) (#347) # Backport This will backport the following commits from `main` to `v2-main`: - [fix: docker build attestations break cdk-assets (400 Bad Request) (#342)](#342) <!--- Backport version: 9.5.1 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) --------- Co-authored-by: Kaizen Conroy <[email protected]>
This was referenced Feb 11, 2025
mergify bot
pushed a commit
to aws/aws-cdk
that referenced
this pull request
Feb 12, 2025
The lastest cdk-assets is required in cdk to mitigate a ECR upload issue. It includes the following fix: cdklabs/cdk-assets#342. The following issues are related to this: #30258 #31549 #33264 I am keeping #31549 open as it is still true. this [feature request](cdklabs/cdk-assets#348) tracks the work to make cdk-assets compatible with containerd Closes #30258 and closes #33264
yashkh-amzn
pushed a commit
to yashkh-amzn/aws-cdk
that referenced
this pull request
Feb 21, 2025
The lastest cdk-assets is required in cdk to mitigate a ECR upload issue. It includes the following fix: cdklabs/cdk-assets#342. The following issues are related to this: aws#30258 aws#31549 aws#33264 I am keeping aws#31549 open as it is still true. this [feature request](cdklabs/cdk-assets#348) tracks the work to make cdk-assets compatible with containerd Closes aws#30258 and closes aws#33264
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There are various issues in cdk that can be traced back to attestations in docker:
aws/aws-cdk#30258
aws/aws-cdk#31549
aws/aws-cdk#33264
cdk-assets cannot work with docker containerd because it will attempt to upload multiple files to the same hash in ECR, and our ECR repository is immutable (by requirement). docker recently changed their default to turn on containerd which causes this issue to skyrocket.
the hotfix here is to add an environment variable when calling
dockerso that the attestation file is not added to the manifest. we can later look into adding support for including provenance attestations if there is need for it.i've chosen to fix this via environment variable instead of as a command option
--provenance=falsebecause we must keep docker replacements in mind, and at least finch does not have aprovenanceproperty at the moment.in addition to this unit test that shows the env variable exists when
docker buildis called, i have also ensured that this solves the issue in my local setup + symlinkedcdk-assets..