feat(codebuild): Lambda compute for codebuild projects #27934

sakurai-ryo · 2023-11-10T18:33:17Z

CodeBuild has added support for Lambda compute.
CloudFormation can be deployed as follows.

Resources:
  CodeBuildProject:
    Type: AWS::CodeBuild::Project
    Properties:
      Artifacts:
        Type: NO_ARTIFACTS
      ServiceRole: !GetAtt CodeBuildRole.Arn
      Source:
        # 
      Environment:
        Type: LINUX_LAMBDA_CONTAINER
        ComputeType: BUILD_LAMBDA_1GB
        Image: aws/codebuild/amazonlinux-x86_64-lambda-standard:go1.21
  CodeBuildRole:
    Type: AWS::IAM::Role
    Properties:
        #

https://aws.amazon.com/about-aws/whats-new/2023/11/aws-codebuild-lambda-compute

This PR implements Lambda ComputeType by adding Classes (LinuxArmLambdaBuildImage, LinuxLambdaBuildImage) that extend the IBuildImage interface.

Supported Docker Images and ComputeTypes are listed below.
https://docs.aws.amazon.com/codebuild/latest/userguide/build-env-ref-available.html
https://docs.aws.amazon.com/codebuild/latest/userguide/build-env-ref-compute-types.html#environment.types

Also, Lambda compute has some limitations and I have added validation for them.
https://docs.aws.amazon.com/codebuild/latest/userguide/lambda.html#lambda.limitations

closes #28418

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

… undefined

…add-lambda-compute-suport

packages/aws-cdk-lib/aws-codebuild/test/linux-arm-lambda-build-image.test.ts

lpizzinidev

Thanks for the contribution! 👍
Looks good overall, just some notes on documentation and code structure.

lpizzinidev · 2023-11-12T09:21:27Z

packages/aws-cdk-lib/aws-codebuild/lib/linux-arm-lambda-build-image.ts

+ * @see https://docs.aws.amazon.com/codebuild/latest/userguide/build-env-ref-available.html
+ */
+export class LinuxArmLambdaBuildImage implements IBuildImage {
+  /** Image "aws/codebuild/amazonlinux-aarch64-lambda-standard:nodejs18". */


Suggested change

/** Image "aws/codebuild/amazonlinux-aarch64-lambda-standard:nodejs18". */

/** The `aws/codebuild/amazonlinux-aarch64-lambda-standard:nodejs18` build image. */

Let's keep this format.

lpizzinidev · 2023-11-12T09:30:39Z

packages/aws-cdk-lib/aws-codebuild/lib/linux-arm-lambda-build-image.ts

+ * This class has a bunch of public constants that represent the CodeBuild aarch64 images.
+ *


Suggested change

* This class has a bunch of public constants that represent the CodeBuild aarch64 images.

*

* This class has a bunch of public constants that represent the CodeBuild aarch64 Lambda images.

lpizzinidev · 2023-11-12T09:36:13Z

packages/aws-cdk-lib/aws-codebuild/lib/linux-arm-lambda-build-image.ts

+  /**
+   * Validates by checking unsupported property and compute type
+   * @param buildEnvironment BuildEnvironment
+   */


Suggested change

/**

* Validates by checking unsupported property and compute type

* @param buildEnvironment BuildEnvironment

*/

Let's keep the interface documentation.

lpizzinidev · 2023-11-12T09:40:01Z

packages/aws-cdk-lib/aws-codebuild/lib/linux-arm-lambda-build-image.ts

+    const ret = [];
+
+    if (buildEnvironment.privileged) {
+      ret.push('Lambda compute type does not support Privileged mode');


Suggested change

ret.push('Lambda compute type does not support Privileged mode');

ret.push('Lambda compute type does not support privileged mode');

Since we are here, can you please also add the validation in WindowsBuildImage? (see last note)

Same validation and testing added for WindowsBuildImage.

lpizzinidev · 2023-11-12T09:47:18Z

packages/aws-cdk-lib/aws-codebuild/lib/linux-lambda-build-image.ts

+ * This class has a bunch of public constants that represent the CodeBuild x86-64 images.
+ *


Suggested change

* This class has a bunch of public constants that represent the CodeBuild x86-64 images.

*

* This class has a bunch of public constants that represent the CodeBuild Lambda x86-64 images.

lpizzinidev · 2023-11-12T09:57:17Z

packages/aws-cdk-lib/aws-codebuild/lib/project.ts

+    // Lambda compute does not support timeoutInMinutes property
+    if (this.isLambdaComputeType(this.buildImage)) {
+      if (props.timeout) {
+        errors.push('Cannot specify timeoutInMinutes for lambda compute');
+      }
+      if (props.queuedTimeout) {
+        errors.push('Cannot specify queuedTimeoutInMinutes for lambda compute');
+      }
+      if (props.cache) {
+        errors.push('Cannot specify cache for lambda compute');
+      }
+      if (props.badge) {
+        errors.push('Cannot specify badgeEnabled for lambda compute');
+      }
+    }


Suggested change

// Lambda compute does not support timeoutInMinutes property

if (this.isLambdaComputeType(this.buildImage)) {

if (props.timeout) {

errors.push('Cannot specify timeoutInMinutes for lambda compute');

}

if (props.queuedTimeout) {

errors.push('Cannot specify queuedTimeoutInMinutes for lambda compute');

}

if (props.cache) {

errors.push('Cannot specify cache for lambda compute');

}

if (props.badge) {

errors.push('Cannot specify badgeEnabled for lambda compute');

}

}

errors.push(...validateLambdaBuildImage(this.buildImage, this.props));

Can you please declare a separate function for this?

/** * Validates a Lambda build image given the project properties. * @see https://docs.aws.amazon.com/codebuild/latest/userguide/lambda.html#lambda.limitations */ private validateLambdaBuildImage(buildImage: IBuildImage, props: ProjectProps): string[] { if (!isLambdaBuildImage(buildImage)) return []; const ret = []; if (props.timeout) { ret.push('Cannot specify timeoutInMinutes for lambda compute'); } if (props.queuedTimeout) { ret.push('Cannot specify queuedTimeoutInMinutes for lambda compute'); } if (props.cache) { ret.push('Cannot specify cache for lambda compute'); } if (props.badge) { ret.push('Cannot specify badgeEnabled for lambda compute'); } return ret; }

lpizzinidev · 2023-11-12T10:09:29Z

packages/aws-cdk-lib/aws-codebuild/lib/project.ts

+    const ret = [];
+
+    const lambdaComputeTypes = Object.values(ComputeType).filter(value => value.startsWith('BUILD_LAMBDA'));
+    if (_env.computeType && lambdaComputeTypes.includes(_env.computeType)) {
+      ret.push(`x86-64 images only support ComputeTypes between '${ComputeType.SMALL}' and '${ComputeType.X2_LARGE}' - ` +
+               `'${_env.computeType}' was given`);
+    }
+
+    return ret;


Suggested change

const ret = [];

const lambdaComputeTypes = Object.values(ComputeType).filter(value => value.startsWith('BUILD_LAMBDA'));

if (_env.computeType && lambdaComputeTypes.includes(_env.computeType)) {

ret.push(`x86-64 images only support ComputeTypes between '${ComputeType.SMALL}' and '${ComputeType.X2_LARGE}' - ` +

`'${_env.computeType}' was given`);

}

return ret;

const ret = [];

const lambdaComputeTypes = Object.values(ComputeType).filter(value => value.startsWith('BUILD_LAMBDA'));

if (this.isLambdaComputeType(env.computeType)) {

ret.push('x86-64 images do not support Lambda compute mode');

}

return ret;

It's better to declare a separate function to check if the compute type is a Lambda one and re-use it.

private isLambdaComputeType(computeType?: string): boolean { if (!computeType) return false; return [ ComputeType.LAMBDA_1GB, ComputeType.LAMBDA_2GB, ... ].includes(computeType); }

added implementation as a function instead of a method so that it can be used in other files.

lpizzinidev · 2023-11-12T10:10:04Z

packages/aws-cdk-lib/aws-codebuild/lib/project.ts

@@ -1542,6 +1562,10 @@ export class Project extends ProjectBase {
      throw new Error('Both source and artifacts must be set to CodePipeline');
    }
  }
+
+  private isLambdaComputeType(buildImage: IBuildImage): boolean {


Suggested change

private isLambdaComputeType(buildImage: IBuildImage): boolean {

private isLambdaBuildImage(buildImage: IBuildImage): boolean {

Naming.

lpizzinidev · 2023-11-12T10:15:13Z

packages/aws-cdk-lib/aws-codebuild/README.md

+Custom images are not supported, so please use a static method such as `AMAZON_LINUX_2_NODE_18`.
+See documentation for other constraints.
+https://docs.aws.amazon.com/codebuild/latest/userguide/lambda.html#lambda.limitations


Suggested change

Custom images are not supported, so please use a static method such as `AMAZON_LINUX_2_NODE_18`.

See documentation for other constraints.

https://docs.aws.amazon.com/codebuild/latest/userguide/lambda.html#lambda.limitations

> Visit [AWS Lambda compute in AWS CodeBuild](https://docs.aws.amazon.com/codebuild/latest/userguide/lambda.html) for more details.

lpizzinidev · 2023-11-12T10:16:41Z

packages/aws-cdk-lib/aws-codebuild/lib/project.ts

-      ? ImagePullPrincipalType.CODEBUILD
-      : ImagePullPrincipalType.SERVICE_ROLE;
+    // For Lambda compute, specifying imagePullPrincipalType is not supported
+    const imagePullPrincipalType = this.isLambdaComputeType(this.buildImage)


Is this needed?
If CFN does not deploy it should be validated in validateLambdaBuildImage otherwise I think we can just ignore the value.

In the original implementation, SERVICE_ROLE is specified as the default value even if imagePullPrincipalType is not specified.

aws-cdk/packages/aws-cdk-lib/aws-codebuild/lib/project.ts

Line 1341 in 0178cd3

const imagePullPrincipalType = this.buildImage.imagePullPrincipalType === ImagePullPrincipalType.CODEBUILD

const imagePullPrincipalType = this.buildImage.imagePullPrincipalType === ImagePullPrincipalType.CODEBUILD ? ImagePullPrincipalType.CODEBUILD : ImagePullPrincipalType.SERVICE_ROLE; return { imagePullCredentialsType: imagePullPrincipalType, };

Therefore, I think some kind of modification other than validation is necessary.
Any advice on a better implementation would be appreciated!

I think that imagePullPrincipalType, secretsManagerCredentials, and repository will just be ignored if a Lambda-compute image is passed (I might be wrong on this).
If this is the case we could provide some feedback with Annotations.addWarningV2() to notify the users that those properties will not be used.
I don't think we should set imagePullPrincipalType to undefined if this.isLambdaBuildImage(this.buildImage) because it would either unset an already ignored value or mask a wrong property specification that should be validated.

I think that imagePullPrincipalType, secretsManagerCredentials, and repository will just be ignored if a Lambda-compute image is passed (I might be wrong on this).

I agree with you.
imagePullPrincipalType, secretsManagerCredentials, and repository are all specified within IBuildImage, but the Lambda Build Image doesn't have these properties. Therefore, users generally cannot explicitly specify these values.

However, when it comes to imagePullPrincipalType, even if these values are not specified within the Lambda Build Image, the Project.renderEnvironment method defaults to setting imagePullPrincipalType to SERVICE_ROLE. This causes deployment to result in an error message Cannot specify imagePullPrincipalType for lambda compute.

So, I think some modifications is needed in lines like the following within the renderEnvironment method for the deployment to work correctly.

aws-cdk/packages/aws-cdk-lib/aws-codebuild/lib/project.ts

Line 1341 in 1466207

const imagePullPrincipalType = this.buildImage.imagePullPrincipalType === ImagePullPrincipalType.CODEBUILD

aws-cdk/packages/aws-cdk-lib/aws-codebuild/lib/project.ts

Line 1364 in 1466207

imagePullCredentialsType: imagePullPrincipalType,

sakurai-ryo · 2023-11-13T04:39:15Z

@lpizzinidev
Thanks for the review! I've learned a lot.
Please check out some of the changes and comments I have made.

lpizzinidev

Thanks for the changes 👍
Just some minor adjustments are needed.
Also, I would change the title to feat(codebuild): support Lambda compute.

lpizzinidev · 2023-11-13T07:38:29Z

packages/aws-cdk-lib/aws-codebuild/lib/project.ts

+    if (isLambdaComputeType(_env.computeType)) {
+      ret.push('x86-64 images do not support Lambda compute mode');
+    }


Suggested change

if (isLambdaComputeType(_env.computeType)) {

ret.push('x86-64 images do not support Lambda compute mode');

}

if (isLambdaComputeType(env.computeType)) {

ret.push('x86-64 images do not support Lambda compute types');

}

Seems more appropriate, can you please also update the other similar error messages?
Also, _ should be removed from the property name as it's now in use.

lpizzinidev · 2023-11-13T07:41:54Z

packages/aws-cdk-lib/aws-codebuild/test/linux-arm-lambda-build-image.test.ts

+import * as codebuild from '../lib';
+
+describe('Linux ARM Lambda build image', () => {
+  describe('AMAZON_LINUX_2_NODE_18', () => {


Suggested change

describe('AMAZON_LINUX_2_NODE_18', () => {

Let's keep only the generic describe as these tests apply also to other image types.

lpizzinidev · 2023-11-13T07:43:00Z

packages/aws-cdk-lib/aws-codebuild/test/linux-lambda-build-image.test.ts

+import * as codebuild from '../lib';
+
+describe('Linux Lambda build image', () => {
+  describe('AMAZON_LINUX_2_NODE_18', () => {


Suggested change

describe('AMAZON_LINUX_2_NODE_18', () => {

lpizzinidev · 2023-11-13T07:46:11Z

packages/aws-cdk-lib/aws-codebuild/lib/project.ts

+    if (props.timeout) {
+      ret.push('Cannot specify timeoutInMinutes for lambda compute');
+    }
+    if (props.queuedTimeout) {
+      ret.push('Cannot specify queuedTimeoutInMinutes for lambda compute');
+    }
+    if (props.cache) {
+      ret.push('Cannot specify cache for lambda compute');
+    }
+    if (props.badge) {
+      ret.push('Cannot specify badgeEnabled for lambda compute');
+    }


Suggested change

if (props.timeout) {

ret.push('Cannot specify timeoutInMinutes for lambda compute');

}

if (props.queuedTimeout) {

ret.push('Cannot specify queuedTimeoutInMinutes for lambda compute');

}

if (props.cache) {

ret.push('Cannot specify cache for lambda compute');

}

if (props.badge) {

ret.push('Cannot specify badgeEnabled for lambda compute');

}

if (props.timeout) {

ret.push('Cannot specify timeout for Lambda compute');

}

if (props.queuedTimeout) {

ret.push('Cannot specify queuedTimeout for Lambda compute');

}

if (props.cache) {

ret.push('Cannot specify cache for Lambda compute');

}

if (props.badge) {

ret.push('Cannot enable badge for Lambda compute');

}

Let's keep CDK property names.

lpizzinidev · 2023-11-13T07:48:06Z

packages/aws-cdk-lib/aws-codebuild/README.md

+Additionally, with the introduction of Lambda compute support, the `LambdaBuildImage` (or `LinuxArmLambdaBuildImage`) class 
+is available for specifying Lambda-compatible images.


Suggested change

Additionally, with the introduction of Lambda compute support, the `LambdaBuildImage` (or `LinuxArmLambdaBuildImage`) class

is available for specifying Lambda-compatible images.

With the introduction of Lambda compute support, the `LinuxLambdaBuildImage ` (or `LinuxArmLambdaBuildImage`) class

is available for specifying Lambda-compatible images.

lpizzinidev · 2023-11-13T07:49:51Z

packages/aws-cdk-lib/aws-codebuild/README.md

@@ -329,6 +332,8 @@ or one of the corresponding methods on `LinuxArmBuildImage`:
 * `LinuxArmBuildImage.fromDockerRegistry(image[, { secretsManagerCredentials }])`
 * `LinuxArmBuildImage.fromEcrRepository(repo[, tag])`

+However, be aware that specifying custom images is not possible with Lambda Images.


Suggested change

However, be aware that specifying custom images is not possible with Lambda Images.

**Note:** You cannot specify custom images on `LinuxLambdaBuildImage` or `LinuxArmLambdaBuildImage` images.

lpizzinidev · 2023-11-13T07:52:23Z

packages/aws-cdk-lib/aws-codebuild/lib/project.ts

-// eslint-disable-next-line no-duplicate-imports, import/order
+/* eslint-disable no-duplicate-imports, import/order */
 import { LinuxArmBuildImage } from './linux-arm-build-image';
+import { LinuxArmLambdaBuildImage } from './linux-arm-lambda-build-image';
+import { LinuxLambdaBuildImage } from './linux-lambda-build-image';
+/* eslint-enable no-duplicate-imports, import/order */


The new imports should be added at the top of the file as not directly related to LinuxBuildImage

Since LinuxArmLambdaBuildImage and LinuxLambdaBuildImage reference ComputeType, it's necessary to place the import statements below ComputeType. I've adjusted their placement just beneath ComputeType. How does that look?

However, these classes only use ComputeType when generating error messages. It's also possible to move the import statements to the top of the file by modifying the error messages that don't use ComputeType. This option might be more appropriate.

Thanks for clarifying, let's just keep them together as you did in the original commit then. It's clearer.

lpizzinidev · 2023-11-13T10:02:24Z

packages/aws-cdk-lib/aws-codebuild/lib/project.ts

-      ? ImagePullPrincipalType.CODEBUILD
-      : ImagePullPrincipalType.SERVICE_ROLE;
+    // For Lambda compute, specifying imagePullPrincipalType is not supported
+    const imagePullPrincipalType = this.isLambdaComputeType(this.buildImage)


I think that imagePullPrincipalType, secretsManagerCredentials, and repository will just be ignored if a Lambda-compute image is passed (I might be wrong on this).
If this is the case we could provide some feedback with Annotations.addWarningV2() to notify the users that those properties will not be used.
I don't think we should set imagePullPrincipalType to undefined if this.isLambdaBuildImage(this.buildImage) because it would either unset an already ignored value or mask a wrong property specification that should be validated.

sakurai-ryo · 2023-11-13T16:12:37Z

@lpizzinidev
Thank you very much for the review! I have made some changes and added comments.

lpizzinidev · 2023-11-14T07:37:45Z

packages/aws-cdk-lib/aws-codebuild/lib/project.ts

+    }
+
+    if (buildEnvironment.computeType && isLambdaComputeType(buildEnvironment.computeType)) {
+      ret.push('Windows images do not support Lambda compute mode');


Suggested change

ret.push('Windows images do not support Lambda compute mode');

ret.push('Windows images do not support Lambda compute types');

lpizzinidev · 2023-11-14T07:38:38Z

packages/aws-cdk-lib/aws-codebuild/lib/project.ts

+export function isLambdaComputeType(computeType?: ComputeType): boolean {
+  if (!computeType) return false;


Suggested change

export function isLambdaComputeType(computeType?: ComputeType): boolean {

if (!computeType) return false;

export function isLambdaComputeType(computeType: ComputeType): boolean {

On second thought, let's make the property non-optional.
After all, we want to check only defined values.

lpizzinidev · 2023-11-14T07:40:57Z

packages/aws-cdk-lib/aws-codebuild/lib/project.ts

+    // For Lambda compute, specifying imagePullPrincipalType is not supported
+    const imagePullPrincipalType = this.isLambdaBuildImage(this.buildImage)
+      ? undefined
+      : this.buildImage.imagePullPrincipalType === ImagePullPrincipalType.CODEBUILD
+        ? ImagePullPrincipalType.CODEBUILD
+        : ImagePullPrincipalType.SERVICE_ROLE;


Suggested change

// For Lambda compute, specifying imagePullPrincipalType is not supported

const imagePullPrincipalType = this.isLambdaBuildImage(this.buildImage)

? undefined

: this.buildImage.imagePullPrincipalType === ImagePullPrincipalType.CODEBUILD

? ImagePullPrincipalType.CODEBUILD

: ImagePullPrincipalType.SERVICE_ROLE;

const imagePullPrincipalType = this.buildImage.imagePullPrincipalType === ImagePullPrincipalType.CODEBUILD

? ImagePullPrincipalType.CODEBUILD

: ImagePullPrincipalType.SERVICE_ROLE;

Let's keep it as it was. See #27934 (comment).

@lpizzinidev

As mentioned in this comment, in the original implementation, imagePullCredentialsType is always set to CODEBUILD or SERVICE_ROLE, causing deployment to fail and Integration Tests to fail as well.
#27934 (comment)

In renderEnvironment method, we set registryCredential to undefined based on the presence of buildImage.secretsManagerCredentials when returning values.
https://github.com/aws/aws-cdk/blob/d6f6dd75383167bda980052786b85c45d9ba9e0b/packages/aws-cdk-lib/aws-codebuild/lib/project.ts#L1365C11-L1365C11

Would it be better to implement imagePullCredentialsType in a similar way? It's essentially the same implementation, but...

return { imagePullCredentialsType: this.isLambdaBuildImage(this.buildImage) ? undefined : imagePullPrincipalType, };

lpizzinidev · 2023-11-14T07:41:29Z

packages/aws-cdk-lib/aws-codebuild/test/codebuild.test.ts

+            computeType: codebuild.ComputeType.LAMBDA_1GB,
+          },
+        });
+      }).toThrow(/Invalid CodeBuild environment: Windows images do not support Lambda compute mode/);


Suggested change

}).toThrow(/Invalid CodeBuild environment: Windows images do not support Lambda compute mode/);

}).toThrow(/Invalid CodeBuild environment: Windows images do not support Lambda compute types/);

lpizzinidev · 2023-11-14T07:48:12Z

packages/aws-cdk-lib/aws-codebuild/lib/project.ts

-// eslint-disable-next-line no-duplicate-imports, import/order
+/* eslint-disable no-duplicate-imports, import/order */
 import { LinuxArmBuildImage } from './linux-arm-build-image';
+import { LinuxArmLambdaBuildImage } from './linux-arm-lambda-build-image';
+import { LinuxLambdaBuildImage } from './linux-lambda-build-image';
+/* eslint-enable no-duplicate-imports, import/order */


Thanks for clarifying, let's just keep them together as you did in the original commit then. It's clearer.

lpizzinidev

Thanks, just some minor adjustments and it will be good to go for me.
Note #27934 (comment).

sakurai-ryo · 2023-11-14T17:05:04Z

@lpizzinidev
Thank you for the review again.
I've made the changes and added the following comment.
#27934 (comment)

lpizzinidev

Thanks for the changes and the clarifications.
I missed #27934 (comment) (sorry for that).
I think we should just validate those properties in validateLambdaBuildImage.

sakurai-ryo · 2023-11-16T04:55:52Z

packages/aws-cdk-lib/aws-codebuild/lib/project.ts

+  private validateLambdaBuildImage(buildImage: IBuildImage, props: ProjectProps): string[] {
+    if (!this.isLambdaBuildImage(buildImage)) return [];
+    const ret = [];
+    if (props.timeout) {


Previously, setting imagePullPrincipalType to CODEBUILD seemed to cause an error (really sorry if I misunderstood). However, I just rechecked the behavior and found that deploying with imagePullPrincipalType set to CODEBUILD succeeds.
Therefore, I've changed the default value of imagePullPrincipalType in the LinuxLambdaBuildImage and LinuxArmLambdaBuildImage classes to CODEBUILD.

For repository and secretsManagerCredentials, these properties are not specified in the Lambda Build Image and are read-only, i.e., they cannot be changed after initialization in the Lambda Build Image. So, I don't think I can write unit tests to check for validation errors.
Do you mean to add own class that implements IBuildImage (like the lambda build image classes) and test it?

lpizzinidev · 2023-11-15T07:45:30Z

packages/aws-cdk-lib/aws-codebuild/lib/project.ts

@@ -1361,7 +1364,7 @@ export class Project extends ProjectBase {
    return {
      type: this.buildImage.type,
      image: this.buildImage.imageId,
-      imagePullCredentialsType: imagePullPrincipalType,
+      imagePullCredentialsType: this.isLambdaBuildImage(this.buildImage) ? undefined : imagePullPrincipalType,


Suggested change

imagePullCredentialsType: this.isLambdaBuildImage(this.buildImage) ? undefined : imagePullPrincipalType,

imagePullCredentialsType: imagePullPrincipalType,

Will remain unchanged after validating the image.

lpizzinidev · 2023-11-15T07:45:51Z

packages/aws-cdk-lib/aws-codebuild/lib/project.ts

@@ -2173,3 +2228,14 @@ export enum ProjectNotificationEvents {
 function isBindableBuildImage(x: unknown): x is IBindableBuildImage {
  return typeof x === 'object' && !!x && !!(x as any).bind;
 }
+
+export function isLambdaComputeType(computeType: ComputeType): boolean {
+  if (!computeType) return false;


Suggested change

if (!computeType) return false;

Always defined.

sorry for missing this.

aws-cdk-automation · 2023-12-05T00:09:58Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

aws-cdk-automation · 2023-12-06T00:09:46Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

…add-lambda-compute-suport

kaizencc

Thanks for starting this @sakurai-ryo. Looks like Luca has provided some detailed reviews -- that's awesome. I have some preliminary comments, nothing too serious. Will take a deeper look in a bit.

kaizencc · 2023-12-13T21:51:30Z

packages/aws-cdk-lib/aws-codebuild/README.md

@@ -396,6 +401,22 @@ new codebuild.Project(this, 'Project', {

 Alternatively, you can reference an image available in an ECR repository using the `LinuxGpuBuildImage.fromEcrRepository(repo[, tag])` method.

+### Lambda images
+
+The `LambdaBuildImage` (or `LinuxArmLambdaBuildImage`) class contains constants for working with


Suggested change

The `LambdaBuildImage` (or `LinuxArmLambdaBuildImage`) class contains constants for working with

The `LinuxLambdaBuildImage` (or `LinuxArmLambdaBuildImage`) class contains constants for working with

kaizencc · 2023-12-13T21:53:26Z

packages/aws-cdk-lib/aws-codebuild/lib/linux-arm-lambda-build-image.ts

+      ret.push(`Lambda images only support ComputeTypes between '${ComputeType.LAMBDA_1GB}' and '${ComputeType.LAMBDA_10GB}' - ` +
+               `'${buildEnvironment.computeType}' was given`);


This formatting is off. I suggest doing something like this if you want to split it into multiple lines:

ret.push([ 'words', 'more words', 'final words', ].join(' '));

kaizencc · 2023-12-13T21:53:48Z

packages/aws-cdk-lib/aws-codebuild/lib/linux-lambda-build-image.ts

+      ret.push(`Lambda images only support ComputeTypes between ${ComputeType.LAMBDA_1GB} and ${ComputeType.LAMBDA_10GB} - ` +
+               `${buildEnvironment.computeType} was given`);


I have the same issue with formatting here

kaizencc · 2023-12-13T21:54:52Z

packages/aws-cdk-lib/aws-codebuild/lib/project.ts

+   */
+  private validateLambdaBuildImage(buildImage: IBuildImage, props: ProjectProps): string[] {
+    if (!this.isLambdaBuildImage(buildImage)) return [];
+    const ret = [];


Suggested change

const ret = [];

const errors = [];

kaizencc · 2023-12-13T22:13:08Z

packages/aws-cdk-lib/aws-codebuild/lib/project.ts

+/* eslint-disable no-duplicate-imports, import/order */
+import { LinuxArmLambdaBuildImage } from './linux-arm-lambda-build-image';
+import { LinuxLambdaBuildImage } from './linux-lambda-build-image';
+/* eslint-enable no-duplicate-imports, import/order */


What are the duplicate imports?

kaizencc · 2023-12-13T22:13:17Z

packages/aws-cdk-lib/aws-codebuild/lib/project.ts

-  public validate(_env: BuildEnvironment): string[] {
-    return [];
+  public validate(env: BuildEnvironment): string[] {
+    const ret = [];


Suggested change

const ret = [];

const errors = [];

kaizencc · 2023-12-13T22:13:30Z

packages/aws-cdk-lib/aws-codebuild/lib/project.ts

@@ -2068,6 +2115,15 @@ export class WindowsBuildImage implements IBuildImage {

  public validate(buildEnvironment: BuildEnvironment): string[] {
    const ret: string[] = [];


Suggested change

const ret: string[] = [];

const errors: string[] = [];

…add-lambda-compute-suport

Pull request has been modified.

sakurai-ryo · 2023-12-14T13:43:57Z

@kaizencc
Thanks for your review 👍
I fixed them.

kaizencc

A few final minor comments, and then we're good to go. Thanks @sakurai-ryo

kaizencc · 2023-12-20T16:34:14Z

packages/aws-cdk-lib/aws-codebuild/lib/linux-arm-lambda-build-image.ts

+  }
+
+  public readonly type = 'ARM_LAMBDA_CONTAINER';
+  public readonly defaultComputeType = ComputeType.LAMBDA_1GB;


what is this for / where is this used / why is this the default?

The defaultComputeType is required for IBuildImage and is used if the user does not specify a computeType.
Here is the relevant codes.

aws-cdk/packages/aws-cdk-lib/aws-codebuild/lib/project.ts

Line 1639 in 4ab5bb2

readonly defaultComputeType: ComputeType;

aws-cdk/packages/aws-cdk-lib/aws-codebuild/lib/project.ts

Line 1375 in 4ab5bb2

computeType: env.computeType || this.buildImage.defaultComputeType,

kaizencc · 2023-12-20T16:40:36Z

packages/aws-cdk-lib/aws-codebuild/lib/project.ts

+/* eslint-disable import/order */
+import { LinuxArmLambdaBuildImage } from './linux-arm-lambda-build-image';
+import { LinuxLambdaBuildImage } from './linux-lambda-build-image';
+/* eslint-enable import/order */


I don't love this actually. I think the right thing to do here is to pull ComputeType out into its own file. Then we can import normally.

kaizencc · 2023-12-20T16:42:39Z

packages/aws-cdk-lib/aws-codebuild/lib/project.ts

+    ComputeType.LAMBDA_4GB,
+    ComputeType.LAMBDA_8GB,
+    ComputeType.LAMBDA_10GB,
+  ].includes(computeType);


We're going to have to continue to maintain this list every time we add a new Lambda compute type. Ideally we'd have this be an enum-like class but can't change that now. The only thing I can think of here is that instead of maintaining a list, we commit to having the BUILD_LAMBDA prefix for all Lambda Compute types and then check for that in this function.

kaizencc · 2023-12-20T16:43:55Z

packages/aws-cdk-lib/aws-codebuild/lib/linux-lambda-build-image.ts

+
+    if (buildEnvironment.computeType && !isLambdaComputeType(buildEnvironment.computeType)) {
+      errors.push([
+        'Lambda images only support ComputeTypes between',


Suggested change

'Lambda images only support ComputeTypes between',

'Lambda images only support Lambda ComputeTypes between',

kaizencc · 2023-12-20T16:44:36Z

packages/aws-cdk-lib/aws-codebuild/lib/linux-arm-lambda-build-image.ts

+
+    if (buildEnvironment.computeType && !isLambdaComputeType(buildEnvironment.computeType)) {
+      errors.push([
+        'Lambda images only support ComputeTypes between',


Suggested change

'Lambda images only support ComputeTypes between',

'Lambda images only support Lambda ComputeTypes between',

kaizencc · 2023-12-20T16:46:26Z

...-cdk-testing/framework-integ/test/aws-codebuild/test/integ.project-linux-lambda-image.lit.ts

Besides ensuring that this deploys successfully, are there any assertions we can make to confirm everything is running smoothly?

It is indeed required.
Added execution of the startBuild API on awsApiCall.

…add-lambda-compute-suport

Pull request has been modified.

sakurai-ryo · 2023-12-21T05:50:13Z

@kaizencc
Thank you always for your code reviews.
Fixed them and left some comments.

kaizencc

Thanks for your work @sakurai-ryo!

mergify · 2023-12-22T22:21:02Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

mergify · 2023-12-22T22:24:49Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation · 2023-12-23T00:24:39Z

AWS CodeBuild CI Report

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

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

mergify · 2023-12-23T00:24:57Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

CodeBuild has added support for Lambda compute. CloudFormation can be deployed as follows. ```yaml Resources: CodeBuildProject: Type: AWS::CodeBuild::Project Properties: Artifacts: Type: NO_ARTIFACTS ServiceRole: !GetAtt CodeBuildRole.Arn Source: # Environment: Type: LINUX_LAMBDA_CONTAINER ComputeType: BUILD_LAMBDA_1GB Image: aws/codebuild/amazonlinux-x86_64-lambda-standard:go1.21 CodeBuildRole: Type: AWS::IAM::Role Properties: # ``` https://aws.amazon.com/about-aws/whats-new/2023/11/aws-codebuild-lambda-compute This PR implements Lambda ComputeType by adding Classes (`LinuxArmLambdaBuildImage`, `LinuxLambdaBuildImage`) that extend the IBuildImage interface. Supported Docker Images and ComputeTypes are listed below. https://docs.aws.amazon.com/codebuild/latest/userguide/build-env-ref-available.html https://docs.aws.amazon.com/codebuild/latest/userguide/build-env-ref-compute-types.html#environment.types Also, Lambda compute has some limitations and I have added validation for them. https://docs.aws.amazon.com/codebuild/latest/userguide/lambda.html#lambda.limitations closes aws#28418 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

sakurai-ryo added 5 commits November 11, 2023 00:54

feat(codebuild): add lambda compute support

35ae34a

feat(codebuild): for Lambda compute, change imagePullPrincipalType to…

6b8fe84

… undefined

feat(codebuild): add integration test

3557354

Merge branch 'master' of https://github.com/sakurai-ryo/aws-cdk into …

6fca0bf

…add-lambda-compute-suport

feat(codebuild): add integration test for arm lambda image

4a1cc22

aws-cdk-automation requested a review from a team November 10, 2023 18:33

github-actions bot added beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK p2 labels Nov 10, 2023

sakurai-ryo added 3 commits November 11, 2023 03:37

feat(codebuild): removed extra whitespace.

2c28108

feat(codebuild): removed extra whitespace.

f2a7a08

feat(codebuild): removed extra whitespace

f7e433e

sakurai-ryo commented Nov 10, 2023

View reviewed changes

packages/aws-cdk-lib/aws-codebuild/test/linux-arm-lambda-build-image.test.ts Show resolved Hide resolved

sakurai-ryo marked this pull request as ready for review November 10, 2023 19:11

aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Nov 10, 2023

lpizzinidev suggested changes Nov 12, 2023

View reviewed changes

aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Nov 12, 2023

sakurai-ryo added 2 commits November 13, 2023 12:57

feat(codebuild): fix for the review

50f59e3

feat(codebuild): remove comment

b9e81a4

lpizzinidev suggested changes Nov 13, 2023

View reviewed changes

feat(codebuild): fix for the review

c5e140a

sakurai-ryo changed the title ~~feat(codebuild): Support for Lambda compute~~ feat(codebuild): support Lambda compute Nov 13, 2023

feat(codebuild): fix comment

fdae4be

lpizzinidev reviewed Nov 14, 2023

View reviewed changes

lpizzinidev suggested changes Nov 14, 2023

View reviewed changes

sakurai-ryo added 2 commits November 15, 2023 01:11

feat(codebuild): fixed

1c0fc9b

feat(codebuild): fixed for the review

4cc8c38

lpizzinidev suggested changes Nov 15, 2023

View reviewed changes

Merge branch 'master' of https://github.com/sakurai-ryo/aws-cdk into …

031f50a

…add-lambda-compute-suport

kaizencc previously requested changes Dec 13, 2023

View reviewed changes

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Dec 13, 2023

sakurai-ryo added 2 commits December 14, 2023 19:14

Merge branch 'master' of https://github.com/sakurai-ryo/aws-cdk into …

6be4824

…add-lambda-compute-suport

feat(codebuild): fix for the review

dee315f

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Dec 14, 2023

github-actions bot added effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. labels Dec 20, 2023

kaizencc changed the title ~~feat(codebuild): support Lambda compute~~ feat(codebuild): Lambda compute for codebuild projects Dec 20, 2023

kaizencc previously requested changes Dec 20, 2023

View reviewed changes

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Dec 20, 2023

sakurai-ryo added 2 commits December 21, 2023 01:40

Merge branch 'master' of https://github.com/sakurai-ryo/aws-cdk into …

55f0270

…add-lambda-compute-suport

fix for the review

aeb5a73

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Dec 21, 2023

add: assertion

a96385e

kaizencc approved these changes Dec 22, 2023

View reviewed changes

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Dec 22, 2023

Merge branch 'main' into add-lambda-compute-suport

8118619

mergify bot merged commit a4a4c6f into aws:main Dec 23, 2023
9 checks passed

	/** Image "aws/codebuild/amazonlinux-aarch64-lambda-standard:nodejs18". */
	/** The `aws/codebuild/amazonlinux-aarch64-lambda-standard:nodejs18` build image. */

		* This class has a bunch of public constants that represent the CodeBuild aarch64 images.
		*

	ret.push('Lambda compute type does not support Privileged mode');
	ret.push('Lambda compute type does not support privileged mode');

	private isLambdaComputeType(buildImage: IBuildImage): boolean {
	private isLambdaBuildImage(buildImage: IBuildImage): boolean {

		Additionally, with the introduction of Lambda compute support, the `LambdaBuildImage` (or `LinuxArmLambdaBuildImage`) class
		is available for specifying Lambda-compatible images.

	However, be aware that specifying custom images is not possible with Lambda Images.
	Note: You cannot specify custom images on `LinuxLambdaBuildImage` or `LinuxArmLambdaBuildImage` images.

	ret.push('Windows images do not support Lambda compute mode');
	ret.push('Windows images do not support Lambda compute types');

		export function isLambdaComputeType(computeType?: ComputeType): boolean {
		if (!computeType) return false;

	}).toThrow(/Invalid CodeBuild environment: Windows images do not support Lambda compute mode/);
	}).toThrow(/Invalid CodeBuild environment: Windows images do not support Lambda compute types/);

	imagePullCredentialsType: this.isLambdaBuildImage(this.buildImage) ? undefined : imagePullPrincipalType,
	imagePullCredentialsType: imagePullPrincipalType,

	The `LambdaBuildImage` (or `LinuxArmLambdaBuildImage`) class contains constants for working with
	The `LinuxLambdaBuildImage` (or `LinuxArmLambdaBuildImage`) class contains constants for working with

		ret.push(`Lambda images only support ComputeTypes between '${ComputeType.LAMBDA_1GB}' and '${ComputeType.LAMBDA_10GB}' - ` +
		`'${buildEnvironment.computeType}' was given`);

		ret.push(`Lambda images only support ComputeTypes between ${ComputeType.LAMBDA_1GB} and ${ComputeType.LAMBDA_10GB} - ` +
		`${buildEnvironment.computeType} was given`);

		@@ -2068,6 +2115,15 @@ export class WindowsBuildImage implements IBuildImage {

		public validate(buildEnvironment: BuildEnvironment): string[] {
		const ret: string[] = [];

	'Lambda images only support ComputeTypes between',
	'Lambda images only support Lambda ComputeTypes between',

feat(codebuild): Lambda compute for codebuild projects #27934

feat(codebuild): Lambda compute for codebuild projects #27934

Conversation

sakurai-ryo commented Nov 10, 2023 • edited Loading

lpizzinidev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sakurai-ryo commented Nov 13, 2023

lpizzinidev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sakurai-ryo commented Nov 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sakurai-ryo Nov 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lpizzinidev left a comment

Choose a reason for hiding this comment

sakurai-ryo commented Nov 14, 2023 • edited Loading

lpizzinidev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aws-cdk-automation commented Dec 5, 2023

aws-cdk-automation commented Dec 6, 2023

kaizencc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sakurai-ryo commented Dec 14, 2023

kaizencc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sakurai-ryo commented Dec 21, 2023

kaizencc left a comment

Choose a reason for hiding this comment

mergify bot commented Dec 22, 2023

mergify bot commented Dec 22, 2023

aws-cdk-automation commented Dec 23, 2023

AWS CodeBuild CI Report

sakurai-ryo commented Nov 10, 2023 •

edited

Loading

sakurai-ryo commented Nov 13, 2023 •

edited

Loading

sakurai-ryo Nov 14, 2023 •

edited

Loading

sakurai-ryo commented Nov 14, 2023 •

edited

Loading