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

Implementing soft delete for storage bucket #2051

Merged
merged 2 commits into from
Jun 21, 2024
Merged

Implementing soft delete for storage bucket #2051

merged 2 commits into from
Jun 21, 2024

Conversation

600lyy
Copy link
Member

@600lyy 600lyy commented Jun 18, 2024

Change description

Add support for soft delete policy for Google storage bucket.

The bucket's soft delete policy, which defines the period of time that soft-deleted objects will be retained, and cannot be permanently deleted. If it is not provided, by default Google Cloud Storage sets this to default soft delete policy`

This is the implementation for #2046

  • Run make ready-pr to ensure this PR is ready for review.
  • Perform necessary E2E testing for changed resources.

@yuwenma
Copy link
Collaborator

yuwenma commented Jun 20, 2024

/lgtm

PR looks good. @600lyy could you fix the presubmit check?

permanently deleted. If it is not provided, by default Google Cloud
Storage sets this to default soft delete policy.
properties:
effectiveTime:
Copy link
Collaborator

Choose a reason for hiding this comment

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

@yuwenma this field is an output-only field that gets inserted under spec. According to our previous discussion, we don't want to add any output-only spec fields any more - is my understanding correct?

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a workaround to hide output-only spec field but still support it under observed state: #2075 .

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you @maqiuyujoyce for the quick workaround!

@@ -23,3 +23,5 @@ spec:
versioning:
enabled: true
lifecycleRule: []
softDeletePolicy:
retentionDurationSeconds: 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

I ran the test locally and found that 0 was not used in the update, and the value kept to be 604800. Could you verify that this is a valid update; or whether setting the value to 0 is actually effective? AFAIK, we don't support setting a primitive-typed field to its empty value.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I checked again and actually it worked.

Add support for bucket's soft delete policy
which defines the period of time that soft-deleted objects will be retained
@google-oss-prow google-oss-prow bot removed the lgtm label Jun 21, 2024
@justinsb
Copy link
Collaborator

Looks like I can do the rebase / cleanup; just pushed to the PR branch 🤞

@justinsb
Copy link
Collaborator

Looks like a (simple) rebase was all that was needed - thank you @600lyy

/approve
/lgtm

@maqiuyujoyce I think we should merge this before #2075

Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: justinsb

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@google-oss-prow google-oss-prow bot removed the lgtm label Jun 21, 2024
@maqiuyujoyce
Copy link
Collaborator

@justinsb as long as we don't cut a release, the order to merge #2051 and #2075 shouldn't matter.

/lgtm

@google-oss-prow google-oss-prow bot added the lgtm label Jun 21, 2024
@google-oss-prow google-oss-prow bot merged commit ddd7cd3 into GoogleCloudPlatform:master Jun 21, 2024
13 checks passed
@600lyy 600lyy deleted the storage-bucket-soft-delete branch June 24, 2024 08:08
@yuwenma yuwenma added this to the 1.120 milestone Oct 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants