`api_gateway_rest_api`: Adds parameter fail_on_warnings [rebased] #22300

fullammo · 2021-12-21T14:40:22Z

Community Note

Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Closes #19154.
Closes #22674.
Closes #18397.

This PR is an up to date version of PR#18397, which complies to the mentioned refactoring changes that had to be addressed.

Output from acceptance testing:

% make testacc TESTS=TestAccAPIGatewayRestApi_Fail_On_Warnings PKG=apigateway
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/apigateway/... -v -count 1 -parallel 20 -run='TestAccAPIGatewayRestApi_Fail_On_Warnings' -timeout 180m
=== RUN   TestAccAPIGatewayRestApi_Fail_On_Warnings
=== PAUSE TestAccAPIGatewayRestApi_Fail_On_Warnings
=== CONT  TestAccAPIGatewayRestApi_Fail_On_Warnings
--- PASS: TestAccAPIGatewayRestApi_Fail_On_Warnings (58.65s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/apigateway 61.240s

@johanwiren This was the easiest way to resolve your merge conflicts, I hope it's not a problem. 😄

github-actions

Welcome @fullammo 👋

It looks like this is your first Pull Request submission to the Terraform AWS Provider! If you haven’t already done so please make sure you have checked out our CONTRIBUTING guide and FAQ to make sure your contribution is adhering to best practice and has all the necessary elements in place for a successful approval.

Also take a look at our FAQ which details how we prioritize Pull Requests for inclusion.

Thanks again, and welcome to the community! 😃

AdamTylerLynch

~~LGTM 🚀~~

After closer review, changes will be needed.

fullammo · 2022-07-19T05:51:13Z

Awesome! Could this change be included in one of the upcoming releases? Thanks in advance!

fullammo · 2022-08-03T13:29:41Z

There were merge conflicts, i resolved them by rebasing the codebase to the latest main, just to make the merge a bit smoother 🥇

michael-rivera-florencehc-com · 2023-01-16T19:41:14Z

Any updates on this? We can use this as well, and it would help a bunch?

thxmike · 2023-01-16T19:43:11Z

This would be a good feature to implement to prevent any deployment issues. Please merge this in

DaniBencz · 2023-01-17T07:49:06Z

It would be an improvement to the current behavior that 'swallows' warnings. Our team could benefit from it.

njovanovic-fhc · 2023-01-17T10:04:51Z

Hey team, any update on this? Our team can utilise this as well...

somaSzolyak · 2023-01-17T13:05:06Z

Hello!

This feature would help our team as well!

AdamTylerLynch

AWS's API requires first creating a REST API and then updating that API if an OpenAPI spec is supplied.

Terraform's implementation of this resource is aligned to that intended use, where Terraform first creates a REST API (without an OpenAPI definition) and then if a OpenAPI definition is supplied immediately updates the API using the OpenAPI definition.

A failOnWarning during create would most likely result in an empty REST API, as the rollback would rollback the OpenAPI application step.

A failOnWarning during update would result in the expected behavior, but would leave the Terraform state in an inconsistent state. My hypothesis is the author saw this inconsistent state when testing, and this is why they removed the computed field from create_date.

Implementation of this failOnWarning feature introduces to many permutations of the potential resource and state due to the AWS APIs requirements, and this would require significant more AccTests to provide surety on permutation coverage.

internal/service/apigateway/rest_api.go

AdamTylerLynch · 2023-01-23T14:05:36Z

internal/service/apigateway/rest_api.go

@@ -55,7 +55,10 @@ func ResourceRestAPI() *schema.Resource {
 			},
 			"created_date": {
 				Type:     schema.TypeString,
-				Computed: true,
+
+			"fail_on_warnings": {


It's important to note that the failOnWarnings parameter only applies to AWS's PutRestApi() method when using an OpenAPI definition. This is an update of an existing API only.

AWS's API requires first creating a REST API and then updating that API if an OpenAPI spec is supplied.

Terraform's implementation of this resource is aligned to that intended use, where Terraform first creates a REST API (without an OpenAPI definition) and then if a OpenAPI definition is supplied immediately updates the API using the OpenAPI definition.

AdamTylerLynch · 2023-01-23T14:08:41Z

website/docs/r/api_gateway_rest_api.html.markdown

@@ -229,6 +229,7 @@ The following arguments are supported:
 * `endpoint_configuration` - (Optional) Configuration block defining API endpoint configuration including endpoint type. Defined below.
 * `minimum_compression_size` - (Optional) Minimum response size to compress for the REST API. Integer between `-1` and `10485760` (10MB). Setting a value greater than `-1` will enable compression, `-1` disables compression (default). If importing an OpenAPI specification via the `body` argument, this corresponds to the [`x-amazon-apigateway-minimum-compression-size` extension](https://docs.aws.amazon.com/apigateway/latest/developerguide/api-gateway-openapi-minimum-compression-size.html). If the argument value (_except_ `-1`) is provided and is different than the OpenAPI value, the argument value will override the OpenAPI value.
 * `name` - (Required) Name of the REST API. If importing an OpenAPI specification via the `body` argument, this corresponds to the `info.title` field. If the argument value is different than the OpenAPI value, the argument value will override the OpenAPI value.
+* `fail_on_warnings` - (Optional) Whether warnings while API Gateway is creating or updating the resource should return an error or not. Defaults to `false`


In our documentation, arguments are listed in alphabetical order. This argument should come after endpoint_configuration and before minimum_compression_size.

website/docs/r/api_gateway_rest_api.html.markdown

Co-authored-by: Tyler Lynch <[email protected]>

github-actions · 2023-05-11T02:10:00Z

I'm going to lock this pull request 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 related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

fullammo mentioned this pull request Dec 21, 2021

api_gateway_rest_api: Adds parameter fail_on_warnings #18397

Closed

github-actions bot reviewed Dec 21, 2021

View reviewed changes

justinretzolk added enhancement Requests to existing resources that expand the functionality or scope. and removed needs-triage Waiting for first response or review from a maintainer. labels Jan 13, 2022

AdamTylerLynch self-assigned this Jul 17, 2022

AdamTylerLynch approved these changes Jul 17, 2022

View reviewed changes

fullammo added 4 commits August 3, 2022 15:19

Adds parameter fail_on_warnings

6661d1b

Adds documentation for fail_on_warnings

e1eed16

Adds acceptance test for fail_on_warnings

2404c5b

Added Changelog entry

f68b6b9

AdamTylerLynch reviewed Jan 23, 2023

View reviewed changes

Update website/docs/r/api_gateway_rest_api.html.markdown

6c862d2

Co-authored-by: Tyler Lynch <[email protected]>

ewbankkit mentioned this pull request Apr 10, 2023

17661: cache_cluster_size should be updated only if cache_cluster_enabled is true #30588

Merged

ewbankkit closed this in #30588 Apr 10, 2023

github-actions bot locked as resolved and limited conversation to collaborators May 11, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`api_gateway_rest_api`: Adds parameter fail_on_warnings [rebased] #22300

`api_gateway_rest_api`: Adds parameter fail_on_warnings [rebased] #22300

fullammo commented Dec 21, 2021 •

edited by ewbankkit

Loading

github-actions bot left a comment

AdamTylerLynch left a comment •

edited

Loading

fullammo commented Jul 19, 2022

fullammo commented Aug 3, 2022 •

edited

Loading

michael-rivera-florencehc-com commented Jan 16, 2023

thxmike commented Jan 16, 2023

DaniBencz commented Jan 17, 2023

njovanovic-fhc commented Jan 17, 2023

somaSzolyak commented Jan 17, 2023

AdamTylerLynch left a comment

AdamTylerLynch Jan 23, 2023 •

edited

Loading

AdamTylerLynch Jan 23, 2023

github-actions bot commented May 11, 2023

api_gateway_rest_api: Adds parameter fail_on_warnings [rebased] #22300

api_gateway_rest_api: Adds parameter fail_on_warnings [rebased] #22300

Conversation

fullammo commented Dec 21, 2021 • edited by ewbankkit Loading

Community Note

github-actions bot left a comment

Choose a reason for hiding this comment

AdamTylerLynch left a comment • edited Loading

Choose a reason for hiding this comment

fullammo commented Jul 19, 2022

fullammo commented Aug 3, 2022 • edited Loading

michael-rivera-florencehc-com commented Jan 16, 2023

thxmike commented Jan 16, 2023

DaniBencz commented Jan 17, 2023

njovanovic-fhc commented Jan 17, 2023

somaSzolyak commented Jan 17, 2023

AdamTylerLynch left a comment

Choose a reason for hiding this comment

AdamTylerLynch Jan 23, 2023 • edited Loading

Choose a reason for hiding this comment

AdamTylerLynch Jan 23, 2023

Choose a reason for hiding this comment

github-actions bot commented May 11, 2023

`api_gateway_rest_api`: Adds parameter fail_on_warnings [rebased] #22300

`api_gateway_rest_api`: Adds parameter fail_on_warnings [rebased] #22300

fullammo commented Dec 21, 2021 •

edited by ewbankkit

Loading

AdamTylerLynch left a comment •

edited

Loading

fullammo commented Aug 3, 2022 •

edited

Loading

AdamTylerLynch Jan 23, 2023 •

edited

Loading