Skip to content

Commit

Permalink
fix(core): asset hash of symlinked dir is wrong (#16429)
Browse files Browse the repository at this point in the history
When the root directory of an asset is a symlink (such as can happen in
CDK Pipelines), the asset hash calculation incorrectly doesn't follow
the symlink and hashes the link itself it instead.

This leads to the asset hash never changing, even though the files
inside the directory do change.

Instead, we resolve the asset root dir, and make sure to hash the target
directory on disk. Handling of symlinks found *inside* the target dir
remains unchanged.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
rix0rrr committed Sep 9, 2021
1 parent cc74f92 commit 36ff738
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 10 deletions.
10 changes: 8 additions & 2 deletions packages/@aws-cdk/core/lib/fs/fingerprint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,14 @@ export function fingerprint(fileOrDirectory: string, options: FingerprintOptions
const follow = options.follow || SymlinkFollowMode.EXTERNAL;
_hashField(hash, 'options.follow', follow);

const rootDirectory = fs.statSync(fileOrDirectory).isDirectory()
// Resolve symlinks in the initial path (for example, the root directory
// might be symlinked). It's important that we know the absolute path, so we
// can judge if further symlinks inside the target directory are within the
// target or not (if we don't resolve, we would test w.r.t. the wrong path).
fileOrDirectory = fs.realpathSync(fileOrDirectory);

const isDir = fs.statSync(fileOrDirectory).isDirectory();
const rootDirectory = isDir
? fileOrDirectory
: path.dirname(fileOrDirectory);

Expand All @@ -41,7 +48,6 @@ export function fingerprint(fileOrDirectory: string, options: FingerprintOptions
}

const ignoreStrategy = IgnoreStrategy.fromCopyOptions(options, fileOrDirectory);
const isDir = fs.statSync(fileOrDirectory).isDirectory();
_processFileOrDirectory(fileOrDirectory, isDir);

return hash.digest('hex');
Expand Down
35 changes: 27 additions & 8 deletions packages/@aws-cdk/core/test/staging.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ 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 userInfo = os.userInfo();
Expand All @@ -42,18 +43,36 @@ describe('staging', () => {
test('base case', () => {
// GIVEN
const stack = new Stack();
const sourcePath = path.join(__dirname, 'fs', 'fixtures', 'test1');
const sourcePath = FIXTURE_TEST1_DIR;

// WHEN
const staging = new AssetStaging(stack, 's1', { sourcePath });

expect(staging.sourceHash).toEqual('2f37f937c51e2c191af66acf9b09f548926008ec68c575bd2ee54b6e997c0e00');
expect(staging.sourceHash).toEqual(FIXTURE_TEST1_HASH);
expect(staging.sourcePath).toEqual(sourcePath);
expect(path.basename(staging.stagedPath)).toEqual('asset.2f37f937c51e2c191af66acf9b09f548926008ec68c575bd2ee54b6e997c0e00');
expect(path.basename(staging.relativeStagedPath(stack))).toEqual('asset.2f37f937c51e2c191af66acf9b09f548926008ec68c575bd2ee54b6e997c0e00');
expect(path.basename(staging.stagedPath)).toEqual(`asset.${FIXTURE_TEST1_HASH}`);
expect(path.basename(staging.relativeStagedPath(stack))).toEqual(`asset.${FIXTURE_TEST1_HASH}`);
expect(staging.packaging).toEqual(FileAssetPackaging.ZIP_DIRECTORY);
expect(staging.isArchive).toEqual(true);
});

test('base case if source directory is a symlink', () => {
// GIVEN
const stack = new Stack();
const sourcePath = path.join(os.tmpdir(), 'asset-symlink');
if (fs.existsSync(sourcePath)) { fs.unlinkSync(sourcePath); }
fs.symlinkSync(FIXTURE_TEST1_DIR, sourcePath);

try {
const staging = new AssetStaging(stack, 's1', { sourcePath });

// Should be the same asset hash as in the previous test
expect(staging.assetHash).toEqual(FIXTURE_TEST1_HASH);
} finally {
if (fs.existsSync(sourcePath)) {
fs.unlinkSync(sourcePath);
}
}
});

test('staging of an archive file correctly sets packaging and isArchive', () => {
Expand Down Expand Up @@ -141,7 +160,7 @@ describe('staging', () => {
// WHEN
const staging = new AssetStaging(stack, 's1', { sourcePath });

expect(staging.sourceHash).toEqual('2f37f937c51e2c191af66acf9b09f548926008ec68c575bd2ee54b6e997c0e00');
expect(staging.sourceHash).toEqual(FIXTURE_TEST1_HASH);
expect(staging.sourcePath).toEqual(sourcePath);
expect(staging.stagedPath).toEqual(sourcePath);
expect(staging.relativeStagedPath(stack)).toEqual(sourcePath);
Expand All @@ -160,7 +179,7 @@ describe('staging', () => {
// THEN
const assembly = app.synth();
expect(fs.readdirSync(assembly.directory)).toEqual([
'asset.2f37f937c51e2c191af66acf9b09f548926008ec68c575bd2ee54b6e997c0e00',
`asset.${FIXTURE_TEST1_HASH}`,
'asset.af10ac04b3b607b0f8659c8f0cee8c343025ee75baf0b146f10f0e5311d2c46b.gz',
'cdk.out',
'manifest.json',
Expand All @@ -187,7 +206,7 @@ describe('staging', () => {
expect(fs.readdirSync(assembly.directory)).toEqual([
'assembly-Stage1',
'assembly-Stage2',
'asset.2f37f937c51e2c191af66acf9b09f548926008ec68c575bd2ee54b6e997c0e00',
`asset.${FIXTURE_TEST1_HASH}`,
'cdk.out',
'manifest.json',
'tree.json',
Expand All @@ -207,7 +226,7 @@ describe('staging', () => {

// THEN
expect(withoutExtra.sourceHash).not.toEqual(withExtra.sourceHash);
expect(withoutExtra.sourceHash).toEqual('2f37f937c51e2c191af66acf9b09f548926008ec68c575bd2ee54b6e997c0e00');
expect(withoutExtra.sourceHash).toEqual(FIXTURE_TEST1_HASH);
expect(withExtra.sourceHash).toEqual('c95c915a5722bb9019e2c725d11868e5a619b55f36172f76bcbcaa8bb2d10c5f');

});
Expand Down

0 comments on commit 36ff738

Please sign in to comment.