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

google_iam_policy with multiple bindings show changes on every plan #8701

Assignees
Labels
forward/review In review; remove label to forward persistent-bug Hard to diagnose or long lived bugs for which resolutions are more like feature work than bug work priority/3 service/iam-core service/iap size/m

Comments

@keeleysam
Copy link

keeleysam commented Mar 16, 2021

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

Terraform 0.13
Google provider 3.60.0

Affected Resource(s)

Primarily this affects:

  • google_iam_policy

It also affects anything which can use a policy_data input, including all of these:

  • google_iap_app_engine_service_iam
  • google_iap_app_engine_version_iam
  • google_iap_tunnel_iam
  • google_iap_tunnel_instance_iam
  • google_iap_web_backend_service_iam
  • google_iap_web_iam
  • google_iap_web_type_app_engine_iam
  • google_iap_web_type_compute_iam

Terraform Configuration Files

This is an example which would trigger the issue, note that our actual config may have many more bindings and longer expressions.

data "google_iam_policy" "example" {
  binding {
    role = "roles/iap.httpsResourceAccessor"
    members = [
      "group:group-a@example.com"
    ]

    condition {
      title       = "policy_group_a"
      expression  = "request.host == 'foo-1.example.com'"
    }
  }

  binding {
    role = "roles/iap.httpsResourceAccessor"
    members = [
      "group:group-3@example.com"
    ]

    condition {
      title       = "policy_group-3"
      expression  = "request.host == 'foo-z.example.com'"
    }
  }

  binding {
    role = "roles/iap.httpsResourceAccessor"
    members = [
      "group:group-3@example.com"
    ]

    condition {
      title       = "policy_group-3-bar"
      expression  = "request.host == 'foo-bar.example.com'"
    }
  }

  binding {
    role = "roles/iap.httpsResourceAccessor"
    members = [
      "user:foo-a@example.com"
    ]

    condition {
      title       = "foo-a"
      expression  = "request.host == 'foo-a.example.com'"
    }
  }
}

resource "google_iap_web_backend_service_iam_policy" "example" {
  web_backend_service = "example"
  policy_data         = data.google_iam_policy.example.policy_data
}

Debug Output

N/A

Panic Output

N/A

Expected Behavior

Subsequent plans should show no changes after a successful apply.

Actual Behavior

The config is able to apply, but subsequent plans will always show changes to apply even when nothing has changed.

The policy data gets encoded a list in json without sorting the input, so once you have more than a couple of bindings in a policy terraform thinks that there are changes because the state doesn't match what the generated json looks like.

If you just have a single binding you won't run into this problem, but it becomes very difficult to see what the actual changes are when it will show significant changes on every plan.

It may be possible to work around this by placing the bindings in a policy in the same order which the json encoder would put them in, but this should not be needed.

Steps to Reproduce

  1. terraform apply

Important Factoids

References

  • b/197114798
@ghost ghost added the bug label Mar 16, 2021
@edwardmedia edwardmedia self-assigned this Mar 16, 2021
@keeleysam
Copy link
Author

I believe the issue here exists within google/resource_iam_policy.go, and that the problem is that the policy data which comes back from the Google API may come back in any order, but when the local policy gets converted to json here its always sorted and an exact match is expected.

Possible solutions could be ignoring the order, or sorting list which come back within bindings so that the difference will actually show properly in plans.

@keeleysam
Copy link
Author

Also this is related to issue #7074, but it seems like the root is a bit different. In that case, the original policy was created outside of Terraform. In this case, everything has been done through Terraform. The issue is simply that the provider here is strict about the ordering of bindings when it should ignore their ordering.

@edwardmedia edwardmedia removed their assignment Mar 25, 2021
@discentem
Copy link
Contributor

#9535 may be a dupe of this? 🤔

@c2thorn c2thorn added persistent-bug Hard to diagnose or long lived bugs for which resolutions are more like feature work than bug work and removed bug labels Oct 13, 2021
@c2thorn c2thorn removed their assignment Oct 13, 2021
@c2thorn
Copy link
Collaborator

c2thorn commented Oct 13, 2021

Setting as persistent-bug due to length of time the bug has existed and amount of resources it affects.

Solution seems to require adding diff suppress logic to ignore the json order, or sorting the list.

@rileykarson rileykarson added this to the Near-Term Goals milestone Oct 18, 2021
@oceyral
Copy link
Contributor

oceyral commented Oct 20, 2021

From what I've seen, it seems to mostly happen when there are conditions present, specifically when there are multiple bindings of the same role with different or identical conditions.

  1. When there is no description set in condition, and an unrelated binding changes, the plan/apply will show an empty description added do the condition, and will switch around parts of the bindings with condition (titles and members).
  2. When there is a description, it will show bindings switched around as a block, instead of just switching parts of them.
  3. Bindings with no conditions are unaffected.

The order of the bindings in the google_iam_policy data source don't seem to matter at all.

@DrStriky
Copy link

Is there a known workaround for this type of bug?

For me, it helps if I deploy all bindings without conditions and then add all the ones with conditions but this is not a very sustainable way of deploying stuff.

@SarahFrench
Copy link
Member

I've been working on a PR to address this, and just adding some notes about how I've explored and reproduced the issue

1. Problem due to different JSONs in state vs returned from Google APIs

Terraform v1.2.5
on darwin_arm64
+ provider registry.terraform.io/hashicorp/google v4.32.0
👉 Configuration I used is here (expand to see)
provider "google" {
  project = var.gcp_project_id
  region  = var.default_region
  zone    = var.default_zone
}

variable "gcp_project_id" {
  type = string
}
variable "default_region" {
  type = string
}
variable "default_zone" {
  type = string
}

locals {
  group_1 = "[name]@[domain].com"
  group_2 =  "[name]@[domain].com"
}

data "google_iam_policy" "example" {

  # Bindings without condition blocks
  binding {
    role = "roles/iap.httpsResourceAccessor"
    members = [
      "group:${local.group_1}"
    ]
  }
  binding {
    role = "roles/iap.admin"
    members = [
      "group:${local.group_1}"
    ]
  }

  # Bindings with condition blocks
  binding {
    role = "roles/iap.httpsResourceAccessor"
    members = [
      "group:${local.group_1}"
    ]

    condition {
      title      = "foobar-a"
      description = "This one has a description"
      expression = "request.host == 'foobar-a.example.com'"
    }
  }

  binding {
    role = "roles/iap.httpsResourceAccessor"
    members = [
      "group:${local.group_1}"
    ]

    condition {
      title      = "foobar-b"
      expression = "request.host == 'foobar-b.example.com'"
    }
  }

  binding {
    role = "roles/iap.httpsResourceAccessor"
    members = [
      "group:${local.group_2}"
    ]

    condition {
      title      = "foobar-c"
      expression = "request.host == 'foobar-c.example.com'"
    }
  }
}

resource "google_iap_web_type_compute_iam_policy" "example" {
  project     = var.gcp_project_id
  policy_data = data.google_iam_policy.example.policy_data
}

After a successful apply the resource is created but debug logs warn that there's an unexpected change in the resource's data after creation.

This is due to the resource's state being set using data that is read from the API, versus the input data coming from the config. From what I've seen, policy data that originates from the google_iam_policy data source is altered by the API to introduce ordering, deduplication etc, and the altered version is stored in the resource's state. If you look at the state and compare the value of policy_data for the data source and policy_data for the google_iap_web_type_compute_iam_policy resource you'll see that they are mismatched but essentially contain the same information.

For me, subsequent apply steps didn't show unexpected changes resulting from those mismatched JSONs unless I make a change to the config.

👉 Example of unexpected changes in a plan (expand to see)

For example, changing the description in one binding in the config above results in a plan with lots of unexpected changes, which are due to the mismatching I mentioned

  # Bindings with condition blocks
  binding {
    role = "roles/iap.httpsResourceAccessor"
    members = [
      "group:${local.group_1}"
    ]

    condition {
      title      = "foobar-a"
-    description = "This one has a description"
+    description = "This one has a description and I edited it"
      expression = "request.host == 'foobar-a.example.com'"
    }
  }

Screenshot 2022-08-25 at 17 10 03

2. Plans adding empty descriptions to conditions inside bindings

There's also the issue where plans contain diffs like this:

~ {
    ~ condition = {
+         description = ""
          # (2 unchanged elements hidden)
      }
      # (2 unchanged elements hidden)
  },
~ {
    ~ condition = {
+         description = ""
          # (2 unchanged elements hidden)
      }
      # (2 unchanged elements hidden)
  },

This seems to be a separate issue to the above, and is due to the Google API not returning empty fields in responses, which affects state, and Terraform sending empty fields in requests. I'll probably change this in a separate PR!

@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 28, 2022
@github-actions github-actions bot added forward/review In review; remove label to forward service/iap service/iam-core labels Jan 14, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.