From 9f21969c64a49f4ff195f20ed294524096e6803e Mon Sep 17 00:00:00 2001 From: Edward Foyle Date: Wed, 28 Jul 2021 19:28:35 -0700 Subject: [PATCH 01/21] chore: dumping table changes --- .../amplify-graphql-resource-manager.ts | 47 +++++++++++++++++-- 1 file changed, 44 insertions(+), 3 deletions(-) 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 cd82d286df8..540abea3ce9 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 @@ -21,6 +21,8 @@ import { loadConfiguration } from '../configuration-manager'; import fs from 'fs-extra'; import path from 'path'; +const environmentWorkflowStage = () => 'DEVELOPMENT'; + export type GQLResourceManagerProps = { cfnClient: CloudFormation; resourceMeta?: ResourceMeta; @@ -103,6 +105,7 @@ export class GraphQLResourceManager { } } this.gsiManagement(gqlDiff.diff, gqlDiff.current, gqlDiff.next); + this.tableRecreationManagement(gqlDiff.diff, gqlDiff.current, gqlDiff.next); return await this.getDeploymentSteps(); }; @@ -217,9 +220,9 @@ export class GraphQLResourceManager { }); const tableWithGSIChanges = _.uniqBy(gsiChanges, diff => diff.path?.slice(0, 3).join('/')).map(gsiChange => { - const tableName = gsiChange.path[3]; + const tableName = gsiChange.path[3] as string; - const stackName = gsiChange.path[1].split('.')[0]; + const stackName = gsiChange.path[1].split('.')[0] as string; const currentTable = this.getTable(gsiChange, currentState); const nextTable = this.getTable(gsiChange, nextState); @@ -233,9 +236,16 @@ export class GraphQLResourceManager { }); for (const gsiChange of tableWithGSIChanges) { - const changeSteps = getGSIDiffs(gsiChange.currentTable, gsiChange.nextTable); const stackName = gsiChange.stackName; const tableName = gsiChange.tableName; + const changeSteps = getGSIDiffs(gsiChange.currentTable, gsiChange.nextTable); + if (environmentWorkflowStage() === 'DEVELOPMENT' && changeSteps.length > 1) { + const ddbResource = this.getStack(stackName, currentState); + this.dropTable(tableName, ddbResource); + this.templateState.add(stackName, JSONUtilities.stringify(ddbResource)); + this.templateState.add(stackName, JSONUtilities.stringify(this.getStack(stackName, nextState))); + continue; + } for (const changeStep of changeSteps) { const ddbResource = this.templateState.getLatest(stackName) || this.getStack(stackName, currentState); let gsiRecord; @@ -266,6 +276,27 @@ export class GraphQLResourceManager { } }; + private tableRecreationManagement = (diffs: DiffChanges, currentState: DiffableProject, nextState: DiffableProject) => { + const tablesToRecreate = _.uniq( + diffs + .filter(diff => diff.path.includes('KeySchema')) // filter diffs with KeySchema changes + .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, + })), + ); + + tablesToRecreate.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))); + }); + }; + private getTable = (gsiChange: Diff, proj: DiffableProject): DynamoDB.Table => { return proj.stacks[gsiChange.path[1]].Resources[gsiChange.path[3]] as DynamoDB.Table; }; @@ -283,6 +314,16 @@ export class GraphQLResourceManager { const table = template.Resources[tableName] as DynamoDB.Table; template.Resources[tableName] = removeGSI(indexName, table); }; + + private dropTable = (tableName: string, template: Template): void => { + template.Resources[tableName] = undefined; + }; + + private clearTemplateState = (stackName: string) => { + while (this.templateState.has(stackName)) { + this.templateState.pop(stackName); + } + }; } // https://stackoverflow.com/questions/39419170/how-do-i-check-that-a-switch-block-is-exhaustive-in-typescript From 8c774e81a4e331dae5c650e5bff5b1eea733c87b Mon Sep 17 00:00:00 2001 From: Edward Foyle Date: Fri, 30 Jul 2021 14:32:48 -0700 Subject: [PATCH 02/21] chore: drop table POC --- .../amplify-graphql-resource-manager.ts | 12 ++---------- .../environment-workflow-stage.ts | 6 ++++++ .../iterative-deployment/deployment-manager.ts | 6 ++++++ .../src/util/amplifyUtils.ts | 18 ++++++++++-------- 4 files changed, 24 insertions(+), 18 deletions(-) create mode 100644 packages/amplify-provider-awscloudformation/src/graphql-transformer/environment-workflow-stage.ts 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 540abea3ce9..f9c7037b9f4 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 @@ -21,8 +21,6 @@ import { loadConfiguration } from '../configuration-manager'; import fs from 'fs-extra'; import path from 'path'; -const environmentWorkflowStage = () => 'DEVELOPMENT'; - export type GQLResourceManagerProps = { cfnClient: CloudFormation; resourceMeta?: ResourceMeta; @@ -239,13 +237,6 @@ export class GraphQLResourceManager { const stackName = gsiChange.stackName; const tableName = gsiChange.tableName; const changeSteps = getGSIDiffs(gsiChange.currentTable, gsiChange.nextTable); - if (environmentWorkflowStage() === 'DEVELOPMENT' && changeSteps.length > 1) { - const ddbResource = this.getStack(stackName, currentState); - this.dropTable(tableName, ddbResource); - this.templateState.add(stackName, JSONUtilities.stringify(ddbResource)); - this.templateState.add(stackName, JSONUtilities.stringify(this.getStack(stackName, nextState))); - continue; - } for (const changeStep of changeSteps) { const ddbResource = this.templateState.getLatest(stackName) || this.getStack(stackName, currentState); let gsiRecord; @@ -279,7 +270,7 @@ export class GraphQLResourceManager { private tableRecreationManagement = (diffs: DiffChanges, currentState: DiffableProject, nextState: DiffableProject) => { const tablesToRecreate = _.uniq( diffs - .filter(diff => diff.path.includes('KeySchema')) // filter diffs with KeySchema changes + .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, @@ -317,6 +308,7 @@ export class GraphQLResourceManager { private dropTable = (tableName: string, template: Template): void => { template.Resources[tableName] = undefined; + template.Outputs = _.omitBy(template.Outputs, (_, key) => key.includes(tableName)); }; private clearTemplateState = (stackName: string) => { diff --git a/packages/amplify-provider-awscloudformation/src/graphql-transformer/environment-workflow-stage.ts b/packages/amplify-provider-awscloudformation/src/graphql-transformer/environment-workflow-stage.ts new file mode 100644 index 00000000000..b957d685fa9 --- /dev/null +++ b/packages/amplify-provider-awscloudformation/src/graphql-transformer/environment-workflow-stage.ts @@ -0,0 +1,6 @@ +export const getEnvironmentWorkflowStage = (): EnvironmentWorkflowStage => EnvironmentWorkflowStage.DEVELOPMENT; + +export enum EnvironmentWorkflowStage { + DEVELOPMENT = 'DEVELOPMENT', + PRODUCTION = 'PRODUCTION', +} 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 32507727607..04d2b94df8d 100644 --- a/packages/amplify-provider-awscloudformation/src/iterative-deployment/deployment-manager.ts +++ b/packages/amplify-provider-awscloudformation/src/iterative-deployment/deployment-manager.ts @@ -327,9 +327,15 @@ 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/graphql-transformer-core/src/util/amplifyUtils.ts b/packages/graphql-transformer-core/src/util/amplifyUtils.ts index 869fafc1914..78a8f234f88 100644 --- a/packages/graphql-transformer-core/src/util/amplifyUtils.ts +++ b/packages/graphql-transformer-core/src/util/amplifyUtils.ts @@ -727,20 +727,22 @@ function getOrDefault(o: any, k: string, d: any) { return o[k] || d; } -export function getSanityCheckRules(isNewAppSyncAPI: boolean, ff: FeatureFlagProvider) { +export function getSanityCheckRules(isNewAppSyncAPI: boolean, ff: FeatureFlagProvider, allowDestructiveUpdates: boolean = false) { 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) { if (ff.getBoolean('enableIterativeGSIUpdates')) { - diffRules.push( - // LSI - cantEditKeySchemaRule, - cantAddLSILaterRule, - cantRemoveLSILater, - cantEditLSIKeySchemaRule, - ); + if (!allowDestructiveUpdates) { + diffRules.push( + // LSI + cantEditKeySchemaRule, + cantAddLSILaterRule, + cantRemoveLSILater, + cantEditLSIKeySchemaRule, + ); + } // Project level rules projectRules.push(cantHaveMoreThan500ResourcesRule); From e6b43f16c33b2833c3820c9bc2245546da431606 Mon Sep 17 00:00:00 2001 From: Edward Foyle Date: Wed, 1 Sep 2021 14:55:23 -0700 Subject: [PATCH 03/21] feat: plumb destructive-updates flag --- packages/amplify-cli/src/commands/push.ts | 2 +- .../src/constants.js | 1 + .../environment-workflow-stage.ts | 6 -- .../src/transform-graphql-schema.ts | 11 ++-- .../graphql-transformer-core/src/errors.ts | 32 +++++++--- .../src/util/amplifyUtils.ts | 39 ++++++++---- .../src/util/sanity-check.ts | 62 ++++++++++++++++--- 7 files changed, 112 insertions(+), 41 deletions(-) delete mode 100644 packages/amplify-provider-awscloudformation/src/graphql-transformer/environment-workflow-stage.ts diff --git a/packages/amplify-cli/src/commands/push.ts b/packages/amplify-cli/src/commands/push.ts index e5893af3aea..42b8a3171fd 100644 --- a/packages/amplify-cli/src/commands/push.ts +++ b/packages/amplify-cli/src/commands/push.ts @@ -62,7 +62,7 @@ export const run = async (context: $TSContext) => { await syncCurrentCloudBackend(context); return await context.amplify.pushResources(context); } catch (e) { - if (e.name !== 'InvalidDirectiveError') { + if (!['InvalidDirectiveError', 'DestructiveMigrationError'].includes(e.name)) { const message = e.name === 'GraphQLError' ? e.toString() : e.message; context.print.error(`An error occurred during the push operation: ${message}`); } diff --git a/packages/amplify-provider-awscloudformation/src/constants.js b/packages/amplify-provider-awscloudformation/src/constants.js index d05deef8387..4b6232b492c 100644 --- a/packages/amplify-provider-awscloudformation/src/constants.js +++ b/packages/amplify-provider-awscloudformation/src/constants.js @@ -15,4 +15,5 @@ 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/graphql-transformer/environment-workflow-stage.ts b/packages/amplify-provider-awscloudformation/src/graphql-transformer/environment-workflow-stage.ts deleted file mode 100644 index b957d685fa9..00000000000 --- a/packages/amplify-provider-awscloudformation/src/graphql-transformer/environment-workflow-stage.ts +++ /dev/null @@ -1,6 +0,0 @@ -export const getEnvironmentWorkflowStage = (): EnvironmentWorkflowStage => EnvironmentWorkflowStage.DEVELOPMENT; - -export enum EnvironmentWorkflowStage { - DEVELOPMENT = 'DEVELOPMENT', - PRODUCTION = 'PRODUCTION', -} diff --git a/packages/amplify-provider-awscloudformation/src/transform-graphql-schema.ts b/packages/amplify-provider-awscloudformation/src/transform-graphql-schema.ts index adff96c3292..19a001c5562 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 { ProviderName as providerName } from './constants'; +import { destructiveUpdatesFlag, ProviderName as providerName } from './constants'; import { AmplifyCLIFeatureFlagAdapter } from './utils/amplify-cli-feature-flag-adapter'; import { isAmplifyAdminApp } from './utils/admin-helpers'; -import { JSONUtilities, stateManager } from 'amplify-cli-core'; +import { $TSContext, JSONUtilities, stateManager } from 'amplify-cli-core'; import { collectDirectivesByTypeNames, @@ -290,7 +290,7 @@ async function migrateProject(context, options) { } } -export async function transformGraphQLSchema(context, options) { +export async function transformGraphQLSchema(context: $TSContext, options) { const useExperimentalPipelineTransformer = FeatureFlags.getBoolean('graphQLTransformer.useExperimentalPipelinedTransformer'); if (useExperimentalPipelineTransformer) { return transformGraphQLSchemaV6(context, options); @@ -361,7 +361,7 @@ export async function transformGraphQLSchema(context, options) { if (!parameters && fs.existsSync(parametersFilePath)) { try { - parameters = context.amplify.readJsonFile(parametersFilePath); + parameters = JSONUtilities.readJson(parametersFilePath); } catch (e) { parameters = {}; } @@ -474,7 +474,8 @@ export async function transformGraphQLSchema(context, options) { } const ff = new AmplifyCLIFeatureFlagAdapter(); - const sanityCheckRulesList = getSanityCheckRules(isNewAppSyncAPI, ff); + const allowDestructiveUpdates = context?.input?.options?.[destructiveUpdatesFlag] || context.input?.options?.force; + const sanityCheckRulesList = getSanityCheckRules(isNewAppSyncAPI, ff, allowDestructiveUpdates); const buildConfig = { ...options, diff --git a/packages/graphql-transformer-core/src/errors.ts b/packages/graphql-transformer-core/src/errors.ts index cd3b97d03e6..11552c6216d 100644 --- a/packages/graphql-transformer-core/src/errors.ts +++ b/packages/graphql-transformer-core/src/errors.ts @@ -41,23 +41,37 @@ 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, run 'amplify push --allow-destructuve-graphql-schema-updates' to continue.`; + } + 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 { - fix: string; - cause: string; - constructor(message: string, cause: string, fix: string) { + constructor(message: string, public cause: string, public fix: string) { super(message); - Object.setPrototypeOf(this, InvalidMigrationError.prototype); + Object.setPrototypeOf(this, new.target.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 78a8f234f88..f9221fa62d7 100644 --- a/packages/graphql-transformer-core/src/util/amplifyUtils.ts +++ b/packages/graphql-transformer-core/src/util/amplifyUtils.ts @@ -20,6 +20,7 @@ import { sanityCheckProject, ProjectRule, cantMutateMultipleGSIAtUpdateTimeRule, + cantRemoveTableAfterCreation, } from './sanity-check'; export const CLOUDFORMATION_FILE_NAME = 'cloudformation-template.json'; @@ -733,30 +734,42 @@ export function getSanityCheckRules(isNewAppSyncAPI: boolean, ff: FeatureFlagPro // 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) { - if (ff.getBoolean('enableIterativeGSIUpdates')) { + const iterativeUpdatesEnabled = ff.getBoolean('enableIterativeGSIUpdates'); + if (iterativeUpdatesEnabled) { if (!allowDestructiveUpdates) { diffRules.push( - // LSI - cantEditKeySchemaRule, - cantAddLSILaterRule, - cantRemoveLSILater, - cantEditLSIKeySchemaRule, + // primary key rule + cantEditKeySchemaRule(iterativeUpdatesEnabled), + + // LSI rules + cantAddLSILaterRule(iterativeUpdatesEnabled), + cantRemoveLSILater(iterativeUpdatesEnabled), + cantEditLSIKeySchemaRule(iterativeUpdatesEnabled), + + // remove table rules + cantRemoveTableAfterCreation, ); } - // Project level rules + // Project level rule projectRules.push(cantHaveMoreThan500ResourcesRule); } else { diffRules.push( - // LSI - cantEditKeySchemaRule, - cantAddLSILaterRule, - cantRemoveLSILater, - cantEditLSIKeySchemaRule, - // GSI + // primary key rule + cantEditKeySchemaRule(), + + // LSI rules + cantAddLSILaterRule(), + cantRemoveLSILater(), + cantEditLSIKeySchemaRule(), + + // GSI rules 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 a043673b3ef..2a44c75d617 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 } from 'cloudform-types'; +import { Template, ResourceBase } 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 } from '../errors'; +import { InvalidMigrationError, InvalidGSIMigrationError, DestructiveMigrationError } from '../errors'; import { TRANSFORM_CONFIG_FILE_NAME } from '..'; type Diff = DeepDiff; @@ -73,12 +73,20 @@ export const sanityCheckDiffs = ( * @param currentBuild The last deployed build. * @param nextBuild The next build. */ -export const cantEditKeySchemaRule = (diff: Diff): void => { +export const cantEditKeySchemaRule = (iterativeUpdatesEnabled: boolean = false) => (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], + ); + } + 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. ', @@ -94,7 +102,7 @@ export const cantEditKeySchemaRule = (diff: Diff): void => { * @param currentBuild The last deployed build. * @param nextBuild The next build. */ -export const cantAddLSILaterRule = (diff: Diff): void => { +export const cantAddLSILaterRule = (iterativeUpdatesEnabled: boolean = false) => (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') || @@ -105,6 +113,14 @@ export const cantAddLSILaterRule = (diff: Diff): void => { 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], + ); + } + 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.', @@ -295,7 +311,11 @@ export const cantMutateMultipleGSIAtUpdateTimeRule = (diffs: Diff[], currentBuil * @param currentBuild The last deployed build. * @param nextBuild The next build. */ -export const cantEditLSIKeySchemaRule = (diff: Diff, currentBuild: DiffableProject, nextBuild: DiffableProject): void => { +export const cantEditLSIKeySchemaRule = (iterativeUpdatesEnabled: boolean = false) => ( + diff: Diff, + currentBuild: DiffableProject, + nextBuild: DiffableProject, +): void => { if ( // ["stacks","Todo.json","Resources","TodoTable","Properties","LocalSecondaryIndexes",0,"KeySchema",0,"AttributeName"] diff.kind === 'E' && @@ -322,6 +342,10 @@ export const cantEditLSIKeySchemaRule = (diff: Diff, currentBuild: DiffableProje 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.', @@ -333,8 +357,15 @@ export const cantEditLSIKeySchemaRule = (diff: Diff, currentBuild: DiffableProje } }; -export function cantRemoveLSILater(diff: Diff, currentBuild: DiffableProject, nextBuild: DiffableProject) { +export const cantRemoveLSILater = (iterativeUpdatesEnabled: boolean = false) => ( + 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.', @@ -353,7 +384,7 @@ export function cantRemoveLSILater(diff: Diff, currentBuild: DiffableProject, ne 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); @@ -373,6 +404,23 @@ 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 || {}; From 12e6c356d59a197c8b47be21b38adfc39f1eb25d Mon Sep 17 00:00:00 2001 From: Edward Foyle Date: Thu, 2 Sep 2021 13:07:29 -0700 Subject: [PATCH 04/21] feat: plumb rebuild --- .../amplify-category-api/amplify-plugin.json | 25 ++++-------- packages/amplify-category-api/package.json | 1 + .../amplify-category-api/src/commands/api.js | 5 +++ .../src/commands/api/rebuild.ts | 39 +++++++++++++++++++ packages/amplify-category-api/tsconfig.json | 2 + packages/amplify-cli-core/src/index.ts | 5 +-- packages/amplify-cli/src/commands/push.ts | 2 +- .../amplify-helpers/push-resources.ts | 28 ++++++------- .../amplify-graphql-resource-manager.ts | 38 +++++++++++++----- .../src/index.ts | 4 +- .../src/push-resources.ts | 31 +++++---------- .../graphql-transformer-core/src/errors.ts | 2 +- 12 files changed, 113 insertions(+), 69 deletions(-) create mode 100644 packages/amplify-category-api/src/commands/api/rebuild.ts diff --git a/packages/amplify-category-api/amplify-plugin.json b/packages/amplify-category-api/amplify-plugin.json index 8cf4ad9d46c..8bd97a6a3c7 100644 --- a/packages/amplify-category-api/amplify-plugin.json +++ b/packages/amplify-category-api/amplify-plugin.json @@ -1,18 +1,9 @@ { - "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 + "name": "api", + "type": "category", + "commands": ["add-graphql-datasource", "add", "console", "gql-compile", "push", "rebuild", "remove", "update", "help"], + "commandAliases": { + "configure": "update" + }, + "eventHandlers": [] +} diff --git a/packages/amplify-category-api/package.json b/packages/amplify-category-api/package.json index 8e440002aed..2906da165f0 100644 --- a/packages/amplify-category-api/package.json +++ b/packages/amplify-category-api/package.json @@ -63,6 +63,7 @@ "@aws-cdk/region-info": "~1.119.0", "@graphql-tools/merge": "^6.0.18", "amplify-cli-core": "1.27.0", + "amplify-prompts": "1.1.2", "amplify-util-headless-input": "1.5.3", "chalk": "^4.1.1", "constructs": "^3.3.125", diff --git a/packages/amplify-category-api/src/commands/api.js b/packages/amplify-category-api/src/commands/api.js index 948dc4ae4a2..e8c261e00df 100644 --- a/packages/amplify-category-api/src/commands/api.js +++ b/packages/amplify-category-api/src/commands/api.js @@ -41,6 +41,11 @@ 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 new file mode 100644 index 00000000000..fe83efb3ba5 --- /dev/null +++ b/packages/amplify-category-api/src/commands/api/rebuild.ts @@ -0,0 +1,39 @@ +import { $TSContext, FeatureFlags, stateManager } from 'amplify-cli-core'; +import { printer, prompter } 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 the 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.', + ); + } + 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: input => (input === apiName ? true : 'Input does not match 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 dd332b004a4..31c85559bbb 100644 --- a/packages/amplify-category-api/tsconfig.json +++ b/packages/amplify-category-api/tsconfig.json @@ -14,7 +14,9 @@ "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-cli-core/src/index.ts b/packages/amplify-cli-core/src/index.ts index 98bef730dc8..22053d6bb57 100644 --- a/packages/amplify-cli-core/src/index.ts +++ b/packages/amplify-cli-core/src/index.ts @@ -242,6 +242,7 @@ interface AmplifyToolkit { category?: string, resourceName?: string, filteredResources?: { category: string; resourceName: string }[], + rebuild?: boolean, ) => $TSAny; storeCurrentCloudBackend: () => $TSAny; readJsonFile: () => $TSAny; @@ -310,9 +311,7 @@ interface AmplifyToolkit { leaveBreadcrumbs: (category: string, resourceName: string, breadcrumbs: unknown) => void; readBreadcrumbs: (category: string, resourceName: string) => $TSAny; loadRuntimePlugin: (context: $TSContext, pluginId: string) => Promise<$TSAny>; - getImportedAuthProperties: ( - context: $TSContext, - ) => { + getImportedAuthProperties: (context: $TSContext) => { imported: boolean; userPoolId?: string; authRoleArn?: string; diff --git a/packages/amplify-cli/src/commands/push.ts b/packages/amplify-cli/src/commands/push.ts index 42b8a3171fd..e5893af3aea 100644 --- a/packages/amplify-cli/src/commands/push.ts +++ b/packages/amplify-cli/src/commands/push.ts @@ -62,7 +62,7 @@ export const run = async (context: $TSContext) => { await syncCurrentCloudBackend(context); return await context.amplify.pushResources(context); } catch (e) { - if (!['InvalidDirectiveError', 'DestructiveMigrationError'].includes(e.name)) { + if (e.name !== 'InvalidDirectiveError') { const message = e.name === 'GraphQLError' ? e.toString() : e.message; context.print.error(`An error occurred during the push operation: ${message}`); } 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 4e71549cc77..9ae1f8a6700 100644 --- a/packages/amplify-cli/src/extensions/amplify-helpers/push-resources.ts +++ b/packages/amplify-cli/src/extensions/amplify-helpers/push-resources.ts @@ -11,6 +11,7 @@ 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 @@ -52,7 +53,7 @@ export async function pushResources( const hasChanges = await showResourceTable(category, resourceName, filteredResources); // no changes detected - if (!hasChanges && !context.exeInfo.forcePush) { + if (!hasChanges && !context.exeInfo.forcePush && !rebuild) { context.print.info('\nNo changes detected'); return context; @@ -68,18 +69,11 @@ export async function pushResources( } if (continueToPush) { - try { - // Get current-cloud-backend's amplify-meta - const currentAmplifyMeta = stateManager.getCurrentMeta(); + // 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}`); - - throw err; - } + await providersPush(context, rebuild, category, resourceName, filteredResources); + await onCategoryOutputsChange(context, currentAmplifyMeta); } else { // there's currently no other mechanism to stop the execution of the postPush workflow in this case, so exiting here exitOnNextTick(1); @@ -88,7 +82,13 @@ export async function pushResources( return continueToPush; } -async function providersPush(context: $TSContext, category, resourceName, filteredResources) { +async function providersPush( + context: $TSContext, + rebuild: boolean = false, + category?: string, + resourceName?: string, + filteredResources?: { category: string; resourceName: string }[], +) { const { providers } = getProjectConfig(); const providerPlugins = getProviderPlugins(context); const providerPromises: (() => Promise<$TSAny>)[] = []; @@ -96,7 +96,7 @@ async function providersPush(context: $TSContext, category, resourceName, filter 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)); + providerPromises.push(providerModule.pushResources(context, resourceDefiniton, rebuild)); } await Promise.all(providerPromises); 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 f9c7037b9f4..a2b337ae768 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,6 +26,7 @@ export type GQLResourceManagerProps = { resourceMeta?: ResourceMeta; backendDir: string; cloudBackendDir: string; + rebuildAllTables?: boolean; }; export type ResourceMeta = { @@ -52,8 +53,9 @@ 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) => { + public static createInstance = async (context: $TSContext, gqlResource: any, StackId: string, rebuildAllTables: boolean = false) => { try { const cred = await loadConfiguration(context); const cfn = new CloudFormation(cred); @@ -65,6 +67,7 @@ export class GraphQLResourceManager { resourceMeta: { ...gqlResource, stackId: apiStack.StackResources[0].PhysicalResourceId }, backendDir: pathManager.getBackendDirPath(), cloudBackendDir: pathManager.getCurrentCloudBackendDirPath(), + rebuildAllTables, }); } catch (err) { throw err; @@ -82,6 +85,7 @@ 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 => { @@ -268,15 +272,26 @@ export class GraphQLResourceManager { }; private tableRecreationManagement = (diffs: DiffChanges, currentState: DiffableProject, nextState: DiffableProject) => { - const tablesToRecreate = _.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, - })), - ); + 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, + })), + ); + + const getAllTables = () => + Object.entries(currentState.stacks) + .map(([name, template]) => ({ + tableName: this.getTableNameFromTemplate(template), + stackName: path.basename(name, '.json'), + })) + .filter(meta => !!meta.tableName); + + const tablesToRecreate = this.rebuildAllTables ? getAllTables() : getTablesRequiringReplacement(); tablesToRecreate.forEach(tableMeta => { const ddbResource = this.getStack(tableMeta.stackName, currentState); @@ -316,6 +331,9 @@ export class GraphQLResourceManager { 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/index.ts b/packages/amplify-provider-awscloudformation/src/index.ts index 1c5e68714f8..9574bd29f60 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) { - return resourcePusher.run(context, resourceList); +function pushResources(context, resourceList, rebuild: boolean = false) { + return resourcePusher.run(context, resourceList, rebuild); } function storeCurrentCloudBackend(context) { diff --git a/packages/amplify-provider-awscloudformation/src/push-resources.ts b/packages/amplify-provider-awscloudformation/src/push-resources.ts index bfacf65bdeb..347b57f58d0 100644 --- a/packages/amplify-provider-awscloudformation/src/push-resources.ts +++ b/packages/amplify-provider-awscloudformation/src/push-resources.ts @@ -67,26 +67,20 @@ const deploymentInProgressErrorMessage = (context: $TSContext) => { context.print.error('"amplify push --force" to re-deploy'); }; -export async function run(context: $TSContext, resourceDefinition: $TSObject) { +export async function run(context: $TSContext, resourceDefinition: $TSObject, rebuild: boolean = false) { 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 ? allResources : resourcesToBeCreated.concat(resourcesToBeUpdated); + let resources = !!context?.exeInfo?.forcePush || rebuild ? allResources : resourcesToBeCreated.concat(resourcesToBeUpdated); layerResources = resources.filter(r => r.service === FunctionServiceNameLambdaLayer); if (deploymentStateManager.isDeploymentInProgress() && !deploymentStateManager.isDeploymentFinished()) { @@ -170,10 +164,10 @@ export async function run(context: $TSContext, resourceDefinition: $TSObject) { // Check if iterative updates are enabled or not and generate the required deployment steps if needed. if (FeatureFlags.getBoolean('graphQLTransformer.enableIterativeGSIUpdates')) { - const gqlResource = getGqlUpdatedResource(resourcesToBeUpdated); + const gqlResource = getGqlUpdatedResource(resources); if (gqlResource) { - const gqlManager = await GraphQLResourceManager.createInstance(context, gqlResource, cloudformationMeta.StackId); + const gqlManager = await GraphQLResourceManager.createInstance(context, gqlResource, cloudformationMeta.StackId, rebuild); deploymentSteps = await gqlManager.run(); if (deploymentSteps.length > 1) { iterativeDeploymentWasInvoked = true; @@ -209,7 +203,8 @@ export async function run(context: $TSContext, resourceDefinition: $TSObject) { resourcesToBeUpdated.length > 0 || resourcesToBeDeleted.length > 0 || tagsUpdated || - context.exeInfo.forcePush + context.exeInfo.forcePush || + rebuild ) { // 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) { @@ -1027,14 +1022,8 @@ 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/graphql-transformer-core/src/errors.ts b/packages/graphql-transformer-core/src/errors.ts index 11552c6216d..78114f1a7b7 100644 --- a/packages/graphql-transformer-core/src/errors.ts +++ b/packages/graphql-transformer-core/src/errors.ts @@ -56,7 +56,7 @@ export class DestructiveMigrationError extends Error { } 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, run 'amplify push --allow-destructuve-graphql-schema-updates' to continue.`; + 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; } From 3da906f6d51ed8608bceb3787d0a1bed36a83d31 Mon Sep 17 00:00:00 2001 From: Edward Foyle Date: Thu, 2 Sep 2021 14:34:47 -0700 Subject: [PATCH 05/21] chore: fix rebuild prompts --- .../src/extensions/amplify-helpers/push-resources.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) 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 9ae1f8a6700..3b86fc0c5e2 100644 --- a/packages/amplify-cli/src/extensions/amplify-helpers/push-resources.ts +++ b/packages/amplify-cli/src/extensions/amplify-helpers/push-resources.ts @@ -50,7 +50,11 @@ export async function pushResources( } } - const hasChanges = await showResourceTable(category, resourceName, filteredResources); + 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); + } // no changes detected if (!hasChanges && !context.exeInfo.forcePush && !rebuild) { @@ -59,7 +63,8 @@ export async function pushResources( return context; } - let continueToPush = context.exeInfo && context.exeInfo.inputParams && context.exeInfo.inputParams.yes; + // 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; if (!continueToPush) { if (context.exeInfo.iterativeRollback) { From 6ae881c9cc635f7b88819aeb9c31f3f58eeea863 Mon Sep 17 00:00:00 2001 From: Edward Foyle Date: Fri, 3 Sep 2021 10:41:34 -0700 Subject: [PATCH 06/21] test: adding unit and e2e tests --- .circleci/config.yml | 47 ++++++++++++++ .../__tests__/commands/api/rebuild.test.ts | 64 +++++++++++++++++++ .../src/commands/api/rebuild.ts | 3 +- .../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 | 60 +++++++++++++++++ .../amplify-graphql-resource-manager.ts | 2 +- 9 files changed, 214 insertions(+), 3 deletions(-) create mode 100644 packages/amplify-category-api/src/__tests__/commands/api/rebuild.test.ts create mode 100644 packages/amplify-e2e-tests/schemas/simple_model_new_primary_key.graphql create mode 100644 packages/amplify-e2e-tests/src/__tests__/api_5.test.ts diff --git a/.circleci/config.yml b/.circleci/config.yml index 009542f8331..83b44b53e3f 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -1274,6 +1274,17 @@ jobs: steps: *ref_5 environment: TEST_SUITE: src/__tests__/auth_6.test.ts +<<<<<<< HEAD +======= + CLI_REGION: ap-southeast-1 + 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 +>>>>>>> test: adding unit and e2e tests CLI_REGION: ap-southeast-2 api_4-amplify_e2e_tests: working_directory: ~/repo @@ -2053,6 +2064,16 @@ jobs: TEST_SUITE: src/__tests__/auth_6.test.ts CLI_REGION: ap-southeast-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: ap-southeast-2 + steps: *ref_6 api_4-amplify_e2e_tests_pkg_linux: working_directory: ~/repo docker: *ref_1 @@ -2167,7 +2188,11 @@ workflows: requires: - notifications-amplify_e2e_tests - schema-iterative-update-locking-amplify_e2e_tests +<<<<<<< HEAD - function_7-amplify_e2e_tests +======= + - function_6-amplify_e2e_tests +>>>>>>> test: adding unit and e2e tests - api_4-amplify_e2e_tests - hosting-amplify_e2e_tests - tags-amplify_e2e_tests @@ -2191,13 +2216,22 @@ workflows: - configure-project-amplify_e2e_tests - schema-versioned-amplify_e2e_tests - plugin-amplify_e2e_tests +<<<<<<< HEAD - hooks-amplify_e2e_tests - auth_6-amplify_e2e_tests +======= + - function_7-amplify_e2e_tests + - api_5-amplify_e2e_tests +>>>>>>> test: adding unit and e2e tests - done_with_pkg_linux_e2e_tests: requires: - notifications-amplify_e2e_tests_pkg_linux - schema-iterative-update-locking-amplify_e2e_tests_pkg_linux +<<<<<<< HEAD - function_7-amplify_e2e_tests_pkg_linux +======= + - function_6-amplify_e2e_tests_pkg_linux +>>>>>>> test: adding unit and e2e tests - api_4-amplify_e2e_tests_pkg_linux - hosting-amplify_e2e_tests_pkg_linux - tags-amplify_e2e_tests_pkg_linux @@ -2221,8 +2255,13 @@ workflows: - configure-project-amplify_e2e_tests_pkg_linux - schema-versioned-amplify_e2e_tests_pkg_linux - plugin-amplify_e2e_tests_pkg_linux +<<<<<<< HEAD - hooks-amplify_e2e_tests_pkg_linux - auth_6-amplify_e2e_tests_pkg_linux +======= + - function_7-amplify_e2e_tests_pkg_linux + - api_5-amplify_e2e_tests_pkg_linux +>>>>>>> test: adding unit and e2e tests - amplify_migration_tests_latest: context: - amplify-ecr-image-pull @@ -2840,7 +2879,11 @@ workflows: filters: *ref_10 requires: - auth_1-amplify_e2e_tests +<<<<<<< HEAD - auth_6-amplify_e2e_tests: +======= + - api_5-amplify_e2e_tests: +>>>>>>> test: adding unit and e2e tests context: *ref_8 post-steps: *ref_9 filters: *ref_10 @@ -3346,7 +3389,11 @@ workflows: filters: *ref_13 requires: - auth_1-amplify_e2e_tests_pkg_linux +<<<<<<< HEAD - auth_6-amplify_e2e_tests_pkg_linux: +======= + - api_5-amplify_e2e_tests_pkg_linux: +>>>>>>> test: adding unit and e2e tests context: *ref_11 post-steps: *ref_12 filters: *ref_13 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 new file mode 100644 index 00000000000..ce28470ecd0 --- /dev/null +++ b/packages/amplify-category-api/src/__tests__/commands/api/rebuild.test.ts @@ -0,0 +1,64 @@ +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/rebuild.ts b/packages/amplify-category-api/src/commands/api/rebuild.ts index fe83efb3ba5..0779ed42b93 100644 --- a/packages/amplify-category-api/src/commands/api/rebuild.ts +++ b/packages/amplify-category-api/src/commands/api/rebuild.ts @@ -10,7 +10,7 @@ const rebuild = true; export const run = async (context: $TSContext) => { if (!FeatureFlags.getBoolean('graphqlTransformer.enableIterativeGSIUpdates')) { - printer.error('Iterative GSI Updates must be enabled to rebuild the API. See https://docs.amplify.aws/cli/reference/feature-flags/'); + 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 || {}) @@ -25,6 +25,7 @@ export const run = async (context: $TSContext) => { 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}.`); diff --git a/packages/amplify-e2e-core/src/categories/api.ts b/packages/amplify-e2e-core/src/categories/api.ts index 571e91efc48..392ad64a427 100644 --- a/packages/amplify-e2e-core/src/categories/api.ts +++ b/packages/amplify-e2e-core/src/categories/api.ts @@ -566,3 +566,13 @@ 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 6bf71b874fa..bf359bb3750 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,3 +254,18 @@ 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 e5e60113372..f5745d7e733 100644 --- a/packages/amplify-e2e-core/src/utils/sdk-calls.ts +++ b/packages/amplify-e2e-core/src/utils/sdk-calls.ts @@ -200,6 +200,16 @@ 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 new file mode 100644 index 00000000000..cdbe2826bfe --- /dev/null +++ b/packages/amplify-e2e-tests/schemas/simple_model_new_primary_key.graphql @@ -0,0 +1,4 @@ +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 new file mode 100644 index 00000000000..d644810bf89 --- /dev/null +++ b/packages/amplify-e2e-tests/src/__tests__/api_5.test.ts @@ -0,0 +1,60 @@ +import { + createNewProjectDir, + initJSProjectWithProfile, + addApiWithSchema, + amplifyPush, + deleteProject, + deleteProjectDir, + putItemInTable, + scanTable, + rebuildApi, + getProjectMeta, + updateApiSchema, + amplifyPushDestructiveApiUpdate, +} 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 + }); +}); 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 a2b337ae768..88328cdc344 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 @@ -238,9 +238,9 @@ export class GraphQLResourceManager { }); for (const gsiChange of tableWithGSIChanges) { + const changeSteps = getGSIDiffs(gsiChange.currentTable, gsiChange.nextTable); const stackName = gsiChange.stackName; const tableName = gsiChange.tableName; - const changeSteps = getGSIDiffs(gsiChange.currentTable, gsiChange.nextTable); for (const changeStep of changeSteps) { const ddbResource = this.templateState.getLatest(stackName) || this.getStack(stackName, currentState); let gsiRecord; From bafa54fa7282805d2d861867ebbe7b84dfd2e4d4 Mon Sep 17 00:00:00 2001 From: Edward Foyle Date: Wed, 8 Sep 2021 19:44:35 -0700 Subject: [PATCH 07/21] feat: sweet jesus it works --- .circleci/config.yml | 63 +++----- .../amplify-category-function/src/index.ts | 1 + .../disconnect-dependent-resources.test.ts | 52 ++++++ .../disconnect-dependent-resources.ts | 149 ++++++++++++++++++ .../amplify-graphql-resource-manager.ts | 38 +++-- .../src/graphql-transformer/utils.ts | 18 +-- .../src/push-resources.ts | 39 ++++- 7 files changed, 288 insertions(+), 72 deletions(-) create mode 100644 packages/amplify-provider-awscloudformation/src/__tests__/disconnect-dependent-resources/disconnect-dependent-resources.test.ts create mode 100644 packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/disconnect-dependent-resources.ts diff --git a/.circleci/config.yml b/.circleci/config.yml index 83b44b53e3f..1ba9f4700fc 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -1274,9 +1274,7 @@ jobs: steps: *ref_5 environment: TEST_SUITE: src/__tests__/auth_6.test.ts -<<<<<<< HEAD -======= - CLI_REGION: ap-southeast-1 + CLI_REGION: ap-southeast-2 api_5-amplify_e2e_tests: working_directory: ~/repo docker: *ref_1 @@ -1284,8 +1282,7 @@ jobs: steps: *ref_5 environment: TEST_SUITE: src/__tests__/api_5.test.ts ->>>>>>> test: adding unit and e2e tests - CLI_REGION: ap-southeast-2 + CLI_REGION: us-east-2 api_4-amplify_e2e_tests: working_directory: ~/repo docker: *ref_1 @@ -1293,7 +1290,7 @@ jobs: steps: *ref_5 environment: TEST_SUITE: src/__tests__/api_4.test.ts - CLI_REGION: us-east-2 + CLI_REGION: us-west-2 schema-iterative-update-4-amplify_e2e_tests_pkg_linux: working_directory: ~/repo docker: *ref_1 @@ -2072,7 +2069,7 @@ jobs: 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: ap-southeast-2 + CLI_REGION: us-east-2 steps: *ref_6 api_4-amplify_e2e_tests_pkg_linux: working_directory: ~/repo @@ -2082,7 +2079,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: us-east-2 + CLI_REGION: us-west-2 steps: *ref_6 workflows: version: 2 @@ -2188,16 +2185,12 @@ workflows: requires: - notifications-amplify_e2e_tests - schema-iterative-update-locking-amplify_e2e_tests -<<<<<<< HEAD - function_7-amplify_e2e_tests -======= - - function_6-amplify_e2e_tests ->>>>>>> test: adding unit and e2e tests - - api_4-amplify_e2e_tests - - hosting-amplify_e2e_tests + - api_5-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 @@ -2216,27 +2209,18 @@ workflows: - configure-project-amplify_e2e_tests - schema-versioned-amplify_e2e_tests - plugin-amplify_e2e_tests -<<<<<<< HEAD - hooks-amplify_e2e_tests - auth_6-amplify_e2e_tests -======= - - function_7-amplify_e2e_tests - - api_5-amplify_e2e_tests ->>>>>>> test: adding unit and e2e tests - done_with_pkg_linux_e2e_tests: requires: - notifications-amplify_e2e_tests_pkg_linux - schema-iterative-update-locking-amplify_e2e_tests_pkg_linux -<<<<<<< HEAD - function_7-amplify_e2e_tests_pkg_linux -======= - - function_6-amplify_e2e_tests_pkg_linux ->>>>>>> test: adding unit and e2e tests - - api_4-amplify_e2e_tests_pkg_linux - - hosting-amplify_e2e_tests_pkg_linux + - api_5-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 @@ -2255,13 +2239,8 @@ workflows: - configure-project-amplify_e2e_tests_pkg_linux - schema-versioned-amplify_e2e_tests_pkg_linux - plugin-amplify_e2e_tests_pkg_linux -<<<<<<< HEAD - hooks-amplify_e2e_tests_pkg_linux - auth_6-amplify_e2e_tests_pkg_linux -======= - - function_7-amplify_e2e_tests_pkg_linux - - api_5-amplify_e2e_tests_pkg_linux ->>>>>>> test: adding unit and e2e tests - amplify_migration_tests_latest: context: - amplify-ecr-image-pull @@ -2483,7 +2462,7 @@ workflows: filters: *ref_10 requires: - schema-key-amplify_e2e_tests - - api_4-amplify_e2e_tests: + - api_5-amplify_e2e_tests: context: *ref_8 post-steps: *ref_9 filters: *ref_10 @@ -2555,6 +2534,12 @@ workflows: filters: *ref_10 requires: - schema-auth-10-amplify_e2e_tests + - api_4-amplify_e2e_tests: + context: *ref_8 + post-steps: *ref_9 + filters: *ref_10 + requires: + - hosting-amplify_e2e_tests - storage-amplify_e2e_tests: context: *ref_8 post-steps: *ref_9 @@ -2879,11 +2864,7 @@ workflows: filters: *ref_10 requires: - auth_1-amplify_e2e_tests -<<<<<<< HEAD - auth_6-amplify_e2e_tests: -======= - - api_5-amplify_e2e_tests: ->>>>>>> test: adding unit and e2e tests context: *ref_8 post-steps: *ref_9 filters: *ref_10 @@ -2969,7 +2950,7 @@ workflows: filters: *ref_13 requires: - schema-key-amplify_e2e_tests_pkg_linux - - api_4-amplify_e2e_tests_pkg_linux: + - api_5-amplify_e2e_tests_pkg_linux: context: *ref_11 post-steps: *ref_12 filters: *ref_13 @@ -3045,6 +3026,12 @@ workflows: filters: *ref_13 requires: - schema-auth-10-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 - storage-amplify_e2e_tests_pkg_linux: context: *ref_11 post-steps: *ref_12 @@ -3389,11 +3376,7 @@ workflows: filters: *ref_13 requires: - auth_1-amplify_e2e_tests_pkg_linux -<<<<<<< HEAD - auth_6-amplify_e2e_tests_pkg_linux: -======= - - api_5-amplify_e2e_tests_pkg_linux: ->>>>>>> test: adding unit and e2e tests context: *ref_11 post-steps: *ref_12 filters: *ref_13 diff --git a/packages/amplify-category-function/src/index.ts b/packages/amplify-category-function/src/index.ts index 0d6de3a6976..63a6841431e 100644 --- a/packages/amplify-category-function/src/index.ts +++ b/packages/amplify-category-function/src/index.ts @@ -28,6 +28,7 @@ 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-provider-awscloudformation/src/__tests__/disconnect-dependent-resources/disconnect-dependent-resources.test.ts b/packages/amplify-provider-awscloudformation/src/__tests__/disconnect-dependent-resources/disconnect-dependent-resources.test.ts new file mode 100644 index 00000000000..6183c596863 --- /dev/null +++ b/packages/amplify-provider-awscloudformation/src/__tests__/disconnect-dependent-resources/disconnect-dependent-resources.test.ts @@ -0,0 +1,52 @@ +import { replaceFnImport } from '../../disconnect-dependent-resources/disconnect-dependent-resources'; + +it('test replacefnimport', () => { + const obj = { + Handler: 'index.handler', + FunctionName: { + 'Fn::If': [ + 'ShouldNotCreateEnvResources', + 'uploadtestfd07b864', + { + 'Fn::Join': [ + '', + [ + 'uploadtestfd07b864', + '-', + { + Ref: 'env', + }, + ], + ], + }, + ], + }, + Environment: { + Variables: { + ENV: { Ref: 'env' }, + REGION: { Ref: 'AWS::Region' }, + API_UPLOADTEST_TODOTABLE_NAME: { 'Fn::ImportValue': { 'Fn::Sub': '${apiuploadtestGraphQLAPIIdOutput}:GetAtt:TodoTable:Name' } }, + API_UPLOADTEST_TODOTABLE_ARN: { + 'Fn::Join': [ + '', + [ + 'arn:aws:dynamodb:', + { Ref: 'AWS::Region' }, + ':', + { Ref: 'AWS::AccountId' }, + ':table/', + { 'Fn::ImportValue': { 'Fn::Sub': '${apiuploadtestGraphQLAPIIdOutput}:GetAtt:TodoTable:Name' } }, + ], + ], + }, + API_UPLOADTEST_GRAPHQLAPIIDOUTPUT: { Ref: 'apiuploadtestGraphQLAPIIdOutput' }, + }, + }, + Role: { 'Fn::GetAtt': ['LambdaExecutionRole', 'Arn'] }, + Runtime: 'nodejs14.x', + Layers: [], + Timeout: 25, + }; + replaceFnImport(obj); + console.log(JSON.stringify(obj, undefined, 2)); +}); diff --git a/packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/disconnect-dependent-resources.ts b/packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/disconnect-dependent-resources.ts new file mode 100644 index 00000000000..81f64c38af0 --- /dev/null +++ b/packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/disconnect-dependent-resources.ts @@ -0,0 +1,149 @@ +import { $TSAny, $TSContext, JSONUtilities, pathManager, readCFNTemplate, 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; +}; + +export const generateTempTemplates = 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); + } +}; + +export const uploadTempFuncTemplates = 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[]) => { + 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 steps; +}; + +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 deploymentStep: DeploymentOp = { + stackTemplatePathOrUrl: getTempFuncTemplateS3Key(functionName), + parameters, + stackName: funcStackId, + capabilities, + tableNames: [], + }; + + JSONUtilities.writeJson(getTempFuncMetaLocalPath(functionName), deploymentStep); + return deploymentStep; +}; + +export const prependDeploymentSteps = (beforeSteps: DeploymentStep[], afterSteps: DeploymentStep[]) => { + if (beforeSteps.length === 0) { + return afterSteps; + } + beforeSteps[0].rollback = _.cloneDeep(afterSteps[0].rollback); + afterSteps[0].rollback = _.cloneDeep(beforeSteps[beforeSteps.length - 1].deployment); + return beforeSteps.concat(afterSteps); +}; + +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`; +const s3Prefix = 'amplify-cfn-templates/function/temp'; +const localPrefix = funcName => path.join(pathManager.getResourceDirectoryPath(undefined, 'function', funcName), 'temp'); + +export 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)); +}; + +export const getFunctionParamsSupplier = (context: $TSContext) => async (functionName: string) => { + return context.amplify.invokePluginMethod(context, 'function', undefined, 'loadFunctionParameters', [ + pathManager.getResourceDirectoryPath(undefined, 'function', functionName), + ]) as $TSAny; +}; + +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 88328cdc344..e4d5077e5c7 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 @@ -107,7 +107,7 @@ export class GraphQLResourceManager { } } this.gsiManagement(gqlDiff.diff, gqlDiff.current, gqlDiff.next); - this.tableRecreationManagement(gqlDiff.diff, gqlDiff.current, gqlDiff.next); + this.tableRecreationManagement(gqlDiff.current, gqlDiff.next); return await this.getDeploymentSteps(); }; @@ -271,7 +271,20 @@ export class GraphQLResourceManager { } }; - private tableRecreationManagement = (diffs: DiffChanges, currentState: DiffableProject, nextState: DiffableProject) => { + private tableRecreationManagement = (currentState: DiffableProject, nextState: DiffableProject) => { + this.getTablesToRecreate().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))); + }); + }; + + getTablesToRecreate = () => { + const gqlDiff = getGQLDiff(this.backendApiProjectRoot, this.cloudBackendApiProjectRoot); + const [diffs, currentState] = [gqlDiff.diff, gqlDiff.current]; const getTablesRequiringReplacement = () => _.uniq( diffs @@ -281,7 +294,7 @@ export class GraphQLResourceManager { 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) @@ -290,17 +303,7 @@ export class GraphQLResourceManager { stackName: path.basename(name, '.json'), })) .filter(meta => !!meta.tableName); - - const tablesToRecreate = this.rebuildAllTables ? getAllTables() : getTablesRequiringReplacement(); - - tablesToRecreate.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))); - }); + return this.rebuildAllTables ? getAllTables() : getTablesRequiringReplacement(); }; private getTable = (gsiChange: Diff, proj: DiffableProject): DynamoDB.Table => { @@ -322,8 +325,15 @@ export class GraphQLResourceManager { }; 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)); + + // rename table and don't remove refs + // template.Resources[tableName].Properties.TableName = undefined; + + // // set output refs to placeholder value + // Object.entries(template.Outputs || {}).filter(([key]) => key.includes(tableName)).forEach(([key]) => template.Outputs[key].Value = 'placeholderDuringReplacement') }; private clearTemplateState = (stackName: string) => { diff --git a/packages/amplify-provider-awscloudformation/src/graphql-transformer/utils.ts b/packages/amplify-provider-awscloudformation/src/graphql-transformer/utils.ts index c849a903223..844a2b3c574 100644 --- a/packages/amplify-provider-awscloudformation/src/graphql-transformer/utils.ts +++ b/packages/amplify-provider-awscloudformation/src/graphql-transformer/utils.ts @@ -34,20 +34,10 @@ export const getGQLDiff = (currentBackendDir: string, cloudBackendDir: string): return 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 const getGqlUpdatedResource = (resources: any[]) => + resources.find( + resource => resource?.service === 'AppSync' && resource?.providerMetadata.logicalId && resource.providerPlugin === 'awscloudformation', + ) || null; export function loadDiffableProject(path: string, rootStackName: string): DiffableProject { const project = readFromPath(path); diff --git a/packages/amplify-provider-awscloudformation/src/push-resources.ts b/packages/amplify-provider-awscloudformation/src/push-resources.ts index 347b57f58d0..e2701781549 100644 --- a/packages/amplify-provider-awscloudformation/src/push-resources.ts +++ b/packages/amplify-provider-awscloudformation/src/push-resources.ts @@ -47,6 +47,16 @@ 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 { + generateIterativeFuncDeploymentSteps, + generateTempTemplates, + getDependentFunctions, + getFunctionParamsSupplier, + prependDeploymentSteps, + uploadTempFuncTemplates, +} from './disconnect-dependent-resources/disconnect-dependent-resources'; +import { CloudFormation } from 'aws-sdk'; +import { loadConfiguration } from './configuration-manager'; const logger = fileLogger('push-resources'); @@ -109,7 +119,7 @@ export async function run(context: $TSContext, resourceDefinition: $TSObject, re } validateCfnTemplates(context, resources); - for await (const resource of resources) { + for (const resource of resources) { if (resource.service === ApiServiceNameElasticContainer && resource.category === 'api') { const { exposedContainer, @@ -136,9 +146,7 @@ export async function run(context: $TSContext, resourceDefinition: $TSObject, re } } - for await (const resource of resources.filter( - r => r.category === FunctionCategoryName && r.service === FunctionServiceNameLambdaLayer, - )) { + for (const resource of resources.filter(r => r.category === FunctionCategoryName && r.service === FunctionServiceNameLambdaLayer)) { await legacyLayerMigration(context, resource.resourceName); } @@ -158,6 +166,7 @@ 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 } = {}; @@ -169,6 +178,25 @@ export async function run(context: $TSContext, resourceDefinition: $TSObject, re if (gqlResource) { const gqlManager = await GraphQLResourceManager.createInstance(context, gqlResource, cloudformationMeta.StackId, rebuild); deploymentSteps = await gqlManager.run(); + + // if any models are being replaced, we need to determine if any functions have a dependency on the replaced models + // if so, we prepend steps to the iterative deployment to remove the references to the replaced table + // then the last step of the iterative deployment adds the references back + const modelsToReplace = gqlManager.getTablesToRecreate().map(meta => meta.stackName); // stackName is the same as the model name + const allFunctionNames = allResources + .filter(meta => meta.category === 'function' && meta.service === 'Lambda') + .map(meta => meta.resourceName); + functionsDependentOnReplacedModelTables = await getDependentFunctions( + modelsToReplace, + allFunctionNames, + getFunctionParamsSupplier(context), + ); + const decoupleFuncSteps = await generateIterativeFuncDeploymentSteps( + new CloudFormation(await loadConfiguration(context)), + cloudformationMeta.StackId, + functionsDependentOnReplacedModelTables, + ); + deploymentSteps = prependDeploymentSteps(decoupleFuncSteps, deploymentSteps); if (deploymentSteps.length > 1) { iterativeDeploymentWasInvoked = true; @@ -195,6 +223,8 @@ export async function run(context: $TSContext, resourceDefinition: $TSObject, re await prePushAuthTransform(context, resources); await prePushGraphQLCodegen(context, resourcesToBeCreated, resourcesToBeUpdated); const projectDetails = context.amplify.getProjectDetails(); + await generateTempTemplates(functionsDependentOnReplacedModelTables); + await uploadTempFuncTemplates(await S3.getInstance(context), functionsDependentOnReplacedModelTables); await updateS3Templates(context, resources, projectDetails.amplifyMeta); // We do not need CloudFormation update if only syncable resources are the changes. @@ -221,6 +251,7 @@ export async function run(context: $TSContext, resourceDefinition: $TSObject, re await generateAndUploadRootStack(context, nestedStackFilepath, nestedStackFileName); // Use state manager to do the final deployment. The final deployment include not just API change but the whole Amplify Project + // TODO check that this step will also reinstate the original function template const finalStep: DeploymentOp = { stackTemplatePathOrUrl: nestedStackFileName, tableNames: [], From ef0524c94832c4289c428fd5e13df97bb08f16b9 Mon Sep 17 00:00:00 2001 From: Edward Foyle Date: Thu, 9 Sep 2021 13:17:27 -0700 Subject: [PATCH 08/21] fix: iterative push lambda with updates --- .../disconnect-dependent-resources.ts | 24 +++++++++++++++---- .../src/graphql-transformer/utils.ts | 3 ++- .../src/push-resources.ts | 6 ++--- 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/disconnect-dependent-resources.ts b/packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/disconnect-dependent-resources.ts index 81f64c38af0..684bbbb8a14 100644 --- a/packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/disconnect-dependent-resources.ts +++ b/packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/disconnect-dependent-resources.ts @@ -1,4 +1,4 @@ -import { $TSAny, $TSContext, JSONUtilities, pathManager, readCFNTemplate, writeCFNTemplate } from 'amplify-cli-core'; +import { $TSAny, $TSContext, 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'; @@ -67,7 +67,11 @@ export const uploadTempFuncTemplates = async (s3Client: S3, funcNames: string[]) } }; -export const generateIterativeFuncDeploymentSteps = async (cfnClient: CloudFormation, rootStackId: string, functionNames: string[]) => { +export const generateIterativeFuncDeploymentSteps = async ( + cfnClient: CloudFormation, + rootStackId: string, + functionNames: string[], +): Promise<{ deploymentSteps: DeploymentStep[]; lastMetaKey: string }> => { let rollback: DeploymentOp; let previousMetaKey: string; const steps: DeploymentStep[] = []; @@ -81,7 +85,7 @@ export const generateIterativeFuncDeploymentSteps = async (cfnClient: CloudForma rollback = deploymentOp; previousMetaKey = getTempFuncMetaS3Key(funcName); } - return steps; + return { deploymentSteps: steps, lastMetaKey: previousMetaKey }; }; const generateIterativeFuncDeploymentOp = async (cfnClient: CloudFormation, rootStackId: string, functionName: string) => { @@ -90,9 +94,17 @@ const generateIterativeFuncDeploymentOp = async (cfnClient: CloudFormation, root .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, + parameters: params, stackName: funcStackId, capabilities, tableNames: [], @@ -102,12 +114,14 @@ const generateIterativeFuncDeploymentOp = async (cfnClient: CloudFormation, root return deploymentStep; }; -export const prependDeploymentSteps = (beforeSteps: DeploymentStep[], afterSteps: DeploymentStep[]) => { +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; return beforeSteps.concat(afterSteps); }; diff --git a/packages/amplify-provider-awscloudformation/src/graphql-transformer/utils.ts b/packages/amplify-provider-awscloudformation/src/graphql-transformer/utils.ts index 844a2b3c574..fbbac527037 100644 --- a/packages/amplify-provider-awscloudformation/src/graphql-transformer/utils.ts +++ b/packages/amplify-provider-awscloudformation/src/graphql-transformer/utils.ts @@ -36,7 +36,8 @@ export const getGQLDiff = (currentBackendDir: string, cloudBackendDir: string): export const getGqlUpdatedResource = (resources: any[]) => resources.find( - resource => resource?.service === 'AppSync' && resource?.providerMetadata.logicalId && resource.providerPlugin === 'awscloudformation', + resource => + resource?.service === 'AppSync' && resource?.providerMetadata?.logicalId && resource?.providerPlugin === 'awscloudformation', ) || null; export function loadDiffableProject(path: string, rootStackName: string): DiffableProject { diff --git a/packages/amplify-provider-awscloudformation/src/push-resources.ts b/packages/amplify-provider-awscloudformation/src/push-resources.ts index e2701781549..15fe0593329 100644 --- a/packages/amplify-provider-awscloudformation/src/push-resources.ts +++ b/packages/amplify-provider-awscloudformation/src/push-resources.ts @@ -173,7 +173,7 @@ export async function run(context: $TSContext, resourceDefinition: $TSObject, re // Check if iterative updates are enabled or not and generate the required deployment steps if needed. if (FeatureFlags.getBoolean('graphQLTransformer.enableIterativeGSIUpdates')) { - const gqlResource = getGqlUpdatedResource(resources); + const gqlResource = getGqlUpdatedResource(rebuild ? resources : resourcesToBeUpdated); if (gqlResource) { const gqlManager = await GraphQLResourceManager.createInstance(context, gqlResource, cloudformationMeta.StackId, rebuild); @@ -191,12 +191,12 @@ export async function run(context: $TSContext, resourceDefinition: $TSObject, re allFunctionNames, getFunctionParamsSupplier(context), ); - const decoupleFuncSteps = await generateIterativeFuncDeploymentSteps( + const { deploymentSteps: decoupleFuncSteps, lastMetaKey } = await generateIterativeFuncDeploymentSteps( new CloudFormation(await loadConfiguration(context)), cloudformationMeta.StackId, functionsDependentOnReplacedModelTables, ); - deploymentSteps = prependDeploymentSteps(decoupleFuncSteps, deploymentSteps); + deploymentSteps = prependDeploymentSteps(decoupleFuncSteps, deploymentSteps, lastMetaKey); if (deploymentSteps.length > 1) { iterativeDeploymentWasInvoked = true; From bfb6b79b5cec1735cfd646ecd04b20210509c9a3 Mon Sep 17 00:00:00 2001 From: Edward Foyle Date: Thu, 9 Sep 2021 14:02:35 -0700 Subject: [PATCH 09/21] chore: organize code better --- .../disconnect-dependent-resources/index.ts | 42 +++++++++++++ ...onnect-dependent-resources.ts => utils.ts} | 61 ++++++++++++------- .../amplify-graphql-resource-manager.ts | 4 +- .../src/push-resources.ts | 36 +++-------- 4 files changed, 90 insertions(+), 53 deletions(-) create mode 100644 packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/index.ts rename packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/{disconnect-dependent-resources.ts => utils.ts} (82%) diff --git a/packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/index.ts b/packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/index.ts new file mode 100644 index 00000000000..e000b726a09 --- /dev/null +++ b/packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/index.ts @@ -0,0 +1,42 @@ +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, +} from './utils'; + +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); + const 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); +}; + +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/disconnect-dependent-resources.ts b/packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/utils.ts similarity index 82% rename from packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/disconnect-dependent-resources.ts rename to packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/utils.ts index 684bbbb8a14..6d37015503d 100644 --- a/packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/disconnect-dependent-resources.ts +++ b/packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/utils.ts @@ -1,4 +1,4 @@ -import { $TSAny, $TSContext, JSONUtilities, pathManager, readCFNTemplate, stateManager, writeCFNTemplate } from 'amplify-cli-core'; +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'; @@ -30,7 +30,10 @@ export const getDependentFunctions = async ( return dependentFunctions; }; -export const generateTempTemplates = async (dependentFunctions: string[]) => { +/** + * 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( @@ -43,7 +46,10 @@ export const generateTempTemplates = async (dependentFunctions: string[]) => { } }; -export const uploadTempFuncTemplates = async (s3Client: S3, funcNames: string[]) => { +/** + * 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 = [ { @@ -88,6 +94,25 @@ export const generateIterativeFuncDeploymentSteps = async ( 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; + 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}` }) @@ -114,17 +139,7 @@ const generateIterativeFuncDeploymentOp = async (cfnClient: CloudFormation, root return deploymentStep; }; -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; - return beforeSteps.concat(afterSteps); -}; - +// 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)); @@ -135,7 +150,12 @@ const tempMetaFilename = (funcName: string) => `temp-${funcName}-deployment-meta const s3Prefix = 'amplify-cfn-templates/function/temp'; const localPrefix = funcName => path.join(pathManager.getResourceDirectoryPath(undefined, 'function', funcName), 'temp'); -export const replaceFnImport = (node: $TSAny) => { +/** + * 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; } @@ -145,18 +165,15 @@ export const replaceFnImport = (node: $TSAny) => { const nodeKeys = Object.keys(node); if (nodeKeys.length === 1 && nodeKeys[0] === 'Fn::ImportValue') { node['Fn::ImportValue'] = undefined; - node['Fn::Sub'] = 'temporaryPlaceholdervalue'; + node['Fn::Sub'] = 'TemporaryPlaceholderValue'; return; } Object.values(node).forEach(value => replaceFnImport(value)); }; -export const getFunctionParamsSupplier = (context: $TSContext) => async (functionName: string) => { - return context.amplify.invokePluginMethod(context, 'function', undefined, 'loadFunctionParameters', [ - pathManager.getResourceDirectoryPath(undefined, 'function', functionName), - ]) as $TSAny; -}; - +/** + * 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)')) 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 e4d5077e5c7..f84cf963939 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 @@ -272,7 +272,7 @@ export class GraphQLResourceManager { }; private tableRecreationManagement = (currentState: DiffableProject, nextState: DiffableProject) => { - this.getTablesToRecreate().forEach(tableMeta => { + 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 @@ -282,7 +282,7 @@ export class GraphQLResourceManager { }); }; - getTablesToRecreate = () => { + getTablesBeingReplaced = () => { const gqlDiff = getGQLDiff(this.backendApiProjectRoot, this.cloudBackendApiProjectRoot); const [diffs, currentState] = [gqlDiff.diff, gqlDiff.current]; const getTablesRequiringReplacement = () => diff --git a/packages/amplify-provider-awscloudformation/src/push-resources.ts b/packages/amplify-provider-awscloudformation/src/push-resources.ts index 15fe0593329..7c13d311b89 100644 --- a/packages/amplify-provider-awscloudformation/src/push-resources.ts +++ b/packages/amplify-provider-awscloudformation/src/push-resources.ts @@ -47,16 +47,7 @@ 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 { - generateIterativeFuncDeploymentSteps, - generateTempTemplates, - getDependentFunctions, - getFunctionParamsSupplier, - prependDeploymentSteps, - uploadTempFuncTemplates, -} from './disconnect-dependent-resources/disconnect-dependent-resources'; -import { CloudFormation } from 'aws-sdk'; -import { loadConfiguration } from './configuration-manager'; +import { prependDeploymentStepsToDisconnectFunctionsFromReplacedModelTables } from './disconnect-dependent-resources'; const logger = fileLogger('push-resources'); @@ -179,24 +170,13 @@ export async function run(context: $TSContext, resourceDefinition: $TSObject, re const gqlManager = await GraphQLResourceManager.createInstance(context, gqlResource, cloudformationMeta.StackId, rebuild); deploymentSteps = await gqlManager.run(); - // if any models are being replaced, we need to determine if any functions have a dependency on the replaced models - // if so, we prepend steps to the iterative deployment to remove the references to the replaced table - // then the last step of the iterative deployment adds the references back - const modelsToReplace = gqlManager.getTablesToRecreate().map(meta => meta.stackName); // stackName is the same as the model name - const allFunctionNames = allResources - .filter(meta => meta.category === 'function' && meta.service === 'Lambda') - .map(meta => meta.resourceName); - functionsDependentOnReplacedModelTables = await getDependentFunctions( - modelsToReplace, - allFunctionNames, - getFunctionParamsSupplier(context), - ); - const { deploymentSteps: decoupleFuncSteps, lastMetaKey } = await generateIterativeFuncDeploymentSteps( - new CloudFormation(await loadConfiguration(context)), - cloudformationMeta.StackId, - functionsDependentOnReplacedModelTables, + // 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, ); - deploymentSteps = prependDeploymentSteps(decoupleFuncSteps, deploymentSteps, lastMetaKey); if (deploymentSteps.length > 1) { iterativeDeploymentWasInvoked = true; @@ -223,8 +203,6 @@ export async function run(context: $TSContext, resourceDefinition: $TSObject, re await prePushAuthTransform(context, resources); await prePushGraphQLCodegen(context, resourcesToBeCreated, resourcesToBeUpdated); const projectDetails = context.amplify.getProjectDetails(); - await generateTempTemplates(functionsDependentOnReplacedModelTables); - await uploadTempFuncTemplates(await S3.getInstance(context), functionsDependentOnReplacedModelTables); await updateS3Templates(context, resources, projectDetails.amplifyMeta); // We do not need CloudFormation update if only syncable resources are the changes. From a40159487dba6ea686b1ad9b96f5d2296a9675f0 Mon Sep 17 00:00:00 2001 From: Edward Foyle Date: Thu, 9 Sep 2021 16:20:49 -0700 Subject: [PATCH 10/21] test: add unit and e2e tests --- .../src/__tests__/api_5.test.ts | 23 ++ .../__snapshots__/utils.test.ts.snap | 114 +++++++++ .../disconnect-dependent-resources.test.ts | 52 ---- .../utils.test.ts | 239 ++++++++++++++++++ .../disconnect-dependent-resources/utils.ts | 3 + 5 files changed, 379 insertions(+), 52 deletions(-) create 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/disconnect-dependent-resources.test.ts create mode 100644 packages/amplify-provider-awscloudformation/src/__tests__/disconnect-dependent-resources/utils.test.ts diff --git a/packages/amplify-e2e-tests/src/__tests__/api_5.test.ts b/packages/amplify-e2e-tests/src/__tests__/api_5.test.ts index d644810bf89..1cd1cffd0e0 100644 --- a/packages/amplify-e2e-tests/src/__tests__/api_5.test.ts +++ b/packages/amplify-e2e-tests/src/__tests__/api_5.test.ts @@ -11,6 +11,7 @@ import { getProjectMeta, updateApiSchema, amplifyPushDestructiveApiUpdate, + addFunction, } from 'amplify-e2e-core'; const projName = 'apitest'; @@ -57,4 +58,26 @@ describe('destructive updates flag', () => { 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: ['apitest'], + operations: ['create', 'read', 'update', 'delete'], + }, + }, + 'nodejs', + ); + await amplifyPush(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-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 new file mode 100644 index 00000000000..6ae1c362769 --- /dev/null +++ b/packages/amplify-provider-awscloudformation/src/__tests__/disconnect-dependent-resources/__snapshots__/utils.test.ts.snap @@ -0,0 +1,114 @@ +// 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/disconnect-dependent-resources.test.ts b/packages/amplify-provider-awscloudformation/src/__tests__/disconnect-dependent-resources/disconnect-dependent-resources.test.ts deleted file mode 100644 index 6183c596863..00000000000 --- a/packages/amplify-provider-awscloudformation/src/__tests__/disconnect-dependent-resources/disconnect-dependent-resources.test.ts +++ /dev/null @@ -1,52 +0,0 @@ -import { replaceFnImport } from '../../disconnect-dependent-resources/disconnect-dependent-resources'; - -it('test replacefnimport', () => { - const obj = { - Handler: 'index.handler', - FunctionName: { - 'Fn::If': [ - 'ShouldNotCreateEnvResources', - 'uploadtestfd07b864', - { - 'Fn::Join': [ - '', - [ - 'uploadtestfd07b864', - '-', - { - Ref: 'env', - }, - ], - ], - }, - ], - }, - Environment: { - Variables: { - ENV: { Ref: 'env' }, - REGION: { Ref: 'AWS::Region' }, - API_UPLOADTEST_TODOTABLE_NAME: { 'Fn::ImportValue': { 'Fn::Sub': '${apiuploadtestGraphQLAPIIdOutput}:GetAtt:TodoTable:Name' } }, - API_UPLOADTEST_TODOTABLE_ARN: { - 'Fn::Join': [ - '', - [ - 'arn:aws:dynamodb:', - { Ref: 'AWS::Region' }, - ':', - { Ref: 'AWS::AccountId' }, - ':table/', - { 'Fn::ImportValue': { 'Fn::Sub': '${apiuploadtestGraphQLAPIIdOutput}:GetAtt:TodoTable:Name' } }, - ], - ], - }, - API_UPLOADTEST_GRAPHQLAPIIDOUTPUT: { Ref: 'apiuploadtestGraphQLAPIIdOutput' }, - }, - }, - Role: { 'Fn::GetAtt': ['LambdaExecutionRole', 'Arn'] }, - Runtime: 'nodejs14.x', - Layers: [], - Timeout: 25, - }; - replaceFnImport(obj); - console.log(JSON.stringify(obj, undefined, 2)); -}); 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 new file mode 100644 index 00000000000..6a0a42fd3c0 --- /dev/null +++ b/packages/amplify-provider-awscloudformation/src/__tests__/disconnect-dependent-resources/utils.test.ts @@ -0,0 +1,239 @@ +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', () => { + // const log_mock = jest.fn(); + // const logger_mock = jest.fn().mockReturnValue(log_mock); + // fileLogger_mock.mockReturnValue(() => () => jest.fn()); + 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/disconnect-dependent-resources/utils.ts b/packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/utils.ts index 6d37015503d..bafcf6f5602 100644 --- a/packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/utils.ts +++ b/packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/utils.ts @@ -106,6 +106,9 @@ export const prependDeploymentSteps = (beforeSteps: DeploymentStep[], afterSteps 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); }; From 17f184d95d28f17d36d2d1ab5d1577f3c06b4b91 Mon Sep 17 00:00:00 2001 From: Edward Foyle Date: Thu, 9 Sep 2021 17:09:18 -0700 Subject: [PATCH 11/21] chore: fit and finish --- .../src/commands/api/rebuild.ts | 4 +- .../src/__tests__/api_5.test.ts | 5 +- packages/amplify-prompts/src/validators.ts | 73 ++++++++++++------- .../disconnect-dependent-resources/index.ts | 24 +++++- .../amplify-graphql-resource-manager.ts | 6 -- .../src/push-resources.ts | 8 +- 6 files changed, 80 insertions(+), 40 deletions(-) diff --git a/packages/amplify-category-api/src/commands/api/rebuild.ts b/packages/amplify-category-api/src/commands/api/rebuild.ts index 0779ed42b93..2173ba77c49 100644 --- a/packages/amplify-category-api/src/commands/api/rebuild.ts +++ b/packages/amplify-category-api/src/commands/api/rebuild.ts @@ -1,5 +1,5 @@ import { $TSContext, FeatureFlags, stateManager } from 'amplify-cli-core'; -import { printer, prompter } from 'amplify-prompts'; +import { printer, prompter, exact } from 'amplify-prompts'; const subcommand = 'rebuild'; const category = 'api'; @@ -31,7 +31,7 @@ export const run = async (context: $TSContext) => { 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: input => (input === apiName ? true : 'Input does not match API name'), + validate: exact(apiName, 'Input does not match the GraphQL API name'), }); const { amplify, parameters } = context; const resourceName = parameters.first; diff --git a/packages/amplify-e2e-tests/src/__tests__/api_5.test.ts b/packages/amplify-e2e-tests/src/__tests__/api_5.test.ts index 1cd1cffd0e0..dda41cf1685 100644 --- a/packages/amplify-e2e-tests/src/__tests__/api_5.test.ts +++ b/packages/amplify-e2e-tests/src/__tests__/api_5.test.ts @@ -12,6 +12,7 @@ import { updateApiSchema, amplifyPushDestructiveApiUpdate, addFunction, + amplifyPushAuth, } from 'amplify-e2e-core'; const projName = 'apitest'; @@ -69,13 +70,13 @@ describe('destructive updates flag', () => { additionalPermissions: { permissions: ['storage'], choices: ['api', 'storage'], - resources: ['apitest'], + resources: ['Todo:@model(appsync)'], operations: ['create', 'read', 'update', 'delete'], }, }, 'nodejs', ); - await amplifyPush(projRoot); + 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 d917ca606c9..9651f4125dc 100644 --- a/packages/amplify-prompts/src/validators.ts +++ b/packages/amplify-prompts/src/validators.ts @@ -12,50 +12,69 @@ 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 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'; /** * 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/disconnect-dependent-resources/index.ts b/packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/index.ts index e000b726a09..6fdae96a7f9 100644 --- a/packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/index.ts +++ b/packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/index.ts @@ -9,8 +9,21 @@ import { 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[], @@ -19,7 +32,7 @@ export const prependDeploymentStepsToDisconnectFunctionsFromReplacedModelTables const amplifyMeta = stateManager.getMeta(); const rootStackId = amplifyMeta?.providers?.awscloudformation?.StackId; const allFunctionNames = Object.keys(amplifyMeta?.function); - const functionsDependentOnReplacedModelTables = await getDependentFunctions( + functionsDependentOnReplacedModelTables = await getDependentFunctions( modelsBeingReplaced, allFunctionNames, getFunctionParamsSupplier(context), @@ -35,6 +48,15 @@ export const prependDeploymentStepsToDisconnectFunctionsFromReplacedModelTables 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), 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 f84cf963939..4de4b2a2523 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 @@ -328,12 +328,6 @@ export class GraphQLResourceManager { // remove table and all output refs to it template.Resources[tableName] = undefined; template.Outputs = _.omitBy(template.Outputs, (_, key) => key.includes(tableName)); - - // rename table and don't remove refs - // template.Resources[tableName].Properties.TableName = undefined; - - // // set output refs to placeholder value - // Object.entries(template.Outputs || {}).filter(([key]) => key.includes(tableName)).forEach(([key]) => template.Outputs[key].Value = 'placeholderDuringReplacement') }; private clearTemplateState = (stackName: string) => { diff --git a/packages/amplify-provider-awscloudformation/src/push-resources.ts b/packages/amplify-provider-awscloudformation/src/push-resources.ts index 7c13d311b89..25b1cdf0360 100644 --- a/packages/amplify-provider-awscloudformation/src/push-resources.ts +++ b/packages/amplify-provider-awscloudformation/src/push-resources.ts @@ -47,7 +47,10 @@ 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 { prependDeploymentStepsToDisconnectFunctionsFromReplacedModelTables } from './disconnect-dependent-resources'; +import { + postDeploymentCleanup, + prependDeploymentStepsToDisconnectFunctionsFromReplacedModelTables, +} from './disconnect-dependent-resources'; const logger = fileLogger('push-resources'); @@ -258,10 +261,11 @@ 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(); From 673d0baf6881f790fcdd81e1fe4f618d9969ccf4 Mon Sep 17 00:00:00 2001 From: Edward Foyle Date: Fri, 10 Sep 2021 11:42:12 -0700 Subject: [PATCH 12/21] chore: didn't save all the files --- .../src/disconnect-dependent-resources/utils.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/utils.ts b/packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/utils.ts index bafcf6f5602..e91ede4168f 100644 --- a/packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/utils.ts +++ b/packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/utils.ts @@ -150,8 +150,8 @@ const getTempFuncMetaS3Key = (funcName: string): string => path.posix.join(s3Pre const tempTemplateFilename = (funcName: string) => `temp-${funcName}-cloudformation-template.json`; const tempMetaFilename = (funcName: string) => `temp-${funcName}-deployment-meta.json`; -const s3Prefix = 'amplify-cfn-templates/function/temp'; -const localPrefix = funcName => path.join(pathManager.getResourceDirectoryPath(undefined, 'function', funcName), 'temp'); +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 From 79edbd5075911a643e0b2aa54802cc10ed727fde Mon Sep 17 00:00:00 2001 From: Edward Foyle Date: Fri, 10 Sep 2021 13:07:21 -0700 Subject: [PATCH 13/21] test: fix some tests and add a couple more --- .../__snapshots__/amplifyUtils.test.ts.snap | 28 +++ .../src/__tests__/util/amplifyUtils.test.ts | 20 ++ .../src/util/amplifyUtils.ts | 24 +- .../src/util/sanity-check.ts | 216 +++++++++--------- 4 files changed, 170 insertions(+), 118 deletions(-) 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 e88c9b1aabf..3ffd983d87e 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,11 +1,38 @@ // 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", ] `; @@ -23,6 +50,7 @@ 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 7d4034acbc3..40f681ea908 100644 --- a/packages/graphql-transformer-core/src/__tests__/util/amplifyUtils.test.ts +++ b/packages/graphql-transformer-core/src/__tests__/util/amplifyUtils.test.ts @@ -32,4 +32,24 @@ 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/util/amplifyUtils.ts b/packages/graphql-transformer-core/src/util/amplifyUtils.ts index f9221fa62d7..436dfb6177f 100644 --- a/packages/graphql-transformer-core/src/util/amplifyUtils.ts +++ b/packages/graphql-transformer-core/src/util/amplifyUtils.ts @@ -10,11 +10,11 @@ import { writeConfig, TransformConfig, TransformMigrationConfig, loadProject, re import { FeatureFlagProvider } from '../FeatureFlags'; import { cantAddAndRemoveGSIAtSameTimeRule, - cantAddLSILaterRule, - cantRemoveLSILater, + getCantAddLSILaterRule, + getCantRemoveLSILater, cantEditGSIKeySchemaRule, - cantEditKeySchemaRule, - cantEditLSIKeySchemaRule, + getCantEditKeySchemaRule, + getCantEditLSIKeySchemaRule, cantHaveMoreThan500ResourcesRule, DiffRule, sanityCheckProject, @@ -739,12 +739,12 @@ export function getSanityCheckRules(isNewAppSyncAPI: boolean, ff: FeatureFlagPro if (!allowDestructiveUpdates) { diffRules.push( // primary key rule - cantEditKeySchemaRule(iterativeUpdatesEnabled), + getCantEditKeySchemaRule(iterativeUpdatesEnabled), // LSI rules - cantAddLSILaterRule(iterativeUpdatesEnabled), - cantRemoveLSILater(iterativeUpdatesEnabled), - cantEditLSIKeySchemaRule(iterativeUpdatesEnabled), + getCantAddLSILaterRule(iterativeUpdatesEnabled), + getCantRemoveLSILater(iterativeUpdatesEnabled), + getCantEditLSIKeySchemaRule(iterativeUpdatesEnabled), // remove table rules cantRemoveTableAfterCreation, @@ -756,12 +756,12 @@ export function getSanityCheckRules(isNewAppSyncAPI: boolean, ff: FeatureFlagPro } else { diffRules.push( // primary key rule - cantEditKeySchemaRule(), + getCantEditKeySchemaRule(), // LSI rules - cantAddLSILaterRule(), - cantRemoveLSILater(), - cantEditLSIKeySchemaRule(), + getCantAddLSILaterRule(), + getCantRemoveLSILater(), + getCantEditLSIKeySchemaRule(), // GSI rules cantEditGSIKeySchemaRule, diff --git a/packages/graphql-transformer-core/src/util/sanity-check.ts b/packages/graphql-transformer-core/src/util/sanity-check.ts index 2a44c75d617..1e2312c00ea 100644 --- a/packages/graphql-transformer-core/src/util/sanity-check.ts +++ b/packages/graphql-transformer-core/src/util/sanity-check.ts @@ -73,26 +73,29 @@ export const sanityCheckDiffs = ( * @param currentBuild The last deployed build. * @param nextBuild The next build. */ -export const cantEditKeySchemaRule = (iterativeUpdatesEnabled: boolean = false) => (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]; +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], + if (iterativeUpdatesEnabled) { + throw new DestructiveMigrationError( + 'Editing the primary key of a model requires replacement of the underlying DynamoDB table.', + [], + [tableName], + ); + } + + 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"]).', ); } - - 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; }; /** @@ -102,32 +105,35 @@ export const cantEditKeySchemaRule = (iterativeUpdatesEnabled: boolean = false) * @param currentBuild The last deployed build. * @param nextBuild The next build. */ -export const cantAddLSILaterRule = (iterativeUpdatesEnabled: boolean = false) => (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]; +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], + ); + } - if (iterativeUpdatesEnabled) { - throw new DestructiveMigrationError( - 'Adding an LSI to a model requires replacement of the underlying DynamoDB table.', - [], - [tableName], + 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.", ); } - - 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; }; /** @@ -311,79 +317,77 @@ export const cantMutateMultipleGSIAtUpdateTimeRule = (diffs: Diff[], currentBuil * @param currentBuild The last deployed build. * @param nextBuild The next build. */ -export const cantEditLSIKeySchemaRule = (iterativeUpdatesEnabled: boolean = false) => ( - 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]); +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.', + ); } - - 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 cantRemoveLSILater = (iterativeUpdatesEnabled: boolean = false) => ( - 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]); +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); } - 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); - } + return cantRemoveLSILater; }; export const cantHaveMoreThan500ResourcesRule = (diffs: Diff[], currentBuild: DiffableProject, nextBuild: DiffableProject): void => { From ea76931ba4d0d967bf75bf0f09789336dc948f4d Mon Sep 17 00:00:00 2001 From: Edward Foyle Date: Tue, 21 Sep 2021 18:23:02 -0700 Subject: [PATCH 14/21] chore: address PR comments --- .../__tests__/disconnect-dependent-resources/utils.test.ts | 3 --- .../graphql-transformer/amplify-graphql-resource-manager.ts | 4 +++- .../amplify-provider-awscloudformation/src/push-resources.ts | 1 - 3 files changed, 3 insertions(+), 5 deletions(-) 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 index 6a0a42fd3c0..9b16d26be34 100644 --- 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 @@ -87,9 +87,6 @@ describe('generateTempFuncCFNTemplates', () => { }); describe('uploadTempFuncDeploymentFiles', () => { - // const log_mock = jest.fn(); - // const logger_mock = jest.fn().mockReturnValue(log_mock); - // fileLogger_mock.mockReturnValue(() => () => jest.fn()); it('uploads template and meta file', async () => { fs_mock.createReadStream .mockReturnValueOnce('func1Template' as any) 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 4de4b2a2523..aaa40b429b7 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 @@ -106,7 +106,9 @@ export class GraphQLResourceManager { throw err; } } - this.gsiManagement(gqlDiff.diff, gqlDiff.current, gqlDiff.next); + if (!this.rebuildAllTables) { + this.gsiManagement(gqlDiff.diff, gqlDiff.current, gqlDiff.next); + } this.tableRecreationManagement(gqlDiff.current, gqlDiff.next); return await this.getDeploymentSteps(); }; diff --git a/packages/amplify-provider-awscloudformation/src/push-resources.ts b/packages/amplify-provider-awscloudformation/src/push-resources.ts index 6b9485fc1a7..1daab06ec7e 100644 --- a/packages/amplify-provider-awscloudformation/src/push-resources.ts +++ b/packages/amplify-provider-awscloudformation/src/push-resources.ts @@ -232,7 +232,6 @@ export async function run(context: $TSContext, resourceDefinition: $TSObject, re await generateAndUploadRootStack(context, nestedStackFilepath, nestedStackFileName); // Use state manager to do the final deployment. The final deployment include not just API change but the whole Amplify Project - // TODO check that this step will also reinstate the original function template const finalStep: DeploymentOp = { stackTemplatePathOrUrl: nestedStackFileName, tableNames: [], From 3bd02b14b792848aaee29d0e653553331dd50b78 Mon Sep 17 00:00:00 2001 From: Edward Foyle Date: Fri, 24 Sep 2021 15:44:24 -0700 Subject: [PATCH 15/21] test: fix e2e failures --- .../amplify-e2e-core/src/init/amplifyPush.ts | 27 +++++++++++++++---- .../amplify-e2e-core/src/utils/headless.ts | 18 ++++++++++--- .../src/__tests__/api_1.test.ts | 2 +- .../src/__tests__/api_3.test.ts | 4 +-- .../schema-iterative-update-1.test.ts | 2 +- .../disconnect-dependent-resources/index.ts | 2 +- .../src/push-resources.ts | 1 - .../graphql-auth-transformer/src/resources.ts | 8 +++--- .../graphql-transformer-core/src/errors.ts | 2 +- .../src/util/sanity-check.ts | 4 ++- 10 files changed, 50 insertions(+), 20 deletions(-) diff --git a/packages/amplify-e2e-core/src/init/amplifyPush.ts b/packages/amplify-e2e-core/src/init/amplifyPush.ts index bf359bb3750..dce24ccb03b 100644 --- a/packages/amplify-e2e-core/src/init/amplifyPush.ts +++ b/packages/amplify-e2e-core/src/init/amplifyPush.ts @@ -98,9 +98,18 @@ export function amplifyPushWithoutCodegen(cwd: string, testingWithLatestCodebase }); } -export function amplifyPushUpdate(cwd: string, waitForText?: RegExp, testingWithLatestCodebase: boolean = false): Promise { +export function amplifyPushUpdate( + cwd: string, + waitForText?: RegExp, + testingWithLatestCodebase: boolean = false, + allowDestructiveUpdates: boolean = false, +): Promise { + const args = ['push']; + if (allowDestructiveUpdates) { + args.push('--allow-destructive-graphql-schema-updates'); + } return new Promise((resolve, reject) => { - spawn(getCLIPath(testingWithLatestCodebase), ['push'], { cwd, stripColors: true, noOutputTimeout: pushTimeoutMS }) + spawn(getCLIPath(testingWithLatestCodebase), args, { cwd, stripColors: true, noOutputTimeout: pushTimeoutMS }) .wait('Are you sure you want to continue?') .sendConfirmYes() .wait(waitForText || /.*/) @@ -130,9 +139,17 @@ export function amplifyPushAuth(cwd: string, testingWithLatestCodebase: boolean }); } -export function amplifyPushUpdateForDependentModel(cwd: string, testingWithLatestCodebase: boolean = false): Promise { +export function amplifyPushUpdateForDependentModel( + cwd: string, + testingWithLatestCodebase: boolean = false, + allowDestructiveUpdates: boolean = false, +): Promise { + const args = ['push']; + if (allowDestructiveUpdates) { + args.push('--allow-destructive-graphql-schema-updates'); + } return new Promise((resolve, reject) => { - spawn(getCLIPath(testingWithLatestCodebase), ['push'], { cwd, stripColors: true, noOutputTimeout: pushTimeoutMS }) + spawn(getCLIPath(testingWithLatestCodebase), args, { cwd, stripColors: true, noOutputTimeout: pushTimeoutMS }) .wait('Are you sure you want to continue?') .sendConfirmYes() .wait(/.*/) @@ -251,7 +268,7 @@ export function amplifyPushWithNoChanges(cwd: string, testingWithLatestCodebase: return new Promise((resolve, reject) => { spawn(getCLIPath(testingWithLatestCodebase), ['push'], { cwd, stripColors: true, noOutputTimeout: pushTimeoutMS }) .wait('No changes detected') - .run((err: Error) => err ? reject(err) : resolve()); + .run((err: Error) => (err ? reject(err) : resolve())); }); } diff --git a/packages/amplify-e2e-core/src/utils/headless.ts b/packages/amplify-e2e-core/src/utils/headless.ts index 2fbce197d92..fcbbb0ef10f 100644 --- a/packages/amplify-e2e-core/src/utils/headless.ts +++ b/packages/amplify-e2e-core/src/utils/headless.ts @@ -6,8 +6,8 @@ export const addHeadlessApi = async (cwd: string, request: AddApiRequest) => { await executeHeadlessCommand(cwd, 'api', 'add', request); }; -export const updateHeadlessApi = async (cwd: string, request: UpdateApiRequest) => { - await executeHeadlessCommand(cwd, 'api', 'update', request); +export const updateHeadlessApi = async (cwd: string, request: UpdateApiRequest, allowDestructiveUpdates?: boolean) => { + await executeHeadlessCommand(cwd, 'api', 'update', request, allowDestructiveUpdates); }; export const removeHeadlessApi = async (cwd: string, apiName: string) => { @@ -33,8 +33,18 @@ export const headlessAuthImport = async (cwd: string, request: ImportAuthRequest const headlessRemoveResource = async (cwd: string, category: string, resourceName: string) => { await execa(getCLIPath(), ['remove', category, resourceName, '--yes'], { cwd }); }; -const executeHeadlessCommand = async (cwd: string, category: string, operation: string, request: AnyHeadlessRequest) => { - await execa(getCLIPath(), [operation, category, '--headless'], { input: JSON.stringify(request), cwd }); +const executeHeadlessCommand = async ( + cwd: string, + category: string, + operation: string, + request: AnyHeadlessRequest, + allowDestructiveUpdates: boolean = false, +) => { + const args = [operation, category, '--headless']; + if (allowDestructiveUpdates) { + args.push('--allow-destructive-graphql-schema-updates'); + } + await execa(getCLIPath(), args, { input: JSON.stringify(request), cwd }); }; type AnyHeadlessRequest = AddApiRequest | UpdateApiRequest | AddAuthRequest | UpdateAuthRequest | ImportAuthRequest; diff --git a/packages/amplify-e2e-tests/src/__tests__/api_1.test.ts b/packages/amplify-e2e-tests/src/__tests__/api_1.test.ts index e20b226cab2..b03f2ce2110 100644 --- a/packages/amplify-e2e-tests/src/__tests__/api_1.test.ts +++ b/packages/amplify-e2e-tests/src/__tests__/api_1.test.ts @@ -235,7 +235,7 @@ describe('amplify add api (GraphQL)', () => { ); await amplifyPush(projRoot); updateApiSchema(projRoot, projectName, nextSchema); - await amplifyPushUpdateForDependentModel(projRoot); + await amplifyPushUpdateForDependentModel(projRoot, undefined, true); const meta = getProjectMeta(projRoot); const region = meta.providers.awscloudformation.Region; const { output } = meta.api.blogapp; diff --git a/packages/amplify-e2e-tests/src/__tests__/api_3.test.ts b/packages/amplify-e2e-tests/src/__tests__/api_3.test.ts index 266fafd3423..1f6630c26f4 100644 --- a/packages/amplify-e2e-tests/src/__tests__/api_3.test.ts +++ b/packages/amplify-e2e-tests/src/__tests__/api_3.test.ts @@ -123,8 +123,8 @@ describe('amplify add api (GraphQL)', () => { await initJSProjectWithProfile(projRoot, {}); await addHeadlessApi(projRoot, addApiRequest); await amplifyPush(projRoot); - await updateHeadlessApi(projRoot, updateApiRequest); - await amplifyPushUpdate(projRoot); + await updateHeadlessApi(projRoot, updateApiRequest, true); + await amplifyPushUpdate(projRoot, undefined, undefined, true); // verify const meta = getProjectMeta(projRoot); diff --git a/packages/amplify-e2e-tests/src/__tests__/schema-iterative-update-1.test.ts b/packages/amplify-e2e-tests/src/__tests__/schema-iterative-update-1.test.ts index 2c4109c2b8b..b76e34a6b72 100644 --- a/packages/amplify-e2e-tests/src/__tests__/schema-iterative-update-1.test.ts +++ b/packages/amplify-e2e-tests/src/__tests__/schema-iterative-update-1.test.ts @@ -33,6 +33,6 @@ describe('Schema iterative update - rename @key', () => { const finalSchema = path.join('iterative-push', 'change-model-name', 'final-schema.graphql'); await updateApiSchema(projectDir, apiName, finalSchema); - await amplifyPushUpdate(projectDir); + await amplifyPushUpdate(projectDir, undefined, undefined, true); }); }); diff --git a/packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/index.ts b/packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/index.ts index 6fdae96a7f9..40f83217ef5 100644 --- a/packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/index.ts +++ b/packages/amplify-provider-awscloudformation/src/disconnect-dependent-resources/index.ts @@ -31,7 +31,7 @@ export const prependDeploymentStepsToDisconnectFunctionsFromReplacedModelTables ): Promise => { const amplifyMeta = stateManager.getMeta(); const rootStackId = amplifyMeta?.providers?.awscloudformation?.StackId; - const allFunctionNames = Object.keys(amplifyMeta?.function); + const allFunctionNames = Object.keys(amplifyMeta?.function || {}); functionsDependentOnReplacedModelTables = await getDependentFunctions( modelsBeingReplaced, allFunctionNames, diff --git a/packages/amplify-provider-awscloudformation/src/push-resources.ts b/packages/amplify-provider-awscloudformation/src/push-resources.ts index e738daccc94..23a1331f0f7 100644 --- a/packages/amplify-provider-awscloudformation/src/push-resources.ts +++ b/packages/amplify-provider-awscloudformation/src/push-resources.ts @@ -160,7 +160,6 @@ 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 } = {}; diff --git a/packages/graphql-auth-transformer/src/resources.ts b/packages/graphql-auth-transformer/src/resources.ts index 6749af89196..f81640e94c2 100644 --- a/packages/graphql-auth-transformer/src/resources.ts +++ b/packages/graphql-auth-transformer/src/resources.ts @@ -109,9 +109,11 @@ export class ResourceFactory { expirationDays = apiKeyConfig.apiKeyExpirationDays; } // add delay expiration time is valid upon resource creation - let expirationDateInSeconds = 60 /* s */ * 60 /* m */ * 24 /* h */ * expirationDays; /* d */ - // Add a 2 minute time delay if set to 1 day: https://github.com/aws-amplify/amplify-cli/issues/4460 - if (expirationDays === 1) expirationDateInSeconds += 60 * 2; + let expirationDateInSeconds = 60 * 60 * 24 * expirationDays; // sec * min * hour * days + // Add a 30 minute time delay if set to 1 day: https://github.com/aws-amplify/amplify-cli/issues/4460 + // initially this was 2 minutes but with iterative deployments it is possible for deployments to take much longer than that + // if an iterative deployment takes longer than 30 mins, and API key expiration is set to 1 day, deployments may still fail + if (expirationDays === 1) expirationDateInSeconds += 60 * 30; const nowEpochTime = Math.floor(Date.now() / 1000); return new AppSync.ApiKey({ ApiId: Fn.GetAtt(ResourceConstants.RESOURCES.GraphQLAPILogicalID, 'ApiId'), diff --git a/packages/graphql-transformer-core/src/errors.ts b/packages/graphql-transformer-core/src/errors.ts index 78114f1a7b7..4c50af965bd 100644 --- a/packages/graphql-transformer-core/src/errors.ts +++ b/packages/graphql-transformer-core/src/errors.ts @@ -56,7 +56,7 @@ export class DestructiveMigrationError extends Error { } 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'.`; + this.message = `${this.message}\nALL EXISTING DATA IN THESE TABLES WILL BE LOST!\nIf this is intended, rerun the command with '--allow-destructive-graphql-schema-updates'.`; } toString = () => this.message; } diff --git a/packages/graphql-transformer-core/src/util/sanity-check.ts b/packages/graphql-transformer-core/src/util/sanity-check.ts index 1e2312c00ea..9de3dcadc9c 100644 --- a/packages/graphql-transformer-core/src/util/sanity-check.ts +++ b/packages/graphql-transformer-core/src/util/sanity-check.ts @@ -415,7 +415,9 @@ export const cantRemoveTableAfterCreation = (_: Diff, currentBuild: DiffableProj .map(([name]) => name); const currentModels = getNestedStackLogicalIds(currentBuild); const nextModels = getNestedStackLogicalIds(nextBuild); - const removedModels = currentModels.filter(currModel => !nextModels.includes(currModel)); + const removedModels = currentModels + .filter(currModel => !nextModels.includes(currModel)) + .filter(stackLogicalId => stackLogicalId !== 'ConnectionStack'); if (removedModels.length > 0) { throw new DestructiveMigrationError( 'Removing a model from the GraphQL schema will also remove the underlying DynamoDB table.', From 2bfdacf619dba863fa6cc8b243dde7e2abb30619 Mon Sep 17 00:00:00 2001 From: Edward Foyle Date: Thu, 30 Sep 2021 16:13:09 -0700 Subject: [PATCH 16/21] chore: fix prompts dep version --- packages/amplify-category-api/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/amplify-category-api/package.json b/packages/amplify-category-api/package.json index 90e7ad88e0b..a4c2c13a753 100644 --- a/packages/amplify-category-api/package.json +++ b/packages/amplify-category-api/package.json @@ -65,7 +65,7 @@ "@octokit/rest": "^18.0.9", "amplify-cli-core": "1.30.0", "amplify-headless-interface": "1.10.0", - "amplify-prompts": "1.1.2", + "amplify-prompts": "1.2.0", "amplify-util-headless-input": "1.5.4", "chalk": "^4.1.1", "constructs": "^3.3.125", From c6f07694eb267b26f93a34376a5e3870d811902b Mon Sep 17 00:00:00 2001 From: Edward Foyle Date: Fri, 1 Oct 2021 09:35:08 -0700 Subject: [PATCH 17/21] test: exclude new api tests from windows --- .circleci/config.yml | 1 - scripts/split-e2e-tests.ts | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 85672b25827..600fc62e59c 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -15234,7 +15234,6 @@ workflows: parameters: os: - linux - - windows - frontend_config_drift-amplify_e2e_tests_pkg: context: - amplify-ecr-image-pull diff --git a/scripts/split-e2e-tests.ts b/scripts/split-e2e-tests.ts index e2eb6997204..77ff70c7589 100644 --- a/scripts/split-e2e-tests.ts +++ b/scripts/split-e2e-tests.ts @@ -18,6 +18,7 @@ const WINDOWS_TEST_FAILURES = [ 'api_3-amplify_e2e_tests', 'api_4-amplify_e2e_tests', 'api_5-amplify_e2e_tests', + 'api_6-amplify_e2e_tests', 'auth_1-amplify_e2e_tests', 'auth_2-amplify_e2e_tests', 'auth_3-amplify_e2e_tests', From 89d4d8241563c131925c427f6fc6fb4b83488f81 Mon Sep 17 00:00:00 2001 From: Edward Foyle Date: Fri, 1 Oct 2021 15:14:21 -0700 Subject: [PATCH 18/21] test: update test schema --- .../iterative-push/change-model-name/initial-schema.graphql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/amplify-e2e-tests/schemas/iterative-push/change-model-name/initial-schema.graphql b/packages/amplify-e2e-tests/schemas/iterative-push/change-model-name/initial-schema.graphql index 01480f28130..d3190805b39 100644 --- a/packages/amplify-e2e-tests/schemas/iterative-push/change-model-name/initial-schema.graphql +++ b/packages/amplify-e2e-tests/schemas/iterative-push/change-model-name/initial-schema.graphql @@ -3,7 +3,7 @@ type Comment @model @key(name: "byTodo", fields: ["todoID"]) { todoID: ID! } -type Todo @model { +type Task @model { id: ID! name: String! description: String From 631cd8bc7052497b302cfb35a2114cd943aca82b Mon Sep 17 00:00:00 2001 From: Edward Foyle Date: Wed, 20 Oct 2021 16:51:38 -0700 Subject: [PATCH 19/21] chore: address PR comments --- .circleci/config.yml | 10 ++++++++++ .../src/commands/api/rebuild.ts | 4 ++-- .../src/__tests__/api_6.test.ts | 12 +++++------ .../src/transform-graphql-schema.ts | 2 +- .../graphql-transformer-core/src/errors.ts | 9 +++++---- scripts/split-e2e-tests.ts | 20 ------------------- 6 files changed, 24 insertions(+), 33 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index ad8ddecaa12..9c05ca9a027 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -14812,6 +14812,7 @@ workflows: parameters: os: - linux + - windows - schema-key-amplify_e2e_tests_pkg: context: - amplify-ecr-image-pull @@ -14957,6 +14958,7 @@ workflows: parameters: os: - linux + - windows - frontend_config_drift-amplify_e2e_tests_pkg: context: - amplify-ecr-image-pull @@ -15144,6 +15146,7 @@ workflows: parameters: os: - linux + - windows - auth_3-amplify_e2e_tests_pkg: context: - amplify-ecr-image-pull @@ -15254,6 +15257,7 @@ workflows: parameters: os: - linux + - windows - function_5-amplify_e2e_tests_pkg: context: - amplify-ecr-image-pull @@ -15420,6 +15424,7 @@ workflows: parameters: os: - linux + - windows - migration-api-key-migration1-amplify_e2e_tests_pkg: context: - amplify-ecr-image-pull @@ -15474,6 +15479,7 @@ workflows: parameters: os: - linux + - windows - function_4-amplify_e2e_tests_pkg: context: - amplify-ecr-image-pull @@ -15528,6 +15534,7 @@ workflows: parameters: os: - linux + - windows - function_6-amplify_e2e_tests_pkg: context: - amplify-ecr-image-pull @@ -15934,6 +15941,7 @@ workflows: parameters: os: - linux + - windows - geo-update-amplify_e2e_tests_pkg: context: - amplify-ecr-image-pull @@ -16340,6 +16348,7 @@ workflows: parameters: os: - linux + - windows - custom_policies_container-amplify_e2e_tests_pkg: context: - amplify-ecr-image-pull @@ -16617,6 +16626,7 @@ workflows: parameters: os: - linux + - windows - AuthV2Transformer-e2e-graphql_e2e_tests: context: - amplify-ecr-image-pull diff --git a/packages/amplify-category-api/src/commands/api/rebuild.ts b/packages/amplify-category-api/src/commands/api/rebuild.ts index 2173ba77c49..26b8ead3641 100644 --- a/packages/amplify-category-api/src/commands/api/rebuild.ts +++ b/packages/amplify-category-api/src/commands/api/rebuild.ts @@ -1,4 +1,4 @@ -import { $TSContext, FeatureFlags, stateManager } from 'amplify-cli-core'; +import { $TSAny, $TSContext, FeatureFlags, stateManager } from 'amplify-cli-core'; import { printer, prompter, exact } from 'amplify-prompts'; const subcommand = 'rebuild'; @@ -14,7 +14,7 @@ export const run = async (context: $TSContext) => { return; } const apiNames = Object.entries(stateManager.getMeta()?.api || {}) - .filter(([_, meta]) => (meta as any).service === 'AppSync') + .filter(([_, apiResource]) => (apiResource as $TSAny).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`.'); diff --git a/packages/amplify-e2e-tests/src/__tests__/api_6.test.ts b/packages/amplify-e2e-tests/src/__tests__/api_6.test.ts index 7aacb724157..79bbebf1ec9 100644 --- a/packages/amplify-e2e-tests/src/__tests__/api_6.test.ts +++ b/packages/amplify-e2e-tests/src/__tests__/api_6.test.ts @@ -1,7 +1,7 @@ import { createNewProjectDir, initJSProjectWithProfile, - addApiWithSchema, + addApiWithoutSchema, amplifyPush, deleteProject, deleteProjectDir, @@ -20,7 +20,7 @@ let projRoot; beforeEach(async () => { projRoot = await createNewProjectDir(projName); await initJSProjectWithProfile(projRoot, { name: projName }); - await addApiWithSchema(projRoot, 'simple_model.graphql', { apiKeyExpirationDays: 7 }); + await addApiWithoutSchema(projRoot); await amplifyPush(projRoot); }); afterEach(async () => { @@ -46,20 +46,20 @@ describe('amplify reset api', () => { 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( @@ -81,4 +81,4 @@ describe('destructive updates flag', () => { await amplifyPushDestructiveApiUpdate(projRoot, false); // success indicates that the push completed }); -}); \ No newline at end of file +}); diff --git a/packages/amplify-provider-awscloudformation/src/transform-graphql-schema.ts b/packages/amplify-provider-awscloudformation/src/transform-graphql-schema.ts index d151bdd2229..c506f7937a9 100644 --- a/packages/amplify-provider-awscloudformation/src/transform-graphql-schema.ts +++ b/packages/amplify-provider-awscloudformation/src/transform-graphql-schema.ts @@ -498,7 +498,7 @@ export async function transformGraphQLSchema(context, options) { } const ff = new AmplifyCLIFeatureFlagAdapter(); - const allowDestructiveUpdates = context?.input?.options?.[destructiveUpdatesFlag] || context.input?.options?.force; + const allowDestructiveUpdates = context?.input?.options?.[destructiveUpdatesFlag] || context?.input?.options?.force; const sanityCheckRulesList = getSanityCheckRules(isNewAppSyncAPI, ff, allowDestructiveUpdates); const buildConfig = { diff --git a/packages/graphql-transformer-core/src/errors.ts b/packages/graphql-transformer-core/src/errors.ts index 4c50af965bd..b50f4ae577b 100644 --- a/packages/graphql-transformer-core/src/errors.ts +++ b/packages/graphql-transformer-core/src/errors.ts @@ -1,4 +1,5 @@ import { GraphQLError } from 'graphql'; +import * as os from 'os'; export class InvalidTransformerError extends Error { constructor(message: string) { @@ -50,13 +51,13 @@ export class DestructiveMigrationError extends Error { 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}]`; + this.message = `${this.message}${os.EOL}This 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}]`; + this.message = `${this.message}${os.EOL}This update will remove table(s) [${removedModelsList}]`; } else if (replacedModelsList) { - this.message = `${this.message}\nThis update will replace table(s) [${replacedModelsList}]`; + this.message = `${this.message}${os.EOL}This 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-destructive-graphql-schema-updates'.`; + this.message = `${this.message}${os.EOL}ALL EXISTING DATA IN THESE TABLES WILL BE LOST!${os.EOL}If this is intended, rerun the command with '--allow-destructive-graphql-schema-updates'.`; } toString = () => this.message; } diff --git a/scripts/split-e2e-tests.ts b/scripts/split-e2e-tests.ts index 6048477d8a3..8a39f8b7cfa 100644 --- a/scripts/split-e2e-tests.ts +++ b/scripts/split-e2e-tests.ts @@ -11,26 +11,6 @@ const CONCURRENCY = 25; // Each of these failures should be independently investigated, resolved, and removed from this list. // For now, this list is being used to skip creation of circleci jobs for these tasks const WINDOWS_TEST_FAILURES = [ - 'amplify-app-amplify_e2e_tests', - 'analytics-amplify_e2e_tests', - 'api_1-amplify_e2e_tests', - 'api_2-amplify_e2e_tests', - 'api_3-amplify_e2e_tests', - 'api_4-amplify_e2e_tests', - 'api_5-amplify_e2e_tests', - 'api_6-amplify_e2e_tests', - 'auth_1-amplify_e2e_tests', - 'auth_2-amplify_e2e_tests', - 'auth_3-amplify_e2e_tests', - 'auth_4-amplify_e2e_tests', - // Auth tests are failing because - // us-east-1 region is not allowed in parent e2e test account - // and `singleSelect` for region is not working properly in windows - 'auth_5-amplify_e2e_tests', - 'auth_6-amplify_e2e_tests', - 'auth_7-amplify_e2e_tests', - 'auth_8-amplify_e2e_tests', - 'containers-api-amplify_e2e_tests', 'datastore-modelgen-amplify_e2e_tests', 'delete-amplify_e2e_tests', 'env-amplify_e2e_tests', From 7779c5da6a16ed687df51de1be86d377221aaa59 Mon Sep 17 00:00:00 2001 From: Edward Foyle Date: Wed, 20 Oct 2021 19:06:40 -0700 Subject: [PATCH 20/21] fix: messed up merge --- packages/amplify-e2e-core/src/utils/headless.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/amplify-e2e-core/src/utils/headless.ts b/packages/amplify-e2e-core/src/utils/headless.ts index 94f92a1eb3f..3d367ec97b8 100644 --- a/packages/amplify-e2e-core/src/utils/headless.ts +++ b/packages/amplify-e2e-core/src/utils/headless.ts @@ -16,8 +16,12 @@ export const addHeadlessApi = async (cwd: string, request: AddApiRequest): Promi return await executeHeadlessCommand(cwd, 'api', 'add', request); }; -export const updateHeadlessApi = async (cwd: string, request: UpdateApiRequest, allowDestructiveUpdates?: boolean): Promise> => { - return await executeHeadlessCommand(cwd, 'api', 'update', request, allowDestructiveUpdates); +export const updateHeadlessApi = async ( + cwd: string, + request: UpdateApiRequest, + allowDestructiveUpdates?: boolean, +): Promise> => { + return await executeHeadlessCommand(cwd, 'api', 'update', request, undefined, allowDestructiveUpdates); }; export const removeHeadlessApi = async (cwd: string, apiName: string): Promise> => { @@ -69,13 +73,14 @@ const executeHeadlessCommand = async ( category: string, operation: string, request: AnyHeadlessRequest, + reject: boolean = true, allowDestructiveUpdates: boolean = false, ): Promise> => { const args = [operation, category, '--headless']; if (allowDestructiveUpdates) { args.push('--allow-destructive-graphql-schema-updates'); } - return await execa(getCLIPath(), args, { input: JSON.stringify(request), cwd }); + return await execa(getCLIPath(), args, { input: JSON.stringify(request), cwd, reject }); }; type AnyHeadlessRequest = From 9acf7e328a693959981f8d6098b313c791aed648 Mon Sep 17 00:00:00 2001 From: Edward Foyle Date: Wed, 20 Oct 2021 21:54:30 -0700 Subject: [PATCH 21/21] test: fix e2e failures --- .circleci/config.yml | 1 - packages/amplify-e2e-core/src/init/amplifyPush.ts | 12 ++++++++++-- .../src/__tests__/function_5.test.ts | 2 +- scripts/split-e2e-tests.ts | 1 + 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 9c05ca9a027..5c86d2ca3fb 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -16626,7 +16626,6 @@ workflows: parameters: os: - linux - - windows - AuthV2Transformer-e2e-graphql_e2e_tests: context: - amplify-ecr-image-pull diff --git a/packages/amplify-e2e-core/src/init/amplifyPush.ts b/packages/amplify-e2e-core/src/init/amplifyPush.ts index 3aa5b6be1f9..8ec31b0a06c 100644 --- a/packages/amplify-e2e-core/src/init/amplifyPush.ts +++ b/packages/amplify-e2e-core/src/init/amplifyPush.ts @@ -83,9 +83,17 @@ export function cancelIterativeAmplifyPush( }); } -export function amplifyPushWithoutCodegen(cwd: string, testingWithLatestCodebase: boolean = false): Promise { +export function amplifyPushWithoutCodegen( + cwd: string, + testingWithLatestCodebase: boolean = false, + allowDestructiveUpdates: boolean = false, +): Promise { + const args = ['push']; + if (allowDestructiveUpdates) { + args.push('--allow-destructive-graphql-schema-updates'); + } return new Promise((resolve, reject) => { - spawn(getCLIPath(testingWithLatestCodebase), ['push'], { cwd, stripColors: true, noOutputTimeout: pushTimeoutMS }) + spawn(getCLIPath(testingWithLatestCodebase), args, { cwd, stripColors: true, noOutputTimeout: pushTimeoutMS }) .wait('Are you sure you want to continue?') .sendCarriageReturn() .run((err: Error) => { diff --git a/packages/amplify-e2e-tests/src/__tests__/function_5.test.ts b/packages/amplify-e2e-tests/src/__tests__/function_5.test.ts index 9bb8ebdd115..986210b2e08 100644 --- a/packages/amplify-e2e-tests/src/__tests__/function_5.test.ts +++ b/packages/amplify-e2e-tests/src/__tests__/function_5.test.ts @@ -117,6 +117,6 @@ describe('test dependency in root stack', () => { }, 'nodejs', ); - await amplifyPushWithoutCodegen(projRoot); + await amplifyPushWithoutCodegen(projRoot, undefined, true); }); }); diff --git a/scripts/split-e2e-tests.ts b/scripts/split-e2e-tests.ts index 8a39f8b7cfa..23815c52e38 100644 --- a/scripts/split-e2e-tests.ts +++ b/scripts/split-e2e-tests.ts @@ -11,6 +11,7 @@ const CONCURRENCY = 25; // Each of these failures should be independently investigated, resolved, and removed from this list. // For now, this list is being used to skip creation of circleci jobs for these tasks const WINDOWS_TEST_FAILURES = [ + 'api_6-amplify_e2e_tests', 'datastore-modelgen-amplify_e2e_tests', 'delete-amplify_e2e_tests', 'env-amplify_e2e_tests',