From 0318011a54beaafa1c522b3563e6fdd409593d66 Mon Sep 17 00:00:00 2001 From: Christopher Sundersingh <83315412+sundersc@users.noreply.github.com> Date: Fri, 24 Sep 2021 10:58:41 -0700 Subject: [PATCH] Revert "feat: Flag to allow schema changes that require table replacement (#8144)" This reverts commit 2d4e65acfd034d33c6fa8ac1f5f8582e7e3bc399. --- .circleci/config.yml | 46 +--- .../amplify-category-api/amplify-plugin.json | 25 +- packages/amplify-category-api/package.json | 1 - .../__tests__/commands/api/rebuild.test.ts | 64 ----- .../amplify-category-api/src/commands/api.js | 5 - .../src/commands/api/rebuild.ts | 40 --- packages/amplify-category-api/tsconfig.json | 2 - .../amplify-category-function/src/index.ts | 1 - packages/amplify-cli-core/src/index.ts | 1 - .../amplify-helpers/push-resources.ts | 37 ++- .../amplify-e2e-core/src/categories/api.ts | 10 - .../amplify-e2e-core/src/init/amplifyPush.ts | 17 +- .../amplify-e2e-core/src/utils/sdk-calls.ts | 10 - .../simple_model_new_primary_key.graphql | 4 - .../src/__tests__/api_5.test.ts | 84 ------- packages/amplify-prompts/src/validators.ts | 73 ++---- .../__snapshots__/utils.test.ts.snap | 114 --------- .../utils.test.ts | 236 ------------------ .../src/constants.js | 1 - .../disconnect-dependent-resources/index.ts | 64 ----- .../disconnect-dependent-resources/utils.ts | 183 -------------- .../amplify-graphql-resource-manager.ts | 65 +---- .../src/graphql-transformer/utils.ts | 19 +- .../src/index.ts | 4 +- .../deployment-manager.ts | 6 - .../src/push-resources.ts | 53 ++-- .../src/transform-graphql-schema.ts | 11 +- .../__snapshots__/amplifyUtils.test.ts.snap | 28 --- .../src/__tests__/util/amplifyUtils.test.ts | 20 -- .../graphql-transformer-core/src/errors.ts | 32 +-- .../src/util/amplifyUtils.ts | 55 ++-- .../src/util/sanity-check.ts | 220 +++++++--------- 32 files changed, 233 insertions(+), 1298 deletions(-) delete mode 100644 packages/amplify-category-api/src/__tests__/commands/api/rebuild.test.ts delete mode 100644 packages/amplify-category-api/src/commands/api/rebuild.ts delete mode 100644 packages/amplify-e2e-tests/schemas/simple_model_new_primary_key.graphql delete mode 100644 packages/amplify-e2e-tests/src/__tests__/api_5.test.ts delete mode 100644 packages/amplify-provider-awscloudformation/src/__tests__/disconnect-dependent-resources/__snapshots__/utils.test.ts.snap delete mode 100644 packages/amplify-provider-awscloudformation/src/__tests__/disconnect-dependent-resources/utils.test.ts delete mode 100644 packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/index.ts delete mode 100644 packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/utils.ts diff --git a/.circleci/config.yml b/.circleci/config.yml index ce6732cbf98..3df8618060e 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -1299,14 +1299,6 @@ jobs: environment: TEST_SUITE: src/__tests__/auth_6.test.ts CLI_REGION: eu-west-2 - api_5-amplify_e2e_tests: - working_directory: ~/repo - docker: *ref_1 - resource_class: large - steps: *ref_5 - environment: - TEST_SUITE: src/__tests__/api_5.test.ts - CLI_REGION: eu-central-1 api_4-amplify_e2e_tests: working_directory: ~/repo docker: *ref_1 @@ -1314,7 +1306,7 @@ jobs: steps: *ref_5 environment: TEST_SUITE: src/__tests__/api_4.test.ts - CLI_REGION: ap-northeast-1 + CLI_REGION: eu-central-1 schema-iterative-update-4-amplify_e2e_tests_pkg_linux: working_directory: ~/repo docker: *ref_1 @@ -2115,16 +2107,6 @@ jobs: TEST_SUITE: src/__tests__/auth_6.test.ts CLI_REGION: eu-west-2 steps: *ref_6 - api_5-amplify_e2e_tests_pkg_linux: - working_directory: ~/repo - docker: *ref_1 - resource_class: large - environment: - AMPLIFY_DIR: /home/circleci/repo/out - AMPLIFY_PATH: /home/circleci/repo/out/amplify-pkg-linux - TEST_SUITE: src/__tests__/api_5.test.ts - CLI_REGION: eu-central-1 - steps: *ref_6 api_4-amplify_e2e_tests_pkg_linux: working_directory: ~/repo docker: *ref_1 @@ -2133,7 +2115,7 @@ jobs: AMPLIFY_DIR: /home/circleci/repo/out AMPLIFY_PATH: /home/circleci/repo/out/amplify-pkg-linux TEST_SUITE: src/__tests__/api_4.test.ts - CLI_REGION: ap-northeast-1 + CLI_REGION: eu-central-1 steps: *ref_6 workflows: version: 2 @@ -2252,11 +2234,11 @@ workflows: - notifications-amplify_e2e_tests - schema-iterative-update-locking-amplify_e2e_tests - function_7-amplify_e2e_tests - - api_5-amplify_e2e_tests + - api_4-amplify_e2e_tests + - hosting-amplify_e2e_tests - tags-amplify_e2e_tests - s3-sse-amplify_e2e_tests - function_6-amplify_e2e_tests - - api_4-amplify_e2e_tests - amplify-app-amplify_e2e_tests - init-amplify_e2e_tests - pull-amplify_e2e_tests @@ -2282,11 +2264,11 @@ workflows: - notifications-amplify_e2e_tests_pkg_linux - schema-iterative-update-locking-amplify_e2e_tests_pkg_linux - function_7-amplify_e2e_tests_pkg_linux - - api_5-amplify_e2e_tests_pkg_linux + - api_4-amplify_e2e_tests_pkg_linux + - hosting-amplify_e2e_tests_pkg_linux - tags-amplify_e2e_tests_pkg_linux - s3-sse-amplify_e2e_tests_pkg_linux - function_6-amplify_e2e_tests_pkg_linux - - api_4-amplify_e2e_tests_pkg_linux - amplify-app-amplify_e2e_tests_pkg_linux - init-amplify_e2e_tests_pkg_linux - pull-amplify_e2e_tests_pkg_linux @@ -2726,7 +2708,7 @@ workflows: filters: *ref_10 requires: - geo-remove-amplify_e2e_tests - - api_5-amplify_e2e_tests: + - api_4-amplify_e2e_tests: context: *ref_8 post-steps: *ref_9 filters: *ref_10 @@ -2798,12 +2780,6 @@ workflows: filters: *ref_10 requires: - geo-update-amplify_e2e_tests - - api_4-amplify_e2e_tests: - context: *ref_8 - post-steps: *ref_9 - filters: *ref_10 - requires: - - hosting-amplify_e2e_tests - schema-auth-2-amplify_e2e_tests: context: *ref_8 post-steps: *ref_9 @@ -3244,7 +3220,7 @@ workflows: filters: *ref_13 requires: - geo-remove-amplify_e2e_tests_pkg_linux - - api_5-amplify_e2e_tests_pkg_linux: + - api_4-amplify_e2e_tests_pkg_linux: context: *ref_11 post-steps: *ref_12 filters: *ref_13 @@ -3320,12 +3296,6 @@ workflows: filters: *ref_13 requires: - geo-update-amplify_e2e_tests_pkg_linux - - api_4-amplify_e2e_tests_pkg_linux: - context: *ref_11 - post-steps: *ref_12 - filters: *ref_13 - requires: - - hosting-amplify_e2e_tests_pkg_linux - schema-auth-2-amplify_e2e_tests_pkg_linux: context: *ref_11 post-steps: *ref_12 diff --git a/packages/amplify-category-api/amplify-plugin.json b/packages/amplify-category-api/amplify-plugin.json index 8bd97a6a3c7..8cf4ad9d46c 100644 --- a/packages/amplify-category-api/amplify-plugin.json +++ b/packages/amplify-category-api/amplify-plugin.json @@ -1,9 +1,18 @@ { - "name": "api", - "type": "category", - "commands": ["add-graphql-datasource", "add", "console", "gql-compile", "push", "rebuild", "remove", "update", "help"], - "commandAliases": { - "configure": "update" - }, - "eventHandlers": [] -} + "name": "api", + "type": "category", + "commands": [ + "add-graphql-datasource", + "add", + "console", + "gql-compile", + "push", + "remove", + "update", + "help" + ], + "commandAliases":{ + "configure": "update" + }, + "eventHandlers": [] +} \ No newline at end of file diff --git a/packages/amplify-category-api/package.json b/packages/amplify-category-api/package.json index 31940a9ea6e..04d8cdc1964 100644 --- a/packages/amplify-category-api/package.json +++ b/packages/amplify-category-api/package.json @@ -65,7 +65,6 @@ "@octokit/rest": "^18.0.9", "amplify-cli-core": "1.29.0", "amplify-headless-interface": "1.10.0", - "amplify-prompts": "1.1.2", "amplify-util-headless-input": "1.5.4", "chalk": "^4.1.1", "constructs": "^3.3.125", diff --git a/packages/amplify-category-api/src/__tests__/commands/api/rebuild.test.ts b/packages/amplify-category-api/src/__tests__/commands/api/rebuild.test.ts deleted file mode 100644 index ce28470ecd0..00000000000 --- a/packages/amplify-category-api/src/__tests__/commands/api/rebuild.test.ts +++ /dev/null @@ -1,64 +0,0 @@ -import { $TSContext, FeatureFlags, stateManager } from 'amplify-cli-core'; -import { printer, prompter } from 'amplify-prompts'; -import { mocked } from 'ts-jest/utils'; -import { run } from '../../../commands/api/rebuild'; - -jest.mock('amplify-cli-core'); -jest.mock('amplify-prompts'); - -const FeatureFlags_mock = mocked(FeatureFlags); -const stateManager_mock = mocked(stateManager); -const printer_mock = mocked(printer); -const prompter_mock = mocked(prompter); - -FeatureFlags_mock.getBoolean.mockReturnValue(true); - -beforeEach(jest.clearAllMocks); - -const pushResourcesMock = jest.fn(); - -const context_stub = { - amplify: { - constructExeInfo: jest.fn(), - pushResources: pushResourcesMock, - }, - parameters: { - first: 'resourceName', - }, -} as unknown as $TSContext; - -it('prints error if iterative updates not enabled', async () => { - FeatureFlags_mock.getBoolean.mockReturnValueOnce(false); - - await run(context_stub); - - expect(printer_mock.error.mock.calls.length).toBe(1); - expect(pushResourcesMock.mock.calls.length).toBe(0); -}); - -it('exits early if no api in project', async () => { - stateManager_mock.getMeta.mockReturnValueOnce({ - api: {}, - }); - - await run(context_stub); - - expect(printer_mock.info.mock.calls.length).toBe(1); - expect(pushResourcesMock.mock.calls.length).toBe(0); -}); - -it('asks for strong confirmation before continuing', async () => { - stateManager_mock.getMeta.mockReturnValueOnce({ - api: { - testapiname: { - service: 'AppSync', - }, - }, - }); - - await run(context_stub); - - expect(prompter_mock.input.mock.calls.length).toBe(1); - expect(pushResourcesMock.mock.calls.length).toBe(1); - expect(pushResourcesMock.mock.calls[0][4]).toBe(true); // rebuild flag is set -}); diff --git a/packages/amplify-category-api/src/commands/api.js b/packages/amplify-category-api/src/commands/api.js index e8c261e00df..948dc4ae4a2 100644 --- a/packages/amplify-category-api/src/commands/api.js +++ b/packages/amplify-category-api/src/commands/api.js @@ -41,11 +41,6 @@ module.exports = { name: 'console', description: 'Opens the web console for the selected api service', }, - { - name: 'rebuild', - description: - 'Removes and recreates all DynamoDB tables backing a GraphQL API. Useful for resetting test data during the development phase of an app', - }, ]; context.amplify.showHelp(header, commands); diff --git a/packages/amplify-category-api/src/commands/api/rebuild.ts b/packages/amplify-category-api/src/commands/api/rebuild.ts deleted file mode 100644 index 2173ba77c49..00000000000 --- a/packages/amplify-category-api/src/commands/api/rebuild.ts +++ /dev/null @@ -1,40 +0,0 @@ -import { $TSContext, FeatureFlags, stateManager } from 'amplify-cli-core'; -import { printer, prompter, exact } from 'amplify-prompts'; - -const subcommand = 'rebuild'; -const category = 'api'; - -export const name = subcommand; - -const rebuild = true; - -export const run = async (context: $TSContext) => { - if (!FeatureFlags.getBoolean('graphqlTransformer.enableIterativeGSIUpdates')) { - printer.error('Iterative GSI Updates must be enabled to rebuild an API. See https://docs.amplify.aws/cli/reference/feature-flags/'); - return; - } - const apiNames = Object.entries(stateManager.getMeta()?.api || {}) - .filter(([_, meta]) => (meta as any).service === 'AppSync') - .map(([name]) => name); - if (apiNames.length === 0) { - printer.info('No GraphQL API configured in the project. Only GraphQL APIs can be rebuilt. To add a GraphQL API run `amplify add api`.'); - return; - } - if (apiNames.length > 1) { - // this condition should never hit as we have upstream defensive logic to prevent multiple GraphQL APIs. But just to cover all the bases - printer.error( - 'You have multiple GraphQL APIs in the project. Only one GraphQL API is allowed per project. Run `amplify remove api` to remove an API.', - ); - return; - } - const apiName = apiNames[0]; - printer.warn(`This will recreate all tables backing models in your GraphQL API ${apiName}.`); - printer.warn('ALL EXISTING DATA IN THESE TABLES WILL BE LOST.'); - await prompter.input('Type the name of the API to confirm you want to continue', { - validate: exact(apiName, 'Input does not match the GraphQL API name'), - }); - const { amplify, parameters } = context; - const resourceName = parameters.first; - amplify.constructExeInfo(context); - return amplify.pushResources(context, category, resourceName, undefined, rebuild); -}; diff --git a/packages/amplify-category-api/tsconfig.json b/packages/amplify-category-api/tsconfig.json index 31c85559bbb..dd332b004a4 100644 --- a/packages/amplify-category-api/tsconfig.json +++ b/packages/amplify-category-api/tsconfig.json @@ -14,9 +14,7 @@ "src/__tests__" ], "references": [ - {"path": "../amplify-cli-core"}, {"path": "../amplify-headless-interface"}, - {"path": "../amplify-prompts"}, {"path": "../graphql-transformer-core"}, {"path": "../amplify-util-headless-input"}, ] diff --git a/packages/amplify-category-function/src/index.ts b/packages/amplify-category-function/src/index.ts index 63a6841431e..0d6de3a6976 100644 --- a/packages/amplify-category-function/src/index.ts +++ b/packages/amplify-category-function/src/index.ts @@ -28,7 +28,6 @@ export { hashLayerResource } from './provider-utils/awscloudformation/utils/laye export { migrateLegacyLayer } from './provider-utils/awscloudformation/utils/layerMigrationUtils'; export { packageResource } from './provider-utils/awscloudformation/utils/package'; export { updateDependentFunctionsCfn } from './provider-utils/awscloudformation/utils/updateDependentFunctionCfn'; -export { loadFunctionParameters } from './provider-utils/awscloudformation/utils/loadFunctionParameters'; export async function add(context, providerName, service, parameters) { const options = { diff --git a/packages/amplify-cli-core/src/index.ts b/packages/amplify-cli-core/src/index.ts index c4ad0a04041..3051364baa7 100644 --- a/packages/amplify-cli-core/src/index.ts +++ b/packages/amplify-cli-core/src/index.ts @@ -251,7 +251,6 @@ interface AmplifyToolkit { category?: string, resourceName?: string, filteredResources?: { category: string; resourceName: string }[], - rebuild?: boolean, ) => $TSAny; storeCurrentCloudBackend: () => $TSAny; readJsonFile: (fileName: string) => $TSAny; diff --git a/packages/amplify-cli/src/extensions/amplify-helpers/push-resources.ts b/packages/amplify-cli/src/extensions/amplify-helpers/push-resources.ts index 3b86fc0c5e2..4e71549cc77 100644 --- a/packages/amplify-cli/src/extensions/amplify-helpers/push-resources.ts +++ b/packages/amplify-cli/src/extensions/amplify-helpers/push-resources.ts @@ -11,7 +11,6 @@ export async function pushResources( category?: string, resourceName?: string, filteredResources?: { category: string; resourceName: string }[], - rebuild: boolean = false, ) { if (context.parameters.options['iterative-rollback']) { // validate --iterative-rollback with --force @@ -50,21 +49,16 @@ export async function pushResources( } } - let hasChanges = false; - if (!rebuild) { - // status table does not have a way to show resource in "rebuild" state so skipping it to avoid confusion - hasChanges = await showResourceTable(category, resourceName, filteredResources); - } + const hasChanges = await showResourceTable(category, resourceName, filteredResources); // no changes detected - if (!hasChanges && !context.exeInfo.forcePush && !rebuild) { + if (!hasChanges && !context.exeInfo.forcePush) { context.print.info('\nNo changes detected'); return context; } - // rebuild has an upstream confirmation prompt so no need to prompt again here - let continueToPush = (context.exeInfo && context.exeInfo.inputParams && context.exeInfo.inputParams.yes) || rebuild; + let continueToPush = context.exeInfo && context.exeInfo.inputParams && context.exeInfo.inputParams.yes; if (!continueToPush) { if (context.exeInfo.iterativeRollback) { @@ -74,11 +68,18 @@ export async function pushResources( } if (continueToPush) { - // Get current-cloud-backend's amplify-meta - const currentAmplifyMeta = stateManager.getCurrentMeta(); + try { + // Get current-cloud-backend's amplify-meta + const currentAmplifyMeta = stateManager.getCurrentMeta(); + + await providersPush(context, category, resourceName, filteredResources); + await onCategoryOutputsChange(context, currentAmplifyMeta); + } catch (err) { + // Handle the errors and print them nicely for the user. + context.print.error(`\n${err.message}`); - await providersPush(context, rebuild, category, resourceName, filteredResources); - await onCategoryOutputsChange(context, currentAmplifyMeta); + throw err; + } } else { // there's currently no other mechanism to stop the execution of the postPush workflow in this case, so exiting here exitOnNextTick(1); @@ -87,13 +88,7 @@ export async function pushResources( return continueToPush; } -async function providersPush( - context: $TSContext, - rebuild: boolean = false, - category?: string, - resourceName?: string, - filteredResources?: { category: string; resourceName: string }[], -) { +async function providersPush(context: $TSContext, category, resourceName, filteredResources) { const { providers } = getProjectConfig(); const providerPlugins = getProviderPlugins(context); const providerPromises: (() => Promise<$TSAny>)[] = []; @@ -101,7 +96,7 @@ async function providersPush( for (const provider of providers) { const providerModule = require(providerPlugins[provider]); const resourceDefiniton = await context.amplify.getResourceStatus(category, resourceName, provider, filteredResources); - providerPromises.push(providerModule.pushResources(context, resourceDefiniton, rebuild)); + providerPromises.push(providerModule.pushResources(context, resourceDefiniton)); } await Promise.all(providerPromises); diff --git a/packages/amplify-e2e-core/src/categories/api.ts b/packages/amplify-e2e-core/src/categories/api.ts index 392ad64a427..571e91efc48 100644 --- a/packages/amplify-e2e-core/src/categories/api.ts +++ b/packages/amplify-e2e-core/src/categories/api.ts @@ -566,13 +566,3 @@ export function addRestContainerApi(projectDir: string) { }); }); } - -export function rebuildApi(projDir: string, apiName: string) { - return new Promise((resolve, reject) => { - spawn(getCLIPath(), ['rebuild', 'api'], { cwd: projDir, stripColors: true }) - .wait('Type the name of the API to confirm you want to continue') - .sendLine(apiName) - .wait('All resources are updated in the cloud') - .run(err => (err ? reject(err) : resolve())); - }); -} diff --git a/packages/amplify-e2e-core/src/init/amplifyPush.ts b/packages/amplify-e2e-core/src/init/amplifyPush.ts index bf359bb3750..6bf71b874fa 100644 --- a/packages/amplify-e2e-core/src/init/amplifyPush.ts +++ b/packages/amplify-e2e-core/src/init/amplifyPush.ts @@ -15,7 +15,7 @@ export function amplifyPush(cwd: string, testingWithLatestCodebase: boolean = fa spawn(getCLIPath(testingWithLatestCodebase), ['status', '-v'], { cwd, stripColors: true, noOutputTimeout: pushTimeoutMS }) .wait(/.*/) .run((err: Error) => { - if (err) { + if ( err ){ reject(err); } }); @@ -254,18 +254,3 @@ export function amplifyPushWithNoChanges(cwd: string, testingWithLatestCodebase: .run((err: Error) => err ? reject(err) : resolve()); }); } - -export function amplifyPushDestructiveApiUpdate(cwd: string, includeForce: boolean) { - return new Promise((resolve, reject) => { - const args = ['push', '--yes']; - if (includeForce) { - args.push('--force'); - } - const chain = spawn(getCLIPath(), args, { cwd, stripColors: true }); - if (includeForce) { - chain.wait('All resources are updated in the cloud').run(err => (err ? reject(err) : resolve())); - } else { - chain.wait('If this is intended, rerun the command with').run(err => (err ? resolve(err) : reject())); // in this case, we expect the CLI to error out - } - }); -} diff --git a/packages/amplify-e2e-core/src/utils/sdk-calls.ts b/packages/amplify-e2e-core/src/utils/sdk-calls.ts index 3b4da4e0f8f..129561f4e1d 100644 --- a/packages/amplify-e2e-core/src/utils/sdk-calls.ts +++ b/packages/amplify-e2e-core/src/utils/sdk-calls.ts @@ -201,16 +201,6 @@ export const deleteTable = async (tableName: string, region: string) => { return await service.deleteTable({ TableName: tableName }).promise(); }; -export const putItemInTable = async (tableName: string, region: string, item: unknown) => { - const ddb = new DynamoDB.DocumentClient({ region }); - return await ddb.put({ TableName: tableName, Item: item }).promise(); -}; - -export const scanTable = async (tableName: string, region: string) => { - const ddb = new DynamoDB.DocumentClient({ region }); - return await ddb.scan({ TableName: tableName }).promise(); -}; - export const getAppSyncApi = async (appSyncApiId: string, region: string) => { const service = new AppSync({ region }); return await service.getGraphqlApi({ apiId: appSyncApiId }).promise(); diff --git a/packages/amplify-e2e-tests/schemas/simple_model_new_primary_key.graphql b/packages/amplify-e2e-tests/schemas/simple_model_new_primary_key.graphql deleted file mode 100644 index cdbe2826bfe..00000000000 --- a/packages/amplify-e2e-tests/schemas/simple_model_new_primary_key.graphql +++ /dev/null @@ -1,4 +0,0 @@ -type Todo @model @key(fields: ["content"]) { - id: ID! - content: String! -} diff --git a/packages/amplify-e2e-tests/src/__tests__/api_5.test.ts b/packages/amplify-e2e-tests/src/__tests__/api_5.test.ts deleted file mode 100644 index dda41cf1685..00000000000 --- a/packages/amplify-e2e-tests/src/__tests__/api_5.test.ts +++ /dev/null @@ -1,84 +0,0 @@ -import { - createNewProjectDir, - initJSProjectWithProfile, - addApiWithSchema, - amplifyPush, - deleteProject, - deleteProjectDir, - putItemInTable, - scanTable, - rebuildApi, - getProjectMeta, - updateApiSchema, - amplifyPushDestructiveApiUpdate, - addFunction, - amplifyPushAuth, -} from 'amplify-e2e-core'; - -const projName = 'apitest'; -let projRoot; -beforeEach(async () => { - projRoot = await createNewProjectDir(projName); - await initJSProjectWithProfile(projRoot, { name: projName }); - await addApiWithSchema(projRoot, 'simple_model.graphql', { apiKeyExpirationDays: 7 }); - await amplifyPush(projRoot); -}); -afterEach(async () => { - await deleteProject(projRoot); - deleteProjectDir(projRoot); -}); - -describe('amplify reset api', () => { - it('recreates all model tables', async () => { - const projMeta = getProjectMeta(projRoot); - const apiId = projMeta?.api?.[projName]?.output?.GraphQLAPIIdOutput; - const region = projMeta?.providers?.awscloudformation?.Region; - expect(apiId).toBeDefined(); - expect(region).toBeDefined(); - const tableName = `Todo-${apiId}-integtest`; - await putItemInTable(tableName, region, { id: 'this is a test value' }); - const scanResultBefore = await scanTable(tableName, region); - expect(scanResultBefore.Items.length).toBe(1); - - await rebuildApi(projRoot, projName); - - const scanResultAfter = await scanTable(tableName, region); - expect(scanResultAfter.Items.length).toBe(0); - }); -}); - -describe('destructive updates flag', () => { - it('blocks destructive updates when flag not present', async () => { - updateApiSchema(projRoot, projName, 'simple_model_new_primary_key.graphql'); - await amplifyPushDestructiveApiUpdate(projRoot, false); - // success indicates that the command errored out - }); - - it('allows destructive updates when flag present', async () => { - updateApiSchema(projRoot, projName, 'simple_model_new_primary_key.graphql'); - await amplifyPushDestructiveApiUpdate(projRoot, true); - // success indicates that the push completed - }); - - it('disconnects and reconnects functions dependent on replaced table', async () => { - const functionName = 'funcTableDep'; - await addFunction( - projRoot, - { - name: functionName, - functionTemplate: 'Hello World', - additionalPermissions: { - permissions: ['storage'], - choices: ['api', 'storage'], - resources: ['Todo:@model(appsync)'], - operations: ['create', 'read', 'update', 'delete'], - }, - }, - 'nodejs', - ); - await amplifyPushAuth(projRoot); - updateApiSchema(projRoot, projName, 'simple_model_new_primary_key.graphql'); - await amplifyPushDestructiveApiUpdate(projRoot, false); - // success indicates that the push completed - }); -}); diff --git a/packages/amplify-prompts/src/validators.ts b/packages/amplify-prompts/src/validators.ts index 9651f4125dc..d917ca606c9 100644 --- a/packages/amplify-prompts/src/validators.ts +++ b/packages/amplify-prompts/src/validators.ts @@ -12,69 +12,50 @@ export type Validator = (value: string) => true | string | Promise - (input: string) => - /^[a-zA-Z0-9]+$/.test(input) ? true : message; +export const alphanumeric = (message: string = 'Input must be alphanumeric'): Validator => (input: string) => + /^[a-zA-Z0-9]+$/.test(input) ? true : message; -export const integer = - (message: string = 'Input must be a number'): Validator => - (input: string) => - /^[0-9]+$/.test(input) ? true : message; +export const integer = (message: string = 'Input must be a number'): Validator => (input: string) => + /^[0-9]+$/.test(input) ? true : message; -export const maxLength = - (maxLen: number, message?: string): Validator => - (input: string) => - input.length > maxLen ? message || `Input must be less than ${maxLen} characters long` : true; +export const maxLength = (maxLen: number, message?: string): Validator => (input: string) => + input.length > maxLen ? message || `Input must be less than ${maxLen} characters long` : true; -export const minLength = - (minLen: number, message?: string): Validator => - (input: string) => - input.length < minLen ? message || `Input must be more than ${minLen} characters long` : true; - -export const exact = - (expected: string, message?: string): Validator => - (input: string) => - input === expected ? true : message ?? 'Input does not match expected value'; +export const minLength = (minLen: number, message?: string): Validator => (input: string) => + input.length < minLen ? message || `Input must be more than ${minLen} characters long` : true; /** * Logically "and"s several validators * If a validator returns an error message, that message is returned by this function, unless an override message is specified */ -export const and = - (validators: [Validator, Validator, ...Validator[]], message?: string): Validator => - async (input: string) => { - for (const validator of validators) { - const result = await validator(input); - if (typeof result === 'string') { - return message ?? result; - } +export const and = (validators: [Validator, Validator, ...Validator[]], message?: string): Validator => async (input: string) => { + for (const validator of validators) { + const result = await validator(input); + if (typeof result === 'string') { + return message ?? result; } - return true; - }; + } + return true; +}; /** * Logically "or" several validators * If all validators return an error message, the LAST error message is returned by this function, unless an override message is specified */ -export const or = - (validators: [Validator, Validator, ...Validator[]], message?: string): Validator => - async (input: string) => { - let result: string | true = true; - for (const validator of validators) { - result = await validator(input); - if (result === true) { - return true; - } +export const or = (validators: [Validator, Validator, ...Validator[]], message?: string): Validator => async (input: string) => { + let result: string | true = true; + for (const validator of validators) { + result = await validator(input); + if (result === true) { + return true; } - return message ?? result; - }; + } + return message ?? result; +}; /** * Logical not operator on a validator * If validator returns true, it returns message. If validator returns an error message, it returns true. */ -export const not = - (validator: Validator, message: string): Validator => - async (input: string) => - typeof (await validator(input)) === 'string' ? true : message; +export const not = (validator: Validator, message: string): Validator => async (input: string) => + typeof (await validator(input)) === 'string' ? true : message; diff --git a/packages/amplify-provider-awscloudformation/src/__tests__/disconnect-dependent-resources/__snapshots__/utils.test.ts.snap b/packages/amplify-provider-awscloudformation/src/__tests__/disconnect-dependent-resources/__snapshots__/utils.test.ts.snap deleted file mode 100644 index 6ae1c362769..00000000000 --- a/packages/amplify-provider-awscloudformation/src/__tests__/disconnect-dependent-resources/__snapshots__/utils.test.ts.snap +++ /dev/null @@ -1,114 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`generateIterativeFuncDeploymentSteps generates steps with correct pointers 1`] = ` -Object { - "deploymentSteps": Array [ - Object { - "deployment": Object { - "capabilities": Array [], - "parameters": Object { - "param1": "value1", - }, - "previousMetaKey": undefined, - "stackName": "testStackId", - "stackTemplatePathOrUrl": "amplify-cfn-templates/function/temp/temp-func1-cloudformation-template.json", - "tableNames": Array [], - }, - "rollback": undefined, - }, - Object { - "deployment": Object { - "capabilities": Array [], - "parameters": Object { - "param2": "value2", - }, - "previousMetaKey": "amplify-cfn-templates/function/temp/temp-func1-deployment-meta.json", - "stackName": "testStackId", - "stackTemplatePathOrUrl": "amplify-cfn-templates/function/temp/temp-func2-cloudformation-template.json", - "tableNames": Array [], - }, - "rollback": Object { - "capabilities": Array [], - "parameters": Object { - "param1": "value1", - }, - "previousMetaKey": undefined, - "stackName": "testStackId", - "stackTemplatePathOrUrl": "amplify-cfn-templates/function/temp/temp-func1-cloudformation-template.json", - "tableNames": Array [], - }, - }, - ], - "lastMetaKey": "amplify-cfn-templates/function/temp/temp-func2-deployment-meta.json", -} -`; - -exports[`generateTempFuncCFNTemplates replaces Fn::ImportValue references with placeholder values in template 1`] = ` -Object { - "a": Object { - "b": Object { - "c": Array [ - Object { - "Fn::ImportValue": undefined, - "Fn::Sub": "TemporaryPlaceholderValue", - }, - Object { - "Fn::Join": Array [ - ":", - Object { - "Fn::ImportValue": undefined, - "Fn::Sub": "TemporaryPlaceholderValue", - }, - ], - }, - ], - }, - "d": Object { - "Fn::ImportValue": undefined, - "Fn::Sub": "TemporaryPlaceholderValue", - }, - }, -} -`; - -exports[`prependDeploymentSteps concatenates arrays and moves pointers appropriately 1`] = ` -Array [ - Object { - "deployment": Object { - "previousMetaKey": undefined, - "stackTemplatePathOrUrl": "deploymentStep1", - }, - "rollback": undefined, - }, - Object { - "deployment": Object { - "previousMetaKey": "deploymentStep1MetaKey", - "stackTemplatePathOrUrl": "deploymentStep2", - }, - "rollback": Object { - "previousMetaKey": undefined, - "stackTemplatePathOrUrl": "deploymentStep1", - }, - }, - Object { - "deployment": Object { - "previousMetaKey": "deploymentStep2MetaKey", - "stackTemplatePathOrUrl": "deploymentStep3", - }, - "rollback": Object { - "previousMetaKey": "deploymentStep1MetaKey", - "stackTemplatePathOrUrl": "deploymentStep2", - }, - }, - Object { - "deployment": Object { - "previousMetaKey": "deploymentStep3MetaKey", - "stackTemplatePathOrUrl": "deploymentStep4", - }, - "rollback": Object { - "previousMetaKey": "deploymentStep2MetaKey", - "stackTemplatePathOrUrl": "deploymentStep3", - }, - }, -] -`; diff --git a/packages/amplify-provider-awscloudformation/src/__tests__/disconnect-dependent-resources/utils.test.ts b/packages/amplify-provider-awscloudformation/src/__tests__/disconnect-dependent-resources/utils.test.ts deleted file mode 100644 index 9b16d26be34..00000000000 --- a/packages/amplify-provider-awscloudformation/src/__tests__/disconnect-dependent-resources/utils.test.ts +++ /dev/null @@ -1,236 +0,0 @@ -import { - generateIterativeFuncDeploymentSteps, - generateTempFuncCFNTemplates, - getDependentFunctions, - prependDeploymentSteps, - uploadTempFuncDeploymentFiles, -} from '../../disconnect-dependent-resources/utils'; -import { pathManager, stateManager, readCFNTemplate, writeCFNTemplate, CFNTemplateFormat } from 'amplify-cli-core'; -import * as fs from 'fs-extra'; -import { S3 } from '../../aws-utils/aws-s3'; -import { CloudFormation } from 'aws-sdk'; -import { getPreviousDeploymentRecord } from '../../utils/amplify-resource-state-utils'; -import Template from 'cloudform-types/types/template'; -import { DeploymentOp, DeploymentStep } from '../../iterative-deployment'; - -jest.mock('fs-extra'); -jest.mock('amplify-cli-core'); -jest.mock('amplify-cli-logger'); -jest.mock('../../utils/amplify-resource-state-utils'); - -const fs_mock = fs as jest.Mocked; -const pathManager_mock = pathManager as jest.Mocked; -const stateManager_mock = stateManager as jest.Mocked; -const readCFNTemplate_mock = readCFNTemplate as jest.MockedFunction; -const writeCFNTemplate_mock = writeCFNTemplate as jest.MockedFunction; - -const getPreviousDeploymentRecord_mock = getPreviousDeploymentRecord as jest.MockedFunction; - -pathManager_mock.getResourceDirectoryPath.mockReturnValue('mock/path'); - -beforeEach(jest.clearAllMocks); - -describe('getDependentFunctions', () => { - it('returns the subset of functions that have a dependency on the models', async () => { - const func1Params = { - permissions: { - storage: { - someOtherTable: {}, - }, - }, - }; - const func2Params = { - permissions: { - storage: { - ['ModelName:@model(appsync)']: {}, - }, - }, - }; - const funcParamsSupplier = jest.fn().mockReturnValueOnce(func1Params).mockReturnValueOnce(func2Params); - const result = await getDependentFunctions(['ModelName', 'OtherModel'], ['func1', 'func2'], funcParamsSupplier); - expect(result).toEqual(['func2']); - }); -}); - -describe('generateTempFuncCFNTemplates', () => { - readCFNTemplate_mock.mockResolvedValueOnce({ - cfnTemplate: { - a: { - b: { - c: [ - { - 'Fn::ImportValue': { - 'Fn::Sub': 'test string', - }, - }, - { - 'Fn::Join': [ - ':', - { - 'Fn::ImportValue': 'testvalue', - }, - ], - }, - ], - }, - d: { - 'Fn::ImportValue': 'something else', - }, - }, - } as Template, - templateFormat: CFNTemplateFormat.JSON, - }); - it('replaces Fn::ImportValue references with placeholder values in template', async () => { - await generateTempFuncCFNTemplates(['func1']); - expect(writeCFNTemplate_mock.mock.calls[0][0]).toMatchSnapshot(); - }); -}); - -describe('uploadTempFuncDeploymentFiles', () => { - it('uploads template and meta file', async () => { - fs_mock.createReadStream - .mockReturnValueOnce('func1Template' as any) - .mockReturnValueOnce('func1Meta' as any) - .mockReturnValueOnce('func2Template' as any) - .mockReturnValueOnce('func2Meta' as any); - const s3Client_stub = { - uploadFile: jest.fn(), - }; - - await uploadTempFuncDeploymentFiles(s3Client_stub as unknown as S3, ['func1', 'func2']); - expect(s3Client_stub.uploadFile.mock.calls).toMatchInlineSnapshot(` - Array [ - Array [ - Object { - "Body": "func1Template", - "Key": "amplify-cfn-templates/function/temp/temp-func1-cloudformation-template.json", - }, - false, - ], - Array [ - Object { - "Body": "func1Meta", - "Key": "amplify-cfn-templates/function/temp/temp-func1-deployment-meta.json", - }, - false, - ], - Array [ - Object { - "Body": "func2Template", - "Key": "amplify-cfn-templates/function/temp/temp-func2-cloudformation-template.json", - }, - false, - ], - Array [ - Object { - "Body": "func2Meta", - "Key": "amplify-cfn-templates/function/temp/temp-func2-deployment-meta.json", - }, - false, - ], - ] - `); - }); - - it('logs and throws upload error', async () => { - const s3Client_stub = { - uploadFile: jest.fn().mockRejectedValue(new Error('test error')), - }; - try { - await uploadTempFuncDeploymentFiles(s3Client_stub as unknown as S3, ['func1', 'func2']); - fail('function call should error'); - } catch (err) { - expect(err.message).toMatchInlineSnapshot(`"test error"`); - } - }); -}); - -describe('generateIterativeFuncDeploymentSteps', () => { - it('generates steps with correct pointers', async () => { - const cfnClient_stub = { - describeStackResources: () => ({ - promise: async () => ({ - StackResources: [ - { - PhysicalResourceId: 'testStackId', - }, - ], - }), - }), - }; - getPreviousDeploymentRecord_mock - .mockResolvedValueOnce({ - parameters: { - param1: 'value1', - }, - capabilities: [], - }) - .mockResolvedValueOnce({ - parameters: { - param2: 'value2', - }, - capabilities: [], - }); - stateManager_mock.getResourceParametersJson.mockReturnValue({}); - stateManager_mock.getTeamProviderInfo.mockReturnValue({}); - stateManager_mock.getLocalEnvInfo.mockReturnValue({ envName: 'testenv' }); - const result = await generateIterativeFuncDeploymentSteps(cfnClient_stub as unknown as CloudFormation, 'testRootStackId', [ - 'func1', - 'func2', - ]); - expect(result).toMatchSnapshot(); - }); -}); - -describe('prependDeploymentSteps', () => { - it('concatenates arrays and moves pointers appropriately', () => { - const beforeSteps: DeploymentStep[] = [ - { - deployment: { - stackTemplatePathOrUrl: 'deploymentStep1', - previousMetaKey: undefined, - } as DeploymentOp, - rollback: undefined, - }, - { - deployment: { - stackTemplatePathOrUrl: 'deploymentStep2', - previousMetaKey: 'deploymentStep1MetaKey', - } as DeploymentOp, - rollback: { - stackTemplatePathOrUrl: 'deploymentStep1', - previousMetaKey: undefined, - } as DeploymentOp, - }, - ]; - - const afterSteps: DeploymentStep[] = [ - { - deployment: { - stackTemplatePathOrUrl: 'deploymentStep3', - previousMetaKey: undefined, - } as DeploymentOp, - rollback: undefined, - }, - { - deployment: { - stackTemplatePathOrUrl: 'deploymentStep4', - previousMetaKey: 'deploymentStep3MetaKey', - } as DeploymentOp, - rollback: { - stackTemplatePathOrUrl: 'deploymentStep3', - previousMetaKey: undefined, - } as DeploymentOp, - }, - ]; - - const result = prependDeploymentSteps(beforeSteps, afterSteps, 'deploymentStep2MetaKey'); - expect(result).toMatchSnapshot(); - }); - - it('returns after array if before array is empty', () => { - const afterSteps = ['test step' as unknown as DeploymentStep]; - const result = prependDeploymentSteps([], afterSteps, 'testmetakey'); - expect(result).toEqual(afterSteps); - }); -}); diff --git a/packages/amplify-provider-awscloudformation/src/constants.js b/packages/amplify-provider-awscloudformation/src/constants.js index 4b6232b492c..d05deef8387 100644 --- a/packages/amplify-provider-awscloudformation/src/constants.js +++ b/packages/amplify-provider-awscloudformation/src/constants.js @@ -15,5 +15,4 @@ module.exports = { FunctionCategoryName: 'function', // keep in sync with ServiceName in amplify-category-function, but probably it will not change FunctionServiceNameLambdaLayer: 'LambdaLayer', - destructiveUpdatesFlag: 'allow-destructive-graphql-schema-updates', }; diff --git a/packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/index.ts b/packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/index.ts deleted file mode 100644 index 6fdae96a7f9..00000000000 --- a/packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/index.ts +++ /dev/null @@ -1,64 +0,0 @@ -import { $TSAny, $TSContext, pathManager, stateManager } from 'amplify-cli-core'; -import { CloudFormation } from 'aws-sdk'; -import { S3 } from '../aws-utils/aws-s3'; -import { loadConfiguration } from '../configuration-manager'; -import { DeploymentStep } from '../iterative-deployment'; -import { - getDependentFunctions, - generateIterativeFuncDeploymentSteps, - prependDeploymentSteps, - generateTempFuncCFNTemplates, - uploadTempFuncDeploymentFiles, - s3Prefix, - localPrefix, -} from './utils'; -import * as fs from 'fs-extra'; - -let functionsDependentOnReplacedModelTables: string[] = []; - -/** - * Identifies if any functions depend on a model table that is being replaced. - * If so, it creates temporary CFN templates for these functions that do not reference the replaced table and adds deployment steps to the array to update the functions before the table is replaced - * @param context Amplify context - * @param modelsBeingReplaced Names of the models being replaced during this push operation - * @param deploymentSteps The existing list of deployment steps that will be prepended to in the case of dependent functions - * @returns The new list of deploymentSteps - */ -export const prependDeploymentStepsToDisconnectFunctionsFromReplacedModelTables = async ( - context: $TSContext, - modelsBeingReplaced: string[], - deploymentSteps: DeploymentStep[], -): Promise => { - const amplifyMeta = stateManager.getMeta(); - const rootStackId = amplifyMeta?.providers?.awscloudformation?.StackId; - const allFunctionNames = Object.keys(amplifyMeta?.function); - functionsDependentOnReplacedModelTables = await getDependentFunctions( - modelsBeingReplaced, - allFunctionNames, - getFunctionParamsSupplier(context), - ); - // generate deployment steps that will remove references to the replaced tables in the dependent functions - const { deploymentSteps: disconnectFuncsSteps, lastMetaKey } = await generateIterativeFuncDeploymentSteps( - new CloudFormation(await loadConfiguration(context)), - rootStackId, - functionsDependentOnReplacedModelTables, - ); - await generateTempFuncCFNTemplates(functionsDependentOnReplacedModelTables); - await uploadTempFuncDeploymentFiles(await S3.getInstance(context), functionsDependentOnReplacedModelTables); - return prependDeploymentSteps(disconnectFuncsSteps, deploymentSteps, lastMetaKey); -}; - -export const postDeploymentCleanup = async (s3Client: S3, deploymentBucketName: string) => { - if (functionsDependentOnReplacedModelTables.length < 1) { - return; - } - await s3Client.deleteDirectory(deploymentBucketName, s3Prefix); - await Promise.all(functionsDependentOnReplacedModelTables.map(funcName => fs.remove(localPrefix(funcName)))); -}; - -// helper function to load the function-parameters.json file given a functionName -const getFunctionParamsSupplier = (context: $TSContext) => async (functionName: string) => { - return context.amplify.invokePluginMethod(context, 'function', undefined, 'loadFunctionParameters', [ - pathManager.getResourceDirectoryPath(undefined, 'function', functionName), - ]) as $TSAny; -}; diff --git a/packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/utils.ts b/packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/utils.ts deleted file mode 100644 index e91ede4168f..00000000000 --- a/packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/utils.ts +++ /dev/null @@ -1,183 +0,0 @@ -import { $TSAny, JSONUtilities, pathManager, readCFNTemplate, stateManager, writeCFNTemplate } from 'amplify-cli-core'; -import * as path from 'path'; -import * as fs from 'fs-extra'; -import { S3 } from '../aws-utils/aws-s3'; -import { fileLogger } from '../utils/aws-logger'; -import { CloudFormation } from 'aws-sdk'; -import { getPreviousDeploymentRecord } from '../utils/amplify-resource-state-utils'; -import { DeploymentOp, DeploymentStep } from '../iterative-deployment'; -import _ from 'lodash'; - -const logger = fileLogger('disconnect-dependent-resources'); - -/** - * Returns the subset of functionNames that have a dependency on a model in modelNames - */ -export const getDependentFunctions = async ( - modelNames: string[], - functionNames: string[], - functionParamsSupplier: (functionName: string) => Promise<$TSAny>, -) => { - const dependentFunctions: string[] = []; - for (const funcName of functionNames) { - const funcParams = await functionParamsSupplier(funcName); - const dependentModels = funcParamsToDependentAppSyncModels(funcParams); - const hasDep = dependentModels.map(model => modelNames.includes(model)).reduce((acc, it) => acc || it, false); - if (hasDep) { - dependentFunctions.push(funcName); - } - } - return dependentFunctions; -}; - -/** - * Generates temporary CFN templates for the given functions that have placeholder values for all references to replaced model tables - */ -export const generateTempFuncCFNTemplates = async (dependentFunctions: string[]) => { - const tempPaths: string[] = []; - for (const funcName of dependentFunctions) { - const { cfnTemplate, templateFormat } = await readCFNTemplate( - path.join(pathManager.getResourceDirectoryPath(undefined, 'function', funcName), `${funcName}-cloudformation-template.json`), - ); - replaceFnImport(cfnTemplate); - const tempPath = getTempFuncTemplateLocalPath(funcName); - await writeCFNTemplate(cfnTemplate, tempPath, { templateFormat }); - tempPaths.push(tempPath); - } -}; - -/** - * Uploads the CFN template and iterative deployment meta file to S3 - */ -export const uploadTempFuncDeploymentFiles = async (s3Client: S3, funcNames: string[]) => { - for (const funcName of funcNames) { - const uploads = [ - { - Body: fs.createReadStream(getTempFuncTemplateLocalPath(funcName)), - Key: getTempFuncTemplateS3Key(funcName), - }, - { - Body: fs.createReadStream(getTempFuncMetaLocalPath(funcName)), - Key: getTempFuncMetaS3Key(funcName), - }, - ]; - const log = logger('uploadTemplateToS3.s3.uploadFile', [{ Key: uploads[0].Key }]); - for (const upload of uploads) { - try { - await s3Client.uploadFile(upload, false); - } catch (error) { - log(error); - throw error; - } - } - } -}; - -export const generateIterativeFuncDeploymentSteps = async ( - cfnClient: CloudFormation, - rootStackId: string, - functionNames: string[], -): Promise<{ deploymentSteps: DeploymentStep[]; lastMetaKey: string }> => { - let rollback: DeploymentOp; - let previousMetaKey: string; - const steps: DeploymentStep[] = []; - for (const funcName of functionNames) { - const deploymentOp = await generateIterativeFuncDeploymentOp(cfnClient, rootStackId, funcName); - deploymentOp.previousMetaKey = previousMetaKey; - steps.push({ - deployment: deploymentOp, - rollback, - }); - rollback = deploymentOp; - previousMetaKey = getTempFuncMetaS3Key(funcName); - } - return { deploymentSteps: steps, lastMetaKey: previousMetaKey }; -}; - -/** - * Prepends beforeSteps and afterSteps into a single array of deployment steps. - * Moves rollback and previousMetaKey pointers to maintain the integrity of the deployment steps. - */ -export const prependDeploymentSteps = (beforeSteps: DeploymentStep[], afterSteps: DeploymentStep[], beforeStepsLastMetaKey: string) => { - if (beforeSteps.length === 0) { - return afterSteps; - } - beforeSteps[0].rollback = _.cloneDeep(afterSteps[0].rollback); - beforeSteps[0].deployment.previousMetaKey = afterSteps[0].deployment.previousMetaKey; - afterSteps[0].rollback = _.cloneDeep(beforeSteps[beforeSteps.length - 1].deployment); - afterSteps[0].deployment.previousMetaKey = beforeStepsLastMetaKey; - if (afterSteps.length > 1) { - afterSteps[1].rollback.previousMetaKey = beforeStepsLastMetaKey; - } - return beforeSteps.concat(afterSteps); -}; - -/** - * Generates a deployment operation for a temporary function deployment. - * Also writes the deployment operation to the temp meta path - */ -const generateIterativeFuncDeploymentOp = async (cfnClient: CloudFormation, rootStackId: string, functionName: string) => { - const funcStack = await cfnClient - .describeStackResources({ StackName: rootStackId, LogicalResourceId: `function${functionName}` }) - .promise(); - const funcStackId = funcStack.StackResources[0].PhysicalResourceId; - const { parameters, capabilities } = await getPreviousDeploymentRecord(cfnClient, funcStackId); - const funcCfnParams = stateManager.getResourceParametersJson(undefined, 'function', functionName, { - throwIfNotExist: false, - default: {}, - }); - const tpi = stateManager.getTeamProviderInfo(undefined, { throwIfNotExist: false, default: {} }); - const env = stateManager.getLocalEnvInfo().envName; - const tpiCfnParams = tpi?.[env]?.categories?.function?.[functionName] || {}; - const params = { ...parameters, ...funcCfnParams, ...tpiCfnParams }; - const deploymentStep: DeploymentOp = { - stackTemplatePathOrUrl: getTempFuncTemplateS3Key(functionName), - parameters: params, - stackName: funcStackId, - capabilities, - tableNames: [], - }; - - JSONUtilities.writeJson(getTempFuncMetaLocalPath(functionName), deploymentStep); - return deploymentStep; -}; - -// helper functions for constructing local paths and S3 keys for function templates and deployment meta files -const getTempFuncTemplateS3Key = (funcName: string): string => path.posix.join(s3Prefix, tempTemplateFilename(funcName)); -const getTempFuncTemplateLocalPath = (funcName: string): string => path.join(localPrefix(funcName), tempTemplateFilename(funcName)); -const getTempFuncMetaLocalPath = (funcName: string): string => path.join(localPrefix(funcName), tempMetaFilename(funcName)); -const getTempFuncMetaS3Key = (funcName: string): string => path.posix.join(s3Prefix, tempMetaFilename(funcName)); - -const tempTemplateFilename = (funcName: string) => `temp-${funcName}-cloudformation-template.json`; -const tempMetaFilename = (funcName: string) => `temp-${funcName}-deployment-meta.json`; -export const s3Prefix = 'amplify-cfn-templates/function/temp'; -export const localPrefix = funcName => path.join(pathManager.getResourceDirectoryPath(undefined, 'function', funcName), 'temp'); - -/** - * Recursively searches for 'Fn::ImportValue' nodes in a CFN template object and replaces them with a placeholder value - * @param node - * @returns - */ -const replaceFnImport = (node: $TSAny) => { - if (typeof node !== 'object') { - return; - } - if (Array.isArray(node)) { - node.forEach(el => replaceFnImport(el)); - } - const nodeKeys = Object.keys(node); - if (nodeKeys.length === 1 && nodeKeys[0] === 'Fn::ImportValue') { - node['Fn::ImportValue'] = undefined; - node['Fn::Sub'] = 'TemporaryPlaceholderValue'; - return; - } - Object.values(node).forEach(value => replaceFnImport(value)); -}; - -/** - * Given the contents of the function-parameters.json file for a function, returns the list of AppSync models this function depends on. - */ -const funcParamsToDependentAppSyncModels = (funcParams: $TSAny): string[] => - Object.keys(funcParams?.permissions?.storage || {}) - .filter(key => key.endsWith(':@model(appsync)')) - .map(key => key.slice(0, key.lastIndexOf(':'))); diff --git a/packages/amplify-provider-awscloudformation/src/graphql-transformer/amplify-graphql-resource-manager.ts b/packages/amplify-provider-awscloudformation/src/graphql-transformer/amplify-graphql-resource-manager.ts index aaa40b429b7..cd82d286df8 100644 --- a/packages/amplify-provider-awscloudformation/src/graphql-transformer/amplify-graphql-resource-manager.ts +++ b/packages/amplify-provider-awscloudformation/src/graphql-transformer/amplify-graphql-resource-manager.ts @@ -26,7 +26,6 @@ export type GQLResourceManagerProps = { resourceMeta?: ResourceMeta; backendDir: string; cloudBackendDir: string; - rebuildAllTables?: boolean; }; export type ResourceMeta = { @@ -53,9 +52,8 @@ export class GraphQLResourceManager { private cloudBackendApiProjectRoot: string; private backendApiProjectRoot: string; private templateState: TemplateState; - private rebuildAllTables: boolean = false; // indicates that all underlying model tables should be rebuilt - public static createInstance = async (context: $TSContext, gqlResource: any, StackId: string, rebuildAllTables: boolean = false) => { + public static createInstance = async (context: $TSContext, gqlResource: any, StackId: string) => { try { const cred = await loadConfiguration(context); const cfn = new CloudFormation(cred); @@ -67,7 +65,6 @@ export class GraphQLResourceManager { resourceMeta: { ...gqlResource, stackId: apiStack.StackResources[0].PhysicalResourceId }, backendDir: pathManager.getBackendDirPath(), cloudBackendDir: pathManager.getCurrentCloudBackendDirPath(), - rebuildAllTables, }); } catch (err) { throw err; @@ -85,7 +82,6 @@ export class GraphQLResourceManager { this.backendApiProjectRoot = path.join(props.backendDir, GraphQLResourceManager.categoryName, this.resourceMeta.resourceName); this.cloudBackendApiProjectRoot = path.join(props.cloudBackendDir, GraphQLResourceManager.categoryName, this.resourceMeta.resourceName); this.templateState = new TemplateState(); - this.rebuildAllTables = props.rebuildAllTables || false; } run = async (): Promise => { @@ -106,10 +102,7 @@ export class GraphQLResourceManager { throw err; } } - if (!this.rebuildAllTables) { - this.gsiManagement(gqlDiff.diff, gqlDiff.current, gqlDiff.next); - } - this.tableRecreationManagement(gqlDiff.current, gqlDiff.next); + this.gsiManagement(gqlDiff.diff, gqlDiff.current, gqlDiff.next); return await this.getDeploymentSteps(); }; @@ -224,9 +217,9 @@ export class GraphQLResourceManager { }); const tableWithGSIChanges = _.uniqBy(gsiChanges, diff => diff.path?.slice(0, 3).join('/')).map(gsiChange => { - const tableName = gsiChange.path[3] as string; + const tableName = gsiChange.path[3]; - const stackName = gsiChange.path[1].split('.')[0] as string; + const stackName = gsiChange.path[1].split('.')[0]; const currentTable = this.getTable(gsiChange, currentState); const nextTable = this.getTable(gsiChange, nextState); @@ -273,41 +266,6 @@ export class GraphQLResourceManager { } }; - private tableRecreationManagement = (currentState: DiffableProject, nextState: DiffableProject) => { - this.getTablesBeingReplaced().forEach(tableMeta => { - const ddbResource = this.getStack(tableMeta.stackName, currentState); - this.dropTable(tableMeta.tableName, ddbResource); - // clear any other states created by GSI updates as dropping and recreating supercedes those changes - this.clearTemplateState(tableMeta.stackName); - this.templateState.add(tableMeta.stackName, JSONUtilities.stringify(ddbResource)); - this.templateState.add(tableMeta.stackName, JSONUtilities.stringify(this.getStack(tableMeta.stackName, nextState))); - }); - }; - - getTablesBeingReplaced = () => { - const gqlDiff = getGQLDiff(this.backendApiProjectRoot, this.cloudBackendApiProjectRoot); - const [diffs, currentState] = [gqlDiff.diff, gqlDiff.current]; - const getTablesRequiringReplacement = () => - _.uniq( - diffs - .filter(diff => diff.path.includes('KeySchema') || diff.path.includes('LocalSecondaryIndexes')) // filter diffs with changes that require replacement - .map(diff => ({ - // extract table name and stack name from diff path - tableName: diff.path?.[3] as string, - stackName: diff.path[1].split('.')[0] as string, - })), - ) as { tableName: string; stackName: string }[]; - - const getAllTables = () => - Object.entries(currentState.stacks) - .map(([name, template]) => ({ - tableName: this.getTableNameFromTemplate(template), - stackName: path.basename(name, '.json'), - })) - .filter(meta => !!meta.tableName); - return this.rebuildAllTables ? getAllTables() : getTablesRequiringReplacement(); - }; - private getTable = (gsiChange: Diff, proj: DiffableProject): DynamoDB.Table => { return proj.stacks[gsiChange.path[1]].Resources[gsiChange.path[3]] as DynamoDB.Table; }; @@ -325,21 +283,6 @@ export class GraphQLResourceManager { const table = template.Resources[tableName] as DynamoDB.Table; template.Resources[tableName] = removeGSI(indexName, table); }; - - private dropTable = (tableName: string, template: Template): void => { - // remove table and all output refs to it - template.Resources[tableName] = undefined; - template.Outputs = _.omitBy(template.Outputs, (_, key) => key.includes(tableName)); - }; - - private clearTemplateState = (stackName: string) => { - while (this.templateState.has(stackName)) { - this.templateState.pop(stackName); - } - }; - - private getTableNameFromTemplate = (template: Template): string | undefined => - Object.entries(template?.Resources || {}).find(([_, resource]) => resource.Type === 'AWS::DynamoDB::Table')?.[0]; } // https://stackoverflow.com/questions/39419170/how-do-i-check-that-a-switch-block-is-exhaustive-in-typescript diff --git a/packages/amplify-provider-awscloudformation/src/graphql-transformer/utils.ts b/packages/amplify-provider-awscloudformation/src/graphql-transformer/utils.ts index fbbac527037..c849a903223 100644 --- a/packages/amplify-provider-awscloudformation/src/graphql-transformer/utils.ts +++ b/packages/amplify-provider-awscloudformation/src/graphql-transformer/utils.ts @@ -34,11 +34,20 @@ export const getGQLDiff = (currentBackendDir: string, cloudBackendDir: string): return null; }; -export const getGqlUpdatedResource = (resources: any[]) => - resources.find( - resource => - resource?.service === 'AppSync' && resource?.providerMetadata?.logicalId && resource?.providerPlugin === 'awscloudformation', - ) || null; +export const getGqlUpdatedResource = (resources: any[]) => { + if (resources.length > 0) { + const resource = resources[0]; + if ( + resource.service === 'AppSync' && + resource.providerMetadata && + resource.providerMetadata.logicalId && + resource.providerPlugin === 'awscloudformation' + ) { + return resource; + } + } + return null; +}; export function loadDiffableProject(path: string, rootStackName: string): DiffableProject { const project = readFromPath(path); diff --git a/packages/amplify-provider-awscloudformation/src/index.ts b/packages/amplify-provider-awscloudformation/src/index.ts index 9574bd29f60..1c5e68714f8 100644 --- a/packages/amplify-provider-awscloudformation/src/index.ts +++ b/packages/amplify-provider-awscloudformation/src/index.ts @@ -54,8 +54,8 @@ function onInitSuccessful(context) { return initializer.onInitSuccessful(context); } -function pushResources(context, resourceList, rebuild: boolean = false) { - return resourcePusher.run(context, resourceList, rebuild); +function pushResources(context, resourceList) { + return resourcePusher.run(context, resourceList); } function storeCurrentCloudBackend(context) { diff --git a/packages/amplify-provider-awscloudformation/src/iterative-deployment/deployment-manager.ts b/packages/amplify-provider-awscloudformation/src/iterative-deployment/deployment-manager.ts index 04d2b94df8d..32507727607 100644 --- a/packages/amplify-provider-awscloudformation/src/iterative-deployment/deployment-manager.ts +++ b/packages/amplify-provider-awscloudformation/src/iterative-deployment/deployment-manager.ts @@ -327,15 +327,9 @@ export class DeploymentManager { try { const response = await this.ddbClient.describeTable({ TableName: tableName }).promise(); - if (response.Table?.TableStatus === 'DELETING') { - return false; - } const gsis = response.Table?.GlobalSecondaryIndexes; return gsis ? gsis.every(idx => idx.IndexStatus === 'ACTIVE') : true; } catch (err) { - if (err?.code === 'ResourceNotFoundException') { - return true; // in the case of an iterative update that recreates a table, non-existance means the table has been fully removed - } this.logger('getTableStatus', [{ tableName }])(err); throw err; } diff --git a/packages/amplify-provider-awscloudformation/src/push-resources.ts b/packages/amplify-provider-awscloudformation/src/push-resources.ts index e738daccc94..f940187e574 100644 --- a/packages/amplify-provider-awscloudformation/src/push-resources.ts +++ b/packages/amplify-provider-awscloudformation/src/push-resources.ts @@ -47,10 +47,6 @@ import { preProcessCFNTemplate } from './pre-push-cfn-processor/cfn-pre-processo import { AUTH_TRIGGER_STACK, AUTH_TRIGGER_TEMPLATE } from './utils/upload-auth-trigger-template'; import { ensureValidFunctionModelDependencies } from './utils/remove-dependent-function'; import { legacyLayerMigration, postPushLambdaLayerCleanup, prePushLambdaLayerPrompt } from './lambdaLayerInvocations'; -import { - postDeploymentCleanup, - prependDeploymentStepsToDisconnectFunctionsFromReplacedModelTables, -} from './disconnect-dependent-resources'; const logger = fileLogger('push-resources'); @@ -71,20 +67,26 @@ const deploymentInProgressErrorMessage = (context: $TSContext) => { context.print.error('"amplify push --force" to re-deploy'); }; -export async function run(context: $TSContext, resourceDefinition: $TSObject, rebuild: boolean = false) { +export async function run(context: $TSContext, resourceDefinition: $TSObject) { const deploymentStateManager = await DeploymentStateManager.createDeploymentStateManager(context); let iterativeDeploymentWasInvoked = false; let layerResources = []; try { - const { resourcesToBeCreated, resourcesToBeUpdated, resourcesToBeSynced, resourcesToBeDeleted, tagsUpdated, allResources } = - resourceDefinition; + const { + resourcesToBeCreated, + resourcesToBeUpdated, + resourcesToBeSynced, + resourcesToBeDeleted, + tagsUpdated, + allResources, + } = resourceDefinition; const cloudformationMeta = context.amplify.getProjectMeta().providers.awscloudformation; const { parameters: { options }, } = context; - let resources = !!context?.exeInfo?.forcePush || rebuild ? allResources : resourcesToBeCreated.concat(resourcesToBeUpdated); + let resources = !!context?.exeInfo?.forcePush ? allResources : resourcesToBeCreated.concat(resourcesToBeUpdated); layerResources = resources.filter(r => r.service === FunctionServiceNameLambdaLayer); if (deploymentStateManager.isDeploymentInProgress() && !deploymentStateManager.isDeploymentFinished()) { @@ -113,7 +115,7 @@ export async function run(context: $TSContext, resourceDefinition: $TSObject, re } validateCfnTemplates(context, resources); - for (const resource of resources) { + for await (const resource of resources) { if (resource.service === ApiServiceNameElasticContainer && resource.category === 'api') { const { exposedContainer, @@ -140,7 +142,9 @@ export async function run(context: $TSContext, resourceDefinition: $TSObject, re } } - for (const resource of resources.filter(r => r.category === FunctionCategoryName && r.service === FunctionServiceNameLambdaLayer)) { + for await (const resource of resources.filter( + r => r.category === FunctionCategoryName && r.service === FunctionServiceNameLambdaLayer, + )) { await legacyLayerMigration(context, resource.resourceName); } @@ -160,26 +164,17 @@ export async function run(context: $TSContext, resourceDefinition: $TSObject, re } let deploymentSteps: DeploymentStep[] = []; - let functionsDependentOnReplacedModelTables: string[] = []; // location where the intermediate deployment steps are stored let stateFolder: { local?: string; cloud?: string } = {}; // Check if iterative updates are enabled or not and generate the required deployment steps if needed. if (FeatureFlags.getBoolean('graphQLTransformer.enableIterativeGSIUpdates')) { - const gqlResource = getGqlUpdatedResource(rebuild ? resources : resourcesToBeUpdated); + const gqlResource = getGqlUpdatedResource(resourcesToBeUpdated); if (gqlResource) { - const gqlManager = await GraphQLResourceManager.createInstance(context, gqlResource, cloudformationMeta.StackId, rebuild); + const gqlManager = await GraphQLResourceManager.createInstance(context, gqlResource, cloudformationMeta.StackId); deploymentSteps = await gqlManager.run(); - - // If any models are being replaced, we prepend steps to the iterative deployment to remove references to the replaced table in functions that have a dependeny on the tables - const modelsBeingReplaced = gqlManager.getTablesBeingReplaced().map(meta => meta.stackName); // stackName is the same as the model name - deploymentSteps = await prependDeploymentStepsToDisconnectFunctionsFromReplacedModelTables( - context, - modelsBeingReplaced, - deploymentSteps, - ); if (deploymentSteps.length > 1) { iterativeDeploymentWasInvoked = true; @@ -214,8 +209,7 @@ export async function run(context: $TSContext, resourceDefinition: $TSObject, re resourcesToBeUpdated.length > 0 || resourcesToBeDeleted.length > 0 || tagsUpdated || - context.exeInfo.forcePush || - rebuild + context.exeInfo.forcePush ) { // If there is an API change, there will be one deployment step. But when there needs an iterative update the step count is > 1 if (deploymentSteps.length > 1) { @@ -260,11 +254,10 @@ export async function run(context: $TSContext, resourceDefinition: $TSObject, re context.print.error(`Could not delete state directory locally: ${err}`); } } - const s3 = await S3.getInstance(context); if (stateFolder.cloud) { + const s3 = await S3.getInstance(context); await s3.deleteDirectory(cloudformationMeta.DeploymentBucketName, stateFolder.cloud); } - postDeploymentCleanup(s3, cloudformationMeta.DeploymentBucketName); } else { // Non iterative update spinner.start(); @@ -1038,8 +1031,14 @@ async function formNestedStack( // If auth is imported check the parameters section of the nested template // and if it has auth or unauth role arn or name or userpool id, then inject it from the // imported auth resource's properties - const { imported, userPoolId, authRoleArn, authRoleName, unauthRoleArn, unauthRoleName } = - context.amplify.getImportedAuthProperties(context); + const { + imported, + userPoolId, + authRoleArn, + authRoleName, + unauthRoleArn, + unauthRoleName, + } = context.amplify.getImportedAuthProperties(context); if (category !== 'auth' && resourceDetails.service !== 'Cognito' && imported) { if (parameters.AuthCognitoUserPoolId) { diff --git a/packages/amplify-provider-awscloudformation/src/transform-graphql-schema.ts b/packages/amplify-provider-awscloudformation/src/transform-graphql-schema.ts index 35074daf0bb..3537ee2983d 100644 --- a/packages/amplify-provider-awscloudformation/src/transform-graphql-schema.ts +++ b/packages/amplify-provider-awscloudformation/src/transform-graphql-schema.ts @@ -13,10 +13,10 @@ import { FunctionTransformer } from 'graphql-function-transformer'; import { HttpTransformer } from 'graphql-http-transformer'; import { PredictionsTransformer } from 'graphql-predictions-transformer'; import { KeyTransformer } from 'graphql-key-transformer'; -import { destructiveUpdatesFlag, ProviderName as providerName } from './constants'; +import { ProviderName as providerName } from './constants'; import { AmplifyCLIFeatureFlagAdapter } from './utils/amplify-cli-feature-flag-adapter'; import { isAmplifyAdminApp } from './utils/admin-helpers'; -import { $TSContext, JSONUtilities, stateManager } from 'amplify-cli-core'; +import { JSONUtilities, stateManager } from 'amplify-cli-core'; import { ResourceConstants } from 'graphql-transformer-common'; import { printer } from 'amplify-prompts'; @@ -307,7 +307,7 @@ async function migrateProject(context, options) { } } -export async function transformGraphQLSchema(context: $TSContext, options) { +export async function transformGraphQLSchema(context, options) { const useExperimentalPipelineTransformer = FeatureFlags.getBoolean('graphQLTransformer.useExperimentalPipelinedTransformer'); if (useExperimentalPipelineTransformer) { return transformGraphQLSchemaV6(context, options); @@ -378,7 +378,7 @@ export async function transformGraphQLSchema(context: $TSContext, options) { if (!parameters && fs.existsSync(parametersFilePath)) { try { - parameters = JSONUtilities.readJson(parametersFilePath); + parameters = context.amplify.readJsonFile(parametersFilePath); } catch (e) { parameters = {}; } @@ -492,8 +492,7 @@ export async function transformGraphQLSchema(context: $TSContext, options) { } const ff = new AmplifyCLIFeatureFlagAdapter(); - const allowDestructiveUpdates = context?.input?.options?.[destructiveUpdatesFlag] || context.input?.options?.force; - const sanityCheckRulesList = getSanityCheckRules(isNewAppSyncAPI, ff, allowDestructiveUpdates); + const sanityCheckRulesList = getSanityCheckRules(isNewAppSyncAPI, ff); const buildConfig = { ...options, diff --git a/packages/graphql-transformer-core/src/__tests__/util/__snapshots__/amplifyUtils.test.ts.snap b/packages/graphql-transformer-core/src/__tests__/util/__snapshots__/amplifyUtils.test.ts.snap index 3ffd983d87e..e88c9b1aabf 100644 --- a/packages/graphql-transformer-core/src/__tests__/util/__snapshots__/amplifyUtils.test.ts.snap +++ b/packages/graphql-transformer-core/src/__tests__/util/__snapshots__/amplifyUtils.test.ts.snap @@ -1,38 +1,11 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`get sanity check rules sanity check rule list when destructive changes flag is present and ff enabled 1`] = `Array []`; - -exports[`get sanity check rules sanity check rule list when destructive changes flag is present and ff enabled 2`] = ` -Array [ - "cantHaveMoreThan500ResourcesRule", -] -`; - -exports[`get sanity check rules sanity check rule list when destructive changes flag is present but ff not enabled 1`] = ` -Array [ - "cantEditKeySchemaRule", - "cantAddLSILaterRule", - "cantRemoveLSILater", - "cantEditLSIKeySchemaRule", - "cantEditGSIKeySchemaRule", - "cantAddAndRemoveGSIAtSameTimeRule", -] -`; - -exports[`get sanity check rules sanity check rule list when destructive changes flag is present but ff not enabled 2`] = ` -Array [ - "cantHaveMoreThan500ResourcesRule", - "cantMutateMultipleGSIAtUpdateTimeRule", -] -`; - exports[`get sanity check rules sanitycheck rule list when api is in update status and ff enabled 1`] = ` Array [ "cantEditKeySchemaRule", "cantAddLSILaterRule", "cantRemoveLSILater", "cantEditLSIKeySchemaRule", - "cantRemoveTableAfterCreation", ] `; @@ -50,7 +23,6 @@ Array [ "cantEditLSIKeySchemaRule", "cantEditGSIKeySchemaRule", "cantAddAndRemoveGSIAtSameTimeRule", - "cantRemoveTableAfterCreation", ] `; diff --git a/packages/graphql-transformer-core/src/__tests__/util/amplifyUtils.test.ts b/packages/graphql-transformer-core/src/__tests__/util/amplifyUtils.test.ts index 40f681ea908..7d4034acbc3 100644 --- a/packages/graphql-transformer-core/src/__tests__/util/amplifyUtils.test.ts +++ b/packages/graphql-transformer-core/src/__tests__/util/amplifyUtils.test.ts @@ -32,24 +32,4 @@ describe('get sanity check rules', () => { expect(diffRulesFn).toMatchSnapshot(); expect(projectRulesFn).toMatchSnapshot(); }); - - test('sanity check rule list when destructive changes flag is present and ff enabled', () => { - const ff_mock = new AmplifyCLIFeatureFlagAdapter(); - (FeatureFlags.getBoolean).mockReturnValue(true); - const sanityCheckRules: SanityCheckRules = getSanityCheckRules(false, ff_mock, true); - const diffRulesFn = sanityCheckRules.diffRules.map(func => func.name); - const projectRulesFn = sanityCheckRules.projectRules.map(func => func.name); - expect(diffRulesFn).toMatchSnapshot(); - expect(projectRulesFn).toMatchSnapshot(); - }); - - test('sanity check rule list when destructive changes flag is present but ff not enabled', () => { - const ff_mock = new AmplifyCLIFeatureFlagAdapter(); - (FeatureFlags.getBoolean).mockReturnValue(false); - const sanityCheckRules: SanityCheckRules = getSanityCheckRules(false, ff_mock, true); - const diffRulesFn = sanityCheckRules.diffRules.map(func => func.name); - const projectRulesFn = sanityCheckRules.projectRules.map(func => func.name); - expect(diffRulesFn).toMatchSnapshot(); - expect(projectRulesFn).toMatchSnapshot(); - }); }); diff --git a/packages/graphql-transformer-core/src/errors.ts b/packages/graphql-transformer-core/src/errors.ts index 78114f1a7b7..cd3b97d03e6 100644 --- a/packages/graphql-transformer-core/src/errors.ts +++ b/packages/graphql-transformer-core/src/errors.ts @@ -41,37 +41,23 @@ export class TransformerContractError extends Error { } } -export class DestructiveMigrationError extends Error { - constructor(message: string, private removedModels: string[], private replacedModels: string[]) { - super(message); - Object.setPrototypeOf(this, new.target.prototype); - this.name = 'DestructiveMigrationError'; - const prependSpace = (str: string) => ` ${str}`; - const removedModelsList = this.removedModels.map(prependSpace).toString().trim(); - const replacedModelsList = this.replacedModels.map(prependSpace).toString().trim(); - if (removedModelsList && replacedModelsList) { - this.message = `${this.message}\nThis update will remove table(s) [${removedModelsList}] and will replace table(s) [${replacedModelsList}]`; - } else if (removedModelsList) { - this.message = `${this.message}\nThis update will remove table(s) [${removedModelsList}]`; - } else if (replacedModelsList) { - this.message = `${this.message}\nThis update will replace table(s) [${replacedModelsList}]`; - } - this.message = `${this.message}\nALL EXISTING DATA IN THESE TABLES WILL BE LOST!\nIf this is intended, rerun the command with '--allow-destructuve-graphql-schema-updates'.`; - } - toString = () => this.message; -} - /** * Thrown by the sanity checker when a user is trying to make a migration that is known to not work. */ export class InvalidMigrationError extends Error { - constructor(message: string, public cause: string, public fix: string) { + fix: string; + cause: string; + constructor(message: string, cause: string, fix: string) { super(message); - Object.setPrototypeOf(this, new.target.prototype); + Object.setPrototypeOf(this, InvalidMigrationError.prototype); this.name = 'InvalidMigrationError'; + this.fix = fix; + this.cause = cause; } - toString = () => `${this.message}\nCause: ${this.cause}\nHow to fix: ${this.fix}`; } +InvalidMigrationError.prototype.toString = function() { + return `${this.message}\nCause: ${this.cause}\nHow to fix: ${this.fix}`; +}; export class InvalidGSIMigrationError extends InvalidMigrationError { fix: string; diff --git a/packages/graphql-transformer-core/src/util/amplifyUtils.ts b/packages/graphql-transformer-core/src/util/amplifyUtils.ts index 436dfb6177f..869fafc1914 100644 --- a/packages/graphql-transformer-core/src/util/amplifyUtils.ts +++ b/packages/graphql-transformer-core/src/util/amplifyUtils.ts @@ -10,17 +10,16 @@ import { writeConfig, TransformConfig, TransformMigrationConfig, loadProject, re import { FeatureFlagProvider } from '../FeatureFlags'; import { cantAddAndRemoveGSIAtSameTimeRule, - getCantAddLSILaterRule, - getCantRemoveLSILater, + cantAddLSILaterRule, + cantRemoveLSILater, cantEditGSIKeySchemaRule, - getCantEditKeySchemaRule, - getCantEditLSIKeySchemaRule, + cantEditKeySchemaRule, + cantEditLSIKeySchemaRule, cantHaveMoreThan500ResourcesRule, DiffRule, sanityCheckProject, ProjectRule, cantMutateMultipleGSIAtUpdateTimeRule, - cantRemoveTableAfterCreation, } from './sanity-check'; export const CLOUDFORMATION_FILE_NAME = 'cloudformation-template.json'; @@ -728,48 +727,34 @@ function getOrDefault(o: any, k: string, d: any) { return o[k] || d; } -export function getSanityCheckRules(isNewAppSyncAPI: boolean, ff: FeatureFlagProvider, allowDestructiveUpdates: boolean = false) { +export function getSanityCheckRules(isNewAppSyncAPI: boolean, ff: FeatureFlagProvider) { let diffRules: DiffRule[] = []; let projectRules: ProjectRule[] = []; // If we have iterative GSI upgrades enabled it means we only do sanity check on LSIs // as the other checks will be carried out as series of updates. if (!isNewAppSyncAPI) { - const iterativeUpdatesEnabled = ff.getBoolean('enableIterativeGSIUpdates'); - if (iterativeUpdatesEnabled) { - if (!allowDestructiveUpdates) { - diffRules.push( - // primary key rule - getCantEditKeySchemaRule(iterativeUpdatesEnabled), - - // LSI rules - getCantAddLSILaterRule(iterativeUpdatesEnabled), - getCantRemoveLSILater(iterativeUpdatesEnabled), - getCantEditLSIKeySchemaRule(iterativeUpdatesEnabled), - - // remove table rules - cantRemoveTableAfterCreation, - ); - } + if (ff.getBoolean('enableIterativeGSIUpdates')) { + diffRules.push( + // LSI + cantEditKeySchemaRule, + cantAddLSILaterRule, + cantRemoveLSILater, + cantEditLSIKeySchemaRule, + ); - // Project level rule + // Project level rules projectRules.push(cantHaveMoreThan500ResourcesRule); } else { diffRules.push( - // primary key rule - getCantEditKeySchemaRule(), - - // LSI rules - getCantAddLSILaterRule(), - getCantRemoveLSILater(), - getCantEditLSIKeySchemaRule(), - - // GSI rules + // LSI + cantEditKeySchemaRule, + cantAddLSILaterRule, + cantRemoveLSILater, + cantEditLSIKeySchemaRule, + // GSI cantEditGSIKeySchemaRule, cantAddAndRemoveGSIAtSameTimeRule, ); - if (!allowDestructiveUpdates) { - diffRules.push(cantRemoveTableAfterCreation); - } projectRules.push(cantHaveMoreThan500ResourcesRule, cantMutateMultipleGSIAtUpdateTimeRule); } diff --git a/packages/graphql-transformer-core/src/util/sanity-check.ts b/packages/graphql-transformer-core/src/util/sanity-check.ts index 1e2312c00ea..a043673b3ef 100644 --- a/packages/graphql-transformer-core/src/util/sanity-check.ts +++ b/packages/graphql-transformer-core/src/util/sanity-check.ts @@ -1,11 +1,11 @@ import * as fs from 'fs-extra'; import * as path from 'path'; import _ from 'lodash'; -import { Template, ResourceBase } from 'cloudform-types'; +import { Template } from 'cloudform-types'; import { JSONUtilities } from 'amplify-cli-core'; import { diff as getDiffs, Diff as DeepDiff } from 'deep-diff'; import { readFromPath } from './fileUtils'; -import { InvalidMigrationError, InvalidGSIMigrationError, DestructiveMigrationError } from '../errors'; +import { InvalidMigrationError, InvalidGSIMigrationError } from '../errors'; import { TRANSFORM_CONFIG_FILE_NAME } from '..'; type Diff = DeepDiff; @@ -73,29 +73,18 @@ export const sanityCheckDiffs = ( * @param currentBuild The last deployed build. * @param nextBuild The next build. */ -export const getCantEditKeySchemaRule = (iterativeUpdatesEnabled: boolean = false) => { - const cantEditKeySchemaRule = (diff: Diff): void => { - if (diff.kind === 'E' && diff.path.length === 8 && diff.path[5] === 'KeySchema') { - // diff.path = [ "stacks", "Todo.json", "Resources", "TodoTable", "Properties", "KeySchema", 0, "AttributeName"] - const stackName = path.basename(diff.path[1], '.json'); - const tableName = diff.path[3]; - - if (iterativeUpdatesEnabled) { - throw new DestructiveMigrationError( - 'Editing the primary key of a model requires replacement of the underlying DynamoDB table.', - [], - [tableName], - ); - } +export const cantEditKeySchemaRule = (diff: Diff): void => { + if (diff.kind === 'E' && diff.path.length === 8 && diff.path[5] === 'KeySchema') { + // diff.path = [ "stacks", "Todo.json", "Resources", "TodoTable", "Properties", "KeySchema", 0, "AttributeName"] + const stackName = path.basename(diff.path[1], '.json'); + const tableName = diff.path[3]; - throw new InvalidMigrationError( - `Attempting to edit the key schema of the ${tableName} table in the ${stackName} stack. `, - 'Adding a primary @key directive to an existing @model. ', - 'Remove the @key directive or provide a name e.g @key(name: "ByStatus", fields: ["status"]).', - ); - } - }; - return cantEditKeySchemaRule; + throw new InvalidMigrationError( + `Attempting to edit the key schema of the ${tableName} table in the ${stackName} stack. `, + 'Adding a primary @key directive to an existing @model. ', + 'Remove the @key directive or provide a name e.g @key(name: "ByStatus", fields: ["status"]).', + ); + } }; /** @@ -105,35 +94,24 @@ export const getCantEditKeySchemaRule = (iterativeUpdatesEnabled: boolean = fals * @param currentBuild The last deployed build. * @param nextBuild The next build. */ -export const getCantAddLSILaterRule = (iterativeUpdatesEnabled: boolean = false) => { - const cantAddLSILaterRule = (diff: Diff): void => { - if ( - // When adding a LSI to a table that has 0 LSIs. - (diff.kind === 'N' && diff.path.length === 6 && diff.path[5] === 'LocalSecondaryIndexes') || - // When adding a LSI to a table that already has at least one LSI. - (diff.kind === 'A' && diff.path.length === 6 && diff.path[5] === 'LocalSecondaryIndexes' && diff.item.kind === 'N') - ) { - // diff.path = [ "stacks", "Todo.json", "Resources", "TodoTable", "Properties", "LocalSecondaryIndexes" ] - const stackName = path.basename(diff.path[1], '.json'); - const tableName = diff.path[3]; - - if (iterativeUpdatesEnabled) { - throw new DestructiveMigrationError( - 'Adding an LSI to a model requires replacement of the underlying DynamoDB table.', - [], - [tableName], - ); - } +export const cantAddLSILaterRule = (diff: Diff): void => { + if ( + // When adding a LSI to a table that has 0 LSIs. + (diff.kind === 'N' && diff.path.length === 6 && diff.path[5] === 'LocalSecondaryIndexes') || + // When adding a LSI to a table that already has at least one LSI. + (diff.kind === 'A' && diff.path.length === 6 && diff.path[5] === 'LocalSecondaryIndexes' && diff.item.kind === 'N') + ) { + // diff.path = [ "stacks", "Todo.json", "Resources", "TodoTable", "Properties", "LocalSecondaryIndexes" ] + const stackName = path.basename(diff.path[1], '.json'); + const tableName = diff.path[3]; - throw new InvalidMigrationError( - `Attempting to add a local secondary index to the ${tableName} table in the ${stackName} stack. ` + - 'Local secondary indexes must be created when the table is created.', - "Adding a @key directive where the first field in 'fields' is the same as the first field in the 'fields' of the primary @key.", - "Change the first field in 'fields' such that a global secondary index is created or delete and recreate the model.", - ); - } - }; - return cantAddLSILaterRule; + throw new InvalidMigrationError( + `Attempting to add a local secondary index to the ${tableName} table in the ${stackName} stack. ` + + 'Local secondary indexes must be created when the table is created.', + "Adding a @key directive where the first field in 'fields' is the same as the first field in the 'fields' of the primary @key.", + "Change the first field in 'fields' such that a global secondary index is created or delete and recreate the model.", + ); + } }; /** @@ -317,78 +295,65 @@ export const cantMutateMultipleGSIAtUpdateTimeRule = (diffs: Diff[], currentBuil * @param currentBuild The last deployed build. * @param nextBuild The next build. */ -export const getCantEditLSIKeySchemaRule = (iterativeUpdatesEnabled: boolean = false) => { - const cantEditLSIKeySchemaRule = (diff: Diff, currentBuild: DiffableProject, nextBuild: DiffableProject): void => { - if ( - // ["stacks","Todo.json","Resources","TodoTable","Properties","LocalSecondaryIndexes",0,"KeySchema",0,"AttributeName"] - diff.kind === 'E' && - diff.path.length === 10 && - diff.path[5] === 'LocalSecondaryIndexes' && - diff.path[7] === 'KeySchema' - ) { - // This error is symptomatic of a change to the GSI array but does not necessarily imply a breaking change. - const pathToGSIs = diff.path.slice(0, 6); - const oldIndexes = _.get(currentBuild, pathToGSIs); - const newIndexes = _.get(nextBuild, pathToGSIs); - const oldIndexesDiffable = _.keyBy(oldIndexes, 'IndexName'); - const newIndexesDiffable = _.keyBy(newIndexes, 'IndexName'); - const innerDiffs = getDiffs(oldIndexesDiffable, newIndexesDiffable) || []; - - // We must look at this inner diff or else we could confuse a situation - // where the user adds a LSI to the beginning of the LocalSecondaryIndex list in CFN. - // We re-key the indexes list so we can determine if a change occurred to an index that - // already exists. - for (const innerDiff of innerDiffs) { - // path: ["AGSI","KeySchema",0,"AttributeName"] - if (innerDiff.kind === 'E' && innerDiff.path.length > 2 && innerDiff.path[1] === 'KeySchema') { - const indexName = innerDiff.path[0]; - const stackName = path.basename(diff.path[1], '.json'); - const tableName = diff.path[3]; - - if (iterativeUpdatesEnabled) { - throw new DestructiveMigrationError('Editing an LSI requires replacement of the underlying DynamoDB table.', [], [tableName]); - } - - throw new InvalidMigrationError( - `Attempting to edit the local secondary index ${indexName} on the ${tableName} table in the ${stackName} stack. `, - 'The key schema of a local secondary index cannot be changed after being deployed.', - 'When enabling new access patterns you should: 1. Add a new @key 2. run amplify push ' + - '3. Verify the new access pattern and remove the old @key.', - ); - } +export const cantEditLSIKeySchemaRule = (diff: Diff, currentBuild: DiffableProject, nextBuild: DiffableProject): void => { + if ( + // ["stacks","Todo.json","Resources","TodoTable","Properties","LocalSecondaryIndexes",0,"KeySchema",0,"AttributeName"] + diff.kind === 'E' && + diff.path.length === 10 && + diff.path[5] === 'LocalSecondaryIndexes' && + diff.path[7] === 'KeySchema' + ) { + // This error is symptomatic of a change to the GSI array but does not necessarily imply a breaking change. + const pathToGSIs = diff.path.slice(0, 6); + const oldIndexes = _.get(currentBuild, pathToGSIs); + const newIndexes = _.get(nextBuild, pathToGSIs); + const oldIndexesDiffable = _.keyBy(oldIndexes, 'IndexName'); + const newIndexesDiffable = _.keyBy(newIndexes, 'IndexName'); + const innerDiffs = getDiffs(oldIndexesDiffable, newIndexesDiffable) || []; + + // We must look at this inner diff or else we could confuse a situation + // where the user adds a LSI to the beginning of the LocalSecondaryIndex list in CFN. + // We re-key the indexes list so we can determine if a change occurred to an index that + // already exists. + for (const innerDiff of innerDiffs) { + // path: ["AGSI","KeySchema",0,"AttributeName"] + if (innerDiff.kind === 'E' && innerDiff.path.length > 2 && innerDiff.path[1] === 'KeySchema') { + const indexName = innerDiff.path[0]; + const stackName = path.basename(diff.path[1], '.json'); + const tableName = diff.path[3]; + + throw new InvalidMigrationError( + `Attempting to edit the local secondary index ${indexName} on the ${tableName} table in the ${stackName} stack. `, + 'The key schema of a local secondary index cannot be changed after being deployed.', + 'When enabling new access patterns you should: 1. Add a new @key 2. run amplify push ' + + '3. Verify the new access pattern and remove the old @key.', + ); } } - }; - return cantEditLSIKeySchemaRule; + } }; -export const getCantRemoveLSILater = (iterativeUpdatesEnabled: boolean = false) => { - const cantRemoveLSILater = (diff: Diff, currentBuild: DiffableProject, nextBuild: DiffableProject) => { - const throwError = (stackName: string, tableName: string): void => { - if (iterativeUpdatesEnabled) { - throw new DestructiveMigrationError('Removing an LSI requires replacement of the underlying DynamoDB table.', [], [tableName]); - } - throw new InvalidMigrationError( - `Attempting to remove a local secondary index on the ${tableName} table in the ${stackName} stack.`, - 'A local secondary index cannot be removed after deployment.', - 'In order to remove the local secondary index you need to delete or rename the table.', - ); - }; - // if removing more than one lsi - if (diff.kind === 'D' && diff.lhs && diff.path.length === 6 && diff.path[5] === 'LocalSecondaryIndexes') { - const tableName = diff.path[3]; - const stackName = path.basename(diff.path[1], '.json'); - throwError(stackName, tableName); - } - // if removing one lsi - if (diff.kind === 'A' && diff.item.kind === 'D' && diff.path.length === 6 && diff.path[5] === 'LocalSecondaryIndexes') { - const tableName = diff.path[3]; - const stackName = path.basename(diff.path[1], '.json'); - throwError(stackName, tableName); - } +export function cantRemoveLSILater(diff: Diff, currentBuild: DiffableProject, nextBuild: DiffableProject) { + const throwError = (stackName: string, tableName: string): void => { + throw new InvalidMigrationError( + `Attempting to remove a local secondary index on the ${tableName} table in the ${stackName} stack.`, + 'A local secondary index cannot be removed after deployment.', + 'In order to remove the local secondary index you need to delete or rename the table.', + ); }; - return cantRemoveLSILater; -}; + // if removing more than one lsi + if (diff.kind === 'D' && diff.lhs && diff.path.length === 6 && diff.path[5] === 'LocalSecondaryIndexes') { + const tableName = diff.path[3]; + const stackName = path.basename(diff.path[1], '.json'); + throwError(stackName, tableName); + } + // if removing one lsi + if (diff.kind === 'A' && diff.item.kind === 'D' && diff.path.length === 6 && diff.path[5] === 'LocalSecondaryIndexes') { + const tableName = diff.path[3]; + const stackName = path.basename(diff.path[1], '.json'); + throwError(stackName, tableName); + } +} export const cantHaveMoreThan500ResourcesRule = (diffs: Diff[], currentBuild: DiffableProject, nextBuild: DiffableProject): void => { const stackKeys = Object.keys(nextBuild.stacks); @@ -408,23 +373,6 @@ export const cantHaveMoreThan500ResourcesRule = (diffs: Diff[], currentBuild: Di } }; -export const cantRemoveTableAfterCreation = (_: Diff, currentBuild: DiffableProject, nextBuild: DiffableProject): void => { - const getNestedStackLogicalIds = (proj: DiffableProject) => - Object.entries(proj.root.Resources || []) - .filter(([_, meta]) => meta.Type === 'AWS::CloudFormation::Stack') - .map(([name]) => name); - const currentModels = getNestedStackLogicalIds(currentBuild); - const nextModels = getNestedStackLogicalIds(nextBuild); - const removedModels = currentModels.filter(currModel => !nextModels.includes(currModel)); - if (removedModels.length > 0) { - throw new DestructiveMigrationError( - 'Removing a model from the GraphQL schema will also remove the underlying DynamoDB table.', - removedModels, - [], - ); - } -}; - const loadDiffableProject = async (path: string, rootStackName: string): Promise => { const project = await readFromPath(path); const currentStacks = project.stacks || {};