Skip to content

Commit d451b30

Browse files
RichiCoder1RichiCoder1
authored
fix(ecr-assets): prefix cache arguments correctly (#24524)
Follow up to #24024, fixes an issue where cache args were not correctly prefixed and adds additional testing. Apologies for the second PR, I realized there was an issue literally as the auto-merge happened! ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 4e02566 commit d451b30

File tree

2 files changed

+141
-2
lines changed

2 files changed

+141
-2
lines changed

‎packages/cdk-assets/lib/private/docker.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,8 @@ export class Docker {
9797
...options.networkMode ? ['--network', options.networkMode] : [],
9898
...options.platform ? ['--platform', options.platform] : [],
9999
...options.outputs ? options.outputs.map(output => [`--output=${output}`]) : [],
100-
...options.cacheFrom ? options.cacheFrom.map(cacheFrom => this.cacheOptionToFlag(cacheFrom)) : [],
101-
...options.cacheTo ? [this.cacheOptionToFlag(options.cacheTo)] : [],
100+
...options.cacheFrom ? [...options.cacheFrom.map(cacheFrom => ['--cache-from', this.cacheOptionToFlag(cacheFrom)]).flat()] : [],
101+
...options.cacheTo ? ['--cache-to', this.cacheOptionToFlag(options.cacheTo)] : [],
102102
'.',
103103
];
104104
await this.execute(buildCommand, { cwd: options.directory });

‎packages/cdk-assets/test/docker-images.test.ts

+139
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,68 @@ beforeEach(() => {
144144
},
145145
},
146146
}),
147+
'/cache/cdk.out/assets.json': JSON.stringify({
148+
version: Manifest.version(),
149+
dockerImages: {
150+
theAsset: {
151+
source: {
152+
directory: 'dockerdir',
153+
cacheFrom: [{ type: 'registry', params: { ref: 'abcdef' } }],
154+
cacheTo: { type: 'inline' },
155+
},
156+
destinations: {
157+
theDestination: {
158+
region: 'us-north-50',
159+
assumeRoleArn: 'arn:aws:role',
160+
repositoryName: 'repo',
161+
imageTag: 'nopqr',
162+
},
163+
},
164+
},
165+
},
166+
}),
167+
'/cache-from-multiple/cdk.out/assets.json': JSON.stringify({
168+
version: Manifest.version(),
169+
dockerImages: {
170+
theAsset: {
171+
source: {
172+
directory: 'dockerdir',
173+
cacheFrom: [
174+
{ type: 'registry', params: { ref: 'cache:ref' } },
175+
{ type: 'registry', params: { ref: 'cache:main' } },
176+
{ type: 'gha' },
177+
],
178+
},
179+
destinations: {
180+
theDestination: {
181+
region: 'us-north-50',
182+
assumeRoleArn: 'arn:aws:role',
183+
repositoryName: 'repo',
184+
imageTag: 'nopqr',
185+
},
186+
},
187+
},
188+
},
189+
}),
190+
'/cache-to-complex/cdk.out/assets.json': JSON.stringify({
191+
version: Manifest.version(),
192+
dockerImages: {
193+
theAsset: {
194+
source: {
195+
directory: 'dockerdir',
196+
cacheTo: { type: 'registry', params: { ref: 'cache:main', mode: 'max', compression: 'zstd' } },
197+
},
198+
destinations: {
199+
theDestination: {
200+
region: 'us-north-50',
201+
assumeRoleArn: 'arn:aws:role',
202+
repositoryName: 'repo',
203+
imageTag: 'nopqr',
204+
},
205+
},
206+
},
207+
},
208+
}),
147209
'/platform-arm64/cdk.out/dockerdir/Dockerfile': 'FROM scratch',
148210
});
149211

@@ -284,6 +346,83 @@ describe('with a complete manifest', () => {
284346
expectAllSpawns();
285347
expect(true).toBeTruthy(); // Expect no exception, satisfy linter
286348
});
349+
350+
test('build with cache option', async () => {
351+
pub = new AssetPublishing(AssetManifest.fromPath('/cache/cdk.out'), { aws });
352+
const defaultNetworkDockerpath = '/cache/cdk.out/dockerdir';
353+
aws.mockEcr.describeImages = mockedApiFailure('ImageNotFoundException', 'File does not exist');
354+
aws.mockEcr.getAuthorizationToken = mockedApiResult({
355+
authorizationData: [
356+
{ authorizationToken: 'dXNlcjpwYXNz', proxyEndpoint: 'https://proxy.com/' },
357+
],
358+
});
359+
360+
const expectAllSpawns = mockSpawn(
361+
{ commandLine: ['docker', 'login', '--username', 'user', '--password-stdin', 'https://proxy.com/'] },
362+
{ commandLine: ['docker', 'inspect', 'cdkasset-theasset'], exitCode: 1 },
363+
{ commandLine: ['docker', 'build', '--tag', 'cdkasset-theasset', '--cache-from', 'type=registry,ref=abcdef', '--cache-to', 'type=inline', '.'], cwd: defaultNetworkDockerpath },
364+
{ commandLine: ['docker', 'tag', 'cdkasset-theasset', '12345.amazonaws.com/repo:nopqr'] },
365+
{ commandLine: ['docker', 'push', '12345.amazonaws.com/repo:nopqr'] },
366+
);
367+
368+
await pub.publish();
369+
370+
expectAllSpawns();
371+
expect(true).toBeTruthy(); // Expect no exception, satisfy linter
372+
});
373+
374+
test('build with multiple cache from option', async () => {
375+
pub = new AssetPublishing(AssetManifest.fromPath('/cache-from-multiple/cdk.out'), { aws });
376+
const defaultNetworkDockerpath = '/cache-from-multiple/cdk.out/dockerdir';
377+
aws.mockEcr.describeImages = mockedApiFailure('ImageNotFoundException', 'File does not exist');
378+
aws.mockEcr.getAuthorizationToken = mockedApiResult({
379+
authorizationData: [
380+
{ authorizationToken: 'dXNlcjpwYXNz', proxyEndpoint: 'https://proxy.com/' },
381+
],
382+
});
383+
384+
const expectAllSpawns = mockSpawn(
385+
{ commandLine: ['docker', 'login', '--username', 'user', '--password-stdin', 'https://proxy.com/'] },
386+
{ commandLine: ['docker', 'inspect', 'cdkasset-theasset'], exitCode: 1 },
387+
{
388+
commandLine: [
389+
'docker', 'build', '--tag', 'cdkasset-theasset', '--cache-from', 'type=registry,ref=cache:ref', '--cache-from', 'type=registry,ref=cache:main', '--cache-from', 'type=gha', '.',
390+
],
391+
cwd: defaultNetworkDockerpath,
392+
},
393+
{ commandLine: ['docker', 'tag', 'cdkasset-theasset', '12345.amazonaws.com/repo:nopqr'] },
394+
{ commandLine: ['docker', 'push', '12345.amazonaws.com/repo:nopqr'] },
395+
);
396+
397+
await pub.publish();
398+
399+
expectAllSpawns();
400+
expect(true).toBeTruthy(); // Expect no exception, satisfy linter
401+
});
402+
403+
test('build with cache to complex option', async () => {
404+
pub = new AssetPublishing(AssetManifest.fromPath('/cache-to-complex/cdk.out'), { aws });
405+
const defaultNetworkDockerpath = '/cache-to-complex/cdk.out/dockerdir';
406+
aws.mockEcr.describeImages = mockedApiFailure('ImageNotFoundException', 'File does not exist');
407+
aws.mockEcr.getAuthorizationToken = mockedApiResult({
408+
authorizationData: [
409+
{ authorizationToken: 'dXNlcjpwYXNz', proxyEndpoint: 'https://proxy.com/' },
410+
],
411+
});
412+
413+
const expectAllSpawns = mockSpawn(
414+
{ commandLine: ['docker', 'login', '--username', 'user', '--password-stdin', 'https://proxy.com/'] },
415+
{ commandLine: ['docker', 'inspect', 'cdkasset-theasset'], exitCode: 1 },
416+
{ commandLine: ['docker', 'build', '--tag', 'cdkasset-theasset', '--cache-to', 'type=registry,ref=cache:main,mode=max,compression=zstd', '.'], cwd: defaultNetworkDockerpath },
417+
{ commandLine: ['docker', 'tag', 'cdkasset-theasset', '12345.amazonaws.com/repo:nopqr'] },
418+
{ commandLine: ['docker', 'push', '12345.amazonaws.com/repo:nopqr'] },
419+
);
420+
421+
await pub.publish();
422+
423+
expectAllSpawns();
424+
expect(true).toBeTruthy(); // Expect no exception, satisfy linter
425+
});
287426
});
288427

289428
describe('external assets', () => {

0 commit comments

Comments
 (0)