From f96997d315e00b49d89d7864d8e213649887f748 Mon Sep 17 00:00:00 2001
From: Kaizen Conroy <36202692+kaizencc@users.noreply.github.com>
Date: Tue, 11 Feb 2025 10:29:23 -0500
Subject: [PATCH 1/5] fix: docker build attestations break cdk-assets (400 Bad
Request) (#342)
There are various issues in cdk that can be traced back to attestations
in docker:
https://github.com/aws/aws-cdk/issues/30258
https://github.com/aws/aws-cdk/issues/31549
https://github.com/aws/aws-cdk/issues/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 8bdea135b8c3ee2f28f0c18f8d8ec912442c9025)
# Conflicts:
# lib/private/docker.ts
# test/private/docker.test.ts
---
lib/private/docker.ts | 7 +++++++
test/private/docker.test.ts | 41 +++++++++++++++++++++++++++++++++++++
2 files changed, 48 insertions(+)
diff --git a/lib/private/docker.ts b/lib/private/docker.ts
index 53f99a3..f861fe8 100644
--- a/lib/private/docker.ts
+++ b/lib/private/docker.ts
@@ -122,7 +122,14 @@ export class Docker {
];
await this.execute(buildCommand, {
cwd: options.directory,
+<<<<<<< HEAD
quiet: options.quiet,
+=======
+ subprocessOutputDestination: this.subprocessOutputDestination,
+ env: {
+ BUILDX_NO_DEFAULT_ATTESTATIONS: '1', // Docker Build adds provenance attestations by default that confuse cdk-assets
+ },
+>>>>>>> 8bdea13 (fix: docker build attestations break cdk-assets (400 Bad Request) (#342))
});
}
diff --git a/test/private/docker.test.ts b/test/private/docker.test.ts
index a9f942a..e6f6863 100644
--- a/test/private/docker.test.ts
+++ b/test/private/docker.test.ts
@@ -6,8 +6,26 @@ type ShellExecuteMock = jest.SpyInstance<
Parameters<Docker['execute']>
>;
+let docker: Docker;
+
+const makeShellExecuteMock = (fn: (params: string[]) => void): ShellExecuteMock =>
+ jest
+ .spyOn<{ execute: Docker['execute'] }, 'execute'>(Docker.prototype as any, 'execute')
+ .mockImplementation(
+ async (params: string[], _options?: Omit<ShellOptions, 'shellEventPublisher'>) => fn(params)
+ );
+
+afterEach(() => {
+ jest.restoreAllMocks();
+});
+
+beforeEach(() => {
+ docker = new Docker(() => {}, 'ignore');
+});
+
describe('Docker', () => {
describe('exists', () => {
+<<<<<<< HEAD
let docker: Docker;
const makeShellExecuteMock = (fn: (params: string[]) => void): ShellExecuteMock =>
@@ -23,6 +41,8 @@ describe('Docker', () => {
docker = new Docker();
});
+=======
+>>>>>>> 8bdea13 (fix: docker build attestations break cdk-assets (400 Bad Request) (#342))
test('returns true when image inspect command does not throw', async () => {
const spy = makeShellExecuteMock(() => undefined);
@@ -92,4 +112,25 @@ describe('Docker', () => {
expect(imageExists).toBe(false);
});
});
+
+ describe('build', () => {
+ test('includes BUILDX_NO_DEFAULT_ATTESTATIONS env variable in commands', async () => {
+ const spy = makeShellExecuteMock(() => undefined);
+
+ await docker.build({
+ directory: 'foo',
+ tag: 'bar',
+ });
+
+ // Verify the options passed to build
+ expect(spy).toHaveBeenCalledWith(
+ expect.any(Array),
+ expect.objectContaining({
+ env: expect.objectContaining({
+ BUILDX_NO_DEFAULT_ATTESTATIONS: '1',
+ }),
+ })
+ );
+ });
+ });
});
From 0fe11fbf92e2bec5b6e43c894189601e35ea85c9 Mon Sep 17 00:00:00 2001
From: Kaizen Conroy <36202692+kaizencc@users.noreply.github.com>
Date: Tue, 11 Feb 2025 10:54:08 -0500
Subject: [PATCH 2/5] Apply suggestions from code review
---
lib/private/docker.ts | 3 ---
test/private/docker.test.ts | 2 --
2 files changed, 5 deletions(-)
diff --git a/lib/private/docker.ts b/lib/private/docker.ts
index f861fe8..2179861 100644
--- a/lib/private/docker.ts
+++ b/lib/private/docker.ts
@@ -122,14 +122,11 @@ export class Docker {
];
await this.execute(buildCommand, {
cwd: options.directory,
-<<<<<<< HEAD
quiet: options.quiet,
-=======
subprocessOutputDestination: this.subprocessOutputDestination,
env: {
BUILDX_NO_DEFAULT_ATTESTATIONS: '1', // Docker Build adds provenance attestations by default that confuse cdk-assets
},
->>>>>>> 8bdea13 (fix: docker build attestations break cdk-assets (400 Bad Request) (#342))
});
}
diff --git a/test/private/docker.test.ts b/test/private/docker.test.ts
index e6f6863..1ea98a7 100644
--- a/test/private/docker.test.ts
+++ b/test/private/docker.test.ts
@@ -41,8 +41,6 @@ describe('Docker', () => {
docker = new Docker();
});
-=======
->>>>>>> 8bdea13 (fix: docker build attestations break cdk-assets (400 Bad Request) (#342))
test('returns true when image inspect command does not throw', async () => {
const spy = makeShellExecuteMock(() => undefined);
From 143b05b12d31095a760ba2c68b526dfead122143 Mon Sep 17 00:00:00 2001
From: Kaizen Conroy <36202692+kaizencc@users.noreply.github.com>
Date: Tue, 11 Feb 2025 10:54:31 -0500
Subject: [PATCH 3/5] Update lib/private/docker.ts
---
lib/private/docker.ts | 1 -
1 file changed, 1 deletion(-)
diff --git a/lib/private/docker.ts b/lib/private/docker.ts
index 2179861..2e584a3 100644
--- a/lib/private/docker.ts
+++ b/lib/private/docker.ts
@@ -123,7 +123,6 @@ export class Docker {
await this.execute(buildCommand, {
cwd: options.directory,
quiet: options.quiet,
- subprocessOutputDestination: this.subprocessOutputDestination,
env: {
BUILDX_NO_DEFAULT_ATTESTATIONS: '1', // Docker Build adds provenance attestations by default that confuse cdk-assets
},
From 2ade861951b2ade2dd3c25bcb962a96e64665b57 Mon Sep 17 00:00:00 2001
From: Kaizen Conroy <36202692+kaizencc@users.noreply.github.com>
Date: Tue, 11 Feb 2025 10:54:49 -0500
Subject: [PATCH 4/5] Update test/private/docker.test.ts
---
test/private/docker.test.ts | 1 -
1 file changed, 1 deletion(-)
diff --git a/test/private/docker.test.ts b/test/private/docker.test.ts
index 1ea98a7..6de39ba 100644
--- a/test/private/docker.test.ts
+++ b/test/private/docker.test.ts
@@ -25,7 +25,6 @@ beforeEach(() => {
describe('Docker', () => {
describe('exists', () => {
-<<<<<<< HEAD
let docker: Docker;
const makeShellExecuteMock = (fn: (params: string[]) => void): ShellExecuteMock =>
From 60409d44470f670e50c33276e7db1acbe41e92d5 Mon Sep 17 00:00:00 2001
From: Kaizen Conroy <36202692+kaizencc@users.noreply.github.com>
Date: Tue, 11 Feb 2025 10:58:15 -0500
Subject: [PATCH 5/5] Update docker.test.ts
---
test/private/docker.test.ts | 15 ---------------
1 file changed, 15 deletions(-)
diff --git a/test/private/docker.test.ts b/test/private/docker.test.ts
index 6de39ba..1782dc8 100644
--- a/test/private/docker.test.ts
+++ b/test/private/docker.test.ts
@@ -25,21 +25,6 @@ beforeEach(() => {
describe('Docker', () => {
describe('exists', () => {
- let docker: Docker;
-
- const makeShellExecuteMock = (fn: (params: string[]) => void): ShellExecuteMock =>
- jest
- .spyOn<{ execute: Docker['execute'] }, 'execute'>(Docker.prototype as any, 'execute')
- .mockImplementation(async (params: string[], _options?: ShellOptions) => fn(params));
-
- afterEach(() => {
- jest.restoreAllMocks();
- });
-
- beforeEach(() => {
- docker = new Docker();
- });
-
test('returns true when image inspect command does not throw', async () => {
const spy = makeShellExecuteMock(() => undefined);