-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
feat(apigateway): configure endpoint types on SpecRestApi #9068
feat(apigateway): configure endpoint types on SpecRestApi #9068
Conversation
… for the SpecRestAPI
…to be set for an openApi 3.0 document
…check for private api
I will check tomorrow or later tonight. I think is just that I reused the same name as another stack (a wee bit too much copy paste) in the integration test |
…of github.com:IsmaelMartinez/aws-cdk into ismaelmartinez/add-endpointconfiguration-for-openapi3
Hi @nija-at , The PR now is ready. I can probably add a better example to the README but wanted to get 1st the approach approved by you. Basically, if you do the openAPI import like this.
With something like this openAPI:
The deployment fails with the following message:
Do let me know if you prefer another approach for this. Thanks in advance! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @IsmaelMartinez -
Thanks for submitting this PR!
@@ -0,0 +1,69 @@ | |||
import * as cdk from '@aws-cdk/core'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of an integration test, add a unit test here instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a unit test. Waiting for the build to confirm it works... my machine (and GitPot) unit test keep failing in jest timeouts so using this slower approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Drop this integration test.
…of github.com:IsmaelMartinez/aws-cdk into ismaelmartinez/add-endpointconfiguration-for-openapi3
…of github.com:IsmaelMartinez/aws-cdk into ismaelmartinez/add-endpointconfiguration-for-openapi3
@IsmaelMartinez - apologies, I was unavailable the last 2 weeks. I will take another look at this during this week. You do not need to keep sync'ing with master. Once approved, our bot will do that once in the end before it is merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks mostly good.
Some comments around documentation below.
@@ -0,0 +1,69 @@ | |||
import * as cdk from '@aws-cdk/core'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Drop this integration test.
Co-authored-by: Niranjan Jayakar <nija@amazon.com>
Co-authored-by: Niranjan Jayakar <nija@amazon.com>
Co-authored-by: Niranjan Jayakar <nija@amazon.com>
Co-authored-by: Niranjan Jayakar <nija@amazon.com>
comments addressed. If the build builds... we are good to go. Configuring a new laptop so took a bit longer than I wanted for this tiny changes. thanks for the review! |
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
@nija-at is this included in a released version yet? |
feat(apigateway): adding the ability to set the endpoint configuration for the OpenAPI 3.0 With this change, it will be possible to modify this by providing the endpointTypes as shown here: ``` const api = new apigateway.SpecRestApi(this, 'ExampleRestApi', { apiDefinition: apigateway.ApiDefinition.fromInline(replacedSwagger), endpointTypes: [apigateway.EndpointType.PRIVATE], }); ``` Note: For private endpoints you will still need to provide the `x-amazon-apigateway-endpoint-configuration` and `x-amazon-apigateway-policy` in your openApi file. The following is an example with both settings: ```json { "openapi": "3.0.2", "servers" : [ { "x-amazon-apigateway-endpoint-configuration": { "vpcEndpointIds": [ "vpce-00111a1111a1aa011" ] } } ], "paths": { ... }, "x-amazon-apigateway-policy": { "Version": "2012-10-17", "Statement": [ { "Effect": "Allow", "Principal": "*", "Action": [ "execute-api:Invoke", "execute-api:GET" ], "Resource": "arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:*", "Condition": { "StringEquals": { "aws:sourceVpce": "vpce-00111a1111a1aa011" } } } ] } } ``` Checklist for this PR: 🧪 Testing: adding integration testing for private API gateway. 📄 Docs: Add example in the README documentation about how to create a private API gateway with swagger Fixes #9060 By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
feat(apigateway): adding the ability to set the endpoint configuration for the OpenAPI 3.0 With this change, it will be possible to modify this by providing the endpointTypes as shown here: ``` const api = new apigateway.SpecRestApi(this, 'ExampleRestApi', { apiDefinition: apigateway.ApiDefinition.fromInline(replacedSwagger), endpointTypes: [apigateway.EndpointType.PRIVATE], }); ``` Note: For private endpoints you will still need to provide the `x-amazon-apigateway-endpoint-configuration` and `x-amazon-apigateway-policy` in your openApi file. The following is an example with both settings: ```json { "openapi": "3.0.2", "servers" : [ { "x-amazon-apigateway-endpoint-configuration": { "vpcEndpointIds": [ "vpce-00111a1111a1aa011" ] } } ], "paths": { ... }, "x-amazon-apigateway-policy": { "Version": "2012-10-17", "Statement": [ { "Effect": "Allow", "Principal": "*", "Action": [ "execute-api:Invoke", "execute-api:GET" ], "Resource": "arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:*", "Condition": { "StringEquals": { "aws:sourceVpce": "vpce-00111a1111a1aa011" } } } ] } } ``` Checklist for this PR: 🧪 Testing: adding integration testing for private API gateway. 📄 Docs: Add example in the README documentation about how to create a private API gateway with swagger Fixes aws#9060 By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
Looks like you can't, and should not, use the If you do, it complains that you can only specify the endpoint configuration for private API Gateway. Using the policy I will create a documentation fix ticket in #9588. |
Actually, closing that documentation ticket and creating a bug as there is an issue on 1st creation. Details in #9684 |
feat(apigateway): adding the ability to set the endpoint configuration for the OpenAPI 3.0
With this change, it will be possible to modify this by providing the endpointTypes as shown here:
Note: For private endpoints you will still need to provide the x-amazon-apigateway-endpoint-configuration and x-amazon-apigateway-policy in your openApi file.
The following is an example with both settings:
Checklist for this PR:
🧪 Testing: adding integration testing for private API gateway.
📄 Docs: Add example in the README documentation about how to create a private API gateway with swagger
Fixes #9060
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license