From 62b44365108ba3410c9023623394aa98a52db84e Mon Sep 17 00:00:00 2001 From: John Hockett Date: Mon, 29 Nov 2021 15:46:30 -0800 Subject: [PATCH] fix: migrate rest apis with protected routes on push (#9068) * fix: migrate rest apis with protected routes on push * fix: gracefully exit after 'no' to REST API migration --- .../apigw-input-state.test.ts | 13 ++--- packages/amplify-category-api/src/index.ts | 4 +- .../awscloudformation/apigw-input-state.ts | 10 ++-- .../service-walkthroughs/apigw-walkthrough.ts | 5 ++ .../src/push-resources.ts | 10 ++-- .../src/resource-package/resource-packager.ts | 2 +- .../src/utils/consolidate-apigw-policies.ts | 58 +++++++++++++------ .../src/utils/env-level-constructs.ts | 2 +- 8 files changed, 63 insertions(+), 41 deletions(-) diff --git a/packages/amplify-category-api/src/__tests__/provider-utils/awscloudformation/apigw-input-state.test.ts b/packages/amplify-category-api/src/__tests__/provider-utils/awscloudformation/apigw-input-state.test.ts index 58057f91798..6211316a175 100644 --- a/packages/amplify-category-api/src/__tests__/provider-utils/awscloudformation/apigw-input-state.test.ts +++ b/packages/amplify-category-api/src/__tests__/provider-utils/awscloudformation/apigw-input-state.test.ts @@ -1,5 +1,6 @@ import { $TSContext, getMigrateResourceMessageForOverride, JSONUtilities, pathManager, stateManager } from 'amplify-cli-core'; import { prompter } from 'amplify-prompts'; +import * as fs from 'fs-extra'; import { ApigwInputState } from '../../../provider-utils/awscloudformation/apigw-input-state'; jest.mock('amplify-cli-core'); @@ -7,6 +8,7 @@ jest.mock('fs-extra'); jest.mock('path'); jest.mock('../../../provider-utils/awscloudformation/cdk-stack-builder'); +const fs_mock = fs as jest.Mocked; const JSONUtilities_mock = JSONUtilities as jest.Mocked; const pathManager_mock = pathManager as jest.Mocked; const prompter_mock = prompter as jest.Mocked; @@ -17,9 +19,6 @@ const context_mock = { updateamplifyMetaAfterResourceAdd: jest.fn(), updateamplifyMetaAfterResourceUpdate: jest.fn(), }, - filesystem: { - remove: jest.fn(), - }, } as unknown as $TSContext; pathManager_mock.findProjectRoot = jest.fn().mockReturnValue('mockProjRoot'); @@ -131,7 +130,7 @@ describe('REST API input state', () => { paths: mockApiPaths, }); expect(stateManager_mock.setResourceParametersJson).toHaveBeenCalled(); - expect(context_mock.filesystem.remove).toHaveBeenCalledTimes(3); + expect(fs_mock.removeSync).toHaveBeenCalledTimes(3); }); it('does nothing when choosing NOT to migrate a REST API', async () => { @@ -145,7 +144,7 @@ describe('REST API input state', () => { expect(stateManager_mock.setResourceInputsJson).not.toHaveBeenCalled(); expect(stateManager_mock.setResourceParametersJson).not.toHaveBeenCalled(); expect(context_mock.amplify.updateamplifyMetaAfterResourceUpdate).not.toHaveBeenCalled(); - expect(context_mock.filesystem.remove).not.toHaveBeenCalled(); + expect(fs_mock.removeSync).not.toHaveBeenCalled(); }); it('generates expected artifacts when choosing to migrate an Admin Queries API', async () => { @@ -162,7 +161,7 @@ describe('REST API input state', () => { expect(getMigrateResourceMessageForOverride).toHaveBeenCalled(); expect(stateManager_mock.setResourceInputsJson).toHaveBeenCalled(); expect(stateManager_mock.setResourceParametersJson).toHaveBeenCalled(); - expect(context_mock.filesystem.remove).toHaveBeenCalledTimes(2); + expect(fs_mock.removeSync).toHaveBeenCalledTimes(2); }); it('does nothing when choosing NOT to migrate an Admin Queries API', async () => { @@ -181,7 +180,7 @@ describe('REST API input state', () => { expect(stateManager_mock.setResourceInputsJson).not.toHaveBeenCalled(); expect(stateManager_mock.setResourceParametersJson).not.toHaveBeenCalled(); expect(context_mock.amplify.updateamplifyMetaAfterResourceUpdate).not.toHaveBeenCalled(); - expect(context_mock.filesystem.remove).not.toHaveBeenCalled(); + expect(fs_mock.removeSync).not.toHaveBeenCalled(); }); it('generates expected artifacts when adding an Admin Queries API', async () => { diff --git a/packages/amplify-category-api/src/index.ts b/packages/amplify-category-api/src/index.ts index 1ea3d3fa91e..907872f87e2 100644 --- a/packages/amplify-category-api/src/index.ts +++ b/packages/amplify-category-api/src/index.ts @@ -45,8 +45,8 @@ export async function console(context: $TSContext) { } export async function migrate(context: $TSContext, serviceName?: string) { - const { projectPath } = context.migrationInfo; - const amplifyMeta = stateManager.getMeta(); + const { projectPath } = context?.migrationInfo ?? { projectPath: pathManager.findProjectRoot() }; + const amplifyMeta = stateManager.getMeta(projectPath); const migrateResourcePromises = []; for (const categoryName of Object.keys(amplifyMeta)) { if (categoryName === category) { diff --git a/packages/amplify-category-api/src/provider-utils/awscloudformation/apigw-input-state.ts b/packages/amplify-category-api/src/provider-utils/awscloudformation/apigw-input-state.ts index 3a20074f6ba..ab9529cfdfe 100644 --- a/packages/amplify-category-api/src/provider-utils/awscloudformation/apigw-input-state.ts +++ b/packages/amplify-category-api/src/provider-utils/awscloudformation/apigw-input-state.ts @@ -108,8 +108,8 @@ export class ApigwInputState { } const resourceDirPath = pathManager.getResourceDirectoryPath(this.projectRootPath, AmplifyCategories.API, this.resourceName); - this.context.filesystem.remove(join(resourceDirPath, PathConstants.ParametersJsonFileName)); - this.context.filesystem.remove(join(resourceDirPath, 'admin-queries-cloudformation-template.json')); + fs.removeSync(join(resourceDirPath, PathConstants.ParametersJsonFileName)); + fs.removeSync(join(resourceDirPath, 'admin-queries-cloudformation-template.json')); return this.updateAdminQueriesResource(adminQueriesProps); }; @@ -199,9 +199,9 @@ export class ApigwInputState { }; }); - this.context.filesystem.remove(deprecatedParametersFilePath); - this.context.filesystem.remove(join(resourceDirPath, PathConstants.ParametersJsonFileName)); - this.context.filesystem.remove(join(resourceDirPath, `${this.resourceName}-cloudformation-template.json`)); + fs.removeSync(deprecatedParametersFilePath); + fs.removeSync(join(resourceDirPath, PathConstants.ParametersJsonFileName)); + fs.removeSync(join(resourceDirPath, `${this.resourceName}-cloudformation-template.json`)); await this.createApigwArtifacts(); }; diff --git a/packages/amplify-category-api/src/provider-utils/awscloudformation/service-walkthroughs/apigw-walkthrough.ts b/packages/amplify-category-api/src/provider-utils/awscloudformation/service-walkthroughs/apigw-walkthrough.ts index 4a61ff32748..e2b6db7ef6d 100644 --- a/packages/amplify-category-api/src/provider-utils/awscloudformation/service-walkthroughs/apigw-walkthrough.ts +++ b/packages/amplify-category-api/src/provider-utils/awscloudformation/service-walkthroughs/apigw-walkthrough.ts @@ -78,6 +78,11 @@ export async function updateWalkthrough(context: $TSContext) { if (!stateManager.resourceInputsJsonExists(projRoot, category, selectedApiName)) { // Not yet migrated await migrate(context, projRoot, selectedApiName); + + // chose not to migrate + if (!stateManager.resourceInputsJsonExists(projRoot, category, selectedApiName)) { + exitOnNextTick(0); + } } const parameters = stateManager.getResourceInputsJson(projRoot, category, selectedApiName); diff --git a/packages/amplify-provider-awscloudformation/src/push-resources.ts b/packages/amplify-provider-awscloudformation/src/push-resources.ts index 50195f8e71d..c32f67909e1 100644 --- a/packages/amplify-provider-awscloudformation/src/push-resources.ts +++ b/packages/amplify-provider-awscloudformation/src/push-resources.ts @@ -932,17 +932,15 @@ export async function formNestedStack( }, }, }; - const apis = amplifyMeta?.api ?? {}; - Object.keys(apis).forEach(apiName => { - const api = apis[apiName]; - - if (loadApiCliInputs(apiName, api)) { + const apis: $TSObject = amplifyMeta?.api ?? {}; + for (const [apiName, api] of Object.entries(apis)) { + if (await loadApiCliInputs(context, apiName, api)) { stack.Properties.Parameters[apiName] = { 'Fn::GetAtt': [api.providerMetadata.logicalId, 'Outputs.ApiId'], }; } - }); + } rootStack.Resources[APIGW_AUTH_STACK_LOGICAL_ID] = stack; } diff --git a/packages/amplify-provider-awscloudformation/src/resource-package/resource-packager.ts b/packages/amplify-provider-awscloudformation/src/resource-package/resource-packager.ts index b31c381d0dc..737e551e3e5 100644 --- a/packages/amplify-provider-awscloudformation/src/resource-package/resource-packager.ts +++ b/packages/amplify-provider-awscloudformation/src/resource-package/resource-packager.ts @@ -230,7 +230,7 @@ export abstract class ResourcePackager { if (this.resourcesHasApiGatewaysButNotAdminQueries(resources)) { const { PROVIDER, PROVIDER_NAME } = Constants; const { StackName: stackName } = this.amplifyMeta[PROVIDER][PROVIDER_NAME]; - consolidateApiGatewayPolicies(this.context, stackName); + await consolidateApiGatewayPolicies(this.context, stackName); } await prePushAuthTransform(this.context, resources); for await (const resource of resources) { diff --git a/packages/amplify-provider-awscloudformation/src/utils/consolidate-apigw-policies.ts b/packages/amplify-provider-awscloudformation/src/utils/consolidate-apigw-policies.ts index a51f62fbffc..005d1590b8b 100644 --- a/packages/amplify-provider-awscloudformation/src/utils/consolidate-apigw-policies.ts +++ b/packages/amplify-provider-awscloudformation/src/utils/consolidate-apigw-policies.ts @@ -7,11 +7,12 @@ import { $TSObject, AmplifyCategories, AmplifySupportedService, + exitOnNextTick, JSONUtilities, pathManager, stateManager, } from 'amplify-cli-core'; -import * as fs from 'fs'; +import * as fs from 'fs-extra'; import * as path from 'path'; import { ProviderName } from '../constants'; @@ -178,33 +179,30 @@ function computePolicySizeIncrease(methodLength: number, pathLength: number, nam return 380 + 2 * (methodLength + pathLength + nameLength); } -export function consolidateApiGatewayPolicies(context: $TSContext, stackName: string): $TSObject { +export async function consolidateApiGatewayPolicies(context: $TSContext, stackName: string): Promise<$TSObject> { const apiGateways = []; const meta = stateManager.getMeta(); - const apis = meta?.api ?? {}; + const apis: $TSObject = meta?.api ?? {}; try { const cfnPath = path.join(pathManager.getBackendDirPath(), AmplifyCategories.API, `${APIGW_AUTH_STACK_LOGICAL_ID}.json`); fs.unlinkSync(cfnPath); } catch {} - Object.keys(apis).forEach(resourceName => { - const resource = apis[resourceName]; - const cliInputs = loadApiCliInputs(resourceName, resource); + for (const [resourceName, resource] of Object.entries(apis)) { + const cliInputs = await loadApiCliInputs(context, resourceName, resource); - if (!cliInputs) { - return; + if (cliInputs) { + const api = { ...resource, resourceName, params: cliInputs }; + apiGateways.push(api); } - - const api = { ...resource, resourceName, params: cliInputs }; - apiGateways.push(api); - }); + } if (apiGateways.length === 0) { return { APIGatewayAuthURL: undefined }; } - return { APIGatewayAuthURL: createApiGatewayAuthResources(context, stackName, apiGateways) }; + return { APIGatewayAuthURL: createApiGatewayAuthResources(stackName, apiGateways) }; } enum CrudOperation { @@ -224,15 +222,14 @@ function convertCrudOperationsToPermissions(crudOps: CrudOperation[]) { return crudOps.flatMap(op => opMap[op]); } -function createApiGatewayAuthResources(context: $TSContext, stackName: string, apiGateways: $TSAny): string | undefined { +function createApiGatewayAuthResources(stackName: string, apiGateways: $TSAny): string | undefined { const stack = new ApiGatewayAuthStack(undefined, 'Amplify', { description: 'API Gateway policy stack created using Amplify CLI', stackName, apiGateways, }); const cfn = stack.toCloudFormation(); - const { amplify } = context; - const { DeploymentBucketName } = amplify.getProjectMeta()?.providers?.[ProviderName] ?? {}; + const { DeploymentBucketName } = stateManager.getMeta()?.providers?.[ProviderName] ?? {}; const cfnPath = path.join(pathManager.getBackendDirPath(), AmplifyCategories.API, `${APIGW_AUTH_STACK_LOGICAL_ID}.json`); if (!cfn.Resources || Object.keys(cfn.Resources).length === 0) { @@ -244,10 +241,33 @@ function createApiGatewayAuthResources(context: $TSContext, stackName: string, a return `https://s3.amazonaws.com/${DeploymentBucketName}/amplify-cfn-templates/${S3_UPLOAD_PATH}`; } -export function loadApiCliInputs(name: string, resource: $TSObject): $TSObject | undefined { - if (resource.providerPlugin !== ProviderName || resource.service !== AmplifySupportedService.APIGW || name === 'AdminQueries') { +export async function loadApiCliInputs(context: $TSContext, resourceName: string, resource: $TSObject): Promise<$TSObject | undefined> { + if (resource.providerPlugin !== ProviderName || resource.service !== AmplifySupportedService.APIGW || resourceName === 'AdminQueries') { return; } - return stateManager.getResourceInputsJson(undefined, AmplifyCategories.API, name, { throwIfNotExist: false }); + const projectRoot = pathManager.findProjectRoot(); + + if (!stateManager.resourceInputsJsonExists(projectRoot, AmplifyCategories.API, resourceName)) { + const legacyParamsFilePath = path.join( + pathManager.getResourceDirectoryPath(projectRoot, AmplifyCategories.API, resourceName), + 'api-params.json', + ); + + if (fs.existsSync(legacyParamsFilePath)) { + // migration is required + context.migrationInfo = { projectPath: projectRoot }; + await context.amplify.invokePluginMethod(context, AmplifyCategories.API, AmplifySupportedService.APIGW, 'migrate', [ + resourceName, + AmplifySupportedService.APIGW, + ]); + + // answered no to migration + if (!stateManager.resourceInputsJsonExists(undefined, AmplifyCategories.API, resourceName)) { + exitOnNextTick(0); + } + } + } + + return stateManager.getResourceInputsJson(projectRoot, AmplifyCategories.API, resourceName, { throwIfNotExist: false }); } diff --git a/packages/amplify-provider-awscloudformation/src/utils/env-level-constructs.ts b/packages/amplify-provider-awscloudformation/src/utils/env-level-constructs.ts index 3ef30c53b20..2f24100f779 100644 --- a/packages/amplify-provider-awscloudformation/src/utils/env-level-constructs.ts +++ b/packages/amplify-provider-awscloudformation/src/utils/env-level-constructs.ts @@ -21,7 +21,7 @@ export async function createEnvLevelConstructs(context: $TSContext) { Object.assign( updatedMeta, await createNetworkResources(context, stackName, hasContainers), - consolidateApiGatewayPolicies(context, stackName), + await consolidateApiGatewayPolicies(context, stackName), await uploadAuthTriggerTemplate(context), );