-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
feat(HNS folders): add new resources to support HNS folders #12101
base: main
Are you sure you want to change the base?
Changes from all commits
ad3cefc
b522bcd
de6f2ea
376eead
0646523
454d6d1
8eb8111
69d2966
ddd5b51
90599d5
9efe5a8
9b5195d
736407c
ad19c6a
1d6b68c
d81ca9e
e7b1ff6
06c78d3
a142e22
4650cd6
6098a8f
2a6d49d
6f767b7
6384419
ccebe43
1030aa1
f8786d8
8a86c0a
f20a949
a156f38
220bd1f
0756c9e
5c3b7cd
caac4ff
453aed3
7267414
92bd5fc
772d7b2
73a5bc8
cce471f
547875d
e6bbaac
d026479
d2f7d28
9221d77
2fa4f7a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,88 @@ | ||
# Copyright 2024 Google Inc. | ||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||
# you may not use this file except in compliance with the License. | ||
# You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
--- | ||
name: 'Folder' | ||
kind: 'storage#folder' | ||
base_url: 'b/{{bucket}}/folders' | ||
self_link: 'b/{{bucket}}/folders/{{%name}}' | ||
id_format: '{{bucket}}/{{name}}' | ||
delete_url: 'b/{{bucket}}/folders/{{%name}}' | ||
create_url: 'b/{{bucket}}/folders' | ||
has_self_link: true | ||
timeouts: | ||
insert_minutes: 20 | ||
update_minutes: 20 | ||
delete_minutes: 20 | ||
exclude_sweeper: true | ||
import_format: | ||
- '{{bucket}}/folders/{{%name}}' | ||
- '{{bucket}}/{{%name}}' | ||
examples: | ||
- name: 'storage_folder_basic' | ||
primary_resource_id: 'folder' | ||
vars: | ||
bucket_name: 'my-bucket' | ||
ignore_read_extra: | ||
- 'force_destroy' | ||
description: | | ||
A Google Cloud Storage Folder. | ||
|
||
The Folder resource represents a folder in a Cloud Storage bucket with hierarchical namespace enabled | ||
references: | ||
guides: | ||
'Official Documentation': 'https://cloud.google.com/storage/docs/folders-overview' | ||
api: 'https://cloud.google.com/storage/docs/json_api/v1/folders' | ||
custom_code: | ||
custom_import: templates/terraform/custom_import/storage_folder.go.tmpl | ||
custom_update: templates/terraform/custom_update/storage_folder_update.go.tmpl | ||
custom_delete: templates/terraform/custom_delete/storage_folder_delete.go.tmpl | ||
virtual_fields: | ||
- name: 'force_destroy' | ||
description: | ||
If set to true, folder will be force destroyed. | ||
type: Boolean | ||
default_value: false | ||
parameters: | ||
- name: 'bucket' | ||
resource: 'Bucket' | ||
imports: 'name' | ||
description: 'The name of the bucket that contains the folder.' | ||
required: true | ||
immutable: true | ||
url_param_only: true | ||
- name: 'name' | ||
description: | | ||
The name of the folder expressed as a path. Must include | ||
trailing '/'. For example, `example_dir/example_dir2/`. | ||
required: true | ||
# The API returns values with trailing slashes, even if not | ||
# provided. Enforcing trailing slashes prevents diffs and ensures | ||
# consistent output. | ||
validation: | ||
regex: '/$' | ||
properties: | ||
- name: createTime | ||
type: String | ||
description: | | ||
The timestamp at which this folder was created. | ||
output: true | ||
- name: updateTime | ||
type: String | ||
description: | | ||
The timestamp at which this folder was most recently updated. | ||
output: true | ||
- name: metageneration | ||
type: String | ||
description: | | ||
The metadata generation of the folder. | ||
output: true |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,88 @@ | ||
bucket := d.Get("bucket").(string) | ||
name := d.Get("name").(string) | ||
|
||
var listError, deleteObjectError error | ||
for deleteObjectError == nil { | ||
res, err := config.NewStorageClient(userAgent).Objects.List(bucket).Prefix(name).Do() | ||
if err != nil { | ||
log.Printf("Error listing contents of bucket %s: %v", bucket, err) | ||
listError = err | ||
break | ||
} | ||
|
||
if len(res.Items) == 0 { | ||
break // 0 items, folder empty | ||
} | ||
|
||
if !d.Get("force_destroy").(bool) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would recommend moving this to the beginning, since you should be able to escape early There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we first check if the folder contains any objects, if no objects we delete the folder either force_destory set to true or false, if any objects found and force_destory set to true then we delete the objects otherwise break out of the loop, so breaking out of loop depends on count of objects in bucket There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, thanks for clarifying, I was mixing this up with how |
||
deleteErr := fmt.Errorf("Error trying to delete folder %s containing objects without force_destroy set to true", bucket) | ||
log.Printf("Error! %s : %s\n\n", bucket, deleteErr) | ||
return deleteErr | ||
} | ||
// GCS requires that a folder be empty (have no objects or object | ||
// versions) before it can be deleted. | ||
log.Printf("[DEBUG] GCS Folder attempting to forceDestroy\n\n") | ||
|
||
// Create a workerpool for parallel deletion of resources. In the | ||
// future, it would be great to expose Terraform's global parallelism | ||
// flag here, but that's currently reserved for core use. Testing | ||
// shows that NumCPUs-1 is the most performant on average networks. | ||
// | ||
// The challenge with making this user-configurable is that the | ||
// configuration would reside in the Terraform configuration file, | ||
// decreasing its portability. Ideally we'd want this to connect to | ||
// Terraform's top-level -parallelism flag, but that's not plumbed nor | ||
// is it scheduled to be plumbed to individual providers. | ||
wp := workerpool.New(runtime.NumCPU() - 1) | ||
|
||
for _, object := range res.Items { | ||
log.Printf("[DEBUG] Found %s", object.Name) | ||
object := object | ||
|
||
wp.Submit(func() { | ||
log.Printf("[TRACE] Attempting to delete %s", object.Name) | ||
if err := config.NewStorageClient(userAgent).Objects.Delete(bucket, object.Name).Generation(object.Generation).Do(); err != nil { | ||
deleteObjectError = err | ||
log.Printf("[ERR] Failed to delete storage object %s: %s", object.Name, err) | ||
} else { | ||
log.Printf("[TRACE] Successfully deleted %s", object.Name) | ||
} | ||
}) | ||
} | ||
|
||
// Wait for everything to finish. | ||
wp.StopWait() | ||
} | ||
|
||
if listError != nil { | ||
return fmt.Errorf("could not delete non-empty folder due to error when listing contents: %v", listError) | ||
} | ||
log.Printf("[DEBUG] force_destroy value: %#v", d.Get("force_destroy").(bool)) | ||
foldersList, err := config.NewStorageClient(userAgent).Folders.List(bucket).Prefix(name).Do() | ||
if err != nil { | ||
return err | ||
} | ||
if len(foldersList.Items) == 1 || d.Get("force_destroy").(bool) { | ||
log.Printf("[DEBUG] folder names to delete: %#v", name) | ||
items := foldersList.Items | ||
for i := len(items) - 1; i >= 0; i-- { | ||
err = transport_tpg.Retry(transport_tpg.RetryOptions{ | ||
RetryFunc: func() error { | ||
err = config.NewStorageClient(userAgent).Folders.Delete(bucket, items[i].Name).Do() | ||
return err | ||
}, | ||
Timeout: d.Timeout(schema.TimeoutDelete), | ||
ErrorRetryPredicates: []transport_tpg.RetryErrorPredicateFunc{transport_tpg.Is429RetryableQuotaError}, | ||
}) | ||
if err != nil { | ||
return err | ||
} | ||
} | ||
|
||
log.Printf("[DEBUG] Finished deleting Folder %q: %#v", d.Id(), name) | ||
} else { | ||
deleteErr := fmt.Errorf("Error trying to delete folder without force_destroy set to true") | ||
log.Printf("Error! %s : %s\n\n", name, deleteErr) | ||
return deleteErr | ||
} | ||
return nil |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
config := meta.(*transport_tpg.Config) | ||
if err := tpgresource.ParseImportId([]string{ | ||
"^(?P<bucket>[^/]+)/folders/(?P<name>.+)$", | ||
"^(?P<bucket>[^/]+)/(?P<name>.+)$", | ||
}, d, config); err != nil { | ||
return nil, err | ||
} | ||
|
||
// Replace import id for the resource id | ||
id, err := tpgresource.ReplaceVars(d, config, "{{"{{"}}bucket{{"}}"}}/{{"{{"}}name{{"}}"}}") | ||
if err != nil { | ||
return nil, fmt.Errorf("Error constructing id: %s", err) | ||
} | ||
d.SetId(id) | ||
if err := d.Set("force_destroy", false); err != nil { | ||
return nil, fmt.Errorf("Error setting force_destroy: %s", err) | ||
} | ||
|
||
return []*schema.ResourceData{d}, nil |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
_ = config | ||
// we can only get here if force_destroy was updated | ||
if d.Get("force_destroy") != nil { | ||
if err := d.Set("force_destroy", d.Get("force_destroy")); err != nil { | ||
return fmt.Errorf("Error updating force_destroy: %s", err) | ||
} | ||
} | ||
Comment on lines
+3
to
+7
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NIt: Since you're already adding this, you may want to go ahead and add the function that checks if this is the only field being changed (in case other fields are added later). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for now this should be fine |
||
|
||
// all other fields are immutable, don't do anything else | ||
return nil |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
resource "google_storage_bucket" "bucket" { | ||
name = "{{index $.Vars "bucket_name"}}" | ||
location = "EU" | ||
uniform_bucket_level_access = true | ||
hierarchical_namespace { | ||
enabled = true | ||
} | ||
force_destroy = true | ||
} | ||
|
||
resource "google_storage_folder" "{{$.PrimaryResourceId}}" { | ||
bucket = google_storage_bucket.bucket.name | ||
name = "parent-folder/" | ||
force_destroy = true | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you may want some more rigorous validation here, mainly because it is used on the path. Do you know what the user will see if they provide a name like
/example_dir/
? If it results in a cryptic error, then validation might be better, and then you may want to do other things like ensure alphanumeric characters or similar.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if the path starts with /, the API validates the request body and rejects it as it cannot be started with /
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, but what I'm asking about is how exactly it responds when that value is rejected. For example, a 404 would be difficult for a user to understand when they actually need to remove a leading slash here.