enable api keys for query endpoints #527
Conversation
| @@ -52,13 +52,13 @@ variable "api_gateway_usage_plans" { | |||
| default = { | |||
| internal_apps = { | |||
| quota_limit = 10000 # per day | |||
| burst_limit = 100 # per second | |||
| rate_limit = 200 | |||
| burst_limit = 300 | |||
There was a problem hiding this comment.
Maybe increase quota_limit and burst_limit even more (50000/1000)? Kinshuk said we need high limits for processing of large lists - there could be a lot of requests if a customer creates a new large lists with 25000 locations.
There was a problem hiding this comment.
@danscales: adding this to develop branch, merged before adding.
| api_key, payload = apikey | ||
| origin = "https://" + payload["domains"][0] | ||
| headers = {"origin": origin, "x-api-key": api_key} | ||
|
|
There was a problem hiding this comment.
Would be nice to make a single function get_header that replaces these three lines everywhere with headers = get_header(apikey)
| @@ -73,7 +73,7 @@ module "query_post" { | |||
| authorizer_id = aws_api_gateway_authorizer.api_key.id | |||
| api_resource = module.query_resource.aws_api_gateway_resource | |||
|
|
|||
| require_api_key = false | |||
| require_api_key = true | |||
There was a problem hiding this comment.
I don't see a terraform module here for zonal statistics, similar to these 'query_post' and 'query_get' modules. Do we need a terraform module for zonal statistics as well to actually enable rate limiting for zona statistics?
There was a problem hiding this comment.
Good catch, that's a deprecated endpoint and not used much looking at the usage stats. How about I merge this and address that in future iteration.
Pull request checklist
Please check if your PR fulfills the following requirements:
Pull request type
Please check the type of change your PR introduces:
What is the current behavior?
query endpoints are not protected with api key
What is the new behavior?
query endpoints protected and rate limits enabled..
Does this introduce a breaking change?
Other information