google_compute_security_policy rules are always recreated if they have a match expr set

[abstrctn]

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

0.15.1

Affected Resource(s)

• google_compute_security_policy

References

• google_compute_security_policy all rules getting recreated #8251
• google_compute_security_policy expr rules getting recreated #8437

I believe #8437 should be re-opened. It was originally created because #8251 was closed, and @melinath asked @Sreerag74031 to open a new one to continue discussion of the larger issue that was found. But then #8437 was closed because #8251 already existed (but was already closed).

The issue still seems to affect me on the latest version, and the above issues don't suggest that a solution was ever implemented.

Internal issue: http://b/196186416

[melinath]

I vaguely remember this - the summary of my findings is in GoogleCloudPlatform/magic-modules#4472. TLDR there is an outstanding issue here. Since I've lost the context it would probably make sense to assign this to the current bug onduty.

What's happening:

-     - rule {
-         - action      = "allow" -> null
-         - description = "description" -> null
-         - preview     = false -> null
-         - priority    = 100 -> null

-         - match {

-             - expr {
-                 - expression = "request.path.matches('/some/path/')" -> null
               }
            }
        }
+     + rule {
+         + action      = "allow"
+         + description = "description"
+         + preview     = false
+         + priority    = 100
+         + match {

+             + expr {
+                 + expression = "request.path.matches('/some/path/')"
                }
            }
        }

How to reproduce:

Should be possible to reproduce by setting up a rule that has a match using expr, then adding a new rule (without changing the one that uses match expr). This will erroneously trigger a recreate of the existing rule.

[trenslow]

I have a similar issue. Terraform always wants to recreate all rules in the security policy when the list of IP ranges differs from what's in the state. This has been going on since Terraform 0.12 AFAIK.

[melinath]

b/196186416

[trenslow]

Is there any update on this issue?

[c2thorn]

Likely need to switch the field to an un-ordered list (similar to BT table, compute disk?). Marking as persistent-bug to discuss in triage

[trenslow]

Hi, any updates?

[wata727]

Our company is facing this bug and I'm working on it. According to my research, the unspecified versioned_expr is treated as a null value of string type in the Terraform configuration, while it is treated as an empty string in the state. versioned_expr is empty only when you set expr, so it will not be reproduced when you set config.

So, setting the default value of versioned_expr to an empty string resolves this problem. In fact, I've confirmed that the diff no longer occurs in the @melinath's case #9084 (comment).
GoogleCloudPlatform/magic-modules#5665

This change looks like a major change because it is a change of the default value, but the default value of versioned_expr on the state is already an empty string, and the behavior of the API call is the same whether it is an empty string or a null value. That is, there are no visible changes to the user other than fixing the bug. I believe this change can be merged as a bug fix.

In our company, there is a module containing google_compute_security_policy referenced by over 50 workspaces. This module is maintained by the security team, and it contains a lot of expr rules. These rules are updated frequently, so workspace admin updating the module are deprived of their time by the large amount of diffs. I hope this bug is fixed.

[slevenick]

@wata727 I'm not seeing your change fix the extra diffs, have you been able to test it out locally and see the fix?

I am able to get the diff to not show up when I specify the description field for all non-expr rules. I think that the fix will be to set a default for the description field, as it seems like the state value is changing from "" to null without a related change in the config/API object

[wata727]

@slevenick Yes. I confirmed the fix suppressed extra diffs in my case. The following are repro steps:

1. Run terraform apply for the following Terraform configuration

terraform {
  required_providers {
    google = {
      version = "4.14.0"
    }
  }
}

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

  rule {
    action   = "deny(403)"
    priority = "1000"
    match {
      expr {
        expression = "request.path.matches('/admin')"
      }
    }
    description = "only admin"
  }

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

2. Change the description of the default rule

diff --git a/workdir/main.tf b/workdir/main.tf
index 5b49c5c04..89b6403cc 100644
--- a/workdir/main.tf
+++ b/workdir/main.tf
@@ -29,6 +29,6 @@ resource "google_compute_security_policy" "policy" {
         src_ip_ranges = ["*"]
       }
     }
-    description = "default rule"
+    description = "default rule (allow all requests)"
   }
 }

3. Run terraform plan. Not only the default rule, but also the "only admin" rule will be recreated

Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # google_compute_security_policy.policy will be updated in-place
  ~ resource "google_compute_security_policy" "policy" {
        id          = "projects/********/global/securityPolicies/my-policy"
        name        = "my-policy"
        # (3 unchanged attributes hidden)

      + rule {
          + action      = "allow"
          + description = "default rule (allow all requests)"
          + preview     = (known after apply)
          + priority    = 2147483647

          + match {
              + versioned_expr = "SRC_IPS_V1"

              + config {
                  + src_ip_ranges = [
                      + "*",
                    ]
                }
            }
        }
      - rule {
          - action      = "allow" -> null
          - description = "default rule" -> null
          - preview     = false -> null
          - priority    = 2147483647 -> null

          - match {
              - versioned_expr = "SRC_IPS_V1" -> null

              - config {
                  - src_ip_ranges = [
                      - "*",
                    ] -> null
                }
            }
        }
      - rule {
          - action      = "deny(403)" -> null
          - description = "only admin" -> null
          - preview     = false -> null
          - priority    = 1000 -> null

          - match {

              - expr {
                  - expression = "request.path.matches('/admin')" -> null
                }
            }
        }
      + rule {
          + action      = "deny(403)"
          + description = "only admin"
          + preview     = false
          + priority    = 1000

          + match {

              + expr {
                  + expression = "request.path.matches('/admin')"
                }
            }
        }
    }

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

─────────────────────────────────────────────────────────────────────────────

Note: You didn't use the -out option to save this plan, so Terraform can't
guarantee to take exactly these actions if you run "terraform apply" now.

4. Apply the Add empty string as default to versioned_expr GoogleCloudPlatform/magic-modules#5665 fix and build the Google provider.

diff --git a/google/resource_compute_security_policy.go b/google/resource_compute_security_policy.go
index 1fd09c3f0..d55f274a5 100644
--- a/google/resource_compute_security_policy.go
+++ b/google/resource_compute_security_policy.go
@@ -99,6 +99,7 @@ func resourceComputeSecurityPolicy() *schema.Resource {
 									"versioned_expr": {
 										Type:         schema.TypeString,
 										Optional:     true,
+										Default:      "",
 										ValidateFunc: validation.StringInSlice([]string{"SRC_IPS_V1"}, false),
 										Description:  `Predefined rule expression. If this field is specified, config must also be specified. Available options:   SRC_IPS_V1: Must specify the corresponding src_ip_ranges field in config.`,
 									},

5. Run terraform plan. The extra diff is suppressed

Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # google_compute_security_policy.policy will be updated in-place
  ~ resource "google_compute_security_policy" "policy" {
        id          = "projects/********/global/securityPolicies/my-policy"
        name        = "my-policy"
        # (3 unchanged attributes hidden)

      + rule {
          + action      = "allow"
          + description = "default rule (allow all requests)"
          + preview     = (known after apply)
          + priority    = 2147483647

          + match {
              + versioned_expr = "SRC_IPS_V1"

              + config {
                  + src_ip_ranges = [
                      + "*",
                    ]
                }
            }
        }
      - rule {
          - action      = "allow" -> null
          - description = "default rule" -> null
          - preview     = false -> null
          - priority    = 2147483647 -> null

          - match {
              - versioned_expr = "SRC_IPS_V1" -> null

              - config {
                  - src_ip_ranges = [
                      - "*",
                    ] -> null
                }
            }
        }
        # (1 unchanged block hidden)
    }

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

─────────────────────────────────────────────────────────────────────────────

Note: You didn't use the -out option to save this plan, so Terraform can't
guarantee to take exactly these actions if you run "terraform apply" now.

Environment:

$ terraform -v
Terraform v1.1.4
on darwin_amd64
+ provider registry.terraform.io/hashicorp/google v4.14.0

I think that the fix will be to set a default for the description field, as it seems like the state value is changing from "" to null without a related change in the config/API object

As in the repro above, I think this solution is not correct because it happens even if the description field is set.

[github-actions]

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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.