Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
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
8 changes: 0 additions & 8 deletions packages/@aws-cdk/toolkit/lib/actions/deploy/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,14 +115,6 @@ export interface BaseDeployOptions {
*/
readonly stacks?: StackSelector;

/**
* @deprecated set on toolkit
* Name of the toolkit stack to use/deploy
*
* @default CDKToolkit
*/
readonly toolkitStackName?: string;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed in favor of the same option on the Toolkit.


/**
* Role to pass to CloudFormation for deployment
*/
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/toolkit/lib/api/aws-cdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export { Deployments, type SuccessfulDeployStackResult } from '../../../../aws-c
export { Settings } from '../../../../aws-cdk/lib/api/settings';
export { tagsForStack } from '../../../../aws-cdk/lib/api/tags';
export { DEFAULT_TOOLKIT_STACK_NAME } from '../../../../aws-cdk/lib/api/toolkit-info';
export { ResourceMigrator } from '../../../../aws-cdk/lib/api/resource-import';

// Context Providers
export * as contextproviders from '../../../../aws-cdk/lib/context-providers';
Expand All @@ -19,7 +20,6 @@ export { formatTime } from '../../../../aws-cdk/lib/api/util/string-manipulation

// @todo Not yet API probably should be
export { formatErrorMessage } from '../../../../aws-cdk/lib/util/error';
export { ResourceMigrator } from '../../../../aws-cdk/lib/migrator';
export { obscureTemplate, serializeStructure } from '../../../../aws-cdk/lib/serialize';
export { loadTree, some } from '../../../../aws-cdk/lib/tree';
export { splitBySize } from '../../../../aws-cdk/lib/util';
Expand Down
6 changes: 5 additions & 1 deletion packages/@aws-cdk/toolkit/lib/api/io/private/codes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,15 @@ export const CODES = {
// 2: List
CDK_TOOLKIT_I2901: 'Provides details on the selected stacks and their dependencies',

// 3: Import & Migrate
CDK_TOOLKIT_E3900: 'Resource import failed',

// 4: Diff

// 5: Deploy
// 5: Deploy & Watch
CDK_TOOLKIT_I5000: 'Provides deployment times',
CDK_TOOLKIT_I5001: 'Provides total time in deploy action, including synth and rollback',
CDK_TOOLKIT_I5002: 'Provides time for resource migration',
CDK_TOOLKIT_I5031: 'Informs about any log groups that are traced as part of the deployment',
CDK_TOOLKIT_I5050: 'Confirm rollback during deployment',
CDK_TOOLKIT_I5060: 'Confirm deploy security sensitive changes',
Expand Down
34 changes: 17 additions & 17 deletions packages/@aws-cdk/toolkit/lib/toolkit/toolkit.ts
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some minor refactoring to make the code more human readable

Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { CachedCloudAssemblySource, IdentityCloudAssemblySource, StackAssembly,
import { ALL_STACKS, CloudAssemblySourceBuilder } from '../api/cloud-assembly/private';
import { ToolkitError } from '../api/errors';
import { IIoHost, IoMessageCode, IoMessageLevel } from '../api/io';
import { asSdkLogger, withAction, Timer, confirm, error, highlight, info, success, warn, ActionAwareIoHost, debug, result, withoutEmojis, withoutColor, withTrimmedWhitespace } from '../api/io/private';
import { asSdkLogger, withAction, Timer, confirm, error, info, success, warn, ActionAwareIoHost, debug, result, withoutEmojis, withoutColor, withTrimmedWhitespace } from '../api/io/private';

/**
* The current action being performed by the CLI. 'none' represents the absence of an action.
Expand Down Expand Up @@ -256,10 +256,8 @@ export class Toolkit extends CloudAssemblySourceBuilder implements AsyncDisposab
}

const deployments = await this.deploymentsForAction('deploy');
const migrator = new ResourceMigrator({ deployments, ioHost, action });

const migrator = new ResourceMigrator({
deployments,
});
await migrator.tryMigrateResources(stackCollection, options);

const requireApproval = options.requireApproval ?? RequireApproval.NEVER;
Expand All @@ -275,7 +273,6 @@ export class Toolkit extends CloudAssemblySourceBuilder implements AsyncDisposab
}

const stacks = stackCollection.stackArtifacts;

const stackOutputs: { [key: string]: any } = {};
const outputsFile = options.outputsFile;

Expand Down Expand Up @@ -303,28 +300,31 @@ export class Toolkit extends CloudAssemblySourceBuilder implements AsyncDisposab
const deployStack = async (stackNode: StackNode) => {
const stack = stackNode.stack;
if (stackCollection.stackCount !== 1) {
await ioHost.notify(highlight(stack.displayName));
await ioHost.notify(info(chalk.bold(stack.displayName)));
}

if (!stack.environment) {
// eslint-disable-next-line max-len
throw new ToolkitError(
`Stack ${stack.displayName} does not define an environment, and AWS credentials could not be obtained from standard locations or no region was configured.`,
);
}

// The generated stack has no resources
if (Object.keys(stack.template.Resources || {}).length === 0) {
// The generated stack has no resources
if (!(await deployments.stackExists({ stack }))) {
await ioHost.notify(warn(`${chalk.bold(stack.displayName)}: stack has no resources, skipping deployment.`));
} else {
await ioHost.notify(warn(`${chalk.bold(stack.displayName)}: stack has no resources, deleting existing stack.`));
await this._destroy(assembly, 'deploy', {
stacks: { patterns: [stack.hierarchicalId], strategy: StackSelectionStrategy.PATTERN_MUST_MATCH_SINGLE },
roleArn: options.roleArn,
ci: options.ci,
});
// stack is empty and doesn't exist => do nothing
const stackExists = await deployments.stackExists({ stack });
if (!stackExists) {
return ioHost.notify(warn(`${chalk.bold(stack.displayName)}: stack has no resources, skipping deployment.`));
}

// stack is empty, but exists => delete
await ioHost.notify(warn(`${chalk.bold(stack.displayName)}: stack has no resources, deleting existing stack.`));
await this._destroy(assembly, 'deploy', {
stacks: { patterns: [stack.hierarchicalId], strategy: StackSelectionStrategy.PATTERN_MUST_MATCH_SINGLE },
roleArn: options.roleArn,
ci: options.ci,
});

return;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,20 +1,68 @@
import { DeployOptions } from '@aws-cdk/cloud-assembly-schema';
import { format } from 'util';
import * as cfnDiff from '@aws-cdk/cloudformation-diff';
import { ResourceDifference } from '@aws-cdk/cloudformation-diff';
import * as cxapi from '@aws-cdk/cx-api';
import * as chalk from 'chalk';
import * as fs from 'fs-extra';
import * as promptly from 'promptly';
import { assertIsSuccessfulDeployStackResult, Deployments, DeploymentMethod, ResourceIdentifierProperties, ResourcesToImport } from './api/deployments';
import { Tag } from './api/tags';
import { StackActivityProgress } from './api/util/cloudformation/stack-activity-monitor';
import { error, info, success, warning } from './logging';
import { ToolkitError } from './toolkit/error';

export interface ImportDeploymentOptions extends DeployOptions {
deploymentMethod?: DeploymentMethod;
progress?: StackActivityProgress;
tags?: Tag[];
import { error, info, warn } from '../../cli/messages';
import { IIoHost, ToolkitAction } from '../../toolkit/cli-io-host';
import { ToolkitError } from '../../toolkit/error';
import { assertIsSuccessfulDeployStackResult, Deployments, DeploymentMethod, ResourceIdentifierProperties, ResourcesToImport } from '../deployments';
import { Tag } from '../tags';
import { StackActivityProgress } from '../util/cloudformation/stack-activity-monitor';

export interface ResourceImporterProps {
deployments: Deployments;
ioHost: IIoHost;
action: ToolkitAction;
}

export interface ImportDeploymentOptions {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made this an explicit copy to limit the interface to a small subset of expected options.

/**
* Role to pass to CloudFormation for deployment
*
* @default - Default stack role
*/
readonly roleArn?: string;

/**
* Deployment method
*
* @default - Change set with default options
*/
readonly deploymentMethod?: DeploymentMethod;

/**
* Stack tags (pass through to CloudFormation)
*
* @default - No tags
*/
readonly tags?: Tag[];

/**
* Use previous values for unspecified parameters
*
* If not set, all parameters must be specified for every deployment.
*
* @default true
*/
readonly usePreviousParameters?: boolean;

/**
* Display mode for stack deployment progress.
*
* @default - StackActivityProgress.Bar - stack events will be displayed for
* the resource currently being deployed.
*/
readonly progress?: StackActivityProgress;

/**
* Rollback failed deployments
*
* @default true
*/
readonly rollback?: boolean;
}

/**
Expand Down Expand Up @@ -61,9 +109,20 @@
export class ResourceImporter {
private _currentTemplate: any;

private readonly stack: cxapi.CloudFormationStackArtifact;
private readonly cfn: Deployments;
private readonly ioHost: IIoHost;
private readonly action: ToolkitAction;

constructor(
private readonly stack: cxapi.CloudFormationStackArtifact,
private readonly cfn: Deployments) { }
stack: cxapi.CloudFormationStackArtifact,
props: ResourceImporterProps,
) {
this.stack = stack;
this.cfn = props.deployments;
this.ioHost = props.ioHost;
this.action = props.action;
}

/**
* Ask the user for resources to import
Expand Down Expand Up @@ -96,19 +155,19 @@
const descr = this.describeResource(resource.logicalId);
const idProps = contents[resource.logicalId];
if (idProps) {
info('%s: importing using %s', chalk.blue(descr), chalk.blue(fmtdict(idProps)));
await this.ioHost.notify(info(this.action, format('%s: importing using %s', chalk.blue(descr), chalk.blue(fmtdict(idProps)))));

Check warning on line 158 in packages/aws-cdk/lib/api/resource-import/importer.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/api/resource-import/importer.ts#L158

Added line #L158 was not covered by tests

ret.importResources.push(resource);
ret.resourceMap[resource.logicalId] = idProps;
delete contents[resource.logicalId];
} else {
info('%s: skipping', chalk.blue(descr));
await this.ioHost.notify(info(this.action, format('%s: skipping', chalk.blue(descr))));

Check warning on line 164 in packages/aws-cdk/lib/api/resource-import/importer.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/api/resource-import/importer.ts#L164

Added line #L164 was not covered by tests
}
}

const unknown = Object.keys(contents);
if (unknown.length > 0) {
warning(`Unrecognized resource identifiers in mapping file: ${unknown.join(', ')}`);
await this.ioHost.notify(warn(this.action, `Unrecognized resource identifiers in mapping file: ${unknown.join(', ')}`));

Check warning on line 170 in packages/aws-cdk/lib/api/resource-import/importer.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/api/resource-import/importer.ts#L170

Added line #L170 was not covered by tests
}

return ret;
Expand All @@ -121,7 +180,7 @@
* @param importMap Mapping from CDK construct tree path to physical resource import identifiers
* @param options Options to pass to CloudFormation deploy operation
*/
public async importResourcesFromMap(importMap: ImportMap, options: ImportDeploymentOptions) {
public async importResourcesFromMap(importMap: ImportMap, options: ImportDeploymentOptions = {}) {
const resourcesToImport: ResourcesToImport = await this.makeResourcesToImport(importMap);
const updatedTemplate = await this.currentTemplateWithAdditions(importMap.importResources);

Expand All @@ -136,7 +195,7 @@
* @param resourcesToImport The mapping created by cdk migrate
* @param options Options to pass to CloudFormation deploy operation
*/
public async importResourcesFromMigrate(resourcesToImport: ResourcesToImport, options: ImportDeploymentOptions) {
public async importResourcesFromMigrate(resourcesToImport: ResourcesToImport, options: ImportDeploymentOptions = {}) {
const updatedTemplate = this.removeNonImportResources();

await this.importResources(updatedTemplate, resourcesToImport, options);
Expand All @@ -158,9 +217,9 @@
? ' ✅ %s (no changes)'
: ' ✅ %s';

success('\n' + message, this.stack.displayName);
await this.ioHost.notify(info(this.action, '\n' + chalk.green(format(message, this.stack.displayName))));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why did we do away with the success helper?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getting rid of all helpers with implicit formatting.

} catch (e) {
error('\n ❌ %s failed: %s', chalk.bold(this.stack.displayName), e);
await this.ioHost.notify(error(this.action, format('\n ❌ %s failed: %s', chalk.bold(this.stack.displayName), e), 'CDK_TOOLKIT_E3900'));

Check warning on line 222 in packages/aws-cdk/lib/api/resource-import/importer.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/api/resource-import/importer.ts#L222

Added line #L222 was not covered by tests
throw e;
}
}
Expand Down Expand Up @@ -189,7 +248,7 @@
const offendingResources = nonAdditions.map(([logId, _]) => this.describeResource(logId));

if (allowNonAdditions) {
warning(`Ignoring updated/deleted resources (--force): ${offendingResources.join(', ')}`);
await this.ioHost.notify(warn(this.action, `Ignoring updated/deleted resources (--force): ${offendingResources.join(', ')}`));
} else {
throw new ToolkitError('No resource updates or deletes are allowed on import operation. Make sure to resolve pending changes ' +
`to existing resources, before attempting an import. Updated/deleted resources: ${offendingResources.join(', ')} (--force to override)`);
Expand Down Expand Up @@ -277,7 +336,7 @@
// Skip resources that do not support importing
const resourceType = chg.resourceDiff.newResourceType;
if (resourceType === undefined || !(resourceType in resourceIdentifiers)) {
warning(`${resourceName}: unsupported resource type ${resourceType}, skipping import.`);
await this.ioHost.notify(warn(this.action, `${resourceName}: unsupported resource type ${resourceType}, skipping import.`));

Check warning on line 339 in packages/aws-cdk/lib/api/resource-import/importer.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/api/resource-import/importer.ts#L339

Added line #L339 was not covered by tests
return undefined;
}

Expand All @@ -303,7 +362,7 @@

// If we got here and the user rejected any available identifiers, then apparently they don't want the resource at all
if (satisfiedPropSets.length > 0) {
info(chalk.grey(`Skipping import of ${resourceName}`));
await this.ioHost.notify(info(this.action, chalk.grey(`Skipping import of ${resourceName}`)));

Check warning on line 365 in packages/aws-cdk/lib/api/resource-import/importer.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/api/resource-import/importer.ts#L365

Added line #L365 was not covered by tests
return undefined;
}

Expand All @@ -321,7 +380,7 @@

// Do the input loop here
if (preamble) {
info(preamble);
await this.ioHost.notify(info(this.action, preamble));
}
for (const idProps of idPropSets) {
const input: Record<string, string> = {};
Expand Down Expand Up @@ -356,7 +415,7 @@
}
}

info(chalk.grey(`Skipping import of ${resourceName}`));
await this.ioHost.notify(info(this.action, chalk.grey(`Skipping import of ${resourceName}`)));
return undefined;
}

Expand Down
2 changes: 2 additions & 0 deletions packages/aws-cdk/lib/api/resource-import/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export * from './importer';
export * from './migrator';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sort of a meta question -- is there a way to block us from importing directly from the file itself when we have an index.ts?

i want to disawllow import * as blah from 'aws-cdk/lib/api/resource-inport/importer'; in favor of import * as blah from 'aws-cdk/lib/api/resource-import'; in this case, so that we don't have multiple ways of doing the same thing

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could write an eslint rule for it, might need a plugin. There can be good reasons to import directly though. My current concern is that anything outside of api should only import form the barrel files. Within api I don't mind so much.

Loading