Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(cdk-assets): Error when building Docker Image Assets with Podman #24003

Merged
merged 3 commits into from
Feb 28, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 31 additions & 3 deletions packages/cdk-assets/lib/private/docker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import * as fs from 'fs';
import * as os from 'os';
import * as path from 'path';
import { cdkCredentialsConfig, obtainEcrCredentials } from './docker-credentials';
import { Logger, shell, ShellOptions } from './shell';
import { Logger, shell, ShellOptions, ProcessFailedError } from './shell';
import { createCriticalSection } from './util';

interface BuildOptions {
Expand Down Expand Up @@ -30,6 +30,11 @@ export interface DockerDomainCredentials {
readonly ecrRepository?: string;
}

enum InspectImageErrorCode {
Docker = 1,
Podman = 125
}

export class Docker {

private configDir: string | undefined = undefined;
Expand All @@ -45,8 +50,31 @@ export class Docker {
await this.execute(['inspect', tag], { quiet: true });
return true;
} catch (e) {
if (e.code !== 'PROCESS_FAILED' || e.exitCode !== 1) { throw e; }
return false;
const error: ProcessFailedError = e;

/**
* The only error we expect to be thrown will have this property and value.
* If it doesn't, it's unrecognized so re-throw it.
*/
if (error.code !== 'PROCESS_FAILED') {
throw error;
}

/**
* If we know the shell command above returned an error, check to see
* if the exit code is one we know to actually mean that the image doesn't
* exist.
*/
switch (error.exitCode) {
case InspectImageErrorCode.Docker:
case InspectImageErrorCode.Podman:
// Docker and Podman will return this exit code when an image doesn't exist, return false
// context: https://github.com/aws/aws-cdk/issues/16209
return false;
default:
// This is an error but it's not an exit code we recognize, throw.
throw error;
}
}
}

Expand Down
2 changes: 2 additions & 0 deletions packages/cdk-assets/lib/private/shell.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ export async function shell(command: string[], options: ShellOptions = {}): Prom
});
}

export type ProcessFailedError = ProcessFailed

class ProcessFailed extends Error {
public readonly code = 'PROCESS_FAILED';

Expand Down
94 changes: 94 additions & 0 deletions packages/cdk-assets/test/private/docker.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
import { Docker } from '../../lib/private/docker';
import { ShellOptions, ProcessFailedError } from '../../lib/private/shell';

type ShellExecuteMock = jest.SpyInstance<ReturnType<Docker['execute']>, Parameters<Docker['execute']>>;

describe('Docker', () => {
describe('exists', () => {
let docker: Docker;

const makeShellExecuteMock = (
fn: (params: string[]) => void,
): ShellExecuteMock =>
jest.spyOn<{ execute: Docker['execute'] }, 'execute'>(Docker.prototype as any, 'execute').mockImplementation(
async (params: string[], _options?: ShellOptions) => fn(params),
);

afterEach(() => {
jest.restoreAllMocks();
});

beforeEach(() => {
docker = new Docker();
});

test('returns true when image inspect command does not throw', async () => {
const spy = makeShellExecuteMock(() => undefined);

const imageExists = await docker.exists('foo');

expect(imageExists).toBe(true);
expect(spy.mock.calls[0][0]).toEqual(['inspect', 'foo']);
});

test('throws when an arbitrary error is caught', async () => {
makeShellExecuteMock(() => {
throw new Error();
});

await expect(docker.exists('foo')).rejects.toThrow();
});

test('throws when the error is a shell failure but the exit code is unrecognized', async () => {
makeShellExecuteMock(() => {
throw new (class extends Error implements ProcessFailedError {
public readonly code = 'PROCESS_FAILED'
public readonly exitCode = 47
public readonly signal = null

constructor() {
super('foo');
}
});
});

await expect(docker.exists('foo')).rejects.toThrow();
});

test('returns false when the error is a shell failure and the exit code is 1 (Docker)', async () => {
makeShellExecuteMock(() => {
throw new (class extends Error implements ProcessFailedError {
public readonly code = 'PROCESS_FAILED'
public readonly exitCode = 1
public readonly signal = null

constructor() {
super('foo');
}
});
});

const imageExists = await docker.exists('foo');

expect(imageExists).toBe(false);
});

test('returns false when the error is a shell failure and the exit code is 125 (Podman)', async () => {
makeShellExecuteMock(() => {
throw new (class extends Error implements ProcessFailedError {
public readonly code = 'PROCESS_FAILED'
public readonly exitCode = 125
public readonly signal = null

constructor() {
super('foo');
}
});
});

const imageExists = await docker.exists('foo');

expect(imageExists).toBe(false);
});
});
});