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

[nmussy]

Issue # (if applicable)

None, initially reported by @michaelbwebb, see #30596

Reason for this change

The following stack would cause a CloudFormation deployment error:

const app = new cdk.App();
const stack = new cdk.Stack(app, 'aws-cdk-pipeline-project-fleet');

const repository = new codecommit.Repository(stack, 'MyRepo', {
  repositoryName: 'integration-repo',
});

const fleet = new codebuild.Fleet(stack, 'MyFleet', {
  fleetName: 'MyFleet',
  baseCapacity: 1,
  computeType: codebuild.FleetComputeType.SMALL,
  environmentType: codebuild.EnvironmentType.LINUX_CONTAINER,
});

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

const artifactBucket = new s3.Bucket(stack, 'MyBucket', {
  versioned: true,
  removalPolicy: cdk.RemovalPolicy.DESTROY,
});
const pipeline = new codepipeline.Pipeline(stack, 'Pipeline', {
  artifactBucket,
});

const sourceOutput = new codepipeline.Artifact();
pipeline.addStage({
  stageName: 'Source',
  actions: [new cpactions.CodeCommitSourceAction({
    actionName: 'Source',
    repository,
    output: sourceOutput,
    role: pipeline.role,
  })],
});

// project.enableBatchBuilds() is invoked here:
pipeline.addStage({
  stageName: 'Build',
  actions: [new cpactions.CodeBuildAction({
    actionName: 'Build',
    project,
    executeBatchBuild: true,
    input: sourceOutput,
    role: pipeline.role,
  })],
});

I initially thought that the fleet property was not correctly set for PipelineProjects, but this is only tangentially related CodePipeline, see Working with reserved capacity in AWS CodeBuild:

Reserved capacity fleets don't support batch builds

This is reflected by the CloudFormation error thrown by the above stack:

Build batch is not supported for project using reserved capacity

While I understand the CDK does not need to double check every CloudFormation exception, fleets can be slow to deploy, and particularly slow to destroy. This process can be costly depending on the compute resource allocation, which in my opinion warrants a synth time check

Description of changes

• Added check and error if a project has both a fleet and batch builds enabled
  • Added public Project.isBatchBuildEnabled property, giving the ability to check whether enableBatchBuilds() has been invoked
  • Added private Project._environment property, to retrieve the environment value synthesized in the constructor method

Description of how you validated changes

Added a unit test. The CloudFormation error was found whilst attempting to add an integration test for a reserved capacity PipelineProject

Checklist

• My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[aws-cdk-automation]

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

[nmussy]

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

[scanlonp]

Agree, this is a good regression check.

[nmussy]

Exemption Request: This is a synth time check, a unit test should be sufficient

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: 1402d51
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[aws-cdk-automation]

This PR has been in the CHANGES REQUESTED state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.

[scanlonp]

Overall I like the idea here. This case seems fairly easy to run in to, and the error from CloudFormation is not immediately clear on how you have messed up your CDK configuration, so I support the synth-time check.

We have ran into a couple issues with synth-time checks being overly restrictive, so a couple questions on hypothetical cases where a user could run into this.

• Is there any way for a user to set the fleet property but not use reserved capacity? (This may be the definition of the CFN fleet resource, but wanted to make sure theres no way to set on-demand fleets with this property)
• Is there a way for a user to pass in a "dummy" fleet for their environment, which would not result in reserved capacity, but would still trigger this check?

I'd expect no to both, but still worth a consideration.

Lastly, I don't quite understand the boolean variable and the getter method. I do not believe both are needed. The variable is never set, and the getter method evaluates a different variable. Could you set the variable to true in the enableBatchBuilds() method, and then reference it directly in configureFleet()? Or could the function be the evaluation without the corresponding variable?

Of the two, I would prefer the direct set of the variable, but up to you.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

[nmussy]

Hey @scanlonp, thanks for the review and sorry for the late reply.

Is there any way for a user to set the fleet property but not use reserved capacity? (This may be the definition of the CFN fleet resource, but wanted to make sure theres no way to set on-demand fleets with this property)

Not as far as I can tell, setting a fleet to a CodeBuild project makes it become a reserved capacity project, see Working with reserved capacity in AWS CodeBuild

Is there a way for a user to pass in a "dummy" fleet for their environment, which would not result in reserved capacity, but would still trigger this check?

I'm not sure how they would do that. The BaseCapacity must be greater than 0, and they must either be EC2 managed on-demand instances, or user managed reserved instances. I suppose you could use trashed reserved instances, but from what I've seen when I implemented #29754, if there are no instances able or available to run a reserved build project, it just doesn't build.

Lastly, I don't quite understand the boolean variable and the getter method.

The boolean value and the getter method are one and the same. isBatchBuildEnabled is defined as a boolean value in the interface, and implemented as getter in the two classes. But you're right, I could switch to a primitive boolean member variable and set it in enableBatchBuilds(), or remove it altogether and derive its value on the fly. Let me know which you would prefer.