fix(cdk-assets): context path not honored by Docker asset build

[MrArnoldPalmer]

Commit Message

fix(cdk-assets): context path not honored by Docker asset build (#6957)

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

End Commit Message

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
• Commit ID: 0283d4b
• Result: FAILED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
• Commit ID: ca318e3
• Result: FAILED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
• Commit ID: 3d0247b
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
• Commit ID: 70f984e
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[anguslees]

lgtm fwiw. The below alternative patch also worked for me if for some reason the reviewers wanted to avoid setting cwd (I don't know why that would be an issue):

--- cdk-assets/lib/private/handlers/container-images.js.orig    2020-03-24 06:03:56.670862836 +0000
+++ cdk-assets/lib/private/handlers/container-images.js 2020-03-24 05:58:01.971071483 +0000
@@ -51,7 +51,7 @@
             tag: this.localTagName,
             buildArgs: source.dockerBuildArgs,
             target: source.dockerBuildTarget,
-            file: source.dockerFile,
+            file: path.resolve(fullPath, source.dockerFile),
         });
     }
 }

[nija-at]

Fix the commit title to all lower case. For fixes, the commit title should summarize the issue it is fixing.
Remember to use mergify's '##Commit Message' in the description so mergify picks up the updated title.

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.

Can we correct this test, or add a better one?

[shivlaks]

Fix the commit title to all lower case. For fixes, the commit title should summarize the issue it is fixing.
Remember to use mergify's '##Commit Message' in the description so mergify picks up the updated title.

done

Can we correct this test, or add a better one?

yep, it's currently failing right now, but I'll address it's flakiness in a follow-up PR once we've mitigated the p0

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
• Commit ID: eeeb8df
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[iliapolo]

@MrArnoldPalmer wrote:

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.

Thats not accurate. To my understanding, in both 1.29.0 and 1.28.0, the context being passed is the asset directory, from the original issue:

in 1.28.0:

docker build --tag 942768376061.dkr.ecr.eu-central-1.amazonaws.com/aws-cdk/assets:xxx --target production --file cdk.out/asset.xxx/services/admin/Dockerfile cdk.out/asset.xxx

The context here is cdk.out/asset.xxx, which is the asset directory, relative to the cdk working directory (the app directory).

in 1.30.0:

docker build  --tag cdkasset-xxx --file services/backend/Dockerfile /Volumes/Projects/whatever/build/cdk/cdk.out/asset.xxx

The context here is /Volumes/Projects/whatever/build/cdk/cdk.out/asset.xxx, which is the same thing, except now its absolute.

This is ok, the problem is that in 1.28.0, the --file value was relative to the cdk working directory (cdk.out/asset.xxx/services/admin/Dockerfile), but in 1.30.0, its relative to the context path (services/backend/Dockerfile). Which would have been ok if docker would take the context path and append the --file path to it, which is what it claims it does:

From https://docs.docker.com/engine/reference/commandline/build/

If a relative path is specified then it is interpreted as relative to the root of the context.

this just doesn't seem to be true. Docker always looks up the --file under the current working directory, not the context path.

So the fix is basically to always have the context directory be the current working directory when running docker build

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
• Commit ID: 3e67579
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[mergify]

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).