From c9959e8297348e75d66cf493f13a80c093ea7ea5 Mon Sep 17 00:00:00 2001 From: Sachin Panemangalore Date: Fri, 5 Nov 2021 12:12:45 -0700 Subject: [PATCH 1/5] (fix) s3 cli-ux loop until permission selected --- .../service-walkthroughs/s3-questions.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-questions.ts b/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-questions.ts index 4f248d59c6b..8736ddac48e 100644 --- a/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-questions.ts +++ b/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-questions.ts @@ -150,7 +150,13 @@ export async function askCRUDQuestion( const message = `What kind of access do you want for ${userRole} users?`; const choices = possibleCRUDOperations; const initialIndexes = getIndexArrayByValue(possibleCRUDOperations, roleDefaultValues); - const selectedPermissions = await prompter.pick<'many', string>(message, choices, { returnSize: 'many', initial: initialIndexes }); + let selectedPermissions; + do { + selectedPermissions = await prompter.pick<'many', string>(message, choices, { returnSize: 'many', initial: initialIndexes }); + if( !selectedPermissions || selectedPermissions.length <= 0 ){ + printer.warn('Select at least one option'); + } + } while( !selectedPermissions || selectedPermissions.length <= 0 ); return selectedPermissions as Array; } From 6a5cdd169d3236053287b72848df39a3c15ccc76 Mon Sep 17 00:00:00 2001 From: Sachin Panemangalore Date: Fri, 5 Nov 2021 13:02:55 -0700 Subject: [PATCH 2/5] (fix) storage unit-tests for prompter function change --- .../service-walkthroughs/s3-walkthrough.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/amplify-category-storage/src/__tests__/provider-utils/awscloudformation/service-walkthroughs/s3-walkthrough.test.ts b/packages/amplify-category-storage/src/__tests__/provider-utils/awscloudformation/service-walkthroughs/s3-walkthrough.test.ts index 95f4b5fbce1..4c3991d622d 100644 --- a/packages/amplify-category-storage/src/__tests__/provider-utils/awscloudformation/service-walkthroughs/s3-walkthrough.test.ts +++ b/packages/amplify-category-storage/src/__tests__/provider-utils/awscloudformation/service-walkthroughs/s3-walkthrough.test.ts @@ -142,7 +142,7 @@ describe('add s3 walkthrough tests', () => { .mockResolvedValueOnce([S3PermissionType.CREATE_AND_UPDATE, S3PermissionType.READ, S3PermissionType.DELETE]) // What kind of permissions (Auth) - prompter.confirmContinue = jest + prompter.yesOrNo = jest .fn() .mockReturnValueOnce(true) //Do you want to add a Lambda Trigger ? .mockResolvedValueOnce(false); //Do you want to edit the lamdba function now? @@ -180,7 +180,7 @@ describe('add s3 walkthrough tests', () => { .mockResolvedValueOnce(S3TriggerFunctionType.EXISTING_FUNCTION) .mockResolvedValueOnce(S3MockDataBuilder.mockExistingFunctionName1 ); //Selected the First Existing function from the list. - prompter.confirmContinue = jest + prompter.yesOrNo = jest .fn() .mockReturnValueOnce(true) //Do you want to add a Lambda Trigger ? .mockResolvedValueOnce(false); //Do you want to edit the lamdba function now? From 67ce21a4835c4d494cddd53c415b2e84e5d364db Mon Sep 17 00:00:00 2001 From: Sachin Panemangalore Date: Fri, 5 Nov 2021 13:10:09 -0700 Subject: [PATCH 3/5] (fix) auto-migrate on headless s3 storage api --- .../service-walkthroughs/s3-resource-api.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-resource-api.ts b/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-resource-api.ts index bf264e8b346..7704325dd9a 100644 --- a/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-resource-api.ts +++ b/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-resource-api.ts @@ -2,7 +2,7 @@ import { $TSContext, AmplifyCategories, AmplifySupportedService, CLISubCommandTy import { AmplifyS3ResourceStackTransform } from '../cdk-stack-builder/s3-stack-transform'; import { S3UserInputTriggerFunctionParams, S3UserInputs } from '../service-walkthrough-types/s3-user-input-types'; import { S3InputState } from './s3-user-input-state'; -import { createNewLambdaAndUpdateCFN } from './s3-walkthrough'; +import { createNewLambdaAndUpdateCFN, migrateStorageCategory } from './s3-walkthrough'; /** * @returns Name of S3 resource or undefined @@ -28,7 +28,9 @@ export function s3GetResourceName(): string | undefined { * @returns */ export async function s3GetUserInput(context: $TSContext, s3ResourceName: string): Promise { - const cliInputsState = new S3InputState(s3ResourceName as string, undefined); + //migrate storage and fetch cliInputsState + await migrateStorageCategory(context, s3ResourceName); + let cliInputsState = new S3InputState(s3ResourceName as string, undefined); return cliInputsState.getUserInput(); } @@ -194,8 +196,12 @@ export async function addLambdaTrigger( /** HELPERS */ async function s3APIHelperTransformAndSaveState(context: $TSContext, storageInput: S3UserInputs, phase: CLISubCommandType) { + //migrate storage and fetch cliInputsState + await migrateStorageCategory(context, storageInput.resourceName as string); + //Save CLI Inputs payload let cliInputsState; + if ( phase === CLISubCommandType.ADD ){ cliInputsState = new S3InputState(storageInput.resourceName as string, storageInput); } else { From b7d7656ce93c5d3c0591698f5080d88d31060dc6 Mon Sep 17 00:00:00 2001 From: Sachin Panemangalore Date: Fri, 5 Nov 2021 15:13:00 -0700 Subject: [PATCH 4/5] (fix) enable migration in headless only if required --- .../service-walkthroughs/s3-resource-api.ts | 10 +++++++--- .../s3-user-input-state.ts | 19 +++++++++++++++++++ .../service-walkthroughs/s3-walkthrough.ts | 17 +++++++++++++++++ 3 files changed, 43 insertions(+), 3 deletions(-) diff --git a/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-resource-api.ts b/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-resource-api.ts index 7704325dd9a..6d895eba319 100644 --- a/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-resource-api.ts +++ b/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-resource-api.ts @@ -2,7 +2,7 @@ import { $TSContext, AmplifyCategories, AmplifySupportedService, CLISubCommandTy import { AmplifyS3ResourceStackTransform } from '../cdk-stack-builder/s3-stack-transform'; import { S3UserInputTriggerFunctionParams, S3UserInputs } from '../service-walkthrough-types/s3-user-input-types'; import { S3InputState } from './s3-user-input-state'; -import { createNewLambdaAndUpdateCFN, migrateStorageCategory } from './s3-walkthrough'; +import { createNewLambdaAndUpdateCFN, migrateStorageCategory, isMigrateStorageRequired } from './s3-walkthrough'; /** * @returns Name of S3 resource or undefined @@ -29,7 +29,9 @@ export function s3GetResourceName(): string | undefined { */ export async function s3GetUserInput(context: $TSContext, s3ResourceName: string): Promise { //migrate storage and fetch cliInputsState - await migrateStorageCategory(context, s3ResourceName); + if ( isMigrateStorageRequired(context, s3ResourceName) ){ + await migrateStorageCategory(context, s3ResourceName); + } let cliInputsState = new S3InputState(s3ResourceName as string, undefined); return cliInputsState.getUserInput(); } @@ -197,7 +199,9 @@ export async function addLambdaTrigger( /** HELPERS */ async function s3APIHelperTransformAndSaveState(context: $TSContext, storageInput: S3UserInputs, phase: CLISubCommandType) { //migrate storage and fetch cliInputsState - await migrateStorageCategory(context, storageInput.resourceName as string); + if ( phase != CLISubCommandType.ADD && isMigrateStorageRequired(context, storageInput.resourceName as string) ){ + await migrateStorageCategory(context, storageInput.resourceName as string); + } //Save CLI Inputs payload let cliInputsState; diff --git a/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-user-input-state.ts b/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-user-input-state.ts index 010acc0b7ec..fd19ee19c26 100644 --- a/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-user-input-state.ts +++ b/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-user-input-state.ts @@ -191,8 +191,27 @@ export class S3InputState { fs.removeSync(migrationParams.storageParamsFilepath); } } + public checkNeedsMigration():boolean{ + const backendDir = pathManager.getBackendDirPath(); + const oldParametersFilepath = path.join(backendDir, AmplifyCategories.STORAGE, this._resourceName, 'parameters.json'); + const oldCFNFilepath = path.join( + backendDir, + AmplifyCategories.STORAGE, + this._resourceName, + `${AmplifySupportedService.S3}-cloudformation-template.json`, + ); + if ( fs.existsSync(oldParametersFilepath) && fs.existsSync(oldCFNFilepath) ){ + return true; + }else { + return false; + } + } public async migrate(context: $TSContext) { + //check if migration is possible + if (!this.checkNeedsMigration() ){ + return; + } try { await migrateAuthDependencyResource(context); } catch (error) { diff --git a/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-walkthrough.ts b/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-walkthrough.ts index 6a9a659fb3f..e96b1a12fce 100644 --- a/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-walkthrough.ts +++ b/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-walkthrough.ts @@ -12,6 +12,7 @@ import { CLISubCommandType, AmplifySupportedService, $TSMeta, + pathManager, } from 'amplify-cli-core'; import { S3InputState } from './s3-user-input-state'; import { S3UserInputs, S3TriggerFunctionType } from '../service-walkthrough-types/s3-user-input-types'; @@ -179,6 +180,22 @@ export async function updateWalkthrough(context: $TSContext) { } } +/** + * Check if Storage feature needs migration + * @param context + * @param resourceName - storage resource name + * @returns + */ +export function isMigrateStorageRequired(context : $TSContext, resourceName: string){ + const projectBackendDirPath = pathManager.getBackendDirPath(); + const cliInputsFilePath = path.resolve(path.join(projectBackendDirPath, AmplifyCategories.STORAGE, resourceName, 'cli-inputs.json')); + if ( !fs.existsSync(cliInputsFilePath) ){ + return true; + }else { + return false; + } +} + /** * Migrate workflow for S3 resource * - converts old context files into cliInputs and transforms into cloudformation. From 9d6300d5c86b9f8b700107599594efb8fb51bd01 Mon Sep 17 00:00:00 2001 From: Sachin Panemangalore Date: Fri, 5 Nov 2021 23:25:39 -0700 Subject: [PATCH 5/5] (fix) unit-test failing because of path incorrect --- .../service-walkthroughs/s3-walkthrough.test.ts | 8 +++++--- .../service-walkthroughs/s3-questions.ts | 2 +- .../service-walkthroughs/s3-resource-api.ts | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/amplify-category-storage/src/__tests__/provider-utils/awscloudformation/service-walkthroughs/s3-walkthrough.test.ts b/packages/amplify-category-storage/src/__tests__/provider-utils/awscloudformation/service-walkthroughs/s3-walkthrough.test.ts index 4c3991d622d..077d085b71d 100644 --- a/packages/amplify-category-storage/src/__tests__/provider-utils/awscloudformation/service-walkthroughs/s3-walkthrough.test.ts +++ b/packages/amplify-category-storage/src/__tests__/provider-utils/awscloudformation/service-walkthroughs/s3-walkthrough.test.ts @@ -1,17 +1,17 @@ import { $TSAny, $TSContext, AmplifySupportedService, stateManager } from 'amplify-cli-core'; import { prompter } from 'amplify-prompts'; import * as uuid from 'uuid'; -import { MigrationParams, S3InputState } from '../../../../provider-utils/awscloudformation/service-walkthroughs/s3-user-input-state'; import { AmplifyS3ResourceStackTransform } from '../../../../provider-utils/awscloudformation/cdk-stack-builder/s3-stack-transform'; -import { addWalkthrough, updateWalkthrough } from '../../../../provider-utils/awscloudformation/service-walkthroughs/s3-walkthrough'; import { S3AccessType, S3PermissionType, S3TriggerFunctionType, - S3UserInputs, + S3UserInputs } from '../../../../provider-utils/awscloudformation/service-walkthrough-types/s3-user-input-types'; import * as s3AuthAPI from '../../../../provider-utils/awscloudformation/service-walkthroughs/s3-auth-api'; import { S3CLITriggerUpdateMenuOptions, UserPermissionTypeOptions } from '../../../../provider-utils/awscloudformation/service-walkthroughs/s3-questions'; +import { MigrationParams, S3InputState } from '../../../../provider-utils/awscloudformation/service-walkthroughs/s3-user-input-state'; +import { addWalkthrough, updateWalkthrough } from '../../../../provider-utils/awscloudformation/service-walkthroughs/s3-walkthrough'; jest.mock('amplify-cli-core'); jest.mock('amplify-prompts'); @@ -19,6 +19,8 @@ jest.mock('../../../../provider-utils/awscloudformation/service-walkthroughs/s3- jest.mock('../../../../provider-utils/awscloudformation/cdk-stack-builder/s3-stack-transform'); jest.mock('../../../../provider-utils/awscloudformation/service-walkthroughs/s3-auth-api'); jest.mock('uuid'); +jest.mock('path'); +jest.mock('fs-extra'); describe('add s3 walkthrough tests', () => { let mockContext: $TSContext; diff --git a/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-questions.ts b/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-questions.ts index 8736ddac48e..ac14e12571a 100644 --- a/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-questions.ts +++ b/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-questions.ts @@ -209,7 +209,7 @@ export async function askUpdateTriggerSelection( return triggerOperationAnswer as S3CLITriggerUpdateMenuOptions; } -export async function askAuthPermissionQuestion(context: $TSContext, defaultValues: S3UserInputs) { +export async function askAuthPermissionQuestion(context: $TSContext, defaultValues: S3UserInputs): Promise { const permissions: S3PermissionType[] = await askCRUDQuestion(S3UserAccessRole.AUTH, undefined, context, defaultValues); return permissions; } diff --git a/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-resource-api.ts b/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-resource-api.ts index 6d895eba319..6d18f7d7460 100644 --- a/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-resource-api.ts +++ b/packages/amplify-category-storage/src/provider-utils/awscloudformation/service-walkthroughs/s3-resource-api.ts @@ -41,7 +41,7 @@ export async function s3GetUserInput(context: $TSContext, s3ResourceName: string * @param context * @returns triggerFunction name or undefined */ -export async function s3GetAdminTriggerFunctionName(context: $TSContext){ +export async function s3GetAdminTriggerFunctionName(context: $TSContext) : Promise { const s3ResourceName : string|undefined = await s3GetResourceName(); const s3UserInput :S3UserInputs | undefined = (s3ResourceName)?await s3GetUserInput(context , s3ResourceName ):undefined; return s3UserInput?.adminTriggerFunction?.triggerFunction;