From 30840f5f6fd644ccbe1337d3f58b720cdf55ead1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=F0=9F=91=A8=F0=9F=8F=BC=E2=80=8D=F0=9F=92=BB=20Romain=20M?= =?UTF-8?q?arcadier-Muller?= Date: Wed, 3 Oct 2018 19:06:00 +0200 Subject: [PATCH 1/5] feat: Manage IAM permissions for (some) CFN CodePipeline actions When adding CloudFormation actions to CodePipeline, the pipeline's role must be granted appropriate permissions on the CloudFormation stacks in order for the pipeline to work. This adds the relevant permission management to the ChangeSet actions (`CreateReplaceChangeSet`, `ExecuteChangeSet`). A bonus BREAKING CHANGE is that the `Artifact.subartifact` method of the CodePipeline API was renamed to `Artifact.atPath`. --- .../lib/pipeline-actions.ts | 24 +++ .../test/test.pipeline-actions.ts | 144 ++++++++++++++++++ .../aws-codepipeline-api/lib/artifact.ts | 2 +- .../test/integ.cfn-template-from-repo.lit.ts | 2 +- .../test.cloudformation-pipeline-actions.ts | 8 +- 5 files changed, 174 insertions(+), 6 deletions(-) create mode 100644 packages/@aws-cdk/aws-cloudformation/test/test.pipeline-actions.ts diff --git a/packages/@aws-cdk/aws-cloudformation/lib/pipeline-actions.ts b/packages/@aws-cdk/aws-cloudformation/lib/pipeline-actions.ts index 372aab93934ce..183f5197bb88d 100644 --- a/packages/@aws-cdk/aws-cloudformation/lib/pipeline-actions.ts +++ b/packages/@aws-cdk/aws-cloudformation/lib/pipeline-actions.ts @@ -88,6 +88,12 @@ export class PipelineExecuteChangeSetAction extends PipelineCloudFormationAction ActionMode: 'CHANGE_SET_EXECUTE', ChangeSetName: props.changeSetName, }); + + const stackArn = cdk.ArnUtils.fromComponents({ service: 'cloudformation', resource: 'stack', resourceName: `${props.stackName}/*` }); + props.stage.pipelineRole.addToPolicy(new cdk.PolicyStatement() + .addAction('cloudformation:ExecuteChangeSet') + .addResource(stackArn) + .addCondition('StringEquals', { 'cloudformation:ChangeSetName': props.changeSetName })); } } @@ -243,6 +249,24 @@ export class PipelineCreateReplaceChangeSetAction extends PipelineCloudFormation }); this.addInputArtifact(props.templatePath.artifact); + if (props.templateConfiguration && props.templateConfiguration.artifact.name !== props.templatePath.artifact.name) { + this.addInputArtifact(props.templateConfiguration.artifact); + } + + const stackArn = cdk.ArnUtils.fromComponents({ service: 'cloudformation', resource: 'stack', resourceName: `${props.stackName}/*` }); + // Allow the pipeline to check for Stack & ChangeSet existence + props.stage.pipelineRole.addToPolicy(new cdk.PolicyStatement() + .addActions('cloudformation:DescribeChangeSet', 'cloudformation:DescribeStacks') + .addResource(stackArn)); + // Allow the pipeline to create & delete the specified ChangeSet + props.stage.pipelineRole.addToPolicy(new cdk.PolicyStatement() + .addActions('cloudformation:CreateChangeSet', 'cloudformation:DeleteChangeSet') + .addResource(stackArn) + .addCondition('StringEquals', { 'cloudformation:ChangeSetName': props.changeSetName })); + // Allow the pipeline to pass this actions' role to CloudFormation + props.stage.pipelineRole.addToPolicy(new cdk.PolicyStatement() + .addAction('iam:PassRole') + .addResource(this.role.roleArn)); } } diff --git a/packages/@aws-cdk/aws-cloudformation/test/test.pipeline-actions.ts b/packages/@aws-cdk/aws-cloudformation/test/test.pipeline-actions.ts new file mode 100644 index 0000000000000..e8f4759c2c66a --- /dev/null +++ b/packages/@aws-cdk/aws-cloudformation/test/test.pipeline-actions.ts @@ -0,0 +1,144 @@ +import cpapi = require('@aws-cdk/aws-codepipeline-api'); +import iam = require('@aws-cdk/aws-iam'); +import cdk = require('@aws-cdk/cdk'); +import nodeunit = require('nodeunit'); +import util = require('util'); +import cloudformation = require('../lib'); + +export = nodeunit.testCase({ + CreateReplaceChangeSet: { + works(test: nodeunit.Test) { + const stack = new cdk.Stack(); + const statements = new Array(); + const pipelineRole = { + addToPolicy: statement => { statements.push(statement.toJson()); } + } as iam.Role; + const actions = new Array(); + const stage = { + _attachAction: act => { actions.push(act); }, + pipelineRole + } as cpapi.IStage; + const artifact = new cpapi.Artifact(stack as any, 'TestArtifact'); + const action = new cloudformation.PipelineCreateReplaceChangeSetAction(stack, 'Action', { + stage, + changeSetName: 'MyChangeSet', + stackName: 'MyStack', + templatePath: artifact.atPath('path/to/file') + }); + + test.ok(_grantsPermission(statements, 'iam:PassRole', action.role.roleArn), + 'The pipelineRole was given permissions to iam:PassRole to the action'); + + const stackArn = cdk.ArnUtils.fromComponents({ + service: 'cloudformation', + resource: 'stack', + resourceName: 'MyStack/*' + }); + test.ok(_grantsPermission(statements, 'cloudformation:DescribeStacks', stackArn) + && _grantsPermission(statements, 'cloudformation:DescribeChangeSet', stackArn), + 'The pipelineRole was given permissions to describe the stack & it\'s ChangeSets'); + test.ok(_grantsPermission(statements, 'cloudformation:CreateChangeSet', stackArn, { + StringEquals: { 'cloudformation:ChangeSetName': 'MyChangeSet' } + }), + 'The pipelineRole was given permissions to create the desired ChangeSet'); + test.ok(_grantsPermission(statements, 'cloudformation:DeleteChangeSet', stackArn, { + StringEquals: { 'cloudformation:ChangeSetName': 'MyChangeSet' } + }), + 'The pipelineRole was given permissions to delete the desired ChangeSet'); + + test.deepEqual(action.inputArtifacts, [artifact], + 'The inputArtifact was correctly registered'); + + test.ok(_hasAction(actions, 'AWS', 'CloudFormation', 'Deploy', { + ActionMode: 'CHANGE_SET_CREATE_REPLACE', + StackName: 'MyStack', + ChangeSetName: 'MyChangeSet' + })); + + test.done(); + } + }, + ExecuteChangeSet: { + works(test: nodeunit.Test) { + const stack = new cdk.Stack(); + const statements = new Array(); + const pipelineRole = { + addToPolicy: statement => { statements.push(statement.toJson()); } + } as iam.Role; + const actions = new Array(); + const stage = { + _attachAction: act => { actions.push(act); }, + pipelineRole + } as cpapi.IStage; + new cloudformation.PipelineExecuteChangeSetAction(stack, 'Action', { + stage, + changeSetName: 'MyChangeSet', + stackName: 'MyStack', + }); + + const stackArn = cdk.ArnUtils.fromComponents({ + service: 'cloudformation', + resource: 'stack', + resourceName: 'MyStack/*' + }); + test.ok(_grantsPermission(statements, 'cloudformation:ExecuteChangeSet', stackArn, { + StringEquals: { 'cloudformation:ChangeSetName': 'MyChangeSet' } + }), + 'The pipelineRole was given permissions to execute the desired ChangeSet'); + + test.ok(_hasAction(actions, 'AWS', 'CloudFormation', 'Deploy', { + ActionMode: 'CHANGE_SET_EXECUTE', + StackName: 'MyStack', + ChangeSetName: 'MyChangeSet' + })); + + test.done(); + } + } +}); + +interface PolicyStatementJson { + Effect: 'Allow' | 'Deny'; + Action: string | string[]; + Resource: string | string[]; + Condition: any; +} + +function _hasAction(actions: cpapi.Action[], owner: string, provider: string, category: string, configuration?: { [key: string]: any}) { + for (const action of actions) { + if (action.owner !== owner) { continue; } + if (action.provider !== provider) { continue; } + if (action.category !== category) { continue; } + if (configuration && !action.configuration) { continue; } + if (configuration) { + for (const key of Object.keys(configuration)) { + if (!util.isDeepStrictEqual(cdk.resolve(action.configuration[key]), cdk.resolve(configuration[key]))) { + continue; + } + } + } + return true; + } + return false; +} + +function _grantsPermission(statements: PolicyStatementJson[], action: string, resource: string, conditions?: any) { + for (const statement of statements.filter(s => s.Effect === 'Allow')) { + if (!_isOrContains(statement.Action, action)) { continue; } + if (!_isOrContains(statement.Resource, resource)) { continue; } + if (conditions && !_isOrContains(statement.Condition, conditions)) { continue; } + return true; + } + return false; +} + +function _isOrContains(entity: string | string[], value: string): boolean { + const resolvedValue = cdk.resolve(value); + const resolvedEntity = cdk.resolve(entity); + if (util.isDeepStrictEqual(resolvedEntity, resolvedValue)) { return true; } + if (!Array.isArray(resolvedEntity)) { return false; } + for (const tested of entity) { + if (util.isDeepStrictEqual(tested, resolvedValue)) { return true; } + } + return false; +} diff --git a/packages/@aws-cdk/aws-codepipeline-api/lib/artifact.ts b/packages/@aws-cdk/aws-codepipeline-api/lib/artifact.ts index d5e3bc7ad90ec..b31da4dc4884a 100644 --- a/packages/@aws-cdk/aws-codepipeline-api/lib/artifact.ts +++ b/packages/@aws-cdk/aws-codepipeline-api/lib/artifact.ts @@ -14,7 +14,7 @@ export class Artifact extends Construct { * Output is in the form "::" * @param fileName The name of the file */ - public subartifact(fileName: string) { + public atPath(fileName: string) { return new ArtifactPath(this, fileName); } diff --git a/packages/@aws-cdk/aws-codepipeline/test/integ.cfn-template-from-repo.lit.ts b/packages/@aws-cdk/aws-codepipeline/test/integ.cfn-template-from-repo.lit.ts index e6c1bdf7dede0..7167d3bbb9afb 100644 --- a/packages/@aws-cdk/aws-codepipeline/test/integ.cfn-template-from-repo.lit.ts +++ b/packages/@aws-cdk/aws-codepipeline/test/integ.cfn-template-from-repo.lit.ts @@ -30,7 +30,7 @@ new cfn.PipelineCreateReplaceChangeSetAction(prodStage, 'PrepareChanges', { stackName, changeSetName, fullPermissions: true, - templatePath: source.artifact.subartifact('template.yaml'), + templatePath: source.artifact.atPath('template.yaml'), }); new codepipeline.ManualApprovalAction(stack, 'ApproveChanges', { diff --git a/packages/@aws-cdk/aws-codepipeline/test/test.cloudformation-pipeline-actions.ts b/packages/@aws-cdk/aws-codepipeline/test/test.cloudformation-pipeline-actions.ts index 5274184ce2daf..4994ee2243047 100644 --- a/packages/@aws-cdk/aws-codepipeline/test/test.cloudformation-pipeline-actions.ts +++ b/packages/@aws-cdk/aws-codepipeline/test/test.cloudformation-pipeline-actions.ts @@ -203,7 +203,7 @@ export = { new PipelineCreateUpdateStackAction(stack.deployStage, 'CreateUpdate', { stage: stack.deployStage, stackName: 'MyStack', - templatePath: stack.source.artifact.subartifact('template.yaml'), + templatePath: stack.source.artifact.atPath('template.yaml'), fullPermissions: true, }); @@ -256,7 +256,7 @@ export = { new PipelineCreateUpdateStackAction(stack, 'CreateUpdate', { stage: stack.deployStage, stackName: 'MyStack', - templatePath: stack.source.artifact.subartifact('template.yaml'), + templatePath: stack.source.artifact.atPath('template.yaml'), outputFileName: 'CreateResponse.json', }); @@ -287,7 +287,7 @@ export = { new PipelineCreateUpdateStackAction(stack, 'CreateUpdate', { stage: stack.deployStage, stackName: 'MyStack', - templatePath: stack.source.artifact.subartifact('template.yaml'), + templatePath: stack.source.artifact.atPath('template.yaml'), replaceOnFailure: true, }); @@ -320,7 +320,7 @@ export = { new PipelineCreateUpdateStackAction(stack, 'CreateUpdate', { stage: stack.deployStage, stackName: 'MyStack', - templatePath: stack.source.artifact.subartifact('template.yaml'), + templatePath: stack.source.artifact.atPath('template.yaml'), parameterOverrides: { RepoName: stack.repo.repositoryName } From 0ee53be0338589940663d9ef6dc2dc6e878d7fb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=F0=9F=91=A8=F0=9F=8F=BC=E2=80=8D=F0=9F=92=BB=20Romain=20M?= =?UTF-8?q?arcadier-Muller?= Date: Thu, 4 Oct 2018 10:00:14 +0200 Subject: [PATCH 2/5] Some DRYing up --- .../lib/pipeline-actions.ts | 17 ++- .../test/test.pipeline-actions.ts | 79 +++++++----- ...g.cfn-template-from-repo.lit.expected.json | 114 ++++++++++++++++++ .../test/integ.pipeline-cfn.expected.json | 79 ++++++++++++ 4 files changed, 254 insertions(+), 35 deletions(-) diff --git a/packages/@aws-cdk/aws-cloudformation/lib/pipeline-actions.ts b/packages/@aws-cdk/aws-cloudformation/lib/pipeline-actions.ts index 183f5197bb88d..cf271353ea47a 100644 --- a/packages/@aws-cdk/aws-cloudformation/lib/pipeline-actions.ts +++ b/packages/@aws-cdk/aws-cloudformation/lib/pipeline-actions.ts @@ -89,10 +89,9 @@ export class PipelineExecuteChangeSetAction extends PipelineCloudFormationAction ChangeSetName: props.changeSetName, }); - const stackArn = cdk.ArnUtils.fromComponents({ service: 'cloudformation', resource: 'stack', resourceName: `${props.stackName}/*` }); props.stage.pipelineRole.addToPolicy(new cdk.PolicyStatement() .addAction('cloudformation:ExecuteChangeSet') - .addResource(stackArn) + .addResource(stackArnFromName(props.stackName)) .addCondition('StringEquals', { 'cloudformation:ChangeSetName': props.changeSetName })); } } @@ -253,14 +252,14 @@ export class PipelineCreateReplaceChangeSetAction extends PipelineCloudFormation this.addInputArtifact(props.templateConfiguration.artifact); } - const stackArn = cdk.ArnUtils.fromComponents({ service: 'cloudformation', resource: 'stack', resourceName: `${props.stackName}/*` }); + const stackArn = stackArnFromName(props.stackName); // Allow the pipeline to check for Stack & ChangeSet existence props.stage.pipelineRole.addToPolicy(new cdk.PolicyStatement() - .addActions('cloudformation:DescribeChangeSet', 'cloudformation:DescribeStacks') + .addAction('cloudformation:DescribeStacks') .addResource(stackArn)); // Allow the pipeline to create & delete the specified ChangeSet props.stage.pipelineRole.addToPolicy(new cdk.PolicyStatement() - .addActions('cloudformation:CreateChangeSet', 'cloudformation:DeleteChangeSet') + .addActions('cloudformation:CreateChangeSet', 'cloudformation:DeleteChangeSet', 'cloudformation:DescribeChangeSet') .addResource(stackArn) .addCondition('StringEquals', { 'cloudformation:ChangeSetName': props.changeSetName })); // Allow the pipeline to pass this actions' role to CloudFormation @@ -361,3 +360,11 @@ export enum CloudFormationCapabilities { */ NamedIAM = 'CAPABILITY_NAMED_IAM' } + +function stackArnFromName(stackName: string): string { + return cdk.ArnUtils.fromComponents({ + service: 'cloudformation', + resource: 'stack', + resourceName: `${stackName}/*` + }); +} diff --git a/packages/@aws-cdk/aws-cloudformation/test/test.pipeline-actions.ts b/packages/@aws-cdk/aws-cloudformation/test/test.pipeline-actions.ts index e8f4759c2c66a..6ceb2c3015ed3 100644 --- a/packages/@aws-cdk/aws-cloudformation/test/test.pipeline-actions.ts +++ b/packages/@aws-cdk/aws-cloudformation/test/test.pipeline-actions.ts @@ -9,15 +9,8 @@ export = nodeunit.testCase({ CreateReplaceChangeSet: { works(test: nodeunit.Test) { const stack = new cdk.Stack(); - const statements = new Array(); - const pipelineRole = { - addToPolicy: statement => { statements.push(statement.toJson()); } - } as iam.Role; - const actions = new Array(); - const stage = { - _attachAction: act => { actions.push(act); }, - pipelineRole - } as cpapi.IStage; + const pipelineRole = new RoleDouble(stack, 'PipelineRole'); + const stage = new StageDouble({ pipelineRole }); const artifact = new cpapi.Artifact(stack as any, 'TestArtifact'); const action = new cloudformation.PipelineCreateReplaceChangeSetAction(stack, 'Action', { stage, @@ -26,7 +19,7 @@ export = nodeunit.testCase({ templatePath: artifact.atPath('path/to/file') }); - test.ok(_grantsPermission(statements, 'iam:PassRole', action.role.roleArn), + test.ok(_grantsPermission(pipelineRole.statements, 'iam:PassRole', action.role.roleArn), 'The pipelineRole was given permissions to iam:PassRole to the action'); const stackArn = cdk.ArnUtils.fromComponents({ @@ -34,22 +27,20 @@ export = nodeunit.testCase({ resource: 'stack', resourceName: 'MyStack/*' }); - test.ok(_grantsPermission(statements, 'cloudformation:DescribeStacks', stackArn) - && _grantsPermission(statements, 'cloudformation:DescribeChangeSet', stackArn), + const changeSetCondition = { StringEquals: { 'cloudformation:ChangeSetName': 'MyChangeSet' } }; + test.ok(_grantsPermission(pipelineRole.statements, 'cloudformation:DescribeStacks', stackArn), 'The pipelineRole was given permissions to describe the stack & it\'s ChangeSets'); - test.ok(_grantsPermission(statements, 'cloudformation:CreateChangeSet', stackArn, { - StringEquals: { 'cloudformation:ChangeSetName': 'MyChangeSet' } - }), + test.ok(_grantsPermission(pipelineRole.statements, 'cloudformation:DescribeChangeSet', stackArn, changeSetCondition), + 'The pipelineRole was given permissions to describe the desired ChangeSet'); + test.ok(_grantsPermission(pipelineRole.statements, 'cloudformation:CreateChangeSet', stackArn, changeSetCondition), 'The pipelineRole was given permissions to create the desired ChangeSet'); - test.ok(_grantsPermission(statements, 'cloudformation:DeleteChangeSet', stackArn, { - StringEquals: { 'cloudformation:ChangeSetName': 'MyChangeSet' } - }), + test.ok(_grantsPermission(pipelineRole.statements, 'cloudformation:DeleteChangeSet', stackArn, changeSetCondition), 'The pipelineRole was given permissions to delete the desired ChangeSet'); test.deepEqual(action.inputArtifacts, [artifact], 'The inputArtifact was correctly registered'); - test.ok(_hasAction(actions, 'AWS', 'CloudFormation', 'Deploy', { + test.ok(_hasAction(stage.actions, 'AWS', 'CloudFormation', 'Deploy', { ActionMode: 'CHANGE_SET_CREATE_REPLACE', StackName: 'MyStack', ChangeSetName: 'MyChangeSet' @@ -61,15 +52,8 @@ export = nodeunit.testCase({ ExecuteChangeSet: { works(test: nodeunit.Test) { const stack = new cdk.Stack(); - const statements = new Array(); - const pipelineRole = { - addToPolicy: statement => { statements.push(statement.toJson()); } - } as iam.Role; - const actions = new Array(); - const stage = { - _attachAction: act => { actions.push(act); }, - pipelineRole - } as cpapi.IStage; + const pipelineRole = new RoleDouble(stack, 'PipelineRole'); + const stage = new StageDouble({ pipelineRole }); new cloudformation.PipelineExecuteChangeSetAction(stack, 'Action', { stage, changeSetName: 'MyChangeSet', @@ -81,12 +65,12 @@ export = nodeunit.testCase({ resource: 'stack', resourceName: 'MyStack/*' }); - test.ok(_grantsPermission(statements, 'cloudformation:ExecuteChangeSet', stackArn, { + test.ok(_grantsPermission(pipelineRole.statements, 'cloudformation:ExecuteChangeSet', stackArn, { StringEquals: { 'cloudformation:ChangeSetName': 'MyChangeSet' } }), 'The pipelineRole was given permissions to execute the desired ChangeSet'); - test.ok(_hasAction(actions, 'AWS', 'CloudFormation', 'Deploy', { + test.ok(_hasAction(stage.actions, 'AWS', 'CloudFormation', 'Deploy', { ActionMode: 'CHANGE_SET_EXECUTE', StackName: 'MyStack', ChangeSetName: 'MyChangeSet' @@ -142,3 +126,38 @@ function _isOrContains(entity: string | string[], value: string): boolean { } return false; } + +class StageDouble implements cpapi.IStage { + public readonly name: string; + public readonly pipelineArn: string; + public readonly pipelineRole: iam.Role; + + public readonly actions = new Array(); + + constructor({ name, pipelineName, pipelineRole }: { name?: string, pipelineName?: string, pipelineRole: iam.Role }) { + this.name = name || 'TestStage'; + this.pipelineArn = cdk.ArnUtils.fromComponents({ service: 'codepipeline', resource: 'pipeline', resourceName: pipelineName || 'TestPipeline' }); + this.pipelineRole = pipelineRole; + } + + public grantPipelineBucketReadWrite() { + throw new Error('Unsupported'); + } + + public _attachAction(action: cpapi.Action) { + this.actions.push(action); + } +} + +class RoleDouble extends iam.Role { + public readonly statements = new Array(); + + constructor(parent: cdk.Construct, id: string, props: iam.RoleProps = { assumedBy: new cdk.ServicePrincipal('test') }) { + super(parent, id, props); + } + + public addToPolicy(statement: cdk.PolicyStatement) { + super.addToPolicy(statement); + this.statements.push(statement.toJson()); + } +} diff --git a/packages/@aws-cdk/aws-codepipeline/test/integ.cfn-template-from-repo.lit.expected.json b/packages/@aws-cdk/aws-codepipeline/test/integ.cfn-template-from-repo.lit.expected.json index cac1870ff16d0..a825f06928a3d 100644 --- a/packages/@aws-cdk/aws-codepipeline/test/integ.cfn-template-from-repo.lit.expected.json +++ b/packages/@aws-cdk/aws-codepipeline/test/integ.cfn-template-from-repo.lit.expected.json @@ -75,6 +75,120 @@ "Arn" ] } + }, + { + "Action": "cloudformation:DescribeStacks", + "Effect": "Allow", + "Resource": { + "Fn::Join": [ + "", + [ + "arn", + ":", + { + "Ref": "AWS::Partition" + }, + ":", + "cloudformation", + ":", + { + "Ref": "AWS::Region" + }, + ":", + { + "Ref": "AWS::AccountId" + }, + ":", + "stack", + "/", + "OurStack/*" + ] + ] + } + }, + { + "Action": [ + "cloudformation:CreateChangeSet", + "cloudformation:DeleteChangeSet", + "cloudformation:DescribeChangeSet" + ], + "Condition": { + "StringEquals": { + "cloudformation:ChangeSetName": "StagedChangeSet" + } + }, + "Effect": "Allow", + "Resource": { + "Fn::Join": [ + "", + [ + "arn", + ":", + { + "Ref": "AWS::Partition" + }, + ":", + "cloudformation", + ":", + { + "Ref": "AWS::Region" + }, + ":", + { + "Ref": "AWS::AccountId" + }, + ":", + "stack", + "/", + "OurStack/*" + ] + ] + } + }, + { + "Action": "iam:PassRole", + "Effect": "Allow", + "Resource": { + "Fn::GetAtt": [ + "PipelineDeployPrepareChangesRoleD28C853C", + "Arn" + ] + } + }, + { + "Action": "cloudformation:ExecuteChangeSet", + "Condition": { + "StringEquals": { + "cloudformation:ChangeSetName": "StagedChangeSet" + } + }, + "Effect": "Allow", + "Resource": { + "Fn::Join": [ + "", + [ + "arn", + ":", + { + "Ref": "AWS::Partition" + }, + ":", + "cloudformation", + ":", + { + "Ref": "AWS::Region" + }, + ":", + { + "Ref": "AWS::AccountId" + }, + ":", + "stack", + "/", + "OurStack/*" + ] + ] + } } ], "Version": "2012-10-17" diff --git a/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-cfn.expected.json b/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-cfn.expected.json index 8067191fc54e1..b2a70c45f0322 100644 --- a/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-cfn.expected.json +++ b/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-cfn.expected.json @@ -90,6 +90,85 @@ ] } ] + }, + { + "Action": "cloudformation:DescribeStacks", + "Effect": "Allow", + "Resource": { + "Fn::Join": [ + "", + [ + "arn", + ":", + { + "Ref": "AWS::Partition" + }, + ":", + "cloudformation", + ":", + { + "Ref": "AWS::Region" + }, + ":", + { + "Ref": "AWS::AccountId" + }, + ":", + "stack", + "/", + "IntegTest-TestActionStack/*" + ] + ] + } + }, + { + "Action": [ + "cloudformation:CreateChangeSet", + "cloudformation:DeleteChangeSet", + "cloudformation:DescribeChangeSet" + ], + "Condition": { + "StringEquals": { + "cloudformation:ChangeSetName": "ChangeSetIntegTest" + } + }, + "Effect": "Allow", + "Resource": { + "Fn::Join": [ + "", + [ + "arn", + ":", + { + "Ref": "AWS::Partition" + }, + ":", + "cloudformation", + ":", + { + "Ref": "AWS::Region" + }, + ":", + { + "Ref": "AWS::AccountId" + }, + ":", + "stack", + "/", + "IntegTest-TestActionStack/*" + ] + ] + } + }, + { + "Action": "iam:PassRole", + "Effect": "Allow", + "Resource": { + "Fn::GetAtt": [ + "CfnChangeSetRole6F05F6FC", + "Arn" + ] + } } ], "Version": "2012-10-17" From e130edf48cbd7766cc35cdd1593e86f8129c3a95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=F0=9F=91=A8=F0=9F=8F=BC=E2=80=8D=F0=9F=92=BB=20Romain=20M?= =?UTF-8?q?arcadier-Muller?= Date: Thu, 4 Oct 2018 18:28:41 +0200 Subject: [PATCH 3/5] Use lodash instead of util, as deepStrictEqual is a node 10+ feature --- .../aws-cloudformation/package-lock.json | 17 ++++++++++++++--- .../@aws-cdk/aws-cloudformation/package.json | 2 ++ .../test/test.pipeline-actions.ts | 8 ++++---- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/packages/@aws-cdk/aws-cloudformation/package-lock.json b/packages/@aws-cdk/aws-cloudformation/package-lock.json index e06fd2d030fd6..bdd10f16c02bd 100644 --- a/packages/@aws-cdk/aws-cloudformation/package-lock.json +++ b/packages/@aws-cdk/aws-cloudformation/package-lock.json @@ -1,5 +1,16 @@ { - "name": "@aws-cdk/aws-cloudformation", - "version": "0.9.0", - "lockfileVersion": 1 + "requires": true, + "lockfileVersion": 1, + "dependencies": { + "@types/lodash": { + "version": "4.14.116", + "resolved": "https://registry.npmjs.org/@types/lodash/-/lodash-4.14.116.tgz", + "integrity": "sha512-lRnAtKnxMXcYYXqOiotTmJd74uawNWuPnsnPrrO7HiFuE3npE2iQhfABatbYDyxTNqZNuXzcKGhw37R7RjBFLg==" + }, + "lodash": { + "version": "4.17.11", + "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.11.tgz", + "integrity": "sha512-cQKh8igo5QUhZ7lg38DYWAxMvjSAKG0A8wGSVimP07SIUEK2UO+arSRKbRZWtelMtN5V0Hkwh5ryOto/SshYIg==" + } + } } diff --git a/packages/@aws-cdk/aws-cloudformation/package.json b/packages/@aws-cdk/aws-cloudformation/package.json index 82acb6edfdf06..f2355394ad050 100644 --- a/packages/@aws-cdk/aws-cloudformation/package.json +++ b/packages/@aws-cdk/aws-cloudformation/package.json @@ -57,9 +57,11 @@ "license": "Apache-2.0", "devDependencies": { "@aws-cdk/assert": "^0.10.0", + "@types/lodash": "^4.14.116", "cdk-build-tools": "^0.10.0", "cdk-integ-tools": "^0.10.0", "cfn2ts": "^0.10.0", + "lodash": "^4.17.11", "pkglint": "^0.10.0" }, "dependencies": { diff --git a/packages/@aws-cdk/aws-cloudformation/test/test.pipeline-actions.ts b/packages/@aws-cdk/aws-cloudformation/test/test.pipeline-actions.ts index 6ceb2c3015ed3..d3dd903d44a81 100644 --- a/packages/@aws-cdk/aws-cloudformation/test/test.pipeline-actions.ts +++ b/packages/@aws-cdk/aws-cloudformation/test/test.pipeline-actions.ts @@ -1,8 +1,8 @@ import cpapi = require('@aws-cdk/aws-codepipeline-api'); import iam = require('@aws-cdk/aws-iam'); import cdk = require('@aws-cdk/cdk'); +import _ = require('lodash'); import nodeunit = require('nodeunit'); -import util = require('util'); import cloudformation = require('../lib'); export = nodeunit.testCase({ @@ -96,7 +96,7 @@ function _hasAction(actions: cpapi.Action[], owner: string, provider: string, ca if (configuration && !action.configuration) { continue; } if (configuration) { for (const key of Object.keys(configuration)) { - if (!util.isDeepStrictEqual(cdk.resolve(action.configuration[key]), cdk.resolve(configuration[key]))) { + if (!_.isEqual(cdk.resolve(action.configuration[key]), cdk.resolve(configuration[key]))) { continue; } } @@ -119,10 +119,10 @@ function _grantsPermission(statements: PolicyStatementJson[], action: string, re function _isOrContains(entity: string | string[], value: string): boolean { const resolvedValue = cdk.resolve(value); const resolvedEntity = cdk.resolve(entity); - if (util.isDeepStrictEqual(resolvedEntity, resolvedValue)) { return true; } + if (_.isEqual(resolvedEntity, resolvedValue)) { return true; } if (!Array.isArray(resolvedEntity)) { return false; } for (const tested of entity) { - if (util.isDeepStrictEqual(tested, resolvedValue)) { return true; } + if (_.isEqual(tested, resolvedValue)) { return true; } } return false; } From eef28fb09769e6df49ec713f31d8d29f65424861 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=F0=9F=91=A8=F0=9F=8F=BC=E2=80=8D=F0=9F=92=BB=20Romain=20M?= =?UTF-8?q?arcadier-Muller?= Date: Fri, 5 Oct 2018 10:29:32 +0200 Subject: [PATCH 4/5] Provide actual observed values in eror message --- .../test/test.pipeline-actions.ts | 48 ++++++++++++------- 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/packages/@aws-cdk/aws-cloudformation/test/test.pipeline-actions.ts b/packages/@aws-cdk/aws-cloudformation/test/test.pipeline-actions.ts index d3dd903d44a81..7b795ab764050 100644 --- a/packages/@aws-cdk/aws-cloudformation/test/test.pipeline-actions.ts +++ b/packages/@aws-cdk/aws-cloudformation/test/test.pipeline-actions.ts @@ -19,8 +19,7 @@ export = nodeunit.testCase({ templatePath: artifact.atPath('path/to/file') }); - test.ok(_grantsPermission(pipelineRole.statements, 'iam:PassRole', action.role.roleArn), - 'The pipelineRole was given permissions to iam:PassRole to the action'); + _assertPermissionGranted(test, pipelineRole.statements, 'iam:PassRole', action.role.roleArn); const stackArn = cdk.ArnUtils.fromComponents({ service: 'cloudformation', @@ -28,23 +27,19 @@ export = nodeunit.testCase({ resourceName: 'MyStack/*' }); const changeSetCondition = { StringEquals: { 'cloudformation:ChangeSetName': 'MyChangeSet' } }; - test.ok(_grantsPermission(pipelineRole.statements, 'cloudformation:DescribeStacks', stackArn), - 'The pipelineRole was given permissions to describe the stack & it\'s ChangeSets'); - test.ok(_grantsPermission(pipelineRole.statements, 'cloudformation:DescribeChangeSet', stackArn, changeSetCondition), - 'The pipelineRole was given permissions to describe the desired ChangeSet'); - test.ok(_grantsPermission(pipelineRole.statements, 'cloudformation:CreateChangeSet', stackArn, changeSetCondition), - 'The pipelineRole was given permissions to create the desired ChangeSet'); - test.ok(_grantsPermission(pipelineRole.statements, 'cloudformation:DeleteChangeSet', stackArn, changeSetCondition), - 'The pipelineRole was given permissions to delete the desired ChangeSet'); + _assertPermissionGranted(test, pipelineRole.statements, 'cloudformation:DescribeStacks', stackArn); + _assertPermissionGranted(test, pipelineRole.statements, 'cloudformation:DescribeChangeSet', stackArn, changeSetCondition); + _assertPermissionGranted(test, pipelineRole.statements, 'cloudformation:CreateChangeSet', stackArn, changeSetCondition); + _assertPermissionGranted(test, pipelineRole.statements, 'cloudformation:DeleteChangeSet', stackArn, changeSetCondition); test.deepEqual(action.inputArtifacts, [artifact], 'The inputArtifact was correctly registered'); - test.ok(_hasAction(stage.actions, 'AWS', 'CloudFormation', 'Deploy', { + _assertActionMatches(test, stage.actions, 'AWS', 'CloudFormation', 'Deploy', { ActionMode: 'CHANGE_SET_CREATE_REPLACE', StackName: 'MyStack', ChangeSetName: 'MyChangeSet' - })); + }); test.done(); } @@ -65,16 +60,14 @@ export = nodeunit.testCase({ resource: 'stack', resourceName: 'MyStack/*' }); - test.ok(_grantsPermission(pipelineRole.statements, 'cloudformation:ExecuteChangeSet', stackArn, { - StringEquals: { 'cloudformation:ChangeSetName': 'MyChangeSet' } - }), - 'The pipelineRole was given permissions to execute the desired ChangeSet'); + _assertPermissionGranted(test, pipelineRole.statements, 'cloudformation:ExecuteChangeSet', stackArn, + { StringEquals: { 'cloudformation:ChangeSetName': 'MyChangeSet' } }); - test.ok(_hasAction(stage.actions, 'AWS', 'CloudFormation', 'Deploy', { + _assertActionMatches(test, stage.actions, 'AWS', 'CloudFormation', 'Deploy', { ActionMode: 'CHANGE_SET_EXECUTE', StackName: 'MyStack', ChangeSetName: 'MyChangeSet' - })); + }); test.done(); } @@ -88,6 +81,18 @@ interface PolicyStatementJson { Condition: any; } +function _assertActionMatches(test: nodeunit.Test, + actions: cpapi.Action[], + owner: string, + provider: string, + category: string, + configuration?: { [key: string]: any }) { + const configurationStr = configuration ? `configuration including ${JSON.stringify(configuration, null, 2)}` : ''; + const actionsStr = JSON.stringify(actions, null, 2); + test.ok(_hasAction(actions, owner, provider, category, configuration), + `Expected to find an action with owner ${owner}, provider ${provider}, category ${category}${configurationStr}, but found ${actionsStr}`); +} + function _hasAction(actions: cpapi.Action[], owner: string, provider: string, category: string, configuration?: { [key: string]: any}) { for (const action of actions) { if (action.owner !== owner) { continue; } @@ -106,6 +111,13 @@ function _hasAction(actions: cpapi.Action[], owner: string, provider: string, ca return false; } +function _assertPermissionGranted(test: nodeunit.Test, statements: PolicyStatementJson[], action: string, resource: string, conditions?: any) { + const conditionStr = conditions ? ` with condition(s) ${JSON.stringify(conditions)}` : ''; + const statementsStr = JSON.stringify(statements, null, 2); + test.ok(_grantsPermission(statements, action, resource, conditions), + `Expected to find a statement granting ${action} on ${cdk.resolve(resource)}${conditionStr}, found:\n${statementsStr}`); +} + function _grantsPermission(statements: PolicyStatementJson[], action: string, resource: string, conditions?: any) { for (const statement of statements.filter(s => s.Effect === 'Allow')) { if (!_isOrContains(statement.Action, action)) { continue; } From afcfdc9135f0e77e15c03241f2b1efdfb850a764 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=F0=9F=91=A8=F0=9F=8F=BC=E2=80=8D=F0=9F=92=BB=20Romain=20M?= =?UTF-8?q?arcadier-Muller?= Date: Fri, 5 Oct 2018 10:49:19 +0200 Subject: [PATCH 5/5] Stop trying to resolve constructs --- .../aws-cloudformation/test/test.cloudformation.ts | 8 -------- .../test/test.pipeline-actions.ts | 14 ++++++++++---- 2 files changed, 10 insertions(+), 12 deletions(-) delete mode 100644 packages/@aws-cdk/aws-cloudformation/test/test.cloudformation.ts diff --git a/packages/@aws-cdk/aws-cloudformation/test/test.cloudformation.ts b/packages/@aws-cdk/aws-cloudformation/test/test.cloudformation.ts deleted file mode 100644 index 820f6b467f38f..0000000000000 --- a/packages/@aws-cdk/aws-cloudformation/test/test.cloudformation.ts +++ /dev/null @@ -1,8 +0,0 @@ -import { Test, testCase } from 'nodeunit'; - -export = testCase({ - notTested(test: Test) { - test.ok(true, 'No tests are specified for this package.'); - test.done(); - } -}); diff --git a/packages/@aws-cdk/aws-cloudformation/test/test.pipeline-actions.ts b/packages/@aws-cdk/aws-cloudformation/test/test.pipeline-actions.ts index 7b795ab764050..c251c63c78bf8 100644 --- a/packages/@aws-cdk/aws-cloudformation/test/test.pipeline-actions.ts +++ b/packages/@aws-cdk/aws-cloudformation/test/test.pipeline-actions.ts @@ -87,8 +87,12 @@ function _assertActionMatches(test: nodeunit.Test, provider: string, category: string, configuration?: { [key: string]: any }) { - const configurationStr = configuration ? `configuration including ${JSON.stringify(configuration, null, 2)}` : ''; - const actionsStr = JSON.stringify(actions, null, 2); + const configurationStr = configuration + ? `configuration including ${JSON.stringify(cdk.resolve(configuration), null, 2)}` + : ''; + const actionsStr = JSON.stringify(actions.map(a => + ({ owner: a.owner, provider: a.provider, category: a.category, configuration: cdk.resolve(a.configuration) }) + ), null, 2); test.ok(_hasAction(actions, owner, provider, category, configuration), `Expected to find an action with owner ${owner}, provider ${provider}, category ${category}${configurationStr}, but found ${actionsStr}`); } @@ -112,8 +116,10 @@ function _hasAction(actions: cpapi.Action[], owner: string, provider: string, ca } function _assertPermissionGranted(test: nodeunit.Test, statements: PolicyStatementJson[], action: string, resource: string, conditions?: any) { - const conditionStr = conditions ? ` with condition(s) ${JSON.stringify(conditions)}` : ''; - const statementsStr = JSON.stringify(statements, null, 2); + const conditionStr = conditions + ? ` with condition(s) ${JSON.stringify(cdk.resolve(conditions))}` + : ''; + const statementsStr = JSON.stringify(cdk.resolve(statements), null, 2); test.ok(_grantsPermission(statements, action, resource, conditions), `Expected to find a statement granting ${action} on ${cdk.resolve(resource)}${conditionStr}, found:\n${statementsStr}`); }