Skip to content

Commit

Permalink
fix(pipelines): policy size too large at around ~70 actions
Browse files Browse the repository at this point in the history
Two changes:

- Collapse CodeBuild action Roles: each CodeBuild step used to create a
  fresh Role to run the CodeBuild action. Change to use one Role for all
  CodeBuild actions. This saves a lot of resources and policy space when
  using a lot of CodeBuild steps, and doesn't appreciably change the
  security posture of the Pipeline (note: this is *not* about the
  Execution Role of the CodeBuild projects, this is about the Role
  assumed by the Pipeline to initiate execution of the Project).
- Grant AssumeRole on all asset/deployment roles by *tag*, instead of
  by ARN. Because all those types of roles will have the same tags
  associated with them, we don't have to enumerate all ARNs but can
  suffice with a single tag-based authorization statement.

Although this formally changes the cloud-assembly-schema, it doesn't
involve the CLI, so I've opted out of bumping the schema version to
not force unnecessary CLI upgrades.
  • Loading branch information
rix0rrr committed Apr 29, 2022
1 parent 5472b11 commit 95fe9ba
Show file tree
Hide file tree
Showing 20 changed files with 317 additions and 30 deletions.
8 changes: 1 addition & 7 deletions packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts
Expand Up @@ -708,13 +708,7 @@ export class Pipeline extends PipelineBase {
}

// the pipeline role needs assumeRole permissions to the action role
if (actionRole) {
this.role.addToPrincipalPolicy(new iam.PolicyStatement({
actions: ['sts:AssumeRole'],
resources: [actionRole.roleArn],
}));
}

actionRole?.grantAssumeRole(this.role);
return actionRole;
}

Expand Down
7 changes: 7 additions & 0 deletions packages/@aws-cdk/aws-iam/lib/lazy-role.ts
Expand Up @@ -119,6 +119,13 @@ export class LazyRole extends cdk.Resource implements IRole {
return this.instantiate().grantPassRole(identity);
}

/**
* Grant permissions to the given principal to assume this role.
*/
public grantAssumeRole(identity: IPrincipal): Grant {
return this.instantiate().grantAssumeRole(identity);
}

private instantiate(): Role {
if (!this.role) {
const role = new Role(this, 'Default', this.props);
Expand Down
4 changes: 4 additions & 0 deletions packages/@aws-cdk/aws-iam/lib/private/immutable-role.ts
Expand Up @@ -67,4 +67,8 @@ export class ImmutableRole extends Resource implements IRole {
public grantPassRole(grantee: IPrincipal): Grant {
return this.role.grantPassRole(grantee);
}

public grantAssumeRole(identity: IPrincipal): Grant {
return this.role.grantAssumeRole(identity);
}
}
20 changes: 20 additions & 0 deletions packages/@aws-cdk/aws-iam/lib/role.ts
Expand Up @@ -246,6 +246,13 @@ export class Role extends Resource implements IRole {
return this.grant(identity, 'iam:PassRole');
}

/**
* Grant permissions to the given principal to pass this role.
*/
public grantAssumeRole(identity: IPrincipal): Grant {
return this.grant(identity, 'sts:AssumeRole');
}

/**
* Grant the actions defined in actions to the identity Principal on this resource.
*/
Expand Down Expand Up @@ -444,6 +451,14 @@ export class Role extends Resource implements IRole {
return this.grant(identity, 'iam:PassRole');
}

/**
* Grant permissions to the given principal to assume this role.
*/
public grantAssumeRole(identity: IPrincipal) {
return this.grant(identity, 'sts:AssumeRole');
}


/**
* Return a copy of this Role object whose Policies will not be updated
*
Expand Down Expand Up @@ -498,6 +513,11 @@ export interface IRole extends IIdentity {
* Grant permissions to the given principal to pass this role.
*/
grantPassRole(grantee: IPrincipal): Grant;

/**
* Grant permissions to the given principal to assume this role.
*/
grantAssumeRole(grantee: IPrincipal): Grant;
}

function createAssumeRolePolicy(principal: IPrincipal, externalIds: string[]) {
Expand Down
24 changes: 24 additions & 0 deletions packages/@aws-cdk/aws-iam/test/role.test.ts
Expand Up @@ -59,6 +59,30 @@ describe('IAM role', () => {
});
});

test('a role can grant AssumeRole permissions', () => {
// GIVEN
const stack = new Stack();
const role = new Role(stack, 'Role', { assumedBy: new ServicePrincipal('henk.amazonaws.com') });
const user = new User(stack, 'User');

// WHEN
role.grantAssumeRole(user);

// THEN
Template.fromStack(stack).hasResourceProperties('AWS::IAM::Policy', {
PolicyDocument: {
Statement: [
{
Action: 'sts:AssumeRole',
Effect: 'Allow',
Resource: { 'Fn::GetAtt': ['Role1ABCC5F0', 'Arn'] },
},
],
Version: '2012-10-17',
},
});
});

testDeprecated('can supply externalId', () => {
// GIVEN
const stack = new Stack();
Expand Down
Expand Up @@ -12,14 +12,29 @@ export interface AwsDestination {
/**
* The role that needs to be assumed while publishing this asset
*
* The `assumeRole` fields is the preferred way of passing this information,
* but this needs to be supported for backwards compatibility.
*
* @default - No role will be assumed
*/
readonly assumeRoleArn?: string;

/**
* The ExternalId that needs to be supplied while assuming this role
*
* The `assumeRole` fields is the preferred way of passing this information,
* but this needs to be supported for backwards compatibility.
*
* @default - No ExternalId will be supplied
*/
readonly assumeRoleExternalId?: string;

/**
* Tags associated with the given role
*
* This information may be used to create IAM policies targeting this role.
*
* @default - No tags
*/
readonly assumeRoleTags?: Record<string, string>;
}
Expand Up @@ -30,6 +30,15 @@ export interface BootstrapRole {
* @default - Discover SSM parameter by reading stack
*/
readonly bootstrapStackVersionSsmParameter?: string;

/**
* Tags associated with the given role
*
* This information may be used to create IAM policies targeting this role.
*
* @default - No tags
*/
readonly tags?: Record<string, string>;
}

/**
Expand Down Expand Up @@ -71,17 +80,32 @@ export interface AwsCloudFormationStackProperties {
/**
* The role that needs to be assumed to deploy the stack
*
* The `assumeRole` field is the preferred way to pass this information, but this field
* needs to be supported for backwards compatibility.
*
* @default - No role is assumed (current credentials are used)
*/
readonly assumeRoleArn?: string;

/**
* External ID to use when assuming role for cloudformation deployments
*
* The `assumeRole` field is the preferred way to pass this information, but this field
* needs to be supported for backwards compatibility.
*
* @default - No external ID
*/
readonly assumeRoleExternalId?: string;

/**
* Tags associated with the given role
*
* This information may be used to create IAM policies targeting this role.
*
* @default - No tags
*/
readonly assumeRoleTags?: Record<string, string>;

/**
* The role that is passed to CloudFormation to execute the change set
*
Expand Down
19 changes: 15 additions & 4 deletions packages/@aws-cdk/cloud-assembly-schema/schema/assets.schema.json
Expand Up @@ -91,19 +91,26 @@
"type": "string"
},
"assumeRoleArn": {
"description": "The role that needs to be assumed while publishing this asset (Default - No role will be assumed)",
"description": "The role that needs to be assumed while publishing this asset\n\nThe `assumeRole` fields is the preferred way of passing this information,\nbut this needs to be supported for backwards compatibility. (Default - No role will be assumed)",
"type": "string"
},
"assumeRoleExternalId": {
"description": "The ExternalId that needs to be supplied while assuming this role (Default - No ExternalId will be supplied)",
"description": "The ExternalId that needs to be supplied while assuming this role\n\nThe `assumeRole` fields is the preferred way of passing this information,\nbut this needs to be supported for backwards compatibility. (Default - No ExternalId will be supplied)",
"type": "string"
},
"assumeRoleTags": {
"description": "Tags associated with the given role\n\nThis information may be used to create IAM policies targeting this role. (Default - No tags)",
"$ref": "#/definitions/Record<string,string>"
}
},
"required": [
"bucketName",
"objectKey"
]
},
"Record<string,string>": {
"type": "object"
},
"DockerImageAsset": {
"description": "A file asset",
"type": "object",
Expand Down Expand Up @@ -178,12 +185,16 @@
"type": "string"
},
"assumeRoleArn": {
"description": "The role that needs to be assumed while publishing this asset (Default - No role will be assumed)",
"description": "The role that needs to be assumed while publishing this asset\n\nThe `assumeRole` fields is the preferred way of passing this information,\nbut this needs to be supported for backwards compatibility. (Default - No role will be assumed)",
"type": "string"
},
"assumeRoleExternalId": {
"description": "The ExternalId that needs to be supplied while assuming this role (Default - No ExternalId will be supplied)",
"description": "The ExternalId that needs to be supplied while assuming this role\n\nThe `assumeRole` fields is the preferred way of passing this information,\nbut this needs to be supported for backwards compatibility. (Default - No ExternalId will be supplied)",
"type": "string"
},
"assumeRoleTags": {
"description": "Tags associated with the given role\n\nThis information may be used to create IAM policies targeting this role. (Default - No tags)",
"$ref": "#/definitions/Record<string,string>"
}
},
"required": [
Expand Down
Expand Up @@ -300,13 +300,17 @@
"type": "boolean"
},
"assumeRoleArn": {
"description": "The role that needs to be assumed to deploy the stack (Default - No role is assumed (current credentials are used))",
"description": "The role that needs to be assumed to deploy the stack\n\nThe `assumeRole` field is the preferred way to pass this information, but this field\nneeds to be supported for backwards compatibility. (Default - No role is assumed (current credentials are used))",
"type": "string"
},
"assumeRoleExternalId": {
"description": "External ID to use when assuming role for cloudformation deployments (Default - No external ID)",
"description": "External ID to use when assuming role for cloudformation deployments\n\nThe `assumeRole` field is the preferred way to pass this information, but this field\nneeds to be supported for backwards compatibility. (Default - No external ID)",
"type": "string"
},
"assumeRoleTags": {
"description": "Tags associated with the given role\n\nThis information may be used to create IAM policies targeting this role. (Default - No tags)",
"$ref": "#/definitions/Record<string,string>"
},
"cloudFormationExecutionRoleArn": {
"description": "The role that is passed to CloudFormation to execute the change set (Default - No role is passed (currently assumed role/credentials are used))",
"type": "string"
Expand Down Expand Up @@ -336,6 +340,9 @@
"templateFile"
]
},
"Record<string,string>": {
"type": "object"
},
"BootstrapRole": {
"description": "Information needed to access an IAM role created\nas part of the bootstrap process",
"type": "object",
Expand All @@ -355,6 +362,10 @@
"bootstrapStackVersionSsmParameter": {
"description": "Name of SSM parameter with bootstrap stack version (Default - Discover SSM parameter by reading stack)",
"type": "string"
},
"tags": {
"description": "Tags associated with the given role\n\nThis information may be used to create IAM policies targeting this role. (Default - No tags)",
"$ref": "#/definitions/Record<string,string>"
}
},
"required": [
Expand Down
Expand Up @@ -47,6 +47,7 @@ export class AssetManifestBuilder {
region: resolvedOr(stack.region, undefined),
assumeRoleArn: role?.assumeRoleArn,
assumeRoleExternalId: role?.assumeRoleExternalId,
assumeRoleTags: role?.tags,
},
},
};
Expand Down Expand Up @@ -101,6 +102,7 @@ export class AssetManifestBuilder {
region: resolvedOr(stack.region, undefined),
assumeRoleArn: role?.assumeRoleArn,
assumeRoleExternalId: role?.assumeRoleExternalId,
assumeRoleTags: role?.tags,
},
},
};
Expand Down Expand Up @@ -160,6 +162,7 @@ export class AssetManifestBuilder {
export interface RoleOptions {
readonly assumeRoleArn?: string;
readonly assumeRoleExternalId?: string;
readonly tags?: Record<string, string>;
}

function validateFileAssetSource(asset: FileAssetSource) {
Expand Down

0 comments on commit 95fe9ba

Please sign in to comment.