resource/aws_lb: Enable NLB access logs, remove Computed from access_logs attributes, properly read subnet_mappings

[bflad]

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" comments, they generate extra noise for pull request followers and do not help prioritize the request

Closes #8208
Closes #7397
Closes #2145

Here we remove the previous behavior in the resource of ignoring access_logs configuration if the load_balancer_type is set to network. This enables NLB access log support.

The access_logs configuration block was previously also set to Optional and Computed, which prevents proper updates when removing the configuration block and also requires additional logic within Sentinel policies to handle both the configured and unconfigured scenarios. The configuration block logic has been updated to always set Terraform state information. Updating from enabled = true to no configuration block will disable access logs, which would be expected operator behavior in this scenario.

While acceptance testing this entirety of the resource in preparation for submitting the above changes, it was discovered that an ELBv2 API update sometime around January 25th changed the API response associated with the subnet_mapping configuration block. If a configuration like the following was present, the resource began showing a resource recreation difference.

Example configuration:

resource "aws_lb" "example" {
  # ... other configuration ...

  subnet_mapping {
    subnet_id = "..."
  }
}

Previous output from acceptance testing:

--- FAIL: TestAccAWSLB_networkLoadbalancerBasic (209.90s)
    testing.go:538: Step 0 error: After applying this step, the plan was not empty:

        DIFF:

        DESTROY/CREATE: aws_lb.lb_test
...
          subnet_mapping.#:                        "0" => "1" (forces new resource)
          subnet_mapping.3505487146.allocation_id: "" => ""
          subnet_mapping.3505487146.subnet_id:     "" => "subnet-03ba23f771f369617" (forces new resource)
          subnets.#:                               "1" => "<computed>"
...

The subnet_mapping logic was incorrectly only setting the subnet_id information in the Terraform state if an EIP allocation was present in the API response. This has been fixed.

Output from acceptance testing (the test failure below is present on master but out of scope for these fixes):

--- PASS: TestAccAWSLB_noSecurityGroup (190.29s)
--- PASS: TestAccAWSLB_generatedName (199.02s)
--- PASS: TestAccAWSLB_namePrefix (212.43s)
--- PASS: TestAccAWSLB_ALB_basic (221.72s)
--- FAIL: TestAccAWSLB_updatedSubnets (234.95s)
    testing.go:538: Step 1 error: Check failed: 1 error occurred:
        	* Check 2/3 error: aws_lb.lb_test: Attribute 'subnets.#' expected "3", got "2"

--- PASS: TestAccAWSLB_networkLoadbalancer_subnet_change (248.20s)
--- PASS: TestAccAWSLB_updatedIpAddressType (252.16s)
--- PASS: TestAccAWSLB_NLB_basic (266.47s)
--- PASS: TestAccAWSLB_generatesNameForZeroValue (268.23s)
--- PASS: TestAccAWSLB_tags (271.86s)
--- PASS: TestAccAWSLB_networkLoadbalancerEIP (289.22s)
--- PASS: TestAccAWSLB_updatedSecurityGroups (300.53s)
--- PASS: TestAccAWSLB_applicationLoadBalancer_updateHttp2 (306.13s)
--- PASS: TestAccAWSLB_ALB_AccessLogs_Prefix (311.18s)
--- PASS: TestAccAWSLB_networkLoadbalancer_updateCrossZone (327.15s)
--- PASS: TestAccAWSLB_applicationLoadBalancer_updateDeletionProtection (329.09s)
--- PASS: TestAccAWSLB_NLB_AccessLogs_Prefix (361.17s)
--- PASS: TestAccAWSLB_ALB_AccessLogs (384.53s)
--- PASS: TestAccAWSLB_NLB_AccessLogs (393.48s)

[facundovictor]

The update for subnet_mapping looks good to me 👍

[nywilken]

The updated resource looks good! The documentation for the access_logs attribute should be updated to reflect the change. But this is otherwise good to go 🚀

[bflad]

Good catch, @nywilken! I'll push that update shortly then get this merged.

[nywilken]

This has been released in version 2.8.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

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