Skip to content

Commit 93c9728

Browse files
cgattcgatt
committed
move user determination behaviour back to assetStaging
1 parent c36ec02 commit 93c9728

File tree

5 files changed

+99
-66
lines changed

5 files changed

+99
-66
lines changed

‎packages/aws-cdk-lib/core/lib/asset-staging.ts‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import * as crypto from 'crypto';
2+
import * as os from 'os';
23
import * as path from 'path';
34
import { Construct } from 'constructs';
45
import * as fs from 'fs-extra';
@@ -451,7 +452,11 @@ export class AssetStaging extends Construct {
451452
sourcePath: this.sourcePath,
452453
bundleDir,
453454
...options,
455+
securityOpt: options.securityOpt ?? '',
456+
user: options.user ?? this.determineUser(),
454457
volumes: [...(options.volumes ?? [])],
458+
workingDirectory:
459+
options.workingDirectory ?? AssetStaging.BUNDLING_INPUT_DIR,
455460
};
456461

457462
// Add the asset input and output volumes based on BundlingFileAccess setting
@@ -483,12 +488,7 @@ export class AssetStaging extends Construct {
483488
break;
484489
}
485490

486-
assetStagingOptions.image.run({
487-
workingDirectory:
488-
assetStagingOptions.workingDirectory ?? AssetStaging.BUNDLING_INPUT_DIR,
489-
securityOpt: assetStagingOptions.securityOpt ?? '',
490-
...assetStagingOptions,
491-
});
491+
assetStagingOptions.image.run(assetStagingOptions);
492492
}
493493
} catch (err) {
494494
// When bundling fails, keep the bundle output for diagnosability, but
@@ -552,6 +552,17 @@ export class AssetStaging extends Construct {
552552
}
553553
return targetPath;
554554
}
555+
556+
/**
557+
* Determines a useful default user if not given otherwise
558+
*/
559+
private determineUser(): string {
560+
// Default to current user
561+
const userInfo = os.userInfo();
562+
return userInfo.uid !== -1 // uid is -1 on Windows
563+
? `${userInfo.uid}:${userInfo.gid}`
564+
: '1000:1000';
565+
}
555566
}
556567

557568
function renderAssetFilename(assetHash: string, extension = '') {

‎packages/aws-cdk-lib/core/lib/bundling.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -279,8 +279,8 @@ export class BundlingDockerImage {
279279
...options.platform
280280
? ['--platform', options.platform]
281281
: [],
282-
...volumeHelper.user
283-
? ['-u', volumeHelper.user]
282+
...options.user
283+
? ['-u', options.user]
284284
: [],
285285
...volumeHelper.volumeCommands,
286286
...flatten(Object.entries(environment).map(([k, v]) => ['--env', `${k}=${v}`])),

‎packages/aws-cdk-lib/core/lib/private/bundling.ts‎

Lines changed: 6 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import { spawnSync, SpawnSyncOptions } from 'child_process';
22
import * as crypto from 'crypto';
3-
import * as os from 'os';
43
import {
54
DockerRunOptions,
65
DockerVolume,
@@ -37,7 +36,6 @@ export interface DockerVolumes {
3736
*/
3837
export class DockerVolumeHelper {
3938
readonly volumeCommands: string[];
40-
readonly user?: string;
4139
readonly containerName?: string;
4240
readonly volumes: DockerVolumes;
4341

@@ -57,16 +55,17 @@ export class DockerVolumeHelper {
5755
}
5856
}
5957
if (this.volumes.volumeCopyVolumes.length > 0) {
60-
// TODO handle user properly
61-
this.user = this.determineUser();
6258
this.containerName = `copyContainer${crypto.randomBytes(12).toString('hex')}`;
6359
const copyVolumeCommands = this.volumes.volumeCopyVolumes.flatMap(volume => [
6460
'-v',
6561
// leading ':' is required for anonymous volume with opts
6662
volume.opts ? `:${volume.containerPath}:${volume.opts.join(',')}` : `${volume.containerPath}`,
6763
]);
68-
const chownCommands = this.volumes.volumeCopyVolumes
69-
.map(volume => `mkdir -p ${volume.containerPath} && chown -R ${this.user} ${volume.containerPath}`);
64+
const directoryCommands: string[] = [];
65+
for (let volume of this.volumes.volumeCopyVolumes) {
66+
directoryCommands.push(`mkdir -p ${volume.containerPath}`);
67+
if (options.user) directoryCommands.push(`chown -R ${options.user} ${volume.containerPath}`);
68+
}
7069
dockerExec([
7170
'run',
7271
'--name',
@@ -75,7 +74,7 @@ export class DockerVolumeHelper {
7574
'public.ecr.aws/docker/library/alpine',
7675
'sh',
7776
'-c',
78-
chownCommands.join(' && '),
77+
directoryCommands.join(' && '),
7978
]);
8079
try {
8180
this.copyInputVolumes();
@@ -84,8 +83,6 @@ export class DockerVolumeHelper {
8483
dockerExec(['rm', '-v', this.containerName]);
8584
throw e;
8685
}
87-
} else {
88-
this.user = this.options.user;
8986
}
9087
this.volumeCommands = this.buildVolumeCommands();
9188
};
@@ -124,24 +121,6 @@ export class DockerVolumeHelper {
124121
return volumeCommands;
125122
}
126123

127-
/**
128-
* Determines a useful default user if not given otherwise
129-
*/
130-
private determineUser(): string {
131-
let user;
132-
if (this.options.user) {
133-
user = this.options.user;
134-
} else {
135-
// Default to current user
136-
const userInfo = os.userInfo();
137-
user =
138-
userInfo.uid !== -1 // uid is -1 on Windows
139-
? `${userInfo.uid}:${userInfo.gid}`
140-
: '1000:1000';
141-
}
142-
return user;
143-
}
144-
145124
/**
146125
* copy files from the host where this is executed into the input volume
147126
* @param sourcePath - path to folder where files should be copied from - without trailing slash

‎packages/aws-cdk-lib/core/test/bundling.test.ts‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -807,6 +807,59 @@ describe('bundling', () => {
807807
]), { encoding: 'utf-8', stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);
808808
});
809809

810+
test('can handle copy volume without user', () => {
811+
// GIVEN
812+
sinon.stub(process, 'platform').value('darwin');
813+
const spawnSyncStub = sinon.stub(child_process, 'spawnSync').returns({
814+
status: 0,
815+
stderr: Buffer.from('stderr'),
816+
stdout: Buffer.from('stdout'),
817+
pid: 123,
818+
output: ['stdout', 'stderr'],
819+
signal: null,
820+
});
821+
822+
// WHEN
823+
const image = DockerImage.fromRegistry('alpine');
824+
image.run({
825+
command: ['cool', 'command'],
826+
volumes: [{
827+
dockerVolumeType: DockerVolumeType.VOLUME_COPY,
828+
containerPath: '/container/path/1',
829+
hostInputPath: '/host/input/1',
830+
}, {
831+
dockerVolumeType: DockerVolumeType.VOLUME_COPY,
832+
containerPath: '/container/path/2',
833+
hostInputPath: '/host/input/2',
834+
opts: ['opt1', 'opt2'],
835+
}],
836+
workingDirectory: '/working-directory',
837+
});
838+
839+
// copy container volume opts are correct
840+
expect(spawnSyncStub.calledWith(dockerCmd, sinon.match([
841+
'run',
842+
'--name', sinon.match(/copyContainer.*/g),
843+
'-v', '/container/path/1',
844+
'-v', ':/container/path/2:opt1,opt2',
845+
'public.ecr.aws/docker/library/alpine',
846+
'sh',
847+
'-c',
848+
[
849+
'mkdir -p /container/path/1',
850+
'mkdir -p /container/path/2',
851+
].join(' && '),
852+
]), { encoding: 'utf-8', stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);
853+
854+
// main image run volume commands are correct
855+
expect(spawnSyncStub.calledWith(dockerCmd, sinon.match([
856+
'run', '--rm',
857+
'--volumes-from', sinon.match(/copyContainer.*/g),
858+
'-w', '/working-directory',
859+
'alpine', 'cool', 'command',
860+
]), { encoding: 'utf-8', stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);
861+
});
862+
810863
test('cleans up volume helper', () => {
811864
// GIVEN
812865
sinon.stub(process, 'platform').value('darwin');

‎packages/aws-cdk-lib/core/test/staging.test.ts‎

Lines changed: 21 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,7 @@ import * as fs from 'fs-extra';
55
import * as sinon from 'sinon';
66
import { FileAssetPackaging } from '../../cloud-assembly-schema';
77
import * as cxapi from '../../cx-api';
8-
import {
9-
App,
10-
AssetHashType,
11-
AssetStaging,
12-
BundlingFileAccess,
13-
BundlingOptions,
14-
BundlingOutput,
15-
DockerImage,
16-
FileSystem,
17-
Stack,
18-
Stage,
19-
} from '../lib';
8+
import { App, AssetHashType, AssetStaging, DockerImage, BundlingOptions, BundlingOutput, FileSystem, Stack, Stage, BundlingFileAccess } from '../lib';
209

2110
const STUB_INPUT_FILE = '/tmp/docker-stub.input';
2211
const STUB_INPUT_CONCAT_FILE = '/tmp/docker-stub.input.concat';
@@ -322,7 +311,7 @@ describe('staging', () => {
322311
const assembly = app.synth();
323312
expect(
324313
readDockerStubInput()).toEqual(
325-
'run --rm -v /input:/asset-input:delegated -v /output:/asset-output:delegated -w /asset-input alpine DOCKER_STUB_SUCCESS',
314+
`run --rm ${USER_ARG} -v /input:/asset-input:delegated -v /output:/asset-output:delegated -w /asset-input alpine DOCKER_STUB_SUCCESS`,
326315
);
327316
expect(fs.readdirSync(assembly.directory)).toEqual([
328317
'asset.b1e32e86b3523f2fa512eb99180ee2975a50a4439e63e8badd153f2a68d61aa4',
@@ -401,7 +390,7 @@ describe('staging', () => {
401390
// We're testing that docker was run exactly once even though there are two bundling assets.
402391
expect(
403392
readDockerStubInputConcat()).toEqual(
404-
'run --rm -v /input:/asset-input:delegated -v /output:/asset-output:delegated -w /asset-input alpine DOCKER_STUB_SUCCESS',
393+
`run --rm ${USER_ARG} -v /input:/asset-input:delegated -v /output:/asset-output:delegated -w /asset-input alpine DOCKER_STUB_SUCCESS`,
405394
);
406395

407396
expect(fs.readdirSync(assembly.directory)).toEqual([
@@ -446,7 +435,7 @@ describe('staging', () => {
446435
// and that the hash is based on the output
447436
expect(
448437
readDockerStubInputConcat()).toEqual(
449-
'run --rm -v /input:/asset-input:delegated -v /output:/asset-output:delegated -w /asset-input alpine DOCKER_STUB_SUCCESS',
438+
`run --rm ${USER_ARG} -v /input:/asset-input:delegated -v /output:/asset-output:delegated -w /asset-input alpine DOCKER_STUB_SUCCESS`,
450439
);
451440

452441
expect(fs.readdirSync(assembly.directory)).toEqual([
@@ -494,8 +483,8 @@ describe('staging', () => {
494483
// operating on the same source asset.
495484
expect(
496485
readDockerStubInputConcat()).toEqual(
497-
'run --rm -v /input:/asset-input:delegated -v /output:/asset-output:delegated -w /asset-input alpine DOCKER_STUB_SUCCESS\n' +
498-
'run --rm -v /input:/asset-input:delegated -v /output:/asset-output:delegated --env UNIQUE_ENV_VAR=SOMEVALUE -w /asset-input alpine DOCKER_STUB_SUCCESS',
486+
`run --rm ${USER_ARG} -v /input:/asset-input:delegated -v /output:/asset-output:delegated -w /asset-input alpine DOCKER_STUB_SUCCESS\n` +
487+
`run --rm ${USER_ARG} -v /input:/asset-input:delegated -v /output:/asset-output:delegated --env UNIQUE_ENV_VAR=SOMEVALUE -w /asset-input alpine DOCKER_STUB_SUCCESS`,
499488
);
500489

501490
expect(fs.readdirSync(assembly.directory)).toEqual([
@@ -543,7 +532,7 @@ describe('staging', () => {
543532
// We're testing that docker was run once, only for the first Asset, since the only difference is the token.
544533
expect(
545534
readDockerStubInputConcat()).toEqual(
546-
'run --rm -v /input:/asset-input:delegated -v /output:/asset-output:delegated --env PIP_INDEX_URL=https://aws:MY_SECRET_TOKEN@your-code-repo.d.codeartifact.us-west-2.amazonaws.com/pypi/python/simple/ -w /asset-input alpine DOCKER_STUB_SUCCESS',
535+
`run --rm ${USER_ARG} -v /input:/asset-input:delegated -v /output:/asset-output:delegated --env PIP_INDEX_URL=https://aws:MY_SECRET_TOKEN@your-code-repo.d.codeartifact.us-west-2.amazonaws.com/pypi/python/simple/ -w /asset-input alpine DOCKER_STUB_SUCCESS`,
547536
);
548537

549538
expect(fs.readdirSync(assembly.directory)).toEqual([
@@ -674,7 +663,7 @@ describe('staging', () => {
674663

675664
expect(
676665
readDockerStubInputConcat()).toEqual(
677-
'run --rm -v /input:/asset-input:delegated -v /output:/asset-output:delegated -w /asset-input alpine DOCKER_STUB_SUCCESS',
666+
`run --rm ${USER_ARG} -v /input:/asset-input:delegated -v /output:/asset-output:delegated -w /asset-input alpine DOCKER_STUB_SUCCESS`,
678667
);
679668

680669
expect(appAssembly.directory).toEqual(app2Assembly.directory);
@@ -736,7 +725,7 @@ describe('staging', () => {
736725

737726
expect(
738727
readDockerStubInputConcat()).toEqual(
739-
'run --rm -v /input:/asset-input:delegated -v /output:/asset-output:delegated --env PIP_EXTRA_INDEX_URL=https://aws:MY_SECRET_TOKEN@your-code-repo.d.codeartifact.us-west-2.amazonaws.com/pypi/python/simple/ -w /asset-input alpine DOCKER_STUB_SUCCESS',
728+
`run --rm ${USER_ARG} -v /input:/asset-input:delegated -v /output:/asset-output:delegated --env PIP_EXTRA_INDEX_URL=https://aws:MY_SECRET_TOKEN@your-code-repo.d.codeartifact.us-west-2.amazonaws.com/pypi/python/simple/ -w /asset-input alpine DOCKER_STUB_SUCCESS`,
740729
);
741730

742731
expect(appAssembly.directory).toEqual(app2Assembly.directory);
@@ -766,7 +755,7 @@ describe('staging', () => {
766755

767756
expect(
768757
readDockerStubInput()).toEqual(
769-
'run --rm -v /input:/asset-input:delegated -v /output:/asset-output:delegated -w /asset-input alpine DOCKER_STUB_SUCCESS_NO_OUTPUT',
758+
`run --rm ${USER_ARG} -v /input:/asset-input:delegated -v /output:/asset-output:delegated -w /asset-input alpine DOCKER_STUB_SUCCESS_NO_OUTPUT`,
770759
);
771760
});
772761

@@ -789,7 +778,7 @@ describe('staging', () => {
789778
// THEN
790779
expect(
791780
readDockerStubInput()).toEqual(
792-
'run --rm -v /input:/asset-input:delegated -v /output:/asset-output:delegated -w /asset-input alpine DOCKER_STUB_SUCCESS',
781+
`run --rm ${USER_ARG} -v /input:/asset-input:delegated -v /output:/asset-output:delegated -w /asset-input alpine DOCKER_STUB_SUCCESS`,
793782
);
794783
expect(asset.assetHash).toEqual('33cbf2cae5432438e0f046bc45ba8c3cef7b6afcf47b59d1c183775c1918fb1f');
795784
});
@@ -814,7 +803,7 @@ describe('staging', () => {
814803
// THEN
815804
expect(
816805
readDockerStubInput()).toEqual(
817-
'run --rm --security-opt no-new-privileges -v /input:/asset-input:delegated -v /output:/asset-output:delegated -w /asset-input alpine DOCKER_STUB_SUCCESS',
806+
`run --rm --security-opt no-new-privileges ${USER_ARG} -v /input:/asset-input:delegated -v /output:/asset-output:delegated -w /asset-input alpine DOCKER_STUB_SUCCESS`,
818807
);
819808
expect(asset.assetHash).toEqual('33cbf2cae5432438e0f046bc45ba8c3cef7b6afcf47b59d1c183775c1918fb1f');
820809
});
@@ -839,7 +828,7 @@ describe('staging', () => {
839828
// THEN
840829
expect(
841830
readDockerStubInput()).toEqual(
842-
'run --rm -v /input:/asset-input:delegated -v /output:/asset-output:delegated -w /asset-input --entrypoint DOCKER_STUB_SUCCESS alpine DOCKER_STUB_SUCCESS',
831+
`run --rm ${USER_ARG} -v /input:/asset-input:delegated -v /output:/asset-output:delegated -w /asset-input --entrypoint DOCKER_STUB_SUCCESS alpine DOCKER_STUB_SUCCESS`,
843832
);
844833
expect(asset.assetHash).toEqual('33cbf2cae5432438e0f046bc45ba8c3cef7b6afcf47b59d1c183775c1918fb1f');
845834
});
@@ -957,7 +946,7 @@ describe('staging', () => {
957946
})).toThrow(/Failed to bundle asset stack\/Asset/);
958947
expect(
959948
readDockerStubInput()).toEqual(
960-
'run --rm -v /input:/asset-input:delegated -v /output:/asset-output:delegated -w /asset-input this-is-an-invalid-docker-image DOCKER_STUB_FAIL',
949+
`run --rm ${USER_ARG} -v /input:/asset-input:delegated -v /output:/asset-output:delegated -w /asset-input this-is-an-invalid-docker-image DOCKER_STUB_FAIL`,
961950
);
962951
});
963952

@@ -1248,7 +1237,7 @@ describe('staging', () => {
12481237

12491238
expect(
12501239
readDockerStubInput()).toEqual(
1251-
'run --rm -v /input:/asset-input:delegated -v /output:/asset-output:delegated -w /asset-input alpine DOCKER_STUB_SUCCESS',
1240+
`run --rm ${USER_ARG} -v /input:/asset-input:delegated -v /output:/asset-output:delegated -w /asset-input alpine DOCKER_STUB_SUCCESS`,
12521241
);
12531242
expect(asset.assetHash).toEqual('33cbf2cae5432438e0f046bc45ba8c3cef7b6afcf47b59d1c183775c1918fb1f'); // hash of MyStack/Asset
12541243
});
@@ -1272,7 +1261,7 @@ describe('staging', () => {
12721261

12731262
expect(
12741263
readDockerStubInput()).toEqual(
1275-
'run --rm -v /input:/asset-input:delegated -v /output:/asset-output:delegated -w /asset-input alpine DOCKER_STUB_SUCCESS',
1264+
`run --rm ${USER_ARG} -v /input:/asset-input:delegated -v /output:/asset-output:delegated -w /asset-input alpine DOCKER_STUB_SUCCESS`,
12761265
);
12771266
expect(asset.assetHash).toEqual('33cbf2cae5432438e0f046bc45ba8c3cef7b6afcf47b59d1c183775c1918fb1f'); // hash of MyStack/Asset
12781267
});
@@ -1656,11 +1645,12 @@ describe('staging with docker cp', () => {
16561645

16571646
// Reads a docker stub and cleans the volume paths out of the stub.
16581647
function readAndCleanDockerStubInput(file: string) {
1659-
return fs
1660-
.readFileSync(file, 'utf-8')
1648+
const commands = fs
1649+
.readFileSync(file, 'utf-8');
1650+
return commands
16611651
.trim()
1662-
.replace(/-v ([^:]+):\/asset-input/g, '-v /input:/asset-input')
1663-
.replace(/-v ([^:]+):\/asset-output/g, '-v /output:/asset-output');
1652+
.replace(/-v ([^:\n]+):\/asset-input/g, '-v /input:/asset-input')
1653+
.replace(/-v ([^:\n]+):\/asset-output/g, '-v /output:/asset-output');
16641654
}
16651655

16661656
// Last docker input since last teardown

0 commit comments

Comments
 (0)