From d96138bf94cf1fad01e337c615f13169fdc72c0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rene=CC=81=20Kroon?= Date: Tue, 3 Apr 2018 10:35:38 +0200 Subject: [PATCH] #843: Add policy support to storage buckets --- google/iam_storage_bucket.go | 5 + google/provider.go | 1 + google/resource_storage_bucket_iam_test.go | 96 +++++++++++++++++++ .../docs/r/storage_bucket_iam.html.markdown | 20 +++- 4 files changed, 121 insertions(+), 1 deletion(-) diff --git a/google/iam_storage_bucket.go b/google/iam_storage_bucket.go index 88db8f76520..be438e75130 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 9db7c7bb44e..bbb60f74c2b 100644 --- a/google/provider.go +++ b/google/provider.go @@ -207,6 +207,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/resource_storage_bucket_iam_test.go b/google/resource_storage_bucket_iam_test.go index 2de7c7edc6e..05d8f15123c 100644 --- a/google/resource_storage_bucket_iam_test.go +++ b/google/resource_storage_bucket_iam_test.go @@ -40,6 +40,35 @@ func TestAccStorageBucketIamBinding(t *testing.T) { }) } +func TestAccStorageBucketIamPolicy(t *testing.T) { + t.Parallel() + + bucket := acctest.RandomWithPrefix("tf-test") + account := acctest.RandomWithPrefix("tf-test") + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + // Test IAM Policy creation + Config: testAccStorageBucketIamPolicy_basic(bucket, account), + 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), + 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,6 +115,73 @@ func testAccCheckGoogleStorageBucketIam(bucket, role string, members []string) r } } +func testAccStorageBucketIamPolicy_update(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_service_account" "test-account-2" { + account_id = "%s-2" + display_name = "Iam Testing Account" +} + + +data "google_iam_policy" "fooPolicy" { + binding { + role = "roles/storage.objectViewer" + + members = [ + "serviceAccount:${google_service_account.test-account-1.email}", + "serviceAccount:${google_service_account.test-account-2.email}" + ] + } +} + +resource "google_storage_bucket_iam_policy" "bucketBinding" { + bucket = "${google_storage_bucket.bucket.name}" + policy_data = "${data.google_iam_policy.fooPolicy.policy_data}" +} + +`, bucket, account, account) +} + +func testAccStorageBucketIamPolicy_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" +} + + +data "google_iam_policy" "fooPolicy" { + binding { + role = "roles/storage.objectViewer" + members = [ + "serviceAccount:${google_service_account.test-account-1.email}", + ] + } +} + +resource "google_storage_bucket_iam_policy" "bucketBinding" { + bucket = "${google_storage_bucket.bucket.name}" + policy_data = "${data.google_iam_policy.fooPolicy.policy_data}" +} + + +`, bucket, account) +} + func testAccStorageBucketIamBinding_basic(bucket, account string) string { return fmt.Sprintf(` resource "google_storage_bucket" "bucket" { diff --git a/website/docs/r/storage_bucket_iam.html.markdown b/website/docs/r/storage_bucket_iam.html.markdown index 561700e4a06..c9391dd0298 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,22 @@ 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 +resource "google_storage_bucket_iam_policy" "member" { + bucket = "your-bucket-name" + policy_data = "policy_data" +} +``` + + ## Argument Reference The following arguments are supported: