Skip to content

Commit

Permalink
fix(cdk-assets): Docker Build Bad Context Path
Browse files Browse the repository at this point in the history
Prior v1.29.0, the context passed to the docker build command was always
the current working directory. In v1.29.0, this was changed to be the
directory the user passed when instantiating the asset. However, docker
doesn't respect the context passed to the build command when the
`--file` argument is also passed. This broke assets for users passing
the `file` prop when constructing a `DockerImageAsset`.

This is fixed by changing the current working directory of the `docker build`
command and passing that context as '.'. Then docker uses the current working
directory as the context and correctly finds the image definition file from the
`file` prop. An existing integration test in the framework covers this case,
but does not fail unless the stack is deployed fresh since the asset isn't
rebuilt when the hash is not changed.

Fix: #6954 #6814
  • Loading branch information
MrArnoldPalmer committed Mar 24, 2020
1 parent 1e92564 commit 70f984e
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 5 deletions.
6 changes: 3 additions & 3 deletions packages/cdk-assets/lib/private/docker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ export class Docker {
'--tag', options.tag,
...options.target ? ['--target', options.target] : [],
...options.file ? ['--file', options.file] : [],
options.directory,
'.'
];
await this.execute(buildCommand);
await this.execute(buildCommand, { cwd: options.directory });
}

/**
Expand Down Expand Up @@ -100,4 +100,4 @@ async function obtainEcrCredentials(ecr: AWS.ECR, logger?: Logger) {

function flatten(x: string[][]) {
return Array.prototype.concat([], ...x);
}
}
4 changes: 2 additions & 2 deletions packages/cdk-assets/test/docker-images.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ describe('with a complete manifest', () => {
mockSpawn(
{ commandLine: ['docker', 'login', '--username', 'user', '--password-stdin', 'https://proxy.com/'] },
{ commandLine: ['docker', 'inspect', 'cdkasset-theasset'], exitCode: 1 },
{ commandLine: ['docker', 'build', '--tag', 'cdkasset-theasset', '/simple/cdk.out/dockerdir'] },
{ commandLine: ['docker', 'build', '--tag', 'cdkasset-theasset', '.'] },
{ commandLine: ['docker', 'tag', 'cdkasset-theasset', '12345.amazonaws.com/repo:abcdef'] },
{ commandLine: ['docker', 'push', '12345.amazonaws.com/repo:abcdef'] },
);
Expand All @@ -135,7 +135,7 @@ test('correctly identify Docker directory if path is absolute', async () => {
// Only care about the 'build' command line
{ commandLine: ['docker', 'login'], prefix: true, },
{ commandLine: ['docker', 'inspect'], exitCode: 1, prefix: true },
{ commandLine: ['docker', 'build', '--tag', 'cdkasset-theasset', '/simple/cdk.out/dockerdir'] },
{ commandLine: ['docker', 'build', '--tag', 'cdkasset-theasset', '.'] },
{ commandLine: ['docker', 'tag'], prefix: true },
{ commandLine: ['docker', 'push'], prefix: true },
);
Expand Down

0 comments on commit 70f984e

Please sign in to comment.