Skip to content

Commit

Permalink
refactor(cli): remove unused internal code and options (aws#33432)
Browse files Browse the repository at this point in the history
### Reason for this change

Unused code path that was previously deprecated needs to be removed in preparation of refactoring.

### Description of changes

Removing previously deprecated and unused methods on `Deployments` class and transitively unused code.
Removed options for asset building/publishing that are unused.

### Describe any new or updated permissions being added

n/a

### Description of how you validated changes

existing tests

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*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 authored Feb 13, 2025
1 parent 91765e2 commit 331d7a5
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 118 deletions.
74 changes: 8 additions & 66 deletions packages/aws-cdk/lib/api/deployments/asset-publishing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,7 @@ import { ToolkitError } from '../../toolkit/error';
import type { SdkProvider } from '../aws-auth';
import { Mode } from '../plugin';

export interface PublishAssetsOptions {
/**
* Print progress at 'debug' level
*/
readonly quiet?: boolean;

/**
* Whether to build assets before publishing.
*
* @default true To remain backward compatible.
*/
readonly buildAssets?: boolean;

interface PublishAssetsOptions {
/**
* Whether to build/publish assets in parallel
*
Expand All @@ -46,6 +34,8 @@ export interface PublishAssetsOptions {

/**
* Use cdk-assets to publish all assets in the given manifest.
*
* @deprecated used in legacy deployments only, should be migrated at some point
*/
export async function publishAssets(
manifest: AssetManifest,
Expand All @@ -66,67 +56,19 @@ export async function publishAssets(

const publisher = new AssetPublishing(manifest, {
aws: new PublishingAws(sdk, targetEnv),
progressListener: new PublishingProgressListener(options.quiet ?? false),
progressListener: new PublishingProgressListener(),
throwOnError: false,
publishInParallel: options.parallel ?? true,
buildAssets: options.buildAssets ?? true,
buildAssets: true,
publishAssets: true,
quiet: options.quiet,
quiet: false,
});
await publisher.publish({ allowCrossAccount: options.allowCrossAccount });
if (publisher.hasFailures) {
throw new ToolkitError('Failed to publish one or more assets. See the error messages above for more information.');
}
}

export interface BuildAssetsOptions {
/**
* Print progress at 'debug' level
*/
readonly quiet?: boolean;

/**
* Build assets in parallel
*
* @default true
*/
readonly parallel?: boolean;
}

/**
* Use cdk-assets to build all assets in the given manifest.
*/
export async function buildAssets(
manifest: AssetManifest,
sdk: SdkProvider,
targetEnv: Environment,
options: BuildAssetsOptions = {},
) {
// This shouldn't really happen (it's a programming error), but we don't have
// the types here to guide us. Do an runtime validation to be super super sure.
if (
targetEnv.account === undefined ||
targetEnv.account === UNKNOWN_ACCOUNT ||
targetEnv.region === undefined ||
targetEnv.account === UNKNOWN_REGION
) {
throw new ToolkitError(`Asset building requires resolved account and region, got ${JSON.stringify(targetEnv)}`);
}

const publisher = new AssetPublishing(manifest, {
aws: new PublishingAws(sdk, targetEnv),
progressListener: new PublishingProgressListener(options.quiet ?? false),
throwOnError: false,
publishInParallel: options.parallel ?? true,
buildAssets: true,
publishAssets: false,
});
await publisher.publish();
if (publisher.hasFailures) {
throw new ToolkitError('Failed to build one or more assets. See the error messages above for more information.');
}
}

export class PublishingAws implements IAws {
private sdkCache: Map<String, SDK> = new Map();

Expand Down Expand Up @@ -241,10 +183,10 @@ export const EVENT_TO_LOGGER: Record<EventType, (x: string) => void> = {
};

class PublishingProgressListener implements IPublishProgressListener {
constructor(private readonly quiet: boolean) {}
constructor() {}

public onPublishEvent(type: EventType, event: IPublishProgress): void {
const handler = this.quiet && type !== 'fail' ? debug : EVENT_TO_LOGGER[type];
const handler = EVENT_TO_LOGGER[type];
handler(`[${event.percentComplete}%] ${type}: ${event.message}`);
}
}
52 changes: 0 additions & 52 deletions packages/aws-cdk/lib/api/deployments/deployments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,7 @@ import * as cdk_assets from 'cdk-assets';
import * as chalk from 'chalk';
import { AssetManifestBuilder } from './asset-manifest-builder';
import {
buildAssets,
type BuildAssetsOptions,
EVENT_TO_LOGGER,
publishAssets,
type PublishAssetsOptions,
PublishingAws,
} from './asset-publishing';
import { determineAllowCrossAccountAssetPublishing } from './checks';
Expand Down Expand Up @@ -297,23 +293,13 @@ interface AssetOptions {
}

export interface BuildStackAssetsOptions extends AssetOptions {
/**
* Options to pass on to `buildAssets()` function
*/
readonly buildOptions?: BuildAssetsOptions;

/**
* Stack name this asset is for
*/
readonly stackName?: string;
}

interface PublishStackAssetsOptions extends AssetOptions {
/**
* Options to pass on to `publishAsests()` function
*/
readonly publishOptions?: Omit<PublishAssetsOptions, 'buildAssets'>;

/**
* Stack name this asset is for
*/
Expand Down Expand Up @@ -640,43 +626,6 @@ export class Deployments {
return stack.exists;
}

private async prepareAndValidateAssets(asset: cxapi.AssetManifestArtifact, options: AssetOptions) {
const env = await this.envs.accessStackForMutableStackOperations(options.stack);

await this.validateBootstrapStackVersion(
options.stack.stackName,
asset.requiresBootstrapStackVersion,
asset.bootstrapStackVersionSsmParameter,
env.resources);

const manifest = cdk_assets.AssetManifest.fromFile(asset.file);

return { manifest, stackEnv: env.resolvedEnvironment };
}

/**
* Build all assets in a manifest
*
* @deprecated Use `buildSingleAsset` instead
*/
public async buildAssets(asset: cxapi.AssetManifestArtifact, options: BuildStackAssetsOptions) {
const { manifest, stackEnv } = await this.prepareAndValidateAssets(asset, options);
await buildAssets(manifest, this.assetSdkProvider, stackEnv, options.buildOptions);
}

/**
* Publish all assets in a manifest
*
* @deprecated Use `publishSingleAsset` instead
*/
public async publishAssets(asset: cxapi.AssetManifestArtifact, options: PublishStackAssetsOptions) {
const { manifest, stackEnv } = await this.prepareAndValidateAssets(asset, options);
await publishAssets(manifest, this.assetSdkProvider, stackEnv, {
...options.publishOptions,
allowCrossAccount: await this.allowCrossAccountAssetPublishingForEnv(options.stack),
});
}

/**
* Build a single asset from an asset manifest
*
Expand Down Expand Up @@ -712,7 +661,6 @@ export class Deployments {
/**
* Publish a single asset from an asset manifest
*/
// eslint-disable-next-line max-len
public async publishSingleAsset(
assetManifest: cdk_assets.AssetManifest,
asset: cdk_assets.IManifestEntry,
Expand Down

0 comments on commit 331d7a5

Please sign in to comment.