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(ec2): add missing vpc endpoints #29524

Merged
merged 7 commits into from
Mar 21, 2024
Merged

chore(ec2): add missing vpc endpoints #29524

merged 7 commits into from
Mar 21, 2024

Conversation

msambol
Copy link
Contributor

@msambol msambol commented Mar 18, 2024

I used this doc for the "friendly" names and tried to make them as close as possible to the AWS service names.

Closes #29523.


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 March 18, 2024 03:25
@github-actions github-actions bot added feature-request A feature should be added or improved. p2 distinguished-contributor [Pilot] contributed 50+ PRs to the CDK labels Mar 18, 2024
@msambol
Copy link
Contributor Author

msambol commented Mar 18, 2024

@nmussy Here's another thing you could automate with your tool. :)

@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 Mar 18, 2024
@nmussy
Copy link
Contributor

nmussy commented Mar 18, 2024

Thanks for letting me know! I'll probably release the first version later today or tomorrow

@nmussy
Copy link
Contributor

nmussy commented Mar 18, 2024

@msambol Just did a quick run of using DescribeVpcEndpointServices, and there are quite a few more missing:

appmesh
b2bi
bedrock
bedrock-agent
bedrock-agent-runtime
bedrock-runtime
cleanrooms
codewhisperer
console
data-servicediscovery
data-servicediscovery-fips
datazone
ds
dynamodb
eks-auth
emrwal.prod
entityresolution
guardduty-data
guardduty-data-fips
iot.credentials
iot.fleethub.api
iotfleetwise
license-manager-user-subscriptions
managedblockchain-query
managedblockchain.bitcoin.mainnet
managedblockchain.bitcoin.testnet
mediaconnect
medical-imaging
neptune-graph
networkmonitor
organizations
organizations-fips
payment-cryptography.controlplane
payment-cryptography.dataplane
pca-connector-ad
personalize
personalize-events
personalize-runtime
pinpoint
runtime-medical-imaging
s3express
scn
servicediscovery
servicediscovery-fips
signin
simspaceweaver
streaming-rekognition
streaming-rekognition-fips
swf
swf-fips
thinclient.api
timestream-influxdb
tnb
trustedadvisor
vpc-lattice

I haven't validated them yet, but if you want to wait until the CLI is available, you can complete this PR with it? If not, I'll just open another one later 👍

@msambol
Copy link
Contributor Author

msambol commented Mar 18, 2024

@nmussy I'll knock it out quickly, thanks for the list. 👍🏼

One note for you tool.. Make sure you check GatewayVpcEndpointAwsService. DynamoDB is there.

@nmussy
Copy link
Contributor

nmussy commented Mar 18, 2024 via email

@@ -265,6 +266,7 @@ export class InterfaceVpcEndpointAwsService implements IInterfaceVpcEndpointServ
public static readonly AIRFLOW_OPS = new InterfaceVpcEndpointAwsService('airflow.ops');
public static readonly APIGATEWAY = new InterfaceVpcEndpointAwsService('execute-api');
public static readonly APP_MESH = new InterfaceVpcEndpointAwsService('appmesh-envoy-management');
public static readonly APP_MESH_OPS = new InterfaceVpcEndpointAwsService('appmesh');
Copy link
Contributor Author

@msambol msambol Mar 18, 2024

Choose a reason for hiding this comment

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

This one is named weird but I don't have much choice.

Copy link
Contributor

Choose a reason for hiding this comment

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

Might be worth deprecating APP_MESH and recreating APP_MESH_ENVOY_MANAGEMENT, or even APPMESH_ENVOY_MANAGEMENT

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated this as well

public static readonly PERSONALIZE = new InterfaceVpcEndpointAwsService('personalize');
public static readonly PERSONALIZE_EVENTS = new InterfaceVpcEndpointAwsService('personalize-events');
public static readonly PERSONALIZE_RUNTIME = new InterfaceVpcEndpointAwsService('personalize-runtime');
public static readonly PINPOINT_V1 = new InterfaceVpcEndpointAwsService('pinpoint');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't like this name either but I don't have much choice.

Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, deprecate PINPOINT, and point to a correctly renamed PINPOINT_SMS_VOICE_V2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But then call InterfaceVpcEndpointAwsService('pinpoint'); what?

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess PINPOINT_V1 or something similar, but with both PINPOINT and PINPOINT_V1 being valid, I would expect PINPOINT to be InterfaceVpcEndpointAwsService('pinpoint');. I feel like also renaming it would lower the chances of confusion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

let me know what you think of that...

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I think this is the solution less likely to cause pitfalls for future users, even if it doesn't feel great. Thanks 👍

@msambol msambol changed the title chore(ec2): add bedrock vpc endpoints chore(ec2): add missing vpc endpoints Mar 18, 2024
@nmussy
Copy link
Contributor

nmussy commented Mar 20, 2024

Hey @msambol, I found a few more missing, some only available in certain regions:

Available in us-east-2:

codecatalyst.git
codecatalyst.packages
repostspace
sagemaker-geospatial

Available in eu-central-1:

inspector-scan

I couldn't find any documentation about this service, but it's currently being listed, might be worth adding it with the io.spotinst.vpce prefix:

io.spotinst.vpce.us-east-1.privatelink-api

Finally, I am unable to list the two following endpoints. They should still be available according to the docs, so no need to deprecate them. I'm unsure if they're never supposed to be listed by DescribeVpcEndpointServices, or if my account isn't set up to access these services

  • migrationhub-strategy (docs)
$ aws ec2 describe-vpc-endpoint-services --region=us-east-1 --service-names=com.amazonaws.us-east-1.migrationhub-strategy

An error occurred (InvalidServiceName) when calling the DescribeVpcEndpointServices operation: The Vpc Endpoint Service 'com.amazonaws.us-east-1.migrationhub-strategy' does not exist
aws ec2 describe-vpc-endpoint-services --region=us-east-1 --service-names=com.amazonaws.us-east-1.sms-fips

An error occurred (InvalidServiceName) when calling the DescribeVpcEndpointServices operation: The Vpc Endpoint Service 'com.amazonaws.us-east-1.sms-fips' does not exist

@nmussy
Copy link
Contributor

nmussy commented Mar 20, 2024

We also have an issue with global endpoints, e.g. S3_MULTI_REGION_ACCESS_POINTS. They are not supposed to have a region prefix (docs), but currently do in the CDK:

$ 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

The region is currently always prefixed:

this.name = `${prefix || defaultEndpointPrefix}.${region}.${name}${defaultEndpointSuffix}`;

I haven't checked if there are other existing cases, but aws.api.global.codecatalyst is currently missing from the endpoint list, and will run into the same issue (docs)

@msambol
Copy link
Contributor Author

msambol commented Mar 20, 2024

I'll get the other ones added. Let's save the global endpoints work for another PR.

@msambol
Copy link
Contributor Author

msambol commented Mar 20, 2024

@nmussy Mind checking this again? Also, io.spotinst.vpce.us-east-1.privatelink-api is an AWS Marketplace service. I don't think we should add that?

@nmussy
Copy link
Contributor

nmussy commented Mar 20, 2024

@nmussy Mind checking this again?

LGTM 👍

Also, io.spotinst.vpce.us-east-1.privatelink-api is an AWS Marketplace service. I don't think we should add that?

io.spotinst.vpce.us-east-1.privatelink-api is definitely being listed by DescribeVpcEndpointServices, no idea why. I agree though, it probably should be ignored, I'll add an exception for it to my tool.

$ aws ec2 describe-vpc-endpoint-services --region=us-east-1 --service-names=io.spotinst.vpce.us-east-1.privatelink-api | jq '.ServiceDetails[] | .ServiceName'

"io.spotinst.vpce.us-east-1.privatelink-api"

Copy link
Contributor

@paulhcsun paulhcsun left a comment

Choose a reason for hiding this comment

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

Thanks @msambol! And thanks for reviewing @nmussy!

@@ -366,14 +390,22 @@ export class InterfaceVpcEndpointAwsService implements IInterfaceVpcEndpointServ
public static readonly GRAFANA = new InterfaceVpcEndpointAwsService('grafana');
public static readonly GRAFANA_WORKSPACE = new InterfaceVpcEndpointAwsService('grafana-workspace');
public static readonly GROUNDSTATION = new InterfaceVpcEndpointAwsService('groundstation');
public static readonly GUARDDUTY_DATA = new InterfaceVpcEndpointAwsService('guardduty-data');
public static readonly GUARDDUTY_DATA_FIPS = new InterfaceVpcEndpointAwsService('guardduty-data-fips');
public static readonly HEALTH_IMAGING = new InterfaceVpcEndpointAwsService('medical-imaging');
Copy link
Contributor

Choose a reason for hiding this comment

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

mildly annoying that the naming doesn't match up here but it is what it is 🤷‍♂️

@paulhcsun
Copy link
Contributor

@Mergifyio update

Copy link
Contributor

mergify bot commented Mar 21, 2024

update

✅ Branch has been successfully updated

Copy link
Contributor

mergify bot commented Mar 21, 2024

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-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Mar 21, 2024
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: 66af188
  • 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 f63656e into aws:main Mar 21, 2024
12 checks passed
Copy link
Contributor

mergify bot commented Mar 21, 2024

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

jun1-t pushed a commit to jun1-t/aws-cdk that referenced this pull request Mar 23, 2024
I used [this doc](https://docs.aws.amazon.com/vpc/latest/privatelink/aws-services-privatelink-support.html) for the "friendly" names and tried to make them as close as possible to the AWS service names.

Closes aws#29523.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
ahammond pushed a commit to ahammond/aws-cdk that referenced this pull request Mar 26, 2024
I used [this doc](https://docs.aws.amazon.com/vpc/latest/privatelink/aws-services-privatelink-support.html) for the "friendly" names and tried to make them as close as possible to the AWS service names.

Closes aws#29523.

----

*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
distinguished-contributor [Pilot] contributed 50+ PRs to the CDK feature-request A feature should be added or improved. p2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

aws_ec2: InterfaceVpcEndpointAwsService not supported for bedrock-runtime and bedrock
4 participants