resource/aws_api_gateway_rest_api: suppressed plan changes for policy

[sergei-ivanov]

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" comments, they generate extra noise for pull request followers and do not help prioritize the request

Closes #5549

Release note for CHANGELOG:

Suppress spurious plan changes for `policy` attribute of `aws_api_gateway_rest_api`

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAWSAPIGatewayRestApi_*'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./... -v -count 1 -parallel 20 -run=TestAccAWSAPIGatewayRestApi_* -timeout 120m
?   	github.com/terraform-providers/terraform-provider-aws	[no test files]
=== RUN   TestAccAWSAPIGatewayRestApi_basic
=== PAUSE TestAccAWSAPIGatewayRestApi_basic
=== RUN   TestAccAWSAPIGatewayRestApi_EndpointConfiguration
=== PAUSE TestAccAWSAPIGatewayRestApi_EndpointConfiguration
=== RUN   TestAccAWSAPIGatewayRestApi_EndpointConfiguration_Private
=== PAUSE TestAccAWSAPIGatewayRestApi_EndpointConfiguration_Private
=== RUN   TestAccAWSAPIGatewayRestApi_api_key_source
=== PAUSE TestAccAWSAPIGatewayRestApi_api_key_source
=== RUN   TestAccAWSAPIGatewayRestApi_policy
=== PAUSE TestAccAWSAPIGatewayRestApi_policy
=== RUN   TestAccAWSAPIGatewayRestApi_openapi
=== PAUSE TestAccAWSAPIGatewayRestApi_openapi
=== CONT  TestAccAWSAPIGatewayRestApi_basic
=== CONT  TestAccAWSAPIGatewayRestApi_policy
=== CONT  TestAccAWSAPIGatewayRestApi_EndpointConfiguration_Private
=== CONT  TestAccAWSAPIGatewayRestApi_api_key_source
=== CONT  TestAccAWSAPIGatewayRestApi_EndpointConfiguration
=== CONT  TestAccAWSAPIGatewayRestApi_openapi
--- PASS: TestAccAWSAPIGatewayRestApi_openapi (56.80s)
--- PASS: TestAccAWSAPIGatewayRestApi_EndpointConfiguration_Private (90.50s)
--- PASS: TestAccAWSAPIGatewayRestApi_policy (141.95s)
--- PASS: TestAccAWSAPIGatewayRestApi_basic (155.77s)
--- PASS: TestAccAWSAPIGatewayRestApi_api_key_source (252.84s)
--- PASS: TestAccAWSAPIGatewayRestApi_EndpointConfiguration (472.19s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	472.219s
testing: warning: no tests to run
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws/internal/flatmap	0.003s [no tests to run]
testing: warning: no tests to run
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags	0.004s [no tests to run]

[sergei-ivanov]

@bflad can you please take a look at the proposed changes? The PR triage job has died for no good reason, but the unit tests pass OK. Thanks!

[sergei-ivanov]

I have just rebased on the latest master. Can anyone from the team please review the changes?

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAWSAPIGatewayRestApi_*'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./... -v -count 1 -parallel 20 -run=TestAccAWSAPIGatewayRestApi_* -timeout 120m
?   	github.com/terraform-providers/terraform-provider-aws	[no test files]
=== RUN   TestAccAWSAPIGatewayRestApi_basic
=== PAUSE TestAccAWSAPIGatewayRestApi_basic
=== RUN   TestAccAWSAPIGatewayRestApi_tags
=== PAUSE TestAccAWSAPIGatewayRestApi_tags
=== RUN   TestAccAWSAPIGatewayRestApi_disappears
=== PAUSE TestAccAWSAPIGatewayRestApi_disappears
=== RUN   TestAccAWSAPIGatewayRestApi_EndpointConfiguration
=== PAUSE TestAccAWSAPIGatewayRestApi_EndpointConfiguration
=== RUN   TestAccAWSAPIGatewayRestApi_EndpointConfiguration_Private
=== PAUSE TestAccAWSAPIGatewayRestApi_EndpointConfiguration_Private
=== RUN   TestAccAWSAPIGatewayRestApi_api_key_source
=== PAUSE TestAccAWSAPIGatewayRestApi_api_key_source
=== RUN   TestAccAWSAPIGatewayRestApi_policy
=== PAUSE TestAccAWSAPIGatewayRestApi_policy
=== RUN   TestAccAWSAPIGatewayRestApi_openapi
=== PAUSE TestAccAWSAPIGatewayRestApi_openapi
=== CONT  TestAccAWSAPIGatewayRestApi_basic
=== CONT  TestAccAWSAPIGatewayRestApi_api_key_source
=== CONT  TestAccAWSAPIGatewayRestApi_policy
=== CONT  TestAccAWSAPIGatewayRestApi_openapi
=== CONT  TestAccAWSAPIGatewayRestApi_EndpointConfiguration
=== CONT  TestAccAWSAPIGatewayRestApi_EndpointConfiguration_Private
=== CONT  TestAccAWSAPIGatewayRestApi_disappears
=== CONT  TestAccAWSAPIGatewayRestApi_tags
--- PASS: TestAccAWSAPIGatewayRestApi_disappears (53.21s)
--- PASS: TestAccAWSAPIGatewayRestApi_openapi (100.83s)
--- PASS: TestAccAWSAPIGatewayRestApi_api_key_source (128.06s)
--- PASS: TestAccAWSAPIGatewayRestApi_EndpointConfiguration_Private (157.38s)
--- PASS: TestAccAWSAPIGatewayRestApi_EndpointConfiguration (196.09s)
--- PASS: TestAccAWSAPIGatewayRestApi_policy (241.63s)
--- PASS: TestAccAWSAPIGatewayRestApi_tags (272.89s)
--- PASS: TestAccAWSAPIGatewayRestApi_basic (493.52s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	493.566s
testing: warning: no tests to run
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws/internal/flatmap	0.013s [no tests to run]
testing: warning: no tests to run
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags	0.005s [no tests to run]

[bflad]

Hi @sergei-ivanov 👋 Thank you for submitting this and for the proposed implementation.

I have added the breaking-change label to this pull request since it changes the API return value and may cause an unexpected difference for configurations using the full ARN or change in behavior to any references pointing to this attribute's value. We may consider this type of change acceptable in a major version update, however we should try to avoid changing this attribute value if possible. As your comment alludes to in the resource code, suppressing the difference can be implemented via DiffSuppressFunc to allow the two values to be seen as equivalent, which is our typical approach to IAM policy differences like these.

[sergei-ivanov]

Let me bite the bullet and see if I can reimplement it using DiffSuppressFunc. My concern was that DiffSuppressFunc would need to pull the ARN out of somewhere, and I was not sure if it'd be easily available in that context.

[sergei-ivanov]

Okay, it was actually easier than I thought on the provider side (and the changes are much smaller and localised now), and a bit ugly on the acceptance test side, because I had to deal with the ever-changing execution ARNs. Please see the linked PR.

[sergei-ivanov]

Closing in favour of #10986

[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!