Skip to content

Commit

Permalink
fix(ecr-assets): prefix cache arguments correctly (#24524)
Browse files Browse the repository at this point in the history 
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*
  • Loading branch information
@RichiCoder1
RichiCoder1 committed Mar 8, 2023
1 parent 4e02566 commit d451b30
Show file tree
Hide file tree
Showing 2 changed files with 141 additions and 2 deletions.
4 changes: 2 additions & 2 deletions packages/cdk-assets/lib/private/docker.ts
View file
Expand Up @@ -97,8 +97,8 @@ export class Docker {
...options.networkMode ? ['--network', options.networkMode] : [],
...options.platform ? ['--platform', options.platform] : [],
...options.outputs ? options.outputs.map(output => [`--output=${output}`]) : [],
...options.cacheFrom ? options.cacheFrom.map(cacheFrom => this.cacheOptionToFlag(cacheFrom)) : [],
...options.cacheTo ? [this.cacheOptionToFlag(options.cacheTo)] : [],
...options.cacheFrom ? [...options.cacheFrom.map(cacheFrom => ['--cache-from', this.cacheOptionToFlag(cacheFrom)]).flat()] : [],
...options.cacheTo ? ['--cache-to', this.cacheOptionToFlag(options.cacheTo)] : [],
'.',
];
await this.execute(buildCommand, { cwd: options.directory });
Expand Down
139 changes: 139 additions & 0 deletions packages/cdk-assets/test/docker-images.test.ts
View file
Expand Up @@ -144,6 +144,68 @@ beforeEach(() => {
},
},
}),
'/cache/cdk.out/assets.json': JSON.stringify({
version: Manifest.version(),
dockerImages: {
theAsset: {
source: {
directory: 'dockerdir',
cacheFrom: [{ type: 'registry', params: { ref: 'abcdef' } }],
cacheTo: { type: 'inline' },
},
destinations: {
theDestination: {
region: 'us-north-50',
assumeRoleArn: 'arn:aws:role',
repositoryName: 'repo',
imageTag: 'nopqr',
},
},
},
},
}),
'/cache-from-multiple/cdk.out/assets.json': JSON.stringify({
version: Manifest.version(),
dockerImages: {
theAsset: {
source: {
directory: 'dockerdir',
cacheFrom: [
{ type: 'registry', params: { ref: 'cache:ref' } },
{ type: 'registry', params: { ref: 'cache:main' } },
{ type: 'gha' },
],
},
destinations: {
theDestination: {
region: 'us-north-50',
assumeRoleArn: 'arn:aws:role',
repositoryName: 'repo',
imageTag: 'nopqr',
},
},
},
},
}),
'/cache-to-complex/cdk.out/assets.json': JSON.stringify({
version: Manifest.version(),
dockerImages: {
theAsset: {
source: {
directory: 'dockerdir',
cacheTo: { type: 'registry', params: { ref: 'cache:main', mode: 'max', compression: 'zstd' } },
},
destinations: {
theDestination: {
region: 'us-north-50',
assumeRoleArn: 'arn:aws:role',
repositoryName: 'repo',
imageTag: 'nopqr',
},
},
},
},
}),
'/platform-arm64/cdk.out/dockerdir/Dockerfile': 'FROM scratch',
});

Expand Down Expand Up @@ -284,6 +346,83 @@ describe('with a complete manifest', () => {
expectAllSpawns();
expect(true).toBeTruthy(); // Expect no exception, satisfy linter
});

test('build with cache option', async () => {
pub = new AssetPublishing(AssetManifest.fromPath('/cache/cdk.out'), { aws });
const defaultNetworkDockerpath = '/cache/cdk.out/dockerdir';
aws.mockEcr.describeImages = mockedApiFailure('ImageNotFoundException', 'File does not exist');
aws.mockEcr.getAuthorizationToken = mockedApiResult({
authorizationData: [
{ authorizationToken: 'dXNlcjpwYXNz', proxyEndpoint: 'https://proxy.com/' },
],
});

const expectAllSpawns = mockSpawn(
{ commandLine: ['docker', 'login', '--username', 'user', '--password-stdin', 'https://proxy.com/'] },
{ commandLine: ['docker', 'inspect', 'cdkasset-theasset'], exitCode: 1 },
{ commandLine: ['docker', 'build', '--tag', 'cdkasset-theasset', '--cache-from', 'type=registry,ref=abcdef', '--cache-to', 'type=inline', '.'], cwd: defaultNetworkDockerpath },
{ commandLine: ['docker', 'tag', 'cdkasset-theasset', '12345.amazonaws.com/repo:nopqr'] },
{ commandLine: ['docker', 'push', '12345.amazonaws.com/repo:nopqr'] },
);

await pub.publish();

expectAllSpawns();
expect(true).toBeTruthy(); // Expect no exception, satisfy linter
});

test('build with multiple cache from option', async () => {
pub = new AssetPublishing(AssetManifest.fromPath('/cache-from-multiple/cdk.out'), { aws });
const defaultNetworkDockerpath = '/cache-from-multiple/cdk.out/dockerdir';
aws.mockEcr.describeImages = mockedApiFailure('ImageNotFoundException', 'File does not exist');
aws.mockEcr.getAuthorizationToken = mockedApiResult({
authorizationData: [
{ authorizationToken: 'dXNlcjpwYXNz', proxyEndpoint: 'https://proxy.com/' },
],
});

const expectAllSpawns = mockSpawn(
{ commandLine: ['docker', 'login', '--username', 'user', '--password-stdin', 'https://proxy.com/'] },
{ commandLine: ['docker', 'inspect', 'cdkasset-theasset'], exitCode: 1 },
{
commandLine: [
'docker', 'build', '--tag', 'cdkasset-theasset', '--cache-from', 'type=registry,ref=cache:ref', '--cache-from', 'type=registry,ref=cache:main', '--cache-from', 'type=gha', '.',
],
cwd: defaultNetworkDockerpath,
},
{ commandLine: ['docker', 'tag', 'cdkasset-theasset', '12345.amazonaws.com/repo:nopqr'] },
{ commandLine: ['docker', 'push', '12345.amazonaws.com/repo:nopqr'] },
);

await pub.publish();

expectAllSpawns();
expect(true).toBeTruthy(); // Expect no exception, satisfy linter
});

test('build with cache to complex option', async () => {
pub = new AssetPublishing(AssetManifest.fromPath('/cache-to-complex/cdk.out'), { aws });
const defaultNetworkDockerpath = '/cache-to-complex/cdk.out/dockerdir';
aws.mockEcr.describeImages = mockedApiFailure('ImageNotFoundException', 'File does not exist');
aws.mockEcr.getAuthorizationToken = mockedApiResult({
authorizationData: [
{ authorizationToken: 'dXNlcjpwYXNz', proxyEndpoint: 'https://proxy.com/' },
],
});

const expectAllSpawns = mockSpawn(
{ commandLine: ['docker', 'login', '--username', 'user', '--password-stdin', 'https://proxy.com/'] },
{ commandLine: ['docker', 'inspect', 'cdkasset-theasset'], exitCode: 1 },
{ commandLine: ['docker', 'build', '--tag', 'cdkasset-theasset', '--cache-to', 'type=registry,ref=cache:main,mode=max,compression=zstd', '.'], cwd: defaultNetworkDockerpath },
{ commandLine: ['docker', 'tag', 'cdkasset-theasset', '12345.amazonaws.com/repo:nopqr'] },
{ commandLine: ['docker', 'push', '12345.amazonaws.com/repo:nopqr'] },
);

await pub.publish();

expectAllSpawns();
expect(true).toBeTruthy(); // Expect no exception, satisfy linter
});
});

describe('external assets', () => {
Expand Down

0 comments on commit d451b30

Please sign in to comment.