provider/aws: add response parameters support to api gateway

[joshrtay]

No description provided.

[pieterjanpintens]

Would it be hard to also add selection_pattern to the AWS API calls?
For some reason this field can be defined but appears to be missing in the api calls.

[radeksimko]

Selection pattern is being fixed in #5893

Your PR is looking pretty good, just few questions:

• would you mind adding at least one extra test case to the existing one that would be testing update of this new field (ideally adding a parameter and removing one)?
• would you mind adding this field into docs for each resource?
• would you mind keeping the // TODO comment there with a suggestion to reimplement this field after mentioned issue is resolved?

Thanks

---

There's also field called request_parameters in aws_api_gateway_integration and aws_api_gateway_method which could be implemented the same way (i.e. request_parameters_in_json), but if you think it's out of scope for this PR, that's totally 👌 .

[radeksimko]

Actually one more thing:

It looks like you're not reading the field back from the API in resourceAwsApiGatewayIntegrationResponseRead and resourceAwsApiGatewayMethodResponseRead. In case something/someone changes these fields outside of Terraform, then the plan would be inaccurate.

[joshrtay]

Yep. No problem.

Thought about implementing request_parameters_in_json, but it isn't as high a priority for me, so I didn't :). I'll include it.

[joshrtay]

@radeksimko It seems like you aren't reading any of the fields back for an IntegrationResponse. It also has SelectionPattern, and ResponseTemplate. Should ResponseTemplate be added in another pull request?

[radeksimko]

@joshrtay

It also has SelectionPattern, and ResponseTemplate

That's being fixed in https://github.com/hashicorp/terraform/pull/5893/files#diff-cdecbad1f1f7f16f3375b30f7e44e844R109 already

[joshrtay]

@radeksimko bailed on request_parameters_in_json for the time being, but I've implemented the rest. AWS's updateMethodResponse API is pretty insane. is there a reason updates dont just use putMethodResponse?

[radeksimko]

This now LGTM, thank you! 👍

---

is there a reason updates dont just use putMethodResponse?

Update via PATCH methods is more efficient as we only need to send fields that are going to be PATCHed, not all of them. It may sound like a small benefit, but if you think about that benefit in scale (more API Gateway Method Responses to update in one scope), it (IMO) makes sense.

Also if someone deletes the resource and Update is basically UPSERT (update or insert/create) - that's what Put* operations mean, then the fact that someone has deleted the resource is completely hidden from the user. If Update only updates existing resource and the resource is gone, then it will fail as expected.

Most of the time it actually won't fail because we wipe such resources during refresh (effectively Read for each resource) which is triggered by default before each plan/apply/destroy command, but it will at least appear in the log as WARN.

[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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.