Feature request: Google IAM policies for all kind of resources (not just google_project)

[pdecat]

Currently, there seems to be no way to apply Google IAM policies to resources other than google_project.

What about defining a generic google_resource_iam_policy resource to apply IAM policies to all kind of Google provider resources, not just google_project?

Rationale

Google IAM policies are transversal items that can apply to different kinds of resources.
They are defined by the same schema for IAM resources (e.g. service accounts):

https://cloud.google.com/iam/reference/rest/v1/Policy

as for PubSub resources like topics:

https://cloud.google.com/pubsub/docs/reference/rest/Shared.Types/Policy

Also, the REST APIs share the same behavior:

• https://cloud.google.com/iam/reference/rest/v1/projects.serviceAccounts/setIamPolicy
• https://cloud.google.com/pubsub/docs/reference/rest/v1/projects.topics/setIamPolicy

Right now, terraform only supports setting IAM policies on google_project resources by the way of google_project_iam_policy.

Supporting other resources could mean to decline google_project_iam_policy into google_<resource_name>_iam_policy resources (e.g. google_pubsub_topic_iam_policy, google_pubsub_subscription_iam_policy, etc.).

An alternative could be to implement a generic google_resource_iam_policy taking the target resource as an argument.

Wouldn't that second option make more sense?

It would be up to the implementation to perform validation and work out the Go API to use based on the passed resource's type.

https://github.com/hashicorp/terraform/blob/master/vendor/google.golang.org/api/cloudresourcemanager/v1/cloudresourcemanager-gen.go#L2519

https://github.com/hashicorp/terraform/blob/master/vendor/google.golang.org/api/pubsub/v1/pubsub-gen.go#L2646

Example

Following is an example of what this would look like setting an IAM policy on a pubsub topic:

resource "google_service_account" "publisher" {
  account_id   = "publisher"
  display_name = "Publisher"
}

resource "google_pubsub_topic" "default" {
  name = "default-topic"
}

data "google_iam_policy" "topic_publisher" {
  binding {
    role = "roles/pubsub.publisher"

    members = [
      "serviceAccount:${google_service_account.publisher.email}",
    ]
  }
}

# THE FOLLOWING RESOURCE DOES NOT EXIST YET!
resource "google_resource_iam_policy" "default_topic_publisher" {
  target      = "${google_pubsub_topic.default.url}"
  policy_data = "${data.google_iam_policy.topic_publisher.policy_data}"
}

This would require to expose the target resource's url to make it possible to evaluate its type.

target and policy_data would be the only required arguments for all resources.

Additional optional or required argument may be added on a per resource type basis,
such as the optional authoritative and disable_project of the existing google_project_iam_policy resource. Edit: authoritative may be needed on all resource types.

The list of supported target resources would have to be documented unless there's a smart way to make all resources supported from the start.

Any thoughts?

[paddycarver]

Hey @pdecat! Thanks for opening the issue.

My concern is mostly at the design of this feature, for the moment--I haven't looked at the code yet. But I'm worried we're abstracting at the wrong level. In fairness, I may just be misunderstanding exactly what you're proposing here.

From what it sounds like, you've noticed that a lot of GCP resources have IAM policy properties, and are proposing an abstract resource for policies, which can then be applied to these different resources. A single implementation, basically.

My concern is that this abstracts at the wrong level. As an example, almost all of our resources have a project field, as well, denoting which project they belong to. In that instance, we abstracted at the implementation level with a getProject helper, not at the conceptual level with a resource that maps projects to resources. I think a similar approach is justified here.

The complicating factors of authoritative and disable_project also come into play; for some resources, they make sense. For example, authoritative was only added-and even then with much consternation-because getting IAM policies messed up on projects is so disastrous. But for other resource types, it's easier to fix IAM mistakes, so I probably wouldn't consider authoritative justified in those situations. So while IAM is the same across resources, the context matters to our UX, and I'm not sure we could provide a reasonable UX when abstracting across all resources. Also, if not all resources support IAM, how does the new resource know which resources it can be applied to?

The idea seems sound on the surface, but the more I dig into it, the more problems I'm running into. I think a better use of time and effort would be just defining a helper that will take care of the common case, and updating the resources that support IAM to use it. Then for special cases (like projects) we can use special logic, and we don't have to worry about resources that don't support IAM.

I appreciate the thought, but I'm going to close this out as wontfix. I think a good direction to go in, if you're missing something, is to open issues to add IAM properties to the resources that are missing them. If I've misunderstood the suggestion or you feel I've missed details in my analysis, please do feel free to continue commenting on the issue.

[pdecat]

All fair and sound points. Thanks for the comprehensive answer. Best regards, Patrick. Le mar. 4 avr. 2017 01:27, Paddy <[email protected]> a écrit :

…

Hey @pdecat <https://github.com/pdecat>! Thanks for opening the issue. My concern is mostly at the design of this feature, for the moment--I haven't looked at the code yet. But I'm worried we're abstracting at the wrong level. In fairness, I may just be misunderstanding exactly what you're proposing here. From what it sounds like, you've noticed that a lot of GCP resources have IAM policy properties, and are proposing an abstract resource for policies, which can then be applied to these different resources. A single implementation, basically. My concern is that this abstracts at the wrong level. As an example, almost all of our resources have a project field, as well, denoting which project they belong to. In that instance, we abstracted at the implementation level with a getProject helper, not at the conceptual level with a resource that maps projects to resources. I think a similar approach is justified here. The complicating factors of authoritative and disable_project also come into play; for some resources, they make sense. For example, authoritative was only added-and even then with much consternation-because getting IAM policies messed up on projects is so disastrous. But for other resource types, it's easier to fix IAM mistakes, so I probably wouldn't consider authoritative justified in those situations. So while IAM is the same across resources, the context matters to our UX, and I'm not sure we could provide a reasonable UX when abstracting across all resources. Also, if not all resources support IAM, how does the new resource know which resources it can be applied to? The idea seems sound on the surface, but the more I dig into it, the more problems I'm running into. I think a better use of time and effort would be just defining a helper that will take care of the common case, and updating the resources that support IAM to use it. Then for special cases (like projects) we can use special logic, and we don't have to worry about resources that don't support IAM. I appreciate the thought, but I'm going to close this out as wontfix. I think a good direction to go in, if you're missing something, is to open issues to add IAM properties to the resources that are missing them. If I've misunderstood the suggestion or you feel I've missed details in my analysis, please do feel free to continue commenting on the issue. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#13003 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AATcGoA_m5GykFjc9gipUd_KolOUodWGks5rsYBhgaJpZM4Mmnt-> .

[pdecat]

Opened #13602 for PubSub topics.

[ghost]

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.