-
Notifications
You must be signed in to change notification settings - Fork 3.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(aws-codebuild): add enableBatchBuilds()
to Project
#12531
Changes from 11 commits
8eade4c
dddcb69
9981c8a
88339ad
e98e12d
0ec5784
48617a2
2aa8ccf
5275bc3
26fde2e
c445ca7
769472b
c5d7ce2
5481254
74d758f
aeda32a
0d7d183
1a1244b
ecf030d
339e0ea
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,6 +44,14 @@ export interface IProject extends IResource, iam.IGrantable, ec2.IConnectable { | |
/** The IAM service Role of this Project. Undefined for imported Projects. */ | ||
readonly role?: iam.IRole; | ||
|
||
/** | ||
* Enable batch builds. | ||
* | ||
* Returns an object contining the batch service role if batch builds | ||
* could be enabled. | ||
*/ | ||
enableBatchBuilds(): IEnableBatchBuildsReturn | undefined; | ||
|
||
addToRolePolicy(policyStatement: iam.PolicyStatement): void; | ||
|
||
/** | ||
|
@@ -156,6 +164,14 @@ export interface IProject extends IResource, iam.IGrantable, ec2.IConnectable { | |
metricFailedBuilds(props?: cloudwatch.MetricOptions): cloudwatch.Metric; | ||
} | ||
|
||
/** | ||
* The return value for the {@link Project} `enableBatchBuilds()` method. | ||
*/ | ||
export interface IEnableBatchBuildsReturn { | ||
/** The IAM batch service Role of this Project. */ | ||
readonly role: iam.IRole; | ||
} | ||
|
||
/** | ||
* Represents a reference to a CodeBuild Project. | ||
* | ||
|
@@ -196,6 +212,10 @@ abstract class ProjectBase extends Resource implements IProject { | |
return this._connections; | ||
} | ||
|
||
public enableBatchBuilds(): IEnableBatchBuildsReturn | undefined { | ||
return undefined; | ||
} | ||
|
||
/** | ||
* Add a permission only if there's a policy attached. | ||
* @param statement The permissions statement to add | ||
|
@@ -729,6 +749,7 @@ export class Project extends ProjectBase { | |
private readonly _secondaryArtifacts: CfnProject.ArtifactsProperty[]; | ||
private _encryptionKey?: kms.IKey; | ||
private readonly _fileSystemLocations: CfnProject.ProjectFileSystemLocationProperty[]; | ||
private _batchServiceRole?: iam.Role; | ||
|
||
constructor(scope: Construct, id: string, props: ProjectProps) { | ||
super(scope, id, { | ||
|
@@ -813,6 +834,14 @@ export class Project extends ProjectBase { | |
sourceVersion: sourceConfig.sourceVersion, | ||
vpcConfig: this.configureVpc(props), | ||
logsConfig: this.renderLoggingConfiguration(props.logging), | ||
buildBatchConfig: Lazy.any({ | ||
produce: () => { | ||
const config: CfnProject.ProjectBuildBatchConfigProperty | undefined = this._batchServiceRole ? { | ||
serviceRole: this._batchServiceRole.roleArn, | ||
} : undefined; | ||
return config; | ||
}, | ||
}), | ||
skinny85 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}); | ||
|
||
this.addVpcRequiredPermissions(props, resource); | ||
|
@@ -853,6 +882,24 @@ export class Project extends ProjectBase { | |
} | ||
} | ||
|
||
public enableBatchBuilds(): IEnableBatchBuildsReturn | undefined { | ||
if (!this._batchServiceRole) { | ||
this._batchServiceRole = new iam.Role(this, 'BatchServiceRole', { | ||
roleName: PhysicalName.GENERATE_IF_NEEDED, | ||
tjenkinson marked this conversation as resolved.
Show resolved
Hide resolved
|
||
assumedBy: new iam.ServicePrincipal('codebuild.amazonaws.com'), | ||
}); | ||
this._batchServiceRole.addToPrincipalPolicy(new iam.PolicyStatement({ | ||
resources: [Lazy.string({ | ||
skinny85 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
produce: () => this.projectArn, | ||
})], | ||
actions: ['codebuild:StartBuild', 'codebuild:StopBuild', 'codebuild:RetryBuild'], | ||
tjenkinson marked this conversation as resolved.
Show resolved
Hide resolved
|
||
})); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't we need some batch permissions here? Like this? https://github.com/aws/aws-cdk/pull/12181/files?file-filters%5B%5D=.ts#diff-7902b8e1bfd8571028c1c97116a58c4f6e090f92d7df3490e16e12defbfa2fab There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think so. The docs don't mention it and it worked when I tested this in a different account. Think this is so that internally non batch builds can be triggered as part of a batch. Don't think a batch can trigger another batch? |
||
} | ||
return { | ||
role: this._batchServiceRole, | ||
}; | ||
} | ||
|
||
/** | ||
* Adds a secondary source to the Project. | ||
* | ||
|
@@ -1139,8 +1186,8 @@ export class Project extends ProjectBase { | |
return undefined; | ||
} | ||
|
||
let s3Config: CfnProject.S3LogsConfigProperty|undefined = undefined; | ||
let cloudwatchConfig: CfnProject.CloudWatchLogsConfigProperty|undefined = undefined; | ||
let s3Config: CfnProject.S3LogsConfigProperty | undefined = undefined; | ||
let cloudwatchConfig: CfnProject.CloudWatchLogsConfigProperty | undefined = undefined; | ||
|
||
if (props.s3) { | ||
const s3Logs = props.s3; | ||
|
@@ -1350,10 +1397,10 @@ export interface IBuildImage { | |
} | ||
|
||
/** Optional arguments to {@link IBuildImage.binder} - currently empty. */ | ||
export interface BuildImageBindOptions {} | ||
export interface BuildImageBindOptions { } | ||
|
||
/** The return type from {@link IBuildImage.binder} - currently empty. */ | ||
export interface BuildImageConfig {} | ||
export interface BuildImageConfig { } | ||
|
||
// @deprecated(not in tsdoc on purpose): add bind() to IBuildImage | ||
// and get rid of IBindableBuildImage | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,188 @@ | ||
{ | ||
"Resources": { | ||
"MyProjectRole9BBE5233": { | ||
"Type": "AWS::IAM::Role", | ||
"Properties": { | ||
"AssumeRolePolicyDocument": { | ||
"Statement": [ | ||
{ | ||
"Action": "sts:AssumeRole", | ||
"Effect": "Allow", | ||
"Principal": { | ||
"Service": "codebuild.amazonaws.com" | ||
} | ||
} | ||
], | ||
"Version": "2012-10-17" | ||
} | ||
} | ||
}, | ||
"MyProjectRoleDefaultPolicyB19B7C29": { | ||
"Type": "AWS::IAM::Policy", | ||
"Properties": { | ||
"PolicyDocument": { | ||
"Statement": [ | ||
{ | ||
"Action": [ | ||
"logs:CreateLogGroup", | ||
"logs:CreateLogStream", | ||
"logs:PutLogEvents" | ||
], | ||
"Effect": "Allow", | ||
"Resource": [ | ||
{ | ||
"Fn::Join": [ | ||
"", | ||
[ | ||
"arn:", | ||
{ | ||
"Ref": "AWS::Partition" | ||
}, | ||
":logs:", | ||
{ | ||
"Ref": "AWS::Region" | ||
}, | ||
":", | ||
{ | ||
"Ref": "AWS::AccountId" | ||
}, | ||
":log-group:/aws/codebuild/", | ||
{ | ||
"Ref": "MyProject39F7B0AE" | ||
} | ||
] | ||
] | ||
}, | ||
{ | ||
"Fn::Join": [ | ||
"", | ||
[ | ||
"arn:", | ||
{ | ||
"Ref": "AWS::Partition" | ||
}, | ||
":logs:", | ||
{ | ||
"Ref": "AWS::Region" | ||
}, | ||
":", | ||
{ | ||
"Ref": "AWS::AccountId" | ||
}, | ||
":log-group:/aws/codebuild/", | ||
{ | ||
"Ref": "MyProject39F7B0AE" | ||
}, | ||
":*" | ||
] | ||
] | ||
} | ||
] | ||
} | ||
], | ||
"Version": "2012-10-17" | ||
}, | ||
"PolicyName": "MyProjectRoleDefaultPolicyB19B7C29", | ||
"Roles": [ | ||
{ | ||
"Ref": "MyProjectRole9BBE5233" | ||
} | ||
] | ||
} | ||
}, | ||
"MyProjectBatchServiceRole6B35CF0E": { | ||
"Type": "AWS::IAM::Role", | ||
"Properties": { | ||
"AssumeRolePolicyDocument": { | ||
"Statement": [ | ||
{ | ||
"Action": "sts:AssumeRole", | ||
"Effect": "Allow", | ||
"Principal": { | ||
"Service": "codebuild.amazonaws.com" | ||
} | ||
} | ||
], | ||
"Version": "2012-10-17" | ||
} | ||
} | ||
}, | ||
"MyProjectBatchServiceRoleDefaultPolicy7A0E5721": { | ||
"Type": "AWS::IAM::Policy", | ||
"Properties": { | ||
"PolicyDocument": { | ||
"Statement": [ | ||
{ | ||
"Action": [ | ||
"codebuild:StartBuild", | ||
"codebuild:StopBuild", | ||
"codebuild:RetryBuild" | ||
], | ||
"Effect": "Allow", | ||
"Resource": { | ||
"Fn::GetAtt": [ | ||
"MyProject39F7B0AE", | ||
"Arn" | ||
] | ||
} | ||
} | ||
], | ||
"Version": "2012-10-17" | ||
}, | ||
"PolicyName": "MyProjectBatchServiceRoleDefaultPolicy7A0E5721", | ||
"Roles": [ | ||
{ | ||
"Ref": "MyProjectBatchServiceRole6B35CF0E" | ||
} | ||
] | ||
} | ||
}, | ||
"MyProject39F7B0AE": { | ||
"Type": "AWS::CodeBuild::Project", | ||
"Properties": { | ||
"Artifacts": { | ||
"Type": "NO_ARTIFACTS" | ||
}, | ||
"Environment": { | ||
"ComputeType": "BUILD_GENERAL1_SMALL", | ||
"Image": "aws/codebuild/standard:1.0", | ||
"ImagePullCredentialsType": "CODEBUILD", | ||
"PrivilegedMode": false, | ||
"Type": "LINUX_CONTAINER" | ||
}, | ||
"ServiceRole": { | ||
"Fn::GetAtt": [ | ||
"MyProjectRole9BBE5233", | ||
"Arn" | ||
] | ||
}, | ||
"Source": { | ||
"Location": "https://github.com/aws/aws-cdk.git", | ||
"ReportBuildStatus": false, | ||
"Type": "GITHUB" | ||
}, | ||
"BuildBatchConfig": { | ||
"ServiceRole": { | ||
"Fn::GetAtt": [ | ||
"MyProjectBatchServiceRole6B35CF0E", | ||
"Arn" | ||
] | ||
} | ||
}, | ||
"EncryptionKey": "alias/aws/s3", | ||
"Triggers": { | ||
"BuildType": "BUILD_BATCH", | ||
"FilterGroups": [ | ||
[ | ||
{ | ||
"Pattern": "PUSH", | ||
"Type": "EVENT" | ||
} | ||
] | ||
], | ||
"Webhook": true | ||
} | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
import * as cdk from '@aws-cdk/core'; | ||
import * as codebuild from '../lib'; | ||
|
||
class TestStack extends cdk.Stack { | ||
tjenkinson marked this conversation as resolved.
Show resolved
Hide resolved
|
||
constructor(scope: cdk.App, id: string) { | ||
super(scope, id); | ||
|
||
const source = codebuild.Source.gitHub({ | ||
owner: 'aws', | ||
repo: 'aws-cdk', | ||
reportBuildStatus: false, | ||
tjenkinson marked this conversation as resolved.
Show resolved
Hide resolved
|
||
webhook: true, | ||
webhookTriggersBatchBuild: true, | ||
webhookFilters: [ | ||
codebuild.FilterGroup.inEventOf(codebuild.EventAction.PUSH), | ||
], | ||
}); | ||
new codebuild.Project(this, 'MyProject', { | ||
source, | ||
grantReportGroupPermissions: false, | ||
}); | ||
} | ||
} | ||
|
||
const app = new cdk.App(); | ||
|
||
new TestStack(app, 'test-codebuild-github-webhook-batch'); | ||
|
||
app.synth(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@skinny85 I named this this instead of
BatchBuildConfig
because I didn't want it to get confused with the actualbuildBatchConfig
in L1There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Meh. The type on the L1 level is called
ProjectBuildBatchConfigProperty
, so I'm not too worried about this one being calledBatchBuildConfig
.The name is not a coincidence - it's actually a convention we use for return types (see examples [1], [2]).
A name ending in
Return
seems a little weird given that, so if you could change it, I would appreciate it 🙂.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No problem will update then. Thanks for the examples