Missing preserve_client_ip attribute in aws_lb_target_group resource

[aluuu]

Community Note

• Please vote on this issue by adding a 👍 reaction to the original issue 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 issue followers and do not help prioritize the request
• If you are interested in working on this issue or have submitted a pull request, please leave a comment

Description

AWS documentation states that target group has an attribute preserve_client_ip which is missing from the terraform's aws_lb_target_group definition.

New or Affected Resource(s)

• aws_lb_target_group

Potential Terraform Configuration

resource "aws_lb_target_group" "service_tcp" {
  name                 = local.target_group_tcp_name
  port                 = local.service_tcp_port
  protocol             = "TCP"
  target_type          = "instance"
  vpc_id               = data.aws_vpc.my_infrastructure.id
  deregistration_delay = 120
  preserve_client_ip   = true # <- this attribute is missing at the moment

  health_check {
    enabled             = true
    port                = local.service_tcp_port
    protocol            = "TCP"
    healthy_threshold   = 3
    unhealthy_threshold = 3
  }

  tags = local.default_tags
}

References

• AWS documentation - Network Load Balancers - section 'Target group attributes' mentions preserve_client_ip.enabled attribute
• AWS documentation - Network Load Balancers - section 'Client IP preservation' describes the effect of the attribute

[amitdaga]

@ewbankkit @anGie44 Is anyone working on this issue? Otherwise, I could work on it. I am new to Go, so I would also appreciate it if you could share a PR that I can use for reference. Thank you.

[amitdaga]

@ewbankkit @anGie44 I did some research to review the required changes. Also, I pulled a PR that I could use for reference. Currently, I have sufficient information to start making the changes. Hence, I would request you to assign the ticket to me.

[amitdaga]

@ewbankkit @anGie44 I have raised a draft PR (linked above). Could you please check and suggest if this is the right direction to go?
Change summary: I have introduced a new field in the resource schema. Even though the field value needs to be bool but given the field's complexity, I had to change it to TypeString because the zero-value for bool is false. It would be great if you could give it a quick look so that I could proceed with writing the tests, updating CHANGELOG and documentation.

[kschu91]

@amitdaga let me know if I can support with anything, we are also waiting for this to come.

[amitdaga]

@kschu91 I am mostly done with the changes. One question that I have is whether this provider should validate that this new field value cannot be false for UDP and TCP_UDP protocol.

[kschu91]

@kschu91 I am mostly done with the changes. One question that I have is whether this provider should validate that this new field value cannot be false for UDP and TCP_UDP protocol.

I personally think that it would make sense. I mean the AWS API would break anyways, but failing early in the plan phase would be better.

[amitdaga]

@ewbankkit @anGie44 The linked PR is ready for review. Can you please review and share your inputs?

[amitdaga]

@kschu91 Right that adding validation does help during the plan phase. I tried adding it earlier, but it seemed to add a bit of complexity. The validation logic also did not seem to work since I added it in the resourceAwsLbTargetGroupCustomizeDiff method, and that does not seem to be the right place. I also explored putting the validation in the schema definition, but it did not seem possible due to the validation dependency on another field. Sorry for my limited knowledge here. If you could suggest where I should put the validation check, I could give it another try.

[amitdaga]

@ewbankkit @anGie44 The linked PR is ready for review. Can you please review and share your inputs?

@ewbankkit @anGie44 @YakDriver Can you please review and share your inputs on the PR? Thank you.

[ghost]

This has been released in version 3.35.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!