Skip to content

Commit 3928eae

Browse files
authored
fix(pipelines): pipeline asset role trust policy has account root principal (#30084)
### Reason for this change CDK Pipeline will create a `AssetFileRole` which has trust policy including the root account principal. The root account principal is not needed in this use case and should be removed to scope down trust policy. ### Description of changes Adding a new feature flag `PIPELINE_REDUCE_ASSET_ROLE_TRUST_SCOPE` with default value `true`. When the feature flag is enabled, remove the root account principal from the trust policy. When the feature flag is disabled, keep the old behavior. Using the feature flag here in case of customers are using the root account principal and it will allow them to turn off this change. ### Description of how you validated changes Unit test/Integration Test Manually tested in cross-account pipeline ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 2c53cf9 commit 3928eae

File tree

13 files changed

+114
-65
lines changed

13 files changed

+114
-65
lines changed

packages/@aws-cdk-testing/framework-integ/test/pipelines/test/integ.newpipeline-with-file-system-locations.js

+2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ const codebuild = require("aws-cdk-lib/aws-codebuild");
55
const ec2 = require("aws-cdk-lib/aws-ec2");
66
const s3 = require("aws-cdk-lib/aws-s3");
77
const s3_assets = require("aws-cdk-lib/aws-s3-assets");
8+
const cx_api_1 = require("aws-cdk-lib/cx-api");
89
const aws_cdk_lib_1 = require("aws-cdk-lib");
910
const integ = require("@aws-cdk/integ-tests-alpha");
1011
const pipelines = require("aws-cdk-lib/pipelines");
@@ -54,6 +55,7 @@ const app = new aws_cdk_lib_1.App({
5455
postCliContext: {
5556
'@aws-cdk/core:newStyleStackSynthesis': '1',
5657
'@aws-cdk/aws-codepipeline:defaultPipelineTypeToV2': false,
58+
[cx_api_1.PIPELINE_REDUCE_ASSET_ROLE_TRUST_SCOPE]: true,
5759
},
5860
});
5961
const stack = new TestStack(app, 'PipelinesFileSystemLocations');

packages/@aws-cdk-testing/framework-integ/test/pipelines/test/integ.newpipeline-with-file-system-locations.js.snapshot/PipelinesFileSystemLocations.assets.json

+2-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/framework-integ/test/pipelines/test/integ.newpipeline-with-file-system-locations.js.snapshot/PipelinesFileSystemLocations.template.json

-16
Original file line numberDiff line numberDiff line change
@@ -1504,22 +1504,6 @@
15041504
"Action": "sts:AssumeRole",
15051505
"Effect": "Allow",
15061506
"Principal": {
1507-
"AWS": {
1508-
"Fn::Join": [
1509-
"",
1510-
[
1511-
"arn:",
1512-
{
1513-
"Ref": "AWS::Partition"
1514-
},
1515-
":iam::",
1516-
{
1517-
"Ref": "AWS::AccountId"
1518-
},
1519-
":root"
1520-
]
1521-
]
1522-
},
15231507
"Service": "codebuild.amazonaws.com"
15241508
}
15251509
}

packages/@aws-cdk-testing/framework-integ/test/pipelines/test/integ.newpipeline-with-file-system-locations.js.snapshot/manifest.json

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/framework-integ/test/pipelines/test/integ.newpipeline-with-file-system-locations.js.snapshot/tree.json

-16
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/framework-integ/test/pipelines/test/integ.newpipeline-with-file-system-locations.ts

+2
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import * as codebuild from 'aws-cdk-lib/aws-codebuild';
33
import * as ec2 from 'aws-cdk-lib/aws-ec2';
44
import * as s3 from 'aws-cdk-lib/aws-s3';
55
import * as s3_assets from 'aws-cdk-lib/aws-s3-assets';
6+
import { PIPELINE_REDUCE_ASSET_ROLE_TRUST_SCOPE } from 'aws-cdk-lib/cx-api';
67
import { App, Stack, StackProps, Stage, StageProps, Aws, RemovalPolicy, DefaultStackSynthesizer } from 'aws-cdk-lib';
78
import * as integ from '@aws-cdk/integ-tests-alpha';
89
import { Construct } from 'constructs';
@@ -61,6 +62,7 @@ const app = new App({
6162
postCliContext: {
6263
'@aws-cdk/core:newStyleStackSynthesis': '1',
6364
'@aws-cdk/aws-codepipeline:defaultPipelineTypeToV2': false,
65+
[PIPELINE_REDUCE_ASSET_ROLE_TRUST_SCOPE]: true,
6466
},
6567
});
6668

packages/@aws-cdk-testing/framework-integ/test/pipelines/test/integ.newpipeline-with-vpc.ts

+2
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import * as sqs from 'aws-cdk-lib/aws-sqs';
77
import { App, Stack, StackProps, Stage, StageProps } from 'aws-cdk-lib';
88
import { Construct } from 'constructs';
99
import * as pipelines from 'aws-cdk-lib/pipelines';
10+
import { PIPELINE_REDUCE_ASSET_ROLE_TRUST_SCOPE } from 'aws-cdk-lib/cx-api';
1011

1112
class PipelineStack extends Stack {
1213
constructor(scope: Construct, id: string, props?: StackProps) {
@@ -50,6 +51,7 @@ const app = new App({
5051
postCliContext: {
5152
'@aws-cdk/core:newStyleStackSynthesis': '1',
5253
'@aws-cdk/aws-codepipeline:defaultPipelineTypeToV2': false,
54+
[PIPELINE_REDUCE_ASSET_ROLE_TRUST_SCOPE]: false,
5355
},
5456
});
5557
new PipelineStack(app, 'PipelineStack');

packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md

+20-1
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ Flags come in three types:
6969
| [@aws-cdk/aws-kms:reduceCrossAccountRegionPolicyScope](#aws-cdkaws-kmsreducecrossaccountregionpolicyscope) | When enabled, IAM Policy created from KMS key grant will reduce the resource scope to this key only. | 2.134.0 | (fix) |
7070
| [@aws-cdk/aws-eks:nodegroupNameAttribute](#aws-cdkaws-eksnodegroupnameattribute) | When enabled, nodegroupName attribute of the provisioned EKS NodeGroup will not have the cluster name prefix. | 2.139.0 | (fix) |
7171
| [@aws-cdk/aws-ec2:ebsDefaultGp3Volume](#aws-cdkaws-ec2ebsdefaultgp3volume) | When enabled, the default volume type of the EBS volume will be GP3 | 2.140.0 | (default) |
72+
| [@aws-cdk/pipelines:reduceAssetRoleTrustScope](#aws-cdkpipelinesreduceassetroletrustscope) | Remove the root account principal from PipelineAssetsFileRole trust policy | V2NEXT | (default) |
7273

7374
<!-- END table -->
7475

@@ -171,6 +172,7 @@ are migrating a v1 CDK project to v2, explicitly set any of these flags which do
171172
| [@aws-cdk/aws-apigateway:usagePlanKeyOrderInsensitiveId](#aws-cdkaws-apigatewayusageplankeyorderinsensitiveid) | Allow adding/removing multiple UsagePlanKeys independently | (fix) | 1.98.0 | `false` | `true` |
172173
| [@aws-cdk/aws-lambda:recognizeVersionProps](#aws-cdkaws-lambdarecognizeversionprops) | Enable this feature flag to opt in to the updated logical id calculation for Lambda Version created using the `fn.currentVersion`. | (fix) | 1.106.0 | `false` | `true` |
173174
| [@aws-cdk/aws-cloudfront:defaultSecurityPolicyTLSv1.2\_2021](#aws-cdkaws-cloudfrontdefaultsecuritypolicytlsv12_2021) | Enable this feature flag to have cloudfront distributions use the security policy TLSv1.2_2021 by default. | (fix) | 1.117.0 | `false` | `true` |
175+
| [@aws-cdk/pipelines:reduceAssetRoleTrustScope](#aws-cdkpipelinesreduceassetroletrustscope) | Remove the root account principal from PipelineAssetsFileRole trust policy | (default) | | `false` | `true` |
174176

175177
<!-- END diff -->
176178

@@ -185,7 +187,8 @@ Here is an example of a `cdk.json` file that restores v1 behavior for these flag
185187
"@aws-cdk/aws-rds:lowercaseDbIdentifier": false,
186188
"@aws-cdk/aws-apigateway:usagePlanKeyOrderInsensitiveId": false,
187189
"@aws-cdk/aws-lambda:recognizeVersionProps": false,
188-
"@aws-cdk/aws-cloudfront:defaultSecurityPolicyTLSv1.2_2021": false
190+
"@aws-cdk/aws-cloudfront:defaultSecurityPolicyTLSv1.2_2021": false,
191+
"@aws-cdk/pipelines:reduceAssetRoleTrustScope": false
189192
}
190193
}
191194
```
@@ -1298,4 +1301,20 @@ When this featuer flag is enabled, the default volume type of the EBS volume wil
12981301
**Compatibility with old behavior:** Pass `volumeType: EbsDeviceVolumeType.GENERAL_PURPOSE_SSD` to `Volume` construct to restore the previous behavior.
12991302

13001303

1304+
### @aws-cdk/pipelines:reduceAssetRoleTrustScope
1305+
1306+
*Remove the root account principal from PipelineAssetsFileRole trust policy* (default)
1307+
1308+
When this feature flag is enabled, the root account principal will not be added to the trust policy of asset role.
1309+
When this feature flag is disabled, it will keep the root account principal in the trust policy.
1310+
1311+
1312+
| Since | Default | Recommended |
1313+
| ----- | ----- | ----- |
1314+
| (not in v1) | | |
1315+
| V2NEXT | `true` | `true` |
1316+
1317+
**Compatibility with old behavior:** Disable the feature flag to add the root account principal back
1318+
1319+
13011320
<!-- END details -->

packages/aws-cdk-lib/cx-api/lib/features.ts

+15
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ export const LAMBDA_PERMISSION_LOGICAL_ID_FOR_LAMBDA_ACTION = '@aws-cdk/aws-clou
101101
export const CODEPIPELINE_CROSS_ACCOUNT_KEYS_DEFAULT_VALUE_TO_FALSE = '@aws-cdk/aws-codepipeline:crossAccountKeysDefaultValueToFalse';
102102
export const CODEPIPELINE_DEFAULT_PIPELINE_TYPE_TO_V2 = '@aws-cdk/aws-codepipeline:defaultPipelineTypeToV2';
103103
export const KMS_REDUCE_CROSS_ACCOUNT_REGION_POLICY_SCOPE = '@aws-cdk/aws-kms:reduceCrossAccountRegionPolicyScope';
104+
export const PIPELINE_REDUCE_ASSET_ROLE_TRUST_SCOPE = '@aws-cdk/pipelines:reduceAssetRoleTrustScope';
104105
export const EKS_NODEGROUP_NAME = '@aws-cdk/aws-eks:nodegroupNameAttribute';
105106
export const EBS_DEFAULT_GP3 = '@aws-cdk/aws-ec2:ebsDefaultGp3Volume';
106107

@@ -1037,6 +1038,20 @@ export const FLAGS: Record<string, FlagInfo> = {
10371038
recommendedValue: true,
10381039
},
10391040

1041+
//////////////////////////////////////////////////////////////////////
1042+
[PIPELINE_REDUCE_ASSET_ROLE_TRUST_SCOPE]: {
1043+
type: FlagType.ApiDefault,
1044+
summary: 'Remove the root account principal from PipelineAssetsFileRole trust policy',
1045+
detailsMd: `
1046+
When this feature flag is enabled, the root account principal will not be added to the trust policy of asset role.
1047+
When this feature flag is disabled, it will keep the root account principal in the trust policy.
1048+
`,
1049+
introducedIn: { v2: 'V2NEXT' },
1050+
defaults: { v2: true },
1051+
recommendedValue: true,
1052+
compatibilityWithOldBehaviorMd: 'Disable the feature flag to add the root account principal back',
1053+
},
1054+
10401055
//////////////////////////////////////////////////////////////////////
10411056
[EKS_NODEGROUP_NAME]: {
10421057
type: FlagType.BugFix,

packages/aws-cdk-lib/cx-api/test/features.test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ test('feature flag defaults may not be changed anymore', () => {
3838
[feats.LAMBDA_RECOGNIZE_VERSION_PROPS]: true,
3939
[feats.CLOUDFRONT_DEFAULT_SECURITY_POLICY_TLS_V1_2_2021]: true,
4040
// Add new disabling feature flags below this line
41-
// ...
41+
[feats.PIPELINE_REDUCE_ASSET_ROLE_TRUST_SCOPE]: true,
4242

4343
});
4444
});

packages/aws-cdk-lib/pipelines/lib/codepipeline/codepipeline.ts

+13-6
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import * as cpa from '../../../aws-codepipeline-actions';
1313
import * as ec2 from '../../../aws-ec2';
1414
import * as iam from '../../../aws-iam';
1515
import * as s3 from '../../../aws-s3';
16-
import { Aws, CfnCapabilities, Duration, PhysicalName, Stack, Names } from '../../../core';
16+
import { Aws, CfnCapabilities, Duration, PhysicalName, Stack, Names, FeatureFlags } from '../../../core';
1717
import * as cxapi from '../../../cx-api';
1818
import { AssetType, FileSet, IFileSetProducer, ManualApprovalStep, ShellStep, StackAsset, StackDeployment, Step } from '../blueprint';
1919
import { DockerCredential, dockerCredentialsInstallCommands, DockerCredentialUsage } from '../docker-credentials';
@@ -984,13 +984,20 @@ export class CodePipeline extends PipelineBase {
984984

985985
const stack = Stack.of(this);
986986

987-
const rolePrefix = assetType === AssetType.DOCKER_IMAGE ? 'Docker' : 'File';
988-
const assetRole = new AssetSingletonRole(this.assetsScope, `${rolePrefix}Role`, {
989-
roleName: PhysicalName.GENERATE_IF_NEEDED,
990-
assumedBy: new iam.CompositePrincipal(
987+
const removeRootPrincipal = FeatureFlags.of(this).isEnabled(cxapi.PIPELINE_REDUCE_ASSET_ROLE_TRUST_SCOPE);
988+
989+
const assumePrincipal = removeRootPrincipal ? new iam.CompositePrincipal(
990+
new iam.ServicePrincipal('codebuild.amazonaws.com'),
991+
) :
992+
new iam.CompositePrincipal(
991993
new iam.ServicePrincipal('codebuild.amazonaws.com'),
992994
new iam.AccountPrincipal(stack.account),
993-
),
995+
);
996+
997+
const rolePrefix = assetType === AssetType.DOCKER_IMAGE ? 'Docker' : 'File';
998+
let assetRole = new AssetSingletonRole(this.assetsScope, `${rolePrefix}Role`, {
999+
roleName: PhysicalName.GENERATE_IF_NEEDED,
1000+
assumedBy: assumePrincipal,
9941001
});
9951002

9961003
// Grant pull access for any ECR registries and secrets that exist

packages/aws-cdk-lib/pipelines/test/codepipeline/codepipeline.test.ts

+56
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import * as s3 from '../../../aws-s3';
77
import * as sqs from '../../../aws-sqs';
88
import * as cdk from '../../../core';
99
import { Stack } from '../../../core';
10+
import * as cxapi from '../../../cx-api';
1011
import * as cdkp from '../../lib';
1112
import { CodePipeline } from '../../lib';
1213
import { PIPELINE_ENV, TestApp, ModernTestGitHubNpmPipeline, FileAssetApp, TwoStackApp, StageWithStackOutput } from '../testhelpers';
@@ -197,6 +198,61 @@ test('CodeBuild action role has the right AssumeRolePolicyDocument', () => {
197198
});
198199
});
199200

201+
test('CodeBuild asset role has the right Principal with the feature enabled', () => {
202+
const stack = new cdk.Stack();
203+
stack.node.setContext(cxapi.PIPELINE_REDUCE_ASSET_ROLE_TRUST_SCOPE, true);
204+
const pipelineStack = new cdk.Stack(stack, 'PipelineStack', { env: PIPELINE_ENV });
205+
const pipeline = new ModernTestGitHubNpmPipeline(pipelineStack, 'Cdk');
206+
pipeline.addStage(new FileAssetApp(pipelineStack, 'App', {}));;
207+
const template = Template.fromStack(pipelineStack);
208+
const assetRole = template.toJSON().Resources.CdkAssetsFileRole6BE17A07;
209+
const statementLength = assetRole.Properties.AssumeRolePolicyDocument.Statement;
210+
expect(statementLength).toStrictEqual(
211+
[
212+
{
213+
Action: 'sts:AssumeRole',
214+
Effect: 'Allow',
215+
Principal: {
216+
Service: 'codebuild.amazonaws.com',
217+
},
218+
},
219+
],
220+
);
221+
});
222+
223+
test('CodeBuild asset role has the right Principal with the feature disabled', () => {
224+
const stack = new cdk.Stack();
225+
stack.node.setContext(cxapi.PIPELINE_REDUCE_ASSET_ROLE_TRUST_SCOPE, false);
226+
const pipelineStack = new cdk.Stack(stack, 'PipelineStack', { env: PIPELINE_ENV });
227+
const pipeline = new ModernTestGitHubNpmPipeline(pipelineStack, 'Cdk');
228+
pipeline.addStage(new FileAssetApp(pipelineStack, 'App', {}));;
229+
const template = Template.fromStack(pipelineStack);
230+
const assetRole = template.toJSON().Resources.CdkAssetsFileRole6BE17A07;
231+
const statementLength = assetRole.Properties.AssumeRolePolicyDocument.Statement;
232+
expect(statementLength).toStrictEqual(
233+
[
234+
{
235+
Action: 'sts:AssumeRole',
236+
Effect: 'Allow',
237+
Principal: {
238+
Service: 'codebuild.amazonaws.com',
239+
},
240+
},
241+
{
242+
Action: 'sts:AssumeRole',
243+
Effect: 'Allow',
244+
Principal: {
245+
AWS: {
246+
'Fn::Join': ['', [
247+
'arn:', { Ref: 'AWS::Partition' }, `:iam::${PIPELINE_ENV.account}:root`,
248+
]],
249+
},
250+
},
251+
},
252+
],
253+
);
254+
});
255+
200256
test('CodePipeline throws when key rotation is enabled without enabling cross account keys', ()=>{
201257
const pipelineStack = new cdk.Stack(app, 'PipelineStack', { env: PIPELINE_ENV });
202258
const repo = new ccommit.Repository(pipelineStack, 'Repo', {

packages/aws-cdk-lib/pipelines/test/compliance/assets.test.ts

-22
Original file line numberDiff line numberDiff line change
@@ -396,17 +396,6 @@ describe('basic pipeline', () => {
396396
Service: 'codebuild.amazonaws.com',
397397
},
398398
},
399-
{
400-
Action: 'sts:AssumeRole',
401-
Effect: 'Allow',
402-
Principal: {
403-
AWS: {
404-
'Fn::Join': ['', [
405-
'arn:', { Ref: 'AWS::Partition' }, `:iam::${PIPELINE_ENV.account}:root`,
406-
]],
407-
},
408-
},
409-
},
410399
],
411400
},
412401
});
@@ -510,17 +499,6 @@ describe('basic pipeline', () => {
510499
Service: 'codebuild.amazonaws.com',
511500
},
512501
},
513-
{
514-
Action: 'sts:AssumeRole',
515-
Effect: 'Allow',
516-
Principal: {
517-
AWS: {
518-
'Fn::Join': ['', [
519-
'arn:', { Ref: 'AWS::Partition' }, `:iam::${PIPELINE_ENV.account}:root`,
520-
]],
521-
},
522-
},
523-
},
524502
],
525503
},
526504
});

0 commit comments

Comments
 (0)