From c3316329e9df5d2cc74aa7f59ce16012425ce4d0 Mon Sep 17 00:00:00 2001 From: Andreas Sieferlinger Date: Mon, 12 Dec 2022 10:01:43 +0100 Subject: [PATCH] feat(core): add volumes-from option to docker run command for bundling (#22829) relates to #8799 follow up to stale #21660 ## Describe the feature Ability to add [--volumes-from](https://docs.docker.com/engine/reference/commandline/run/#mount-volumes-from-container---volumes-from) flag when bundling assets with docker. This enabled people using Docker in Docker to use CDKs bundling functionality, which is currently not possible. ## Use Case CICD systems often run within a docker container already. Many systems mount the ` /var/run/docker.sock` from the host system into the CICD container. When running bundling within such a container it currently breaks, as docker assume the path is from the host system, not within the CICD container. The options allows to mount the data from any other container. Very often it will be the current one which can be used by using the `HOSTNAME` environment variable ## Proposed Solution Add optional property to [DockerRunOptions](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.DockerRunOptions.html) and [BundlingOptions](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.BundlingOptions.html) that would translate into --volumes-from {user provided option} This change would not reflect in any CloudFormation changes, but only with the docker commands performed when bundling. Due to using the `--volumes-from` option, docker will instead of trying to find the path on the host (where it does not exist) try to use the volume that is created by the container C1 that is actually running the CDK. With that it is able to access the files from CDK and can continue the build. ![Docker volumes from](https://user-images.githubusercontent.com/2162832/193787498-de03c66c-7bce-458b-9776-7ba421b9d929.jpg) The following plain docker steps show how this works from the docker side, and why we need to adjust the `--volumes-from` parameter. ```sh docker volume create builds docker run -v /var/run/docker.sock:/var/run/docker.sock -v builds:/builds -it docker ``` Now within the just created docker container, run the following commands. ```sh echo "testfile" > /builds/my-share-file.txt docker run --rm --name DinDContainer --volumes-from="${HOSTNAME}" ubuntu bash -c "ls -hla /builds" ``` We see that the second container C2 (here `DinDContainer`) has the same files available as the container C1. ## Alternative solutions I'm not aware of alternative solutions for this docker in docker use cases, besides of not relying on docker at all, which is out of scope for this MR. ---- ### All Submissions: * [X] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? I ran it, but it seems not to have generated something, i might need some guidance there. *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- packages/@aws-cdk/core/lib/asset-staging.ts | 1 + packages/@aws-cdk/core/lib/bundling.ts | 17 +++++++++ packages/@aws-cdk/core/test/bundling.test.ts | 38 ++++++++++++++++++++ 3 files changed, 56 insertions(+) diff --git a/packages/@aws-cdk/core/lib/asset-staging.ts b/packages/@aws-cdk/core/lib/asset-staging.ts index 96b16554a67f1..6831790d50938 100644 --- a/packages/@aws-cdk/core/lib/asset-staging.ts +++ b/packages/@aws-cdk/core/lib/asset-staging.ts @@ -469,6 +469,7 @@ export class AssetStaging extends Construct { entrypoint: options.entrypoint, workingDirectory: options.workingDirectory ?? AssetStaging.BUNDLING_INPUT_DIR, securityOpt: options.securityOpt ?? '', + volumesFrom: options.volumesFrom, }); } } catch (err) { diff --git a/packages/@aws-cdk/core/lib/bundling.ts b/packages/@aws-cdk/core/lib/bundling.ts index c496bf31d8672..a78f5e8a6aca0 100644 --- a/packages/@aws-cdk/core/lib/bundling.ts +++ b/packages/@aws-cdk/core/lib/bundling.ts @@ -43,6 +43,13 @@ export interface BundlingOptions { */ readonly volumes?: DockerVolume[]; + /** + * Where to mount the specified volumes from + * @see https://docs.docker.com/engine/reference/commandline/run/#mount-volumes-from-container---volumes-from + * @default - no containers are specified to mount volumes from + */ + readonly volumesFrom?: string[]; + /** * The environment variables to pass to the Docker container. * @@ -210,6 +217,9 @@ export class BundlingDockerImage { ...options.user ? ['-u', options.user] : [], + ...options.volumesFrom + ? flatten(options.volumesFrom.map(v => ['--volumes-from', v])) + : [], ...flatten(volumes.map(v => ['-v', `${v.hostPath}:${v.containerPath}:${isSeLinux() ? 'z,' : ''}${v.consistency ?? DockerVolumeConsistency.DELEGATED}`])), ...flatten(Object.entries(environment).map(([k, v]) => ['--env', `${k}=${v}`])), ...options.workingDirectory @@ -441,6 +451,13 @@ export interface DockerRunOptions { */ readonly volumes?: DockerVolume[]; + /** + * Where to mount the specified volumes from + * @see https://docs.docker.com/engine/reference/commandline/run/#mount-volumes-from-container---volumes-from + * @default - no containers are specified to mount volumes from + */ + readonly volumesFrom?: string[]; + /** * The environment variables to pass to the container. * diff --git a/packages/@aws-cdk/core/test/bundling.test.ts b/packages/@aws-cdk/core/test/bundling.test.ts index 49f26d8dab7b2..3879d0b27fc41 100644 --- a/packages/@aws-cdk/core/test/bundling.test.ts +++ b/packages/@aws-cdk/core/test/bundling.test.ts @@ -438,6 +438,44 @@ describe('bundling', () => { ], { stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true); }); + test('adding user provided docker volume options', () => { + // GIVEN + sinon.stub(process, 'platform').value('darwin'); + const spawnSyncStub = sinon.stub(child_process, 'spawnSync').returns({ + status: 1, + stderr: Buffer.from('stderr'), + stdout: Buffer.from('stdout'), + pid: 123, + output: ['stdout', 'stderr'], + signal: null, + }); + const image = DockerImage.fromRegistry('alpine'); + + try { + image.run({ + command: ['cool', 'command'], + volumesFrom: ['foo', 'bar'], + volumes: [{ hostPath: '/host-path', containerPath: '/container-path' }], + workingDirectory: '/working-directory', + user: 'user:group', + }); + } catch (e) { + // We expect this to fail as the test environment will not have the required docker setup for the command to exit successfully + // nevertheless what we want to check here is that the command was built correctly and triggered + }; + + expect(spawnSyncStub.calledWith('docker', [ + 'run', '--rm', + '-u', 'user:group', + '--volumes-from', 'foo', + '--volumes-from', 'bar', + '-v', '/host-path:/container-path:delegated', + '-w', '/working-directory', + 'alpine', + 'cool', 'command', + ], { stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true); + }); + test('ensure selinux docker mount', () => { // GIVEN sinon.stub(process, 'platform').value('linux');