Fix flipping health_check_path on GA

[aquarhead]

• Remove default value for health_check_path
• Update documentation
• Skip setting the resource data field for TCP GAs

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

Release note for CHANGELOG:

Remove default value of `health_check_path`

Output from acceptance testing:

❯ make testacc TEST=./aws TESTARGS='-run=TestAccAwsGlobalAcceleratorEndpointGroup_tcp'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAwsGlobalAcceleratorEndpointGroup_tcp -timeout 120m
=== RUN   TestAccAwsGlobalAcceleratorEndpointGroup_tcp
=== PAUSE TestAccAwsGlobalAcceleratorEndpointGroup_tcp
=== CONT  TestAccAwsGlobalAcceleratorEndpointGroup_tcp
--- PASS: TestAccAwsGlobalAcceleratorEndpointGroup_tcp (312.50s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	312.561s

❯ make testacc TEST=./aws TESTARGS='-run=TestAccAwsGlobalAcceleratorEndpointGroup'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAwsGlobalAcceleratorEndpointGroup -timeout 120m
go: downloading github.com/aws/aws-sdk-go v1.29.16
=== RUN   TestAccAwsGlobalAcceleratorEndpointGroup_basic
=== PAUSE TestAccAwsGlobalAcceleratorEndpointGroup_basic
=== RUN   TestAccAwsGlobalAcceleratorEndpointGroup_update
=== PAUSE TestAccAwsGlobalAcceleratorEndpointGroup_update
=== RUN   TestAccAwsGlobalAcceleratorEndpointGroup_tcp
=== PAUSE TestAccAwsGlobalAcceleratorEndpointGroup_tcp
=== CONT  TestAccAwsGlobalAcceleratorEndpointGroup_basic
=== CONT  TestAccAwsGlobalAcceleratorEndpointGroup_tcp
=== CONT  TestAccAwsGlobalAcceleratorEndpointGroup_update
--- PASS: TestAccAwsGlobalAcceleratorEndpointGroup_basic (245.99s)
--- PASS: TestAccAwsGlobalAcceleratorEndpointGroup_tcp (281.40s)
--- PASS: TestAccAwsGlobalAcceleratorEndpointGroup_update (307.20s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	307.279s

I'm proposing this change because for TCP endpoint groups there's no health_check_path:

With previous code, terraform plan will attempt to add a default / every time:

But AWS doesn't care about it so it keeps try to add the field forever.

[ewbankkit]

Relates: #9329.
Relates: #12882.

[aquarhead]

Thanks for linking related issues. I wasn't able to find the previous one when I submit my PR. Hopefully there will be some traction now that multiple issues exist.

[ewbankkit]

@aquarhead Yes, we are hoping to make progress on some of the non-Core services.
It may be worth considering bflad's suggestion. I think for HTTP(S) endpoints, removing / as the default health_check_path value will cause any configuration that were assuming that to now show a continual diff.

[aquarhead]

So should Default: "/" be left untouched? And add "Computed: true"? Not sure how the resource update function should behave in that case... Wouldn't it make the statefile think the resource has a health_check_path of / while in reality it does not (for TCP endpoint groups)?

If Computed: true is to replace Default: "/" wouldn't that be the same situation as this one (in regards to causing diff) ? Or does it behave differently for existing resources? (Sorry I'm not familiar with these options)

[ewbankkit]

@aquarhead Remove Default: "/", and add Computed: true. Then if no value is specified by the user in their configuration, whatever value DescribeEndpointGroup returns for health_check_path will not show as a diff.

[aquarhead]

Added, do I need to change other if statement?

[ewbankkit]

@aquarhead Yes, there should be no need for those additional if statements now, but of course use the acceptance tests to verify. Thanks.

[aquarhead]

Just removed all if and GetOkExists changes and also rebased to current master, now there's an error with acceptance test:

    TestAccAwsGlobalAcceleratorEndpointGroup_tcp: testing.go:684: Step 0 error: Check failed: Check 3/8 error: aws_globalaccelerator_endpoint_group.example: Attribute 'health_check_path' found when not expected
--- FAIL: TestAccAwsGlobalAcceleratorEndpointGroup_tcp (207.73s)

Seems GetOk will inject some value by default? Should I leave GetOkExists in place?

Actually all I care is that TF does not consider itself needs to update any values because of health_check_path is changed. Is there any particular technique I can use to specifically check that health_check_path does not trigger an update? (instead of checking values after apply)

[ewbankkit]

@aquarhead After every acceptance test step effectively a terraform plan is run and by default any reported diffs will report as an error, so a successful test will verify that there is no drift.
For the currently failing test it is OK to have the health_check_path attribute set.
Yes GetOk will return the "zero" value, an empty string.

[aquarhead]

@ewbankkit Thanks for that information, according to that I think just running a single TCP endpoint group creation test is enough, I've rebased all the changes and onto (this morning's) master. And acceptance test passes too:

<$> make testacc TEST=./aws TESTARGS='-run=TestAccAwsGlobalAcceleratorEndpointGroup_tcp'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAwsGlobalAcceleratorEndpointGroup_tcp -timeout 120m
=== RUN   TestAccAwsGlobalAcceleratorEndpointGroup_tcp
=== PAUSE TestAccAwsGlobalAcceleratorEndpointGroup_tcp
=== CONT  TestAccAwsGlobalAcceleratorEndpointGroup_tcp
--- PASS: TestAccAwsGlobalAcceleratorEndpointGroup_tcp (244.77s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	244.834s

[ewbankkit]

Verified acceptance tests:

$ make testacc TEST=./aws TESTARGS='-run=TestAccAwsGlobalAcceleratorEndpointGroup_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAwsGlobalAcceleratorEndpointGroup_ -timeout 120m
=== RUN   TestAccAwsGlobalAcceleratorEndpointGroup_basic
=== PAUSE TestAccAwsGlobalAcceleratorEndpointGroup_basic
=== RUN   TestAccAwsGlobalAcceleratorEndpointGroup_update
=== PAUSE TestAccAwsGlobalAcceleratorEndpointGroup_update
=== RUN   TestAccAwsGlobalAcceleratorEndpointGroup_tcp
=== PAUSE TestAccAwsGlobalAcceleratorEndpointGroup_tcp
=== CONT  TestAccAwsGlobalAcceleratorEndpointGroup_basic
=== CONT  TestAccAwsGlobalAcceleratorEndpointGroup_tcp
=== CONT  TestAccAwsGlobalAcceleratorEndpointGroup_update
--- PASS: TestAccAwsGlobalAcceleratorEndpointGroup_basic (215.15s)
--- PASS: TestAccAwsGlobalAcceleratorEndpointGroup_tcp (216.12s)
--- PASS: TestAccAwsGlobalAcceleratorEndpointGroup_update (240.11s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	240.215s

@aquarhead One minor tweak to the documentation and then this looks good. Thanks.

[ewbankkit]

Closes #9329.

[aquarhead]

Do I need to tweak the "Release note for CHANGELOG:" section in the OP ?

[ewbankkit]

CHANGELOG looks OK.

[aquarhead]

@ewbankkit Sorry, in the afternoon I thought I've added the doc change.... Just added now according to our discussion

[ewbankkit]

LGTM.

$ make testacc TEST=./aws TESTARGS='-run=TestAccAwsGlobalAcceleratorEndpointGroup_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAwsGlobalAcceleratorEndpointGroup_ -timeout 120m
=== RUN   TestAccAwsGlobalAcceleratorEndpointGroup_basic
=== PAUSE TestAccAwsGlobalAcceleratorEndpointGroup_basic
=== RUN   TestAccAwsGlobalAcceleratorEndpointGroup_update
=== PAUSE TestAccAwsGlobalAcceleratorEndpointGroup_update
=== RUN   TestAccAwsGlobalAcceleratorEndpointGroup_tcp
=== PAUSE TestAccAwsGlobalAcceleratorEndpointGroup_tcp
=== CONT  TestAccAwsGlobalAcceleratorEndpointGroup_basic
=== CONT  TestAccAwsGlobalAcceleratorEndpointGroup_tcp
=== CONT  TestAccAwsGlobalAcceleratorEndpointGroup_update
--- PASS: TestAccAwsGlobalAcceleratorEndpointGroup_tcp (195.20s)
--- PASS: TestAccAwsGlobalAcceleratorEndpointGroup_basic (196.92s)
--- PASS: TestAccAwsGlobalAcceleratorEndpointGroup_update (231.21s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	231.272s

[bflad]

Please note that the commits from this pull request were included with #14486 and merged for release in version 3.4.0 of the Terraform AWS Provider.

[ghost]

This has been released in version 3.4.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

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