From 14d8a47e7b8ab29ee78d9ac7e87694c1772f899e Mon Sep 17 00:00:00 2001 From: Nick Lynch Date: Wed, 29 Jul 2020 17:42:45 +0100 Subject: [PATCH 1/2] fix(pipelines): reduce assets IAM policy size Collapse the PipelineAssetsRoleDefaultPolicy into a single up-front policy that doesn't grow per-asset. This relaxes some of the permissions in exchange for avoiding an O(N) policy size. fixes #9316 --- packages/@aws-cdk/pipelines/lib/pipeline.ts | 77 ++++++++++++++++++--- 1 file changed, 67 insertions(+), 10 deletions(-) diff --git a/packages/@aws-cdk/pipelines/lib/pipeline.ts b/packages/@aws-cdk/pipelines/lib/pipeline.ts index 6dd7af9e472ed..54d9addef4e26 100644 --- a/packages/@aws-cdk/pipelines/lib/pipeline.ts +++ b/packages/@aws-cdk/pipelines/lib/pipeline.ts @@ -245,7 +245,8 @@ class AssetPublishing extends Construct { private readonly myCxAsmRoot: string; private readonly stage: codepipeline.IStage; - private assetRole?: iam.Role; + private readonly pipeline: codepipeline.Pipeline; + private assetRole?: iam.IRole; private _fileAssetCtr = 1; private _dockerAssetCtr = 1; @@ -256,6 +257,7 @@ class AssetPublishing extends Construct { // We MUST add the Stage immediately here, otherwise it will be in the wrong place // in the pipeline! this.stage = this.props.pipeline.addStage({ stageName: 'Assets' }); + this.pipeline = this.props.pipeline; } /** @@ -269,15 +271,9 @@ class AssetPublishing extends Construct { // FIXME: this is silly, we need the relative path here but no easy way to get it const relativePath = path.relative(this.myCxAsmRoot, command.assetManifestPath); - // This role is used by both the CodePipeline build action and related CodeBuild project. Consolidating these two - // roles into one, and re-using across all assets, saves significant size of the final synthesized output. - // Modeled after the CodePipeline role and 'CodePipelineActionRole' roles. - // Late-binding here to prevent creating the role in cases where no asset actions are created. + // Late-binding here (rather than in the constructor) to prevent creating the role in cases where no asset actions are created. if (!this.assetRole) { - this.assetRole = new iam.Role(this, 'Role', { - roleName: PhysicalName.GENERATE_IF_NEEDED, - assumedBy: new iam.CompositePrincipal(new iam.ServicePrincipal('codebuild.amazonaws.com'), new iam.AccountPrincipal(Stack.of(this).account)), - }); + this.generateAssetRole(); } let action = this.publishers[command.assetId]; @@ -321,9 +317,70 @@ class AssetPublishing extends Construct { } } } + + /** + * This role is used by both the CodePipeline build action and related CodeBuild project. Consolidating these two + * roles into one, and re-using across all assets, saves significant size of the final synthesized output. + * Modeled after the CodePipeline role and 'CodePipelineActionRole' roles. + */ + private generateAssetRole() { + const assetRole = new iam.Role(this, 'Role', { + roleName: PhysicalName.GENERATE_IF_NEEDED, + assumedBy: new iam.CompositePrincipal(new iam.ServicePrincipal('codebuild.amazonaws.com'), new iam.AccountPrincipal(Stack.of(this).account)), + }); + + // Logging permissions + const logGroupArn = Stack.of(this).formatArn({ + service: 'logs', + resource: 'log-group', + sep: ':', + resourceName: '/aws/codebuild/*', + }); + assetRole.addToPolicy(new iam.PolicyStatement({ + resources: [logGroupArn], + actions: ['logs:CreateLogGroup', 'logs:CreateLogStream', 'logs:PutLogEvents'], + })); + + // CodeBuild report groups + const codeBuildArn = Stack.of(this).formatArn({ + service: 'codebuild', + resource: 'report-group', + resourceName: '*', + }); + assetRole.addToPolicy(new iam.PolicyStatement({ + actions: [ + 'codebuild:CreateReportGroup', + 'codebuild:CreateReport', + 'codebuild:UpdateReport', + 'codebuild:BatchPutTestCases', + ], + resources: [codeBuildArn], + })); + + // CodeBuild start/stop + assetRole.addToPolicy(new iam.PolicyStatement({ + resources: ['*'], + actions: [ + 'codebuild:BatchGetBuilds', + 'codebuild:StartBuild', + 'codebuild:StopBuild', + ], + })); + + // Publishing role access + assetRole.addToPolicy(new iam.PolicyStatement({ + actions: ['sts:AssumeRole'], + resources: ['arn:*:iam::*:role/*-image-publishing-role-*', 'arn:*:iam::*:role/*-file-publishing-role-*'], + })); + + this.pipeline.artifactBucket.grantRead(assetRole); + + this.assetRole = assetRole.withoutPolicyUpdates(); + return this.assetRole; + } } function maybeSuffix(x: string | undefined, suffix: string): string | undefined { if (x === undefined) { return undefined; } return `${x}${suffix}`; -} \ No newline at end of file +} From aec6d71fa5011d4252e803c840849e0b15dcb2da Mon Sep 17 00:00:00 2001 From: Nick Lynch Date: Thu, 30 Jul 2020 12:55:58 +0100 Subject: [PATCH 2/2] Split asset roles by asset type, added tests --- packages/@aws-cdk/pipelines/lib/pipeline.ts | 26 ++- .../integ.pipeline-with-assets.expected.json | 183 +++--------------- .../pipelines/test/pipeline-assets.test.ts | 107 ++++++++++ 3 files changed, 153 insertions(+), 163 deletions(-) diff --git a/packages/@aws-cdk/pipelines/lib/pipeline.ts b/packages/@aws-cdk/pipelines/lib/pipeline.ts index 54d9addef4e26..03120bf7c9866 100644 --- a/packages/@aws-cdk/pipelines/lib/pipeline.ts +++ b/packages/@aws-cdk/pipelines/lib/pipeline.ts @@ -242,11 +242,11 @@ interface AssetPublishingProps { */ class AssetPublishing extends Construct { private readonly publishers: Record = {}; + private readonly assetRoles: Record = {}; private readonly myCxAsmRoot: string; private readonly stage: codepipeline.IStage; private readonly pipeline: codepipeline.Pipeline; - private assetRole?: iam.IRole; private _fileAssetCtr = 1; private _dockerAssetCtr = 1; @@ -272,8 +272,8 @@ class AssetPublishing extends Construct { const relativePath = path.relative(this.myCxAsmRoot, command.assetManifestPath); // Late-binding here (rather than in the constructor) to prevent creating the role in cases where no asset actions are created. - if (!this.assetRole) { - this.generateAssetRole(); + if (!this.assetRoles[command.assetType]) { + this.generateAssetRole(command.assetType); } let action = this.publishers[command.assetId]; @@ -294,7 +294,7 @@ class AssetPublishing extends Construct { cloudAssemblyInput: this.props.cloudAssemblyInput, cdkCliVersion: this.props.cdkCliVersion, assetType: command.assetType, - role: this.assetRole, + role: this.assetRoles[command.assetType], }); this.stage.addAction(action); } @@ -322,9 +322,13 @@ class AssetPublishing extends Construct { * This role is used by both the CodePipeline build action and related CodeBuild project. Consolidating these two * roles into one, and re-using across all assets, saves significant size of the final synthesized output. * Modeled after the CodePipeline role and 'CodePipelineActionRole' roles. + * Generates one role per asset type to separate file and Docker/image-based permissions. */ - private generateAssetRole() { - const assetRole = new iam.Role(this, 'Role', { + private generateAssetRole(assetType: AssetType) { + if (this.assetRoles[assetType]) { return this.assetRoles[assetType]; } + + const rolePrefix = assetType === AssetType.DOCKER_IMAGE ? 'Docker' : 'File'; + const assetRole = new iam.Role(this, `${rolePrefix}Role`, { roleName: PhysicalName.GENERATE_IF_NEEDED, assumedBy: new iam.CompositePrincipal(new iam.ServicePrincipal('codebuild.amazonaws.com'), new iam.AccountPrincipal(Stack.of(this).account)), }); @@ -368,15 +372,19 @@ class AssetPublishing extends Construct { })); // Publishing role access + const rolePattern = assetType === AssetType.DOCKER_IMAGE + ? 'arn:*:iam::*:role/*-image-publishing-role-*' + : 'arn:*:iam::*:role/*-file-publishing-role-*'; assetRole.addToPolicy(new iam.PolicyStatement({ actions: ['sts:AssumeRole'], - resources: ['arn:*:iam::*:role/*-image-publishing-role-*', 'arn:*:iam::*:role/*-file-publishing-role-*'], + resources: [rolePattern], })); + // Artifact access this.pipeline.artifactBucket.grantRead(assetRole); - this.assetRole = assetRole.withoutPolicyUpdates(); - return this.assetRole; + this.assetRoles[assetType] = assetRole.withoutPolicyUpdates(); + return this.assetRoles[assetType]; } } diff --git a/packages/@aws-cdk/pipelines/test/integ.pipeline-with-assets.expected.json b/packages/@aws-cdk/pipelines/test/integ.pipeline-with-assets.expected.json index 7795321cf451a..058eb255d030a 100644 --- a/packages/@aws-cdk/pipelines/test/integ.pipeline-with-assets.expected.json +++ b/packages/@aws-cdk/pipelines/test/integ.pipeline-with-assets.expected.json @@ -344,7 +344,7 @@ "Principal": { "AWS": { "Fn::GetAtt": [ - "PipelineAssetsRole9B011B83", + "PipelineAssetsFileRole59943A77", "Arn" ] } @@ -362,7 +362,7 @@ "Principal": { "AWS": { "Fn::GetAtt": [ - "PipelineAssetsRole9B011B83", + "PipelineAssetsFileRole59943A77", "Arn" ] } @@ -561,7 +561,7 @@ "Effect": "Allow", "Resource": { "Fn::GetAtt": [ - "PipelineAssetsRole9B011B83", + "PipelineAssetsFileRole59943A77", "Arn" ] } @@ -723,7 +723,7 @@ "Name": "FileAsset1", "RoleArn": { "Fn::GetAtt": [ - "PipelineAssetsRole9B011B83", + "PipelineAssetsFileRole59943A77", "Arn" ] }, @@ -749,7 +749,7 @@ "Name": "FileAsset2", "RoleArn": { "Fn::GetAtt": [ - "PipelineAssetsRole9B011B83", + "PipelineAssetsFileRole59943A77", "Arn" ] }, @@ -1413,7 +1413,7 @@ } } }, - "PipelineAssetsRole9B011B83": { + "PipelineAssetsFileRole59943A77": { "Type": "AWS::IAM::Role", "Properties": { "AssumeRolePolicyDocument": { @@ -1442,7 +1442,7 @@ } } }, - "PipelineAssetsRoleDefaultPolicyB41726CA": { + "PipelineAssetsFileRoleDefaultPolicy14DB8755": { "Type": "AWS::IAM::Policy", "Properties": { "PolicyDocument": { @@ -1454,39 +1454,18 @@ "logs:PutLogEvents" ], "Effect": "Allow", - "Resource": [ - { - "Fn::Join": [ - "", - [ - "arn:", - { - "Ref": "AWS::Partition" - }, - ":logs:test-region:12345678:log-group:/aws/codebuild/", - { - "Ref": "PipelineAssetsFileAsset185A67CB4" - } - ] - ] - }, - { - "Fn::Join": [ - "", - [ - "arn:", - { - "Ref": "AWS::Partition" - }, - ":logs:test-region:12345678:log-group:/aws/codebuild/", - { - "Ref": "PipelineAssetsFileAsset185A67CB4" - }, - ":*" - ] + "Resource": { + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":logs:test-region:12345678:log-group:/aws/codebuild/*" ] - } - ] + ] + } }, { "Action": [ @@ -1504,20 +1483,11 @@ { "Ref": "AWS::Partition" }, - ":codebuild:test-region:12345678:report-group/", - { - "Ref": "PipelineAssetsFileAsset185A67CB4" - }, - "-*" + ":codebuild:test-region:12345678:report-group/*" ] ] } }, - { - "Action": "sts:AssumeRole", - "Effect": "Allow", - "Resource": "arn:*:iam::*:role/*-file-publishing-role-*" - }, { "Action": [ "codebuild:BatchGetBuilds", @@ -1525,12 +1495,12 @@ "codebuild:StopBuild" ], "Effect": "Allow", - "Resource": { - "Fn::GetAtt": [ - "PipelineAssetsFileAsset185A67CB4", - "Arn" - ] - } + "Resource": "*" + }, + { + "Action": "sts:AssumeRole", + "Effect": "Allow", + "Resource": "arn:*:iam::*:role/*-file-publishing-role-*" }, { "Action": [ @@ -1574,109 +1544,14 @@ "Arn" ] } - }, - { - "Action": [ - "kms:Decrypt", - "kms:Encrypt", - "kms:ReEncrypt*", - "kms:GenerateDataKey*" - ], - "Effect": "Allow", - "Resource": { - "Fn::GetAtt": [ - "PipelineArtifactsBucketEncryptionKeyF5BF0670", - "Arn" - ] - } - }, - { - "Action": [ - "logs:CreateLogGroup", - "logs:CreateLogStream", - "logs:PutLogEvents" - ], - "Effect": "Allow", - "Resource": [ - { - "Fn::Join": [ - "", - [ - "arn:", - { - "Ref": "AWS::Partition" - }, - ":logs:test-region:12345678:log-group:/aws/codebuild/", - { - "Ref": "PipelineAssetsFileAsset24D2D639B" - } - ] - ] - }, - { - "Fn::Join": [ - "", - [ - "arn:", - { - "Ref": "AWS::Partition" - }, - ":logs:test-region:12345678:log-group:/aws/codebuild/", - { - "Ref": "PipelineAssetsFileAsset24D2D639B" - }, - ":*" - ] - ] - } - ] - }, - { - "Action": [ - "codebuild:CreateReportGroup", - "codebuild:CreateReport", - "codebuild:UpdateReport", - "codebuild:BatchPutTestCases" - ], - "Effect": "Allow", - "Resource": { - "Fn::Join": [ - "", - [ - "arn:", - { - "Ref": "AWS::Partition" - }, - ":codebuild:test-region:12345678:report-group/", - { - "Ref": "PipelineAssetsFileAsset24D2D639B" - }, - "-*" - ] - ] - } - }, - { - "Action": [ - "codebuild:BatchGetBuilds", - "codebuild:StartBuild", - "codebuild:StopBuild" - ], - "Effect": "Allow", - "Resource": { - "Fn::GetAtt": [ - "PipelineAssetsFileAsset24D2D639B", - "Arn" - ] - } } ], "Version": "2012-10-17" }, - "PolicyName": "PipelineAssetsRoleDefaultPolicyB41726CA", + "PolicyName": "PipelineAssetsFileRoleDefaultPolicy14DB8755", "Roles": [ { - "Ref": "PipelineAssetsRole9B011B83" + "Ref": "PipelineAssetsFileRole59943A77" } ] } @@ -1695,7 +1570,7 @@ }, "ServiceRole": { "Fn::GetAtt": [ - "PipelineAssetsRole9B011B83", + "PipelineAssetsFileRole59943A77", "Arn" ] }, @@ -1725,7 +1600,7 @@ }, "ServiceRole": { "Fn::GetAtt": [ - "PipelineAssetsRole9B011B83", + "PipelineAssetsFileRole59943A77", "Arn" ] }, diff --git a/packages/@aws-cdk/pipelines/test/pipeline-assets.test.ts b/packages/@aws-cdk/pipelines/test/pipeline-assets.test.ts index 084fe467413bb..41c05e322fa54 100644 --- a/packages/@aws-cdk/pipelines/test/pipeline-assets.test.ts +++ b/packages/@aws-cdk/pipelines/test/pipeline-assets.test.ts @@ -173,6 +173,60 @@ test('can control fix/CLI version used in pipeline selfupdate', () => { }); }); +describe('asset roles and policies', () => { + test('includes file publishing assets role for apps with file assets', () => { + pipeline.addApplicationStage(new FileAssetApp(app, 'App1')); + + expect(pipelineStack).toHaveResourceLike('AWS::IAM::Role', { + AssumeRolePolicyDocument: { + Statement: [{ + Action: 'sts:AssumeRole', + Effect: 'Allow', + Principal: { + Service: 'codebuild.amazonaws.com', + AWS: { 'Fn::Join': [ '', [ + 'arn:', { Ref: 'AWS::Partition' }, `:iam::${PIPELINE_ENV.account}:root`, + ] ] }, + }, + }], + }, + }); + expect(pipelineStack).toHaveResourceLike('AWS::IAM::Policy', + expectedAssetRolePolicy('arn:*:iam::*:role/*-file-publishing-role-*', 'CdkAssetsFileRole6BE17A07')); + }); + + test('includes image publishing assets role for apps with Docker assets', () => { + pipeline.addApplicationStage(new DockerAssetApp(app, 'App1')); + + expect(pipelineStack).toHaveResourceLike('AWS::IAM::Role', { + AssumeRolePolicyDocument: { + Statement: [{ + Action: 'sts:AssumeRole', + Effect: 'Allow', + Principal: { + Service: 'codebuild.amazonaws.com', + AWS: { 'Fn::Join': [ '', [ + 'arn:', { Ref: 'AWS::Partition' }, `:iam::${PIPELINE_ENV.account}:root`, + ] ] }, + }, + }], + }, + }); + expect(pipelineStack).toHaveResourceLike('AWS::IAM::Policy', + expectedAssetRolePolicy('arn:*:iam::*:role/*-image-publishing-role-*', 'CdkAssetsDockerRole484B6DD3')); + }); + + test('includes both roles for apps with both file and Docker assets', () => { + pipeline.addApplicationStage(new FileAssetApp(app, 'App1')); + pipeline.addApplicationStage(new DockerAssetApp(app, 'App2')); + + expect(pipelineStack).toHaveResourceLike('AWS::IAM::Policy', + expectedAssetRolePolicy('arn:*:iam::*:role/*-file-publishing-role-*', 'CdkAssetsFileRole6BE17A07')); + expect(pipelineStack).toHaveResourceLike('AWS::IAM::Policy', + expectedAssetRolePolicy('arn:*:iam::*:role/*-image-publishing-role-*', 'CdkAssetsDockerRole484B6DD3')); + }); +}); + class PlainStackApp extends Stage { constructor(scope: Construct, id: string, props?: StageProps) { super(scope, id, props); @@ -212,3 +266,56 @@ class DockerAssetApp extends Stage { }); } } + +function expectedAssetRolePolicy(assumeRolePattern: string, attachedRole: string) { + return { + PolicyDocument: { + Statement: [{ + Action: ['logs:CreateLogGroup', 'logs:CreateLogStream', 'logs:PutLogEvents'], + Effect: 'Allow', + Resource: { + 'Fn::Join': ['', [ + 'arn:', + {Ref: 'AWS::Partition'}, + `:logs:${PIPELINE_ENV.region}:${PIPELINE_ENV.account}:log-group:/aws/codebuild/*`, + ]], + }, + }, + { + Action: ['codebuild:CreateReportGroup', 'codebuild:CreateReport', 'codebuild:UpdateReport', 'codebuild:BatchPutTestCases'], + Effect: 'Allow', + Resource: { + 'Fn::Join': ['', [ + 'arn:', + {Ref: 'AWS::Partition'}, + `:codebuild:${PIPELINE_ENV.region}:${PIPELINE_ENV.account}:report-group/*`, + ]], + }, + }, + { + Action: ['codebuild:BatchGetBuilds', 'codebuild:StartBuild', 'codebuild:StopBuild'], + Effect: 'Allow', + Resource: '*', + }, + { + Action: 'sts:AssumeRole', + Effect: 'Allow', + Resource: assumeRolePattern, + }, + { + Action: ['s3:GetObject*', 's3:GetBucket*', 's3:List*'], + Effect: 'Allow', + Resource: [ + { 'Fn::GetAtt': ['CdkPipelineArtifactsBucket7B46C7BF', 'Arn' ] }, + { 'Fn::Join': ['', [{'Fn::GetAtt': ['CdkPipelineArtifactsBucket7B46C7BF', 'Arn']}, '/*']] }, + ], + }, + { + Action: ['kms:Decrypt', 'kms:DescribeKey'], + Effect: 'Allow', + Resource: { 'Fn::GetAtt': [ 'CdkPipelineArtifactsBucketEncryptionKeyDDD3258C', 'Arn' ]}, + }], + }, + Roles: [ {Ref: attachedRole} ], + }; +}