From 7816d52bdaa83a3177c37fa418bd26918dee5c25 Mon Sep 17 00:00:00 2001 From: Toshimitsu Takahashi Date: Sat, 13 Jul 2019 19:17:32 +0900 Subject: [PATCH 1/7] Upgrade policy specification V2 --- README.md | 30 ++++++++++++-------- example/policies/bar-logs-lambda-ENV.json | 14 --------- example/policies/bar-logs-lambda.json | 20 +++++++++++++ example/policies/baz-dynamodb-items-ENV.json | 18 ------------ example/policies/baz-dynamodb-items.json | 24 ++++++++++++++++ example/policies/foo-s3-logs-ENV.json | 14 --------- example/policies/foo-s3-logs.json | 20 +++++++++++++ example/policies/foo-s3-storage-ENV.json | 14 --------- example/policies/foo-s3-storage.json | 20 +++++++++++++ 9 files changed, 102 insertions(+), 72 deletions(-) delete mode 100644 example/policies/bar-logs-lambda-ENV.json create mode 100644 example/policies/bar-logs-lambda.json delete mode 100644 example/policies/baz-dynamodb-items-ENV.json create mode 100644 example/policies/baz-dynamodb-items.json delete mode 100644 example/policies/foo-s3-logs-ENV.json create mode 100644 example/policies/foo-s3-logs.json delete mode 100644 example/policies/foo-s3-storage-ENV.json create mode 100644 example/policies/foo-s3-storage.json diff --git a/README.md b/README.md index 064bf7c..1106561 100644 --- a/README.md +++ b/README.md @@ -61,18 +61,24 @@ A filename minus the extension (.json) decides the policy name. ```json { - "Version": "2012-10-17", - "Statement": [ - { - "Effect": "Allow", - "Action": [ - "s3:PutObject", - "s3:GetObject", - "s3:DeleteObject" - ], - "Resource": "arn:aws:s3:::yourapp-storage-ENV/*" - } - ] + "Policy": { + "PolicyName": "yourapp-s3-storage-ENV", + "Path": "/" + }, + "Documen": { + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Action": [ + "s3:PutObject", + "s3:GetObject", + "s3:DeleteObject" + ], + "Resource": "arn:aws:s3:::yourapp-storage-ENV/*" + } + ] + } } ``` diff --git a/example/policies/bar-logs-lambda-ENV.json b/example/policies/bar-logs-lambda-ENV.json deleted file mode 100644 index 5ee9eee..0000000 --- a/example/policies/bar-logs-lambda-ENV.json +++ /dev/null @@ -1,14 +0,0 @@ -{ - "Version": "2012-10-17", - "Statement": [ - { - "Action": [ - "logs:CreateLogGroup", - "logs:CreateLogStream", - "logs:PutLogEvents" - ], - "Effect": "Allow", - "Resource": "arn:aws:logs:*:*:*" - } - ] -} \ No newline at end of file diff --git a/example/policies/bar-logs-lambda.json b/example/policies/bar-logs-lambda.json new file mode 100644 index 0000000..fa9fb77 --- /dev/null +++ b/example/policies/bar-logs-lambda.json @@ -0,0 +1,20 @@ +{ + "Policy": { + "PolicyName": "bar-logs-lambda-ENV", + "Path": "/" + }, + "Document": { + "Version": "2012-10-17", + "Statement": [ + { + "Action": [ + "logs:CreateLogGroup", + "logs:CreateLogStream", + "logs:PutLogEvents" + ], + "Effect": "Allow", + "Resource": "arn:aws:logs:*:*:*" + } + ] + } +} \ No newline at end of file diff --git a/example/policies/baz-dynamodb-items-ENV.json b/example/policies/baz-dynamodb-items-ENV.json deleted file mode 100644 index 873d173..0000000 --- a/example/policies/baz-dynamodb-items-ENV.json +++ /dev/null @@ -1,18 +0,0 @@ -{ - "Version": "2012-10-17", - "Statement": [ - { - "Effect": "Allow", - "Action": [ - "dynamodb:BatchGetItem", - "dynamodb:BatchWriteItem", - "dynamodb:DescribeStream", - "dynamodb:ListStreams", - "dynamodb:PutItem", - "dynamodb:Query", - "dynamodb:UpdateItem" - ], - "Resource": "arn:aws:dynamodb:ap-northeast-1:ACCOUNT_ID:table/baz-items-ENV" - } - ] -} \ No newline at end of file diff --git a/example/policies/baz-dynamodb-items.json b/example/policies/baz-dynamodb-items.json new file mode 100644 index 0000000..296a96b --- /dev/null +++ b/example/policies/baz-dynamodb-items.json @@ -0,0 +1,24 @@ +{ + "Policy": { + "PolicyName": "baz-dynamodb-items-ENV", + "Path": "/" + }, + "Document": { + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Action": [ + "dynamodb:BatchGetItem", + "dynamodb:BatchWriteItem", + "dynamodb:DescribeStream", + "dynamodb:ListStreams", + "dynamodb:PutItem", + "dynamodb:Query", + "dynamodb:UpdateItem" + ], + "Resource": "arn:aws:dynamodb:ap-northeast-1:ACCOUNT_ID:table/baz-items-ENV" + } + ] + } +} \ No newline at end of file diff --git a/example/policies/foo-s3-logs-ENV.json b/example/policies/foo-s3-logs-ENV.json deleted file mode 100644 index 419c18d..0000000 --- a/example/policies/foo-s3-logs-ENV.json +++ /dev/null @@ -1,14 +0,0 @@ -{ - "Version": "2012-10-17", - "Statement": [ - { - "Effect": "Allow", - "Action": [ - "s3:PutObject", - "s3:GetObject", - "s3:DeleteObject" - ], - "Resource": "arn:aws:s3:::foo-s3-logs-ENV-ACCOUNT_ID/*" - } - ] -} \ No newline at end of file diff --git a/example/policies/foo-s3-logs.json b/example/policies/foo-s3-logs.json new file mode 100644 index 0000000..087cc57 --- /dev/null +++ b/example/policies/foo-s3-logs.json @@ -0,0 +1,20 @@ +{ + "Policy": { + "PolicyName": "foo-s3-logs-ENV", + "Path": "/" + }, + "Document": { + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Action": [ + "s3:PutObject", + "s3:GetObject", + "s3:DeleteObject" + ], + "Resource": "arn:aws:s3:::foo-s3-logs-ENV-ACCOUNT_ID/*" + } + ] + } +} \ No newline at end of file diff --git a/example/policies/foo-s3-storage-ENV.json b/example/policies/foo-s3-storage-ENV.json deleted file mode 100644 index 382db4e..0000000 --- a/example/policies/foo-s3-storage-ENV.json +++ /dev/null @@ -1,14 +0,0 @@ -{ - "Version": "2012-10-17", - "Statement": [ - { - "Effect": "Allow", - "Action": [ - "s3:PutObject", - "s3:GetObject", - "s3:DeleteObject" - ], - "Resource": "arn:aws:s3:::foo-s3-storage-ENV-ACCOUNT_ID/*" - } - ] -} \ No newline at end of file diff --git a/example/policies/foo-s3-storage.json b/example/policies/foo-s3-storage.json new file mode 100644 index 0000000..ffb9ca8 --- /dev/null +++ b/example/policies/foo-s3-storage.json @@ -0,0 +1,20 @@ +{ + "Policy": { + "PolicyName": "foo-s3-storage-ENV", + "Path": "/" + }, + "Document": { + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Action": [ + "s3:PutObject", + "s3:GetObject", + "s3:DeleteObject" + ], + "Resource": "arn:aws:s3:::foo-s3-storage-ENV-ACCOUNT_ID/*" + } + ] + } +} \ No newline at end of file From 4535a06cc3c7462a04af4b53ff4687caa0a45ab6 Mon Sep 17 00:00:00 2001 From: Toshimitsu Takahashi Date: Sat, 13 Jul 2019 20:01:35 +0900 Subject: [PATCH 2/7] Change export policy for V2 --- src/aws/operation.ts | 6 ++++++ src/aws/policy.ts | 47 +++++++++++++++++++++++++++----------------- src/export_policy.ts | 5 ++--- 3 files changed, 37 insertions(+), 21 deletions(-) diff --git a/src/aws/operation.ts b/src/aws/operation.ts index 61855a8..d719388 100644 --- a/src/aws/operation.ts +++ b/src/aws/operation.ts @@ -56,6 +56,12 @@ export async function listPolicyVersions( return data.Versions || [] } +export async function getPolicy(arn: string): Promise { + const params = { PolicyArn: arn } + const data = await iam.getPolicy(params).promise() + return data.Policy! +} + export async function getPolicyVersion( arn: string, verionId: string diff --git a/src/aws/policy.ts b/src/aws/policy.ts index 45e993a..6715c98 100644 --- a/src/aws/policy.ts +++ b/src/aws/policy.ts @@ -5,6 +5,7 @@ import { DocJson, listPolicyVersions, getPolicyVersion, + getPolicy, } from './operation' export interface PolicyStatementNode { @@ -29,20 +30,27 @@ export class PolicyEntry { policyNode: PolicyNode document: PolicyDocumentNode - constructor(arn: ArnType, document: PolicyDocumentNode) { - this.arn = arn - const arnParts = arn.split('/') + constructor(policy: IAM.Policy, document: PolicyDocumentNode) { + this.arn = policy.Arn! this.policyNode = { - PolicyName: arnParts[arnParts.length - 1], - Path: arnParts.length === 3 ? arnParts[1] : undefined, + PolicyName: policy.PolicyName!, + Path: policy.Path || '/', } this.document = document + console.log(this.policyNode) } get policyName(): string { return this.policyNode.PolicyName } + asJson(): any { + return { + Policy: this.policyNode, + Document: this.document, + } + } + documentAsJson(): DocJson { return this.convertDocToJSON(this.document, 4) } @@ -77,38 +85,41 @@ export class PolicyFetcher { this.arnPrefix = arnPrefix } + async getPolicyEntry(arn: string, versionId: string) { + const [policyInfo, docNode] = await Promise.all([ + getPolicy(arn), + this.getPolicyDocumentVersion(arn, versionId), + ]) + return new PolicyEntry(policyInfo, docNode) + } + async getPolicyDefaultWithVersionInfo( - name: string + name: string, + path: string = '/' ): Promise { - const arn = `${this.arnPrefix!}/${name}` + const arn = `${this.arnPrefix!}${path}${name}` const versions = await listPolicyVersions(arn) if (versions.length === 0) { throw new Error('Failed to get policy versions') } - const { defaultId, oldestId, count } = this.getPolicyVersionsInfoFrom( - versions - ) + const { defaultId, oldestId, count } = this.getPolicyVersionsInfoFrom(versions) return { defaultId, oldestId, count, - currentPolicy: await this.getPolicyVersion(arn, defaultId), + currentPolicy: await this.getPolicyEntry(arn, defaultId), } } - async getPolicyVersion( + async getPolicyDocumentVersion( policyArn: string, verionId: string - ): Promise { + ): Promise { const result = await getPolicyVersion(policyArn, verionId) - const docNode: PolicyDocumentNode = JSON.parse( - decodeURIComponent(result.Document!) - ) - - return new PolicyEntry(policyArn, docNode) + return JSON.parse(decodeURIComponent(result.Document!)) } private getPolicyVersionsInfoFrom( diff --git a/src/export_policy.ts b/src/export_policy.ts index e523cb9..e6af67f 100644 --- a/src/export_policy.ts +++ b/src/export_policy.ts @@ -15,11 +15,10 @@ async function writePolicyFile( parentDir: string, entry: PolicyEntry ): Promise { - const content = entry.document const fileName = `${entry.policyName}.json` try { - await writeJSONFile(parentDir, fileName, content) + await writeJSONFile(parentDir, fileName, entry.asJson()) return OK('Wrote %1', fileName) } catch (err) { return NG('Failed to write %1', fileName) @@ -39,7 +38,7 @@ export async function main(outDir: string, nameMatcher: any, opts: any = {}) { return !nameMatcher || policy.PolicyName!.match(nameMatcher) }), promisedStream((policy: IAM.Policy) => - policyFetcher.getPolicyVersion(policy.Arn!, policy.DefaultVersionId!) + policyFetcher.getPolicyEntry(policy.Arn!, policy.DefaultVersionId!) ), promisedStream((entry: PolicyEntry) => writePolicyFile(outDir, entry)), createWriter(opts), From 29d041ce60ea36e1a37dfe8362d036684d0a185b Mon Sep 17 00:00:00 2001 From: Toshimitsu Takahashi Date: Sat, 13 Jul 2019 20:31:44 +0900 Subject: [PATCH 3/7] Change import policy for V2 --- src/aws/file_reader.ts | 33 +++++++++++++++--- src/aws/policy.ts | 18 +++++----- .../setup/policies/bar-logs-lambda-ENV.json | 30 +++++++++------- .../policies/baz-dynamodb-items-ENV.json | 34 +++++++++++-------- .../setup/policies/foo-s3-logs-ENV.json | 30 +++++++++------- .../setup/policies/foo-s3-storage-ENV.json | 30 +++++++++------- 6 files changed, 111 insertions(+), 64 deletions(-) diff --git a/src/aws/file_reader.ts b/src/aws/file_reader.ts index 9b33b09..075b561 100644 --- a/src/aws/file_reader.ts +++ b/src/aws/file_reader.ts @@ -1,7 +1,7 @@ import path from 'path' import { readFile } from '../utils/file' import { substitute, parseJSON } from '../utils/varset' -import { PolicyEntry } from './policy' +import { PolicyEntry, PolicyDocumentNode, PolicyNode } from './policy' import { ArnType } from './operation' import { RoleEntry, RoleDocument } from './role' @@ -28,11 +28,34 @@ export async function readPolicyFile( let name = '' try { name = path.basename(filePath, '.json') + name = substitute(name, varSet) + const text = await readFile(filePath) - return new PolicyEntry( - arnPrefix + '/' + substitute(name, varSet), - parseJSON(text, varSet) - ) + const rawJson: any = parseJSON(text, varSet) + + let arn: ArnType + let policyInfo: PolicyNode + let docNode: PolicyDocumentNode + if (rawJson.Document) { + // V2 + const { Policy: policy } = rawJson + arn = arnPrefix + policy.Path + policy.PolicyName + policyInfo = { + PolicyName: policy.PolicyName, + Path: policy.Path, + } + docNode = rawJson.Document + } else { + // V1 + arn = arnPrefix + '/' + substitute(name, varSet) + policyInfo = { + PolicyName: substitute(name, varSet), + Path: '/', + } + docNode = rawJson + } + + return new PolicyEntry(arn, policyInfo, docNode) } catch (err) { console.error(`Failed to read ${name}`) throw err diff --git a/src/aws/policy.ts b/src/aws/policy.ts index 6715c98..c6ca4fc 100644 --- a/src/aws/policy.ts +++ b/src/aws/policy.ts @@ -22,7 +22,7 @@ export interface PolicyDocumentNode { export interface PolicyNode { PolicyName: string; - Path?: string; + Path: string; } export class PolicyEntry { @@ -30,14 +30,10 @@ export class PolicyEntry { policyNode: PolicyNode document: PolicyDocumentNode - constructor(policy: IAM.Policy, document: PolicyDocumentNode) { - this.arn = policy.Arn! - this.policyNode = { - PolicyName: policy.PolicyName!, - Path: policy.Path || '/', - } + constructor(arn: ArnType, policy: PolicyNode, document: PolicyDocumentNode) { + this.arn = arn + this.policyNode = policy this.document = document - console.log(this.policyNode) } get policyName(): string { @@ -90,7 +86,11 @@ export class PolicyFetcher { getPolicy(arn), this.getPolicyDocumentVersion(arn, versionId), ]) - return new PolicyEntry(policyInfo, docNode) + const policyNode: PolicyNode = { + PolicyName: policyInfo.PolicyName!, + Path: policyInfo.Path || '/', + } + return new PolicyEntry(arn, policyNode, docNode) } async getPolicyDefaultWithVersionInfo( diff --git a/test/fixtures/setup/policies/bar-logs-lambda-ENV.json b/test/fixtures/setup/policies/bar-logs-lambda-ENV.json index 5ee9eee..227c24e 100644 --- a/test/fixtures/setup/policies/bar-logs-lambda-ENV.json +++ b/test/fixtures/setup/policies/bar-logs-lambda-ENV.json @@ -1,14 +1,20 @@ { - "Version": "2012-10-17", - "Statement": [ - { - "Action": [ - "logs:CreateLogGroup", - "logs:CreateLogStream", - "logs:PutLogEvents" - ], - "Effect": "Allow", - "Resource": "arn:aws:logs:*:*:*" - } - ] + "Policy": { + "PolicyName": "bar-logs-lambda-ENV", + "Path": "/" + }, + "Document" : { + "Version": "2012-10-17", + "Statement": [ + { + "Action": [ + "logs:CreateLogGroup", + "logs:CreateLogStream", + "logs:PutLogEvents" + ], + "Effect": "Allow", + "Resource": "arn:aws:logs:*:*:*" + } + ] + } } \ No newline at end of file diff --git a/test/fixtures/setup/policies/baz-dynamodb-items-ENV.json b/test/fixtures/setup/policies/baz-dynamodb-items-ENV.json index c58e9ea..67bbfa3 100644 --- a/test/fixtures/setup/policies/baz-dynamodb-items-ENV.json +++ b/test/fixtures/setup/policies/baz-dynamodb-items-ENV.json @@ -1,16 +1,22 @@ { - "Version": "2012-10-17", - "Statement": [ - { - "Effect": "Allow", - "Action": [ - "dynamodb:BatchGetItem", - "dynamodb:BatchWriteItem", - "dynamodb:PutItem", - "dynamodb:Query", - "dynamodb:UpdateItem" - ], - "Resource": "arn:aws:dynamodb:ap-northeast-1:ACCOUNT_ID:table/baz-dynamodb-items-ENV" - } - ] + "Policy": { + "PolicyName": "baz-dynamodb-items-ENV", + "Path": "/" + }, + "Document": { + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Action": [ + "dynamodb:BatchGetItem", + "dynamodb:BatchWriteItem", + "dynamodb:PutItem", + "dynamodb:Query", + "dynamodb:UpdateItem" + ], + "Resource": "arn:aws:dynamodb:ap-northeast-1:ACCOUNT_ID:table/baz-dynamodb-items-ENV" + } + ] + } } \ No newline at end of file diff --git a/test/fixtures/setup/policies/foo-s3-logs-ENV.json b/test/fixtures/setup/policies/foo-s3-logs-ENV.json index 419c18d..087cc57 100644 --- a/test/fixtures/setup/policies/foo-s3-logs-ENV.json +++ b/test/fixtures/setup/policies/foo-s3-logs-ENV.json @@ -1,14 +1,20 @@ { - "Version": "2012-10-17", - "Statement": [ - { - "Effect": "Allow", - "Action": [ - "s3:PutObject", - "s3:GetObject", - "s3:DeleteObject" - ], - "Resource": "arn:aws:s3:::foo-s3-logs-ENV-ACCOUNT_ID/*" - } - ] + "Policy": { + "PolicyName": "foo-s3-logs-ENV", + "Path": "/" + }, + "Document": { + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Action": [ + "s3:PutObject", + "s3:GetObject", + "s3:DeleteObject" + ], + "Resource": "arn:aws:s3:::foo-s3-logs-ENV-ACCOUNT_ID/*" + } + ] + } } \ No newline at end of file diff --git a/test/fixtures/setup/policies/foo-s3-storage-ENV.json b/test/fixtures/setup/policies/foo-s3-storage-ENV.json index 382db4e..ffb9ca8 100644 --- a/test/fixtures/setup/policies/foo-s3-storage-ENV.json +++ b/test/fixtures/setup/policies/foo-s3-storage-ENV.json @@ -1,14 +1,20 @@ { - "Version": "2012-10-17", - "Statement": [ - { - "Effect": "Allow", - "Action": [ - "s3:PutObject", - "s3:GetObject", - "s3:DeleteObject" - ], - "Resource": "arn:aws:s3:::foo-s3-storage-ENV-ACCOUNT_ID/*" - } - ] + "Policy": { + "PolicyName": "foo-s3-storage-ENV", + "Path": "/" + }, + "Document": { + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Action": [ + "s3:PutObject", + "s3:GetObject", + "s3:DeleteObject" + ], + "Resource": "arn:aws:s3:::foo-s3-storage-ENV-ACCOUNT_ID/*" + } + ] + } } \ No newline at end of file From 897cd4c4ec87735beedd9f2517dbf6eb80d15d7d Mon Sep 17 00:00:00 2001 From: Toshimitsu Takahashi Date: Sat, 13 Jul 2019 22:10:03 +0900 Subject: [PATCH 4/7] Test policy for having valuable Path --- src/aws/file_reader.ts | 1 + test/11_import_policy.test.ts | 7 +++++ test/90_delete_policy.test.ts | 20 +++++++++++-- ...BasicExecutionRole-b3c4ecbd-319d-475f.json | 26 ++++++++++++++++ .../setup/policies/foo-s3-storage-ENV.json | 30 ++++++++----------- .../fixtures/setup/roles/qux-lambda-role.json | 24 +++++++++++++++ .../terminate/roles/qux-lambda-role.json | 19 ++++++++++++ 7 files changed, 107 insertions(+), 20 deletions(-) create mode 100644 test/fixtures/setup/policies/AWSLambdaBasicExecutionRole-b3c4ecbd-319d-475f.json create mode 100644 test/fixtures/setup/roles/qux-lambda-role.json create mode 100644 test/fixtures/terminate/roles/qux-lambda-role.json diff --git a/src/aws/file_reader.ts b/src/aws/file_reader.ts index 075b561..4f12ae7 100644 --- a/src/aws/file_reader.ts +++ b/src/aws/file_reader.ts @@ -53,6 +53,7 @@ export async function readPolicyFile( Path: '/', } docNode = rawJson + console.warn('[WARN] %s : This policy definition is old version.', path.basename(filePath)) } return new PolicyEntry(arn, policyInfo, docNode) diff --git a/test/11_import_policy.test.ts b/test/11_import_policy.test.ts index 92ed10b..ece0115 100644 --- a/test/11_import_policy.test.ts +++ b/test/11_import_policy.test.ts @@ -16,6 +16,13 @@ describe('import_policy on setup stage', () => { ACCOUNT_ID: process.env.ACCOUNT_ID, }, { writer }) + assert.deepEqual(values.shift(), { + status: 'OK', + message: 'Created %1', + target: 'AWSLambdaBasicExecutionRole-b3c4ecbd-319d-475f', + diff: undefined + }) + assert.deepEqual(values.shift(), { status: 'OK', message: 'Created %1', diff --git a/test/90_delete_policy.test.ts b/test/90_delete_policy.test.ts index 51a2e75..b323f4d 100644 --- a/test/90_delete_policy.test.ts +++ b/test/90_delete_policy.test.ts @@ -10,9 +10,9 @@ describe('delete_policy on terminate stage', () => { before(async () => { const roleDir = path.resolve(__dirname, './fixtures/terminate/roles') const globPromise = util.promisify(glob) - const files = await globPromise(`${roleDir}/*ENV.json`) + const files = await globPromise(`${roleDir}/*.json`) const roleNames = files.map(file => { - return file.split('/').pop().replace(/\-ENV\.json$/, '-test') + return file.split('/').pop().replace(/\.json$/, '').replace(/\-ENV/, '-test') }) for (let roleName of roleNames) { @@ -34,6 +34,22 @@ describe('delete_policy on terminate stage', () => { } }) + it('succeeds with valid results', async () => { + let values: any[] + const writer = writeArray((_, vals) => { + values = vals + }) + + await main("^AWSLambdaBasicExecutionRole\-b3c4ecbd\-319d\-475f$", { noconfirm: true, writer }) + + assert.deepEqual(values.shift(), { + status: 'OK', + message: 'Deleted %1', + target: 'AWSLambdaBasicExecutionRole-b3c4ecbd-319d-475f', + diff: undefined + }) + }) + it('succeeds with valid results', async () => { let values: any[] const writer = writeArray((_, vals) => { diff --git a/test/fixtures/setup/policies/AWSLambdaBasicExecutionRole-b3c4ecbd-319d-475f.json b/test/fixtures/setup/policies/AWSLambdaBasicExecutionRole-b3c4ecbd-319d-475f.json new file mode 100644 index 0000000..d9bfb68 --- /dev/null +++ b/test/fixtures/setup/policies/AWSLambdaBasicExecutionRole-b3c4ecbd-319d-475f.json @@ -0,0 +1,26 @@ +{ + "Policy": { + "PolicyName": "AWSLambdaBasicExecutionRole-b3c4ecbd-319d-475f", + "Path": "/service-role/" + }, + "Document": { + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Action": "logs:CreateLogGroup", + "Resource": "arn:aws:logs:us-east-1:ACCOUNT_ID:*" + }, + { + "Effect": "Allow", + "Action": [ + "logs:CreateLogStream", + "logs:PutLogEvents" + ], + "Resource": [ + "arn:aws:logs:us-east-1:ACCOUNT_ID:log-group:/aws/lambda/qux-lambda:*" + ] + } + ] + } +} \ No newline at end of file diff --git a/test/fixtures/setup/policies/foo-s3-storage-ENV.json b/test/fixtures/setup/policies/foo-s3-storage-ENV.json index ffb9ca8..382db4e 100644 --- a/test/fixtures/setup/policies/foo-s3-storage-ENV.json +++ b/test/fixtures/setup/policies/foo-s3-storage-ENV.json @@ -1,20 +1,14 @@ { - "Policy": { - "PolicyName": "foo-s3-storage-ENV", - "Path": "/" - }, - "Document": { - "Version": "2012-10-17", - "Statement": [ - { - "Effect": "Allow", - "Action": [ - "s3:PutObject", - "s3:GetObject", - "s3:DeleteObject" - ], - "Resource": "arn:aws:s3:::foo-s3-storage-ENV-ACCOUNT_ID/*" - } - ] - } + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Action": [ + "s3:PutObject", + "s3:GetObject", + "s3:DeleteObject" + ], + "Resource": "arn:aws:s3:::foo-s3-storage-ENV-ACCOUNT_ID/*" + } + ] } \ No newline at end of file diff --git a/test/fixtures/setup/roles/qux-lambda-role.json b/test/fixtures/setup/roles/qux-lambda-role.json new file mode 100644 index 0000000..2681ce9 --- /dev/null +++ b/test/fixtures/setup/roles/qux-lambda-role.json @@ -0,0 +1,24 @@ +{ + "Role": { + "Path": "/service-role/", + "RoleName": "qux-lambda-role", + "AssumeRolePolicyDocument": { + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Principal": { + "Service": "lambda.amazonaws.com" + }, + "Action": "sts:AssumeRole" + } + ] + } + }, + "AttachedPolicies": [ + { + "PolicyName": "AWSLambdaBasicExecutionRole-b3c4ecbd-319d-475f", + "PolicyArn": "arn:aws:iam::ACCOUNT_ID:policy/service-role/AWSLambdaBasicExecutionRole-b3c4ecbd-319d-475f" + } + ] +} \ No newline at end of file diff --git a/test/fixtures/terminate/roles/qux-lambda-role.json b/test/fixtures/terminate/roles/qux-lambda-role.json new file mode 100644 index 0000000..b22c568 --- /dev/null +++ b/test/fixtures/terminate/roles/qux-lambda-role.json @@ -0,0 +1,19 @@ +{ + "Role": { + "Path": "/service-role/", + "RoleName": "qux-lambda-role", + "AssumeRolePolicyDocument": { + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Principal": { + "Service": "lambda.amazonaws.com" + }, + "Action": "sts:AssumeRole" + } + ] + } + }, + "AttachedPolicies": [] +} \ No newline at end of file From 2c00568e8aec6e9aa0d771ff8cf1039c1ee2b1cc Mon Sep 17 00:00:00 2001 From: Toshimitsu Takahashi Date: Sat, 13 Jul 2019 22:33:58 +0900 Subject: [PATCH 5/7] Add upgrade_policy script --- README.md | 3 ++ example/upgrade_policy.js | 52 +++++++++++++++++++ .../policies/baz-dynamodb-users-ENV.json | 26 ++++++---- 3 files changed, 71 insertions(+), 10 deletions(-) create mode 100644 example/upgrade_policy.js diff --git a/README.md b/README.md index 1106561..b777fa9 100644 --- a/README.md +++ b/README.md @@ -84,6 +84,9 @@ A filename minus the extension (.json) decides the policy name. See an [example](example) of *Role* and *Policy* definitions. +If you encounter `[WARN] policy-file.json : This policy definition is old version.` message, upgrade your policy definition files to new version. +There is [example/upgrade_policy.js](example/upgrade_policy.js) for the conversion script. + ## Install * Require Node.js 8.10 or later diff --git a/example/upgrade_policy.js b/example/upgrade_policy.js new file mode 100644 index 0000000..bc389ab --- /dev/null +++ b/example/upgrade_policy.js @@ -0,0 +1,52 @@ +#!/usr/bin/env + +const fs = require('fs'); +const path = require('path'); + +async function readJSONFile(filePath) { + return new Promise((resolve, reject) => { + fs.readFile(filePath, 'utf8', (err, text) => { + if (err) reject(err) + else resolve(JSON.parse(text)) + }) + }) +} + +async function writeJSONFile(filePath, content) { + const json = JSON.stringify(content, null, 4) + return new Promise((resolve, reject) => { + fs.writeFile(filePath, json, function(err) { + if (err) reject(err) + else resolve() + }) + }) +} + +;(async () => { + const filePath = process.argv[2] + if (!filePath) { + console.log('Usage) node upgrade_policy.js ') + process.exit(2) + return + } + + const document = await readJSONFile(filePath) + if (document.Document) { + console.warn('[WARN] %s : This policy definition is already new version.', filePath) + process.exit(3) + return + } + + const newDoc = { + Policy: { + PolicyName: path.basename(filePath, '.json'), + Path: '/' + }, + Document: document + } + await writeJSONFile(filePath, newDoc) +})() +.catch(err => { + console.error(err) + process.exit(1) +}) diff --git a/test/fixtures/change/policies/baz-dynamodb-users-ENV.json b/test/fixtures/change/policies/baz-dynamodb-users-ENV.json index 418ec9b..a863854 100644 --- a/test/fixtures/change/policies/baz-dynamodb-users-ENV.json +++ b/test/fixtures/change/policies/baz-dynamodb-users-ENV.json @@ -1,12 +1,18 @@ { - "Version": "2012-10-17", - "Statement": [ - { - "Effect": "Deny", - "Action": [ - "dynamodb:DescribeTable" - ], - "Resource": "arn:aws:dynamodb:ap-northeast-1:ACCOUNT_ID:table/baz-dynamodb-users-ENV" - } - ] + "Policy": { + "PolicyName": "baz-dynamodb-users-ENV", + "Path": "/" + }, + "Document": { + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Deny", + "Action": [ + "dynamodb:DescribeTable" + ], + "Resource": "arn:aws:dynamodb:ap-northeast-1:ACCOUNT_ID:table/baz-dynamodb-users-ENV" + } + ] + } } \ No newline at end of file From e7beb1cbf54d335f34226712fc358d254841a0e2 Mon Sep 17 00:00:00 2001 From: Toshimitsu Takahashi Date: Sat, 13 Jul 2019 23:14:03 +0900 Subject: [PATCH 6/7] No console for delete_policy test --- src/delete_policy.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/delete_policy.ts b/src/delete_policy.ts index 2c3c00a..c263b75 100644 --- a/src/delete_policy.ts +++ b/src/delete_policy.ts @@ -6,14 +6,14 @@ const promisedLife = require('promised-lifestream') import prompt from './utils/prompt' import { IAM } from 'aws-sdk' -import { iam } from './aws/iam' import { ListPolicyStream } from './aws/list_stream' import { filterStream, promisedStream } from './utils/stream' -import { OK, NG } from './utils/result' import { createWriter } from './utils/result_writer' import { PolicyCleaner } from './logic/policy_cleaner'; export async function main(nameMatcher: any, opts: any = {}) { + const needConfirm: boolean = !opts.noconfirm + try { const policies = await promisedLife( [ @@ -23,7 +23,7 @@ export async function main(nameMatcher: any, opts: any = {}) { }), filterStream((policy: IAM.Policy) => { if (policy.PolicyName!.match(nameMatcher)) { - console.info(policy.Arn) + if (needConfirm) console.info(policy.Arn) return true } return false @@ -37,7 +37,7 @@ export async function main(nameMatcher: any, opts: any = {}) { return } - if (process.env.NODE_ENV !== 'test' && !opts.noconfirm) { + if (needConfirm) { const answer = await prompt( 'Do you really delete above policies? yes|no> ' ) From 3fb924dbb49c8815ec15b60a15479e1a2bb302c2 Mon Sep 17 00:00:00 2001 From: Toshimitsu Takahashi Date: Sat, 13 Jul 2019 23:49:13 +0900 Subject: [PATCH 7/7] Add test for service-role and its policy --- src/aws/policy.ts | 7 ++----- src/delete_policy.ts | 4 ++-- src/logic/policy_cleaner.ts | 2 +- src/logic/policy_registerer.ts | 2 +- src/logic/policy_validator.ts | 2 +- test/21_export_policy.test.ts | 9 ++++++++- test/22_export_role.test.ts | 2 +- test/31_validate_policy.test.ts | 7 +++++++ test/32_validate_role.test.ts | 7 +++++++ 9 files changed, 30 insertions(+), 12 deletions(-) diff --git a/src/aws/policy.ts b/src/aws/policy.ts index c6ca4fc..26a592b 100644 --- a/src/aws/policy.ts +++ b/src/aws/policy.ts @@ -93,11 +93,8 @@ export class PolicyFetcher { return new PolicyEntry(arn, policyNode, docNode) } - async getPolicyDefaultWithVersionInfo( - name: string, - path: string = '/' - ): Promise { - const arn = `${this.arnPrefix!}${path}${name}` + async getPolicyDefaultWithVersionInfo(entry: PolicyEntry): Promise { + const { arn } = entry const versions = await listPolicyVersions(arn) if (versions.length === 0) { diff --git a/src/delete_policy.ts b/src/delete_policy.ts index c263b75..6156826 100644 --- a/src/delete_policy.ts +++ b/src/delete_policy.ts @@ -9,10 +9,10 @@ import { IAM } from 'aws-sdk' import { ListPolicyStream } from './aws/list_stream' import { filterStream, promisedStream } from './utils/stream' import { createWriter } from './utils/result_writer' -import { PolicyCleaner } from './logic/policy_cleaner'; +import { PolicyCleaner } from './logic/policy_cleaner' export async function main(nameMatcher: any, opts: any = {}) { - const needConfirm: boolean = !opts.noconfirm + const needConfirm = !opts.noconfirm try { const policies = await promisedLife( diff --git a/src/logic/policy_cleaner.ts b/src/logic/policy_cleaner.ts index 035a3f6..1a3586c 100644 --- a/src/logic/policy_cleaner.ts +++ b/src/logic/policy_cleaner.ts @@ -42,7 +42,7 @@ export class PolicyCleaner { private async deletePolicyVersion(arn: string, versionId: string) { const params = { PolicyArn: arn, - VersionId: versionId + VersionId: versionId, } return iam.deletePolicyVersion(params).promise() } diff --git a/src/logic/policy_registerer.ts b/src/logic/policy_registerer.ts index b8c1e4d..7e18301 100644 --- a/src/logic/policy_registerer.ts +++ b/src/logic/policy_registerer.ts @@ -49,7 +49,7 @@ export class PolicyRegisterer { oldestId: deleteVerId, count, currentPolicy: remotePolicy, - } = await this.policyFetcher.getPolicyDefaultWithVersionInfo(name) + } = await this.policyFetcher.getPolicyDefaultWithVersionInfo(entry) if (localDocJson === remotePolicy.documentAsJson()) { return Skip('%1 not changed', name) diff --git a/src/logic/policy_validator.ts b/src/logic/policy_validator.ts index dbe7300..1e6a7b0 100644 --- a/src/logic/policy_validator.ts +++ b/src/logic/policy_validator.ts @@ -25,7 +25,7 @@ export class PolicyValidator { try { const { currentPolicy: remotePolicy, - } = await this.policyFetcher.getPolicyDefaultWithVersionInfo(policyName) + } = await this.policyFetcher.getPolicyDefaultWithVersionInfo(entry) if (!remotePolicy) { this._invalidCnt++ return NG('%1 does not exist.', policyName) diff --git a/test/21_export_policy.test.ts b/test/21_export_policy.test.ts index c3a3c9e..4abd479 100644 --- a/test/21_export_policy.test.ts +++ b/test/21_export_policy.test.ts @@ -18,7 +18,14 @@ describe('export_policy on setup stage', () => { values = vals.sort((a, b) => a.target.localeCompare(b.target)) }) - await main(outDir, '\-test$', { writer }) + await main(outDir, '\-test|^AWSLambdaBasicExecutionRole\-b3c4ecbd\-319d\-475f$', { writer }) + + assert.deepEqual(values.shift(), { + status: 'OK', + message: 'Wrote %1', + target: 'AWSLambdaBasicExecutionRole-b3c4ecbd-319d-475f.json', + diff: undefined + }) assert.deepEqual(values.shift(), { status: 'OK', diff --git a/test/22_export_role.test.ts b/test/22_export_role.test.ts index 8666fc6..934348a 100644 --- a/test/22_export_role.test.ts +++ b/test/22_export_role.test.ts @@ -18,7 +18,7 @@ describe('export_role on setup stage', () => { values = vals.sort((a, b) => a.target.localeCompare(b.target)) }) - await main(outDir, "\-test$", { writer }) + await main(outDir, "\-test|qux\-lambda\-role$", { writer }) assert.deepEqual(values.shift(), { status: 'OK', diff --git a/test/31_validate_policy.test.ts b/test/31_validate_policy.test.ts index 91e024f..2705430 100644 --- a/test/31_validate_policy.test.ts +++ b/test/31_validate_policy.test.ts @@ -11,6 +11,13 @@ describe('validate_policy on setup stage', () => { await main(path.resolve(__dirname, './out/policies'), {}, { writer }) + assert.deepEqual(values.shift(), { + status: 'OK', + message: '%1', + target: 'AWSLambdaBasicExecutionRole-b3c4ecbd-319d-475f', + diff: undefined + }) + assert.deepEqual(values.shift(), { status: 'OK', message: '%1', diff --git a/test/32_validate_role.test.ts b/test/32_validate_role.test.ts index 6c7920d..0ac8d23 100644 --- a/test/32_validate_role.test.ts +++ b/test/32_validate_role.test.ts @@ -31,5 +31,12 @@ describe('validate_role on setup stage', () => { target: 'foo-ec2-admin-test', diff: undefined }) + + assert.deepEqual(values.shift(), { + status: 'OK', + message: '%1', + target: 'qux-lambda-role', + diff: undefined + }) }) })