#38917 #39197 Fixing the expand function and the remove functi…

[lorodoes]

Description

#38917 #39197 Fix bug in Network Firewall configuration. Only two entries would ever work due to a missed count check in the expandconfiguration function.

Relations

Closes #39197.
Closes #38917.
Relates #38824.

References

https://docs.aws.amazon.com/network-firewall/latest/developerguide/tls-inspection-logging.html

Output from Acceptance Testing

% go test ./internal/service/networkfirewall/... -v -count 1 -parallel 20 -run='TestAccNetworkFirewallLoggingConfiguration_'  -timeout 360m
2024/11/11 22:07:50 Initializing Terraform AWS Provider...
=== RUN   TestAccNetworkFirewallLoggingConfiguration_CloudWatchLogDestination_logGroup
=== PAUSE TestAccNetworkFirewallLoggingConfiguration_CloudWatchLogDestination_logGroup
=== RUN   TestAccNetworkFirewallLoggingConfiguration_CloudWatchLogDestination_logType
=== PAUSE TestAccNetworkFirewallLoggingConfiguration_CloudWatchLogDestination_logType
=== RUN   TestAccNetworkFirewallLoggingConfiguration_KinesisLogDestination_deliveryStream
=== PAUSE TestAccNetworkFirewallLoggingConfiguration_KinesisLogDestination_deliveryStream
=== RUN   TestAccNetworkFirewallLoggingConfiguration_KinesisLogDestination_logType
=== PAUSE TestAccNetworkFirewallLoggingConfiguration_KinesisLogDestination_logType
=== RUN   TestAccNetworkFirewallLoggingConfiguration_S3LogDestination_bucketName
=== PAUSE TestAccNetworkFirewallLoggingConfiguration_S3LogDestination_bucketName
=== RUN   TestAccNetworkFirewallLoggingConfiguration_S3LogDestination_logType
=== PAUSE TestAccNetworkFirewallLoggingConfiguration_S3LogDestination_logType
=== RUN   TestAccNetworkFirewallLoggingConfiguration_S3LogDestination_prefix
=== PAUSE TestAccNetworkFirewallLoggingConfiguration_S3LogDestination_prefix
=== RUN   TestAccNetworkFirewallLoggingConfiguration_updateFirewallARN
=== PAUSE TestAccNetworkFirewallLoggingConfiguration_updateFirewallARN
=== RUN   TestAccNetworkFirewallLoggingConfiguration_updateLogDestinationType
=== PAUSE TestAccNetworkFirewallLoggingConfiguration_updateLogDestinationType
=== RUN   TestAccNetworkFirewallLoggingConfiguration_updateToMultipleLogDestinations
=== PAUSE TestAccNetworkFirewallLoggingConfiguration_updateToMultipleLogDestinations
=== RUN   TestAccNetworkFirewallLoggingConfiguration_updateToSingleAlertTypeLogDestination
=== PAUSE TestAccNetworkFirewallLoggingConfiguration_updateToSingleAlertTypeLogDestination
=== RUN   TestAccNetworkFirewallLoggingConfiguration_updateToSingleFlowTypeLogDestination
=== PAUSE TestAccNetworkFirewallLoggingConfiguration_updateToSingleFlowTypeLogDestination
=== RUN   TestAccNetworkFirewallLoggingConfiguration_updateToSingleTLSTypeLogDestination
=== PAUSE TestAccNetworkFirewallLoggingConfiguration_updateToSingleTLSTypeLogDestination
=== RUN   TestAccNetworkFirewallLoggingConfiguration_disappears
=== PAUSE TestAccNetworkFirewallLoggingConfiguration_disappears
=== CONT  TestAccNetworkFirewallLoggingConfiguration_CloudWatchLogDestination_logGroup
=== CONT  TestAccNetworkFirewallLoggingConfiguration_S3LogDestination_prefix
=== CONT  TestAccNetworkFirewallLoggingConfiguration_KinesisLogDestination_deliveryStream
=== CONT  TestAccNetworkFirewallLoggingConfiguration_KinesisLogDestination_logType
=== CONT  TestAccNetworkFirewallLoggingConfiguration_CloudWatchLogDestination_logType
=== CONT  TestAccNetworkFirewallLoggingConfiguration_updateFirewallARN
=== CONT  TestAccNetworkFirewallLoggingConfiguration_S3LogDestination_logType
=== CONT  TestAccNetworkFirewallLoggingConfiguration_S3LogDestination_bucketName
=== CONT  TestAccNetworkFirewallLoggingConfiguration_updateToSingleFlowTypeLogDestination
=== CONT  TestAccNetworkFirewallLoggingConfiguration_updateToSingleAlertTypeLogDestination
=== CONT  TestAccNetworkFirewallLoggingConfiguration_updateToMultipleLogDestinations
=== CONT  TestAccNetworkFirewallLoggingConfiguration_disappears
=== CONT  TestAccNetworkFirewallLoggingConfiguration_updateLogDestinationType
=== CONT  TestAccNetworkFirewallLoggingConfiguration_updateToSingleTLSTypeLogDestination
--- PASS: TestAccNetworkFirewallLoggingConfiguration_updateToSingleAlertTypeLogDestination (546.36s)
--- PASS: TestAccNetworkFirewallLoggingConfiguration_S3LogDestination_prefix (547.01s)
--- PASS: TestAccNetworkFirewallLoggingConfiguration_S3LogDestination_logType (576.46s)
--- PASS: TestAccNetworkFirewallLoggingConfiguration_S3LogDestination_bucketName (617.34s)
--- PASS: TestAccNetworkFirewallLoggingConfiguration_updateFirewallARN (1015.85s)
--- PASS: TestAccNetworkFirewallLoggingConfiguration_CloudWatchLogDestination_logGroup (1139.17s)
--- PASS: TestAccNetworkFirewallLoggingConfiguration_disappears (1148.43s)
--- PASS: TestAccNetworkFirewallLoggingConfiguration_KinesisLogDestination_logType (1217.22s)
--- PASS: TestAccNetworkFirewallLoggingConfiguration_updateToMultipleLogDestinations (1264.48s)
--- PASS: TestAccNetworkFirewallLoggingConfiguration_CloudWatchLogDestination_logType (1646.39s)
--- PASS: TestAccNetworkFirewallLoggingConfiguration_updateToSingleTLSTypeLogDestination (1670.60s)
--- PASS: TestAccNetworkFirewallLoggingConfiguration_updateLogDestinationType (1679.02s)
--- PASS: TestAccNetworkFirewallLoggingConfiguration_KinesisLogDestination_deliveryStream (2024.37s)
--- PASS: TestAccNetworkFirewallLoggingConfiguration_updateToSingleFlowTypeLogDestination (2127.66s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/networkfirewall    2127.934s

...

[github-actions]

Community Note

Voting for Prioritization

• Please vote on this pull request by adding a 👍 reaction to the original post to help the community and maintainers prioritize this pull request.
• Please see our prioritization guide for information on how we prioritize.
• 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.

For Submitters

• Review the contribution guide relating to the type of change you are making to ensure all of the necessary steps have been taken.
• For new resources and data sources, use skaff to generate scaffolding with comments detailing common expectations.
• Whether or not the branch has been rebased will not impact prioritization, but doing so is always a welcome surprise.

[lorodoes]

Fixed the expand function and the remove function to make sure it can remove 1-3 configurations. This is currently the best way since the only way to create each log configuration is create one at a time. You can't do it in one request. Each log configuration has to be it's own request and they have to be done all together at one time. If someone at AWS want's a feedback, there has to be a better way to handle this via API.

[lorodoes]

[root@fedora test_terraform_network_firewall_config]# ./terraform apply
╷
│ Warning: Provider development overrides are in effect
│
│ The following provider development overrides are set in the CLI configuration:
│  - hashicorp/aws in /opt/aws_terraform_install
│
│ The behavior may therefore not match any released version of the provider and applying changes may cause the state to become incompatible with published releases.
╵

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  # aws_networkfirewall_logging_configuration.example will be created
  + resource "aws_networkfirewall_logging_configuration" "example" {
      + firewall_arn = "arn:aws:network-firewall:us-east-2:012345678901:firewall/test"
      + id           = (known after apply)

      + logging_configuration {
          + log_destination_config {
              + log_destination      = {
                  + "bucketName" = "lorodoes"
                  + "prefix"     = "/example"
                }
              + log_destination_type = "S3"
              + log_type             = "ALERT"
            }
          + log_destination_config {
              + log_destination      = {
                  + "bucketName" = "lorodoes"
                  + "prefix"     = "/example"
                }
              + log_destination_type = "S3"
              + log_type             = "TLS"
            }
          + log_destination_config {
              + log_destination      = {
                  + "bucketName" = "lorodoes"
                  + "prefix"     = "example"
                }
              + log_destination_type = "S3"
              + log_type             = "FLOW"
            }
        }
    }

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

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

aws_networkfirewall_logging_configuration.example: Creating...
aws_networkfirewall_logging_configuration.example: Creation complete after 1s [id=arn:aws:network-firewall:us-east-2:012345678901:firewall/test]

Apply complete! Resources: 1 added, 0 changed, 0 destroyed.

[lorodoes]

@ewbankkit during development I did get an error once where the Logging config couldn't be applied because the NetworkFirewall was created via the AWS console and the console showed that it created with an arn and everything but was in a pending state and had never been marked as Available. I know terraform waits till the network firewall is fully complete before it moves on, but I was wondering if adding in a catch for that type of error and a retry might be appropriate?

[ewbankkit]

LGTM 🚀.

% ACCTEST_TIMEOUT=720m make testacc TESTARGS='-run=TestAccNetworkFirewallLoggingConfiguration_' PKG=networkfirewall ACCTEST_PARALLELISM=3
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.23.3 test ./internal/service/networkfirewall/... -v -count 1 -parallel 3  -run=TestAccNetworkFirewallLoggingConfiguration_ -timeout 720m
2024/12/09 08:50:35 Initializing Terraform AWS Provider...
=== RUN   TestAccNetworkFirewallLoggingConfiguration_CloudWatchLogDestination_logGroup
=== PAUSE TestAccNetworkFirewallLoggingConfiguration_CloudWatchLogDestination_logGroup
=== RUN   TestAccNetworkFirewallLoggingConfiguration_CloudWatchLogDestination_logType
=== PAUSE TestAccNetworkFirewallLoggingConfiguration_CloudWatchLogDestination_logType
=== RUN   TestAccNetworkFirewallLoggingConfiguration_KinesisLogDestination_deliveryStream
=== PAUSE TestAccNetworkFirewallLoggingConfiguration_KinesisLogDestination_deliveryStream
=== RUN   TestAccNetworkFirewallLoggingConfiguration_KinesisLogDestination_logType
=== PAUSE TestAccNetworkFirewallLoggingConfiguration_KinesisLogDestination_logType
=== RUN   TestAccNetworkFirewallLoggingConfiguration_S3LogDestination_bucketName
=== PAUSE TestAccNetworkFirewallLoggingConfiguration_S3LogDestination_bucketName
=== RUN   TestAccNetworkFirewallLoggingConfiguration_S3LogDestination_logType
=== PAUSE TestAccNetworkFirewallLoggingConfiguration_S3LogDestination_logType
=== RUN   TestAccNetworkFirewallLoggingConfiguration_S3LogDestination_prefix
=== PAUSE TestAccNetworkFirewallLoggingConfiguration_S3LogDestination_prefix
=== RUN   TestAccNetworkFirewallLoggingConfiguration_updateFirewallARN
=== PAUSE TestAccNetworkFirewallLoggingConfiguration_updateFirewallARN
=== RUN   TestAccNetworkFirewallLoggingConfiguration_updateLogDestinationType
=== PAUSE TestAccNetworkFirewallLoggingConfiguration_updateLogDestinationType
=== RUN   TestAccNetworkFirewallLoggingConfiguration_updateToMultipleLogDestinations
=== PAUSE TestAccNetworkFirewallLoggingConfiguration_updateToMultipleLogDestinations
=== RUN   TestAccNetworkFirewallLoggingConfiguration_updateToSingleAlertTypeLogDestination
=== PAUSE TestAccNetworkFirewallLoggingConfiguration_updateToSingleAlertTypeLogDestination
=== RUN   TestAccNetworkFirewallLoggingConfiguration_updateToSingleFlowTypeLogDestination
=== PAUSE TestAccNetworkFirewallLoggingConfiguration_updateToSingleFlowTypeLogDestination
=== RUN   TestAccNetworkFirewallLoggingConfiguration_updateToSingleTLSTypeLogDestination
=== PAUSE TestAccNetworkFirewallLoggingConfiguration_updateToSingleTLSTypeLogDestination
=== RUN   TestAccNetworkFirewallLoggingConfiguration_disappears
=== PAUSE TestAccNetworkFirewallLoggingConfiguration_disappears
=== CONT  TestAccNetworkFirewallLoggingConfiguration_CloudWatchLogDestination_logGroup
=== CONT  TestAccNetworkFirewallLoggingConfiguration_updateFirewallARN
=== CONT  TestAccNetworkFirewallLoggingConfiguration_S3LogDestination_bucketName
--- PASS: TestAccNetworkFirewallLoggingConfiguration_CloudWatchLogDestination_logGroup (478.56s)
=== CONT  TestAccNetworkFirewallLoggingConfiguration_KinesisLogDestination_deliveryStream
--- PASS: TestAccNetworkFirewallLoggingConfiguration_S3LogDestination_bucketName (486.23s)
=== CONT  TestAccNetworkFirewallLoggingConfiguration_KinesisLogDestination_logType
--- PASS: TestAccNetworkFirewallLoggingConfiguration_KinesisLogDestination_deliveryStream (479.44s)
=== CONT  TestAccNetworkFirewallLoggingConfiguration_updateToSingleFlowTypeLogDestination
--- PASS: TestAccNetworkFirewallLoggingConfiguration_KinesisLogDestination_logType (480.44s)
=== CONT  TestAccNetworkFirewallLoggingConfiguration_disappears
--- PASS: TestAccNetworkFirewallLoggingConfiguration_updateFirewallARN (978.78s)
=== CONT  TestAccNetworkFirewallLoggingConfiguration_updateToSingleTLSTypeLogDestination
--- PASS: TestAccNetworkFirewallLoggingConfiguration_updateToSingleFlowTypeLogDestination (405.82s)
=== CONT  TestAccNetworkFirewallLoggingConfiguration_updateToMultipleLogDestinations
--- PASS: TestAccNetworkFirewallLoggingConfiguration_disappears (439.36s)
=== CONT  TestAccNetworkFirewallLoggingConfiguration_updateToSingleAlertTypeLogDestination
--- PASS: TestAccNetworkFirewallLoggingConfiguration_updateToSingleTLSTypeLogDestination (460.82s)
=== CONT  TestAccNetworkFirewallLoggingConfiguration_updateLogDestinationType
--- PASS: TestAccNetworkFirewallLoggingConfiguration_updateToSingleAlertTypeLogDestination (154.67s)
=== CONT  TestAccNetworkFirewallLoggingConfiguration_S3LogDestination_prefix
--- PASS: TestAccNetworkFirewallLoggingConfiguration_updateToMultipleLogDestinations (300.86s)
=== CONT  TestAccNetworkFirewallLoggingConfiguration_S3LogDestination_logType
--- PASS: TestAccNetworkFirewallLoggingConfiguration_updateLogDestinationType (528.72s)
=== CONT  TestAccNetworkFirewallLoggingConfiguration_CloudWatchLogDestination_logType
--- PASS: TestAccNetworkFirewallLoggingConfiguration_S3LogDestination_prefix (463.91s)
--- PASS: TestAccNetworkFirewallLoggingConfiguration_S3LogDestination_logType (480.25s)
--- PASS: TestAccNetworkFirewallLoggingConfiguration_CloudWatchLogDestination_logType (502.87s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/networkfirewall	2476.414s

[jar-b]

LGTM 🚀

[ewbankkit]

@lorodoes Thanks for the contribution 🎉 👏.

[github-actions]

This functionality has been released in v5.81.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. Thank you!

[github-actions]

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.