GoogleCloudPlatform / cloud-foundation-fabric

End-to-end modular samples and landing zones toolkit for Terraform on GCP.
Apache License 2.0
1.48k stars 848 forks source link

IAM permissions regarding keys should be fully authoritative. #2055

Closed michaeldeman closed 7 months ago

michaeldeman commented 7 months ago

Describe the bug IAM provisioning for the keys themselves does not follow the 'authoritative' pattern if a specific role is not already included in configuration terraform configuration. Which can result in additional privileges being granted and retained outside of terraform related procedures.

This also does not result any awareness by terraform about any drift of the deployed infrastructure in regards to what is expected and what is actually out in the real world regarding permissions.

Environment

% terraform --version
Terraform v1.7.2
on darwin_arm64
+ provider registry.terraform.io/hashicorp/google v5.13.0
+ provider registry.terraform.io/hashicorp/google-beta v5.13.0
+ provider registry.terraform.io/hashicorp/time v0.10.0
output from `git rev-parse --short HEAD`
This may be a few weeks stale as it against a local downstream from GitHub that I only sync up as needed.
4c68c016

To Reproduce Use this module to manage IAM roles/permissions for a principal against a key within a keyring using either parameter iam or iam_bindings to manage the permissions for that key. Manually add an additional IAM role to that principal.

For example:

  1. In terraform, assign only something like below via variable 'iam'. "roles/cloudkms.cryptoKeyEncrypterDecrypter" = [ such as "serviceAccount:xyz..."]
  2. Apply that.
  3. In the Google Cloud Console, grant that another different role (any will do) such as "Cloud KMS Admin" on the key.
  4. Run an apply again and note in the cloud console that the role that was manually added still exists and terraform reported no change of state.

Expected behavior I would expect that assigning IAM permissions via either variable iam oriam_bindings would be authoritative as described in the documentation and remove any and all IAM permissions granted via the cloud console (or by other means) in between terraform plans/apply.

Result There are no terraform errors. The manually added IAM permission remains.

Additional context Clarify in documentation that the use of iam and iam_bindings is not authoritative over all IAM permissions over the key sub and might be misleading. In particular, any permissions granted via other mechanisms such as the cloud console will not be removed unless the role itself is explicitly defined in terraform.

For backwards compatibility, possibly implement a new variable like iam_policy_fully_authoritative relying on the terraform 'google_kms_crypto_key_iam_policy' resource which will remove additional permissions manually added in the cloud console irrespective of whether the specific roles are defined in terraform.

google_kms_crypto_key_iam_policy: Authoritative. Sets the IAM policy for the crypto key and replaces any existing policy already attached. google_kms_crypto_key_iam_binding: Authoritative for a given role. Updates the IAM policy to grant a role to a list of members. Other roles within the IAM policy for the crypto key are preserved.

ludoo commented 7 months ago

the KMS module iam and iam_bindings variable use the authoritative resource. Please provide repro steps, but even if it's confirmed it's nothing that we can fix from our side since we're already consuming the correct provider resources.

I am closing this, feel free to reopen with actual repro code and steps.