Add support for specifying cache key parameters in API Gateway.

[betabandido]

Pull request to fix issue #856

This pull request provides a way to specify cache key parameters, so that caching can take into account varying path parameters or query strings.

I have tested the changes, and I was able to effectively create, read or update cache key parameters. The following (adapted) example shows how to do so for path parameters:

resource "aws_api_gateway_resource" "path1" {
  rest_api_id = "${aws_api_gateway_rest_api.api.id}"
  parent_id   = "${aws_api_gateway_rest_api.api.root_resource_id}"
  path_part   = "{param1}"
}

resource "aws_api_gateway_resource" "path2" {
  rest_api_id = "${aws_api_gateway_rest_api.api.id}"
  parent_id   = "${aws_api_gateway_resource.path1.id}"
  path_part   = "{param2}"
}

resource "aws_api_gateway_method" "method" {
  rest_api_id   = "${aws_api_gateway_rest_api.api.id}"
  resource_id   = "${aws_api_gateway_resource.path2.id}"
  http_method   = "GET"
  authorization = "NONE"
  request_parameters = {
      "method.request.path.param1" = true
      "method.request.path.param2" = true
  }
}

resource "aws_api_gateway_integration" "integration" {
  rest_api_id   = "${aws_api_gateway_rest_api.api.id}"
  resource_id   = "${aws_api_gateway_resource.path2.id}"
  http_method   = "${aws_api_gateway_method.method.http_method}"
  type          = "AWS"
  uri           = "arn:aws:apigateway:${var.region}:lambda:path/2015-03-31/functions/${var.lambda_arn}/invocations"
  integration_http_method = "POST"
  request_parameters = {
      "integration.request.path.param1" = "method.request.path.param1"
      "integration.request.path.param2" = "method.request.path.param2"
  }
  cache_key_parameters = ["method.request.path.param1", "method.request.path.param2"]
}

The example shows how to set the new field (cache_key_parameters) as well as request_parameters (both in the method and integration resources).

[radeksimko]

Thanks for the PR, I left you 2 comments/questions there.

[radeksimko]

Is there any reason the cache namespace should always be resource ID?
I had a peek 👀 into the official API docs and couldn't find any suggestions around that: https://docs.aws.amazon.com/apigateway/api-reference/link-relation/integration-put/#cacheNamespace

If there isn't a reason I think we should let the user specify their own namespace.

Also I'm not sure what kind of data it contains and how these are structured, so it's hard to tell (for me at least) whether resource_id is the best namespace key - e.g. does it automatically assign the cache distribution to a given API ID so that we don't need to put API ID to the namespace? Same questions about other fields. If it's not clear from the docs it would be worth asking the AWS support - I'm happy to ask for you.

[radeksimko]

Also I think we should leave this as nil in case the user didn't specify any caching.

[betabandido]

I observed that when caching was manually enabled the namespace set by AWS matched the resource id. But, as you well say it might not be a requirement. I will conduct a test using a custom namespace. If it works, users should indeed be able to specify it.

In the meanwhile, if you can please ask AWS support that would be great. Hopefully we can then have a clear understanding of the usage (and the potential restrictions) of a cache namespace.

Setting the default to nil sounds good to me. I will do that.

[betabandido]

I just updated the pull request with your feedback. I did some testing and using a user-defined cache namespace works too. So, I added a field named cache_namespace.

[radeksimko]

Instead of expanding this _basic test would you mind creating a separate one? That way we can also run tests much faster as we generally run whole tests in parallel, we cannot do that with test steps.

[betabandido]

Do you mean adding a new function named func TestAccAWSAPIGatewayIntegration_cache_key_parameters(t *testing.T) (or similar) to the same file resource_aws_api_gateway_integration_test.go?

[radeksimko]

TestAccAWSAPIGatewayIntegration_cache_key_parameters - that's pretty much what I mean. There is no need for a new file.

[radeksimko]

Thank you for making those changes.

I ran the relevant acceptance tests and it looks like AWS is setting the namespace by default to the resource_id (even if not specified in the create request), which is causing spurious diff:

=== RUN   TestAccAWSAPIGatewayIntegration_basic
--- FAIL: TestAccAWSAPIGatewayIntegration_basic (51.39s)
	testing.go:428: Step 0 error: After applying this step, the plan was not empty:

		DIFF:

		UPDATE: aws_api_gateway_integration.test
		  cache_namespace: "or4rhf" => ""

The good news is that this behaviour answers my earlier question about how the data are structured in the cache. If AWS chooses to use this key by default then it should be good (unique) enough.

Do you mind setting cache_namespace as Computed, so that Terraform can accept such default namespace instead of trying to change it to empty?

It would be obviously best if we could just set a Default to resource_id, but we don't have a good mechanism in the schema to do this - there's no way to get access to values of other fields in the schema, so Computed is the only option for now.

[betabandido]

Done :) I ran the two acceptance tests for the integration resource, and both of them passed.

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!