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

chore(codebuild): added validation when using Windows Image #27946

Merged

Conversation

sakurai-ryo
Copy link
Contributor

When using WINDOWS_SERVER_2019_CONTAINER, only MEDIUM and LARGE computeType is supported.
https://docs.aws.amazon.com/codebuild/latest/userguide/build-env-ref-compute-types.html

However, currently only ComputeType SMALL is validated.
This PR modified to generate an error when ComputeType is specified as X2_LARGE in WindowsBuildImage.

    new codebuild.Project(this, 'CodeBuildCdk', {
      source: codebuild.Source.codeCommit({ repository: codecommit.Repository.fromRepositoryName(this, "Repo", "sample") }),
      environment: {
        computeType: codebuild.ComputeType.X2_LARGE, // generate error in synth stage
        buildImage:  codebuild.WindowsBuildImage.WINDOWS_BASE_2_0
      }
    });

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

@github-actions github-actions bot added p2 beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK labels Nov 11, 2023
@aws-cdk-automation aws-cdk-automation requested a review from a team November 11, 2023 11:45
@@ -1965,6 +1965,8 @@ export class WindowsBuildImage implements IBuildImage {
/**
* The standard CodeBuild image `aws/codebuild/windows-base:2.0`, which is
* based off Windows Server Core 2016.
*
* @deprecated `WindowsBuildImage.WIN_SERVER_CORE_2019_BASE_2_0` should be used instead.
Copy link
Contributor Author

@sakurai-ryo sakurai-ryo Nov 11, 2023

Choose a reason for hiding this comment

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

Added @deprecated to WINDOWS_BASE_2_0.
Since WINDOWS_CONTAINER type is already deprecated, a new deployment of WINDOWS_BASE_2_0 will result in the following error.
The environment type WINDOWS_CONTAINER is deprecated for new projects or existing project environment updates. Please consider using Windows Server 2019 instead.

I could not find any mention of deprecation in the documentation, but Terraform has already deprecated it.
https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/codebuild_project#environment

This is not the main topic of this PR, so if it should be a separate PR, I will do so.

@sakurai-ryo sakurai-ryo marked this pull request as ready for review November 11, 2023 12:44
@sakurai-ryo sakurai-ryo changed the title chore(codebuild): Added validation when using Windows Image chore(codebuild): added validation when using Windows Image Nov 17, 2023
@aws-cdk-automation 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 17, 2023
Copy link
Contributor

@lpizzinidev lpizzinidev left a comment

Choose a reason for hiding this comment

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

Thanks 👍
Just a small adjustment in my opinion.
Also, the deprecation message for WIN_SERVER_CORE_2016_BASE should be updated as well.

Comment on lines 2071 to 2076
if (buildEnvironment.computeType === ComputeType.SMALL) {
ret.push('Windows images do not support the Small ComputeType');
}
if (buildEnvironment.computeType === ComputeType.X2_LARGE) {
ret.push('Windows images do not support the 2xLarge ComputeType');
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (buildEnvironment.computeType === ComputeType.SMALL) {
ret.push('Windows images do not support the Small ComputeType');
}
if (buildEnvironment.computeType === ComputeType.X2_LARGE) {
ret.push('Windows images do not support the 2xLarge ComputeType');
}
if (buildEnvironment.computeType === ComputeType.SMALL || buildEnvironment.computeType === ComputeType.X2_LARGE) {
ret.push(`Windows images do not support the '${buildEnvironment.computeType}' compute type`);
}

A bit cleaner.

@aws-cdk-automation 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 17, 2023
@sakurai-ryo
Copy link
Contributor Author

@lpizzinidev
Thank you for the correction!
Fixed them.

Copy link
Contributor

@lpizzinidev lpizzinidev left a comment

Choose a reason for hiding this comment

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

Thanks 👍

@aws-cdk-automation aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Nov 18, 2023
Copy link
Contributor

@vinayak-kukreja vinayak-kukreja left a comment

Choose a reason for hiding this comment

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

Hey, this is great! Thank you for your contribution.

Copy link
Contributor

mergify bot commented Nov 27, 2023

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 aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Nov 27, 2023
Copy link
Contributor

mergify bot commented Nov 27, 2023

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).

@vinayak-kukreja
Copy link
Contributor

@Mergifyio update

Copy link
Contributor

mergify bot commented Nov 28, 2023

update

✅ Branch has been successfully updated

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

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

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

Copy link
Contributor

mergify bot commented Nov 28, 2023

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 mergify bot merged commit 640817b into aws:main Nov 28, 2023
10 checks passed
chenjane-dev pushed a commit to chenjane-dev/aws-cdk that referenced this pull request Dec 5, 2023
When using `WINDOWS_SERVER_2019_CONTAINER`, only MEDIUM and LARGE computeType is supported.
https://docs.aws.amazon.com/codebuild/latest/userguide/build-env-ref-compute-types.html

However, currently only ComputeType `SMALL` is validated.
This PR modified to generate an error when ComputeType is specified as X2_LARGE in WindowsBuildImage.
```ts
    new codebuild.Project(this, 'CodeBuildCdk', {
      source: codebuild.Source.codeCommit({ repository: codecommit.Repository.fromRepositoryName(this, "Repo", "sample") }),
      environment: {
        computeType: codebuild.ComputeType.X2_LARGE, // generate error in synth stage
        buildImage:  codebuild.WindowsBuildImage.WINDOWS_BASE_2_0
      }
    });
```

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK p2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants