Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(s3-assets): cannot publish a file without extension #30597

Merged
merged 8 commits into from
Oct 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,17 @@ class TestStack extends Stack {

const user = new iam.User(this, 'MyUser');
asset.grantRead(user);

new assets.Asset(this, 'BundledAssetWithoutExtension', {
path: path.join(__dirname, 'markdown-asset'),
bundling: {
image: DockerImage.fromBuild(path.join(__dirname, 'alpine-markdown')),
outputType: BundlingOutput.SINGLE_FILE,
command: [
'sh', '-c', 'echo 123 > /asset-output/main',
],
},
});
}
}

Expand Down
21 changes: 18 additions & 3 deletions packages/aws-cdk-lib/core/lib/asset-staging.ts
Original file line number Diff line number Diff line change
Expand Up @@ -278,9 +278,10 @@ export class AssetStaging extends Construct {
*/
private stageByCopying(): StagedAsset {
const assetHash = this.calculateHash(this.hashType);
const stagedPath = this.stagingDisabled
const targetPath = this.stagingDisabled
? this.sourcePath
: path.resolve(this.assetOutdir, renderAssetFilename(assetHash, getExtension(this.sourcePath)));
const stagedPath = this.renderStagedPath(this.sourcePath, targetPath);

if (!this.sourceStats.isDirectory() && !this.sourceStats.isFile()) {
throw new Error(`Asset ${this.sourcePath} is expected to be either a directory or a regular file`);
Expand Down Expand Up @@ -338,7 +339,10 @@ export class AssetStaging extends Construct {
// Calculate assetHash afterwards if we still must
assetHash = assetHash ?? this.calculateHash(this.hashType, bundling, bundledAsset.path);

const stagedPath = path.resolve(this.assetOutdir, renderAssetFilename(assetHash, bundledAsset.extension));
const stagedPath = this.renderStagedPath(
bundledAsset.path,
path.resolve(this.assetOutdir, renderAssetFilename(assetHash, bundledAsset.extension)),
);

this.stageAsset(bundledAsset.path, stagedPath, 'move');

Expand Down Expand Up @@ -388,7 +392,7 @@ export class AssetStaging extends Construct {
}

// Moving can be done quickly
if (style == 'move') {
if (style === 'move') {
fs.renameSync(sourcePath, targetPath);
return;
}
Expand Down Expand Up @@ -511,6 +515,17 @@ export class AssetStaging extends Construct {
throw new Error('Unknown asset hash type.');
}
}

private renderStagedPath(sourcePath: string, targetPath: string): string {
// Add a suffix to the asset file name
// because when a file without extension is specified, the source directory name is the same as the staged asset file name.
// But when the hashType is `AssetHashType.OUTPUT`, the source directory name begins with `bundling-temp-` and the staged asset file name is different.
// We only need to add a suffix when the hashType is not `AssetHashType.OUTPUT`.
if (this.hashType !== AssetHashType.OUTPUT && path.dirname(sourcePath) === targetPath) {
targetPath = targetPath + '_noext';
}
return targetPath;
}
}

function renderAssetFilename(assetHash: string, extension = '') {
Expand Down
11 changes: 11 additions & 0 deletions packages/aws-cdk-lib/core/test/docker-stub-cp.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,17 @@ set -euo pipefail
echo "$@" >> /tmp/docker-stub-cp.input.concat
echo "$@" > /tmp/docker-stub-cp.input

# create a file without extension to emulate created files, fetch the target path from the "docker cp" command
if cat /tmp/docker-stub-cp.input.concat | grep "DOCKER_STUB_SINGLE_FILE_WITHOUT_EXT"; then
if echo "$@" | grep "cp"| grep "/asset-output"; then
outdir=$(echo "$@" | grep cp | grep "/asset-output" | xargs -n1 | grep "cdk.out" | head -n1 | cut -d":" -f1)
if [ -n "$outdir" ]; then
touch "${outdir}/test" # create a file witout extension
exit 0
fi
fi
fi

# create a fake zip to emulate created files, fetch the target path from the "docker cp" command
if echo "$@" | grep "cp"| grep "/asset-output"; then
outdir=$(echo "$@" | grep cp | grep "/asset-output" | xargs -n1 | grep "cdk.out" | head -n1 | cut -d":" -f1)
Expand Down
6 changes: 6 additions & 0 deletions packages/aws-cdk-lib/core/test/docker-stub.sh
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ if echo "$@" | grep "DOCKER_STUB_SINGLE_ARCHIVE"; then
exit 0
fi

if echo "$@" | grep "DOCKER_STUB_SINGLE_FILE_WITHOUT_EXT"; then
outdir=$(echo "$@" | xargs -n1 | grep "/asset-output" | head -n1 | cut -d":" -f1)
touch ${outdir}/test # create a file witout extension
exit 0
fi

if echo "$@" | grep "DOCKER_STUB_SINGLE_FILE"; then
outdir=$(echo "$@" | xargs -n1 | grep "/asset-output" | head -n1 | cut -d":" -f1)
touch ${outdir}/test.txt
Expand Down
128 changes: 128 additions & 0 deletions packages/aws-cdk-lib/core/test/staging.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ enum DockerStubCommand {
MULTIPLE_FILES = 'DOCKER_STUB_MULTIPLE_FILES',
SINGLE_ARCHIVE = 'DOCKER_STUB_SINGLE_ARCHIVE',
SINGLE_FILE = 'DOCKER_STUB_SINGLE_FILE',
SINGLE_FILE_WITHOUT_EXT = 'DOCKER_STUB_SINGLE_FILE_WITHOUT_EXT',
VOLUME_SINGLE_ARCHIVE = 'DOCKER_STUB_VOLUME_SINGLE_ARCHIVE',
}

Expand Down Expand Up @@ -1450,6 +1451,68 @@ describe('staging', () => {
expect(staging.isArchive).toEqual(false);
});

test('bundling that produces a single file with SINGLE_FILE_WITHOUT_EXT and hash type SOURCE', () => {
// GIVEN
const app = new App({ context: { [cxapi.NEW_STYLE_STACK_SYNTHESIS_CONTEXT]: false } });
const stack = new Stack(app, 'stack');
const directory = path.join(__dirname, 'fs', 'fixtures', 'test1');

// WHEN
const staging = new AssetStaging(stack, 'Asset', {
sourcePath: directory,
bundling: {
image: DockerImage.fromRegistry('alpine'),
command: [DockerStubCommand.SINGLE_FILE_WITHOUT_EXT],
outputType: BundlingOutput.SINGLE_FILE,
},
assetHashType: AssetHashType.SOURCE, // default
});

// THEN
const assembly = app.synth();
expect(fs.readdirSync(assembly.directory)).toEqual([
'asset.ef734136dc22840a94140575a2f98cbc061074e09535589d1cd2c11a4ac2fd75',
'asset.ef734136dc22840a94140575a2f98cbc061074e09535589d1cd2c11a4ac2fd75_noext',
'cdk.out',
'manifest.json',
'stack.template.json',
'tree.json',
]);
expect(staging.packaging).toEqual(FileAssetPackaging.FILE);
expect(staging.isArchive).toEqual(false);
});

test('bundling that produces a single file with SINGLE_FILE_WITHOUT_EXT and hash type CUSTOM', () => {
// GIVEN
const app = new App({ context: { [cxapi.NEW_STYLE_STACK_SYNTHESIS_CONTEXT]: false } });
const stack = new Stack(app, 'stack');
const directory = path.join(__dirname, 'fs', 'fixtures', 'test1');

// WHEN
const staging = new AssetStaging(stack, 'Asset', {
sourcePath: directory,
bundling: {
image: DockerImage.fromRegistry('alpine'),
command: [DockerStubCommand.SINGLE_FILE_WITHOUT_EXT],
outputType: BundlingOutput.SINGLE_FILE,
},
assetHashType: AssetHashType.CUSTOM,
assetHash: 'custom',
});

// THEN
const assembly = app.synth();
expect(fs.readdirSync(assembly.directory)).toEqual([
'asset.f81c5ba9e81eebb202881a8e61a83ab4b69f6bee261989eb93625c9cf5d35335',
'asset.f81c5ba9e81eebb202881a8e61a83ab4b69f6bee261989eb93625c9cf5d35335_noext',
'cdk.out',
'manifest.json',
'stack.template.json',
'tree.json',
]);
expect(staging.packaging).toEqual(FileAssetPackaging.FILE);
expect(staging.isArchive).toEqual(false);
});
});

describe('staging with docker cp', () => {
Expand Down Expand Up @@ -1517,6 +1580,71 @@ describe('staging with docker cp', () => {
expect.stringContaining('volume rm assetOutput'),
]));
});

test('bundling that produces a single file with docker image copy variant and hash type SOURCE', () => {
// GIVEN
const app = new App({ context: { [cxapi.NEW_STYLE_STACK_SYNTHESIS_CONTEXT]: false } });
const stack = new Stack(app, 'stack');
const directory = path.join(__dirname, 'fs', 'fixtures', 'test1');

// WHEN
const staging = new AssetStaging(stack, 'Asset', {
sourcePath: directory,
bundling: {
image: DockerImage.fromRegistry('alpine'),
command: [DockerStubCommand.SINGLE_FILE_WITHOUT_EXT],
outputType: BundlingOutput.SINGLE_FILE,
bundlingFileAccess: BundlingFileAccess.VOLUME_COPY,
},
assetHashType: AssetHashType.SOURCE, // default
});

// THEN
const assembly = app.synth();
expect(fs.readdirSync(assembly.directory)).toEqual([
'asset.93bd4079bff7440a725991ecf249416ae9ad73cb639f4a8d9e8f3ad8d491e89f',
'asset.93bd4079bff7440a725991ecf249416ae9ad73cb639f4a8d9e8f3ad8d491e89f_noext',
'cdk.out',
'manifest.json',
'stack.template.json',
'tree.json',
]);
expect(staging.packaging).toEqual(FileAssetPackaging.FILE);
expect(staging.isArchive).toEqual(false);
});

test('bundling that produces a single file with docker image copy variant and hash type CUSTOM', () => {
// GIVEN
const app = new App({ context: { [cxapi.NEW_STYLE_STACK_SYNTHESIS_CONTEXT]: false } });
const stack = new Stack(app, 'stack');
const directory = path.join(__dirname, 'fs', 'fixtures', 'test1');

// WHEN
const staging = new AssetStaging(stack, 'Asset', {
sourcePath: directory,
bundling: {
image: DockerImage.fromRegistry('alpine'),
command: [DockerStubCommand.SINGLE_FILE_WITHOUT_EXT],
outputType: BundlingOutput.SINGLE_FILE,
bundlingFileAccess: BundlingFileAccess.VOLUME_COPY,
},
assetHashType: AssetHashType.CUSTOM,
assetHash: 'custom',
});

// THEN
const assembly = app.synth();
expect(fs.readdirSync(assembly.directory)).toEqual([
'asset.53a51b4c68874a8e831e24e8982120be2a608f50b2e05edb8501143b3305baa8',
'asset.53a51b4c68874a8e831e24e8982120be2a608f50b2e05edb8501143b3305baa8_noext',
'cdk.out',
'manifest.json',
'stack.template.json',
'tree.json',
]);
expect(staging.packaging).toEqual(FileAssetPackaging.FILE);
expect(staging.isArchive).toEqual(false);
});
});

// Reads a docker stub and cleans the volume paths out of the stub.
Expand Down