From 95e584c489f7b8576db38c4d44a0e8efbd496102 Mon Sep 17 00:00:00 2001 From: Josh Kellendonk Date: Wed, 27 Apr 2022 20:37:25 -0600 Subject: [PATCH 1/4] fix(cdk-assets): osx errors during parallel docker image publishing --- .../lib/private/handlers/container-images.ts | 146 +++++++++++++----- packages/cdk-assets/lib/private/util.ts | 12 ++ .../cdk-assets/test/docker-images.test.ts | 103 ++++++++++++ packages/cdk-assets/test/util.test.ts | 32 ++++ 4 files changed, 258 insertions(+), 35 deletions(-) create mode 100644 packages/cdk-assets/lib/private/util.ts create mode 100644 packages/cdk-assets/test/util.test.ts diff --git a/packages/cdk-assets/lib/private/handlers/container-images.ts b/packages/cdk-assets/lib/private/handlers/container-images.ts index 6e8af3e787289..24cdef6efda20 100644 --- a/packages/cdk-assets/lib/private/handlers/container-images.ts +++ b/packages/cdk-assets/lib/private/handlers/container-images.ts @@ -6,10 +6,9 @@ import { IAssetHandler, IHandlerHost } from '../asset-handler'; import { Docker } from '../docker'; import { replaceAwsPlaceholders } from '../placeholders'; import { shell } from '../shell'; +import { createCriticalSection } from '../util'; export class ContainerImageAssetHandler implements IAssetHandler { - private readonly docker = new Docker(m => this.host.emitMessage(EventType.DEBUG, m)); - constructor( private readonly workDir: string, private readonly asset: DockerImageManifestEntry, @@ -31,17 +30,16 @@ export class ContainerImageAssetHandler implements IAssetHandler { if (await this.destinationAlreadyExists(ecr, destination, imageUri)) { return; } if (this.host.aborted) { return; } - // Default behavior is to login before build so that the Dockerfile can reference images in the ECR repo - // However, if we're in a pipelines environment (for example), - // we may have alternative credentials to the default ones to use for the build itself. - // If the special config file is present, delay the login to the default credentials until the push. - // If the config file is present, we will configure and use those credentials for the build. - let cdkDockerCredentialsConfigured = this.docker.configureCdkCredentials(); - if (!cdkDockerCredentialsConfigured) { await this.docker.login(ecr); } + const containerImageDockerOptions = { + repoUri, + logger: (m: string) => this.host.emitMessage(EventType.DEBUG, m), + ecr, + }; - const localTagName = this.asset.source.executable - ? await this.buildExternalAsset(this.asset.source.executable) - : await this.buildDirectoryAsset(); + const dockerForBuilding = await ContainerImageDocker.forBuild(containerImageDockerOptions); + + const builder = new ContainerImageBuilder(dockerForBuilding, this.workDir, this.asset, this.host); + const localTagName = await builder.build(); if (localTagName === undefined || this.host.aborted) { return; @@ -49,14 +47,110 @@ export class ContainerImageAssetHandler implements IAssetHandler { this.host.emitMessage(EventType.UPLOAD, `Push ${imageUri}`); if (this.host.aborted) { return; } - await this.docker.tag(localTagName, imageUri); - if (cdkDockerCredentialsConfigured) { - this.docker.resetAuthPlugins(); - await this.docker.login(ecr); + await dockerForBuilding.tag(localTagName, imageUri); + + const pushDocker = await ContainerImageDocker.forEcrPush(containerImageDockerOptions); + + await pushDocker.push(imageUri); + } + + /** + * Check whether the image already exists in the ECR repo + * + * Use the fields from the destination to do the actual check. The imageUri + * should correspond to that, but is only used to print Docker image location + * for user benefit (the format is slightly different). + */ + private async destinationAlreadyExists(ecr: AWS.ECR, destination: DockerImageDestination, imageUri: string): Promise { + this.host.emitMessage(EventType.CHECK, `Check ${imageUri}`); + if (await imageExists(ecr, destination.repositoryName, destination.imageTag)) { + this.host.emitMessage(EventType.FOUND, `Found ${imageUri}`); + return true; + } + + return false; + } +} + +interface ContainerImageDockerOptions { + readonly repoUri: string; + readonly ecr: AWS.ECR; + readonly logger: (m: string) => void; +} + +/** + * Helps get appropriately configured Docker instances during the container + * image publishing process. + */ +export class ContainerImageDocker { + /** + * Clear state. + */ + static clearState() { + this.loggedInDestinations.clear(); + } + + /** + * Gets a Docker instance for building images. + */ + static async forBuild(options: ContainerImageDockerOptions): Promise { + const docker = new Docker(options.logger); + + // Default behavior is to login before build so that the Dockerfile can reference images in the ECR repo + // However, if we're in a pipelines environment (for example), + // we may have alternative credentials to the default ones to use for the build itself. + // If the special config file is present, delay the login to the default credentials until the push. + // If the config file is present, we will configure and use those credentials for the build. + let cdkDockerCredentialsConfigured = docker.configureCdkCredentials(); + if (!cdkDockerCredentialsConfigured) { + await this.loginOncePerDestination(docker, options); } - await this.docker.push(imageUri); + return docker; + } + + /** + * Gets a Docker instance for pushing images to ECR. + */ + static async forEcrPush(options: ContainerImageDockerOptions) { + const docker = new Docker(options.logger); + await this.loginOncePerDestination(docker, options); + return docker; + } + + private static cacheCriticalSection = createCriticalSection(); + private static loggedInDestinations = new Set(); + + private static async loginOncePerDestination(docker: Docker, options: ContainerImageDockerOptions) { + // Changes: 012345678910.dkr.ecr.us-west-2.amazonaws.com/tagging-test + // To this: 012345678910.dkr.ecr.us-west-2.amazonaws.com + const repositoryDomain = options.repoUri.split('/')[0]; + + // Ensure one-at-a-time access to loggedInDestinations. + await this.cacheCriticalSection(async () => { + if (this.loggedInDestinations.has(repositoryDomain)) { + return; + } + + this.loggedInDestinations.add(repositoryDomain); + await docker.login(options.ecr); + }); + } +} + +class ContainerImageBuilder { + constructor( + private readonly docker: Docker, + private readonly workDir: string, + private readonly asset: DockerImageManifestEntry, + private readonly host: IHandlerHost) { + } + + async build(): Promise { + return this.asset.source.executable + ? this.buildExternalAsset(this.asset.source.executable) + : this.buildDirectoryAsset(); } /** @@ -84,7 +178,6 @@ export class ContainerImageAssetHandler implements IAssetHandler { * and is expected to return the generated image identifier on stdout. */ private async buildExternalAsset(executable: string[], cwd?: string): Promise { - const assetPath = cwd ?? this.workDir; this.host.emitMessage(EventType.BUILD, `Building Docker image using command '${executable}'`); @@ -93,23 +186,6 @@ export class ContainerImageAssetHandler implements IAssetHandler { return (await shell(executable, { cwd: assetPath, quiet: true })).trim(); } - /** - * Check whether the image already exists in the ECR repo - * - * Use the fields from the destination to do the actual check. The imageUri - * should correspond to that, but is only used to print Docker image location - * for user benefit (the format is slightly different). - */ - private async destinationAlreadyExists(ecr: AWS.ECR, destination: DockerImageDestination, imageUri: string): Promise { - this.host.emitMessage(EventType.CHECK, `Check ${imageUri}`); - if (await imageExists(ecr, destination.repositoryName, destination.imageTag)) { - this.host.emitMessage(EventType.FOUND, `Found ${imageUri}`); - return true; - } - - return false; - } - private async buildImage(localTagName: string): Promise { const source = this.asset.source; if (!source.directory) { diff --git a/packages/cdk-assets/lib/private/util.ts b/packages/cdk-assets/lib/private/util.ts new file mode 100644 index 0000000000000..a042641104ac2 --- /dev/null +++ b/packages/cdk-assets/lib/private/util.ts @@ -0,0 +1,12 @@ +/** + * Creates a critical section, ensuring that at most one function can + * enter the critical section at a time. + */ +export function createCriticalSection() { + let lock = Promise.resolve(); + return async (criticalFunction: () => Promise) => { + const res = lock.then(() => criticalFunction()); + lock = res.catch(e => e); + return res.then(v => v); + }; +}; \ No newline at end of file diff --git a/packages/cdk-assets/test/docker-images.test.ts b/packages/cdk-assets/test/docker-images.test.ts index a8d2561197f06..9cce7352070cb 100644 --- a/packages/cdk-assets/test/docker-images.test.ts +++ b/packages/cdk-assets/test/docker-images.test.ts @@ -5,6 +5,7 @@ import { Manifest } from '@aws-cdk/cloud-assembly-schema'; import * as mockfs from 'mock-fs'; import { AssetManifest, AssetPublishing } from '../lib'; import * as dockercreds from '../lib/private/docker-credentials'; +import { ContainerImageDocker } from '../lib/private/handlers/container-images'; import { mockAws, mockedApiFailure, mockedApiResult } from './mock-aws'; import { mockSpawn } from './mock-child_process'; @@ -12,6 +13,8 @@ import { mockSpawn } from './mock-child_process'; let aws: ReturnType; const absoluteDockerPath = '/simple/cdk.out/dockerdir'; beforeEach(() => { + ContainerImageDocker.clearState(); + jest.resetAllMocks(); // By default, assume no externally-configured credentials. @@ -36,6 +39,37 @@ beforeEach(() => { }, }, }), + '/multi/cdk.out/assets.json': JSON.stringify({ + version: Manifest.version(), + dockerImages: { + theAsset1: { + source: { + directory: 'dockerdir', + }, + destinations: { + theDestination: { + region: 'us-north-50', + assumeRoleArn: 'arn:aws:role', + repositoryName: 'repo', + imageTag: 'theAsset1', + }, + }, + }, + theAsset2: { + source: { + directory: 'dockerdir', + }, + destinations: { + theDestination: { + region: 'us-north-50', + assumeRoleArn: 'arn:aws:role', + repositoryName: 'repo', + imageTag: 'theAsset2', + }, + }, + }, + }, + }), '/external/cdk.out/assets.json': JSON.stringify({ version: Manifest.version(), dockerImages: { @@ -295,3 +329,72 @@ test('when external credentials are present, explicit Docker config directories expectAllSpawns(); }); + +test('logging in only once for two assets', async () => { + const pub = new AssetPublishing(AssetManifest.fromPath('/multi/cdk.out'), { aws, throwOnError: false }); + aws.mockEcr.describeImages = mockedApiFailure('ImageNotFoundException', 'File does not exist'); + aws.mockEcr.getAuthorizationToken = mockedApiResult({ + authorizationData: [ + { authorizationToken: 'dXNlcjpwYXNz', proxyEndpoint: 'https://proxy.com/' }, + ], + }); + + const expectAllSpawns = mockSpawn( + { commandLine: ['docker', 'login', '--username', 'user', '--password-stdin', 'https://proxy.com/'] }, + { commandLine: ['docker', 'inspect', 'cdkasset-theasset1'], exitCode: 1 }, + { commandLine: ['docker', 'build', '--tag', 'cdkasset-theasset1', '.'], cwd: '/multi/cdk.out/dockerdir' }, + { commandLine: ['docker', 'tag', 'cdkasset-theasset1', '12345.amazonaws.com/repo:theAsset1'] }, + { commandLine: ['docker', 'push', '12345.amazonaws.com/repo:theAsset1'] }, + { commandLine: ['docker', 'inspect', 'cdkasset-theasset2'], exitCode: 1 }, + { commandLine: ['docker', 'build', '--tag', 'cdkasset-theasset2', '.'], cwd: '/multi/cdk.out/dockerdir' }, + { commandLine: ['docker', 'tag', 'cdkasset-theasset2', '12345.amazonaws.com/repo:theAsset2'] }, + { commandLine: ['docker', 'push', '12345.amazonaws.com/repo:theAsset2'] }, + ); + + await pub.publish(); + + expectAllSpawns(); + expect(true).toBeTruthy(); // Expect no exception, satisfy linter +}); + +test('logging in twice for two repository domains (containing account id & region)', async () => { + const pub = new AssetPublishing(AssetManifest.fromPath('/multi/cdk.out'), { aws, throwOnError: false }); + aws.mockEcr.describeImages = mockedApiFailure('ImageNotFoundException', 'File does not exist'); + + let repoIdx = 12345; + aws.mockEcr.describeRepositories = jest.fn().mockReturnValue({ + promise: jest.fn().mockImplementation(() => Promise.resolve({ + repositories: [ + // Usually looks like: 012345678910.dkr.ecr.us-west-2.amazonaws.com/aws-cdk/assets + { repositoryUri: `${repoIdx++}.amazonaws.com/aws-cdk/assets` }, + ], + })), + }); + + let proxyIdx = 12345; + aws.mockEcr.getAuthorizationToken = jest.fn().mockReturnValue({ + promise: jest.fn().mockImplementation(() => Promise.resolve({ + authorizationData: [ + { authorizationToken: 'dXNlcjpwYXNz', proxyEndpoint: `https://${proxyIdx++}.proxy.com/` }, + ], + })), + }); + + const expectAllSpawns = mockSpawn( + { commandLine: ['docker', 'login', '--username', 'user', '--password-stdin', 'https://12345.proxy.com/'] }, + { commandLine: ['docker', 'inspect', 'cdkasset-theasset1'], exitCode: 1 }, + { commandLine: ['docker', 'build', '--tag', 'cdkasset-theasset1', '.'], cwd: '/multi/cdk.out/dockerdir' }, + { commandLine: ['docker', 'tag', 'cdkasset-theasset1', '12345.amazonaws.com/aws-cdk/assets:theAsset1'] }, + { commandLine: ['docker', 'push', '12345.amazonaws.com/aws-cdk/assets:theAsset1'] }, + { commandLine: ['docker', 'login', '--username', 'user', '--password-stdin', 'https://12346.proxy.com/'] }, + { commandLine: ['docker', 'inspect', 'cdkasset-theasset2'], exitCode: 1 }, + { commandLine: ['docker', 'build', '--tag', 'cdkasset-theasset2', '.'], cwd: '/multi/cdk.out/dockerdir' }, + { commandLine: ['docker', 'tag', 'cdkasset-theasset2', '12346.amazonaws.com/aws-cdk/assets:theAsset2'] }, + { commandLine: ['docker', 'push', '12346.amazonaws.com/aws-cdk/assets:theAsset2'] }, + ); + + await pub.publish(); + + expectAllSpawns(); + expect(true).toBeTruthy(); // Expect no exception, satisfy linter +}); diff --git a/packages/cdk-assets/test/util.test.ts b/packages/cdk-assets/test/util.test.ts new file mode 100644 index 0000000000000..8e498076913f2 --- /dev/null +++ b/packages/cdk-assets/test/util.test.ts @@ -0,0 +1,32 @@ +import { createCriticalSection } from '../lib/private/util'; + +test('critical section', async () => { + // GIVEN + const criticalSection = createCriticalSection(); + + // WHEN + const arr = new Array(); + void criticalSection(async () => { + await new Promise(res => setTimeout(res, 500)); + arr.push('first'); + }); + await criticalSection(async () => { + arr.push('second'); + }); + + // THEN + expect(arr).toEqual([ + 'first', + 'second', + ]); +}); + +test('exceptions in critical sections', async () => { + // GIVEN + const criticalSection = createCriticalSection(); + + // WHEN/THEN + await expect(() => criticalSection(async () => { + throw new Error('Thrown'); + })).rejects.toThrow('Thrown'); +}); \ No newline at end of file From 102f908315760a2811caf5062d85a565b363de95 Mon Sep 17 00:00:00 2001 From: Josh Kellendonk Date: Fri, 29 Apr 2022 12:47:08 -0600 Subject: [PATCH 2/4] put the docker logic in IHandlerHost instead of static --- .../cdk-assets/lib/private/asset-handler.ts | 2 + packages/cdk-assets/lib/private/docker.ts | 60 +++++++++++++++ .../lib/private/handlers/container-images.ts | 74 +------------------ packages/cdk-assets/lib/publishing.ts | 2 + .../cdk-assets/test/docker-images.test.ts | 3 - 5 files changed, 67 insertions(+), 74 deletions(-) diff --git a/packages/cdk-assets/lib/private/asset-handler.ts b/packages/cdk-assets/lib/private/asset-handler.ts index af57422f95915..9b4eb4fa305c7 100644 --- a/packages/cdk-assets/lib/private/asset-handler.ts +++ b/packages/cdk-assets/lib/private/asset-handler.ts @@ -1,5 +1,6 @@ import { IAws } from '../aws'; import { EventType } from '../progress'; +import { DockerFactory } from './docker'; export interface IAssetHandler { publish(): Promise; @@ -8,6 +9,7 @@ export interface IAssetHandler { export interface IHandlerHost { readonly aws: IAws; readonly aborted: boolean; + readonly dockerFactory: DockerFactory; emitMessage(type: EventType, m: string): void; } \ No newline at end of file diff --git a/packages/cdk-assets/lib/private/docker.ts b/packages/cdk-assets/lib/private/docker.ts index dac365b176dcc..9a57f6e933a4a 100644 --- a/packages/cdk-assets/lib/private/docker.ts +++ b/packages/cdk-assets/lib/private/docker.ts @@ -3,6 +3,7 @@ import * as os from 'os'; import * as path from 'path'; import { cdkCredentialsConfig, obtainEcrCredentials } from './docker-credentials'; import { Logger, shell, ShellOptions } from './shell'; +import { createCriticalSection } from './util'; interface BuildOptions { readonly directory: string; @@ -146,6 +147,65 @@ export class Docker { } } +export interface DockerFactoryOptions { + readonly repoUri: string; + readonly ecr: AWS.ECR; + readonly logger: (m: string) => void; +} + +/** + * Helps get appropriately configured Docker instances during the container + * image publishing process. + */ +export class DockerFactory { + private cacheCriticalSection = createCriticalSection(); + private loggedInDestinations = new Set(); + + /** + * Gets a Docker instance for building images. + */ + public async forBuild(options: DockerFactoryOptions): Promise { + const docker = new Docker(options.logger); + + // Default behavior is to login before build so that the Dockerfile can reference images in the ECR repo + // However, if we're in a pipelines environment (for example), + // we may have alternative credentials to the default ones to use for the build itself. + // If the special config file is present, delay the login to the default credentials until the push. + // If the config file is present, we will configure and use those credentials for the build. + let cdkDockerCredentialsConfigured = docker.configureCdkCredentials(); + if (!cdkDockerCredentialsConfigured) { + await this.loginOncePerDestination(docker, options); + } + + return docker; + } + + /** + * Gets a Docker instance for pushing images to ECR. + */ + public async forEcrPush(options: DockerFactoryOptions) { + const docker = new Docker(options.logger); + await this.loginOncePerDestination(docker, options); + return docker; + } + + private async loginOncePerDestination(docker: Docker, options: DockerFactoryOptions) { + // Changes: 012345678910.dkr.ecr.us-west-2.amazonaws.com/tagging-test + // To this: 012345678910.dkr.ecr.us-west-2.amazonaws.com + const repositoryDomain = options.repoUri.split('/')[0]; + + // Ensure one-at-a-time access to loggedInDestinations. + await this.cacheCriticalSection(async () => { + if (this.loggedInDestinations.has(repositoryDomain)) { + return; + } + + this.loggedInDestinations.add(repositoryDomain); + await docker.login(options.ecr); + }); + } +} + function flatten(x: string[][]) { return Array.prototype.concat([], ...x); } diff --git a/packages/cdk-assets/lib/private/handlers/container-images.ts b/packages/cdk-assets/lib/private/handlers/container-images.ts index 24cdef6efda20..61ac1004cc714 100644 --- a/packages/cdk-assets/lib/private/handlers/container-images.ts +++ b/packages/cdk-assets/lib/private/handlers/container-images.ts @@ -6,7 +6,6 @@ import { IAssetHandler, IHandlerHost } from '../asset-handler'; import { Docker } from '../docker'; import { replaceAwsPlaceholders } from '../placeholders'; import { shell } from '../shell'; -import { createCriticalSection } from '../util'; export class ContainerImageAssetHandler implements IAssetHandler { constructor( @@ -36,7 +35,7 @@ export class ContainerImageAssetHandler implements IAssetHandler { ecr, }; - const dockerForBuilding = await ContainerImageDocker.forBuild(containerImageDockerOptions); + const dockerForBuilding = await this.host.dockerFactory.forBuild(containerImageDockerOptions); const builder = new ContainerImageBuilder(dockerForBuilding, this.workDir, this.asset, this.host); const localTagName = await builder.build(); @@ -50,9 +49,8 @@ export class ContainerImageAssetHandler implements IAssetHandler { await dockerForBuilding.tag(localTagName, imageUri); - const pushDocker = await ContainerImageDocker.forEcrPush(containerImageDockerOptions); - - await pushDocker.push(imageUri); + const dockerForPushing = await this.host.dockerFactory.forEcrPush(containerImageDockerOptions); + await dockerForPushing.push(imageUri); } /** @@ -73,72 +71,6 @@ export class ContainerImageAssetHandler implements IAssetHandler { } } -interface ContainerImageDockerOptions { - readonly repoUri: string; - readonly ecr: AWS.ECR; - readonly logger: (m: string) => void; -} - -/** - * Helps get appropriately configured Docker instances during the container - * image publishing process. - */ -export class ContainerImageDocker { - /** - * Clear state. - */ - static clearState() { - this.loggedInDestinations.clear(); - } - - /** - * Gets a Docker instance for building images. - */ - static async forBuild(options: ContainerImageDockerOptions): Promise { - const docker = new Docker(options.logger); - - // Default behavior is to login before build so that the Dockerfile can reference images in the ECR repo - // However, if we're in a pipelines environment (for example), - // we may have alternative credentials to the default ones to use for the build itself. - // If the special config file is present, delay the login to the default credentials until the push. - // If the config file is present, we will configure and use those credentials for the build. - let cdkDockerCredentialsConfigured = docker.configureCdkCredentials(); - if (!cdkDockerCredentialsConfigured) { - await this.loginOncePerDestination(docker, options); - } - - return docker; - } - - /** - * Gets a Docker instance for pushing images to ECR. - */ - static async forEcrPush(options: ContainerImageDockerOptions) { - const docker = new Docker(options.logger); - await this.loginOncePerDestination(docker, options); - return docker; - } - - private static cacheCriticalSection = createCriticalSection(); - private static loggedInDestinations = new Set(); - - private static async loginOncePerDestination(docker: Docker, options: ContainerImageDockerOptions) { - // Changes: 012345678910.dkr.ecr.us-west-2.amazonaws.com/tagging-test - // To this: 012345678910.dkr.ecr.us-west-2.amazonaws.com - const repositoryDomain = options.repoUri.split('/')[0]; - - // Ensure one-at-a-time access to loggedInDestinations. - await this.cacheCriticalSection(async () => { - if (this.loggedInDestinations.has(repositoryDomain)) { - return; - } - - this.loggedInDestinations.add(repositoryDomain); - await docker.login(options.ecr); - }); - } -} - class ContainerImageBuilder { constructor( private readonly docker: Docker, diff --git a/packages/cdk-assets/lib/publishing.ts b/packages/cdk-assets/lib/publishing.ts index 804265a56acc8..a4eb709df0efd 100644 --- a/packages/cdk-assets/lib/publishing.ts +++ b/packages/cdk-assets/lib/publishing.ts @@ -1,6 +1,7 @@ import { AssetManifest, IManifestEntry } from './asset-manifest'; import { IAws } from './aws'; import { IHandlerHost } from './private/asset-handler'; +import { DockerFactory } from './private/docker'; import { makeAssetHandler } from './private/handlers'; import { EventType, IPublishProgress, IPublishProgressListener } from './progress'; @@ -76,6 +77,7 @@ export class AssetPublishing implements IPublishProgress { aws: this.options.aws, get aborted() { return self.aborted; }, emitMessage(t, m) { self.progressEvent(t, m); }, + dockerFactory: new DockerFactory(), }; } diff --git a/packages/cdk-assets/test/docker-images.test.ts b/packages/cdk-assets/test/docker-images.test.ts index 9cce7352070cb..62af7cf399abb 100644 --- a/packages/cdk-assets/test/docker-images.test.ts +++ b/packages/cdk-assets/test/docker-images.test.ts @@ -5,7 +5,6 @@ import { Manifest } from '@aws-cdk/cloud-assembly-schema'; import * as mockfs from 'mock-fs'; import { AssetManifest, AssetPublishing } from '../lib'; import * as dockercreds from '../lib/private/docker-credentials'; -import { ContainerImageDocker } from '../lib/private/handlers/container-images'; import { mockAws, mockedApiFailure, mockedApiResult } from './mock-aws'; import { mockSpawn } from './mock-child_process'; @@ -13,8 +12,6 @@ import { mockSpawn } from './mock-child_process'; let aws: ReturnType; const absoluteDockerPath = '/simple/cdk.out/dockerdir'; beforeEach(() => { - ContainerImageDocker.clearState(); - jest.resetAllMocks(); // By default, assume no externally-configured credentials. From 67ad55ac866fffb1f58ade962e3c179d220c839a Mon Sep 17 00:00:00 2001 From: Josh Kellendonk Date: Fri, 29 Apr 2022 13:01:54 -0600 Subject: [PATCH 3/4] reorder loggedInDestinations.add after docker.login --- packages/cdk-assets/lib/private/docker.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/cdk-assets/lib/private/docker.ts b/packages/cdk-assets/lib/private/docker.ts index 9a57f6e933a4a..1a9b2293230f6 100644 --- a/packages/cdk-assets/lib/private/docker.ts +++ b/packages/cdk-assets/lib/private/docker.ts @@ -158,7 +158,7 @@ export interface DockerFactoryOptions { * image publishing process. */ export class DockerFactory { - private cacheCriticalSection = createCriticalSection(); + private enterLoggedInDestinationsCriticalSection = createCriticalSection(); private loggedInDestinations = new Set(); /** @@ -195,13 +195,13 @@ export class DockerFactory { const repositoryDomain = options.repoUri.split('/')[0]; // Ensure one-at-a-time access to loggedInDestinations. - await this.cacheCriticalSection(async () => { + await this.enterLoggedInDestinationsCriticalSection(async () => { if (this.loggedInDestinations.has(repositoryDomain)) { return; } - this.loggedInDestinations.add(repositoryDomain); await docker.login(options.ecr); + this.loggedInDestinations.add(repositoryDomain); }); } } From c2ca07668f19d97e69ad3295331237bc545642a8 Mon Sep 17 00:00:00 2001 From: Josh Kellendonk Date: Tue, 10 May 2022 09:53:07 -0600 Subject: [PATCH 4/4] replace res.then(v=>v) with res --- packages/cdk-assets/lib/private/util.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cdk-assets/lib/private/util.ts b/packages/cdk-assets/lib/private/util.ts index a042641104ac2..88a87a18e6ba9 100644 --- a/packages/cdk-assets/lib/private/util.ts +++ b/packages/cdk-assets/lib/private/util.ts @@ -7,6 +7,6 @@ export function createCriticalSection() { return async (criticalFunction: () => Promise) => { const res = lock.then(() => criticalFunction()); lock = res.catch(e => e); - return res.then(v => v); + return res; }; }; \ No newline at end of file