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

chore(migrate): only allow migrate on healthy stacks #28452

Merged
merged 2 commits into from
Dec 23, 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
5 changes: 5 additions & 0 deletions packages/aws-cdk/lib/api/util/cloudformation/stack-status.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@ export class StackStatus {
return !this.isNotFound && (this.name === 'CREATE_COMPLETE' || this.name === 'UPDATE_COMPLETE' || this.name === 'IMPORT_COMPLETE');
}

get isRollbackSuccess(): boolean {
return this.name === 'ROLLBACK_COMPLETE'
|| this.name === 'UPDATE_ROLLBACK_COMPLETE';
}

public toString(): string {
return this.name + (this.reason ? ` (${this.reason})` : '');
}
Expand Down
11 changes: 8 additions & 3 deletions packages/aws-cdk/lib/commands/migrate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { Environment, UNKNOWN_ACCOUNT, UNKNOWN_REGION } from '@aws-cdk/cx-api';
import * as cdk_from_cfn from 'cdk-from-cfn';
import { cliInit } from '../../lib/init';
import { Mode, SdkProvider } from '../api';
import { CloudFormationStack } from '../api/util/cloudformation';
import { zipDirectory } from '../util/archive';

const camelCase = require('camelcase');
Expand Down Expand Up @@ -112,9 +113,13 @@ export function readFromPath(inputPath?: string): string | undefined {
export async function readFromStack(stackName: string, sdkProvider: SdkProvider, environment: Environment): Promise<string | undefined> {
const cloudFormation = (await sdkProvider.forEnvironment(environment, Mode.ForReading)).sdk.cloudFormation();

return (await cloudFormation.getTemplate({
StackName: stackName,
}).promise()).TemplateBody;
const stack = await CloudFormationStack.lookup(cloudFormation, stackName);
if (stack.stackStatus.isDeploySuccess || stack.stackStatus.isRollbackSuccess) {
return JSON.stringify(await stack.template());
} else {
throw new Error(`Stack '${stackName}' in account ${environment.account} and region ${environment.region} has a status of '${stack.stackStatus.name}' due to '${stack.stackStatus.reason}'. The stack cannot be migrated until it is in a healthy state.`);
}
return;
}

/**
Expand Down
8 changes: 4 additions & 4 deletions packages/aws-cdk/test/cdk-toolkit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -869,7 +869,7 @@ describe('synth', () => {

test('migrate fails when neither --from-path or --from-stack are provided', async () => {
const toolkit = defaultToolkitSetup();
await expect(() => toolkit.migrate({ stackName: 'no-source' })).rejects.toThrowError('Either `--from-path` or `--from-stack` must be used to provide the source of the CloudFormation template.');
await expect(() => toolkit.migrate({ stackName: 'no-source' })).rejects.toThrow('Either `--from-path` or `--from-stack` must be used to provide the source of the CloudFormation template.');
expect(stderrMock.mock.calls[1][0]).toContain(' ❌ Migrate failed for `no-source`: Either `--from-path` or `--from-stack` must be used to provide the source of the CloudFormation template.');
});

Expand All @@ -879,7 +879,7 @@ describe('synth', () => {
stackName: 'no-source',
fromPath: './here/template.yml',
fromStack: true,
})).rejects.toThrowError('Only one of `--from-path` or `--from-stack` may be provided.');
})).rejects.toThrow('Only one of `--from-path` or `--from-stack` may be provided.');
expect(stderrMock.mock.calls[1][0]).toContain(' ❌ Migrate failed for `no-source`: Only one of `--from-path` or `--from-stack` may be provided.');
});

Expand All @@ -888,14 +888,14 @@ describe('synth', () => {
await expect(() => toolkit.migrate({
stackName: 'bad-local-source',
fromPath: './here/template.yml',
})).rejects.toThrowError('\'./here/template.yml\' is not a valid path.');
})).rejects.toThrow('\'./here/template.yml\' is not a valid path.');
expect(stderrMock.mock.calls[1][0]).toContain(' ❌ Migrate failed for `bad-local-source`: \'./here/template.yml\' is not a valid path.');
});

test('migrate fails when --from-stack is used and stack does not exist in account', async () => {
const mockSdkProvider = new MockSdkProvider();
mockSdkProvider.stubCloudFormation({
getTemplate(_request) {
describeStacks(_request) {
throw new Error('Stack does not exist in this environment');
},
});
Expand Down
39 changes: 32 additions & 7 deletions packages/aws-cdk/test/commands/migrate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const exec = promisify(_exec);
describe('Migrate Function Tests', () => {
let sdkProvider: MockSdkProvider;
let getTemplateMock: jest.Mock;
let describeStacksMock: jest.Mock;
let cfnMocks: MockedObject<SyncHandlerSubsetOf<AWS.CloudFormation>>;

const testResourcePath = [__dirname, 'test-resources'];
Expand All @@ -20,10 +21,11 @@ describe('Migrate Function Tests', () => {
const validTemplatePath = path.join(...templatePath, 's3-template.json');
const validTemplate = readFromPath(validTemplatePath)!;

beforeEach(() => {
beforeEach(async () => {
sdkProvider = new MockSdkProvider();
getTemplateMock = jest.fn();
cfnMocks = { getTemplate: getTemplateMock };
describeStacksMock = jest.fn();
cfnMocks = { getTemplate: getTemplateMock, describeStacks: describeStacksMock };
sdkProvider.stubCloudFormation(cfnMocks as any);
});

Expand Down Expand Up @@ -57,17 +59,40 @@ describe('Migrate Function Tests', () => {
});

test('readFromStack produces a string representation of the template retrieved from CloudFormation', async () => {
const template = fs.readFileSync(validTemplatePath);
getTemplateMock.mockImplementationOnce(() => ({
const template = fs.readFileSync(validTemplatePath, { encoding: 'utf-8' });
getTemplateMock.mockImplementation(() => ({
TemplateBody: template,
}));

expect(await readFromStack('this-one', sdkProvider, { account: 'num', region: 'here', name: 'hello-my-name-is-what...' })).toEqual(template);
describeStacksMock.mockImplementation(() => ({
Stacks: [
{
StackName: 'this-one',
StackStatus: 'CREATE_COMPLETE',
},
],
}));

expect(await readFromStack('this-one', sdkProvider, { account: 'num', region: 'here', name: 'hello-my-name-is-what...' })).toEqual(JSON.stringify(JSON.parse(template)));
});

test('readFromStack throws error when no stack exists with the stack name in the account and region', async () => {
getTemplateMock.mockImplementationOnce(() => { throw new Error('No stack. This did not go well.'); });
await expect(() => readFromStack('that-one', sdkProvider, { account: 'num', region: 'here', name: 'hello-my-name-is-who...' })).rejects.toThrowError('No stack. This did not go well.');
describeStacksMock.mockImplementation(() => { throw new Error('No stack. This did not go well.'); });
await expect(() => readFromStack('that-one', sdkProvider, { account: 'num', region: 'here', name: 'hello-my-name-is-who...' })).rejects.toThrow('No stack. This did not go well.');
});

test('readFromStack throws error when stack exists but the status is not healthy', async () => {
describeStacksMock.mockImplementation(() => ({
Stacks: [
{
StackName: 'this-one',
StackStatus: 'CREATE_FAILED',
StackStatusReason: 'Something went wrong',
},
],
}));

await expect(() => readFromStack('that-one', sdkProvider, { account: 'num', region: 'here', name: 'hello-my-name-is-chicka-chicka...' })).rejects.toThrow('Stack \'that-one\' in account num and region here has a status of \'CREATE_FAILED\' due to \'Something went wrong\'. The stack cannot be migrated until it is in a healthy state.');
});

test('setEnvironment sets account and region when provided', () => {
Expand Down
Loading