From 4b1070862fd6c03615813af13f1e24faf6f53951 Mon Sep 17 00:00:00 2001 From: Jonathan Goldwasser Date: Wed, 14 Jul 2021 19:43:01 +0200 Subject: [PATCH] fix(ecr-assets): There is already a Construct with name 'Staging' when using tarball image (#15540) Use `this` and not `scope` for the scope of `AssetStaging` in `TarballImageAsset`. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../@aws-cdk/aws-ecr-assets/lib/tarball-asset.ts | 2 +- .../aws-ecr-assets/test/tarball-asset.test.ts | 13 ++++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/@aws-cdk/aws-ecr-assets/lib/tarball-asset.ts b/packages/@aws-cdk/aws-ecr-assets/lib/tarball-asset.ts index 48af505e1148e..bb7c40617b4ea 100644 --- a/packages/@aws-cdk/aws-ecr-assets/lib/tarball-asset.ts +++ b/packages/@aws-cdk/aws-ecr-assets/lib/tarball-asset.ts @@ -60,7 +60,7 @@ export class TarballImageAsset extends CoreConstruct implements IAsset { throw new Error(`Cannot find file at ${props.tarballFile}`); } - const stagedTarball = new AssetStaging(scope, 'Staging', { sourcePath: props.tarballFile }); + const stagedTarball = new AssetStaging(this, 'Staging', { sourcePath: props.tarballFile }); this.sourceHash = stagedTarball.assetHash; this.assetHash = stagedTarball.assetHash; diff --git a/packages/@aws-cdk/aws-ecr-assets/test/tarball-asset.test.ts b/packages/@aws-cdk/aws-ecr-assets/test/tarball-asset.test.ts index c4654fed87044..20d7517e23915 100644 --- a/packages/@aws-cdk/aws-ecr-assets/test/tarball-asset.test.ts +++ b/packages/@aws-cdk/aws-ecr-assets/test/tarball-asset.test.ts @@ -16,7 +16,7 @@ describe('image asset', () => { testFutureBehavior('test instantiating Asset Image', flags, App, (app) => { // GIVEN const stack = new Stack(app); - const assset = new TarballImageAsset(stack, 'Image', { + const asset = new TarballImageAsset(stack, 'Image', { tarballFile: __dirname + '/demo-tarball/empty.tar', }); @@ -30,23 +30,26 @@ describe('image asset', () => { expect(Object.keys(manifest.files ?? {}).length).toBe(1); expect(Object.keys(manifest.dockerImages ?? {}).length).toBe(1); - expect(manifest.dockerImages?.[assset.assetHash]?.destinations?.['current_account-current_region']).toStrictEqual( + expect(manifest.dockerImages?.[asset.assetHash]?.destinations?.['current_account-current_region']).toStrictEqual( { assumeRoleArn: 'arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-image-publishing-role-${AWS::AccountId}-${AWS::Region}', - imageTag: assset.assetHash, + imageTag: asset.assetHash, repositoryName: 'cdk-hnb659fds-container-assets-${AWS::AccountId}-${AWS::Region}', }, ); - expect(manifest.dockerImages?.[assset.assetHash]?.source).toStrictEqual( + expect(manifest.dockerImages?.[asset.assetHash]?.source).toStrictEqual( { executable: [ 'sh', '-c', - `docker load -i asset.${assset.assetHash}.tar | sed "s/Loaded image: //g"`, + `docker load -i asset.${asset.assetHash}.tar | sed "s/Loaded image: //g"`, ], }, ); + + // AssetStaging in TarballImageAsset uses `this` as scope' + expect(asset.node.tryFindChild('Staging')).toBeDefined(); }); testFutureBehavior('asset.repository.grantPull can be used to grant a principal permissions to use the image', flags, App, (app) => {