Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate bucket policy field in google_storage_bucket #7143

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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