feat(ecr-assets): custom docker files

[iliapolo]

Added ability to provide a custom Dockerfile in the relevant asset constructs.

Those are:

• DockerImageAsset
• AssetImage

Things to note

1. The file property is assumed to be a relative path inside the asset directory. While its possible to specify a Dockerfile outside the context path, in our case, we want to keep things as self-contained as possible inside the asset directory. If a different need arrises, we will address it then.

2. If the property is not defined, we use Dockerfile as the default. This happens in the constructor of DockerImageAsset.

3. Resolving and validating the Dockerfile is done as soon as possible --> in the constructor. I didn't want the resolving logic to appear twice, so the prepareContainerAsset function simply adds the --file option to the docker command as is (if given).

4. Prior to this change, users had to have a Dockerfile (hardcoded) in their asset directory.

Closes #3829

---

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

[iliapolo]

@eladb Question:

I've changed the way we invoke docker build command. Is there an integration test covering this? How/When are those executed?

[aws-cdk-automation]

AWS CodeBuild CI Report

• 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

• 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

• Result: SUCCEEDED
• Build Logs (available for 30 days)

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

[eladb]

Looks great. Minor comments.

[aws-cdk-automation]

AWS CodeBuild CI Report

• 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 is now being automatically merged.

[eladb]

This needs to be a relative path since the docker directory is copied into cloud assembly ("staging"). Since we resolve dir above, it results in an absolute path.

[eladb]

I think props.file would just work here

[iliapolo]

This was actually done intentionally. The idea was to do the validation as soon as possible (like the other validations). To validate - I have to join the directory and the file. If I were to pass only the relative path here, it means the cli would have to join again - resulting in duplicate logic. I think its also inline with the notion of letting the framework do the work and keeping the cli as simple as possible.

I think what I missed though is that the final command would look something like:

docker build --file ${props.directory}/${props.file} /generated/path/to/cloudassemby/....

Which means the command relies on resources outside the cloudassemly. Is this what you meant by:

needs to be a relative path since the docker directory is copied into cloud assembly ?

[eladb]

Ok that the validation is done early, but the value passed to "docker build" needs to be relative to the docker directory.

[iliapolo]

Actually the value can't be relative because docker will fail - it doesn't assume you pass a relative path to the context when you use --file.

The path passed to docker build --file should be absolute, it just needs to be derived from the asset path inside the cloud assembly. What has to be relative is the property passed to the asset, since otherwise, the manifest in the cloud-assembly will contain an absolute path of the machine the assembly was created on.

[eladb]

This assertion should have just been:

Suggested change

	test.deepEqual(assetMetadata && assetMetadata.data.file, path.join(directoryPath, 'Dockerfile.Custom'));
	test.deepEqual(assetMetadata && assetMetadata.data.file, 'Dockerfile.Custom');

[nalhabash]

Has anyone been able to use custom dockerfiles since the 1.20 release ?
Trying right now in my stack, with v1.21.1.

const nginxImage = new DockerImageAsset(this, 'NginxDockerImage', {
            directory: __dirname + '/../../app/',
            target: 'some_target',
            repositoryName: `some_repo`,
            file: 'Dockerfile.nginx',
        });

But I am getting this error Error response from daemon: failed to reach build target some_target in Dockerfile.

I have another php image that I build from a Dockerfile file, and it works fine.

[iliapolo]

Hi @nalhabash

We indeed had an issue regarding this feature in 1.20.0 and 1.21.0, but version 1.21.1 should work properly. You can have a look here for some more information.

Regarding your specific issue, the error you are getting means that the Dockerfile.nginx file is missing the some_target target. Are you sure you defined it?

If possible, can you share your project structure and the contents of your Dockerfile.nginx file? This will help us figure out this issue quickly.

Thanks

[nalhabash]

Hi !
I don't know what is going on, I reverted to reproduce the issue and it looks like it works now. It might be that I somehow forgot to run npm run build or something silly like that, I don't know.
Still thanks for replying @iliapolo !

[iliapolo]

Awesome, glad all is well 👍