Skip to content

Commit

Permalink
fix(aws-s3-assets): support asset url with two extension name like ta…
Browse files Browse the repository at this point in the history
…r.gz (#20874)

using aws-s3-assets to upload data artifacts of extension `tar.gz` returns an uploaded asset renamed to `<random Id>.gz`. 

This PR proposes that the AssetStaging Object should be able to check if the uploaded artifact is a `tar.gz` or any other archive tar file with a compression extension and return the appropriate extension name as stagedPath.

closes #12699

### 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)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
ntachukwu authored Jul 14, 2022
1 parent cd4851a commit 673b0d1
Show file tree
Hide file tree
Showing 9 changed files with 63 additions and 9 deletions.
2 changes: 1 addition & 1 deletion packages/@aws-cdk/assets/test/staging.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ describeDeprecated('staging', () => {
const assembly = app.synth();
expect(fs.readdirSync(assembly.directory)).toEqual([
'asset.2f37f937c51e2c191af66acf9b09f548926008ec68c575bd2ee54b6e997c0e00',
'asset.af10ac04b3b607b0f8659c8f0cee8c343025ee75baf0b146f10f0e5311d2c46b.gz',
'asset.af10ac04b3b607b0f8659c8f0cee8c343025ee75baf0b146f10f0e5311d2c46b.tar.gz',
'cdk.out',
'manifest.json',
'stack.template.json',
Expand Down
27 changes: 21 additions & 6 deletions packages/@aws-cdk/core/lib/asset-staging.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { Cache } from './private/cache';
import { Stack } from './stack';
import { Stage } from './stage';

const ARCHIVE_EXTENSIONS = ['.zip', '.jar'];
const ARCHIVE_EXTENSIONS = ['.tar.gz', '.zip', '.jar', '.tar', '.tgz'];

/**
* A previously staged asset
Expand Down Expand Up @@ -270,7 +270,7 @@ export class AssetStaging extends Construct {
const assetHash = this.calculateHash(this.hashType);
const stagedPath = this.stagingDisabled
? this.sourcePath
: path.resolve(this.assetOutdir, renderAssetFilename(assetHash, path.extname(this.sourcePath)));
: path.resolve(this.assetOutdir, renderAssetFilename(assetHash, getExtension(this.sourcePath)));

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 All @@ -282,7 +282,7 @@ export class AssetStaging extends Construct {
assetHash,
stagedPath,
packaging: this.sourceStats.isDirectory() ? FileAssetPackaging.ZIP_DIRECTORY : FileAssetPackaging.FILE,
isArchive: this.sourceStats.isDirectory() || ARCHIVE_EXTENSIONS.includes(path.extname(this.sourcePath).toLowerCase()),
isArchive: this.sourceStats.isDirectory() || ARCHIVE_EXTENSIONS.includes(getExtension(this.sourcePath).toLowerCase()),
};
}

Expand Down Expand Up @@ -577,7 +577,7 @@ function singleArchiveFile(directory: string): string | undefined {
const content = fs.readdirSync(directory);
if (content.length === 1) {
const file = path.join(directory, content[0]);
const extension = path.extname(content[0]).toLowerCase();
const extension = getExtension(content[0]).toLowerCase();
if (fs.statSync(file).isFile() && ARCHIVE_EXTENSIONS.includes(extension)) {
return file;
}
Expand Down Expand Up @@ -610,8 +610,23 @@ function determineBundledAsset(bundleDir: string, outputType: BundlingOutput): B
return { path: bundleDir, packaging: FileAssetPackaging.ZIP_DIRECTORY };
case BundlingOutput.ARCHIVED:
if (!archiveFile) {
throw new Error('Bundling output directory is expected to include only a single .zip or .jar file when `output` is set to `ARCHIVED`');
throw new Error('Bundling output directory is expected to include only a single archive file when `output` is set to `ARCHIVED`');
}
return { path: archiveFile, packaging: FileAssetPackaging.FILE, extension: path.extname(archiveFile) };
return { path: archiveFile, packaging: FileAssetPackaging.FILE, extension: getExtension(archiveFile) };
}
}

/**
* Return the extension name of a source path
*
* Loop through ARCHIVE_EXTENSIONS for valid archive extensions.
*/
function getExtension(source: string): string {
for ( const ext of ARCHIVE_EXTENSIONS ) {
if (source.toLowerCase().endsWith(ext)) {
return ext;
};
};

return path.extname(source);
};
Empty file.
Binary file not shown.
Binary file added packages/@aws-cdk/core/test/archive/artifact.tar
Binary file not shown.
Binary file added packages/@aws-cdk/core/test/archive/artifact.tar.gz
Binary file not shown.
Binary file added packages/@aws-cdk/core/test/archive/artifact.tgz
Binary file not shown.
Empty file.
43 changes: 41 additions & 2 deletions packages/@aws-cdk/core/test/staging.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ enum DockerStubCommand {
const FIXTURE_TEST1_DIR = path.join(__dirname, 'fs', 'fixtures', 'test1');
const FIXTURE_TEST1_HASH = '2f37f937c51e2c191af66acf9b09f548926008ec68c575bd2ee54b6e997c0e00';
const FIXTURE_TARBALL = path.join(__dirname, 'fs', 'fixtures.tar.gz');
const NOT_ARCHIVED_ZIP_TXT_HASH = '95c924c84f5d023be4edee540cb2cb401a49f115d01ed403b288f6cb412771df';
const ARCHIVE_TARBALL_TEST_HASH = '3e948ff54a277d6001e2452fdbc4a9ef61f916ff662ba5e05ece1e2ec6dec9f5';

const userInfo = os.userInfo();
const USER_ARG = `-u ${userInfo.uid}:${userInfo.gid}`;
Expand Down Expand Up @@ -88,6 +90,43 @@ describe('staging', () => {
expect(staging.isArchive).toEqual(true);
});

test('staging of an archive with multiple extension name correctly sets packaging and isArchive', () => {
// GIVEN
const stack = new Stack();
const sourcePathTarGz1 = path.join(__dirname, 'archive', 'artifact.tar.gz');
const sourcePathTarGz2 = path.join(__dirname, 'archive', 'artifact.da.vinci.monalisa.tar.gz');
const sourcePathTgz = path.join(__dirname, 'archive', 'artifact.tgz');
const sourcePathTar = path.join(__dirname, 'archive', 'artifact.tar');
const sourcePathNotArchive = path.join(__dirname, 'archive', 'artifact.zip.txt');
const sourcePathDockerFile = path.join(__dirname, 'archive', 'DockerFile');

// WHEN
const stagingTarGz1 = new AssetStaging(stack, 's1', { sourcePath: sourcePathTarGz1 });
const stagingTarGz2 = new AssetStaging(stack, 's2', { sourcePath: sourcePathTarGz2 });
const stagingTgz = new AssetStaging(stack, 's3', { sourcePath: sourcePathTgz });
const stagingTar = new AssetStaging(stack, 's4', { sourcePath: sourcePathTar });
const stagingNotArchive = new AssetStaging(stack, 's5', { sourcePath: sourcePathNotArchive });
const stagingDockerFile = new AssetStaging(stack, 's6', { sourcePath: sourcePathDockerFile });

expect(stagingTarGz1.packaging).toEqual(FileAssetPackaging.FILE);
expect(stagingTarGz1.isArchive).toEqual(true);
expect(stagingTarGz2.packaging).toEqual(FileAssetPackaging.FILE);
expect(path.basename(stagingTarGz2.absoluteStagedPath)).toEqual(`asset.${ARCHIVE_TARBALL_TEST_HASH}.tar.gz`);
expect(path.basename(stagingTarGz2.relativeStagedPath(stack))).toEqual(`asset.${ARCHIVE_TARBALL_TEST_HASH}.tar.gz`);
expect(stagingTarGz2.isArchive).toEqual(true);
expect(stagingTgz.packaging).toEqual(FileAssetPackaging.FILE);
expect(stagingTgz.isArchive).toEqual(true);
expect(stagingTar.packaging).toEqual(FileAssetPackaging.FILE);
expect(stagingTar.isArchive).toEqual(true);
expect(stagingNotArchive.packaging).toEqual(FileAssetPackaging.FILE);
expect(path.basename(stagingNotArchive.absoluteStagedPath)).toEqual(`asset.${NOT_ARCHIVED_ZIP_TXT_HASH}.txt`);
expect(path.basename(stagingNotArchive.relativeStagedPath(stack))).toEqual(`asset.${NOT_ARCHIVED_ZIP_TXT_HASH}.txt`);
expect(stagingNotArchive.isArchive).toEqual(false);
expect(stagingDockerFile.packaging).toEqual(FileAssetPackaging.FILE);
expect(stagingDockerFile.isArchive).toEqual(false);

});

test('asset packaging type is correct when staging is skipped because of memory cache', () => {
// GIVEN
const stack = new Stack();
Expand Down Expand Up @@ -175,7 +214,7 @@ describe('staging', () => {
const assembly = app.synth();
expect(fs.readdirSync(assembly.directory)).toEqual([
`asset.${FIXTURE_TEST1_HASH}`,
'asset.af10ac04b3b607b0f8659c8f0cee8c343025ee75baf0b146f10f0e5311d2c46b.gz',
'asset.af10ac04b3b607b0f8659c8f0cee8c343025ee75baf0b146f10f0e5311d2c46b.tar.gz',
'cdk.out',
'manifest.json',
'stack.template.json',
Expand Down Expand Up @@ -1080,7 +1119,7 @@ describe('staging', () => {
command: [DockerStubCommand.MULTIPLE_FILES],
outputType: BundlingOutput.ARCHIVED,
},
})).toThrow(/Bundling output directory is expected to include only a single .zip or .jar file when `output` is set to `ARCHIVED`/);
})).toThrow(/Bundling output directory is expected to include only a single archive file when `output` is set to `ARCHIVED`/);
});
});

Expand Down

0 comments on commit 673b0d1

Please sign in to comment.