feat(apigateway): expose endpointconfiguration to include vpcEndpointIds #6078

bulka777 · 2020-02-03T21:10:35Z

fixes #6038 by exposing endpointConfiguration to include vpcEndpointIds

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

fixes aws#6038 BREAKING CHANGE: the interface now accepts endpointconfiguration property instead of endpoint type as defined by cfn

aws-cdk-automation · 2020-02-03T21:30:31Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-02-04T14:25:26Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

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

bulka777 · 2020-02-04T14:29:15Z

It looks like the AWS CodeBuild is throwing an error around this:

@aws-cdk/aws-apigateway... CHANGES.
Original assembly: @aws-cdk/aws-apigateway@1.22.0
Updated assembly:  @aws-cdk/aws-apigateway@1.22.0
API elements with incompatible changes:
err  - PROP @aws-cdk/aws-apigateway.LambdaRestApiProps.endpointTypes: has been removed [removed:@aws-cdk/aws-apigateway.LambdaRestApiProps.endpointTypes]
err  - PROP @aws-cdk/aws-apigateway.RestApiProps.endpointTypes: has been removed [removed:@aws-cdk/aws-apigateway.RestApiProps.endpointTypes]

If that's correct, then I must be missing something? I expected there to be breaking changes with this. Any recommendations on how to proceed to resolve this?

nija-at · 2020-02-05T10:35:01Z

apigateway is a stable module. By contract, breaking changes are not allowed in this package.

The closest you can do is mark methods and properties as @deprecated and add new ones. The changes must stay backwards compatible.

nija-at

Please add entries into the README describing your changes and code snippets.

Fix the PR title so it correctly describes the new feature being added. I would suggest "support private vpc endpoints"

packages/@aws-cdk/aws-apigateway/lib/restapi.ts

Pull request has been modified.

aws-cdk-automation · 2020-02-05T18:52:18Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-02-05T18:52:41Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

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

…a777/aws-cdk into bulka777/add_vpcEndpointIds

aws-cdk-automation · 2020-02-05T19:46:21Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-02-07T14:56:10Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

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

bulka777 · 2020-02-07T15:02:37Z

Your suggestions have been implemented. Ready for review!

nija-at

It's mostly there. A few comments and suggestions below to polish it up -

packages/@aws-cdk/aws-apigateway/README.md

nija-at · 2020-02-10T11:12:13Z

packages/@aws-cdk/aws-apigateway/lib/restapi.ts

+      const vpcEndpointIds = (props.endpointConfiguration.vpcEndpoints) ?
+        props.endpointConfiguration.vpcEndpoints.map(vpcEndpoint => vpcEndpoint.vpcEndpointId) : undefined;


Would optional chaining simplify this?

Suggested change

const vpcEndpointIds = (props.endpointConfiguration.vpcEndpoints) ?

props.endpointConfiguration.vpcEndpoints.map(vpcEndpoint => vpcEndpoint.vpcEndpointId) : undefined;

const vpcEndpointIds = props.endpointConfiguration.vpcEndpoints?.map(vpce => vpce.vpcEndpointId);

packages/@aws-cdk/aws-apigateway/package.json

packages/@aws-cdk/aws-apigateway/lib/restapi.ts

packages/@aws-cdk/aws-apigateway/README.md

nija-at · 2020-02-10T11:31:50Z

packages/@aws-cdk/aws-apigateway/README.md

+You can also create an association between your Rest Api and a Vpc Endpoint. By doing so,
+Api Gateway will generate a new Route53 Alias DNS record which you can use to invoke your private
+APIs. Here is an example:
+
+```ts
+const someEndpoint: IVpcEndpoint = /* Get or Create endpoint here */
+const api = new apigw.RestApi(stack, 'api', {
+  endpointConfiguration: {
+    types: [ apigw.EndpointType.PRIVATE ],
+    vpcEndpoints: [someEndpoint]
+  }
+});
+```
+
+By performing this association, we can invoke the api gateway using the following format:
+```
+https://{rest-api-id}-{vpce-id}.execute-api.{region}.amazonaws.com/{stage}
+```


How hard is it to add an integration test that sets up a simple VPC endpoint and associates it to the Rest API, such that it is invokable by running curl against https://{rest-api-id}-{vpce-id}.execute-api.{region}.amazonaws.com/{stage}?

This would be useful as described here as well as to link off from the documentation as a working example.

Thanks for the feedback. No problem at all. I can go ahead and add the integration test, but you are not suggesting to add an automated way to curl right? Just create the integration test and add Stack verification steps: so that it could be tested manually?

Sorry for the late reply. Yes, that's correct, Manual verification is fine.

Pull request has been modified.

aws-cdk-automation · 2020-02-10T18:05:35Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

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

proposed changes Co-Authored-By: Niranjan Jayakar <16217941+nija-at@users.noreply.github.com>

aws-cdk-automation · 2020-02-10T20:40:52Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-02-11T04:10:10Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

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

nija-at

Could you add the suggested integration test please?

packages/@aws-cdk/aws-apigateway/README.md

packages/@aws-cdk/aws-apigateway/lib/restapi.ts

Pull request has been modified.

aws-cdk-automation · 2020-02-18T23:09:18Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 1bf0509
Result: FAILED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-02-18T23:57:26Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 92de69b
Result: FAILED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-02-19T00:49:54Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: f809617
Result: SUCCEEDED
Build Logs (available for 30 days)

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

bulka777 · 2020-02-19T00:54:26Z

@nija-at Thanks for reviewing. I added the integration test - I think it's ready for another go.

mergify · 2020-02-19T15:53:28Z

Thank you for contributing! Your pull request is now being automatically merged.

aws-cdk-automation · 2020-02-19T21:38:40Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 4f91e24
Result: SUCCEEDED
Build Logs (available for 30 days)

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

mergify · 2020-02-19T21:39:06Z

Thank you for contributing! Your pull request is now being automatically merged.

feat(apigateway): expose endpointconfiguration to include vpcEndpointIds

629365d

fixes aws#6038 BREAKING CHANGE: the interface now accepts endpointconfiguration property instead of endpoint type as defined by cfn

SomayaB assigned nija-at Feb 3, 2020

Merge branch 'master' into bulka777/add_vpcEndpointIds

f5aa716

nija-at previously requested changes Feb 5, 2020

View reviewed changes

packages/@aws-cdk/aws-apigateway/lib/restapi.ts Outdated Show resolved Hide resolved

Abduljalil, Timur and others added 2 commits February 5, 2020 13:01

feature(apigateway): support private vpc endpoints

1947b9a

Merge branch 'master' into bulka777/add_vpcEndpointIds

946052f

Abduljalil, Timur added 2 commits February 5, 2020 14:06

feature(apigateway): support private vpc endpoints

5ab5bcf

Merge branch 'bulka777/add_vpcEndpointIds' of https://github.com/bulk…

2a400d9

…a777/aws-cdk into bulka777/add_vpcEndpointIds

Merge branch 'master' into bulka777/add_vpcEndpointIds

bfab93a

nija-at previously requested changes Feb 10, 2020

View reviewed changes

Merge branch 'master' into bulka777/add_vpcEndpointIds

b0b55d0

fix(apigateway): documentation changes for the endpoint configuration

5336844

proposed changes Co-Authored-By: Niranjan Jayakar <16217941+nija-at@users.noreply.github.com>

fix(apigateway): apply recommended changes

a0e3d1f

nija-at previously requested changes Feb 17, 2020

View reviewed changes

packages/@aws-cdk/aws-apigateway/README.md Outdated Show resolved Hide resolved

packages/@aws-cdk/aws-apigateway/README.md Outdated Show resolved Hide resolved

packages/@aws-cdk/aws-apigateway/lib/restapi.ts Outdated Show resolved Hide resolved

Merge branch 'master' into bulka777/add_vpcEndpointIds

1bf0509

fix(apigateway): adding requested changes and integration test

92de69b

fix(apigateway): incorrect vpc endpoint in integration test

f809617

nija-at approved these changes Feb 19, 2020

View reviewed changes

Merge branch 'master' into bulka777/add_vpcEndpointIds

4f91e24

mergify bot merged commit 99de6ca into aws:master Feb 19, 2020

bulka777 deleted the bulka777/add_vpcEndpointIds branch February 19, 2020 21:39

stefanolczak mentioned this pull request Mar 18, 2020

feat(apigateway): support vpc endpoints #5537

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(apigateway): expose endpointconfiguration to include vpcEndpointIds #6078

feat(apigateway): expose endpointconfiguration to include vpcEndpointIds #6078

bulka777 commented Feb 3, 2020 •

edited

Loading

aws-cdk-automation commented Feb 3, 2020

aws-cdk-automation commented Feb 4, 2020

bulka777 commented Feb 4, 2020

nija-at commented Feb 5, 2020

nija-at left a comment

aws-cdk-automation commented Feb 5, 2020

aws-cdk-automation commented Feb 5, 2020

aws-cdk-automation commented Feb 5, 2020

aws-cdk-automation commented Feb 7, 2020

bulka777 commented Feb 7, 2020

nija-at left a comment •

edited

Loading

nija-at Feb 10, 2020

nija-at Feb 10, 2020

bulka777 Feb 10, 2020 •

edited

Loading

nija-at Feb 17, 2020

aws-cdk-automation commented Feb 10, 2020

aws-cdk-automation commented Feb 10, 2020

aws-cdk-automation commented Feb 11, 2020

nija-at left a comment

aws-cdk-automation commented Feb 18, 2020

aws-cdk-automation commented Feb 18, 2020

aws-cdk-automation commented Feb 19, 2020

bulka777 commented Feb 19, 2020

mergify bot commented Feb 19, 2020

aws-cdk-automation commented Feb 19, 2020

mergify bot commented Feb 19, 2020

		const vpcEndpointIds = (props.endpointConfiguration.vpcEndpoints) ?
		props.endpointConfiguration.vpcEndpoints.map(vpcEndpoint => vpcEndpoint.vpcEndpointId) : undefined;

	const vpcEndpointIds = (props.endpointConfiguration.vpcEndpoints) ?
	props.endpointConfiguration.vpcEndpoints.map(vpcEndpoint => vpcEndpoint.vpcEndpointId) : undefined;
	const vpcEndpointIds = props.endpointConfiguration.vpcEndpoints?.map(vpce => vpce.vpcEndpointId);

feat(apigateway): expose endpointconfiguration to include vpcEndpointIds #6078

feat(apigateway): expose endpointconfiguration to include vpcEndpointIds #6078

Conversation

bulka777 commented Feb 3, 2020 • edited Loading

aws-cdk-automation commented Feb 3, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Feb 4, 2020

AWS CodeBuild CI Report

bulka777 commented Feb 4, 2020

nija-at commented Feb 5, 2020

nija-at left a comment

Choose a reason for hiding this comment

aws-cdk-automation commented Feb 5, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Feb 5, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Feb 5, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Feb 7, 2020

AWS CodeBuild CI Report

bulka777 commented Feb 7, 2020

nija-at left a comment • edited Loading

Choose a reason for hiding this comment

nija-at Feb 10, 2020

Choose a reason for hiding this comment

nija-at Feb 10, 2020

Choose a reason for hiding this comment

bulka777 Feb 10, 2020 • edited Loading

Choose a reason for hiding this comment

nija-at Feb 17, 2020

Choose a reason for hiding this comment

aws-cdk-automation commented Feb 10, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Feb 10, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Feb 11, 2020

AWS CodeBuild CI Report

nija-at left a comment

Choose a reason for hiding this comment

aws-cdk-automation commented Feb 18, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Feb 18, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Feb 19, 2020

AWS CodeBuild CI Report

bulka777 commented Feb 19, 2020

mergify bot commented Feb 19, 2020

aws-cdk-automation commented Feb 19, 2020

AWS CodeBuild CI Report

mergify bot commented Feb 19, 2020

bulka777 commented Feb 3, 2020 •

edited

Loading

nija-at left a comment •

edited

Loading

bulka777 Feb 10, 2020 •

edited

Loading