feat(HNS folders): add new resources to support HNS folders #12101

gurusai-voleti · 2024-10-23T11:30:51Z

Release Note Template for Downstream PRs (will be copied)

`google_storage_new_resource`

modular-magician · 2024-10-23T11:40:57Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 1 file changed, 354 insertions(+))
google-beta provider: Diff ( 1 file changed, 354 insertions(+))

Errors

google provider:

The diff processor failed to build. This is usually due to the downstream provider failing to compile.

google-beta provider:

The diff processor failed to build. This is usually due to the downstream provider failing to compile.

modular-magician · 2024-10-23T11:41:17Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 1 file changed, 354 insertions(+))
google-beta provider: Diff ( 1 file changed, 354 insertions(+))

Errors

google provider:

The diff processor failed to build. This is usually due to the downstream provider failing to compile.

google-beta provider:

The diff processor failed to build. This is usually due to the downstream provider failing to compile.

… into hns_folders

modular-magician · 2024-10-23T11:43:07Z

Tests analytics

Total tests: 0
Passed tests: 0
Skipped tests: 0
Affected tests: 0

Click here to see the affected service packages

🔴 Errors occurred during REPLAYING mode. Please fix them to complete your PR.

View the build log

modular-magician · 2024-10-23T11:43:24Z

Tests analytics

Total tests: 0
Passed tests: 0
Skipped tests: 0
Affected tests: 0

Click here to see the affected service packages

🔴 Errors occurred during REPLAYING mode. Please fix them to complete your PR.

View the build log

modular-magician · 2024-10-23T11:53:17Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 1 file changed, 353 insertions(+))
google-beta provider: Diff ( 1 file changed, 353 insertions(+))

modular-magician · 2024-10-23T12:01:51Z

Tests analytics

Total tests: 109
Passed tests: 100
Skipped tests: 9
Affected tests: 0

Click here to see the affected service packages

🟢 All tests passed!

View the build log

modular-magician · 2024-10-30T05:28:24Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 5 files changed, 692 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 5 files changed, 692 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 89 insertions(+))
Open in Cloud Shell: Diff ( 4 files changed, 116 insertions(+))

modular-magician · 2024-10-30T05:37:06Z

Tests analytics

Total tests: 111
Passed tests: 100
Skipped tests: 9
Affected tests: 2

Click here to see the affected service packages

Action taken

Found 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

Get to know how VCR tests work

modular-magician · 2024-10-30T05:37:25Z

🔴 Several tests terminated during RECORDING mode.

🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR.

View the build log or the debug log for each test

modular-magician · 2024-10-30T05:58:25Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 5 files changed, 692 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 5 files changed, 692 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 89 insertions(+))
Open in Cloud Shell: Diff ( 4 files changed, 116 insertions(+))

modular-magician · 2024-10-30T06:07:08Z

Tests analytics

Total tests: 111
Passed tests: 100
Skipped tests: 9
Affected tests: 2

Click here to see the affected service packages

Action taken

Found 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

Get to know how VCR tests work

modular-magician · 2024-10-30T06:07:54Z

🟢 Tests passed during RECORDING mode:
TestAccStorageFolder_storageFolderBasicForceDestroy [Debug log]

🟢 No issues found for passed tests after REPLAYING rerun.

🔴 Tests failed during RECORDING mode:
TestAccStorageFolder_storageFolderBasicExample [Error message] [Debug log]

🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR.

View the build log or the debug log for each test

modular-magician · 2024-10-30T08:37:24Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 4 files changed, 552 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 4 files changed, 552 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 89 insertions(+))

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_storage_folder (0 total tests)
Please add an acceptance test which includes these fields. The test should include the following:

resource "google_storage_folder" "primary" {
  bucket        = # value needed
  force_destroy = # value needed
  name          = # value needed
  recursive     = # value needed
}

modular-magician · 2024-10-30T08:46:39Z

Tests analytics

Total tests: 110
Passed tests: 101
Skipped tests: 9
Affected tests: 0

Click here to see the affected service packages

🟢 All tests passed!

View the build log

modular-magician · 2024-10-30T08:55:48Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 4 files changed, 552 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 4 files changed, 552 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 89 insertions(+))

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_storage_folder (0 total tests)
Please add an acceptance test which includes these fields. The test should include the following:

resource "google_storage_folder" "primary" {
  bucket        = # value needed
  force_destroy = # value needed
  name          = # value needed
  recursive     = # value needed
}

modular-magician · 2024-10-30T09:04:08Z

Tests analytics

Total tests: 110
Passed tests: 101
Skipped tests: 9
Affected tests: 0

Click here to see the affected service packages

🟢 All tests passed!

View the build log

modular-magician · 2024-12-09T12:27:43Z

Tests analytics

Total tests: 115
Passed tests: 106
Skipped tests: 9
Affected tests: 0

Click here to see the affected service packages

🟢 All tests passed!

View the build log

modular-magician · 2024-12-10T05:55:17Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 6 files changed, 818 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 6 files changed, 818 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 69 insertions(+))
Open in Cloud Shell: Diff ( 4 files changed, 116 insertions(+))

modular-magician · 2024-12-10T05:55:45Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 6 files changed, 818 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 6 files changed, 818 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 69 insertions(+))
Open in Cloud Shell: Diff ( 4 files changed, 116 insertions(+))

modular-magician · 2024-12-10T06:03:52Z

Tests analytics

Total tests: 115
Passed tests: 106
Skipped tests: 9
Affected tests: 0

Click here to see the affected service packages

🟢 All tests passed!

View the build log

modular-magician · 2024-12-10T06:04:08Z

Tests analytics

Total tests: 115
Passed tests: 106
Skipped tests: 9
Affected tests: 0

Click here to see the affected service packages

🟢 All tests passed!

View the build log

kautikdk · 2024-12-10T06:13:45Z

You can remove hand-written files if MMV1 source code is enough for the feature support.

gurusai-voleti · 2024-12-10T06:55:05Z

Delete, import, update are customised operations so I kept the handwritten tmpl file and updated the yaml with custom_code attribute

modular-magician · 2024-12-10T10:49:07Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 6 files changed, 853 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 6 files changed, 853 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 69 insertions(+))
Open in Cloud Shell: Diff ( 4 files changed, 116 insertions(+))

modular-magician · 2024-12-10T10:51:22Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 6 files changed, 853 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 6 files changed, 853 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 69 insertions(+))
Open in Cloud Shell: Diff ( 4 files changed, 116 insertions(+))

modular-magician · 2024-12-10T10:57:43Z

Tests analytics

Total tests: 115
Passed tests: 100
Skipped tests: 9
Affected tests: 6

Click here to see the affected service packages

Action taken

Found 6 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

Get to know how VCR tests work

modular-magician · 2024-12-10T10:58:33Z

🟢 Tests passed during RECORDING mode:
TestAccStorageFolder_DeleteEmptyFolderWithForceDestroyDefault [Debug log]
TestAccStorageFolder_FailDeleteNonEmptyFolder [Debug log]
TestAccStorageFolder_FolderForceDestroy [Debug log]
TestAccStorageFolder_hnsDisabled [Debug log]
TestAccStorageFolder_storageFolderBasic [Debug log]
TestAccStorageFolder_storageFolderBasicExample [Debug log]

🟢 No issues found for passed tests after REPLAYING rerun.

🟢 All tests passed!

View the build log or the debug log for each test

modular-magician · 2024-12-10T10:59:29Z

Tests analytics

Total tests: 115
Passed tests: 100
Skipped tests: 9
Affected tests: 6

Click here to see the affected service packages

Action taken

Found 6 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

Get to know how VCR tests work

modular-magician · 2024-12-10T11:00:23Z

🟢 Tests passed during RECORDING mode:
TestAccStorageFolder_DeleteEmptyFolderWithForceDestroyDefault [Debug log]
TestAccStorageFolder_FailDeleteNonEmptyFolder [Debug log]
TestAccStorageFolder_FolderForceDestroy [Debug log]
TestAccStorageFolder_hnsDisabled [Debug log]
TestAccStorageFolder_storageFolderBasic [Debug log]
TestAccStorageFolder_storageFolderBasicExample [Debug log]

🟢 No issues found for passed tests after REPLAYING rerun.

🟢 All tests passed!

View the build log or the debug log for each test

roaks3 · 2024-12-10T22:27:10Z

mmv1/products/storage/Folder.yaml

+    # The API returns values with trailing slashes, even if not
+    # provided. Enforcing trailing slashes prevents diffs and ensures
+    # consistent output.
+    validation:


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.

if the path starts with /, the API validates the request body and rejects it as it cannot be started with /

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.

roaks3 · 2024-12-10T22:47:26Z

mmv1/templates/terraform/custom_update/storage_folder_update.go.tmpl

+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)
+    }
+}


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).

for now this should be fine

roaks3 · 2024-12-10T22:47:39Z

mmv1/templates/terraform/custom_delete/storage_folder_delete.go.tmpl

+        break // 0 items, folder empty
+    }
+
+    if !d.Get("force_destroy").(bool) {


I would recommend moving this to the beginning, since you should be able to escape early

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

Ah, thanks for clarifying, I was mixing this up with how prevent_destroy would be working.

modular-magician · 2024-12-11T05:48:57Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 6 files changed, 852 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 6 files changed, 852 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 69 insertions(+))
Open in Cloud Shell: Diff ( 4 files changed, 116 insertions(+))

modular-magician · 2024-12-11T05:57:37Z

Tests analytics

Total tests: 115
Passed tests: 105
Skipped tests: 9
Affected tests: 1

Click here to see the affected service packages

Action taken

Found 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

Get to know how VCR tests work

modular-magician · 2024-12-11T05:58:00Z

🔴 Tests failed during RECORDING mode:
TestAccStorageFolder_DeleteEmptyFolderWithForceDestroyDefault [Error message] [Debug log]

🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR.

View the build log or the debug log for each test

modular-magician · 2024-12-11T06:30:45Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 6 files changed, 853 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 6 files changed, 853 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 69 insertions(+))
Open in Cloud Shell: Diff ( 4 files changed, 116 insertions(+))

modular-magician · 2024-12-11T06:39:36Z

Tests analytics

Total tests: 115
Passed tests: 106
Skipped tests: 9
Affected tests: 0

Click here to see the affected service packages

🟢 All tests passed!

View the build log

modular-magician · 2024-12-11T10:20:54Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 6 files changed, 853 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 6 files changed, 853 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 69 insertions(+))
Open in Cloud Shell: Diff ( 4 files changed, 116 insertions(+))

modular-magician · 2024-12-11T10:29:21Z

Tests analytics

Total tests: 115
Passed tests: 106
Skipped tests: 9
Affected tests: 0

Click here to see the affected service packages

🟢 All tests passed!

View the build log

github-actions · 2024-12-13T09:42:36Z

@roaks3 This PR has been waiting for review for 3 weekdays. Please take a look! Use the label disable-review-reminders to disable these notifications.

gurusai-voleti added 2 commits October 23, 2024 11:28

add hns folder logic

ad3cefc

Merge branch 'GoogleCloudPlatform:main' into hns_folders

b522bcd

modular-magician added the service/storage label Oct 23, 2024

modular-magician requested a review from kautikdk October 23, 2024 11:41

gurusai-voleti added 2 commits October 23, 2024 11:41

update

de6f2ea

Merge branch 'hns_folders' of github.com:gurusai-voleti/magic-modules…

376eead

… into hns_folders

gurusai-voleti added 5 commits October 24, 2024 05:00

add example and yaml

0646523

added test case

454d6d1

yaml changes

8eb8111

Merge branch 'GoogleCloudPlatform:main' into hns_folders

69d2966

update

ddd5b51

update

90599d5

remove example

9efe5a8

update test case

9b5195d

gurusai-voleti added 2 commits December 10, 2024 05:43

update yaml

cce471f

update yaml

547875d

gurusai-voleti added 2 commits December 10, 2024 10:38

auto generate create and read

e6bbaac

fix yaml

d026479

roaks3 reviewed Dec 10, 2024

View reviewed changes

update delete

d2f7d28

github-actions bot requested a review from roaks3 December 11, 2024 05:38

update

9221d77

Merge branch 'GoogleCloudPlatform:main' into hns_folders

2fa4f7a

feat(HNS folders): add new resources to support HNS folders #12101

Are you sure you want to change the base?

feat(HNS folders): add new resources to support HNS folders #12101

Conversation

gurusai-voleti commented Oct 23, 2024 • edited Loading

modular-magician commented Oct 23, 2024

Diff report

Errors

modular-magician commented Oct 23, 2024

Diff report

Errors

modular-magician commented Oct 23, 2024

Tests analytics

modular-magician commented Oct 23, 2024

Tests analytics

modular-magician commented Oct 23, 2024

Diff report

modular-magician commented Oct 23, 2024

Tests analytics

modular-magician commented Oct 30, 2024

Diff report

modular-magician commented Oct 30, 2024

Tests analytics

Action taken

modular-magician commented Oct 30, 2024

modular-magician commented Oct 30, 2024

Diff report

modular-magician commented Oct 30, 2024

Tests analytics

Action taken

modular-magician commented Oct 30, 2024

modular-magician commented Oct 30, 2024

Diff report

Missing test report

modular-magician commented Oct 30, 2024

Tests analytics

modular-magician commented Oct 30, 2024

Diff report

Missing test report

modular-magician commented Oct 30, 2024

Tests analytics

modular-magician commented Dec 9, 2024

Tests analytics

modular-magician commented Dec 10, 2024

Diff report

modular-magician commented Dec 10, 2024

Diff report

modular-magician commented Dec 10, 2024

Tests analytics

modular-magician commented Dec 10, 2024

Tests analytics

kautikdk commented Dec 10, 2024

gurusai-voleti commented Dec 10, 2024

modular-magician commented Dec 10, 2024

Diff report

modular-magician commented Dec 10, 2024

Diff report

modular-magician commented Dec 10, 2024

Tests analytics

Action taken

modular-magician commented Dec 10, 2024

modular-magician commented Dec 10, 2024

Tests analytics

Action taken

modular-magician commented Dec 10, 2024

roaks3 Dec 10, 2024

Choose a reason for hiding this comment

gurusai-voleti Dec 11, 2024

Choose a reason for hiding this comment

roaks3 Dec 13, 2024

Choose a reason for hiding this comment

roaks3 Dec 10, 2024

Choose a reason for hiding this comment

gurusai-voleti Dec 11, 2024

Choose a reason for hiding this comment

roaks3 Dec 10, 2024

Choose a reason for hiding this comment

gurusai-voleti Dec 11, 2024 • edited Loading

Choose a reason for hiding this comment

roaks3 Dec 13, 2024

gurusai-voleti commented Oct 23, 2024 •

edited

Loading

gurusai-voleti Dec 11, 2024 •

edited

Loading