#936 adding support for AuditLogConfigs in IAM Policies

[adamenger]

Hi,

We have a use case for accessing AuditConfigLogs at Etsy. This PR addresses #936 and adds support for AuditLogConfigs through a google_iam_policy data block.

See below for an example:

resource "google_project_iam_policy" "project" {
  project     = "${google_project.project.id}"
  policy_data = "${data.google_iam_policy.audit_config.policy_data}"
}

data "google_iam_policy" "audit_config" {
  binding {
    role = "roles/owner"

    members = [
      "user:[email protected]",
    ]
  }

  audit_config {
    service = "cloudkms.googleapis.com"
    audit_log_configs = [
      {
        log_type = "DATA_READ",
        exempted_members = [
          "user:[email protected],
        ]
      },
    ]
  }

}

Please let me know how I can improve this PR to make it easier to merge.

Best,

Adam

[endemics]

Hi,

@danawillow @paddycarver, any chance a maintainer could review this PR?

I would love to see this feature shipping in the next version of the provider.

Cheers,
Gildas

[danawillow]

@paddycarver, I remember you had some ideas about audit configs being a separate resource instead (search our slack convo for "audit" to find it). do you think you'll be able to get to this review this week?

[adamenger]

Oh that's an interesting idea! I'd be happy to take a crack at that if that's the path the maintainers decide is best.

Let me know.

[paddycarver]

Ah, ok, yes, I apologise for the delay here, but I believe I remember what @danawillow is referencing.

So how we manage IAM resources in the GCP provider is a bit weird. That's because we offer three separate levels of control over IAM, as explained on https://www.terraform.io/docs/providers/google/r/google_project_iam.html

Essentially, you can control a policy entirely, in which case the entire policy for the resource under management is owned by Terraform. Anything added outside of Terraform is considered drift, and Terraform removes it on the next run. This can get you locked out of your own projects, for example, and can remove access the GCP automatically adds for other resources in your account, but gives you the most locked-down option.

You can also control a specific binding on a policy, ensuring that certain users have a role, and nobody else does. If anyone else does, Terraform considers it drift, and corrects it on the next run. This is less foot-gun-y than the policy control option, because you're only managing one role, so you can tiptoe around the changes that GCP automatically makes to your policy, but it's also less locked down.

Finally, you can control a specific member on a policy, ensuring that they have a specific role. This version is the least locked down, and is only concerned with making sure that user has the role; it won't detect any drift except the user not having that role. It doesn't ensure the user only has that role, etc.

The way this PR is implemented, from what I can tell, only gives the first way of working with IAM support for audit log configs. Which is fine, and a good place to start, but is likely not going to be sufficient, as people using the binding and member options are likely going to want this feature.

My proposal was to:

1. Add the feature to the IAM level, as you have.
2. Add a new resource, similar to member and binding, that only controls the audit log configs. This would allow Terraform to just own the audit log config, and not interfere with any of the permissions managed outside of Terraform. It could, therefore, be used in conjunction with the binding and member versions of the IAM resources.

What do you think about this? Do you have any thoughts on this approach? Is that work you'd like to take on, or would you prefer we treat this PR as only addressing the first point above, and a separate PR be opened to address the second point?

[paddycarver]

Does the order of the exempted members matter? Would you ever want to reference them using interpolation? If the answer to both is "no", this would probably be better off as a Set.

[paddycarver]

As a general rule of thumb, camelCase, not snake_case, is preferred. :)

[adamenger]

Thank you! I'll get this fixed up.

[morgante]

@danawillow @paddycarver @adamenger Could we get someone to take a look at this? It'd be very useful to configure this in Terraform.

[paddycarver]

I'll take over this PR and pick it up on Monday. Can't promise an ETA, but I'll have an update on it next week.

[paddycarver]

I'm late on my update to this, and it's very dissatisfying! Sorry. The paddy_audit_log_configs branch on this repo has my updated code from this PR: fixing some style issues I mentioned, adding support for AuditConfigs in our diff suppress function, and adding support to our test checks for AuditConfigs. The only step remaining is to add tests that exercise AuditConfigs and then get PR review. My day today is going to be primarily bug fixes, test fixes, and issue triage, but I'll try to add tests tonight and update this PR tomorrow with a reference to the new PR. Sorry for the delay here, there's turning out to be a bit more code than I hoped.

[morgante]

@paddycarver Gentle ping. Would love to see this added soon.

[nat-henderson]

Ah, sorry, my mistake - I thought this was stale, but now it seems it's actually just waiting on something to be implemented on the GCP side. I might have missed a side conversation - I'll see what I can find out and report back here.

[stepanstipl]

@ndmckinley can you please elaborate on what's this waiting for on the GCP side?

Looks to me this PR is missing tests - is this something @paddycarver is working on? Would there be interest in some help with the tests?

[nat-henderson]

Yes, Paddy is on it, and you can follow along at GoogleCloudPlatform/magic-modules#1066. :)

[paddycarver]

My PR has been merged, and audit configs should be configurable when setting an entire project's IAM policy (e.g., using google_project_iam_policy) in the 2.0.0 release.

I'm currently working to add a google_project_iam_audit_config resource, which will allow configuring just the audit config for a project, without needing to overwrite all your configured bindings. I'm hoping to open a PR for that shortly.

[paddycarver]

GoogleCloudPlatform/magic-modules#1094 brings that fine-grained support, which I think should be enough support for the moment, until we can get a better understanding of how people are using this and want to use it.

[paddycarver]

Both PRs have been merged, so as of 2.0.0, you should be able to manage audit configs as part of managing an entire project's policy, or you should be able to manage just the audit configs for a project.

I'm going to close this now, as the functionality has been implemented.

[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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!