Skip to content

Commit

Permalink
fix(cli): specifying --quiet does not suppress asset building and pub…
Browse files Browse the repository at this point in the history
…lishing logs (#26493)

> We're currently maintaining a patched version of CDK to use with [SST](https://docs.sst.dev/) because this flag isn't fully respected. When the quiet flag is specified, the docker build and docker push shell commands still print to the terminal. This PR propagates the quiet flag down to the cdk-assets's container-images handler.

Replaces #26265

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
mrgrain committed Aug 3, 2023
1 parent c562075 commit b12bc67
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 13 deletions.
1 change: 1 addition & 0 deletions packages/aws-cdk/lib/util/asset-publishing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ export async function publishAssets(
publishInParallel: options.parallel ?? true,
buildAssets: options.buildAssets ?? true,
publishAssets: true,
quiet: options.quiet,
});
await publisher.publish();
if (publisher.hasFailures) {
Expand Down
9 changes: 8 additions & 1 deletion packages/cdk-assets/lib/private/asset-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,11 @@ export interface IHandlerHost {
readonly dockerFactory: DockerFactory;

emitMessage(type: EventType, m: string): void;
}
}

export interface IHandlerOptions {
/**
* Suppress all output
*/
readonly quiet?: boolean;
}
15 changes: 12 additions & 3 deletions packages/cdk-assets/lib/private/docker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@ interface BuildOptions {
readonly outputs?: string[];
readonly cacheFrom?: DockerCacheOption[];
readonly cacheTo?: DockerCacheOption;
readonly quiet?: boolean;
}

interface PushOptions {
readonly tag: string;
readonly quiet?: boolean;
}

export interface DockerCredentialsConfig {
Expand Down Expand Up @@ -101,7 +107,10 @@ export class Docker {
...options.cacheTo ? ['--cache-to', this.cacheOptionToFlag(options.cacheTo)] : [],
'.',
];
await this.execute(buildCommand, { cwd: options.directory });
await this.execute(buildCommand, {
cwd: options.directory,
quiet: options.quiet,
});
}

/**
Expand All @@ -128,8 +137,8 @@ export class Docker {
await this.execute(['tag', sourceTag, targetTag]);
}

public async push(tag: string) {
await this.execute(['push', tag]);
public async push(options: PushOptions) {
await this.execute(['push', options.tag], { quiet: options.quiet });
}

/**
Expand Down
19 changes: 14 additions & 5 deletions packages/cdk-assets/lib/private/handlers/container-images.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { DockerImageDestination } from '@aws-cdk/cloud-assembly-schema';
import type * as AWS from 'aws-sdk';
import { DockerImageManifestEntry } from '../../asset-manifest';
import { EventType } from '../../progress';
import { IAssetHandler, IHandlerHost } from '../asset-handler';
import { IAssetHandler, IHandlerHost, IHandlerOptions } from '../asset-handler';
import { Docker } from '../docker';
import { replaceAwsPlaceholders } from '../placeholders';
import { shell } from '../shell';
Expand All @@ -21,7 +21,8 @@ export class ContainerImageAssetHandler implements IAssetHandler {
constructor(
private readonly workDir: string,
private readonly asset: DockerImageManifestEntry,
private readonly host: IHandlerHost) {
private readonly host: IHandlerHost,
private readonly options: IHandlerOptions) {
}

public async build(): Promise<void> {
Expand All @@ -36,7 +37,9 @@ export class ContainerImageAssetHandler implements IAssetHandler {
ecr: initOnce.ecr,
});

const builder = new ContainerImageBuilder(dockerForBuilding, this.workDir, this.asset, this.host);
const builder = new ContainerImageBuilder(dockerForBuilding, this.workDir, this.asset, this.host, {
quiet: this.options.quiet,
});
const localTagName = await builder.build();

if (localTagName === undefined || this.host.aborted) { return; }
Expand Down Expand Up @@ -70,7 +73,7 @@ export class ContainerImageAssetHandler implements IAssetHandler {
if (this.host.aborted) { return; }

this.host.emitMessage(EventType.UPLOAD, `Push ${initOnce.imageUri}`);
await dockerForPushing.push(initOnce.imageUri);
await dockerForPushing.push({ tag: initOnce.imageUri, quiet: this.options.quiet });
}

private async initOnce(options: { quiet?: boolean } = {}): Promise<ContainerImageAssetHandlerInit> {
Expand Down Expand Up @@ -120,12 +123,17 @@ export class ContainerImageAssetHandler implements IAssetHandler {
}
}

interface ContainerImageBuilderOptions {
readonly quiet?: boolean;
}

class ContainerImageBuilder {
constructor(
private readonly docker: Docker,
private readonly workDir: string,
private readonly asset: DockerImageManifestEntry,
private readonly host: IHandlerHost) {
private readonly host: IHandlerHost,
private readonly options: ContainerImageBuilderOptions) {
}

async build(): Promise<string | undefined> {
Expand Down Expand Up @@ -188,6 +196,7 @@ class ContainerImageBuilder {
outputs: source.dockerOutputs,
cacheFrom: source.cacheFrom,
cacheTo: source.cacheTo,
quiet: this.options.quiet,
});
}

Expand Down
6 changes: 3 additions & 3 deletions packages/cdk-assets/lib/private/handlers/index.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import { ContainerImageAssetHandler } from './container-images';
import { FileAssetHandler } from './files';
import { AssetManifest, DockerImageManifestEntry, FileManifestEntry, IManifestEntry } from '../../asset-manifest';
import { IAssetHandler, IHandlerHost } from '../asset-handler';
import { IAssetHandler, IHandlerHost, IHandlerOptions } from '../asset-handler';

export function makeAssetHandler(manifest: AssetManifest, asset: IManifestEntry, host: IHandlerHost): IAssetHandler {
export function makeAssetHandler(manifest: AssetManifest, asset: IManifestEntry, host: IHandlerHost, options: IHandlerOptions): IAssetHandler {
if (asset instanceof FileManifestEntry) {
return new FileAssetHandler(manifest.directory, asset, host);
}
if (asset instanceof DockerImageManifestEntry) {
return new ContainerImageAssetHandler(manifest.directory, asset, host);
return new ContainerImageAssetHandler(manifest.directory, asset, host, options);
}

throw new Error(`Unrecognized asset type: '${asset}'`);
Expand Down
11 changes: 10 additions & 1 deletion packages/cdk-assets/lib/publishing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,13 @@ export interface AssetPublishingOptions {
* @default true
*/
readonly publishAssets?: boolean;

/**
* Whether to print publishing logs
*
* @default true
*/
readonly quiet?: boolean;
}

/**
Expand Down Expand Up @@ -240,7 +247,9 @@ export class AssetPublishing implements IPublishProgress {
if (existing) {
return existing;
}
const ret = makeAssetHandler(this.manifest, asset, this.handlerHost);
const ret = makeAssetHandler(this.manifest, asset, this.handlerHost, {
quiet: this.options.quiet,
});
this.handlerCache.set(asset, ret);
return ret;
}
Expand Down

0 comments on commit b12bc67

Please sign in to comment.