[ 
https://issues.apache.org/jira/browse/HDDS-17246?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Siyao Meng updated HDDS-17246:
------------------------------
    Attachment: TestBugCR10TrashInRecreatedBucket.java

> Lifecycle task can create the previous owner's .Trash directory in a bucket 
> that was deleted and recreated while the task ran
> -----------------------------------------------------------------------------------------------------------------------------
>
>                 Key: HDDS-17246
>                 URL: https://issues.apache.org/jira/browse/HDDS-17246
>             Project: Apache Ozone
>          Issue Type: Bug
>    Affects Versions: 2.3.0
>            Reporter: Siyao Meng
>            Priority: Minor
>         Attachments: CR-10-skip-trash-dir-creation-in-recreated-bucket.patch, 
> TestBugCR10TrashInRecreatedBucket.java
>
>
> h3. Mechanism
> {{KeyLifecycleService.LifecycleActionTask.call}} compares the bucket object 
> ID with the lifecycle configuration once, at task start, and then uses the 
> cached {{OmBucketInfo}} for the whole scan. When expired keys go to trash, 
> {{moveToTrash}} calls {{checkAndCreateTrashDirIfNeeded}}, which looks the 
> bucket up by volume and bucket name and, if {{.Trash/<owner>/Current}} does 
> not exist, submits a recursive {{CreateDirectory}} as the cached owner. 
> Nothing checks that the bucket is still the one the task started on. The 
> renames and deletes the task sends afterwards carry the update ID of each 
> scanned key, so they fail in a recreated bucket (on LEGACY buckets since 
> HDDS-16993). The trash directories are the only write with an effect.
> h3. Trigger
> # An FSO or LEGACY bucket has an expiration rule, lifecycle move to trash is 
> on ({{ozone.lifecycle.service.move.to.trash.enabled}}, default true) and the 
> OM trash interval is above 0.
> # A lifecycle task passes its start check and collects expired keys.
> # Before the task handles that batch, the keys are deleted, the bucket is 
> deleted and a bucket with the same name is created.
> # The task creates {{.Trash/<old owner>/Current}} in the new bucket. Its 
> renames then fail with KEY_NOT_FOUND.
> Batches are handled while the scan is still running, so the window grows with 
> the size of the bucket. I reproduced it with the service's existing pause 
> hook and did not try to hit the window without one.
> h3. Impact
> * The recreated bucket has no lifecycle configuration and was never written, 
> but it now holds {{.Trash/<old owner>/Current}} with an ACL for the old 
> owner, and {{DeleteBucket}} on it fails with BUCKET_NOT_EMPTY.
> * The directories stay until someone deletes them by hand. The new bucket has 
> no lifecycle rule, and from reading {{TrashPolicyOzone}} the trash emptier 
> only rolls over and deletes checkpoints under {{.Trash/<user>}}, it never 
> removes {{.Trash/<user>}} itself (argued, not run).
> * With native ACLs the {{CreateDirectory}} runs as the old owner and is 
> refused unless the old owner can already create keys in the new bucket (WRITE 
> on the bucket, or admin or volume owner), checked against 
> {{OzoneNativeAuthorizer}}, not run end to end.
> h3. Reproduction
> PASS, deterministic, unmodified source. 
> [^TestBugCR10TrashInRecreatedBucket.java] ({{hadoop-ozone/ozone-manager}}) 
> runs one OM with its real {{KeyLifecycleService}}. The bucket, the key, the 
> lifecycle configuration and the bucket delete and create all go through 
> {{OzoneManagerProtocol}}. The task is held with the service's existing fault 
> injector just before it sends the delete for the expired key, and move to 
> trash is switched on during that pause (the task reads the flag per batch), 
> so the next batch takes the trash path. A passing test means the defect is 
> present. Output on the unfixed code:
> {noformat}
> bob's bucket contains .Trash/alice/Current, acls=[user:alice:a[ACCESS]]
> bob deletes his empty bucket: BUCKET_NOT_EMPTY Bucket is not empty
> {noformat}
> h3. Suggested fix
> The patch below re-reads the bucket on the submitting side, which narrows the 
> window to the time between that read and the apply of the 
> {{CreateDirectory}}. A possible follow up closes it completely: an optional 
> expected bucket object ID on the {{CreateDirectory}} request, set only by the 
> lifecycle task and checked at apply under the bucket lock in 
> {{OMDirectoryCreateRequest}} and {{OMDirectoryCreateRequestWithFSO}}. That is 
> a request change. It needs no new layout feature if it lands before the 
> lifecycle feature is released (no lifecycle configuration can be set before 
> {{S3_LIFECYCLE_SUPPORT}} is finalized), but it touches the 
> {{CreateDirectory}} request on both layouts.
> h3. Patch
> [^CR-10-skip-trash-dir-creation-in-recreated-bucket.patch], against 
> 20d95ed37fe35013b5320713665db03c06ee36fa. It also applies to master at 
> 64d4a791502.
> Before it creates a missing trash directory, 
> {{checkAndCreateTrashDirIfNeeded}} reads the bucket by name and refuses if it 
> is gone or has a different object ID. The task then ends the same way as when 
> the trash root cannot be prepared (HDDS-16162): the error is logged and no 
> directory is created. Nothing changes for a bucket that is not recreated.
> Covered by {{testMoveToTrashSkipsRecreatedBucket}} in the existing 
> {{TestKeyLifecycleService.Normal}}. Without the change it fails for both 
> parameterizations because the trash directory exists. With it all of 
> {{TestKeyLifecycleService}} (726 tests, 18 skipped) passes and checkstyle is 
> clean. With the patch the reproduction fails, because 
> {{.Trash/alice/Current}} is not created.
> Found by code review of the OM volume, bucket and snapshot lifecycle paths, 
> as part of the TLA+ verification effort under HDDS-15926, on commit 
> 20d95ed37fe35013b5320713665db03c06ee36fa. Checked against HDDS issues and 
> apache/ozone pull requests for duplicates before filing. The attached patch 
> is a proposal for review. Generated with Specula (Claude Opus 5.5).



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to