hashicorp / terraform-provider-google

Terraform Provider for Google Cloud Platform
https://registry.terraform.io/providers/hashicorp/google/latest/docs
Mozilla Public License 2.0
2.33k stars 1.73k forks source link

Handle 3rd-party identities in IAM correctly #7852

Closed wvanderdeijl closed 3 years ago

wvanderdeijl commented 3 years ago

Community Note

Terraform Version

% terraform -v
Terraform v0.13.5

google-beta v3.48.0

Affected Resource(s)

Terraform Configuration Files

terraform {
  required_providers {
    google = {
      source = "hashicorp/google-beta"
    }
  }
}

provider "google" {
  project     = "eurotransplant-dummya"
  region      = "europe-west1"
}

resource "google_iam_workload_identity_pool" "example" {
  workload_identity_pool_id = "example-pool"
}

resource "google_service_account" "sa" {
  account_id   = "my-service-account"
}

resource "google_service_account_iam_member" "impersonate" {
  service_account_id = google_service_account.sa.name
  role               = "roles/iam.workloadIdentityUser"
  member             = "principalSet://iam.googleapis.com/${google_iam_workload_identity_pool.example.name}/attribute.aws_role/arn:aws:sts::999999999999:assumed-role/some-eu-central-1-lambdarole"
}

Debug Output

https://gist.github.com/wvanderdeijl/8095c9f208e362db2d8b8ba031512535

Expected Behavior

The role roles/iam.workloadIdentityUser on the service account should have been granted to the principalSet (an identity provided by the new Google Workload Identity Federation feature).

Actual Behavior

Error: Error applying IAM policy for service account 
'projects/cosmic-adapter-296112/serviceAccounts/my-service-account@cosmic-adapter-296112.iam.gserviceaccount.com': 
Error setting IAM policy for service account 
'projects/cosmic-adapter-296112/serviceAccounts/my-service-account@cosmic-adapter-296112.iam.gserviceaccount.com':
googleapi: Error 400: The member
principalSet://iam.googleapis.com/projects/1066737951711/locations/global/workloadidentitypools/example-pool/attribute.aws_role/arn:aws:sts::999999999999:assumed-role/some-eu-central-1-lambdarole
is of an unknown type. Please set a valid type prefix for the member., badRequest

createIamBindingsMap in google/iam.go seems to split a member in its "type" (the part before :) and the "value". It then converts the value to lowercase. See https://github.com/hashicorp/terraform-provider-google/blob/master/google/iam.go#L232

This converts my request to add principalSet://iam.googleapis.com/projects/1066737951711/locations/global/workloadIdentityPools/example-pool/attribute.aws_role/arn:aws:sts::999999999999:assumed-role/some-eu-central-1-lambdaRole into principalSet://iam.googleapis.com/projects/1066737951711/locations/global/workloadidentitypools/example-pool/attribute.aws_role/arn:aws:sts::999999999999:assumed-role/some-eu-central-1-lambdarole

This then throws a 400 when applying as the value of a principalSet is case sensitive.

The key element is workloadIdentityPools being transformed to workloadidentitypools, although the transformation from lambdaRole to lambdarole will not lead to a deployment issue it will lead to a runtime issue as the AWS lambaRole will not be granted access.

Steps to Reproduce

  1. terraform apply

Important Factoids

I can use the gcloud cli to demonstrate that the principalSet value is case sensitive:

% gcloud iam service-accounts add-iam-policy-binding my-service-account@cosmic-adapter-296112.iam.gserviceaccount.com \
    --project cosmic-adapter-296112 \
    --role roles/iam.workloadIdentityUser \
    --member "principalSet://iam.googleapis.com/projects/1066737951711/locations/global/workloadidentitypools/example-pool/attribute.aws_role/arn:aws:sts::999999999999:assumed-role/some-eu-central-1-lambdarole"

ERROR: Policy modification failed. For a binding with condition, run "gcloud alpha iam policies lint-condition" to identify issues in condition.
ERROR: (gcloud.iam.service-accounts.add-iam-policy-binding) INVALID_ARGUMENT: The member principalSet://iam.googleapis.com/projects/1066737951711/locations/global/workloadidentitypools/example-pool/attribute.aws_role/arn:aws:sts::999999999999:assumed-role/some-eu-central-1-lambdarole is of an unknown type. Please set a valid type prefix for the member.

% gcloud iam service-accounts add-iam-policy-binding my-service-account@cosmic-adapter-296112.iam.gserviceaccount.com \
    --project cosmic-adapter-296112 \
    --role roles/iam.workloadIdentityUser \
    --member "principalSet://iam.googleapis.com/projects/1066737951711/locations/global/workloadIdentityPools/example-pool/attribute.aws_role/arn:aws:sts::999999999999:assumed-role/some-eu-central-1-lambdaRole"

Updated IAM policy for serviceAccount [my-service-account@cosmic-adapter-296112.iam.gserviceaccount.com].
bindings:
- members:
  - principalSet://iam.googleapis.com/projects/1066737951711/locations/global/workloadIdentityPools/example-pool/attribute.aws_role/arn:aws:sts::999999999999:assumed-role/some-eu-central-1-lambdaRole
  role: roles/iam.workloadIdentityUser
etag: BwW0dRngveE=
version: 1

References

Documentation on Workload Identity Federation from AWS including service account impersonation: https://cloud.google.com/iam/docs/access-resources-aws

There have been more issues and PR's which led to the introduction of case-insensitivity and thus lowercasing the value:

edwardmedia commented 3 years ago

This appears to be a legit case

wvanderdeijl commented 3 years ago

I have been working on a number of PR’s over the last couple of weeks to introduce Workload Identity Federation resources and datasources to magic modules/terraform. Now that those are merged we were looking forward to using them in our stack, but we now run into this showstopper that we cannot use the federated identities 😢

Fixing this last issue myself will be a real challenge as I have very limited Go experience. @rileykarson is this something you guys will fix or would you otherwise have some guidance where and how I should fix this, so I can attempt a PR

rileykarson commented 3 years ago

I intend to get to this! It's taken a little longer to find the time than I expected, though.

wvanderdeijl commented 3 years ago

@rileykarson I don’t want to be a PITA, but is there anything we can do to help? This issue is preventing us to roll our workload identity federation, which is a great new feature.

rileykarson commented 3 years ago

I had the start of a solution to this which corrected createIamBindingsMap but calling strings.ToLower throughout our IAM resources is fairly pervasive. It's going to be a more involved fix than originally anticipated.

I've relabelled using the persistent-bug label which means we'll treat this like an enhancement, triaging and assigning this to a sprint.

stuagano commented 3 years ago

also ran into

ghost commented 3 years ago

Are there any workarounds for this (which don't involve null resources with local-exec of "gcloud iam ...")?

rileykarson commented 3 years ago

None I'm aware of, the issue is within the resource code so there are no user-side workarounds until we can make a provider fix.

ghost commented 3 years ago

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 👉 hashibot-feedback@hashicorp.com. Thanks!