Skip to content

api gateway endpoint configuration support#1160

Merged
kyleknap merged 6 commits intoaws:masterfrom
kapilt:api-gw-endpoint-config
Jul 28, 2019
Merged

api gateway endpoint configuration support#1160
kyleknap merged 6 commits intoaws:masterfrom
kapilt:api-gw-endpoint-config

Conversation

@kapilt
Copy link
Contributor

@kapilt kapilt commented Jul 1, 2019

Closes #956
Closes #897
Closes #976

Support configuring and updating the api gateway endpoint type (EDGE (default), REGIONAL, PRIVATE).

Support configure api gateway resource policy, with additional provisions for automatic policy construction in case of PRIVATE endpoints if VPC Endpoint is configured (else a manually specified policy is required).

Note for PRIVATE endpoints this does not create a vpc endpoint, which needs to be created out of band. A single vpc endpoint can be used for multiple apis though, so its one time per region config per account.

@kapilt kapilt force-pushed the api-gw-endpoint-config branch from fe1cd84 to d4d6f47 Compare July 1, 2019 23:01
@codecov-io
Copy link

codecov-io commented Jul 1, 2019

Codecov Report

Merging #1160 into master will increase coverage by <.01%.
The diff coverage is 96.49%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1160      +/-   ##
==========================================
+ Coverage   95.81%   95.81%   +<.01%     
==========================================
  Files          28       28              
  Lines        4973     5021      +48     
  Branches      626      636      +10     
==========================================
+ Hits         4765     4811      +46     
- Misses        135      136       +1     
- Partials       73       74       +1
Impacted Files Coverage Δ
chalice/package.py 100% <ø> (ø) ⬆️
chalice/deploy/planner.py 96.99% <100%> (+0.05%) ⬆️
chalice/config.py 95.78% <100%> (+0.15%) ⬆️
chalice/constants.py 100% <100%> (ø) ⬆️
chalice/deploy/validate.py 100% <100%> (ø) ⬆️
chalice/cli/factory.py 92.4% <100%> (+0.04%) ⬆️
chalice/awsclient.py 94.55% <100%> (+0.01%) ⬆️
chalice/deploy/swagger.py 100% <100%> (ø) ⬆️
chalice/deploy/models.py 99.43% <100%> (ø) ⬆️
chalice/deploy/deployer.py 97.8% <77.77%> (-0.45%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 32fabb3...b86b569. Read the comment docs.

@kapilt kapilt force-pushed the api-gw-endpoint-config branch 6 times, most recently from 42e6d4d to 5f3f76a Compare July 3, 2019 12:57
Copy link
Contributor

@stealthycoin stealthycoin left a comment

Choose a reason for hiding this comment

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

Looks good to me, just one comment about docs.

@kapilt kapilt force-pushed the api-gw-endpoint-config branch from 5f3f76a to bc03e60 Compare July 17, 2019 12:16
@kapilt
Copy link
Contributor Author

kapilt commented Jul 17, 2019

ci is failing because with the websocket addition, new additions to planner end up going above the module size limit. I don't see many options here beyond disable this check globally for pylint

chalice/deploy/planner.py:2:0: C0302: Too many lines in module (1009/1000) (too-many-lines)

@kapilt kapilt force-pushed the api-gw-endpoint-config branch from bc03e60 to 4c3b37b Compare July 17, 2019 12:30
Copy link
Contributor

@kyleknap kyleknap left a comment

Choose a reason for hiding this comment

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

I just wanted to post some feedback that can be addressed. Just capturing some of the offline discussion we had...

In short the new configuration option, looks good to me. However for private api gateway, it seems like there needs to be configuration on the api gateway policy to get this all to work (we still need to confirm this). This would means we probably would want to expose a vpc endpoint id as a configuration option to auto-inject a policy and also probably expose the ability to provide a policy in the .chalice/config.json file. If it ends up being that we do need a policy applied, it may make sense to scope this PR down to regional endpoint types. Then make two subsequent PR's to support api policy and then private api's in order to narrow the scope of the PR's and not completely block support for regional endpoints.

@kapilt kapilt force-pushed the api-gw-endpoint-config branch from 7eace46 to 91d4c67 Compare July 18, 2019 19:54
@kapilt
Copy link
Contributor Author

kapilt commented Jul 18, 2019

i went ahead and disabled the PRIVATE endpoint type (with validation) for a followup branch that will expose api policy and configuring private endpoint types.

[update] per comments below, i went ahead and added in private endpoints back into the branch with auto or manual policy configuration and vpce config.

@kapilt kapilt force-pushed the api-gw-endpoint-config branch 3 times, most recently from 922096f to b1d10bc Compare July 19, 2019 18:31
@kapilt kapilt force-pushed the api-gw-endpoint-config branch 5 times, most recently from 2fd7d12 to 0b2268a Compare July 19, 2019 20:48
@kapilt kapilt force-pushed the api-gw-endpoint-config branch from 0b2268a to 0657b87 Compare July 19, 2019 20:57
@kapilt
Copy link
Contributor Author

kapilt commented Jul 19, 2019

fwiw, addressed review comments and did some manual verification, change from review are in the second commit.

@kapilt kapilt force-pushed the api-gw-endpoint-config branch from 386b065 to 0a99f48 Compare July 20, 2019 03:54
@kapilt
Copy link
Contributor Author

kapilt commented Jul 20, 2019

i went ahead and added in support for private endpoints and automated or manual policy config to this pr.

@kapilt kapilt force-pushed the api-gw-endpoint-config branch from 0a99f48 to 2b303d5 Compare July 20, 2019 04:04
@kapilt kapilt force-pushed the api-gw-endpoint-config branch 2 times, most recently from 7eb0d61 to aa3e706 Compare July 20, 2019 04:17
@kapilt kapilt force-pushed the api-gw-endpoint-config branch 4 times, most recently from 1cc1e4a to 13d5e74 Compare July 21, 2019 01:07
@kapilt kapilt force-pushed the api-gw-endpoint-config branch from 13d5e74 to 117adae Compare July 21, 2019 02:46
Copy link
Contributor

@kyleknap kyleknap left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for adding the vpce and APIG policies support. I mainly had ideas/feedback on these newly added configuration options.

@kapilt kapilt force-pushed the api-gw-endpoint-config branch from 845aeae to 2f0038e Compare July 24, 2019 18:01
@kapilt kapilt force-pushed the api-gw-endpoint-config branch 2 times, most recently from 3bb06a1 to b86b569 Compare July 25, 2019 06:33
@kapilt kapilt force-pushed the api-gw-endpoint-config branch from b86b569 to 314ab78 Compare July 25, 2019 13:39
Copy link
Contributor

@kyleknap kyleknap left a comment

Choose a reason for hiding this comment

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

Awesome! This looks great to me. Thanks for all of the work on this. Merging

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

Labels

None yet

Projects

None yet

5 participants