Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[aws_lb_target_group] Validation forces invalid path for TCP health check #2978

Closed
whereisaaron opened this issue Jan 12, 2018 · 4 comments · Fixed by #2980
Closed

[aws_lb_target_group] Validation forces invalid path for TCP health check #2978

whereisaaron opened this issue Jan 12, 2018 · 4 comments · Fixed by #2980
Labels
bug Addresses a defect in current functionality. service/elbv2 Issues and PRs that pertain to the elbv2 service.
Milestone

Comments

@whereisaaron
Copy link
Contributor

It is desirable to be able to configure either a TCP or HTTP/HTTPS health check based on variables as in the below example. Currently the validation forces the health check path to begin with a slash("/"), even when the protocol is TCP, preventing this use case. When the protocol is TCP, blank ("") is the only valid path for the AWS API.

  health_check {
    healthy_threshold   = 2
    unhealthy_threshold = 2
    timeout             = 10
    port                = 8000
    path                = "${var.protocol == "TCP" ? "" : "/healthz"}"
    protocol            = "${var.protocol}"
    interval            = 30
    matcher             = "${var.protocol == "TCP" ? "" : "200-399"}"
  }

Terraform Version

Terraform v0.11.1
+ provider.aws v1.7.0

Affected Resource(s)

  • aws_lb_target_group

Terraform Configuration Files

  health_check {
    healthy_threshold   = 2
    unhealthy_threshold = 2
    timeout             = 10
    port                = 8000
    path                = ""
    protocol            = "TCP"
    interval            = 30
    matcher             = ""
  }

Expected Behavior

Expected to be able to use a blank 'path' for a TCP health check.
Or else the path validation should be left to the AWS API.

Actual Behavior

Error: module.nlb.aws_lb_target_group.nlb[0]: "health_check.0.path" must begin with a '/' character: ""

Steps to Reproduce

  1. Create a aws_lb_target_group with health_check.protocol="TCP" and health_check.path=""
  2. terraform init
  3. terraform apply

References

Validation was recently improved to enable HTTP/HTTPS health checks to be able to be specified for TCP Target Groups in PR #2906.

whereisaaron added a commit to whereisaaron/terraform-provider-aws that referenced this issue Jan 12, 2018
… checks

Fixes hashicorp#2978 "Validation forces invalid path for TCP health check" 

Currently the validation forces the health check path to begin with a slash("/"), even when the protocol is TCP, in which case the health check path must be blank. The validation is preventing valid use cases. 

Since this validation can only check the one field on its own, the check is amended to only validate the path begins with a "/" if the path is not blank. Note that when the path is blank, it is not passed to AWS at all (see lines 226-228), so I think this change does not weaken validation.
@whereisaaron
Copy link
Contributor Author

Looks like there is a fairly trivial fix for this: PR #2980

Only check for "/" if the path is not blank.

@bflad bflad added bug Addresses a defect in current functionality. service/elbv2 Issues and PRs that pertain to the elbv2 service. labels Jan 16, 2018
@bflad bflad added this to the v1.7.1 milestone Jan 16, 2018
@bflad
Copy link
Contributor

bflad commented Jan 16, 2018

This fix is in master and will be released in v1.7.1, which we are planning for later this week.

@bflad
Copy link
Contributor

bflad commented Jan 22, 2018

This has been released in terraform-provider-aws version 1.7.1. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

drewsonne pushed a commit to drewsonne/terraform-provider-aws that referenced this issue Mar 3, 2018
… checks

Fixes hashicorp#2978 "Validation forces invalid path for TCP health check" 

Currently the validation forces the health check path to begin with a slash("/"), even when the protocol is TCP, in which case the health check path must be blank. The validation is preventing valid use cases. 

Since this validation can only check the one field on its own, the check is amended to only validate the path begins with a "/" if the path is not blank. Note that when the path is blank, it is not passed to AWS at all (see lines 226-228), so I think this change does not weaken validation.
@ghost
Copy link

ghost commented Apr 8, 2020

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!

@ghost ghost locked and limited conversation to collaborators Apr 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Addresses a defect in current functionality. service/elbv2 Issues and PRs that pertain to the elbv2 service.
Projects
None yet
2 participants