WIP: providers/google: restore_policy supports update

[evandbrown]

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 #11736

[paddycarver]

Is this still a WIP @evandbrown, or is it ready for review?

[evandbrown]

@paddyforan sorry, should have clarified what I meant by WIP. It is ready for review, but I have a few acceptance tests I'd like to add before merging.

[paddycarver]

Fair enough! I will never turn down moar acceptance tests.

[paddycarver]

So, thoughts on this.

I think this will remove any ability Terraform has to detect drift, because it considers any changes outside of it to be totally OK and not something to be corrected. Also, I think that's a much bigger hammer than we need for the actual bug at hand, which is that module state and root state aren't aware of each other's changes to the same resource, which is (as I understand it) something Terraform should be able to handle.

I have some reservations about this approach, just because it's such a departure from what Terraform does in other cases.

[danawillow]

Looks like the associated issue was closed. @evandbrown, can this PR be closed?

[pdecat]

I think this will remove any ability Terraform has to detect drift, because it considers any changes outside of it to be totally OK and not something to be corrected.

What about ignoring drift about identities (IAM users, service accounts) that are not managed by terraform (i.e. not created by google_service_account)?

In my use case, this would let me manage service accounts using terraform, and the customer would still be able to manage real users and custom service accounts by hand.

This could be made an additional option, or a sub-option of authoritative like lenient or lazy.

[paddycarver]

This could be made an additional option, or a sub-option of authoritative like lenient or lazy.

I'd really like to avoid getting into a situation where we're specifying different levels of control Terraform has. At the root of the issue, I guess, is that Terraform is an infrastructure-as-code tool. If the code (i.e., Terraform) isn't documenting what your infrastructure actually should be, something has gone wrong somewhere.

In practice, that gets complicated sometimes, and we err on the side of UX over theory. Which is where authoritative comes into play (though I still feel :/ about it). As soon as you get into "by hand", we're getting away from what Terraform is good at.

I'm going to close this out, as the issue the PR is meant to resolve has been closed. If there's a continued interest in this PR and getting the implementation merged, feel free to reopen/comment, @evandbrown, and we'll pick this up. I don't mean to slam the door on the conversation, but I also don't want to leave stale PRs just sitting around. :)

If there's continued interest in supporting IAM use cases not covered by our current implementation, we'd love to hear them. But let's break them out into issues, so poor @evandbrown doesn't get a million notifications. :)

[munnerz]

Hi there - I've just been reading through this PR and the issues related to it.

I'd like to express my support for this, or some similar implementation. I too have a customer that would like to manage their users through the admin console, but also have their application/infrastructure managed by Terraform. We need to create service accounts/roles for eg. SQL users, but right now it's an all-or-nothing as whenever they add a new user through the cloud console, the users are removed later.

We also may have multiple Terraform projects that deploy different environments but within the same project - once again, each of these stacks needs to set IAM permissions. As it stands, the two stacks 'fight' each other, thus making it effectively unusable (it is far too easy to accidentally break a different environment when updating an environment!)

[paddycarver]

Hey @munnerz! Thanks for the feedback. We're mostly tracking this in hashicorp/terraform-provider-google#72 and its linked issues and PRs right now.

I'm sorry for the problems! Definitely see how those valid use cases could totally be left in the cold by the way things are right now. Fortunately, hashicorp/terraform-provider-google#171 is in (I hope) its final rounds of review and should be merged soon, hopefully. That should cover the use cases you discussed. If not, I'd love to hear more about it on that PR.

In the meantime, I'm going to lock the conversation on this PR. I want to say this is explicitly not in the interest of not hearing more discussion about this issue, but is simply attempting to move the conversation to the appropriate venue (namely, the terraform-providers/terraform-provider-google repository, ideally in one of the issues or PRs linked.) If you have further comments or input, I encourage you to send them there, where they'll be seen by the appropriate people (which is less likely if they get put in this thread.)