hashicorp / terraform-provider-aws

The AWS Provider enables Terraform to manage AWS resources.
https://registry.terraform.io/providers/hashicorp/aws
Mozilla Public License 2.0
9.82k stars 9.17k forks source link

AWS ALB Access Logs Dynamic Block Fails Plan Correctly #16674

Closed duganth-va closed 6 months ago

duganth-va commented 3 years ago

We are utilizing a Module with an ALB which exposes the ability to add access_logs based upon a variable. To populate the parameters the lookup function is used on the optional parameters such as enabled and prefix. When the lookup function is used on the enabled parameter the bucket parameter is not populated in the terraform plan.

We have a work around which consists of setting the enabled parameter to true in the module.

Community Note

Terraform CLI and Terraform AWS Provider Version

Terraform v0.13.5

Affected Resource(s)

Terraform Configuration Files

Calling Terraform

variable "region" {}
variable "bucket_name" {}
variable "subnets" {}
variable "vpc_id" {}

provider "aws" {
  region = var.region
}

module "ecs_oauth_proxy_log_bucket" {
  source                         = "terraform-aws-modules/s3-bucket/aws"
  version                        = "1.17.0"
  bucket                         = var.bucket_name
  acl                            = "log-delivery-write"
  force_destroy                  = true
  attach_elb_log_delivery_policy = true
}

module "lb" {
  source  = "./alb/"
  subnets = var.subnets
  vpc_id  = var.vpc_id
  lb_access_logs = [
    {
      enabled = true
      bucket  = module.ecs_oauth_proxy_log_bucket[0].this_s3_bucket_id
    }
  ]
}

Module

variable "lb_access_logs" {}
variable "subnets" {}
variable "vpc_id" {}

resource "aws_lb" "this" {
  name               = "testing"
  internal           = true
  load_balancer_type = "application"
  security_groups    = [aws_security_group.lb.id]
  subnets            = var.subnets
  dynamic "access_logs" {
    for_each = var.lb_access_logs
    content {
      bucket  = access_logs.value.bucket
      enabled = lookup(access_logs.value, "enabled", null)
      prefix  = lookup(access_logs.value, "prefix", null)
    }
  }
}

resource "aws_security_group" "lb" {
  name   = "test-lb-sg"
  vpc_id = var.vpc_id

  egress {
    protocol    = "-1"
    from_port   = 0
    to_port     = 0
    cidr_blocks = ["0.0.0.0/0"]
  }
}

Debug Output

https://gist.github.com/duganth-va/586335f45db17f46a48059a4d4f186b6

Expected Behavior

  # module.lb.aws_lb.this 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                   = true
      + ip_address_type            = (known after apply)
      + load_balancer_type         = "application"
      + name                       = "testing"
      + security_groups            = (known after apply)
      + subnets                    = [
          + "subnet-xxx",
          + "subnet-yyy",
        ]
      + vpc_id                     = (known after apply)
      + zone_id                    = (known after apply)

      + access_logs {
          + bucket  = (known after apply)
          + enabled = true
          + prefix  = (known after apply)
        }

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

Actual Behavior

Running terraform plan results in the following output

  # module.lb.aws_lb.this 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                   = true
      + ip_address_type            = (known after apply)
      + load_balancer_type         = "application"
      + name                       = "testing"
      + security_groups            = (known after apply)
      + subnets                    = [
          + "subnet-xxx",
          + "subnet-yyy",
        ]
      + vpc_id                     = (known after apply)
      + zone_id                    = (known after apply)

      + access_logs {
          + enabled = (known after apply)
          + prefix  = (known after apply)
        }

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

Running Terraform apply results in:

Error: Provider produced inconsistent final plan

When expanding the plan for module.lb.aws_lb.this to include new values learned so far during apply, provider "registry.terraform.io/hashicorp/aws" produced an invalid new value for .access_logs[0].bucket: was cty.StringVal(""), but now cty.StringVal("td-is-awesome-at-s3-bucket-names").

This is a bug in the provider, which should be reported in the provider's own issue tracker.

Steps to Reproduce

  1. terraform plan
  2. terraform apply

Important Factoid

This was tested on Gov Cloud.

seanturner026 commented 3 years ago

I've encountered the exact same issue also while writing an ELB module. Only encountered the problem when Terraform is creating the bucket in the same apply as the ALB / NLB. No problems if the bucket exists already.

vojtapol commented 3 years ago

Same exact issue as @seanturner026. I can also confirm that it worked on the second run.

Any workaround for this?

SpComb commented 3 years ago

At least here on terraform 0.12.31 this happens even without the use of a dynamic block for the access_logs.

Both with the (broken) simplified conditional form:

resource "aws_lb" "alb" {
  ...

  access_logs {
    enabled = var.access_logs_bucket != null ? true : false
    bucket  = var.access_logs_bucket
    prefix  = var.name
  }
}

As well as with the dynamic form as a workaround for the related but different aws_lb access_logs issue: https://github.com/hashicorp/terraform-provider-aws/issues/2072#issuecomment-873887700

  dynamic "access_logs" {
    for_each = var.access_logs_bucket != null ? { enabled = true } : {}

    content {
      enabled = true
      bucket  = var.access_logs_bucket
      prefix  = var.name
    }
  }

The terraform plan is only seeing the access_logs { enabled => ... } parameter in the terraform plan - both the bucket and prefix are missing:

Terraform will perform the following actions:

  # module.aws_alb.aws_lb.alb will be updated in-place
  ~ resource "aws_lb" "alb" {
        ...
      ~ access_logs {
          ~ enabled = false -> (known after apply)
        }
    }

And the terraform apply produces errors for both access_logs attributes which were missing from the plan:

Error: Provider produced inconsistent final plan

When expanding the plan for module.aws_alb.aws_lb.alb to include new values
learned so far during apply, provider "registry.terraform.io/-/aws" produced
an invalid new value for .access_logs[0].bucket: was cty.StringVal(""), but
now cty.StringVal("aws-harri-test.alb-access-logs").

This is a bug in the provider, which should be reported in the provider's own
issue tracker.

Error: Provider produced inconsistent final plan

When expanding the plan for module.aws_alb.aws_lb.alb to include new values
learned so far during apply, provider "registry.terraform.io/-/aws" produced
an invalid new value for .access_logs[0].prefix: was cty.StringVal(""), but
now cty.StringVal("aws-harri-test").

This is a bug in the provider, which should be reported in the provider's own
issue tracker.

As in the comments above, this only seems to happens when setting the bucket from the id attribute of an aws_s3_bucket resource that is being created in the same terraform apply. The second run where the referenced aws_s3_bucket already exists during the plan stage is planned and applied correctly.

kumartushar commented 2 years ago

As mentioned in issue description, the below workaround do the job:

 module 'aws_lb' {
     ...
     access_logs = {
          enabled = true
          prefix  = "my-prefix"
          bucket  = module.s3_logs_bucket.s3_bucket_id
      }
      depends_on = [module.s3_logs_bucket]
  }
}
3bbbeau commented 1 year ago

I experienced this issue using the terraform-aws-alb module.

enabled within the access_logs block is interpreted based on the value not yet known before apply.

The fix was to explicitly set enabled to true as follows:

HCL Code Snippet ```hcl access_logs = { enabled = true # Explicitly set this to true bucket = module.s3_bucket.s3_bucket_id } ```
jar-b commented 6 months ago

I've attempted to reproduce this behavior with a configuration similar to that described in the original issue, and believe this has been resolved by upstream changes to Terraform core.

To allow time for feedback on the reproduction, we're going to leave this issue open for two weeks (until 2024-04-30). If no ongoing issues with the AWS provider are identified in this time, we'll close this issue as fixed upstream with https://github.com/hashicorp/terraform/pull/28424.

Background

A reproduction is included below. Because the Terraform CLI and s3-bucket module versions from the original report are now significantly outdated, I've instead used the latest available versions of each. This is version 1.8.0 of Terraform and 4.1.2 of the s3-bucket module.

Configuration

This section contains the configuration used for this reproduction.

Show/Hide Configuration `main.tf`: ```terraform terraform { required_providers { aws = { source = "hashicorp/aws" version = "~> 5.0" } } } # Issue Ref: https://github.com/hashicorp/terraform-provider-aws/issues/16674 # # No defaults in provided issue config, but we'll set them here. variable "region" { default = "us-west-2" } variable "bucket_name" { default = "jb-test-lb-inconsistent-final-plan" } provider "aws" { region = var.region } data "aws_availability_zones" "available" { exclude_zone_ids = ["usw2-az4"] state = "available" filter { name = "opt-in-status" values = ["opt-in-not-required"] } } # We'll provision a VPC and subnets in the main configuration rather than using variables # so this reproduction is self-contained. resource "aws_vpc" "test" { cidr_block = "10.1.0.0/16" tags = { Name = "jb-test" } } resource "aws_subnet" "test" { count = 2 vpc_id = aws_vpc.test.id availability_zone = data.aws_availability_zones.available.names[count.index] cidr_block = cidrsubnet(aws_vpc.test.cidr_block, 8, count.index) tags = { Name = "jb-test" } } module "ecs_oauth_proxy_log_bucket" { source = "terraform-aws-modules/s3-bucket/aws" # 1.17.0 uses syntax that is no longer supported in 1.X versions of Terraform. # For this reproduction, use the latest module version instead. # version = "1.17.0" version = "4.1.2" bucket = var.bucket_name acl = "log-delivery-write" force_destroy = true attach_elb_log_delivery_policy = true # Added - now required due to changes to default S3 permissions control_object_ownership = true object_ownership = "ObjectWriter" } module "lb" { source = "./alb/" subnets = aws_subnet.test[*].id vpc_id = aws_vpc.test.id lb_access_logs = [ { enabled = true bucket = module.ecs_oauth_proxy_log_bucket.s3_bucket_id } ] } ``` `alb/main.tf`: ```terraform variable "lb_access_logs" {} variable "subnets" {} variable "vpc_id" {} resource "aws_lb" "this" { name = "testing" internal = true load_balancer_type = "application" security_groups = [aws_security_group.lb.id] subnets = var.subnets dynamic "access_logs" { for_each = var.lb_access_logs content { bucket = access_logs.value.bucket enabled = lookup(access_logs.value, "enabled", null) prefix = lookup(access_logs.value, "prefix", null) } } } resource "aws_security_group" "lb" { name = "test-lb-sg" vpc_id = var.vpc_id egress { protocol = "-1" from_port = 0 to_port = 0 cidr_blocks = ["0.0.0.0/0"] } } ```

Reproduction

This section contains the reproduction steps and relevant output.

Show/Hide Reproduction ```console % terraform -version Terraform v1.8.0 on darwin_arm64 + provider registry.terraform.io/hashicorp/aws v5.45.0 ``` ```console % terraform plan + resource "aws_lb" "this" { + arn = (known after apply) + arn_suffix = (known after apply) + desync_mitigation_mode = "defensive" + dns_name = (known after apply) + drop_invalid_header_fields = false + enable_deletion_protection = false + enable_http2 = true + enable_tls_version_and_cipher_suite_headers = false + enable_waf_fail_open = false + enable_xff_client_port = false + enforce_security_group_inbound_rules_on_private_link_traffic = (known after apply) + id = (known after apply) + idle_timeout = 60 + internal = true + ip_address_type = (known after apply) + load_balancer_type = "application" + name = "testing" + name_prefix = (known after apply) + preserve_host_header = false + security_groups = (known after apply) + subnets = (known after apply) + tags_all = (known after apply) + vpc_id = (known after apply) + xff_header_processing_mode = "append" + zone_id = (known after apply) + access_logs { + bucket = (known after apply) + enabled = true } } Plan: 8 to add, 0 to change, 0 to destroy. ``` From the output above, the dynamic `access_logs` block is correctly planning the `enabled` argument with a known value. `terraform apply` completes without error. ```console % terraform apply -auto-approve Plan: 10 to add, 0 to change, 0 to destroy. aws_vpc.test: Creating... module.ecs_oauth_proxy_log_bucket.aws_s3_bucket.this[0]: Creating... aws_vpc.test: Creation complete after 2s [id=vpc-0d0ccee32e65c195c] aws_subnet.test[0]: Creating... aws_subnet.test[1]: Creating... module.lb.aws_security_group.lb: Creating... module.ecs_oauth_proxy_log_bucket.aws_s3_bucket.this[0]: Creation complete after 2s [id=jb-test-lb-inconsistent-final-plan] module.ecs_oauth_proxy_log_bucket.data.aws_iam_policy_document.elb_log_delivery[0]: Reading... module.ecs_oauth_proxy_log_bucket.aws_s3_bucket_public_access_block.this[0]: Creating... module.ecs_oauth_proxy_log_bucket.data.aws_iam_policy_document.elb_log_delivery[0]: Read complete after 0s [id=521196102] module.ecs_oauth_proxy_log_bucket.data.aws_iam_policy_document.combined[0]: Reading... module.ecs_oauth_proxy_log_bucket.data.aws_iam_policy_document.combined[0]: Read complete after 0s [id=521196102] module.ecs_oauth_proxy_log_bucket.aws_s3_bucket_public_access_block.this[0]: Creation complete after 1s [id=jb-test-lb-inconsistent-final-plan] module.ecs_oauth_proxy_log_bucket.aws_s3_bucket_policy.this[0]: Creating... aws_subnet.test[0]: Creation complete after 1s [id=subnet-0018f63c11fabd858] aws_subnet.test[1]: Creation complete after 1s [id=subnet-01c1172e47a5c05c2] module.ecs_oauth_proxy_log_bucket.aws_s3_bucket_policy.this[0]: Creation complete after 0s [id=jb-test-lb-inconsistent-final-plan] module.ecs_oauth_proxy_log_bucket.aws_s3_bucket_ownership_controls.this[0]: Creating... module.ecs_oauth_proxy_log_bucket.aws_s3_bucket_ownership_controls.this[0]: Creation complete after 1s [id=jb-test-lb-inconsistent-final-plan] module.ecs_oauth_proxy_log_bucket.aws_s3_bucket_acl.this[0]: Creating... module.ecs_oauth_proxy_log_bucket.aws_s3_bucket_acl.this[0]: Creation complete after 0s [id=jb-test-lb-inconsistent-final-plan,log-delivery-write] module.lb.aws_security_group.lb: Creation complete after 2s [id=sg-06fd2a2b78e0c6df0] module.lb.aws_lb.this: Creating... module.lb.aws_lb.this: Still creating... [10s elapsed] module.lb.aws_lb.this: Still creating... [20s elapsed] module.lb.aws_lb.this: Still creating... [30s elapsed] module.lb.aws_lb.this: Still creating... [40s elapsed] module.lb.aws_lb.this: Still creating... [50s elapsed] module.lb.aws_lb.this: Still creating... [1m0s elapsed] module.lb.aws_lb.this: Still creating... [1m10s elapsed] module.lb.aws_lb.this: Still creating... [1m20s elapsed] module.lb.aws_lb.this: Still creating... [1m30s elapsed] module.lb.aws_lb.this: Still creating... [1m40s elapsed] module.lb.aws_lb.this: Still creating... [1m50s elapsed] module.lb.aws_lb.this: Still creating... [2m0s elapsed] module.lb.aws_lb.this: Still creating... [2m10s elapsed] module.lb.aws_lb.this: Still creating... [2m20s elapsed] module.lb.aws_lb.this: Still creating... [2m30s elapsed] module.lb.aws_lb.this: Still creating... [2m40s elapsed] module.lb.aws_lb.this: Creation complete after 2m44s [id=arn:aws:elasticloadbalancing:us-west-2:727561393803:loadbalancer/app/testing/0f33304e5eae06ce] Apply complete! Resources: 10 added, 0 changed, 0 destroyed. ```

Conclusion

Absent any concerns with this reproduction, we'll close this issue as fixed upstream on 2024-04-30.

jar-b commented 6 months ago

Closing as fixed upstream in Terraform core.

github-actions[bot] commented 6 months ago

[!WARNING] This issue has been closed, meaning that any additional comments are hard for our team to see. Please assume that the maintainers will not see them.

Ongoing conversations amongst community members are welcome, however, the issue will be locked after 30 days. Moving conversations to another venue, such as the AWS Provider forum, is recommended. If you have additional concerns, please open a new issue, referencing this one where needed.

github-actions[bot] commented 5 months ago

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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.