From 57fe47dba5e595806a1c092d4eaea32da257731c Mon Sep 17 00:00:00 2001 From: MURAKAMI Masahiko Date: Tue, 24 Aug 2021 09:25:21 +0900 Subject: [PATCH 1/3] fix(amplify-category-function): check for new function when adding permissions it is handled as new function when function not found in amplify meta fix #7970 --- .../execPermissionsWalkthrough.test.ts.snap | 124 +++++++++++++++ .../execPermissionsWalkthrough.test.ts | 149 ++++++++++++++++++ .../execPermissionsWalkthrough.ts | 8 +- 3 files changed, 278 insertions(+), 3 deletions(-) diff --git a/packages/amplify-category-function/src/__tests__/provider-utils/awscloudformation/service-walkthroughs/__snapshots__/execPermissionsWalkthrough.test.ts.snap b/packages/amplify-category-function/src/__tests__/provider-utils/awscloudformation/service-walkthroughs/__snapshots__/execPermissionsWalkthrough.test.ts.snap index bfaa4aa1690..1571565c76b 100644 --- a/packages/amplify-category-function/src/__tests__/provider-utils/awscloudformation/service-walkthroughs/__snapshots__/execPermissionsWalkthrough.test.ts.snap +++ b/packages/amplify-category-function/src/__tests__/provider-utils/awscloudformation/service-walkthroughs/__snapshots__/execPermissionsWalkthrough.test.ts.snap @@ -1,5 +1,129 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`askExecRolePermissionsQuestions returns permissions for exists function 1`] = ` +Object { + "categoryPolicies": Array [ + Object { + "Action": Array [ + "lambda:Get*", + "lambda:List*", + "lambda:Invoke*", + ], + "Effect": "Allow", + "Resource": Array [ + Object { + "Fn::Join": Array [ + "", + Array [ + "arn:aws:lambda:", + Object { + "Ref": "AWS::Region", + }, + ":", + Object { + "Ref": "AWS::AccountId", + }, + ":function:", + Object { + "Ref": "functionlambda2Name", + }, + ], + ], + }, + ], + }, + ], + "dependsOn": Array [ + Object { + "attributes": Array [ + "Name", + ], + "category": "function", + "resourceName": "lambda2", + }, + ], + "environmentMap": Object { + "FUNCTION_LAMBDA2_NAME": Object { + "Ref": "functionlambda2Name", + }, + }, + "mutableParametersState": Object { + "permissions": Object { + "function": Object { + "lambda2": Array [ + "read", + ], + }, + }, + }, + "topLevelComment": "/* Amplify Params - DO NOT EDIT + FUNCTION_LAMBDA2_NAME +Amplify Params - DO NOT EDIT */", +} +`; + +exports[`askExecRolePermissionsQuestions returns permissions for function that be about to add right 1`] = ` +Object { + "categoryPolicies": Array [ + Object { + "Action": Array [ + "lambda:Get*", + "lambda:List*", + "lambda:Invoke*", + ], + "Effect": "Allow", + "Resource": Array [ + Object { + "Fn::Join": Array [ + "", + Array [ + "arn:aws:lambda:", + Object { + "Ref": "AWS::Region", + }, + ":", + Object { + "Ref": "AWS::AccountId", + }, + ":function:", + Object { + "Ref": "functionlambda2Name", + }, + ], + ], + }, + ], + }, + ], + "dependsOn": Array [ + Object { + "attributes": Array [ + "Name", + ], + "category": "function", + "resourceName": "lambda2", + }, + ], + "environmentMap": Object { + "FUNCTION_LAMBDA2_NAME": Object { + "Ref": "functionlambda2Name", + }, + }, + "mutableParametersState": Object { + "permissions": Object { + "function": Object { + "lambda2": Array [ + "read", + ], + }, + }, + }, + "topLevelComment": "/* Amplify Params - DO NOT EDIT + FUNCTION_LAMBDA2_NAME +Amplify Params - DO NOT EDIT */", +} +`; + exports[`check CFN resources 1`] = ` Object { "cfnResources": Array [ diff --git a/packages/amplify-category-function/src/__tests__/provider-utils/awscloudformation/service-walkthroughs/execPermissionsWalkthrough.test.ts b/packages/amplify-category-function/src/__tests__/provider-utils/awscloudformation/service-walkthroughs/execPermissionsWalkthrough.test.ts index 71db2e728bc..3efe424687b 100644 --- a/packages/amplify-category-function/src/__tests__/provider-utils/awscloudformation/service-walkthroughs/execPermissionsWalkthrough.test.ts +++ b/packages/amplify-category-function/src/__tests__/provider-utils/awscloudformation/service-walkthroughs/execPermissionsWalkthrough.test.ts @@ -1,14 +1,39 @@ +import { $TSContext } from 'amplify-cli-core'; import { getResourcesForCfn, generateEnvVariablesForCfn, + askExecRolePermissionsQuestions, } from '../../../../provider-utils/awscloudformation/service-walkthroughs/execPermissionsWalkthrough'; import { constructCFModelTableNameComponent, constructCFModelTableArnComponent, } from '../../../../provider-utils/awscloudformation/utils/cloudformationHelpers'; +import { stateManager } from 'amplify-cli-core'; +import { CRUDOperation } from '../../../../constants'; +import inquirer from 'inquirer'; + +const backendDirPathStub = 'backendDirPath'; jest.mock('../../../../provider-utils/awscloudformation/utils/cloudformationHelpers'); +jest.mock('amplify-cli-core', () => ({ + stateManager: { + getMeta: jest.fn(), + }, + FeatureFlags: { + getBoolean: jest.fn().mockReturnValue(false), + }, + pathManager: { + getBackendDirPath: jest.fn(() => backendDirPathStub), + }, +})); + +jest.mock('inquirer', () => { + return { + prompt: jest.fn(), + }; +}); + export const appsyncTableSuffix = '@model(appsync)'; const constructCFModelTableNameComponent_mock = constructCFModelTableNameComponent as jest.MockedFunction< @@ -103,3 +128,127 @@ test('env resources for CFN for auth and storage for api', async () => { ]; expect(await generateEnvVariablesForCfn(contextStub, resources, {})).toMatchSnapshot(); }); + +describe('askExecRolePermissionsQuestions', () => { + beforeEach(() => { + const stateManagerMock = stateManager as jest.Mocked; + stateManagerMock.getMeta.mockReturnValue({ + providers: { + awscloudformation: {}, + }, + function: { + lambda1: { + service: 'Lambda', + providerPlugin: 'awscloudformation', + }, + lambda2: { + service: 'Lambda', + providerPlugin: 'awscloudformation', + lastPushTimeStamp: '2021-07-12T00:41:17.966Z', + }, + }, + auth: { + authResourceName: { + service: 'Cognito', + serviceType: 'imported', + providerPlugin: 'awscloudformation', + }, + }, + storage: { + s3Bucket: { + service: 'S3', + serviceType: 'imported', + providerPlugin: 'awscloudformation', + }, + }, + }); + }); + + it('returns permissions for function that be about to add right', async () => { + const inquirerMock = inquirer as jest.Mocked; + + inquirerMock.prompt.mockResolvedValueOnce({ categories: ['function'] }); + inquirerMock.prompt.mockResolvedValueOnce({ resources: ['lambda2'] }); + inquirerMock.prompt.mockResolvedValueOnce({ options: [CRUDOperation.READ] }); + + const resourceAttributes = [ + { + attributes: ['Name'], + category: 'function', + resourceName: 'lambda2', + }, + ]; + const permissionPolicies = [ + { + Action: ['lambda:Get*', 'lambda:List*', 'lambda:Invoke*'], + Effect: 'Allow', + Resource: [ + { + 'Fn::Join': [ + '', + ['arn:aws:lambda:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':function:', { Ref: 'functionlambda2Name' }], + ], + }, + ], + }, + ]; + const contextStub = { + print: { + warning: jest.fn(), + info: jest.fn(), + }, + usageData: { + emitError: jest.fn(), + }, + amplify: { + invokePluginMethod: jest.fn().mockResolvedValueOnce({ permissionPolicies, resourceAttributes }), + }, + }; + const answers = await askExecRolePermissionsQuestions((contextStub as unknown) as $TSContext, 'lambda3', undefined); + expect(answers).toMatchSnapshot(); + }); + + it('returns permissions for exists function', async () => { + const inquirerMock = inquirer as jest.Mocked; + + inquirerMock.prompt.mockResolvedValueOnce({ categories: ['function'] }); + //inquirerMock.prompt.mockResolvedValueOnce({ resources: ['lambda2'] }); + inquirerMock.prompt.mockResolvedValueOnce({ options: [CRUDOperation.READ] }); + + const resourceAttributes = [ + { + attributes: ['Name'], + category: 'function', + resourceName: 'lambda2', + }, + ]; + const permissionPolicies = [ + { + Action: ['lambda:Get*', 'lambda:List*', 'lambda:Invoke*'], + Effect: 'Allow', + Resource: [ + { + 'Fn::Join': [ + '', + ['arn:aws:lambda:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':function:', { Ref: 'functionlambda2Name' }], + ], + }, + ], + }, + ]; + const contextStub = { + print: { + warning: jest.fn(), + info: jest.fn(), + }, + usageData: { + emitError: jest.fn(), + }, + amplify: { + invokePluginMethod: jest.fn().mockResolvedValueOnce({ permissionPolicies, resourceAttributes }), + }, + }; + const answers = await askExecRolePermissionsQuestions((contextStub as unknown) as $TSContext, 'lambda1', undefined); + expect(answers).toMatchSnapshot(); + }); +}); diff --git a/packages/amplify-category-function/src/provider-utils/awscloudformation/service-walkthroughs/execPermissionsWalkthrough.ts b/packages/amplify-category-function/src/provider-utils/awscloudformation/service-walkthroughs/execPermissionsWalkthrough.ts index 70fd8d3d42e..3b084eca19b 100644 --- a/packages/amplify-category-function/src/provider-utils/awscloudformation/service-walkthroughs/execPermissionsWalkthrough.ts +++ b/packages/amplify-category-function/src/provider-utils/awscloudformation/service-walkthroughs/execPermissionsWalkthrough.ts @@ -80,12 +80,14 @@ export const askExecRolePermissionsQuestions = async ( // A Lambda function cannot depend on itself // Lambda layer dependencies are handled seperately, also apply the filter if the selected resource is within the function category // but serviceName argument was no passed in - const selectedResource = _.get(amplifyMeta, [categoryName, resourceNameToUpdate]); - if (serviceName === ServiceName.LambdaFunction || selectedCategory === categoryName) { + const selectedResource = _.get(amplifyMeta, [categoryName, resourceNameToUpdate]); + // A new function resource is not exsits in amplifyMeta yet + const isNewFunctionResource = !selectedResource; resourcesList = resourcesList.filter( resourceName => - resourceName !== resourceNameToUpdate && amplifyMeta[selectedCategory][resourceName].service === selectedResource.service, + resourceName !== resourceNameToUpdate && + (isNewFunctionResource || amplifyMeta[selectedCategory][resourceName].service === selectedResource.service), ); } else { resourcesList = resourcesList.filter( From 3cbff336cc4fab3b53a1bcd31892ea48f378ca98 Mon Sep 17 00:00:00 2001 From: MURAKAMI Masahiko Date: Wed, 25 Aug 2021 08:49:54 +0900 Subject: [PATCH 2/3] test(amplify-category-function): remove unnecessary line --- .../service-walkthroughs/execPermissionsWalkthrough.test.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/amplify-category-function/src/__tests__/provider-utils/awscloudformation/service-walkthroughs/execPermissionsWalkthrough.test.ts b/packages/amplify-category-function/src/__tests__/provider-utils/awscloudformation/service-walkthroughs/execPermissionsWalkthrough.test.ts index 3efe424687b..decb18f4712 100644 --- a/packages/amplify-category-function/src/__tests__/provider-utils/awscloudformation/service-walkthroughs/execPermissionsWalkthrough.test.ts +++ b/packages/amplify-category-function/src/__tests__/provider-utils/awscloudformation/service-walkthroughs/execPermissionsWalkthrough.test.ts @@ -204,7 +204,7 @@ describe('askExecRolePermissionsQuestions', () => { invokePluginMethod: jest.fn().mockResolvedValueOnce({ permissionPolicies, resourceAttributes }), }, }; - const answers = await askExecRolePermissionsQuestions((contextStub as unknown) as $TSContext, 'lambda3', undefined); + const answers = await askExecRolePermissionsQuestions(contextStub as unknown as $TSContext, 'lambda3', undefined); expect(answers).toMatchSnapshot(); }); @@ -212,7 +212,6 @@ describe('askExecRolePermissionsQuestions', () => { const inquirerMock = inquirer as jest.Mocked; inquirerMock.prompt.mockResolvedValueOnce({ categories: ['function'] }); - //inquirerMock.prompt.mockResolvedValueOnce({ resources: ['lambda2'] }); inquirerMock.prompt.mockResolvedValueOnce({ options: [CRUDOperation.READ] }); const resourceAttributes = [ @@ -248,7 +247,7 @@ describe('askExecRolePermissionsQuestions', () => { invokePluginMethod: jest.fn().mockResolvedValueOnce({ permissionPolicies, resourceAttributes }), }, }; - const answers = await askExecRolePermissionsQuestions((contextStub as unknown) as $TSContext, 'lambda1', undefined); + const answers = await askExecRolePermissionsQuestions(contextStub as unknown as $TSContext, 'lambda1', undefined); expect(answers).toMatchSnapshot(); }); }); From 7fb434334b31e5291352261b13c1afe11b134799 Mon Sep 17 00:00:00 2001 From: MURAKAMI Masahiko Date: Wed, 25 Aug 2021 08:51:47 +0900 Subject: [PATCH 3/3] Update packages/amplify-category-function/src/provider-utils/awscloudformation/service-walkthroughs/execPermissionsWalkthrough.ts Co-authored-by: John Hockett --- .../service-walkthroughs/execPermissionsWalkthrough.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/amplify-category-function/src/provider-utils/awscloudformation/service-walkthroughs/execPermissionsWalkthrough.ts b/packages/amplify-category-function/src/provider-utils/awscloudformation/service-walkthroughs/execPermissionsWalkthrough.ts index 3b084eca19b..a04fc68bc2b 100644 --- a/packages/amplify-category-function/src/provider-utils/awscloudformation/service-walkthroughs/execPermissionsWalkthrough.ts +++ b/packages/amplify-category-function/src/provider-utils/awscloudformation/service-walkthroughs/execPermissionsWalkthrough.ts @@ -82,7 +82,7 @@ export const askExecRolePermissionsQuestions = async ( // but serviceName argument was no passed in if (serviceName === ServiceName.LambdaFunction || selectedCategory === categoryName) { const selectedResource = _.get(amplifyMeta, [categoryName, resourceNameToUpdate]); - // A new function resource is not exsits in amplifyMeta yet + // A new function resource does not exist in amplifyMeta yet const isNewFunctionResource = !selectedResource; resourcesList = resourcesList.filter( resourceName =>