hashicorp / terraform-provider-google

Terraform Provider for Google Cloud Platform
https://registry.terraform.io/providers/hashicorp/google/latest/docs
Mozilla Public License 2.0
2.36k stars 1.75k forks source link

source_ranges = null doesn't work on EGRESS rule #9280

Open thuck opened 3 years ago

thuck commented 3 years ago

Terraform Version

Affected Resource(s)

Expected Behavior

When setting source_ranges = null on egress rule terraform shouldn't try to set it.

Actual Behavior

terraform includes source_ranges on request and rule fails.

Steps to Reproduce

Create a EGRESS rule using source_rnages = null. terraform apply.

Error when null is set


│ Error: Error creating Firewall: googleapi: Error 400: Should not specify source range/tag for egress direction., badRequest
│
│   with module.rules["XXXXXXXXXXXXXXXXXXXXXXXXX"].google_compute_firewall.rules["YYYYYYYYYYYY"],
│   on terraform-modules/modules/gcp_firewall_rules/main.tf line 1, in resource "google_compute_firewall" "rules":
│    1: resource google_compute_firewall rules {
│

Code

resource google_compute_firewall rules {
  for_each = var.rules
  name    = "${var.prefix}-${each.key}"
  network = var.network

  direction = each.value.direction
  description = try(each.value.description, null)
  priority = try(each.value.priority, 1000)
  disabled = try(each.value.disabled, false)

  dynamic allow {
    for_each = try(each.value.allow, [])
    content {
      ports = try(allow.value.ports, null)
      protocol = allow.value.protocol
    }
  }

  dynamic deny {
    for_each = try(each.value.deny, [])
    content {
      ports = try(deny.value.ports, null)
      protocol = deny.value.protocol
    }
  }

  dynamic log_config {
    for_each = try(each.value.log_config, [])
    content {
      metadata = log_config.value.metadata
    }
  }

  source_tags = try(each.value.source_tags, null)
  source_ranges = each.value.direction == "INGRESS" ? lookup(each.value, "source_ranges", null) : null
  source_service_accounts = try(each.value.source_service_accounts, null)

  target_tags = try(each.value.target_tags, null)
  destination_ranges = each.value.direction == "EGRESS" ? lookup(each.value, "destination_ranges", null) : null
  target_service_accounts = try(each.value.target_service_accounts, null)
}

b/304968093

venkykuberan commented 3 years ago

@thuck Can you please share the plan output of the config .

thuck commented 3 years ago

@venkykuberan I tried to see if changing the functions for the source_ranges would create a different effect on the plan; so I tried lookup(), try() and to force the source_ranges to null on the resource itself, in all occasions the result is the same: source_ranges = (known after apply)

test configuration is this:

test:
  description: egress test
  direction: EGRESS
  destination_ranges:
    - 10.10.10.0/26
  deny:
    - protocol: tcp
      ports:
        - 22

plan output

  # module.rules["XXXXXXXXXXXXXXX"].google_compute_firewall.rules["test"] will be created
  + resource "google_compute_firewall" "rules" {
      + creation_timestamp = (known after apply)
      + description        = "egress test"
      + destination_ranges = [
          + "10.10.10.0/26",
        ]
      + direction          = "EGRESS"
      + disabled           = false
      + enable_logging     = (known after apply)
      + id                 = (known after apply)
      + name               = "test-test"
      + network            = "test-network"
      + priority           = 1000
      + project            = (known after apply)
      + self_link          = (known after apply)
      + source_ranges      = (known after apply)

      + deny {
          + ports    = [
              + "22",
            ]
          + protocol = "tcp"
        }
    }

resource with forced null

resource google_compute_firewall rules {
  for_each = var.rules
  name    = "${var.prefix}-${each.key}"
  network = var.network

  direction = each.value.direction
  description = try(each.value.description, null)
  priority = try(each.value.priority, 1000)
  disabled = try(each.value.disabled, false)

  dynamic allow {
    for_each = try(each.value.allow, [])
    content {
      ports = try(allow.value.ports, null)
      protocol = allow.value.protocol
    }
  }

  dynamic deny {
    for_each = try(each.value.deny, [])
    content {
      ports = try(deny.value.ports, null)
      protocol = deny.value.protocol
    }
  }

  dynamic log_config {
    for_each = try(each.value.log_config, [])
    content {
      metadata = log_config.value.metadata
    }
  }

  source_tags = try(each.value.source_tags, null)
  source_ranges = null
  source_service_accounts = try(each.value.source_service_accounts, null)

  target_tags = try(each.value.target_tags, null)
  destination_ranges = each.value.direction == "EGRESS" ? lookup(each.value, "destination_ranges", null) : null
  target_service_accounts = try(each.value.target_service_accounts, null)
}
thuck commented 3 years ago

@venkykuberan did you had so time to take a look on this? Can I help in any way?