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

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

lorodoes
Copy link
Contributor

@lorodoes lorodoes commented Nov 12, 2024

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

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

...

…ction and the remove function two allow for more than 2
Copy link

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.

@github-actions github-actions bot added service/networkfirewall Issues and PRs that pertain to the networkfirewall service. needs-triage Waiting for first response or review from a maintainer. labels Nov 12, 2024
@lorodoes lorodoes changed the title #38917 #39197 #38487 Fixing the expand function and the remove functi… #38917 #39197 Fixing the expand function and the remove functi… Nov 12, 2024
@lorodoes
Copy link
Contributor Author

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 lorodoes marked this pull request as ready for review November 12, 2024 03:50
@lorodoes lorodoes requested a review from a team as a code owner November 12, 2024 03:50
@lorodoes
Copy link
Contributor Author

lorodoes commented Nov 12, 2024

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

image

@justinretzolk justinretzolk added bug Addresses a defect in current functionality. and removed needs-triage Waiting for first response or review from a maintainer. labels Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Addresses a defect in current functionality. service/networkfirewall Issues and PRs that pertain to the networkfirewall service.
Projects
None yet
2 participants