Add client_ip_preservation_enabled to global accelerator

[zsiddique]

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 #9940

Release note for CHANGELOG:

* Add client_ip_preservation_enabled to Global Accelerator Endpoint Group.  Note it must be used with EC2 or ALB.

Output from acceptance testing:
Ok, a little bit about acceptance testing. The current acceptance testing for the global accelerator endpoint only checks if the endpoint configuration exists but not the values. I expanded the test to be more detailed but hit a major roadblock. It seems to check client ip I needed to setup a VPC, ALBs and etc. No problem, but, if its set to true it seems tearing down the VPC causes AWS to throw error Error: errors during apply: Error deleting VPC: DependencyViolation: The vpc 'vpc-XXXX' has dependencies and cannot be deleted. I cant for the life of me figure out why or how does it tie back to my changes. Some googling seems to show in general VPC deletion can be flaky and its possible AWS has a race of some kind as deleting the VPC from AWSs UI works fine. Open to ideas on how to fix this, or is it fine testing that setting the value (in this case false) is fine.

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

[jakauppila]

Is there anything holding up this PR?

[ewbankkit]

@zsiddique Thanks for the contribution and apologies for the delay in getting to review.

• Yes it does seem cheating to test the false case, so please could you add a step that tests the true case (and so will also test updating the flag)
• If you start with true and update to false that may help with the lingering ENI issue
• Could you update the documentation in website/docs/r/globalaccelerator_endpoint_group.html.markdown?

If you don't have cycles to make the changes please reply and I can take over your commits and enhance.

[ewbankkit]

Please rebase or merge to the current HEAD and change to

data "aws_availability_zones" "available" {
  state = "available"

  filter {
    name   = "opt-in-status"
    values = ["opt-in-not-required"]
  }
}

otherwise the acceptance tests may fail with attempting to use any enabled Local Zone in us-west-2.

[ewbankkit]

I think we need to add Computed: true, here as the flag is implicitly enabled for internal ALB or instances: https://docs.aws.amazon.com/global-accelerator/latest/dg/preserve-client-ip-address.how-to-enable-preservation.html.

[zsiddique]

@ewbankkit your welcome to try and make those improvements, i will make best effort as well but if you have more freecycle you are welcome to make the fixes.

[ewbankkit]

@zsiddique Thanks for responding. I will include your commits in any PR that replaces this one so that you receive attribution.

[ewbankkit]

@zsiddique I have pulled over your commits into #14486 and am closing this PR. Thanks for your work on this.
The VPC deletion DependencyViolation error is caused by the security group that the Global Accelerator service adds into the VPC but doesn't delete: https://docs.aws.amazon.com/global-accelerator/latest/dg/best-practices-aga.html.
I modified the acceptance tests to explictly delete this security group before the VPC is deleted.

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