Skip to content

Commit

Permalink
fix: migrate rest apis with protected routes on push (#9068)
Browse files Browse the repository at this point in the history
* fix: migrate rest apis with protected routes on push

* fix: gracefully exit after 'no' to REST API migration
  • Loading branch information
jhockett authored Nov 29, 2021
1 parent cb9fdd3 commit 62b4436
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 41 deletions.
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
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');
jest.mock('fs-extra');
jest.mock('path');
jest.mock('../../../provider-utils/awscloudformation/cdk-stack-builder');

const fs_mock = fs as jest.Mocked<typeof fs>;
const JSONUtilities_mock = JSONUtilities as jest.Mocked<typeof JSONUtilities>;
const pathManager_mock = pathManager as jest.Mocked<typeof pathManager>;
const prompter_mock = prompter as jest.Mocked<typeof prompter>;
Expand All @@ -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');
Expand Down Expand Up @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand Down
4 changes: 2 additions & 2 deletions packages/amplify-category-api/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};
Expand Down Expand Up @@ -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();
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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 {
Expand All @@ -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) {
Expand All @@ -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 });
}
Original file line number Diff line number Diff line change
Expand Up @@ -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),
);

Expand Down

0 comments on commit 62b4436

Please sign in to comment.