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

Non-authoritative policies don't survive updates #11736

Closed
evandbrown opened this issue Feb 7, 2017 · 5 comments
Closed

Non-authoritative policies don't survive updates #11736

evandbrown opened this issue Feb 7, 2017 · 5 comments

Comments

@evandbrown
Copy link
Contributor

Initially reported by @allandrick in #11556:

Some additional data points that may be useful here - I too was doing this the old way like @craigfurman and when I updated to 0.8.5 and updated the syntax for google_project_iam_policy, I too lost access to the project I was working on, despite specifying "authortative = false" in my configuration. Thankfully I am using the org.node and was able to re-add my Terraform service account to the specific project and start clean using the 0.8.5 syntax - however, things are still not quite right.

I use a Terraform module in all projects to add the default policy bindings I want - this works. I then use local policy bindings and another google_project_iam_policy resource, also with authorative = false, in the same project to add custom roles to the project that aren't needed globally in the module.

This causes terraform to flip-flop between re-writing the policy bindings, especially where I am trying to add a specific IAM role to existing groups or service accounts that were defined with the base policy bindings for the project.

and

Example:

in module "iam-account", we create:

resource "google_project_iam_policy" "google-project" {
   project = "${var.gcp_project}"
   policy_data = "${data.google_iam_policy.policy-bindings.policy_data}"
   authoritative = false
}

data "google_iam_policy" "policy-bindings" {
   # App Engine
   binding {
       role = "roles/appengine.appAdmin"
       members = [
           "group:${var.admins_group}",
       ]
   }
   binding {
       role = "roles/appengine.deployer"
       members = [
           "group:${var.users_group}",
       ]
   }

and in a local terraform file, we later create:

resource "google_service_account" "gcr-readonly" {
   account_id = "gcr-readonly"
   display_name = "Google Container Registry read-only service account"
}

data "google_iam_policy" "local-policy-bindings" {
 binding {
   role = "roles/container.viewer"
   members = [
     "serviceAccount:${google_service_account.gcr-readonly.email}",
   ]
 }
}

resource "google_project_iam_policy" "google-project" {
   project = "${var.gcp-project}"
   policy_data = "${data.google_iam_policy.local-policy-bindings.policy_data}"
   authoritative = false
}

this should only add this new policy binding to the Project IAM - and terraform plan says that's all it wants to do:

~ google_project_iam_policy.google-project
   policy_data: "{}" => "{\"bindings\":[{\"members\":[\"serviceAccount:[email protected]\"],\"role\":\"roles/container.viewer\"}]}"

So we apply it:

google_project_iam_policy.google-project: Modifying...
 policy_data: "{}" => "{\"bindings\":[{\"members\":[\"serviceAccount:[email protected]\"],\"role\":\"roles/container.viewer\"}]}"
google_project_iam_policy.google-project: Modifications complete

But then immediately a new terraform plan says it wants to change things again, because reality doesn't match what is in the module's state, even though it shouldn't be removing things that are already in IAM...

~ module.iam-account.google_project_iam_policy.google-project
   policy_data: "{\"bindings\":[{\"members\":[\"group:[email protected]\"],\"role\":\"roles/appengine.appAdmin\"},{\"members\":[\"group:[email protected]\"],\"role\":\"roles/appengine.deployer\"},{\"members\":[\"serviceAccount:[email protected]\"],\"role\":\"roles/container.viewer\"}]}" => "{\"bindings\":[{\"members\":[\"group:[email protected]\"],\"role\":\"roles/appengine.appAdmin\"},{\"members\":[\"group:[email protected]\"],\"role\":\"roles/appengine.deployer\"}]}"
@evandbrown evandbrown self-assigned this Feb 7, 2017
evandbrown added a commit to evandbrown/terraform that referenced this issue Feb 7, 2017
The initial version of google_project_iam_policy would capture an IAM
policy that could be restored (for non-authoritative resources), but it
would only do this on create. If the policy drifted outside of Terraform
after create, the originaly restore_policy would be applied on an
update, removing the out-of-band changes.

This change makes non-authoritative google_project_iam_policy resources
truly non-destructive. The restore_policy is recalculated on update and
delete, ensuring that only the policy_data is removed from a project's
IAM policy.

Fixes hashicorp#11736
evandbrown added a commit to evandbrown/terraform that referenced this issue Feb 7, 2017
The initial version of google_project_iam_policy would capture an IAM
policy that could be restored (for non-authoritative resources), but it
would only do this on create. If the policy drifted outside of Terraform
after create, the originaly restore_policy would be applied on an
update, removing the out-of-band changes.

This change makes non-authoritative google_project_iam_policy resources
truly non-destructive. The restore_policy is recalculated on update and
delete, ensuring that only the policy_data is removed from a project's
IAM policy.

Fixes hashicorp#11736
@craigfurman
Copy link

craigfurman commented Feb 7, 2017

Off the top of my head, we migrated to google_project_iam_policy after #11556 and found no trouble.

However, we were using terraform in a slightly unusual way:

  1. the .tf config files were part of a tool designed to provision environments
  2. the state files were kept in an environments repo.
  3. Therefore, the restore_policy will contain arbitrarily different state per environment, from the same tf.
  4. Finally, the policy data we are trying to merge in may change over time.

To be able to deprovision test environments without the restore_policy deleting too much from the IAM config on terrraform destroy, we pruned our google_project_iam_policy resource from the tfstate using terraform state rm and I believe any IAM users we create on environment provision are currently a resource we leak on deprovision until we come up with a better way.

@paddycarver
Copy link
Contributor

To be able to deprovision test environments without the restore_policy deleting too much from the IAM config on terrraform destroy, we pruned our google_project_iam_policy resource from the tfstate using terraform state rm and I believe any IAM users we create on environment provision are currently a resource we leak on deprovision until we come up with a better way.

That's definitely not the workflow we want. :( If I understand this correctly, you're using different environments/tfstates in the same project. Is that correct?

Finally, the policy data we are trying to merge in may change over time.

This is setting off alarm bells in my head, and I'd love to hear more about it. If I understand correctly, you want to (over time) change what Terraform considers policies it shouldn't change, but which are not in its config. Is that correct?

Currently digging into the bug report a bit (I see #11737, so won't code anything, but I want to understand exactly what's happening here). Thanks for breaking it out, @evandbrown. :)

@paddycarver
Copy link
Contributor

paddycarver commented Feb 8, 2017

OK, from what I'm seeing:

For @allandrick:

  • When you apply just the module, everything is fine
  • When you add more policies not in the module but in the root, the plan looks fine
  • When you apply with iam policies in the module and iam policies in the root on the same project, a new plan has the module trying to remove the policies defined in the root

Does that sound accurate?

My attempt at reproducing (code here: https://github.com/paddyforan/terraform_11736)

  • When I apply with just IAM in the module, everything is fine
  • After applying the IAM in the module, the plan gives me a diff (both members listed in one policy => each member listed in their own policy; they have the same role)
  • When I add more policies not in the module but in the root, the module state has a diff (both members listed in one policy => each member listed in their own policy), and the root resources are shown as added, as I expect
  • When I apply the policies in the root, everything is fine, it looks like what I'd expect (the module is "changed", the root policy is created)
  • When I plan after applying policies in the root, the module gives me a diff, doing the same thing (breaking grouped roles in state apart) but also removing the policy added in the root.

So I've stumbled into another bug, in which semantically equivalent policies are treated as diffs. I'll open another bug to fix that (edit: #11763), a DiffSuppressFunc will take care of it without a problem.

But the core problem I'm seeing here is that module state is held separately from root state. When the module reads the policies from the API, it strips out the common ones, but doesn't know to strip out the ones that the root "owns", so it sets them in its own state--even though the root should be responsible for them. Then its config sees them in the state and not in the config, and thinks they're different.

That's my understanding of things at the moment. If anyone has any info that proves me wrong, let me know!

@paddycarver
Copy link
Contributor

So I've checked in on this, and module state and root state being held separately is a known limitation; because google_project_iam_policys all correspond to the same project's IAM policy, managing it across module boundaries isn't supported at this time. The recommended workaround is to only assign IAM policies for the project from within a single module (so, in this case, either the root, or the iam-account module). This is a known area for improvement in core, but there's not much we can do in this case until those improvements are made. I'm going to close the issue, but feel free to comment again if you have issues using google_project_iam_policy from within a single module, or if there is a portion of this I'm missing.

@ghost
Copy link

ghost commented Apr 17, 2020

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.

@ghost ghost locked and limited conversation to collaborators Apr 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants