diff --git a/google/iam_storage_bucket.go b/google/iam_storage_bucket.go index 88db8f76..be438e75 100644 --- a/google/iam_storage_bucket.go +++ b/google/iam_storage_bucket.go @@ -51,6 +51,11 @@ func (u *StorageBucketIamUpdater) SetResourceIamPolicy(policy *cloudresourcemana return errwrap.Wrapf(fmt.Sprintf("Invalid IAM policy for %s: {{err}}", u.DescribeResource()), err) } + ppolicy, err := u.Config.clientStorage.Buckets.GetIamPolicy(u.bucket).Do() + if err != nil { + return errwrap.Wrapf(fmt.Sprintf("Error setting IAM policy for %s: {{err}}", u.DescribeResource()), err) + } + storagePolicy.Etag = ppolicy.Etag _, err = u.Config.clientStorage.Buckets.SetIamPolicy(u.bucket, storagePolicy).Do() if err != nil { diff --git a/google/provider.go b/google/provider.go index e660c291..4966131b 100644 --- a/google/provider.go +++ b/google/provider.go @@ -215,6 +215,7 @@ func Provider() terraform.ResourceProvider { // google_storage_bucket_iam_policy resource. "google_storage_bucket_iam_binding": ResourceIamBinding(IamStorageBucketSchema, NewStorageBucketIamUpdater), "google_storage_bucket_iam_member": ResourceIamMember(IamStorageBucketSchema, NewStorageBucketIamUpdater), + "google_storage_bucket_iam_policy": ResourceIamPolicy(IamStorageBucketSchema, NewStorageBucketIamUpdater), "google_storage_bucket_object": resourceStorageBucketObject(), "google_storage_object_acl": resourceStorageObjectAcl(), "google_storage_default_object_acl": resourceStorageDefaultObjectAcl(), diff --git a/google/provider_test.go b/google/provider_test.go index cc469600..c00d3605 100644 --- a/google/provider_test.go +++ b/google/provider_test.go @@ -37,6 +37,10 @@ var orgEnvVars = []string{ "GOOGLE_ORG", } +var serviceAccountEnvVars = []string{ + "GOOGLE_SERVICE_ACCOUNT", +} + var orgTargetEnvVars = []string{ "GOOGLE_ORG_2", } @@ -168,6 +172,11 @@ func getTestBillingAccountFromEnv(t *testing.T) string { return multiEnvSearch(billingAccountEnvVars) } +func getTestServiceAccountFromEnv(t *testing.T) string { + skipIfEnvNotSet(t, serviceAccountEnvVars...) + return multiEnvSearch(serviceAccountEnvVars) +} + func multiEnvSearch(ks []string) string { for _, k := range ks { if v := os.Getenv(k); v != "" { diff --git a/google/resource_storage_bucket_iam_test.go b/google/resource_storage_bucket_iam_test.go index 2de7c7ed..12fa2b95 100644 --- a/google/resource_storage_bucket_iam_test.go +++ b/google/resource_storage_bucket_iam_test.go @@ -40,6 +40,36 @@ func TestAccStorageBucketIamBinding(t *testing.T) { }) } +func TestAccStorageBucketIamPolicy(t *testing.T) { + t.Parallel() + + bucket := acctest.RandomWithPrefix("tf-test") + account := acctest.RandomWithPrefix("tf-test") + serviceAcct := getTestServiceAccountFromEnv(t) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + // Test IAM Policy creation + Config: testAccStorageBucketIamPolicy_basic(bucket, account, serviceAcct), + Check: testAccCheckGoogleStorageBucketIam(bucket, "roles/storage.objectViewer", []string{ + fmt.Sprintf("serviceAccount:%s-1@%s.iam.gserviceaccount.com", account, getTestProjectFromEnv()), + }), + }, + { + // Test IAM Policy update + Config: testAccStorageBucketIamPolicy_update(bucket, account, serviceAcct), + Check: testAccCheckGoogleStorageBucketIam(bucket, "roles/storage.objectViewer", []string{ + fmt.Sprintf("serviceAccount:%s-1@%s.iam.gserviceaccount.com", account, getTestProjectFromEnv()), + fmt.Sprintf("serviceAccount:%s-2@%s.iam.gserviceaccount.com", account, getTestProjectFromEnv()), + }), + }, + }, + }) +} + func TestAccStorageBucketIamMember(t *testing.T) { t.Parallel() @@ -86,23 +116,104 @@ func testAccCheckGoogleStorageBucketIam(bucket, role string, members []string) r } } -func testAccStorageBucketIamBinding_basic(bucket, account string) string { +func testAccStorageBucketIamPolicy_update(bucket, account, serviceAcct string) string { return fmt.Sprintf(` resource "google_storage_bucket" "bucket" { - name = "%s" + name = "%s" } resource "google_service_account" "test-account-1" { - account_id = "%s-1" - display_name = "Iam Testing Account" + account_id = "%s-1" + display_name = "Iam Testing Account" +} + +resource "google_service_account" "test-account-2" { + account_id = "%s-2" + display_name = "Iam Testing Account" +} + + +data "google_iam_policy" "foo-policy" { + binding { + role = "roles/storage.objectViewer" + + members = [ + "serviceAccount:${google_service_account.test-account-1.email}", + "serviceAccount:${google_service_account.test-account-2.email}", + ] + } + + binding { + role = "roles/storage.admin" + members = [ + "serviceAccount:%s", + ] + } +} + +resource "google_storage_bucket_iam_policy" "bucket-binding" { + bucket = "${google_storage_bucket.bucket.name}" + policy_data = "${data.google_iam_policy.foo-policy.policy_data}" +} + +`, bucket, account, account, serviceAcct) +} + +func testAccStorageBucketIamPolicy_basic(bucket, account, serviceAcct string) string { + return fmt.Sprintf(` + +resource "google_storage_bucket" "bucket" { + name = "%s" +} + +resource "google_service_account" "test-account-1" { + account_id = "%s-1" + display_name = "Iam Testing Account" +} + + +data "google_iam_policy" "foo-policy" { + binding { + role = "roles/storage.objectViewer" + members = [ + "serviceAccount:${google_service_account.test-account-1.email}", + ] + } + + binding { + role = "roles/storage.admin" + members = [ + "serviceAccount:%s", + ] + } +} + +resource "google_storage_bucket_iam_policy" "bucket-binding" { + bucket = "${google_storage_bucket.bucket.name}" + policy_data = "${data.google_iam_policy.foo-policy.policy_data}" +} + + +`, bucket, account, serviceAcct) +} + +func testAccStorageBucketIamBinding_basic(bucket, account string) string { + return fmt.Sprintf(` +resource "google_storage_bucket" "bucket" { + name = "%s" +} + +resource "google_service_account" "test-account-1" { + account_id = "%s-1" + display_name = "Iam Testing Account" } resource "google_storage_bucket_iam_binding" "foo" { - bucket = "${google_storage_bucket.bucket.name}" - role = "roles/storage.objectViewer" - members = [ - "serviceAccount:${google_service_account.test-account-1.email}", - ] + bucket = "${google_storage_bucket.bucket.name}" + role = "roles/storage.objectViewer" + members = [ + "serviceAccount:${google_service_account.test-account-1.email}", + ] } `, bucket, account) } @@ -110,26 +221,26 @@ resource "google_storage_bucket_iam_binding" "foo" { func testAccStorageBucketIamBinding_update(bucket, account string) string { return fmt.Sprintf(` resource "google_storage_bucket" "bucket" { - name = "%s" + name = "%s" } resource "google_service_account" "test-account-1" { - account_id = "%s-1" - display_name = "Iam Testing Account" + account_id = "%s-1" + display_name = "Iam Testing Account" } resource "google_service_account" "test-account-2" { - account_id = "%s-2" - display_name = "Iam Testing Account" + account_id = "%s-2" + display_name = "Iam Testing Account" } resource "google_storage_bucket_iam_binding" "foo" { - bucket = "${google_storage_bucket.bucket.name}" - role = "roles/storage.objectViewer" - members = [ - "serviceAccount:${google_service_account.test-account-1.email}", - "serviceAccount:${google_service_account.test-account-2.email}", - ] + bucket = "${google_storage_bucket.bucket.name}" + role = "roles/storage.objectViewer" + members = [ + "serviceAccount:${google_service_account.test-account-1.email}", + "serviceAccount:${google_service_account.test-account-2.email}", + ] } `, bucket, account, account) } @@ -137,18 +248,18 @@ resource "google_storage_bucket_iam_binding" "foo" { func testAccStorageBucketIamMember_basic(bucket, account string) string { return fmt.Sprintf(` resource "google_storage_bucket" "bucket" { - name = "%s" + name = "%s" } resource "google_service_account" "test-account-1" { - account_id = "%s-1" - display_name = "Iam Testing Account" + account_id = "%s-1" + display_name = "Iam Testing Account" } resource "google_storage_bucket_iam_member" "foo" { - bucket = "${google_storage_bucket.bucket.name}" - role = "roles/storage.admin" - member = "serviceAccount:${google_service_account.test-account-1.email}" + bucket = "${google_storage_bucket.bucket.name}" + role = "roles/storage.admin" + member = "serviceAccount:${google_service_account.test-account-1.email}" } `, bucket, account) } diff --git a/website/docs/r/storage_bucket_iam.html.markdown b/website/docs/r/storage_bucket_iam.html.markdown index 561700e4..494ed5e1 100644 --- a/website/docs/r/storage_bucket_iam.html.markdown +++ b/website/docs/r/storage_bucket_iam.html.markdown @@ -8,10 +8,12 @@ description: |- # IAM policy for Google storage bucket -Two different resources help you manage your IAM policy for storage bucket. Each of these resources serves a different use case: +Three different resources help you manage your IAM policy for storage bucket. Each of these resources serves a different use case: * `google_storage_bucket_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 storage bucket are preserved. * `google_storage_bucket_iam_member`: Non-authoritative. Updates the IAM policy to grant a role to a new member. Other members for the role for the storage bucket are preserved. +* `google_storage_bucket_iam_policy`: Setting a policy removes all other permissions on the bucket, and if done incorrectly, there's a real chance you will lock yourself out of the bucket. If possible for your use case, using multiple google_storage_bucket_iam_binding resources will be much safer. See the usage example on how to work with policy correctly. + ~> **Note:** `google_storage_bucket_iam_binding` resources **can be** used in conjunction with `google_storage_bucket_iam_member` resources **only if** they do not grant privilege to the same role. @@ -38,6 +40,30 @@ resource "google_storage_bucket_iam_member" "member" { } ``` +## google\_storage\_bucket\_iam\_policy + +When applying a policy that does not include the roles listed below, you lose the default permissions which google adds to your bucket: +* `roles/storage.legacyBucketOwner` +* `roles/storage.legacyBucketReader` + +If this happens only an entity with `roles/storage.admin` privileges can repair this bucket's policies. It is recommended to include the above roles in policies to get the same behaviour as with the other two options. + +```hcl +data "google_iam_policy" "foo-policy" { + binding { + role = "roles/your-role" + + members = [ "group:yourgroup@example.com" ] + } +} + +resource "google_storage_bucket_iam_policy" "member" { + bucket = "your-bucket-name" + policy_data = "${data.google_iam_policy.foo-policy.policy_data}" +} +``` + + ## Argument Reference The following arguments are supported: @@ -61,4 +87,4 @@ The following arguments are supported: In addition to the arguments listed above, the following computed attributes are exported: -* `etag` - (Computed) The etag of the storage bucket's IAM policy. \ No newline at end of file +* `etag` - (Computed) The etag of the storage bucket's IAM policy.