Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Custom policies IAM Policies for Lambda and Containers #8068

Merged
merged 27 commits into from
Oct 4, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
fc5fef8
Custom policy implementation
luhanamz Aug 12, 2021
85f7eea
feat: add custom policies file to function and API container
luhanamz Aug 16, 2021
5ae975a
feat: changes for first PR
luhanamz Aug 16, 2021
c6ceb2f
feat: Some changes according to the PR comments
luhanamz Aug 17, 2021
8ddd755
feat: replace env to current env in the resource when checkout and ad…
luhanamz Aug 20, 2021
8966a7d
feat: e2e test and replacing env
luhanamz Aug 26, 2021
83e4155
feat: Minor changes for env replacement
luhanamz Aug 27, 2021
048b1e5
feat: remove changing env between env
luhanamz Aug 27, 2021
a2cd9fa
feat: Add cloudform type for type safety, move validation to provider…
luhanamz Aug 27, 2021
b7038fe
feat: remove some unused function and import, change regex for resource
luhanamz Aug 30, 2021
1f7efc7
feat: Some changes according to the PR comment
luhanamz Aug 30, 2021
c3f4d05
feat: changes according to PR comments
luhanamz Aug 31, 2021
41f242d
feat: remove unused import
luhanamz Aug 31, 2021
1bb1a42
feat: remove previous unused code
luhanamz Aug 31, 2021
28dab7d
feat: Changes according to PR comments
luhanamz Aug 31, 2021
bb85696
feat: some changes according to PR comments
luhanamz Sep 1, 2021
cac4100
feat: work on PR comments
luhanamz Sep 3, 2021
d1da5e8
feat: rebase for conflict
luhanamz Sep 3, 2021
5f9bb10
feat: rebase for failure of hooksmanager test failed
luhanamz Sep 4, 2021
0aa1933
feat: unit test
luhanamz Sep 7, 2021
8a8268c
feat: fix fail test
luhanamz Sep 7, 2021
453b8b2
feat: change default template of custom policies
luhanamz Sep 8, 2021
d9ed2e4
feat: fix failed test
luhanamz Sep 8, 2021
b7dda25
feat: PR comments
luhanamz Sep 9, 2021
c0c01d5
feat: pr comments
luhanamz Sep 9, 2021
64096b9
feat: fix failed test
luhanamz Sep 9, 2021
22c178e
feat: PR comments from ED
luhanamz Sep 9, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { DEPLOYMENT_MECHANISM } from './base-api-stack';
import { GitHubSourceActionInfo } from './pipeline-with-awaiter';
import { API_TYPE, IMAGE_SOURCE_TYPE, ResourceDependency, ServiceConfiguration } from './service-walkthroughs/containers-walkthrough';
import { ApiResource, generateContainersArtifacts } from './utils/containers-artifacts';
import { addCustomPoliciesFile, pathManager } from "amplify-cli-core";
import { createDefaultCustomPoliciesFile, pathManager } from 'amplify-cli-core';

export const addResource = async (
serviceWalkthroughPromise: Promise<ServiceConfiguration>,
Expand Down Expand Up @@ -97,7 +97,7 @@ export const addResource = async (

}

addCustomPoliciesFile(category, resourceName);
createDefaultCustomPoliciesFile(category, resourceName);

const customPoliciesPath = pathManager.getCustomPoliciesPath(category, resourceName);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import {
saveMutableState,
updateLayerArtifacts,
} from './utils/storeResources';
import {addCustomPoliciesFile} from "amplify-cli-core";
import { createDefaultCustomPoliciesFile } from 'amplify-cli-core';

/**
* Entry point for creating a new function
Expand Down Expand Up @@ -113,15 +113,14 @@ export async function addFunctionResource(

await createFunctionResources(context, completeParams);

addCustomPoliciesFile(category, completeParams.resourceName);
createDefaultCustomPoliciesFile(category, completeParams.resourceName);

if (!completeParams.skipEdit) {
await openEditor(context, category, completeParams.resourceName, completeParams.functionTemplate);
}

const { print } = context;


const customPoliciesPath = pathManager.getCustomPoliciesPath(category, completeParams.resourceName);

print.success(`Successfully added resource ${completeParams.resourceName} locally.`);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {addCustomPoliciesFile} from "../customPoliciesUtils"
import { JSONUtilities } from "..";
import { pathManager, PathConstants } from "../state-manager";
import * as path from 'path';
import {createDefaultCustomPoliciesFile} from '../customPoliciesUtils'
import { JSONUtilities } from '..';
import { pathManager, PathConstants } from '../state-manager';
import path from 'path';

describe('Custom policies util test', () => {

Expand All @@ -16,7 +16,7 @@ describe('Custom policies util test', () => {

test('Write default custom policy file to the specified resource name', () => {

addCustomPoliciesFile(testCategoryName, testResourceName);
createDefaultCustomPoliciesFile(testCategoryName, testResourceName);

const data = JSONUtilities.readJson(expectedFilePath);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
{
"policies": [
{
"Action": [],
"Resource": []
}
]
}
[
{
"Action": [],
"Resource": []
}
]
2 changes: 1 addition & 1 deletion packages/amplify-cli-core/src/cfnUtilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ const CF_SCHEMA = yaml.JSON_SCHEMA.extend([
}),
]);

export function isJsonFileContent(fileContent: string): boolean {
function isJsonFileContent(fileContent: string): boolean {
// We use the first character to determine if the content is json or yaml because historically the CLI could
// have emitted JSON with YML extension, so we can't rely on filename extension.
return fileContent?.trim()[0] === '{'; // CFN templates are always objects, never arrays
Expand Down
49 changes: 18 additions & 31 deletions packages/amplify-cli-core/src/customPoliciesUtils.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import { Fn, IAM } from "cloudform-types";
import * as iam from '@aws-cdk/aws-iam';
import { JSONUtilities, pathManager } from ".";
import * as fs from 'fs-extra';
import { Fn, IAM } from 'cloudform-types';
import { JSONUtilities, pathManager } from '.';

export type CustomIAMPolicies = CustomIAMPolicy[];

Expand All @@ -11,23 +9,22 @@ export type CustomIAMPolicy = {
Resource: string[];
}


export const CustomIAMPoliciesSchema = {
type : "array",
minItems: 1,
items: {type: "object"},
additionalProperties: false
}

export const CustomIAMPolicySchema = {
type: "object",
properties: {
Action: {type: "array", items: {type: "string"}, minItems: 1, nullable: false},
Effect: {type: "string", nullable: true, default: iam.Effect.ALLOW},
Resource: {type: "array", items: {type: "string"}, minItems: 1, nullable: false},
},
required: ["Resource", "Action"],
additionalProperties: false
type : 'array',
minItems: 1,
items: {
type: 'object',
properties: {
Action: { type: 'array', items: { type: 'string' }, minItems: 1, nullable: false },
Resource: { type: 'array', items: { type: 'string' }, minItems: 1, nullable: false }
},
optionalProperties: {
Effect: { type: 'string', enum:['Allow', 'Deny'], default: 'Allow' },
},
required: ['Resource', 'Action'],
additionalProperties: true
},
additionalProperties: false
}

export const customExecutionPolicyForFunction = new IAM.Policy({
Expand All @@ -52,7 +49,7 @@ export const customExecutionPolicyForContainer = new IAM.Policy({
]
});

export function addCustomPoliciesFile(categoryName: string, resourceName: string) {
export function createDefaultCustomPoliciesFile(categoryName: string, resourceName: string) {
const customPoliciesPath = pathManager.getCustomPoliciesPath(categoryName, resourceName);
const defaultCustomPolicies = [
{
Expand All @@ -63,15 +60,5 @@ export function addCustomPoliciesFile(categoryName: string, resourceName: string
JSONUtilities.writeJson(customPoliciesPath, defaultCustomPolicies);
edwardfoyle marked this conversation as resolved.
Show resolved Hide resolved
}

export function isCustomPoliciesFile(filePath: string) {
try{
const fileString = fs.readFileSync(filePath, 'utf-8');
JSON.parse(fileString);
return true;
} catch(err) {
return false;
}
}



2 changes: 1 addition & 1 deletion packages/amplify-cli-core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export * from './cliGetCategories';
export * from './cliRemoveResourcePrompt';
export * from './cliViewAPI';
export * from './hooks';
export * from "./cliViewAPI";
export * from './cliViewAPI';
export * from './customPoliciesUtils'

// Temporary types until we can finish full type definition across the whole CLI
Expand Down
9 changes: 5 additions & 4 deletions packages/amplify-cli-core/src/state-manager/stateManager.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import * as fs from 'fs-extra';
import * as path from 'path';
import _ from 'lodash';
import { PathConstants, pathManager } from './pathManager';
import { $TSMeta, $TSTeamProviderInfo, $TSAny, DeploymentSecrets, HooksConfig } from '..';
import { JSONUtilities } from '../jsonUtilities';
import { SecretFileMode } from '../cliConstants';
import { HydrateTags, ReadTags, Tag } from '../tags';
import { CustomIAMPolicies, isCustomPoliciesFile } from '../customPoliciesUtils';
import path from 'path';
import { CustomIAMPolicies } from '../customPoliciesUtils';

export type GetOptions<T> = {
throwIfNotExist?: boolean;
Expand Down Expand Up @@ -79,10 +79,11 @@ export class StateManager {

getCustomPolicies = (categoryName: string, resourceName: string): CustomIAMPolicies | undefined => {
const filePath = pathManager.getCustomPoliciesPath(categoryName, resourceName);
if (!(fs.existsSync(filePath)) || !isCustomPoliciesFile(filePath)) {
try{
return JSONUtilities.readJson<CustomIAMPolicies>(filePath);
} catch(err) {
edwardfoyle marked this conversation as resolved.
Show resolved Hide resolved
return undefined;
}
return JSONUtilities.readJson<CustomIAMPolicies>(filePath);
};

localEnvInfoExists = (projectPath?: string): boolean => this.doesExist(pathManager.getLocalEnvFilePath, projectPath);
Expand Down
4 changes: 2 additions & 2 deletions packages/amplify-e2e-core/src/categories/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -567,7 +567,7 @@ export function addRestContainerApi(projectDir: string) {
});
}

export function addRestContainerApiForCustomPolicies(projectDir: string) {
export function addRestContainerApiForCustomPolicies(projectDir: string, settings: { name: string }) {
return new Promise<void>((resolve, reject) => {
spawn(getCLIPath(), ['add', 'api'], { cwd: projectDir, stripColors: true })
.wait('Please select from one of the below mentioned services:')
Expand All @@ -577,7 +577,7 @@ export function addRestContainerApiForCustomPolicies(projectDir: string) {
.sendKeyDown()
.sendCarriageReturn()
.wait('Provide a friendly name for your resource to be used as a label for this category in the project:')
.send('containertest')
.send(settings.name)
.sendCarriageReturn()
.wait('What image would you like to use')
.sendKeyDown()
Expand Down
2 changes: 1 addition & 1 deletion packages/amplify-e2e-core/src/utils/projectMeta.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ function getProjectMeta(projectRoot: string) {
return JSON.parse(fs.readFileSync(metaFilePath, 'utf8'));
}
function getCustomPoliciesPath(projectRoot: string, category: string, resourceName: string): string {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be just used from pathmanager in core?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it failed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I will have to take a look at it

return path.join(projectRoot, 'amplify', 'backend', category, resourceName, 'custom-iam-policy-documents.json');
return path.join(projectRoot, 'amplify', 'backend', category, resourceName, 'custom-policies.json');
}

function getProjectTags(projectRoot: string) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ it(`should init and deploy a api container, attach custom policies to the Fargat
const name = 'containertest';
await initJSProjectWithProfile(projRoot, { name: containerName, envName });
await setupAmplifyProject(projRoot);
await addRestContainerApiForCustomPolicies(projRoot);
await addRestContainerApiForCustomPolicies(projRoot, { name: name });

const meta = getProjectMeta(projRoot);
const { Region: region } = meta?.providers?.awscloudformation;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@ const customPolicies: CustomIAMPolicies = [
Action : ['test:test'],
Effect : 'Allow',
Resource : ['arn:aws:s3:us-east-2:012345678910:testResource']
}
];
}];

readCFNTemplate_mock.mockResolvedValue({
templateFormat,
Expand All @@ -39,6 +38,7 @@ const backendPath = '/project/amplify/backend';
const resourcePath = 'api/resourceName/cfn-template-name.json';

pathManager_mock.getBackendDirPath.mockReturnValue(backendPath);
pathManager_mock.getResourceDirectoryPath.mockReturnValue(backendPath);
stateManager_mock.getCustomPolicies.mockReturnValue(customPolicies);
stateManager_mock.getLocalEnvInfo.mockReturnValue({envName:'test'});

Expand Down Expand Up @@ -68,8 +68,8 @@ describe('preProcessCFNTemplate', () => {
it('writes valid custom policies to cfn template', async () => {
const cfnTemplate = ({
Resources: {
"LambdaExecutionRole": {
"Type": "AWS::IAM::Role"
LambdaExecutionRole: {
Type: 'AWS::IAM::Role'
}
},
} as unknown) as Template;
Expand All @@ -79,7 +79,7 @@ describe('preProcessCFNTemplate', () => {
cfnTemplate,
});

await writeCustomPoliciesToCFNTemplate('test', 'Lambda', backendPath, 'cfn-template.json', 'test', path.join(backendPath, resourcePath));
await writeCustomPoliciesToCFNTemplate('test', 'Lambda', 'cfn-template.json', 'test');

const templateWithCustomPolicies = writeCFNTemplate_mock.mock.calls[0][0] as any;

Expand Down
Loading