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

No-op changes keep appearing in plans for layer_7_ddos_defense_config #12743

Open
bobdanek opened this issue Oct 7, 2022 · 16 comments · Fixed by GoogleCloudPlatform/magic-modules#10823, hashicorp/terraform-provider-google-beta#7470 or #18345
Labels
forward/linked persistent-bug Hard to diagnose or long lived bugs for which resolutions are more like feature work than bug work service/compute-security-policy size/s upstream
Milestone

Comments

@bobdanek
Copy link

bobdanek commented Oct 7, 2022

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request.
  • Please do not leave +1 or me too comments, they generate extra noise for issue followers and do not help prioritize the request.
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment.
  • If an issue is assigned to the modular-magician user, it is either in the process of being autogenerated, or is planned to be autogenerated soon. If an issue is assigned to a user, that user is claiming responsibility for the issue. If an issue is assigned to hashibot, a community member has claimed the issue already.

Terraform Version

2022-10-07T19:17:35.820Z [INFO]  Terraform version: 1.1.7
2022-10-07T19:17:35.820Z [INFO]  Go runtime version: go1.17.5
2022-10-07T19:17:35.820Z [INFO]  CLI args: []string{"terraform", "-v"}
2022-10-07T19:17:35.820Z [DEBUG] Attempting to open CLI config file: /root/.terraformrc
2022-10-07T19:17:35.821Z [INFO]  Loading CLI configuration from /root/.terraformrc
2022-10-07T19:17:35.821Z [DEBUG] Explicit provider installation configuration is set
2022-10-07T19:17:35.821Z [INFO]  CLI command args: []string{"version", "-v"}
Terraform v1.1.7
on linux_amd64
+ provider registry.terraform.io/hashicorp/google v4.39.0
+ provider registry.terraform.io/hashicorp/google-beta v4.39.0
+ provider registry.terraform.io/ns1-terraform/ns1 v1.12.0

Your version of Terraform is out of date! The latest version
is 1.3.1. You can update by downloading from https://www.terraform.io/downloads.html

Affected Resource(s)

  • google_compute_security_policy

Terraform Configuration Files

locals {
  ip_allow_list = [
    "192.168.1.0/24",
    "192.168.2.0/24",
    "192.168.3.0/24",
    "192.168.4.0/24",
    "192.168.0.1/32",
  ]
}

resource "google_compute_security_policy" "ip-whitelist" {
  name        = "ip-whitelist"
  description = "Limit incoming traffic to only IP ranges specified here"

  dynamic "rule" {
    for_each = chunklist(local.ip_allow_list, 10)

    content {
      description = "Allow traffic from client network ranges"
      preview     = true
      action      = "allow"
      priority    = rule.key + 10

      match {
        versioned_expr = "SRC_IPS_V1"

        config {
          src_ip_ranges = rule.value
        }
      }
    }
  }
  rule {
    preview  = true
    action   = "deny(403)"
    priority = "10000"
    match {
      versioned_expr = "SRC_IPS_V1"
      config {
        src_ip_ranges = ["*"]
      }
    }
    description = "Deny all other traffic"
  }
  rule {
    action   = "allow"
    priority = "2147483647"
    match {
      versioned_expr = "SRC_IPS_V1"
      config {
        src_ip_ranges = ["*"]
      }
    }
    description = "Default rule, higher priority overrides it"
  }
}

Debug Output

This has been heavily redacted and trimmed, so please let me know if anything is missing or needs clarification and I'll be happy to provide it: https://gist.github.com/bobdanek/cdb9da6d062b828f1c7215b85189b9ae

Panic Output

None

Expected Behavior

Terraform plan should show no changes

Actual Behavior

This change continues to show up even after applying it repeatedly:

      - adaptive_protection_config {
          - layer_7_ddos_defense_config {
              - enable = false -> null

Attempted workarounds

First, I updated the google and google-beta providers to 4.39.0 to pick up this change: #12554

Same issue is occurring, so I next tried to silence this by adding those three lines. When I tried that the plan suggests setting rule_visibility to STANDARD:

     ~ adaptive_protection_config {
         ~ layer_7_ddos_defense_config {
             + rule_visibility = "STANDARD"

After which, the apply fails:

Error: Error updating SecurityPolicy "ip-whitelist": googleapi: Error 400: Invalid value for field 'resource.adaptiveProtectionConfig.layer7DdosDefenseConfig.enable': 'false'. Cannot set layer 7 DDoS options if it's not enabled., invalid

Steps to Reproduce

I'm unsure why only one of two existing network security policies are showing this issue, so I'm not entirely sure how to reproduce this. Both have adaptive protection disabled, both were created with Terraform, same version, same provider version, same GCP project, same region, within a few weeks of each other.

Important Factoids

References

b/321386288

@bobdanek bobdanek added the bug label Oct 7, 2022
@edwardmedia edwardmedia self-assigned this Oct 8, 2022
@edwardmedia
Copy link
Contributor

edwardmedia commented Oct 10, 2022

@bobdanek I do see below block in the API response in your log. But using your code above, I don't see this block in the response. I wonder how you created the resource. Can you share the API request that created it?

 "adaptiveProtectionConfig": {
  "layer7DdosDefenseConfig": {
   "enable": false
  }
 },

@bobdanek
Copy link
Author

@bobdanek I do see below block in the API response in your log. But using your code above, I don't see this block in the response. I wonder how you created the resource. Can you share the API request that created it?

 "adaptiveProtectionConfig": {
  "layer7DdosDefenseConfig": {
   "enable": false
  }
 },

I don't have that info unfortunately. I'll see if I can reproduce this in a dev env and report back the details of an offending API request if so

@edwardmedia
Copy link
Contributor

@bobdanek is this still an issue?

@bobdanek
Copy link
Author

Yes, still an issue. I can't reproduce exactly how the production resource I'm working with got into this state, but in a testing environment with the policy in my initial comment, I can reproduce it by:

  1. enabling layer_7_ddos_defense_config (enable = true) and applying (exactly like in the example terraform in Enabling Adaptive Protection on an existing security policy does nothing #11647)
  2. changing enable = true to enable = false and applying
  3. commenting out or deleting the whole adaptive_protection_config block and applying
  4. observing the problem when planning or applying without any further changes to the terraform

@edwardmedia
Copy link
Contributor

edwardmedia commented Nov 4, 2022

I can see the issue now. But I doubt there is a viable solution at this moment. Basically below payload is sent to the api for the update. But there is no good way available to communicate with the api for removing the block from the config.

For users, setting enable to false should achieve the same behavior as by removing the whole block.

PATCH /compute/v1/projects/myporject/global/securityPolicies/issue12743
Host: compute.googleapis.com
{
 "adaptiveProtectionConfig": {
  "layer7DdosDefenseConfig": {
   "enable": false,
   "ruleVisibility": "STANDARD"
  }
 },
 "fingerprint": "8whJutPkS1w="
}

@edwardmedia edwardmedia added the persistent-bug Hard to diagnose or long lived bugs for which resolutions are more like feature work than bug work label Nov 4, 2022
@edwardmedia edwardmedia removed their assignment Nov 4, 2022
@rafael-fecha
Copy link

any news here ?

@rileykarson rileykarson removed the bug label Mar 6, 2023
@uthark
Copy link

uthark commented Mar 14, 2023

I observe the same issue.
If I try to explicitly disable it

  adaptive_protection_config {
    layer_7_ddos_defense_config {
      enable = false
    }
  }

I get the following error:

│ Error: Error updating SecurityPolicy "backend-policy": googleapi: Error 400: Invalid value for field 'resource.adaptiveProtectionConfig.layer7DdosDefenseConfig.enable': 'false'. Cannot set layer 7 DDoS options if it's not enabled., invalid
│
│   with google_compute_security_policy.backend-policy,
│   on securityPolicy.tf line 1, in resource "google_compute_security_policy" "backend-policy":
│    1: resource "google_compute_security_policy" "backend-policy" {

Provider version 4.56.0

@smcgivern
Copy link
Contributor

I've created a PR that fixes part of the issue in GoogleCloudPlatform/magic-modules#8060, but I'm not sure if that's the right way to go.

@smcgivern
Copy link
Contributor

But there is no good way available to communicate with the api for removing the block from the config

I confirmed this with the API explorer. Removing the block (once this setting is disabled) through the API manually doesn't do anything; the API still returns the adaptiveProtectionConfig block in subsequent calls with enable set to false.

Something like GoogleCloudPlatform/magic-modules#8060 will at least allow the enable = false inside a Terraform block to work, but it seems that once you've enabled this on a security policy, you can never remove that block.

@github-actions github-actions bot added forward/review In review; remove label to forward service/compute-security-policy labels Aug 17, 2023
@bf-dennis
Copy link

bf-dennis commented Nov 22, 2023

I managed to make the "change" disappear with the following combination of settings:

adaptive_protection_config {
      layer_7_ddos_defense_config {
          enable = false
      }
  }

  lifecycle {
    ignore_changes = [ 
      adaptive_protection_config[0]
     ]
  }

I also deleted the whole block manually from the state file first and had to do an "initial apply", so this might be part of the solution as well.
However now my plan doesn't show any more infrastructure changes.

@melinath
Copy link
Collaborator

melinath commented Jan 19, 2024

Confirmed that this issue still exists in version 5.12 of the provider. marking upstream since the issue is likely best fixed on the API side.

Config to reproduce:

resource "google_compute_security_policy" "policy" {
  name = "my-policy"

  rule {
    action   = "deny(403)"
    priority = "1000"
    match {
      versioned_expr = "SRC_IPS_V1"
      config {
        src_ip_ranges = ["9.9.9.0/24"]
      }
    }
    description = "Deny access to IPs in 9.9.9.0/24"
  }

  rule {
    action   = "allow"
    priority = "2147483647"
    match {
      versioned_expr = "SRC_IPS_V1"
      config {
        src_ip_ranges = ["*"]
      }
    }
    description = "default rule"
  }

  adaptive_protection_config {
      layer_7_ddos_defense_config {
          enable = true
      }
  }
}

Apply this, then try to remove the adaptive_protection_config block.

@melinath melinath added upstream and removed forward/review In review; remove label to forward labels Jan 19, 2024
@twistedpair
Copy link

Confirming this issues remains.

If the following block is present, when making a new security policy, the apply always fails with the following message

TF Block:

  adaptive_protection_config {
    layer_7_ddos_defense_config {
      enable = false
    }
  }

Error Message:

Error: Error creating SecurityPolicy: googleapi: Error 400: Invalid value for field 'resource.adaptiveProtectionConfig.layer7DdosDefenseConfig.enable': 'false'. Cannot set layer 7 DDoS options if it's not enabl
ed., invalid 
``

Terraform v1.7.3
hashicorp/google v5.9.0
hashicorp/google-beta v5.9.0

@melinath
Copy link
Collaborator

Took another look at this because I didn't leave enough details last time to remember what was going on.

If I apply the config from my previous comment and then remove the adaptive_protection_config block, the provider will create a plan that says it's unsetting the block, but will then send an empty PATCH request to the API, which has no effect. It may be reasonable for the API to not do anything in this case - we should potentially be sending some value to cause the unset / disable to happen.

@melinath
Copy link
Collaborator

I can also confirm that setting enable=false on create specifically triggers an error. However, after the resource is created, enable can be safely set to false.

@SarahFrench
Copy link
Member

I've just merged GoogleCloudPlatform/magic-modules#10823 which should allow users to create new google_compute_security_policy resources where adaptive_protection_config.layer_7_ddos_defense_config is explicitly disabled:

adaptive_protection_config {
      layer_7_ddos_defense_config {
          enable = false
      }
}

That PR doesn't make it so the feature will be disabled when the layer_7_ddos_defense_config or adaptive_protection_config block is deleted.

Is that sufficient to close this issue?

@melinath
Copy link
Collaborator

melinath commented Jun 6, 2024

It sounds like the primary issue here is the permadiff, which isn't yet addressed? So I'd be inclined to leave it open for now?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment