Skip to content

Commit

Permalink
Deprecate bucket policy field in google_storage_bucket (#3916) (#7143)
Browse files Browse the repository at this point in the history
Signed-off-by: Modular Magician <magic-modules@google.com>
  • Loading branch information
modular-magician committed Aug 28, 2020
1 parent ca85af2 commit 4a46e8e
Show file tree
Hide file tree
Showing 4 changed files with 91 additions and 12 deletions.
3 changes: 3 additions & 0 deletions .changelog/3916.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:deprecation
storage: deprecated `bucket_policy_only` field in `google_storage_bucket` in favour of `uniform_bucket_level_access`
```
56 changes: 46 additions & 10 deletions google/resource_storage_bucket.go
Original file line number Diff line number Diff line change
Expand Up @@ -325,10 +325,19 @@ func resourceStorageBucket() *schema.Resource {
Description: `The bucket's Access & Storage Logs configuration.`,
},
"bucket_policy_only": {
Type: schema.TypeBool,
Optional: true,
Computed: true,
Description: `Enables Bucket Policy Only access to a bucket.`,
Type: schema.TypeBool,
Optional: true,
Computed: true,
Description: `Enables Bucket Policy Only access to a bucket.`,
Deprecated: `Please use the uniform_bucket_level_access as this field has been renamed by Google.`,
ConflictsWith: []string{"uniform_bucket_level_access"},
},
"uniform_bucket_level_access": {
Type: schema.TypeBool,
Optional: true,
Computed: true,
Description: `Enables uniform bucket-level access on a bucket.`,
ConflictsWith: []string{"bucket_policy_only"},
},
},
}
Expand Down Expand Up @@ -550,6 +559,9 @@ func resourceStorageBucketUpdate(d *schema.ResourceData, meta interface{}) error
if d.HasChange("bucket_policy_only") {
sb.IamConfiguration = expandIamConfiguration(d)
}
if d.HasChange("uniform_bucket_level_access") {
sb.IamConfiguration = expandIamConfiguration(d)
}

res, err := config.clientStorage.Buckets.Patch(d.Get("name").(string), sb).Do()

Expand Down Expand Up @@ -632,10 +644,13 @@ func resourceStorageBucketRead(d *schema.ResourceData, meta interface{}) error {
d.Set("website", flattenBucketWebsite(res.Website))
d.Set("retention_policy", flattenBucketRetentionPolicy(res.RetentionPolicy))

if res.IamConfiguration != nil && res.IamConfiguration.BucketPolicyOnly != nil {
d.Set("bucket_policy_only", res.IamConfiguration.BucketPolicyOnly.Enabled)
// Delete the bucket_policy_only field in the next major version of the provider.
if res.IamConfiguration != nil && res.IamConfiguration.UniformBucketLevelAccess != nil {
d.Set("uniform_bucket_level_access", res.IamConfiguration.UniformBucketLevelAccess.Enabled)
d.Set("bucket_policy_only", res.IamConfiguration.UniformBucketLevelAccess.Enabled)
} else {
d.Set("bucket_policy_only", false)
d.Set("uniform_bucket_level_access", false)
}

if res.Billing == nil {
Expand Down Expand Up @@ -995,20 +1010,41 @@ func expandBucketWebsite(v interface{}) *storage.BucketWebsite {
if v := website["main_page_suffix"]; v != "" {
w.MainPageSuffix = v.(string)
}

return w
}

// remove this on next major release of the provider.
func expandIamConfiguration(d *schema.ResourceData) *storage.BucketIamConfiguration {
// We are checking for a change because the last else block is only executed on Create.
enabled := false
if d.HasChange("bucket_policy_only") {
enabled = d.Get("bucket_policy_only").(bool)
} else if d.HasChange("uniform_bucket_level_access") {
enabled = d.Get("uniform_bucket_level_access").(bool)
} else {
enabled = d.Get("bucket_policy_only").(bool) || d.Get("uniform_bucket_level_access").(bool)
}

return &storage.BucketIamConfiguration{
ForceSendFields: []string{"BucketPolicyOnly"},
BucketPolicyOnly: &storage.BucketIamConfigurationBucketPolicyOnly{
Enabled: d.Get("bucket_policy_only").(bool),
ForceSendFields: []string{"UniformBucketLevelAccess"},
UniformBucketLevelAccess: &storage.BucketIamConfigurationUniformBucketLevelAccess{
Enabled: enabled,
ForceSendFields: []string{"Enabled"},
},
}
}

// Uncomment once the previous function is removed.
// func expandIamConfiguration(d *schema.ResourceData) *storage.BucketIamConfiguration {
// return &storage.BucketIamConfiguration{
// ForceSendFields: []string{"UniformBucketLevelAccess"},
// UniformBucketLevelAccess: &storage.BucketIamConfigurationUniformBucketLevelAccess{
// Enabled: d.Get("uniform_bucket_level_access").(bool),
// ForceSendFields: []string{"Enabled"},
// },
// }
// }

func expandStorageBucketLifecycle(v interface{}) (*storage.BucketLifecycle, error) {
if v == nil {
return &storage.BucketLifecycle{
Expand Down
38 changes: 38 additions & 0 deletions google/resource_storage_bucket_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -762,6 +762,35 @@ func TestAccStorageBucket_bucketPolicyOnly(t *testing.T) {
})
}

func TestAccStorageBucket_uniformBucketAccessOnly(t *testing.T) {
t.Parallel()

bucketName := fmt.Sprintf("tf-test-acl-bucket-%d", randInt(t))

vcrTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
Steps: []resource.TestStep{
{
Config: testAccStorageBucket_uniformBucketAccessOnly(bucketName, true),
},
{
ResourceName: "google_storage_bucket.bucket",
ImportState: true,
ImportStateVerify: true,
},
{
Config: testAccStorageBucket_uniformBucketAccessOnly(bucketName, false),
},
{
ResourceName: "google_storage_bucket.bucket",
ImportState: true,
ImportStateVerify: true,
},
},
})
}

func TestAccStorageBucket_labels(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -1375,6 +1404,15 @@ resource "google_storage_bucket" "bucket" {
`, bucketName, enabled)
}

func testAccStorageBucket_uniformBucketAccessOnly(bucketName string, enabled bool) string {
return fmt.Sprintf(`
resource "google_storage_bucket" "bucket" {
name = "%s"
uniform_bucket_level_access = %t
}
`, bucketName, enabled)
}

func testAccStorageBucket_encryption(context map[string]interface{}) string {
return Nprintf(`
resource "google_project" "acceptance" {
Expand Down
6 changes: 4 additions & 2 deletions website/docs/r/storage_bucket.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ resource "google_storage_bucket" "static-site" {
location = "EU"
force_destroy = true
bucket_policy_only = true
uniform_bucket_level_access = true
website {
main_page_suffix = "index.html"
Expand Down Expand Up @@ -101,7 +101,9 @@ The following arguments are supported:

* `requester_pays` - (Optional, Default: false) Enables [Requester Pays](https://cloud.google.com/storage/docs/requester-pays) on a storage bucket.

* `bucket_policy_only` - (Optional, Default: false) Enables [Bucket Policy Only](https://cloud.google.com/storage/docs/bucket-policy-only) access to a bucket.
* `bucket_policy_only` - (Deprecated, Default: false) Enables [Bucket Policy Only](https://cloud.google.com/storage/docs/bucket-policy-only) access to a bucket. This field will be removed in the next major release of the provider.

* `uniform_bucket_level_access` - (Optional, Default: false) Enables [Uniform bucket-level access](https://cloud.google.com/storage/docs/uniform-bucket-level-access) access to a bucket.

The `lifecycle_rule` block supports:

Expand Down

0 comments on commit 4a46e8e

Please sign in to comment.