fix(ec2): global vpc endpoint support by nmussy · Pull Request #29563 · aws/aws-cdk

nmussy · 2024-03-20T17:27:22Z

Issue # (if applicable)

Reason for this change

The service name generated by the CDK for global VPC endpoints was incorrect, as it contained the stack's region:

$ aws ec2 describe-vpc-endpoint-services --region=us-east-1 --service-names=com.amazonaws.s3-global.accesspoint | jq '.ServiceDetails[] | .ServiceName'

"com.amazonaws.s3-global.accesspoint"

new CfnOutput(this, "endpoint", {
	value: ec2.InterfaceVpcEndpointAwsService.S3_MULTI_REGION_ACCESS_POINTS.name,
});

// TestDeployStack.endpoint = com.amazonaws.eu-west-1.s3-global.accesspoint

In addition, another global endpoint was missing from InterfaceVpcEndpointAwsService.

Description of changes

The InterfaceVpcEndpointAwsService constructor was modified to
- I would have preferred to switch to a single object for the optional constructor properties (prefix, port, and now global), but couldn't make a breaking change to a publicly accessible constructor
InterfaceVpcEndpointAwsService.S3_MULTI_REGION_ACCESS_POINTS was changed to be a global VPC endpoint
InterfaceVpcEndpointAwsService.CODECATALYST was added

Description of how you validated changes

I've added a unit test to check that the global endpoints' name were set correctly.

I also added an integration test for InterfaceVpcEndpointAwsService.S3_MULTI_REGION_ACCESS_POINTS.
To test it, I created a publicly accessible EC2 instance on the VPC, connected to it and ran nslookup accesspoint.s3-global.amazonaws.com to make sure it was resolvable (see Configuring a Multi-Region Access Point for use with AWS PrivateLink):

Without the InterfaceVpcEndpointAwsService.S3_MULTI_REGION_ACCESS_POINTS interface endpoint:

$ nslookup accesspoint.s3-global.amazonaws.com
Server:         10.0.0.2
Address:        10.0.0.2#53

Non-authoritative answer:
*** Can't find accesspoint.s3-global.amazonaws.com: No answer

With the InterfaceVpcEndpointAwsService.S3_MULTI_REGION_ACCESS_POINTS interface endpoint:

$ nslookup accesspoint.s3-global.amazonaws.com
Server:         10.0.0.2
Address:        10.0.0.2#53

Non-authoritative answer:
Name:   accesspoint.s3-global.amazonaws.com
Address: 10.0.156.75
Name:   accesspoint.s3-global.amazonaws.com
Address: 10.0.246.83

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.

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

msambol · 2024-03-21T23:26:00Z

@nmussy want to rebase this and i'll take a look?

nmussy · 2024-03-22T05:05:22Z

@msambol Should be good to go, thanks 👍

msambol

LGTM

packages/aws-cdk-lib/aws-ec2/lib/vpc-endpoint.ts

msambol

NIce! Thanks.

aaythapa

Thanks for making this change! Just one nit and question

packages/aws-cdk-lib/aws-ec2/lib/vpc-endpoint.ts

aaythapa · 2024-03-22T17:50:37Z

I would have preferred to switch to a single object for the optional constructor properties (prefix, port, and now global), but couldn't make a breaking change to a publicly accessible constructor

Not sure I fully understand this part of the description, could you elaborate? Did you want to add prefix, port and global as one props like object?

nmussy · 2024-03-22T20:39:46Z

Did you want to add prefix, port and global as one props like object?

Ideally, yes. The Props interface we ended up with still leaves us with undefined args before it, but changing that would cause a breaking change. We could have a factory function I suppose, but I would require storing the global property to allow the constructor to access it.

mergify · 2024-03-25T17:30:01Z

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 · 2024-03-25T17:34:04Z

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

msambol · 2024-03-29T03:37:58Z

@Mergifyio refresh

mergify · 2024-03-29T03:38:01Z

refresh

❌ Command disallowed due to command restrictions in the Mergify configuration.

Details

any of:
- sender-permission>=write
- sender={{author}}

nmussy · 2024-03-29T06:38:17Z

@Mergifyio refresh

mergify · 2024-03-29T06:38:23Z

refresh

✅ Pull request refreshed

aws-cdk-automation · 2024-03-29T07:07:02Z

AWS CodeBuild CI Report

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

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

mergify · 2024-03-29T07:07:20Z

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

nmussy · 2024-03-30T10:40:59Z

@Mergifyio refresh

mergify · 2024-03-30T10:41:04Z

refresh

✅ Pull request refreshed

github-actions bot added bug This issue is a bug. effort/medium Medium work item – several days of effort p2 distinguished-contributor [Pilot] contributed 50+ PRs to the CDK labels Mar 20, 2024

aws-cdk-automation requested a review from a team March 20, 2024 17:27

aws-cdk-automation previously requested changes Mar 20, 2024

View reviewed changes

nmussy added 2 commits March 22, 2024 06:03

fix(ec2): global vpc endpoint support

448bb9c

chore: add integ test

6874c9b

nmussy force-pushed the fix-ec2-global-vpc-endpoints branch from 250a767 to 6874c9b Compare March 22, 2024 05:04

nmussy marked this pull request as ready for review March 22, 2024 05:04

msambol approved these changes Mar 22, 2024

View reviewed changes

msambol reviewed Mar 22, 2024

View reviewed changes

packages/aws-cdk-lib/aws-ec2/lib/vpc-endpoint.ts Outdated Show resolved Hide resolved

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Mar 22, 2024

nmussy added 2 commits March 22, 2024 06:36

chore: props refactor

f9b5657

chore: readonly prop

f598ba8

msambol reviewed Mar 22, 2024

View reviewed changes

packages/aws-cdk-lib/aws-ec2/lib/vpc-endpoint.ts Outdated Show resolved Hide resolved

chore: signature refactor

a346229

msambol approved these changes Mar 22, 2024

View reviewed changes

aaythapa reviewed Mar 22, 2024

View reviewed changes

packages/aws-cdk-lib/aws-ec2/lib/vpc-endpoint.ts Show resolved Hide resolved

github-actions bot mentioned this pull request Mar 25, 2024

Weekly PR metrics report #29602

Closed

aaythapa approved these changes Mar 25, 2024

View reviewed changes

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Mar 25, 2024

Merge branch 'main' into fix-ec2-global-vpc-endpoints

5f4a42d

mergify bot merged commit df48fd7 into aws:main Mar 30, 2024

nmussy deleted the fix-ec2-global-vpc-endpoints branch March 30, 2024 10:41

github-actions bot mentioned this pull request Apr 1, 2024

Monthly PR metrics report #29668

Closed

This was referenced May 23, 2024

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#98

Open

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#99

Open

NOUIY mentioned this pull request May 30, 2024

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#101

Open

aws locked as resolved and limited conversation to collaborators Jul 25, 2024

Conversation

nmussy commented Mar 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue # (if applicable)

Reason for this change

Description of changes

Description of how you validated changes

Checklist

Uh oh!

aws-cdk-automation left a comment

Choose a reason for hiding this comment

Uh oh!

msambol commented Mar 21, 2024

Uh oh!

nmussy commented Mar 22, 2024

Uh oh!

msambol left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

msambol left a comment

Choose a reason for hiding this comment

Uh oh!

aaythapa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aaythapa commented Mar 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nmussy commented Mar 22, 2024

Uh oh!

mergify bot commented Mar 25, 2024

Uh oh!

mergify bot commented Mar 25, 2024

Uh oh!

msambol commented Mar 29, 2024

Uh oh!

mergify bot commented Mar 29, 2024

❌ Command disallowed due to command restrictions in the Mergify configuration.

Uh oh!

nmussy commented Mar 29, 2024

Uh oh!

mergify bot commented Mar 29, 2024

✅ Pull request refreshed

Uh oh!

aws-cdk-automation commented Mar 29, 2024

AWS CodeBuild CI Report

Uh oh!

mergify bot commented Mar 29, 2024

Uh oh!

nmussy commented Mar 30, 2024

Uh oh!

mergify bot commented Mar 30, 2024

✅ Pull request refreshed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

nmussy commented Mar 20, 2024 •

edited

Loading

aaythapa commented Mar 22, 2024 •

edited

Loading