From d45606181cce7fb182df77a4c96b0a7ce1911251 Mon Sep 17 00:00:00 2001 From: kaizen3031593 Date: Fri, 7 Jan 2022 10:48:29 -0500 Subject: [PATCH 1/2] fix(pipelines): DockerCredential.dockerHub() silently fails auth --- .../@aws-cdk/pipelines/lib/docker-credentials.ts | 4 ++-- .../cdk-assets/bin/docker-credential-cdk-assets.ts | 8 +------- .../cdk-assets/lib/private/docker-credentials.ts | 11 ++++++++--- .../test/private/docker-credentials.test.ts | 14 +++++++++++++- 4 files changed, 24 insertions(+), 13 deletions(-) diff --git a/packages/@aws-cdk/pipelines/lib/docker-credentials.ts b/packages/@aws-cdk/pipelines/lib/docker-credentials.ts index 77b7d2c1b4381..05144d4957771 100644 --- a/packages/@aws-cdk/pipelines/lib/docker-credentials.ts +++ b/packages/@aws-cdk/pipelines/lib/docker-credentials.ts @@ -10,10 +10,10 @@ import { Fn } from '@aws-cdk/core'; export abstract class DockerCredential { /** * Creates a DockerCredential for DockerHub. - * Convenience method for `fromCustomRegistry('index.docker.io', opts)`. + * Convenience method for `customRegistry('https://index.docker.io/v1/', opts)`. */ public static dockerHub(secret: secretsmanager.ISecret, opts: ExternalDockerCredentialOptions = {}): DockerCredential { - return new ExternalDockerCredential('index.docker.io', secret, opts); + return new ExternalDockerCredential('https://index.docker.io/v1/', secret, opts); } /** diff --git a/packages/cdk-assets/bin/docker-credential-cdk-assets.ts b/packages/cdk-assets/bin/docker-credential-cdk-assets.ts index b04f2ba8510bc..0ec36d82a6060 100644 --- a/packages/cdk-assets/bin/docker-credential-cdk-assets.ts +++ b/packages/cdk-assets/bin/docker-credential-cdk-assets.ts @@ -30,13 +30,7 @@ async function main() { // Read the domain to fetch from stdin let rawDomain = fs.readFileSync(0, { encoding: 'utf-8' }).trim(); - // Paranoid handling to ensure new URL() doesn't throw if the schema is missing. - // Not convinced docker will ever pass in a url like 'index.docker.io/v1', but just in case... - rawDomain = rawDomain.includes('://') ? rawDomain : `https://${rawDomain}`; - const domain = new URL(rawDomain).hostname; - - const credentials = await fetchDockerLoginCredentials(new DefaultAwsClient(), config, domain); - + const credentials = await fetchDockerLoginCredentials(new DefaultAwsClient(), config, rawDomain); // Write the credentials back to stdout fs.writeFileSync(1, JSON.stringify(credentials)); } diff --git a/packages/cdk-assets/lib/private/docker-credentials.ts b/packages/cdk-assets/lib/private/docker-credentials.ts index b5c3f42139581..a840c1158bf21 100644 --- a/packages/cdk-assets/lib/private/docker-credentials.ts +++ b/packages/cdk-assets/lib/private/docker-credentials.ts @@ -39,12 +39,17 @@ export function cdkCredentialsConfig(): DockerCredentialsConfig | undefined { } /** Fetches login credentials from the configured source (e.g., SecretsManager, ECR) */ -export async function fetchDockerLoginCredentials(aws: IAws, config: DockerCredentialsConfig, domain: string) { - if (!Object.keys(config.domainCredentials).includes(domain)) { +export async function fetchDockerLoginCredentials(aws: IAws, config: DockerCredentialsConfig, rawDomain: string) { + // Paranoid handling to ensure new URL() doesn't throw if the schema is missing + // For official docker registry, docker will pass https://index.docker.io/v1/ + rawDomain = rawDomain.includes('://') ? rawDomain : `https://${rawDomain}`; + const domain = new URL(rawDomain).hostname; + + if (!Object.keys(config.domainCredentials).includes(domain) && !Object.keys(config.domainCredentials).includes(rawDomain)) { throw new Error(`unknown domain ${domain}`); } - const domainConfig = config.domainCredentials[domain]; + let domainConfig = config.domainCredentials[domain] ?? config.domainCredentials[rawDomain]; if (domainConfig.secretsManagerSecretId) { const sm = await aws.secretsManagerClient({ assumeRoleArn: domainConfig.assumeRoleArn }); diff --git a/packages/cdk-assets/test/private/docker-credentials.test.ts b/packages/cdk-assets/test/private/docker-credentials.test.ts index 6b521c67457b6..19160ccd0c880 100644 --- a/packages/cdk-assets/test/private/docker-credentials.test.ts +++ b/packages/cdk-assets/test/private/docker-credentials.test.ts @@ -97,8 +97,12 @@ describe('fetchDockerLoginCredentials', () => { await expect(fetchDockerLoginCredentials(aws, config, 'misconfigured.example.com')).rejects.toThrow(/unknown credential type/); }); + test('does not throw on correctly configured raw domain', async () => { + expect(fetchDockerLoginCredentials(aws, config, 'https://secret.example.com/v1/')).resolves; + }); + describe('SecretsManager', () => { - test('returns the credentials sucessfully if configured correctly', async () => { + test('returns the credentials sucessfully if configured correctly - domain', async () => { mockSecretWithSecretString({ username: 'secretUser', secret: 'secretPass' }); const creds = await fetchDockerLoginCredentials(aws, config, 'secret.example.com'); @@ -106,6 +110,14 @@ describe('fetchDockerLoginCredentials', () => { expect(creds).toEqual({ Username: 'secretUser', Secret: 'secretPass' }); }); + test('returns the credentials successfully if configured correctly - raw domain', async () => { + mockSecretWithSecretString({ username: 'secretUser', secret: 'secretPass' }); + + const creds = await fetchDockerLoginCredentials(aws, config, 'https://secret.example.com'); + + expect(creds).toEqual({ Username: 'secretUser', Secret: 'secretPass' }); + }); + test('throws when SecretsManager returns an error', async () => { const errMessage = "Secrets Manager can't find the specified secret."; aws.mockSecretsManager.getSecretValue = mockedApiFailure('ResourceNotFoundException', errMessage); From 297e7cd629dc0b654c639d2be585346555d93df7 Mon Sep 17 00:00:00 2001 From: kaizen3031593 Date: Fri, 7 Jan 2022 12:42:06 -0500 Subject: [PATCH 2/2] add path to cdk assets to docker command --- .../pipelines/test/docker-credentials.test.ts | 2 +- .../cdk-assets/bin/docker-credential-cdk-assets.ts | 4 ++-- packages/cdk-assets/lib/private/docker-credentials.ts | 10 +++++----- packages/cdk-assets/lib/private/docker.ts | 11 ++++++++++- 4 files changed, 18 insertions(+), 9 deletions(-) diff --git a/packages/@aws-cdk/pipelines/test/docker-credentials.test.ts b/packages/@aws-cdk/pipelines/test/docker-credentials.test.ts index a2b5fc2c577dd..902c13a4129b7 100644 --- a/packages/@aws-cdk/pipelines/test/docker-credentials.test.ts +++ b/packages/@aws-cdk/pipelines/test/docker-credentials.test.ts @@ -29,7 +29,7 @@ describe('ExternalDockerCredential', () => { test('dockerHub defaults registry domain', () => { const creds = cdkp.DockerCredential.dockerHub(secret); - expect(Object.keys(creds._renderCdkAssetsConfig())).toEqual(['index.docker.io']); + expect(Object.keys(creds._renderCdkAssetsConfig())).toEqual(['https://index.docker.io/v1/']); }); test('minimal example only renders secret', () => { diff --git a/packages/cdk-assets/bin/docker-credential-cdk-assets.ts b/packages/cdk-assets/bin/docker-credential-cdk-assets.ts index 0ec36d82a6060..6dccb5521cf55 100644 --- a/packages/cdk-assets/bin/docker-credential-cdk-assets.ts +++ b/packages/cdk-assets/bin/docker-credential-cdk-assets.ts @@ -29,8 +29,8 @@ async function main() { } // Read the domain to fetch from stdin - let rawDomain = fs.readFileSync(0, { encoding: 'utf-8' }).trim(); - const credentials = await fetchDockerLoginCredentials(new DefaultAwsClient(), config, rawDomain); + let endpoint = fs.readFileSync(0, { encoding: 'utf-8' }).trim(); + const credentials = await fetchDockerLoginCredentials(new DefaultAwsClient(), config, endpoint); // Write the credentials back to stdout fs.writeFileSync(1, JSON.stringify(credentials)); } diff --git a/packages/cdk-assets/lib/private/docker-credentials.ts b/packages/cdk-assets/lib/private/docker-credentials.ts index a840c1158bf21..923d18d70a3ee 100644 --- a/packages/cdk-assets/lib/private/docker-credentials.ts +++ b/packages/cdk-assets/lib/private/docker-credentials.ts @@ -39,17 +39,17 @@ export function cdkCredentialsConfig(): DockerCredentialsConfig | undefined { } /** Fetches login credentials from the configured source (e.g., SecretsManager, ECR) */ -export async function fetchDockerLoginCredentials(aws: IAws, config: DockerCredentialsConfig, rawDomain: string) { +export async function fetchDockerLoginCredentials(aws: IAws, config: DockerCredentialsConfig, endpoint: string) { // Paranoid handling to ensure new URL() doesn't throw if the schema is missing // For official docker registry, docker will pass https://index.docker.io/v1/ - rawDomain = rawDomain.includes('://') ? rawDomain : `https://${rawDomain}`; - const domain = new URL(rawDomain).hostname; + endpoint = endpoint.includes('://') ? endpoint : `https://${endpoint}`; + const domain = new URL(endpoint).hostname; - if (!Object.keys(config.domainCredentials).includes(domain) && !Object.keys(config.domainCredentials).includes(rawDomain)) { + if (!Object.keys(config.domainCredentials).includes(domain) && !Object.keys(config.domainCredentials).includes(endpoint)) { throw new Error(`unknown domain ${domain}`); } - let domainConfig = config.domainCredentials[domain] ?? config.domainCredentials[rawDomain]; + let domainConfig = config.domainCredentials[domain] ?? config.domainCredentials[endpoint]; if (domainConfig.secretsManagerSecretId) { const sm = await aws.secretsManagerClient({ assumeRoleArn: domainConfig.assumeRoleArn }); diff --git a/packages/cdk-assets/lib/private/docker.ts b/packages/cdk-assets/lib/private/docker.ts index e1fc54429f18f..aed2631ab2852 100644 --- a/packages/cdk-assets/lib/private/docker.ts +++ b/packages/cdk-assets/lib/private/docker.ts @@ -124,8 +124,17 @@ export class Docker { private async execute(args: string[], options: ShellOptions = {}) { const configArgs = this.configDir ? ['--config', this.configDir] : []; + const pathToCdkAssets = path.resolve(__dirname, '..', '..', 'bin'); try { - await shell(['docker', ...configArgs, ...args], { logger: this.logger, ...options }); + await shell(['docker', ...configArgs, ...args], { + logger: this.logger, + ...options, + env: { + ...process.env, + ...options.env, + PATH: `${pathToCdkAssets}${path.delimiter}${options.env?.PATH ?? process.env.PATH}`, + }, + }); } catch (e) { if (e.code === 'ENOENT') { throw new Error('Unable to execute \'docker\' in order to build a container asset. Please install \'docker\' and try again.');