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

fix: Changed default values for lambda_multi_value_headers_enabled and pro… #160

Merged
merged 1 commit into from
May 11, 2020

Conversation

nahuel242
Copy link
Contributor

@nahuel242 nahuel242 commented May 10, 2020

Description

Changed the default values for lambda_multi_value_headers_enabled and proxy_protocol_v2 from "null" to "default" in the "aws_lb_target_group" "main" resource

Motivation and Context

As mentioned by @antonbabenko in issue #156 , lambda_multi_value_headers_enabled and proxy_protocol_v2 are booleans so their default value should be also a boolean and not null.
This is also stated in the provider documentation:
https://www.terraform.io/docs/providers/aws/r/lb_target_group.html#proxy_protocol_v2
https://www.terraform.io/docs/providers/aws/r/lb_target_group.html#lambda_multi_value_headers_enabled

It's related to issue #156

Breaking Changes

No

How Has This Been Tested?

I've written a small test without specifying values for lambda_multi_value_headers_enabled and proxy_protocol_v2 and used as module source my forked branch with the fix

nahuelbatista@SSFF-00170 terraform-test % terraform plan -out=issue156.plan
Refreshing Terraform state in-memory prior to plan...
The refreshed state will be used to calculate this plan, but will not be
persisted to local or remote state storage.


------------------------------------------------------------------------

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  # module.alb.aws_lb.this[0] will be created
  + resource "aws_lb" "this" {
      + arn                        = (known after apply)
      + arn_suffix                 = (known after apply)
      + dns_name                   = (known after apply)
      + drop_invalid_header_fields = false
      + enable_deletion_protection = false
      + enable_http2               = true
      + id                         = (known after apply)
      + idle_timeout               = 60
      + internal                   = false
      + ip_address_type            = "ipv4"
      + load_balancer_type         = "application"
      + name                       = "my-alb"
      + security_groups            = [
          + "sg-033c46aea87a3d025",
        ]
      + subnets                    = [
          + "subnet-0b51932bf0e7664fa",
          + "subnet-0e22184d41bda1e94",
        ]
      + tags                       = {
          + "Environment" = "Test"
          + "Name"        = "my-alb"
        }
      + vpc_id                     = (known after apply)
      + zone_id                    = (known after apply)

      + subnet_mapping {
          + allocation_id = (known after apply)
          + subnet_id     = (known after apply)
        }

      + timeouts {
          + create = "10m"
          + delete = "10m"
          + update = "10m"
        }
    }

  # module.alb.aws_lb_listener.frontend_http_tcp[0] will be created
  + resource "aws_lb_listener" "frontend_http_tcp" {
      + arn               = (known after apply)
      + id                = (known after apply)
      + load_balancer_arn = (known after apply)
      + port              = 80
      + protocol          = "HTTP"
      + ssl_policy        = (known after apply)

      + default_action {
          + order            = (known after apply)
          + target_group_arn = (known after apply)
          + type             = "forward"
        }
    }

  # module.alb.aws_lb_target_group.main[0] will be created
  + resource "aws_lb_target_group" "main" {
      + arn                                = (known after apply)
      + arn_suffix                         = (known after apply)
      + deregistration_delay               = 300
      + id                                 = (known after apply)
      + lambda_multi_value_headers_enabled = false
      + load_balancing_algorithm_type      = (known after apply)
      + name                               = (known after apply)
      + name_prefix                        = "test"
      + port                               = 80
      + protocol                           = "HTTP"
      + proxy_protocol_v2                  = false
      + slow_start                         = 0
      + tags                               = {
          + "Environment" = "Test"
          + "Name"        = "test"
        }
      + target_type                        = "instance"
      + vpc_id                             = "vpc-0c35f33459bcaf9ae"

      + health_check {
          + enabled             = (known after apply)
          + healthy_threshold   = (known after apply)
          + interval            = (known after apply)
          + matcher             = (known after apply)
          + path                = (known after apply)
          + port                = (known after apply)
          + protocol            = (known after apply)
          + timeout             = (known after apply)
          + unhealthy_threshold = (known after apply)
        }

      + stickiness {
          + cookie_duration = (known after apply)
          + enabled         = (known after apply)
          + type            = (known after apply)
        }
    }

Plan: 3 to add, 0 to change, 0 to destroy.

------------------------------------------------------------------------

This plan was saved to: issue156.plan

To perform exactly these actions, run the following command to apply:
    terraform apply "issue156.plan"

@nahuel242 nahuel242 changed the title Changed default values for lambda_multi_value_headers_enabled and pro… fix: Changed default values for lambda_multi_value_headers_enabled and pro… May 10, 2020
@antonbabenko antonbabenko merged commit 8eef881 into terraform-aws-modules:master May 11, 2020
@antonbabenko
Copy link
Member

Thanks, @nahuel242 !

v5.5.0 has been just released.

@nahuel242 nahuel242 deleted the fix/issue156 branch May 11, 2020 08:46
paikend pushed a commit to paikend/terraform-provider-aws that referenced this pull request May 31, 2021
- hashicorp#19590 Issue solved

Description
[This PR](terraform-aws-modules/terraform-aws-alb#160) is merged. But not apply in terraform docs in site
lambda_multi_value_headers_enabled is changed from null to false
The changes were not reflected in the document.
paikend pushed a commit to paikend/terraform-provider-aws that referenced this pull request Jun 1, 2021
- hashicorp#19602 Issue solved

Description
[This PR](terraform-aws-modules/terraform-aws-alb#160) is merged. But not apply in terraform docs in site
default valud of proxy_protocol_v2 is changed from null to false
The changes were not reflected in the document.
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants