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

Object lock feature implementation for Storage #9363

Merged
merged 16 commits into from
Oct 31, 2023

Conversation

thomasmaclean
Copy link
Contributor

@thomasmaclean thomasmaclean commented Oct 25, 2023

Adds support for the Object Lock feature, which allows for setting retention policies on objects.

Release Note Template for Downstream PRs (will be copied)

storage: added `retention` field to `google_storage_bucket_object` resource
storage: added `enable_object_retention` to `google_storage_bucket` resource

@modular-magician
Copy link
Collaborator

Hello! I am a robot. It looks like you are a: Community Contributor Googler Core Contributor. Tests will run automatically.

@melinath, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look.

You can help make sure that review is quick by doing a self-review and by running impacted tests locally.

@modular-magician
Copy link
Collaborator

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.

Terraform GA: Diff ( 5 files changed, 241 insertions(+), 1 deletion(-))
Terraform Beta: Diff ( 5 files changed, 241 insertions(+), 1 deletion(-))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 3089
Passed tests 2776
Skipped tests: 313
Affected tests: 0

$\textcolor{red}{\textsf{Errors occurred during REPLAYING mode. Please fix them to complete your PR}}$
View the build log

@modular-magician
Copy link
Collaborator

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.

Terraform GA: Diff ( 5 files changed, 241 insertions(+), 1 deletion(-))
Terraform Beta: Diff ( 5 files changed, 241 insertions(+), 1 deletion(-))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 3176
Passed tests 2853
Skipped tests: 319
Affected tests: 4

Action taken

Found 4 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
TestAccBigQueryDataTable_bigtable|TestAccHealthcareDatasetIamPolicy|TestAccStorageObject_retention|TestAccStorageBucket_per_object_retention

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccBigQueryDataTable_bigtable[Debug log]
TestAccHealthcareDatasetIamPolicy[Debug log]
TestAccStorageObject_retention[Debug log]
TestAccStorageBucket_per_object_retention[Debug log]

Rerun these tests in REPLAYING mode to catch issues

$\textcolor{green}{\textsf{No issues found for passed tests after REPLAYING rerun.}}$


$\textcolor{green}{\textsf{All tests passed!}}$
View the build log or the debug log for each test

Copy link
Member

@melinath melinath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks pretty straightforward - thanks! A few questions & requests below.

thomasmaclean and others added 2 commits October 30, 2023 10:44
…ucket_object.go

Co-authored-by: Stephen Lewis (Burrows) <[email protected]>
…ucket_object.go

Co-authored-by: Stephen Lewis (Burrows) <[email protected]>
thomasmaclean and others added 3 commits October 30, 2023 10:48
…ucket_test.go.erb

Co-authored-by: Stephen Lewis (Burrows) <[email protected]>
…ucket_object.go

Co-authored-by: Stephen Lewis (Burrows) <[email protected]>
@modular-magician
Copy link
Collaborator

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.

Terraform GA: Diff ( 5 files changed, 242 insertions(+), 1 deletion(-))
Terraform Beta: Diff ( 5 files changed, 242 insertions(+), 1 deletion(-))

@modular-magician
Copy link
Collaborator

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.

Terraform GA: Diff ( 5 files changed, 242 insertions(+), 1 deletion(-))
Terraform Beta: Diff ( 5 files changed, 242 insertions(+), 1 deletion(-))

Missing test report

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

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

resource "google_storage_bucket" "primary" {
  enable_object_retention = # value needed
}

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

resource "google_storage_bucket_object" "primary" {
  retention {
    retain_until_time = # value needed
  }
}

@modular-magician
Copy link
Collaborator

$\textcolor{red}{\textsf{The provider crashed while running the VCR tests in REPLAYING mode}}$
$\textcolor{red}{\textsf{Please fix it to complete your PR}}$
View the build log

@modular-magician
Copy link
Collaborator

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.

Terraform GA: Diff ( 7 files changed, 252 insertions(+), 9 deletions(-))
Terraform Beta: Diff ( 6 files changed, 252 insertions(+), 4 deletions(-))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 3186
Passed tests 2858
Skipped tests: 325
Affected tests: 3

Action taken

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

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccDataprocJobIamPolicy[Debug log]
TestAccStorageObject_retention[Debug log]
TestAccStorageBucket_enable_object_retention[Debug log]

Rerun these tests in REPLAYING mode to catch issues

$\textcolor{red}{\textsf{Tests failed when rerunning REPLAYING mode:}}$
TestAccStorageObject_retention[Error message] [Debug log]

Tests failed due to non-determinism or randomness when the VCR replayed the response after the HTTP request was made.

Please fix these to complete your PR. If you believe these test failures to be incorrect or unrelated to your change, or if you have any questions, please raise the concern with your reviewer.


$\textcolor{red}{\textsf{Errors occurred during RECORDING mode. Please fix them to complete your PR.}}$
View the build log or the debug log for each test

Copy link
Member

@melinath melinath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM except a couple tiny changes and also reverting one of the changes I previously recommended 🤦

@modular-magician
Copy link
Collaborator

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

Breaking Change Detection Failed

The breaking change detector crashed during execution. This is usually due to the downstream provider(s) failing to compile. Please investigate or follow up with your reviewer.

Diff report

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

Terraform GA: Diff ( 6 files changed, 249 insertions(+), 4 deletions(-))
Terraform Beta: Diff ( 6 files changed, 249 insertions(+), 4 deletions(-))

@modular-magician
Copy link
Collaborator

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.

Terraform GA: Diff ( 6 files changed, 248 insertions(+), 5 deletions(-))
Terraform Beta: Diff ( 6 files changed, 248 insertions(+), 5 deletions(-))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 3186
Passed tests 2858
Skipped tests: 325
Affected tests: 3

Action taken

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

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccDataprocClusterIamPolicy[Debug log]
TestAccDataprocJobIamPolicy[Debug log]
TestAccStorageObject_retention[Debug log]

Rerun these tests in REPLAYING mode to catch issues

$\textcolor{green}{\textsf{No issues found for passed tests after REPLAYING rerun.}}$


$\textcolor{green}{\textsf{All tests passed!}}$
View the build log or the debug log for each test

@melinath
Copy link
Member

updated release notes to match current field names & specify one note per block

@melinath
Copy link
Member

I think the TGC test failures are spurious - looks like it's not able to compile the provider. This PR doesn't alter TGC so going forward with merge.

@melinath melinath merged commit 4f30808 into GoogleCloudPlatform:main Oct 31, 2023
11 of 12 checks passed
BBBmau pushed a commit to BBBmau/magic-modules that referenced this pull request Nov 28, 2023
)

* Early implementation of bucket object lock setting.

* Complete implementation of object lock for buckets.

* Compelete implementation, waiting for allowlist.

* Tests pass against test environment.

* Fixes bug in object test.

* Update mmv1/third_party/terraform/services/storage/resource_storage_bucket_object.go

Co-authored-by: Stephen Lewis (Burrows) <[email protected]>

* Update mmv1/third_party/terraform/services/storage/resource_storage_bucket_object.go

Co-authored-by: Stephen Lewis (Burrows) <[email protected]>

* Update mmv1/third_party/terraform/services/storage/resource_storage_bucket_test.go.erb

Co-authored-by: Stephen Lewis (Burrows) <[email protected]>

* Update mmv1/third_party/terraform/services/storage/resource_storage_bucket_object.go

Co-authored-by: Stephen Lewis (Burrows) <[email protected]>

* Update mmv1/third_party/terraform/services/storage/resource_storage_bucket.go.erb

Co-authored-by: Stephen Lewis (Burrows) <[email protected]>

* Updates from review comments

* Update mmv1/third_party/terraform/services/storage/resource_storage_bucket_object.go

Co-authored-by: Stephen Lewis (Burrows) <[email protected]>

* Update mmv1/third_party/terraform/website/docs/r/storage_bucket_object.html.markdown

Co-authored-by: Stephen Lewis (Burrows) <[email protected]>

* Removing non-deterministic date from test

* Removing unused variable

* fixing nested_customer_encryption anchor link

---------

Co-authored-by: Stephen Lewis (Burrows) <[email protected]>
jialei-chen pushed a commit to jialei-chen/magic-modules that referenced this pull request Nov 29, 2023
)

* Early implementation of bucket object lock setting.

* Complete implementation of object lock for buckets.

* Compelete implementation, waiting for allowlist.

* Tests pass against test environment.

* Fixes bug in object test.

* Update mmv1/third_party/terraform/services/storage/resource_storage_bucket_object.go

Co-authored-by: Stephen Lewis (Burrows) <[email protected]>

* Update mmv1/third_party/terraform/services/storage/resource_storage_bucket_object.go

Co-authored-by: Stephen Lewis (Burrows) <[email protected]>

* Update mmv1/third_party/terraform/services/storage/resource_storage_bucket_test.go.erb

Co-authored-by: Stephen Lewis (Burrows) <[email protected]>

* Update mmv1/third_party/terraform/services/storage/resource_storage_bucket_object.go

Co-authored-by: Stephen Lewis (Burrows) <[email protected]>

* Update mmv1/third_party/terraform/services/storage/resource_storage_bucket.go.erb

Co-authored-by: Stephen Lewis (Burrows) <[email protected]>

* Updates from review comments

* Update mmv1/third_party/terraform/services/storage/resource_storage_bucket_object.go

Co-authored-by: Stephen Lewis (Burrows) <[email protected]>

* Update mmv1/third_party/terraform/website/docs/r/storage_bucket_object.html.markdown

Co-authored-by: Stephen Lewis (Burrows) <[email protected]>

* Removing non-deterministic date from test

* Removing unused variable

* fixing nested_customer_encryption anchor link

---------

Co-authored-by: Stephen Lewis (Burrows) <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants