Skip to content
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

fix(codebuild): prevent fleet and batch build combined usage #30615

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 34 additions & 1 deletion packages/aws-cdk-lib/aws-codebuild/lib/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,13 @@ export interface IProject extends IResource, iam.IGrantable, ec2.IConnectable, n
/** The IAM service Role of this Project. Undefined for imported Projects. */
readonly role?: iam.IRole;

/**
* If true, batch builds have been enabled.
*
* @default false - batch builds have not been enabled.
*/
readonly isBatchBuildEnabled: boolean;

/**
* Enable batch builds.
*
Expand Down Expand Up @@ -279,6 +286,14 @@ abstract class ProjectBase extends Resource implements IProject {
return this._connections;
}

/**
* Actual value will be determined for a Project
* using a getter depending on the effect of enableBatchBuilds
*/
public get isBatchBuildEnabled(): boolean {
return false;
}

public enableBatchBuilds(): BatchBuildConfig | undefined {
return undefined;
}
Expand Down Expand Up @@ -1045,6 +1060,7 @@ export class Project extends ProjectBase {
private readonly _secondarySources: CfnProject.SourceProperty[];
private readonly _secondarySourceVersions: CfnProject.ProjectSourceVersionProperty[];
private readonly _secondaryArtifacts: CfnProject.ArtifactsProperty[];
private readonly _environment?: CfnProject.EnvironmentProperty;
private _encryptionKey?: kms.IKey;
private readonly _fileSystemLocations: CfnProject.ProjectFileSystemLocationProperty[];
private _batchServiceRole?: iam.Role;
Expand Down Expand Up @@ -1107,6 +1123,8 @@ export class Project extends ProjectBase {
this.addFileSystemLocation(fileSystemLocation);
}

this._environment = this.renderEnvironment(props, environmentVariables);

const resource = new CfnProject(this, 'Resource', {
description: props.description,
source: {
Expand All @@ -1115,7 +1133,7 @@ export class Project extends ProjectBase {
},
artifacts: artifactsConfig.artifactsProperty,
serviceRole: this.role.roleArn,
environment: this.renderEnvironment(props, environmentVariables),
environment: this._environment,
fileSystemLocations: Lazy.any({ produce: () => this.renderFileSystemLocations() }),
// lazy, because we have a setter for it in setEncryptionKey
// The 'alias/aws/s3' default is necessary because leaving the `encryptionKey` field
Expand Down Expand Up @@ -1204,7 +1222,15 @@ export class Project extends ProjectBase {
this.node.addValidation({ validate: () => this.validateProject() });
}

public get isBatchBuildEnabled(): boolean {
return !!this._batchServiceRole;
}

public enableBatchBuilds(): BatchBuildConfig | undefined {
if (this._environment?.fleet) {
throw new Error('Build batch is not supported for project using reserved capacity');
}

if (!this._batchServiceRole) {
this._batchServiceRole = new iam.Role(this, 'BatchServiceRole', {
assumedBy: new iam.ServicePrincipal('codebuild.amazonaws.com'),
Expand Down Expand Up @@ -1426,6 +1452,13 @@ export class Project extends ProjectBase {
return undefined;
}

// !! Failsafe !! This should not occur with the current methods, given:
// * The fleet can only be set with the constructor
// * The batch builds can only be enabled with the `enableBatchBuilds` method
if (this.isBatchBuildEnabled) {
throw new Error('Build batch is not supported for project using reserved capacity');
}
Comment on lines +1455 to +1460
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned in the comment, this condition should never be true. I've added it just in case a future CDK API change enables this behavior

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, this is a good regression check.


// If the fleetArn is resolved, the fleet is imported and we cannot validate the environment type
if (Token.isUnresolved(fleet.fleetArn) && this.buildImage.type !== fleet.environmentType) {
throw new Error(`The environment type of the fleet (${fleet.environmentType}) must match the environment type of the build image (${this.buildImage.type})`);
Expand Down
63 changes: 63 additions & 0 deletions packages/aws-cdk-lib/aws-codebuild/test/codebuild.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { Match, Template } from '../../assertions';
import * as codecommit from '../../aws-codecommit';
import * as codepipeline from '../../aws-codepipeline';
import * as cpactions from '../../aws-codepipeline-actions';
import * as ec2 from '../../aws-ec2';
import * as kms from '../../aws-kms';
import * as s3 from '../../aws-s3';
Expand Down Expand Up @@ -1595,6 +1597,65 @@ test('environment variables can be overridden at the project level', () => {
});
});

test('throws when batch builds are enabled for a reserved capacity project', () => {
// GIVEN
const stack = new cdk.Stack();
const fleet = codebuild.Fleet.fromFleetArn(stack, 'Fleet', 'arn:aws:codebuild:us-east-1:123456789012:fleet/MyFleet:uuid');
const bucket = new s3.Bucket(stack, 'MyBucket');

// WHEN
const project = new codebuild.Project(stack, 'MyProject', {
source: codebuild.Source.s3({
bucket,
path: 'path/to/source.zip',
}),
environment: {
fleet,
buildImage: codebuild.LinuxBuildImage.STANDARD_7_0,
},
});

// THEN
expect(() => {
project.enableBatchBuilds();
}).toThrow(/Build batch is not supported for project using reserved capacity/);
});

test('throws when pipeline are enabled for a reserved capacity project', () => {
// GIVEN
const stack = new cdk.Stack();
const fleet = codebuild.Fleet.fromFleetArn(stack, 'Fleet', 'arn:aws:codebuild:us-east-1:123456789012:fleet/MyFleet:uuid');
const sourceOutput = new codepipeline.Artifact();
const pipeline = new codepipeline.Pipeline(stack, 'Pipeline', {
artifactBucket: new s3.Bucket(stack, 'Bucket', {
versioned: true,
removalPolicy: cdk.RemovalPolicy.DESTROY,
}),
});

// WHEN
const project = new codebuild.PipelineProject(stack, 'MyProject', {
environment: {
fleet,
buildImage: codebuild.LinuxBuildImage.STANDARD_7_0,
},
});

// THEN
expect(() => {
pipeline.addStage({
stageName: 'Build',
actions: [new cpactions.CodeBuildAction({
actionName: 'Build',
project,
executeBatchBuild: true,
input: sourceOutput,
role: pipeline.role,
})],
});
}).toThrow(/Build batch is not supported for project using reserved capacity/);
});

test('.metricXxx() methods can be used to obtain Metrics for CodeBuild projects', () => {
const stack = new cdk.Stack();

Expand Down Expand Up @@ -2043,11 +2104,13 @@ test('enableBatchBuilds()', () => {
repo: 'testrepo',
}),
});
expect(project.isBatchBuildEnabled).toBeFalsy();

const returnVal = project.enableBatchBuilds();
if (!returnVal?.role) {
throw new Error('Expecting return value with role');
}
expect(project.isBatchBuildEnabled).toBeTruthy();

Template.fromStack(stack).hasResourceProperties('AWS::CodeBuild::Project', {
BuildBatchConfig: {
Expand Down
Loading