Skip to content

Commit

Permalink
fix(cdk-assets): Error when building Docker Image Assets with Podman (#…
Browse files Browse the repository at this point in the history
…24003)

Encountering the same error as #16209 (comment). When using `podman inspect` to check whether or not an image exists, the exit code for when an image does not exist is `125`, while Docker's is `1`. This change will treat either of these exit codes as meaning that the image with the given tag does not currently exist.

Closes #16209

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
JQuezada0 committed Feb 28, 2023
1 parent 643042b commit 4b08e20
Show file tree
Hide file tree
Showing 3 changed files with 127 additions and 3 deletions.
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 @@ -31,6 +31,11 @@ export interface DockerDomainCredentials {
readonly ecrRepository?: string;
}

enum InspectImageErrorCode {
Docker = 1,
Podman = 125
}

export class Docker {

private configDir: string | undefined = undefined;
Expand All @@ -46,8 +51,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);
});
});
});

0 comments on commit 4b08e20

Please sign in to comment.