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.

Sign up for GitHub

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
+330 −1
Merged

Implementing soft delete for storage bucket #2051

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
924c54e
soft_delete_policy.patch
600lyy Jun 18, 2024
cfe404c
generate manifests and CRD definition
600lyy Jun 18, 2024
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
38 changes: 38 additions & 0 deletions ...ions.k8s.io_v1_customresourcedefinition_storagebuckets.storage.cnrm.cloud.google.com.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,23 @@ spec:
required:
- retentionPeriod
type: object
softDeletePolicy:
description: 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.
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!

description: Server-determined value that indicates the time from
which the policy, or one with a greater retention, was effective.
This value is in RFC 3339 format.
type: string
retentionDurationSeconds:
description: The duration in seconds that soft-deleted objects
in the bucket will be retained and cannot be permanently deleted.
Default value is 604800.
type: integer
type: object
storageClass:
description: 'The Storage Class of the new bucket. Supported values
include: STANDARD, MULTI_REGIONAL, REGIONAL, NEARLINE, COLDLINE,
Expand Down Expand Up @@ -353,6 +370,27 @@ spec:
current reported status reflects the most recent desired state of
the resource.
type: integer
observedState:
description: The observed state of the underlying GCP resource.
properties:
softDeletePolicy:
description: 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.
properties:
effectiveTime:
description: Server-determined value that indicates the time
from which the policy, or one with a greater retention,
was effective. This value is in RFC 3339 format.
type: string
retentionDurationSeconds:
description: The duration in seconds that soft-deleted objects
in the bucket will be retained and cannot be permanently
deleted. Default value is 604800.
type: integer
type: object
type: object
selfLink:
description: The URI of the created resource.
type: string
Expand Down
2 changes: 2 additions & 0 deletions config/samples/resources/storagebucket/storage_v1beta1_storagebucket.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,5 @@ spec:
method: ["GET", "HEAD", "DELETE"]
maxAgeSeconds: 3600
uniformBucketLevelAccess: true
softDeletePolicy:
retentionDurationSeconds: 604800
3 changes: 3 additions & 0 deletions config/servicemappings/storage.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ spec:
labels: labels
resourceID:
targetField: name
observedFields:
- soft_delete_policy.effective_time
- soft_delete_policy.retention_duration_seconds
idTemplate: "{{name}}"
# odd resource where the project is 'optional', needs more thought -- also not supported by gcloud (yet), problem applies to all Storage resources
idTemplateCanBeUsedToMatchResourceName: false
Expand Down
3 changes: 2 additions & 1 deletion mockgcp/mockstorage/bucket.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,8 @@ func (s *buckets) InsertBucket(ctx context.Context, req *pb.InsertBucketRequest)
defaultRetention := time.Hour * 7 * 24
softDeletePolicy.RetentionDurationSeconds = PtrTo(int64(defaultRetention.Seconds()))
}
softDeletePolicy.EffectiveTime = now
// TODO: Should be now
softDeletePolicy.EffectiveTime = timestamppb.New(time.Date(2024, time.April, 1, 0, 0, 0, 0, time.UTC))

if err := s.storage.Create(ctx, fqn, obj); err != nil {
return nil, err
Expand Down
34 changes: 34 additions & 0 deletions pkg/clients/generated/apis/storage/v1beta1/storagebucket_types.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

83 changes: 83 additions & 0 deletions pkg/clients/generated/apis/storage/v1beta1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions pkg/test/resourcefixture/testdata/basic/storage/v1beta1/storagebucket/create.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,5 @@ spec:
type: Delete
condition:
age: 7
softDeletePolicy:
retentionDurationSeconds: 604800
2 changes: 2 additions & 0 deletions pkg/test/resourcefixture/testdata/basic/storage/v1beta1/storagebucket/update.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.

67 changes: 67 additions & 0 deletions ...oogle3-docs/resource-reference/generated/resource-docs/storage/storagebucket.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,9 @@ resourceID: string
retentionPolicy:
isLocked: boolean
retentionPeriod: integer
softDeletePolicy:
effectiveTime: string
retentionDurationSeconds: integer
storageClass: string
uniformBucketLevelAccess: boolean
versioning:
Expand Down Expand Up @@ -684,6 +687,36 @@ Enables Bucket PolicyOnly access to a bucket.{% endverbatim %}</p>
<p>{% verbatim %}The period of time, in seconds, that objects in the bucket must be retained and cannot be deleted, overwritten, or archived. The value must be less than 3,155,760,000 seconds.{% endverbatim %}</p>
</td>
</tr>
<tr>
<td>
<p><code>softDeletePolicy</code></p>
<p><i>Optional</i></p>
</td>
<td>
<p><code class="apitype">object</code></p>
<p>{% verbatim %}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.{% endverbatim %}</p>
</td>
</tr>
<tr>
<td>
<p><code>softDeletePolicy.effectiveTime</code></p>
<p><i>Optional</i></p>
</td>
<td>
<p><code class="apitype">string</code></p>
<p>{% verbatim %}Server-determined value that indicates the time from which the policy, or one with a greater retention, was effective. This value is in RFC 3339 format.{% endverbatim %}</p>
</td>
</tr>
<tr>
<td>
<p><code>softDeletePolicy.retentionDurationSeconds</code></p>
<p><i>Optional</i></p>
</td>
<td>
<p><code class="apitype">integer</code></p>
<p>{% verbatim %}The duration in seconds that soft-deleted objects in the bucket will be retained and cannot be permanently deleted. Default value is 604800.{% endverbatim %}</p>
</td>
</tr>
<tr>
<td>
<p><code>storageClass</code></p>
Expand Down Expand Up @@ -771,6 +804,10 @@ conditions:
status: string
type: string
observedGeneration: integer
observedState:
softDeletePolicy:
effectiveTime: string
retentionDurationSeconds: integer
selfLink: string
url: string
```
Expand Down Expand Up @@ -838,6 +875,34 @@ url: string
<p>{% verbatim %}ObservedGeneration is the generation of the resource that was most recently observed by the Config Connector controller. If this is equal to metadata.generation, then that means that the current reported status reflects the most recent desired state of the resource.{% endverbatim %}</p>
</td>
</tr>
<tr>
<td><code>observedState</code></td>
<td>
<p><code class="apitype">object</code></p>
<p>{% verbatim %}The observed state of the underlying GCP resource.{% endverbatim %}</p>
</td>
</tr>
<tr>
<td><code>observedState.softDeletePolicy</code></td>
<td>
<p><code class="apitype">object</code></p>
<p>{% verbatim %}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.{% endverbatim %}</p>
</td>
</tr>
<tr>
<td><code>observedState.softDeletePolicy.effectiveTime</code></td>
<td>
<p><code class="apitype">string</code></p>
<p>{% verbatim %}Server-determined value that indicates the time from which the policy, or one with a greater retention, was effective. This value is in RFC 3339 format.{% endverbatim %}</p>
</td>
</tr>
<tr>
<td><code>observedState.softDeletePolicy.retentionDurationSeconds</code></td>
<td>
<p><code class="apitype">integer</code></p>
<p>{% verbatim %}The duration in seconds that soft-deleted objects in the bucket will be retained and cannot be permanently deleted. Default value is 604800.{% endverbatim %}</p>
</td>
</tr>
<tr>
<td><code>selfLink</code></td>
<td>
Expand Down Expand Up @@ -897,6 +962,8 @@ spec:
method: ["GET", "HEAD", "DELETE"]
maxAgeSeconds: 3600
uniformBucketLevelAccess: true
softDeletePolicy:
retentionDurationSeconds: 604800
```


Expand Down
2 changes: 2 additions & 0 deletions tests/e2e/normalize.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ func normalizeKRMObject(u *unstructured.Unstructured, project testgcp.GCPProject
visitor.replacePaths[".updated"] = "2024-04-01T12:34:56.123456Z"
visitor.replacePaths[".acl[].etag"] = "abcdef0123A"
visitor.replacePaths[".defaultObjectAcl[].etag"] = "abcdef0123A="
visitor.replacePaths[".spec.softDeletePolicy.effectiveTime"] = "1970-01-01T00:00:00Z"
visitor.replacePaths[".status.observedState.softDeletePolicy.effectiveTime"] = "1970-01-01T00:00:00Z"

visitor.sortSlices = sets.New[string]()
// TODO: This should not be needed, we want to avoid churning the kube objects
Expand Down
7 changes: 7 additions & 0 deletions tests/e2e/testdata/scenarios/powertool/powertool_set_bucket_location/_object00.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ spec:
location: US
publicAccessPrevention: inherited
resourceID: storagebucket-${uniqueId}
softDeletePolicy:
effectiveTime: "1970-01-01T00:00:00Z"
retentionDurationSeconds: 604800
storageClass: STANDARD
versioning:
enabled: false
Expand All @@ -48,5 +51,9 @@ status:
status: "True"
type: Ready
observedGeneration: 2
observedState:
softDeletePolicy:
effectiveTime: "1970-01-01T00:00:00Z"
retentionDurationSeconds: 604800
selfLink: https://www.googleapis.com/storage/v1/b/storagebucket-${uniqueId}
url: gs://storagebucket-${uniqueId}
Loading
Loading