Skip to content

chore(codebuild): improve the doc for subnetSelection#26592

Merged
mergify[bot] merged 6 commits intoaws:mainfrom
pahud:codebuild-doc
Aug 18, 2023
Merged

chore(codebuild): improve the doc for subnetSelection#26592
mergify[bot] merged 6 commits intoaws:mainfrom
pahud:codebuild-doc

Conversation

@pahud
Copy link
Copy Markdown
Contributor

@pahud pahud commented Aug 1, 2023

If vpc is specified with subnetSelection undefined, according to this:

if (placement.subnetType === undefined && placement.subnetGroupName === undefined && placement.subnets === undefined) {
// Return default subnet type based on subnets that actually exist
let subnetType = this.privateSubnets.length
? SubnetType.PRIVATE_WITH_EGRESS : this.isolatedSubnets.length ? SubnetType.PRIVATE_ISOLATED : SubnetType.PUBLIC;
placement = { ...placement, subnetType: subnetType };
}

CDK will look for PRIVATE_WITH_EGRESS, PRIVATE_ISOLATED, and PUBLIC in order. If customer does not have PRIVATE_WITH_EGRESS subnets, they will need to have vpc endpoints if they need to access AWS services such as AWS Secrets Manager or Amazon ECR.

This PR improves the doc to clarify.

Closes #.


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

@aws-cdk-automation aws-cdk-automation requested a review from a team August 1, 2023 22:38
@github-actions github-actions bot added the p2 label Aug 1, 2023
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Aug 1, 2023
@pahud pahud marked this pull request as ready for review August 1, 2023 22:39
@aws-cdk-automation aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Aug 1, 2023
@aws-cdk-automation aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Aug 3, 2023
@rix0rrr
Copy link
Copy Markdown
Contributor

rix0rrr commented Aug 3, 2023

If you have no PRIVATE_WITH_EGRESS subnets in the specified vpc, leaving this undefined will require VPC endpoints to access AWS services.

I think this is giving vague instructions but don't really explain the underlying reasons. How about:

To access AWS services, your CodeBuild project needs to be in one of the following types of subnets:

* Subnets with access to the internet (of type PRIVATE_WITH_EGRESS or PUBLIC).
* Private subnets unconnected to the internet, but with [VPC endpoints](link here) for the necessary services.

If you don't specify a subnet selection, the default behavior is to use PRIVATE_WITH_EGRESS subnets first if they exist, then PRIVATE_WITHOUT_EGRESS, and finally PUBLIC subnets. If your VPC doesn't have PRIVATE_WITH_EGRESS subnets but you need AWS service access, either select PUBLIC subnets explicitly or add VPC Endpoints to your private subnets.

Copy link
Copy Markdown
Contributor

@rix0rrr rix0rrr left a comment

Choose a reason for hiding this comment

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

See comment above

@pahud
Copy link
Copy Markdown
Contributor Author

pahud commented Aug 17, 2023

@rix0rrr

According to Best practices for VPCs,

You need a NAT gateway or NAT instance to use CodeBuild with your VPC so that CodeBuild can reach public endpoints (for example, to run CLI commands when running builds). You cannot use the internet gateway instead of a NAT gateway or a NAT instance because CodeBuild does not support assigning Elastic IP addresses to the network interfaces that it creates, and auto-assigning a public IP address is not supported by Amazon EC2 for any network interfaces created outside of Amazon EC2 instance launches.

I believe if user selects public subnets, codebuild will not be able to access the public endpoints. So the option would be either PRIVATE_WITH_EGRESS or enable vpc endpoints.

I have modified the suggested changes. Let me know if it works for you.

Copy link
Copy Markdown
Contributor

@rix0rrr rix0rrr left a comment

Choose a reason for hiding this comment

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

Good find on the CodeBuild exception, I didn't know that. Thanks!

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Aug 18, 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
Copy link
Copy Markdown
Contributor

mergify bot commented Aug 18, 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
Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

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

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

@mergify mergify bot merged commit dbe5615 into aws:main Aug 18, 2023
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Aug 18, 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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contribution/core This is a PR that came from AWS. p2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants