mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-07-23 00:14:26 +00:00
4.39
162 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Chris Lu
|
17af32f3ff |
s3: paginate ListBuckets and serve it from a bucket owner index (#10214)
* s3: paginate ListBuckets with max-buckets, continuation-token, and prefix ListBuckets buffered every bucket entry into one slice and one XML body, which falls over with very large bucket counts. Page through the filer listing instead, cap each response at 10000 buckets like AWS, and honor max-buckets, prefix, and an opaque keyset continuation-token. * s3: maintain a bucket owner index under /buckets/.system/owners Map each bucket owner to its buckets as zero-length entries at /buckets/.system/owners/<owner>/<bucket>, with Crtime mirroring the bucket's creation time. The bucket handlers write the index synchronously, the /buckets metadata subscription reconciles changes made elsewhere (weed shell, other gateways, direct filer operations), and a startup backfill indexes pre-existing buckets before writing a ready marker. Owner names are path-escaped so no identity name can escape the index directory. * s3: serve ListBuckets from the bucket owner index Once the owner index is ready, non-admin identities list their owned buckets straight from it, merged with any buckets their legacy actions name explicitly, so ListBuckets costs O(own buckets) instead of a scan of the global /buckets directory. Admins, identities with a bare List grant or wildcard action patterns, and policy-authorized identities whose grants cannot be enumerated keep the paged scan; policy-routed identities get their owned buckets, matching AWS ListBuckets returning only the caller's buckets. * s3: keep dot-prefixed names under /buckets out of bucket surfaces Dot-prefixed entries (.system) can never be valid bucket names, so refuse to resolve them as buckets and skip them in the shell bucket listing, matching what ListBuckets and the admin UI already do. * test: cover ListBuckets pagination and the owner index end to end * s3: fail closed on a nil identity when routing ListBuckets * s3: decide the IAM authorization mechanism in one place VerifyActionPermission and the ListBuckets owner-index routing each re-derived the session-token / attached-policy / legacy-actions split; extract the decision so the two cannot drift. * s3: heal the owner index on concurrent bucket recreation too The mkdir-lost-the-race path answers BucketAlreadyOwnedByYou just like the up-front existence check, so give it the same index repair. * s3: drop owner-index records for buckets deleted during backfill A bucket removed between the backfill reading its page and writing the index record became a permanent phantom in its owner's listing: the delete's own cleanup ran before the record existed. After indexing each page, re-list the same name range and remove records whose bucket is gone; deletes landing after the re-list find the record and remove it themselves. * s3: add ContinuationToken and Prefix to the ListBuckets schema Keep AmazonS3.xsd aligned with the generated ListAllMyBucketsResult so a regeneration does not drop the pagination fields. |
||
|
Chris Lu
|
a88acaf061 |
Add performance CI (profiling, throughput, S3 read/write) (#10105)
* test: add self-contained S3 read/write load tool Concurrent PUT/GET against the S3 gateway, reporting requests/sec, transfer rate, and latency percentiles. Built on the aws-sdk-go-v2 client the S3 tests already use, so no extra benchmark binary is needed. * ci: add performance workflow Three parallel jobs: cpu/heap pprof of the server under write load, native throughput via weed benchmark plus the Go micro-benchmarks, and an S3 read/write benchmark against the gateway. Runs on push to master and manual dispatch with tunable duration, object count, size, and concurrency. |
||
 7y-9
|
44d575100a |
fix(s3api): preserve requested AES256 copy encryption (#10049)
* fix(s3api): preserve requested AES256 copy encryption Problem CopyObject metadata processing ignored an explicit x-amz-server-side-encryption: AES256 request header. A destination copy could lose the requested SSE-S3 metadata even though KMS requests were handled. Root cause processMetadataBytes only wrote the destination SSE header when the requested algorithm was aws:kms. Any other explicit SSE algorithm fell through to the source-preservation branch. Fix Write the requested SSE algorithm whenever x-amz-server-side-encryption is present, and keep KMS-specific metadata handling limited to aws:kms. Co-authored-by: Codex <noreply@openai.com> * fix(s3api): reject unsupported copy encryption algorithms A mistyped or unsupported x-amz-server-side-encryption value on a copy request slipped past validation and got persisted as the destination's algorithm header, advertising encryption that was never applied. Reject anything other than AES256 or aws:kms up front. * fix(s3api): write SSE key metadata for empty encrypted copies A zero-byte source copied with an explicit SSE request took the no-content branch and never ran the encryption path, leaving the object with a bare algorithm header but no key. HEAD then advertised SSE while the encryption-state machine saw the header as orphaned. Run the inline encryption path when the destination requests encryption so the key metadata is written too. * s3api: use SSEAlgorithmKMS constant in copy metadata handling * test(s3api): cover source SSE preservation on copy * test(iam): allow the local client's real source IP in SourceIp tests The aws:SourceIp allow policies hardcoded the loopback CIDRs, but a CI runner reaching the server over localhost can be observed with one of the host's RFC1918 addresses (the S3 endpoint is advertised on a 10.x interface), so the positive-condition PutObject was denied and the allow assertion flaked while the deny path passed trivially. Broaden the allow list to loopback plus private ranges via a shared helper, and log the denial on each failed attempt so any residual failure is diagnosable. --------- Co-authored-by: Codex <noreply@openai.com> Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
Chris Lu
|
561768a426 |
[s3]: preserve multipart copy checksums (#9948)
* s3: preserve checksums for copied multipart parts * s3: return checksums from multipart copy * s3: pin the upload's checksum algorithm on copy-part re-stream * s3: note why UploadPartCopy uses the re-stream slow path * s3: explain the TLS proxy in the multipart copy checksum test * s3: cover nil and unknown-algorithm edge cases in copy checksum tests * s3: cover all checksum algorithms in the multipart copy test * s3: run all checksum integration tests, not just presigned |
||
|
Chris Lu
|
3eb550a3f1 |
fix(tests): 32-bit build of EC e2e tests, type-check linux/386 in CI (#9922)
* fix(tests): keep EC e2e fid cookie arithmetic in uint32 The cookie constants 0x9490CA00 and 0x9500CA00 were added to the int loop variable before conversion, overflowing 32-bit int at compile time on linux/386 and linux/arm. Convert the loop variable instead so the addition stays in uint32. * fix(tests): pass s3client max backoff in milliseconds MaxBackoffDelay is documented as milliseconds and multiplied by 1e6 before use, but the example set it to 5s in nanoseconds, yielding an absurd backoff on 64-bit and a compile-time int overflow on 32-bit. * ci: type-check code and tests for linux/386 64-bit-only constant arithmetic keeps slipping into test files and breaking 32-bit downstream builds. Vet the whole root module under GOOS=linux GOARCH=386 so these fail in CI instead of after release. * fix(tests): convert s3client backoff to Duration before scaling The ms-to-ns multiplication ran in int, wrapping at runtime on 32-bit; scale by time.Millisecond after the Duration conversion instead. |
||
|
Chris Lu
|
2871e6552a |
fix(s3api): drop ancestor directory markers from prefixed ListObjectVersions (#9885)
processExplicitDirectory appended a directory-key object as a version without checking it against the prefix. A versioned listing descends through ancestor markers to reach a deeper prefix, so every ancestor (Veeam/, Veeam/Backup/, ...) leaked into Versions even though none of them match the prefix - which makes Veeam's immutable repository scan abort on an unexpected key. Guard on the prefix so only keys at or under it surface, matching ListObjectsV2 and AWS. |
||
|
Chris Lu
|
01637410e2 |
test(s3): address review feedback on the versioning suite (#9846)
- Different-users bucket test: use getNewBucketName() so the bucket carries the tracked prefix and run id and gets swept if the test leaks, instead of an untracked name. - Makefile: clarify that '.' matches the opt-in stress tests but they self-skip without ENABLE_STRESS_TESTS, so they don't execute in the default run. - Versioned list test: guard the Object.Size dereference with require.NotNil. |
||
|
Chris Lu
|
d321f9efb4 |
s3: collapse suspended-versioning deletes onto one null marker (#9845)
A suspended-versioning DELETE was recorded with createDeleteMarker, which mints a fresh real version id each time, so repeated suspended deletes piled up delete markers instead of overwriting a single null marker as S3 specifies. Record the suspended delete as a 'null' marker with a fixed file name (v_null) and point the latest-version pointer at it explicitly; putSuspendedVersioningObject's existing null-version cleanup removes it on the next suspended PUT, so the object undeletes cleanly and at most one null marker exists. Enabled-versioning deletes are unchanged (still distinct historical markers). Update TestSuspendedVersioningDeleteBehavior to the AWS-correct counts: one null marker after a suspended delete, and the null marker plus one real marker after a re-enabled delete. |
||
|
Chris Lu
|
fa9bf58c86 |
test(s3): make the whole versioning suite pass and gate it in CI (#9844)
* test(s3): correct bucket-recreate expectations and cover the different-owner case A same-owner CreateBucket on an existing bucket returns BucketAlreadyOwnedByYou (idempotent recreate); the suite expected BucketAlreadyExists, which only applies when the name is owned by someone else. Fix the same-owner cases (plain and Object-Lock) and implement the previously-skipped different-owner test, which now exercises the BucketAlreadyExists path via a second identity. * test(s3): assert the deletion invariant for suspended-versioning delete A suspended-versioning DELETE removes the null version and records a delete marker so the object reads as deleted; the test expected no marker, which would let an older version resurface. Assert that a marker is recorded (and read DeleteMarker through aws.ToBool) rather than an exact count, so it holds whether or not the suspended-marker id/dedup is later collapsed to AWS's single null marker. * test(s3): run the whole versioning suite by default TEST_PATTERN was TestVersioning, which left bucket-creation, suspended-delete and directory/version-listing tests ungated. Default to '.' so every test runs; opt-in stress tests self-skip without ENABLE_STRESS_TESTS and keep their own targets. |
||
|
Chris Lu
|
795349d796 |
test(s3): deref Object.Size in versioned list assertion (#9843)
TestVersionedObjectListBehavior compared int64 against listedObject.Size, which is *int64, so the assertion always failed on a type mismatch once reached. Dereference it (and in the log line). |
||
|
Chris Lu
|
309cb32416 |
s3: list directory key objects in versioned bucket version listings (#9842)
ListObjectVersions gated explicit directory objects on Mime == FolderMimeType, but an SDK PutObject of "dir/" carries a default Content-Type (e.g. application/octet-stream), so those directory keys were dropped from the version listing while ListObjectsV2 - which keys off IsDirectoryKeyObject (any non-empty mime) - still showed them. Use the same IsDirectoryKeyObject check so the two listings agree. The directory test's storage-class assertion compared an ObjectStorageClass constant against ObjectVersion.StorageClass (ObjectVersionStorageClass); the values matched but the SDK enum types did not, so it only surfaced once the directories started appearing. Use the matching constant. |
||
 Konstantin Lebedev
|
df833d485f | [test] update docker image for s3test (#9811) | ||
|
Chris Lu
|
b6a0bde16b |
test(s3/iam): scope ListBucket isolation via s3:prefix condition (#9805)
The username-isolation policy denied s3:ListBucket through an object-path NotResource. ListBucket is bucket-level, so its resource ARN is the bucket and never matches an object path: the Deny always fired and a user could not list their own prefix. Scope the per-user List deny with a StringNotLike s3:prefix condition instead, the same mechanism the matching Allow uses. |
||
|
Konstantin Lebedev
|
3537312045 |
[docker] add make test_keycloak_s3 for local develop and debug (#9719)
* add make test_keylock_s3 for local develop and debug * fix typos * add condition oidc:azp * docker: reuse test/s3/iam realm and iam config for keycloak dev compose Point the keycloak dev compose at the existing test/s3/iam configs instead of a parallel realm/port/key/role set. Adds one declarative realm import (seaweedfs-test-realm.json) as the single realm source and drops the duplicated iam.json/s3.json. --------- Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
Chris Lu
|
6fc212cedb |
test: wait for a writable volume before lifecycle tests' first write (#9658)
Probe one throwaway write once per process before the lifecycle tests run, absorbing the post-start volume-growth window so the first real PutObject doesn't race volume growth and 500. Each call is bounded by the remaining 60s budget; CreateBucket is retried within it. |
||
|
Chris Lu
|
dc5621d2ae |
s3: use oidc: prefix for trust-policy conditions in IAM example (#9653)
* s3: use oidc: prefix for trust-policy conditions in IAM example Trust-policy conditions for AssumeRoleWithWebIdentity see OIDC claims under the oidc: prefix, so the docker example's bare "roles" key never matched and denied every web-identity assume against those roles. Switch the three roles to oidc:roles. Also document the available trust-policy condition keys (oidc:iss/sub/aud, oidc:<claim>, aws:FederatedProvider, aws:userid, sts:DurationSeconds) and note that roleMapping selects the role for direct OIDC bearer auth while STS uses the explicit RoleArn plus trust policy. * s3: clarify aws:userid differs between trust policy and request auth aws:userid is the raw sub claim during trust-policy evaluation, but a stable sub+iss hash (ComputeParentUser) during S3 request authorization after the role is assumed. Note both so the two contexts aren't conflated. |
||
|
Chris Lu
|
285025eb73 |
s3api: support group inline policies + Condition enforcement (#9569)
* test(s3api): cover IAM inline policy aws:SourceIp + group inline gap Unit tests under weed/s3api/ drive PutUserPolicy / PutGroupPolicy → reload → VerifyActionPermission with a synthetic 127.0.0.1 request and assert that the policy's IpAddress condition flips the outcome. The user-policy cases pass on master (hydrateRuntimePolicies already routes inline docs through the policy engine, so Condition blocks are honored end- to-end). The group-policy case fails: PutGroupPolicy still returns NotImplemented, so a group inline doc never lands in the engine. Integration counterparts live under test/s3/iam/ and exercise the same paths against a live SeaweedFS S3+IAM endpoint. * s3api: support group inline policies + Condition enforcement PutGroupPolicy/GetGroupPolicy/DeleteGroupPolicy/ListGroupPolicies used to return NotImplemented in embedded IAM mode, so anything attached to a group as an inline doc — including aws:SourceIp or any other Condition — was simply unreachable. Wire the four endpoints to the credential-store methods that were already in place (memory, postgres, filer_etc all implement GroupInlinePolicyStore). On every config reload, hydrateRuntimePolicies now also walks LoadGroupInlinePolicies, registers each doc in the IAM policy engine under __inline_group_policy__/<group>/<policy>, and appends that key to Group.PolicyNames so evaluateIAMPolicies picks it up through its existing group walk. PutGroupPolicy/DeleteGroupPolicy are added to the ReloadConfiguration trigger list in DoActions. Side fix: MemoryStore.LoadConfiguration now surfaces store.groups too. Without it iam.groups never repopulated on a memory-store reload, so group policy evaluation silently no-op'd whether the policy was inline or attached. The existing tests didn't notice because no test reloaded through cm after creating a group. The NotImplemented unit test is inverted to drive the new round-trip. * s3api: drop redundant refreshIAMConfiguration from Put/DeleteGroupPolicy DoActions already triggers ReloadConfiguration for both actions via the explicit reload list, so calling refreshIAMConfiguration inline runs the load twice per request. Per PR review. * s3api: scope group-policy resource names per test; tighten deny polling - Integration test resource names get a per-test suffix so retried or parallel CI jobs don't trip EntityAlreadyExists / BucketAlreadyExists. - Deny-path Eventually loops gate on AccessDenied via a typed helper rather than any non-nil error; transient setup errors no longer end the wait prematurely. - ListGroupPolicies returns ServiceFailure when the credential manager is nil, matching Put/Get/DeleteGroupPolicy. * test(s3 iam): cover both IPv4 and IPv6 loopback in allow CIDRs CI runners with happy-eyeballs resolve `localhost` to ::1 first, in which case a 127.0.0.0/8-only allow would silently never match and the deny-driven enforcement test would hang for the allow case. Add ::1/128 to every loopback-matching policy so the allow path works regardless of which loopback family the SDK lands on. |
||
|
Chris Lu
|
f72983c1fd |
fix(s3): stop S3 Tables routes from swallowing buckets named "buckets" or "get-table" (#9566)
* fix(s3): stop S3 Tables routes from swallowing buckets named "buckets" or "get-table"
The S3 Tables REST endpoints share top-level paths with the regular S3
API (/buckets for ListTableBuckets/CreateTableBucket, /get-table for
GetTable). They are registered first on the same router as the bucket
subrouter, so a path-style request such as GET /buckets?list-type=2 on
a bucket actually named "buckets" matched ListTableBuckets and returned
JSON. AWS SDK V2 (and Hadoop s3a / Spark) then failed XML parsing with
"Unexpected character '{' (code 123) in prolog".
Disambiguate by requiring the AWS V4 credential scope to name the
s3tables service on the colliding routes. Regular S3 SDKs sign with
service=s3, S3 Tables SDKs sign with service=s3tables, and the scope is
present in both the Authorization header and the X-Amz-Credential query
parameter for presigned URLs, so the matcher works for both flavors.
ARN-bearing S3 Tables routes (/buckets/<arn>, /namespaces/<arn>, etc.)
already cannot collide because colons are not valid in bucket names, so
they are left untouched.
* fix(s3): accept AWS JSON RPC content type as S3 Tables intent signal
The Iceberg catalog integration tests send unsigned PUT /buckets with
Content-Type: application/x-amz-json-1.1 to create table buckets. With
only the credential-scope check, those requests fell through to the
regular S3 CreateBucket handler and the suite went red on this branch.
Extend the matcher so a request is recognized as S3 Tables when either:
- its AWS V4 credential scope names SERVICE=s3tables; or
- it carries the canonical AWS JSON RPC 1.1 content type and is
unsigned (a request explicitly signed for SERVICE=s3 still wins).
The regular S3 SDKs do not send application/x-amz-json-1.1, so the
signal is safe for the colliding paths (/buckets, /get-table).
Also add an AWS SDK V2 for Go integration test under
test/s3/sdk_v2_routing/ that drives the SDK's own XML deserializer
against a bucket literally named "buckets" and "get-table" — the SDK
errors before the test asserts if the server returns the wrong body
shape. Wired up via .github/workflows/s3-sdk-v2-routing-tests.yml,
mirroring the etag/acl workflow.
* s3api: extend service matcher to all S3 Tables routes; simplify scope check
- Apply serviceMatcher to every S3 Tables route, not just the bare-path
ones. ARN-bearing paths could otherwise be hit by an S3 object key
that starts with arn:aws:s3tables:..., inside a bucket named
"buckets", "namespaces", "tables", or "tag". One matcher everywhere
closes both collision classes.
- Replace strings.Split + index lookup with strings.Contains for the
credential-scope check. The scope shape is fixed at
AK/DATE/REGION/SERVICE/aws4_request, slashes only delimit components,
and access keys are alphanumeric — so /s3tables/ matches iff SERVICE
is exactly s3tables. Existing unit cases (including the
access-key-substring case) still pass.
- Read the GetObject body in the SDK v2 routing test with io.ReadAll;
the single Read could return short and make the equality check flaky.
* s3api: drop content-type fallback; sign s3 tables harness traffic instead
The content-type fallback in isS3TablesSignedRequest let an anonymous
regular-S3 request whose body type is application/x-amz-json-1.1 hit
an S3 Tables route when the path-style object key happened to be
shaped like an S3 Tables ARN (e.g. PutObject on bucket "buckets"
with key arn:aws:s3tables:.../bucket/foo/policy). Narrow the matcher
back to the AWS V4 credential scope so only requests signed for
SERVICE=s3tables match the S3 Tables routes.
Update the Iceberg catalog test harness — the only caller still
sending unsigned PUT /buckets — to sign with SERVICE=s3tables. The
mini instance runs in default-allow mode, so the signature itself is
not verified; only the credential scope matters for the route match.
Drop the stale unit cases for the JSON-RPC content-type signal and
the routing test that exercised unsigned harness traffic.
|
||
|
Chris Lu
|
b4289abb0a |
admin: convert filer address to gRPC form before dispatch (#9523)
The master returns each registered filer in pb.ServerAddress dual-port form (host:httpPort.grpcPort, e.g. 10.0.0.1:8888.18888). The admin's plugin context builder forwarded that string verbatim as filer_grpc_address, so workers calling grpc.DialContext on it failed every job in ~3ms with "dial tcp: lookup tcp/8888.18888: unknown port". Run each entry through pb.ServerAddress.ToGrpcAddress before populating ClusterContext.FilerGrpcAddresses. The lifecycle integration test now pins filer.port.grpc to a value that breaks the FILER_PORT+10000 assumption, and a new dispatch test drives the admin's /api/plugin/job-types/s3_lifecycle/run path end-to-end and asserts the dispatched job both reaches the filer and deletes the backdated object. |
||
|
Chris Lu
|
b1d59b04a8 |
fix(s3/lifecycle): walker dispatch uses entry.Path for ABORT_MPU (#9477)
* fix(s3/lifecycle): WalkerDispatcher uses entry.Path for ABORT_MPU + shell announces load Two CI-surfaced bugs caught by PR #9471's S3 Lifecycle Tests run on master after PRs #9475 + #9466: 1. Walker dispatch for ABORT_MPU was sending entry.DestKey as req.ObjectPath. The server's ABORT_MPU handler (weed/s3api/s3api_internal_lifecycle.go) strips the .uploads/ prefix to extract the upload id and reads the init record from that directory, so it expects the .uploads/<id> path verbatim. DestKey looks like a regular object path; the server's prefix check fails and the dispatch returns BLOCKED with "FATAL_EVENT_ERROR: ABORT_MPU object_path missing .uploads/ prefix". The test fix renames TestWalkerDispatcher_MPUInitUsesDestKey to ...UsesUploadsPath and inverts the assertion to match the actual server contract. DestKey is still used for the WalkBuckets shard predicate and for rule-prefix matching in bootstrap.walker; both surfaces want the user's intended path, while DISPATCH wants the .uploads/<id> directory. The bootstrap test (TestLifecycleAbortIncompleteMultipartUpload) caught this when the walker's BLOCKED error surfaced as FATAL output. 2. test/s3/lifecycle/s3_lifecycle_empty_bucket_test.go asserts the shell command logs "loaded lifecycle for N bucket(s)" so a regression that produces half-shaped output (no load summary) is caught. The restored shell command (PR #9475) didn't print that line; add it back on the first pass that finds non-zero inputs. * fix(s3/lifecycle): walker fires for walker-only buckets (empty replay path) runShard's empty-replay sentinel (rsh == [32]byte{}) was returning BEFORE the steady-state walker check. A bucket whose only lifecycle rule was walker-only (ExpirationDate / ExpiredDeleteMarker / NewerNoncurrent) would never have it dispatched because: - ReplayContentHash only hashes replay-eligible kinds, so walker-only-only snapshots produce rsh == empty. - The early-return persisted the empty cursor and exited before the steady-state walker block at the bottom of the function. Move the walker invocation INTO the empty-replay branch so walker- only rules dispatch on the same path as mixed-rule buckets. TestLifecycleExpirationDateInThePast and TestLifecycleExpiredDeleteMarkerCleanup were both timing out their "object must be deleted" Eventually polls because of this. Caught on PR #9471's S3 Lifecycle Tests run after PR #9475 restored the shell entry point that exercises the integration tests. * fix(s3/lifecycle): cold-start walker covers pre-existing objects runShard only walked the bucket tree on the recovery branch (found && hash mismatch). For a fresh worker with no persisted cursor, found=false, so the recovery walker never fired and the meta-log replay only scanned runNow - maxTTL of events. Objects PUT before that window — including pre-existing objects in a newly-rule-enabled bucket — never matched the rule. The streaming worker handled this with scheduler.BucketBootstrapper. Daily-replay needed the equivalent: walk the live tree once on the first run for each shard so pre-existing objects get evaluated even when their PUT events are outside meta-log scan window. Restructured the recovery branch to fire the walker on either (found && mismatch) OR !found. On cold-start the cursor isn't rewound — we keep TsNs=0 and let the drain below floor to runNow - maxTTL like before; the walker just handles whatever the sliding window can't reach. TestLifecycleBootstrapWalkOnExistingObjects was the exact CI failure this addresses (https://github.com/seaweedfs/seaweedfs/actions/runs/25777823522/job/75714014151). * fix(s3/lifecycle): restore walker tag and null-version state Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(s3/lifecycle): parallelize shell shard sweeps Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(s3/lifecycle): bound each runPass ctx + refresh in runLifecycleShard Two CI bugs surfaced after PR #9466 deleted the streaming worker: 1. The shell command's -refresh loop never fires. runPass used the outer ctx (full -runtime), so dailyrun.Run blocked for the entire 1800s s3tests window — the background worker only ran one pass and never re-loaded configs that tests created mid-run. test_lifecycle_expiration sees 6 objects when expecting 4 because expire1/* never reaches the worker's snapshot. Cap each pass to cadence+5s when cadence>0; one-shot (cadence=0) keeps the full ctx. 2. TestLifecycleExpiredDeleteMarkerCleanup's docstring says "pass 1 cleans v1; pass 2 removes the now-orphaned marker," but runLifecycleShard invoked with no -refresh — only one pass ran. The marker rule can't fire in the same pass that dispatches v1's delete because v1 is still in .versions/. Add -refresh 1s so the 10s runtime gets multiple passes. * fix(s3/lifecycle): persist cursor with fresh ctx after passCtx timeout drainShardEvents only exits via ctx cancellation for an idle subscription — that's the steady-state when all replayed events are already past. Saving the cursor with the canceled passCtx silently drops every advance, so the next pass re-subscribes from the same floor and re-replays the same events. Symptom in s3tests: status=error shards=16 errors=16 on every pass, and 1/6 expire3/* dispatches lost to a race between concurrent shard drains all retrying the same events. Use a 5s timeout derived from context.Background for the save, and treat passCtx Deadline/Canceled from drain as a clean end-of-pass — not a shard-level error to log. * fix(s3/lifecycle): trust persisted cursor; never bump past pending events The drain freezes cursorAdvanceTo at the last pre-skip event so pending matches (DueTime > runNow) re-enter the subscription next pass. Combined with the new cursor persistence, the floor bump (runNow - maxTTL) then orphans the very events the drain stopped at. Concrete: a rule with TTL == maxTTL fires at runNow == PUT_TIME + maxTTL, so floor (= runNow - maxTTL) lands exactly on PUT_TIME. If the last advance saved a cursor right before the not-yet-due PUT (e.g., keep2/* between expire1/* and expire3/* on the same shard), the floor bump on pass 9 skips past the expire3 event itself — the worker never re-reads it. Test symptom: expire3/* never expires when worker shards include other earlier no-match events. Cold start (found=false) still subscribes from runNow - maxTTL. Steady state honors the cursor verbatim. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Chris Lu
|
69da20bdae |
volume: gate FetchAndWriteNeedle behind admin auth and refuse internal endpoints (#9441)
volume: require admin auth and refuse loopback endpoints in FetchAndWriteNeedle Gate the RPC behind checkGrpcAdminAuth for parity with the rest of the destructive volume-server RPCs, and reject cluster-internal remote S3 endpoints (loopback / link-local / IMDS / RFC 1918 / CGNAT) before dialing. Pin the validated address against DNS rebinding by routing the AWS SDK through an HTTP transport whose DialContext re-resolves the host and re-applies the deny list on every dial, so an endpoint that resolves to a public IP at validate-time and then flips to 127.0.0.1 at connect time is refused. Operators that legitimately fetch from private hosts can opt out with -volume.allowUntrustedRemoteEndpoints. |
||
|
Chris Lu
|
46bb70d93e |
feat(s3): stamp noncurrent_since on versioned demotions (#9431)
* feat(s3): stamp noncurrent_since on versioned demotions A version's noncurrent TTL clock starts when the next version is written, not at its own mtime. Today the lifecycle engine derives that moment from the next-newer sibling's mtime — a heuristic that drifts if the sibling is later modified and is unavailable when the demoting event sits outside meta-log retention. Stamp Seaweed-X-Amz-Noncurrent-Since-Ns on the demoted entry at the two places where a PUT flips the latest pointer: updateLatestVersionInDirectory and updateIsLatestFlagsForSuspendedVersioning. Timestamp source is time.Now().UnixNano() captured once per demotion — the documented Phase 1 fallback until the filer write API surfaces its own TsNs. Engine reads the stamp on both the bootstrap walker path and the event-driven router; missing/zero falls back to the legacy sibling-mtime derivation, so pre-stamp entries keep working. Prerequisite for the daily-replay lifecycle worker (Phase 2+). * fix(s3): address CI failure and PR review feedback - Backdating tests must move both clocks: the lifecycle integration tests backdate version mtimes to simulate aging, but my earlier commit made the engine prefer the explicit demotion stamp over sibling mtime, so a real-now stamp dominated a backdated mtime and the rule never fired. Update backdateVersionedMtime to also rewrite Seaweed-X-Amz-Noncurrent-Since-Ns when the entry already carries it. This is a test simplification — production stamps record when the successor was written, not the demoted version's own mtime — but the resulting clock is correctly old enough. - Refactor stamp parsing into one shared helper. Per gemini-code-assist: the parsing logic for ExtNoncurrentSinceNsKey was duplicated in router/router.go and scheduler/bootstrap.go. Move it to a new weed/s3api/s3lifecycle/noncurrent_since.go as exported SuccessorFromEntryStamp; both call sites now go through it. - Make the parser ordering test deterministic. Per coderabbitai: time.Now().UnixNano() drops the monotonic clock component, so two back-to-back calls can decrease if the wall clock steps backward — the prior test was exercising OS clock behavior rather than the parser. Replace with fixed nanosecond values. - Close a suspended-versioning race. Per coderabbitai: the prior putSuspendedVersioningObject called updateIsLatestFlagsForSuspendedVersioning after putToFiler returned, i.e. after the object write lock released. A concurrent PUT could promote a newer latest version, which we'd then wipe — leaving the older "null" object incorrectly current. Move the cleanup into the afterCreate callback so the null write and the .versions pointer clear (including the new demotion stamp) run atomically under the same lock. Best-effort logging is preserved. * fix(s3/lifecycle): clear noncurrent_since stamp on test backdate Backdating a version's mtime in tests is not a coherent claim about when it became noncurrent — production stamps record the successor's PUT time, which the test doesn't manipulate. The prior commit rewrote the stamp to the backdated instant, but for TestLifecycleNewerNoncurrent that creates an inconsistent state: v3's stamp says "demoted 30 days ago" while v4's mtime (the supposed demoter) is real-now. With both NewerNoncurrentVersions and NoncurrentDays in the same rule, the NoncurrentDays floor passes against the backdated stamp and the rank-based check then deletes v3 via the meta-log historical replay that misranks against current state. Clearing the stamp instead lets the lifecycle engine fall back to the sibling-mtime derivation the tests were originally written against: the legacy code path is preserved end-to-end while the new explicit- stamp path is exercised by the unit tests in s3lifecycle/noncurrent_since_test.go and the bootstrap-walker integration in scheduler/bootstrap_test.go. The deeper interaction — historical meta-log replay ranking against current state inside routePointerTransitionExpand — is pre-existing and is no longer masked by the freshly-PUT successor's mtime once the stamp is read. Tracked separately; not blocking this PR. * fix(s3): stamp noncurrent_since before the .versions/ pointer flip The pointer-flip on the .versions/ directory emits a meta-log event that the lifecycle router consumes via routePointerTransition. The router then calls LookupVersion on the demoted version's id. With the prior ordering — pointer flip first, stamp second — the router could read the demoted entry before markVersionNoncurrent landed and fall back to the legacy sibling-mtime derivation. Versioned COPY is the clean break: the new latest version keeps the source object's mtime instead of recording the moment v_old was demoted, so the fallback's successor clock can be arbitrarily wrong. Reorder both updateLatestVersionInDirectory and updateIsLatestFlagsForSuspendedVersioning so the stamp is written first; the pointer flip then emits an event into a state where the stamp is already present. Failure of the stamp write remains non-fatal — lifecycle still falls back to the legacy derivation in that case, with the same caveats as before the PR but no race window. |
||
|
Chris Lu
|
c7b01c72b2 |
test(s3/lifecycle): integration coverage for versioning + filters (#9415)
* test(s3/lifecycle): integration coverage for versioning + filters
First integration-test bundle building on the existing single-test
backdating harness. Each scenario follows the same shape: create
bucket, set lifecycle, PUT object, backdate mtime via filer
UpdateEntry, run the shell command for one shard sweep, assert
S3-side state.
Five new tests:
- TestLifecycleVersionedBucketCreatesDeleteMarker: Expiration on a
versioned bucket must produce a delete marker (latest after worker
runs is a marker) AND keep the original version directly addressable
by versionId. ListObjectVersions confirms IsLatest=true on the
marker.
- TestLifecycleNoncurrentVersionExpiration: NoncurrentVersionExpiration
fires only on demoted versions. PUT v1, PUT v2 (so v1 → noncurrent),
backdate v1, run worker. v1 must be gone, v2 still current.
- TestLifecycleExpiredDeleteMarkerCleanup: combined rule (noncurrent +
expired-delete-marker) cleans up a sole-survivor marker. PUT v1,
DELETE (creates marker), backdate both, run worker. Every version
AND marker must be gone for the key.
- TestLifecycleDisabledRuleSkipsObject: rule with Status=Disabled
must not produce dispatches even on a backdated match. Negative
test for the engine's enabled-status gate.
- TestLifecycleTagFilter: rule with And{Prefix, Tag} only matches
objects carrying the tag. Two backdated objects (one tagged, one
not) — only the tagged one is removed.
Helpers extracted to keep each test focused: putVersioningEnabled,
putNoncurrentExpirationLifecycle, putExpiredDeleteMarkerLifecycle,
backdateVersionedMtime (ages a specific .versions/v_<id> entry),
runLifecycleShard (one-shot shell invocation with FATAL guard).
* test(s3/lifecycle): tighten noncurrent expiration diagnostics
Local run showed TestLifecycleNoncurrentVersionExpiration failing
with a bare 404 on HEAD(latest), not enough to tell whether v2 was
deleted, the bare-key pointer was removed, or a delete marker was
synthesized. Strengthen the test to:
- HEAD by versionId=v2 first, so we pin "v2 file still on disk"
separately from "the latest pointer resolves to v2"
- on HEAD(latest) failure, log ListObjectVersions output (versions +
markers, with IsLatest) so the next failure shows which side the
bug is on rather than just NotFound
* test(s3/lifecycle): integration coverage for AbortIncompleteMultipartUpload
Exercises the lifecycleAbortMPU handler path that the prefix-based
expiration tests can't reach — routing keys off of .uploads/<id>/
directory events, not regular object events, and the dispatcher uses
a different RPC path (rm on the .uploads/<id>/ folder).
Setup: AbortIncompleteMultipartUpload rule with DaysAfterInitiation=1,
CreateMultipartUpload, UploadPart (so the directory carries the
right shape), backdate the .uploads/<uploadID>/ directory entry 30
days, run the worker. The upload must drop out of
ListMultipartUploads.
Helpers added: putAbortMPULifecycle, backdateUploadDir.
* test(s3/lifecycle): integration coverage for NewerNoncurrentVersions
NewerNoncurrentVersions=N keeps the N most recent noncurrent versions
and expires the rest. Distinct from per-version NoncurrentDays —
depends on per-version rank, not just per-version age — and routes
through routePointerTransition's "needs full expansion" path.
Setup: PUT v1, v2, v3, v4 on a versioned bucket (v4 current; v1-v3
noncurrent), backdate v1+v2+v3 so all satisfy the NoncurrentDays>=1
floor, run the worker. Expect v1+v2 expired (older noncurrent),
v3 (newest noncurrent within keep=1) and v4 (current) preserved.
Helper added: putNewerNoncurrentLifecycle.
* test(s3/lifecycle): integration coverage for suspended-versioning Expiration
Suspended versioning takes a distinct code path in lifecycleDispatch:
the VersioningSuspended branch first deletes the null version (via
deleteSpecificObjectVersion(versionId="null")) and then writes a
fresh delete marker on top. Other branches (Enabled → only writes a
marker; Off → straight rm) miss this two-step.
Setup: enable versioning, PUT v1 (real versionId), suspend
versioning, PUT again (creates the null version, demotes v1 to
noncurrent), set the Expiration rule, backdate the null at the
bare path. Expect: latest is now a fresh delete marker, the
"null" version is gone from ListObjectVersions, and v1 (noncurrent
under Enabled) still addressable directly — suspended Expiration
must only touch the null, not other versions.
Helper added: putVersioningSuspended.
* test(s3/lifecycle): integration coverage for multi-bucket sweep
A single shell-driven shard sweep must process every bucket carrying
lifecycle config, not just the first one alphabetically. Pinned
because the scheduler iterates the buckets directory and a regression
that returns early after the first match would silently disable
lifecycle for every later bucket.
Two buckets, each with their own prefix-expiration rule and a
backdated object. Both must be expired after the same sweep.
* test(s3/lifecycle): integration coverage for ObjectSizeGreaterThan filter
ObjectSizeGreaterThan is a strict > gate (filterAllows uses
ev.Size <= rule.FilterSizeGreaterThan to reject). Pinned at the
boundary: an object whose size equals the threshold must remain;
only an object strictly larger expires. Catches a > vs >= flip.
Two backdated objects on the same prefix, sizes 100 and 150 with
threshold=100 — boundary survives, larger expires.
* test(s3/lifecycle): scrub bucket lifecycle config + versions on cleanup
Tests share one weed mini server. Two pollution modes were producing
order-dependent failures:
- A later test's shard sweep would still load the prior test's
lifecycle config (the worker reads every bucket's XML from filer
state, and DeleteBucket alone doesn't drop lifecycle config
cleanly on this codebase).
- Versioned-bucket tests left versions + delete markers behind that
ListObjectsV2 can't see, so the existing best-effort empty-then-
delete didn't actually empty those buckets.
- The AbortMPU test intentionally leaves an in-flight upload; without
an explicit AbortMultipartUpload the bucket DELETE hits NotEmpty.
Cleanup now runs DeleteBucketLifecycle, ListObjectVersions →
DeleteObject(versionId), ListObjectsV2 → DeleteObject (catches what
ListObjectVersions missed), ListMultipartUploads → AbortMultipartUpload,
then DeleteBucket. Best-effort throughout so a half-torn-down bucket
doesn't fail the cleanup chain.
* test(s3/lifecycle): backdate both versions for NoncurrentDays clock
Per codex review: NoncurrentDays is clocked from the SUCCESSOR
version's mtime (when the displaced version became noncurrent), not
from the displaced version's own mtime. Backdating only v1 left the
clock (v2's mtime) at "now" and the rule never fired — the test was
wrong, not the production path.
Backdate v1=31d and v2=30d so v1 sits past the 1-day threshold
relative to v2, the noncurrent rule fires, and v2 stays current.
* test(s3/lifecycle): assert specific NotFound on multi-bucket deletion
Per codex review: TestLifecycleMultipleBucketsInOneSweep treated any
HeadObject error as "deleted", which lets a transport failure or
dead endpoint mask a real bug. Recognize NoSuchKey/NotFound/HTTP-404
specifically via a small isS3NotFound helper so the assertion
actually proves deletion happened, not just that the call broke.
* test(s3/lifecycle): gofmt size-filter test
* test(s3/lifecycle): integration coverage for Object Lock skip
Object Lock retention must override the lifecycle rule. The handler's
enforceObjectLockProtections check (s3api_internal_lifecycle.go:47)
returns an error when retention is active; the dispatcher then
classifies the outcome as SKIPPED_OBJECT_LOCK and the object stays.
No existing integration test reaches that outcome.
Setup: bucket created with ObjectLockEnabledForBucket=true, expiration
rule on prefix "lock/", two backdated objects under the same prefix —
one with GOVERNANCE retention until 1h from now, one without. After
the worker runs, the unlocked object expires (positive control); the
locked one survives.
Custom cleanup uses BypassGovernanceRetention so the test can drop
the locked version when the test finishes — otherwise the retention
window keeps the bucket from being deleted.
* test(s3/lifecycle): integration coverage for config update between sweeps
An operator changes the lifecycle rule between two shell-driven
sweeps. The second sweep must respect the NEW rule, not a cached
copy of the old one. Each runLifecycleShard invocation spawns a
fresh weed shell subprocess, so cached engine state from a previous
sweep doesn't persist — but a regression that caches rules across
PutBucketLifecycleConfiguration calls within the S3 server itself
would still surface here.
Sweep 1: rule prefix="first/", PUT + backdate firstKey, run worker
→ firstKey expires.
Update rule to prefix="second/", PUT + backdate secondKey AND a
new key under the OLD prefix ("first/post-update.txt"). Sweep 2
must expire only the second-prefix object; the post-update old-
prefix one must survive — config replacement, not merge.
* test(s3/lifecycle): integration coverage for ExpirationDate (past)
Rules with Expiration{Date: <past>} route through ScanAtDate in the
engine (decideMode's ActionKindExpirationDate case) — a separate
compile + dispatch branch from the EventDriven delay-group path the
Days-based tests exercise.
Past date + in-prefix object → must expire. Out-of-prefix object →
must remain. Object also backdated as defense-in-depth so the
assertion doesn't depend on whether the dispatcher consults
MinTriggerAge for date kinds.
* test(s3/lifecycle): integration coverage for bootstrap walk on existing objects
Production scenario: operator enables lifecycle on a bucket that
already holds objects from before the policy. The worker must
discover them via the bootstrap walk (BucketBootstrapper) — there
were no meta-log events to observe because the objects predate the
rule. Without the bootstrap path, only NEW writes would ever match.
Setup: PUT 5 objects (no lifecycle config yet) + 1 out-of-prefix
survivor, backdate all, THEN set the Expiration rule, run the
worker. Every in-prefix pre-existing object must be expired; the
out-of-prefix one must remain.
* test(s3/lifecycle): integration coverage for DeleteBucketLifecycle stops dispatching
Operator UX: after DeleteBucketLifecycle, the worker must observe the
removal on the next sweep and stop expiring objects under the now-gone
rule. A regression that caches old configs across
PutBucketLifecycleConfiguration → DeleteBucketLifecycle would keep
silently dropping objects.
Setup: positive control (rule active, backdated obj expires) →
DeleteBucketLifecycle → PUT + backdate a fresh object → second
sweep. The fresh object must remain.
* test(s3/lifecycle): integration coverage for empty bucket sweep no-op
A bucket carrying lifecycle config but no objects must produce a
successful sweep — no hangs, no errors, no dispatches. Pinned
because the bootstrap walker iterates bucket directories, and an
empty directory is a corner of that traversal that's easy to break
(slice-bounds bug on the first listing returning zero entries).
Asserts: worker logs "loaded lifecycle for" and "shards 0-15
complete", no FATAL output, bucket still exists after the sweep.
* test(s3/lifecycle): fix Object Lock backdate path + skip unwired ScanAtDate
ObjectLock: enabling Object Lock on a bucket implicitly enables
versioning, so PUT objects land at .versions/v_<id>, not at the bare
key. The test was calling backdateMtime (bare path) and failing in
the helper with "filer: no entry is found". Switch to
backdateVersionedMtime with the versionId returned by PutObject.
ExpirationDate: ScanAtDate dispatch path isn't wired to the run-shard
shell command yet — the bootstrap walker explicitly skips actions in
ModeScanAtDate (walker.go:141 says "SCAN_AT_DATE runs its own date-
triggered bootstrap" but no such bootstrap exists in the scheduler or
shell). Skip with a t.Skip + explanation so the test activates the
moment the date-triggered path lands.
* fix(s3/lifecycle): wire ExpirationDate dispatch through bootstrap walker
The walker explicitly skipped ModeScanAtDate actions on the comment
"SCAN_AT_DATE runs its own date-triggered bootstrap" — but no such
bootstrap exists in the scheduler or shell layer. The result: rules
with Expiration{Date: ...} compiled correctly, populated the
snapshot's dateActions map, and were never dispatched.
ExpirationDate is silently a no-op in production.
EvaluateAction already handles ActionKindExpirationDate correctly
(rejects when now.Before(rule.ExpirationDate), otherwise emits
ActionDeleteObject). The walker just needed to fall through instead
of skipping. Pre-date walks become no-ops via EvaluateAction's date
check; post-date walks expire eligible objects.
Un-skip TestLifecycleExpirationDateInThePast — it now exercises the
fixed path end-to-end.
* test(s3/lifecycle): integration coverage for multiple rules per bucket
A single bucket carries two independent Expiration rules with disjoint
prefix filters and different Days thresholds. Each rule must fire
only on its prefix; objects outside both prefixes must survive.
Pinned because Compile builds one CompiledAction per rule per kind
all sharing the same bucket index — a bug that lets one rule's
prefix or threshold leak into another (e.g. last-write-wins on a
shared map) would silently expire wrong objects.
Setup: rule A with prefix=logs/ Days=1, rule B with prefix=tmp/
Days=7. Three backdated objects: logs/access.log, tmp/scratch.bin,
data/keep.bin. After the worker runs, logs/ + tmp/ are gone;
data/ — outside both rule prefixes — survives.
* fix(s3/lifecycle): mark ScanAtDate actions active in Compile
Two layers were silently filtering ScanAtDate actions out of routing:
the walker's mode skip (fixed in
|
||
|
Chris Lu
|
85abf3ca88 |
feat(shell): s3.lifecycle.run-shard + integration test (#9361)
* feat(shell): s3.lifecycle.run-shard for manual Phase 3 dispatch Subscribes to the filer meta-log filtered to one (bucket, key-prefix-hash) shard, routes events through the compiled lifecycle engine, and dispatches due actions to the S3 server's LifecycleDelete RPC. Persists the per-shard cursor to /etc/s3/lifecycle/cursors/shard-NN.json so subsequent runs resume. Operator-runnable harness for end-to-end Phase 3 validation while the plugin-worker auto-scheduler is still pending. EventBudget bounds a single invocation; flags expose dispatch + checkpoint cadence. Discovers buckets by walking the configured DirBuckets path and reading each bucket entry's Extended[s3-bucket-lifecycle-configuration-xml] through lifecycle_xml.ParseCanonical. All compiled actions are seeded BootstrapComplete=true so the run dispatches whatever fires immediately; production bootstrap walks set this incrementally per bucket. * test(s3/lifecycle): integration test driving the run-shard shell command Spins up 'weed mini', creates a bucket with a 1-day expiration on a prefix, PUTs the target object, then rewrites the entry's Mtime via filer UpdateEntry to 30 days ago. Runs 's3.lifecycle.run-shard' for every shard via 'weed shell' subprocess and asserts the backdated object is deleted within 30s, and the in-prefix-but-recent object remains. The S3 API rejects Expiration.Days < 1, so 'wait a day' is unworkable. Backdating via the filer's gRPC sidesteps that constraint while still exercising the real Reader -> Router -> Schedule -> Dispatcher -> LifecycleDelete RPC path end-to-end. Wires a new s3-lifecycle-tests job into s3-go-tests.yml. The test runs all 16 shards because ShardID(bucket, key) is hash-based and the test shouldn't couple to that detail; running every shard keeps the test independent of the hash function. * fix(shell/s3.lifecycle.run-shard): address review findings - Reject negative -events explicitly. Help text already defines 0 as unbounded; negative budgets created ambiguous behavior in pipeline.Run. - Bound the gRPC dial with a 30s timeout instead of context.Background() so an unreachable S3 endpoint doesn't hang the shell. - Paginate the bucket listing in loadLifecycleCompileInputs. SeaweedList takes a single-RPC limit; the prior 4096 silently dropped buckets past that page on large clusters. Loop with startFrom until a page comes back short. - Surface parse errors instead of swallowing them. Buckets with malformed lifecycle XML now print the first three errors verbatim and a count for the rest, so an operator running this command for diagnostics can find what's wrong. * feat(shell/s3.lifecycle.run-shard): -shards range/set with one subscription Adds -shards "lo-hi" or "a,b,c" to the manual run command and threads the same model through Reader and Pipeline. - reader.Reader gains ShardPredicate (func(int) bool) and StartTsNs; ShardID stays for the single-shard short form. Event carries the computed ShardID so consumers can route per-shard without rehashing. - dispatcher.Pipeline gains Shards []int. When set, Run holds one Cursor + Schedule + Dispatcher per shard, opens one filer SubscribeMetadata stream with a predicate covering the whole set, and routes events into the matching shard's schedule from a single dispatch goroutine — no per-shard goroutine fan-out. - shell command parses -shard or -shards (mutually exclusive), formats progress messages with a contiguous-range label when applicable, and validates against ShardCount. Integration test now uses -shards 0-15 (one subprocess invocation) instead of a 16-iteration loop. * fix(s3/lifecycle): allow Reader with StartTsNs=0 + Cursor=nil The reader rejected the legitimate 'fresh subscription from epoch' state when called from a fresh Pipeline.Run on a multi-shard worker (no cursor file yet, all shards' MinTsNs=0). The downstream SubscribeMetadata call handles SinceNs=0 fine; the up-front check was over-defensive and broke the auto-scheduler completely (CI showed 5-second-cadence retries with this exact error). * fix(s3/lifecycle): schedule from ModTime not eventTime A backdated or out-of-band entry update has eventTime ≈ now while ModTime is far in the past; eventTime+Delay would push the dispatch into the future even though the rule already fires. ModTime+Delay is the correct fire moment. The dispatcher's identity-CAS still catches drift between schedule and dispatch. * fix(s3/lifecycle): -runtime cap on run-shard so it exits on quiet shards The CI integration test sets -events 200 expecting the subprocess to return after 200 in-shard events. But -events counts only events that pass the shard filter; the test produces ~5 such events (bucket create, lifecycle PUT, two object PUTs, mtime backdate), so the reader stays in stream.Recv forever and runShellCommand hangs the test deadline. - weed/shell/command_s3_lifecycle_run_shard.go: add -runtime D flag. When > 0, Pipeline.Run runs under context.WithTimeout(D); on expiry the reader/dispatcher drain cleanly and the cursor saves. - weed/s3api/s3lifecycle/dispatcher/pipeline.go: treat context.DeadlineExceeded the same as context.Canceled at exit (both are graceful shutdown signals). * test(s3/lifecycle): pass -runtime 10s to run-shard Pair with the new -runtime flag so the subprocess exits cleanly after 10s instead of waiting for an event budget that never lands on quiet shards. * refactor(s3/lifecycle): extract HashExtended to s3lifecycle pkg The worker's router needs the same length-prefixed sha256 of the entry's Extended map; pulling it out of the s3api private file lets both sides import it. * fix(s3/lifecycle): worker captures ExtendedHash for identity-CAS Without this, the dispatcher sends ExpectedIdentity.ExtendedHash = nil while the live entry on the server has a non-nil hash, so every dispatch returns NOOP_RESOLVED:STALE_IDENTITY and nothing is ever deleted. * fix(s3/lifecycle): identity HeadFid via GetFileIdString Meta-log events go through BeforeEntrySerialization, which clears FileChunk.FileId and writes the Fid struct instead. Reading .FileId directly returns "" on the worker side while the server's freshly fetched entry still has a populated string, so the identity-CAS would mismatch and every expiration ended in NOOP_RESOLVED:STALE_IDENTITY. * fix(s3/lifecycle): treat gRPC Canceled/DeadlineExceeded as graceful exit errors.Is doesn't unwrap a gRPC status error back to the stdlib ctx errors, so a subscription that ends because runCtx was canceled was being logged as a fatal reader error. Check status.Code as well so the shell's -runtime cap exits cleanly. * fix(test/s3/lifecycle): pass the gRPC port (not HTTP) to run-shard run-shard's -s3 flag dials the LifecycleDelete gRPC service, which listens on s3.port + 10000. The integration test was passing the HTTP port instead, so the dispatcher's RPC just timed out and the shell command exited under -runtime with no work done. * chore(test/s3/lifecycle): drop emoji from Makefile output * docs(test/s3/lifecycle): correct '-shards 0-15' wording * fix(s3/lifecycle): reject out-of-range shard IDs in Pipeline.Run The shell's parseShardsSpec already validates, but a programmatic caller (scheduler, future worker config) shouldn't be able to silently produce no-op states by passing -1 or 99. * fix(s3/lifecycle): bound drain + final-save with their own timeouts Shutdown was using context.Background, so a stuck dispatcher RPC or filer save could keep Pipeline.Run from ever returning. * fix(test/s3/lifecycle): drop self-killing pkill in stop-server The pkill pattern \"weed mini -dir=...\" is also in the running shell's argv (it's the recipe body), so pkill -f matches its own bash and the recipe exits with Terminated. CI test job passed but the cleanup step failed with exit 2. The PID file is sufficient on its own. * docs(test/s3/lifecycle): document S3_GRPC_ENDPOINT env var |
||
|
Chris Lu
|
12f283357f |
fix(iam): four phase-3 follow-ups (provider scoping, public path wrapper, static mirror, claim-mode RoleArn) (#9333)
* fix(iam): scope IAM-managed OIDC provider lookup by role account Two account-scoped OIDC records sharing an issuer were collapsed into a single map slot keyed only by the URL. The last-write-wins entry then served every AssumeRoleWithWebIdentity, so a token destined for account B's role could be validated by account A's record (its clientIDs and thumbprints), defeating the per-account isolation the records exist for. The role-account check in enforceProviderAccountScope still rejected the cross-account assumption, but only after the wrong record's audience and TLS pin had already accepted the token. Refresh now keys IAM-managed records as (issuer, account), and validation parses the requested role's account up front and matches the record under that issuer in this order: exact account, global (account-less), static-config fallback. An unknown account hint deliberately skips account-scoped entries — picking one arbitrarily is the bug this commit fixes — and falls through to global or static. * fix(iam): route public AssumeRoleWithWebIdentity through IAMManager handleAssumeRoleWithWebIdentity called stsService.AssumeRoleWithWebIdentity directly, bypassing the IAMManager wrapper. The wrapper is where enforceProviderAccountScope rejects cross-account assumption attempts and capDurationByRole clamps to the role's MaxSessionDuration; both silently became no-ops for any AWS-SDK caller hitting the public endpoint. Dispatch through the IAMManager (via the existing IAMManagerProvider interface that other handlers in this file already use) when one is wired. Embedded test setups without an IAM integration fall back to the bare STS service unchanged. * fix(iam): mirror thumbprints, principal-tag keys, and policy claim from static OIDC config initOIDCProviderStore mirrored only URL and ClientIDs. Once RefreshOIDCProvidersFromStore ran (on any IAM-managed mutation, or on boot once the metadata-subscribe loop kicked in), buildOIDCProviderFromRecord rebuilt the runtime provider from this truncated record. Because IAM-managed entries take precedence over the static-config map, the rebuild silently shadowed the bootstrap with a weaker provider: - Thumbprints: dropped, so TLS-pinned issuers fell back to the system trust store. - AllowedPrincipalTagKeys: dropped, so principal-tag claims stopped reaching the session. - PolicyClaim: dropped, so claim-based policy mode stopped triggering. Pull all three from the provider's static Config map at mirror time so the stored record round-trips to a runtime provider equivalent to the one the static config produced directly. * fix(iam): allow empty RoleArn in AssumeRoleWithWebIdentity HTTP handler Phase 3b advertises that RoleArn MAY be omitted in claim-based policy mode — the STS service then derives the assumed-role ARN from the configured policy claim. The HTTP handler still rejected empty RoleArn up front with MissingParameter, so SDK callers using the documented omitted-role flow never reached the STS layer. Drop the pre-check; STS still validates that claim-based mode is configured and that the IDP emits policies, returning a precise error when either is missing. The existing error mapping below this point surfaces those as InvalidParameterValue, matching what an AWS SDK expects. * test(iam): update missing-RoleArn STS integration test for the new contract The previous commit drops the HTTP-layer RoleArn pre-check so claim-based mode can derive the ARN from a JWT claim. The integration test still asserted MissingParameter for the missing-RoleArn case, which now reaches the STS layer and surfaces a JWT-parse error instead. Update the assertion to match: missing RoleArn alone must no longer surface as MissingParameter, but a bogus JWT must still be rejected. |
||
|
Chris Lu
|
6141222ab0 |
fix(test/s3/policy): allocate fresh admin port per subtest (#9332)
* fix(test/s3/policy): allocate fresh admin port per subtest startMiniCluster ran weed mini in-process and explicitly assigned master/volume/filer/s3 ports allocated by MustAllocatePorts, but it left -admin.port and -admin.port.grpc unset, so each subtest reused the hardcoded defaults 23646 / 33646. The package's subtests run sequentially within the same go test process. The previous subtest's admin goroutine is still bound to 23646 by the time the next subtest spins up its own mini, so the new admin can never bind, mini.go's waitForAdminServerReady hits its 240-attempt cap, and glog.Fatalf kills the test binary. This has been the dominant cause of "admin server did not become ready" flakes across recent IAM PRs. Allocate two extra ports for admin and pass them through. The other subprocess-based tests (s3tables/*) are not affected because each launches weed mini in a fresh OS process. * fix(mini): make admin readiness wait context-aware waitForAdminServerReady polled for 240 attempts × 500ms regardless of whether the surrounding mini context was cancelled. When mini is run in-process from a test harness (test/s3/policy/...) and the test calls its cancel func, the leftover wait keeps spinning for the full two minutes and then glog.Fatalf's, terminating the entire test binary — including any sibling subtest that has since started its own mini. Thread the existing miniClientsCtx through the wait so a Stop / cancel returns context.Canceled immediately. The caller (startMiniAdminWithWorker) treats a context-cancelled outcome as a graceful shutdown signal and logs+returns instead of fataling. |
||
 Chris Lu
|
6d95a5592a |
test(s3/policy): stop racing t.TempDir cleanup against mini shutdown
The mini cluster's admin/plugin worker keeps creating files under admin/plugin/job_types/ for ~1s after subtests finish, while the previous Stop() only cancelled an unobserved context and slept 500ms. t.TempDir()'s registered RemoveAll then raced the worker and intermittently failed with "directory not empty" (CI run 25352039081). Manage the data dir manually so it is removed only after the mini goroutine has exited, and wire MiniClusterCtx so cancel actually drains master/volume/filer/admin/s3/webdav. |
||
|
Chris Lu
|
6844ec067c |
fix(s3): cache remote-only source before CopyObject (#9304) (#9305)
* fix(s3): cache remote-only source before CopyObject (#9304) CopyObject from a remote.mount source whose object lives only upstream created a destination entry with FileSize > 0 but no chunks/content, because the resolved source entry has no local chunks and the copy path fell into the "inline/empty chunks" branch with empty entry.Content. A subsequent GET returned 500 with "data integrity error: size N reported but no content available". CopyObjectPart had the same shape via copyChunksForRange iterating an empty chunk list. Detect entry.IsInRemoteOnly() right after resolving the source in both CopyObjectHandler and CopyObjectPartHandler and cache the object to the local cluster first via a new cacheRemoteObjectForCopy helper (a copy-time analogue of cacheRemoteObjectForStreaming with a bounded 30s timeout and version-aware path resolution). If caching fails or produces no chunks, return 503 with Retry-After: 5 instead of writing a metadata-only destination, mirroring the GetObject behavior added in the #7817 cold-cache fix. Adds TestCopyObjectRemoteOnlySourceDetection pinning the four entry shapes the fix branches on plus the pre-fix broken-output shape. * address PR review on remote-only copy fix - Use the resolved entry's version id when srcVersionId is empty so a CopyObject reading the latest object in a versioning-enabled bucket caches the correct .versions/v_<id> path instead of getting stuck in a 503 retry loop. New helper resolvedSourceVersionId handles the fallback for both CopyObject and CopyObjectPart. - Drop the redundant cachedEntry.IsInRemoteOnly() recheck in both handlers; the cache helper now reports success based on local data presence, and IsInRemoteOnly does not look at inline Content so keeping the check would 503 on small inline-cached objects. - Treat inline Content as a successful cache result in both cacheRemoteObjectForStreaming and cacheRemoteObjectForCopy via a shared cachedEntryHasLocalData predicate. The CopyObject inline branch already handles entries that have Content but no chunks. - Extract buildVersionedRemoteObjectPath so the streaming and copy cache helpers share path construction. Adds TestResolvedSourceVersionId and TestCachedEntryHasLocalData to pin the new helpers' contracts. * narrow streaming cache contract back to chunks-only CodeRabbit flagged that cacheRemoteObjectForStreaming's caller in streamFromVolumeServers (lines 997-1002) still required non-empty chunks, so content-only cache hits would fall through to a 503 retry loop instead of being honored. Resolve by keeping the helper's contract chunks-only: the filer's caching code only ever writes chunks, the streaming downstream isn't wired to read inline Content from a cached entry, and a partial range- aware inline writer here would be overkill for a path that doesn't actually occur in practice. cacheRemoteObjectForCopy keeps the relaxed contract since the copy path's inline branch genuinely handles both chunked and content-only entries. Document the asymmetry on cachedEntryHasLocalData and on cacheRemoteObjectForStreaming so a future reader can see why the two helpers diverge. * extend version-id resolution to streaming cache path CodeRabbit flagged that GetObjectHandler still passed the raw query versionId to cacheRemoteObjectForStreaming. For latest-version reads in versioning-enabled buckets that stays empty even though the resolved entry lives at .versions/v_<id>, so remote-only GETs would keep caching the wrong path and 503-ing forever. Reuse the new resolvedSourceVersionId helper at the streaming call site too. Also document on cachedEntryHasLocalData that the zero-byte case flagged in the same review is handled upstream (IsInRemoteOnly requires RemoteSize > 0, so the cache helper is never invoked for empty remote objects -- CopyObject's pre-existing inline branch writes a correct empty destination directly). Pin this with a new test case. * trim verbose comments Drop tutorial-style and review-history comments. Keep only the WHY that isn't obvious from identifiers: the #9304 reference on the new branches in CopyObject / CopyObjectPart, the latest-version-fallback rationale on resolvedSourceVersionId, and the streaming/copy contract asymmetry on cachedEntryHasLocalData. * drop issue references from comments Issue numbers belong in PR descriptions and commit messages, not in source comments where they rot. Replace with the underlying invariant the code is preserving. * test: drive remote-object cache helpers through real gRPC Existing tests only re-enacted helper-function branching in test space, so they could not have caught a handler that consumed a remote-only entry without going through the cache. Stand up an in-process filer gRPC server (UnimplementedSeaweedFilerServer + configurable CacheRemoteObjectToLocalCluster response) and exercise the two cache helpers end-to-end. What's pinned: - cacheRemoteObjectForCopy returns nil when the cache makes no progress (response is still remote-only), lets gRPC errors through as nil, accepts both chunked and inline-content cache hits, and surfaces deadline-exceeded as nil so callers can 503 instead of holding the request open. - Versioned source paths route to .versions/v_<id>; non-versioned and "null" stay at the bucket-relative path. Captured by reading the request the stub server received. - cacheRemoteObjectForStreaming holds the stricter chunks-only contract: a content-only cache hit is not propagated, since streamFromVolumeServers' downstream isn't wired to read from inline Content there. Any current or future handler that calls these helpers exercises the same gRPC path under test, so the bug class is closed for helper-routed cache calls. * move remote-only copy test into the integration suite The previous gRPC-stub test in weed/s3api/ was integration-flavored but stubbed; relocate the coverage to the existing two-server suite under test/s3/remote_cache/, which already exercises the real remote.mount + remote.uncache flow against a primary SeaweedFS plus a secondary acting as remote storage. The new test/s3/remote_cache/remote_cache_copy_test.go drives: - TestRemoteCacheCopyObject: upload to primary, uncache (entry now remote-only), CopyObject to a new key, GET the destination. Pre- fix the GET returned 500 'data integrity error: size N reported but no content'; this pins the fixed behavior over real HTTP through the actual handler stack. - TestRemoteCacheCopyObjectPart: same shape via multipart UploadPartCopy on a 6 MiB object split into two parts, exercising CopyObjectPartHandler's range-copy path. Drop weed/s3api/s3api_remote_storage_grpc_test.go: the helper-level classification tests in s3api_remote_storage_test.go still cover the contract pieces (cachedEntryHasLocalData, resolvedSourceVersionId, the remote-only entry shape), and the integration suite covers the end-to-end behavior that those classifications enable. |
||
|
Chris Lu
|
1da091f798 |
ci: bring previously-uncovered integration tests into CI (#9281 follow-up) (#9283)
* ci: bring previously-uncovered integration tests into CI (#9281 follow-up) Six integration test packages had _test.go files but no GitHub workflow running them. The s3-sse-tests CI gap that let #8908's UploadPartCopy bug (and the four cross-SSE copy bugs in #9281) ship undetected was an instance of this same pattern. This change wires three of them into CI and removes a fourth that was deadcode: test/multi_master/ NEW workflow: multi-master-tests.yml - 3-node master raft cluster failover/recovery (5 tests, ~65s) test/testutil/ (run alongside multi_master) - port-allocator regression test test/s3/etag/ NEW workflow: s3-etag-acl-tests.yml - PutObject ETag format regression for #7768 (must be pure MD5 hex, not "<md5>-N" composite, for AWS Java SDK v2 compatibility) test/s3/acl/ (same workflow as etag) - object-ACL behavior on versioned buckets test/s3/catalog_trino/ DELETED (deadcode) - Single-file copy of test/s3tables/catalog_trino/trino_catalog_test.go from a 2024 commit that was never iterated, while the test/s3tables/ counterpart has been actively maintained (and IS in CI via s3-tables-tests.yml's trino-iceberg-catalog-tests job). Both workflows trigger only on changes to relevant code paths and use the existing simple "build weed → run go test" pattern (no per-test-dir Makefile boilerplate). The S3 workflow starts a single `weed mini` shared by etag and acl, which keeps the job under 2 minutes on a fresh runner. Two tests remain knowingly uncovered: test/s3/basic/ — order-dependent state across tests (TestListObjectV2 expects a bucket created by an earlier test, etc.) and uses the deprecated aws-sdk-go v1. Treated as sample programs, not a regression suite. Fixing them is out of scope for this PR. test/s3/catalog_trino/ — see "DELETED" above. Verified locally: - go test -v -timeout=8m ./test/multi_master/... ./test/testutil/... PASS (5 multi_master + 1 testutil tests, 64s) - weed mini + go test ./test/s3/etag/... + go test ./test/s3/acl/... PASS (8 etag + 5 acl tests, ~6s after server startup) * ci: fix log-collector glob for multi-master tests (review feedback on #9283) test/multi_master/cluster.go creates per-test temp dirs via os.MkdirTemp("", "seaweedfs_multi_master_it_"), so the glob has to match that prefix. The previous version looked for MasterCluster* / TestLeader* / TestTwoMasters* / TestAllMasters* which never matches — the failure-artifact upload would have been empty on a real failure. Switch the find to /tmp/seaweedfs_multi_master_it_* (maxdepth 1) so it actually picks up the per-node master*.log files under <baseDir>/logs/. Found by coderabbitai review on PR #9283. |
||
|
Chris Lu
|
82cf60a44f |
fix(s3api): re-encrypt UploadPartCopy bytes for the destination's SSE config (#8908) (#9280)
* fix(s3api): re-encrypt UploadPartCopy bytes for the destination's SSE config (#8908) The remaining failure mode in #8908 was that Docker Registry's blob finalization (server-side Move via UploadPartCopy) silently corrupts SSE-S3 multipart objects. Reproduces with `aws s3api upload-part-copy` under bucket-default SSE-S3: the GET on the completed object returns deterministic wrong bytes (correct length, same wrong SHA-256 across runs). The metadata is mathematically self-consistent — every chunk's stored IV equals `calculateIVWithOffset(baseIV_dst, partLocalOffset)` — but the bytes on disk were encrypted with the SOURCE upload's key+baseIV. Root cause: - `copyChunksForRange` (and `createDestinationChunk`) constructs new chunks for UploadPartCopy without copying `SseType` / `SseMetadata`, so destination chunks are written with `SseType=NONE`. - At completion, `completedMultipartChunk` (PR #9224's NONE→SSE_S3 backfill, intended to recover from a different missing-metadata bug) sees those NONE chunks under an SSE-S3 multipart upload and backfills SSE-S3 metadata derived from the destination upload's baseIV. The chunk metadata is now internally consistent and the GET path applies decryption — but the bytes on disk are encrypted with the source upload's key, not the destination's. Decryption produces deterministic garbage. Docker Registry pulls then fail with "Digest did not match". Fix: when either the source object or the destination multipart upload has any SSE configured, take a slow-path UploadPartCopy that (1) opens a plaintext reader of the source range — decrypting the source's per-chunk SSE-S3 metadata if needed via a reused `buildMultipartSSES3Reader`, and (2) feeds that plaintext through `putToFiler`'s existing encryption pipeline by staging the destination upload entry's SSE-S3/SSE-KMS headers on a cloned request. Encryption then matches PutObjectPart's contract: every part starts a fresh CTR stream from counter 0 with `baseIV_dst`, and each internal chunk's metadata records `calculateIVWithOffset(baseIV_dst, chunk.partLocalOffset)`. The `non-SSE → non-SSE` case still takes the existing fast raw-byte copy path — bytes on disk are plaintext on both sides, so chunk-level metadata is irrelevant. Cross-encryption from SSE-KMS / SSE-C sources is left as TODO — the new path returns an explicit error rather than the previous silent corruption. SSE-S3 (the user-reported case) round-trips correctly. Tests: - test/s3/sse/s3_sse_uploadpartcopy_integration_test.go pins three UploadPartCopy shapes against bucket-default SSE-S3: * Docker-Registry-shape 32MB+tail (the user's exact 5-chunk / 2-part metadata layout) * single full-object UploadPartCopy * many small range copies Each round-trips SHA-256. - test/s3/sse/s3_sse_concurrent_repro_test.go covers the parallel multipart-upload shape from the user report (5 blobs in parallel, full GET and chunked range GET both hash-checked) — pre-existing coverage; added here as a regression sentinel. * test(s3-sse): rename UploadPartCopy regression test so CI matches it The CI workflow .github/workflows/s3-sse-tests.yml dispatches on the TEST_PATTERN ".*Multipart.*Integration" — i.e. the test name must contain both "Multipart" and "Integration" for CI to run it. The previous name TestSSES3UploadPartCopyIntegration had only "Integration"; "UploadPart" isn't "Multipart". Rename to TestSSES3MultipartUploadPartCopyIntegration so the regression test actually runs in CI rather than only locally. * fix(s3api): map unsupported UploadPartCopy SSE source to 501, not 500 (review feedback on #9280) openSourcePlaintextReader explicitly rejects SSE-KMS and SSE-C sources (SSE-S3 is the only one wired up in this slow path so far). Earlier the caller blanket-mapped that to ErrInternalError, which collapses "this shape isn't implemented yet" into the same 500 response a real server failure would produce. Clients can no longer tell whether they hit a feature gap or a bug. Introduce a sentinel errCopySourceSSEUnsupported and have copyObjectPartViaReencryption errors.Is-check it; on match, return ErrNotImplemented (501) instead of ErrInternalError (500). Other failures still map to 500. Found by coderabbitai review on PR #9280. * fix(s3api): UploadPartCopy must fail with NoSuchUpload when upload entry is missing (review feedback on #9280) CopyObjectPartHandler's earlier checkUploadId call only verifies that the uploadID's hash prefix matches dstObject; it does not prove the upload directory exists in the filer. The previous logic silently swallowed filer_pb.ErrNotFound from getEntry(uploadDir) and fell through with uploadEntry=nil, which then skipped the destination SSE check and could route a plain-source copy through the raw-byte fast path even though the destination's encryption state is unknown. Treat ErrNotFound as ErrNoSuchUpload so the client sees the right status, matching the AWS S3 contract for UploadPartCopy on a non-existent upload. Found by coderabbitai review on PR #9280. * feat(s3api): set SSE response headers on UploadPartCopy slow path (review feedback on #9280) PutObjectPartHandler writes x-amz-server-side-encryption (and the KMS key-id header for SSE-KMS) on every successful part response so clients can confirm the destination's encryption state. The new UploadPartCopy slow path was missing this — it returned only the ETag in the response body and no SSE response headers. Plumb putToFiler's SSEResponseMetadata back through copyObjectPartViaReencryption to the handler, then call setSSEResponseHeaders before writing the XML response, matching the PutObjectPart contract. Found by gemini-code-assist review on PR #9280. * fix(s3api): map transient filer errors on UploadPartCopy upload-entry fetch to 503 (review feedback on #9280) Earlier non-ErrNotFound errors from getEntry(uploadDir, uploadID) all returned 500 InternalError, which most SDKs treat as fatal — even though a transient filer outage (gRPC Unavailable, leader election in flight, deadline exceeded) is exactly the kind of failure SDK retry logic is supposed to recover from. Add an isTransientFilerError helper that recognises: - context.DeadlineExceeded / context.Canceled - gRPC codes.Unavailable, DeadlineExceeded, ResourceExhausted, Aborted When the upload-entry fetch fails for one of those reasons, return 503 ServiceUnavailable so the client retries; everything else still maps to 500. Log line now also carries dstObject (in addition to dstBucket and uploadID) to make incident triage easier. Found by gemini-code-assist review on PR #9280. |
||
|
Chris Lu
|
02574314f6 |
test(s3): force-drop collection after deleteBucket in tagging/versioning/cors/copying (#9270)
* test(s3): force-drop collection after deleteBucket across tagging/versioning/cors/copying Each test creates a unique bucket (= new SeaweedFS collection) and the master's warm-create issues a 7-volume grow batch. The S3 DeleteBucket-driven collection sweep snapshots the layout once, but in-flight `volume_grow` requests keep registering volumes after the snapshot, leaking 1-3 volumes per bucket. On a single `weed mini` data node with the auto-derived volume cap, those leaks pile up fast and every subsequent PutObject 500s with "Not enough data nodes found". Mirror the retention-suite fix (commits |
||
|
Chris Lu
|
363d5caa85 |
test(s3-retention): purge stale buckets before each create to avoid volume exhaustion
The WORM suite creates one bucket per test, each backed by ~3 reserved volumes on the data node. With ~30 tests and the default `weed mini` volume cap, the data node runs out of slots midway through the run and every PutObject after that fails with InternalError. Hook a sweep of every test-prefix bucket into the create helpers so a panicked or interrupted prior test cannot leak buckets into the next. |
||
|
Chris Lu
|
ac3a756dae |
test(s3-retention): force-drop collection after deleteBucket to free volumes
COMPLIANCE-mode retention leaves objects that BypassGovernanceRetention cannot clear, so the test's DeleteBucket keeps returning BucketNotEmpty and the underlying SeaweedFS collection (with its 7 reserved volumes) leaks. After a few leaks on the single-node `weed mini` server, the master logs "Not enough data nodes found" and every subsequent PutObject 500s, timing the suite out. Call the master's /col/delete admin endpoint from deleteBucket so the collection's volumes are reclaimed even when S3-level cleanup is blocked. |
||
|
Chris Lu
|
4f628ff4e5 |
fix(s3api): stream multipart-SSE chunks lazily to avoid truncated GETs (#8908) (#9228)
* fix(s3api): stream multipart SSE-S3 chunks lazily to avoid truncated GETs (#8908) buildMultipartSSES3Reader opened a volume-server HTTP response for EVERY chunk upfront, then walked them with io.MultiReader. For a multipart SSE-S3 object with N internal chunks (e.g. a 200MB Docker Registry blob with 25+ chunks), N volume-server bodies sat live at once; chunks 1..N-1 were idle while io.MultiReader drained chunk 0. Under concurrent load the volume server's keep-alive logic closed those idle responses mid-flight, and the S3 client saw `unexpected EOF` partway through the GET. Truncated bytes hash to the wrong SHA-256, which is exactly the "Digest did not match" symptom Docker Registry reports in #8908 (and which persisted even after the per-chunk metadata fix in #9211 and the completion backfill in #9224). Introduce lazyMultipartChunkReader + preparedMultipartChunk{chunk, wrap}: a generic lazy chunk streamer with a per-chunk wrap closure for the SSE-specific decryption setup. Per-chunk metadata is still validated UPFRONT so a malformed chunk fails fast without opening any HTTP connection -- the eager validation contract callers and tests rely on is preserved. The volume-server GET and the SSE-specific decrypt wrap, however, fire LAZILY: at most one chunk body is live at any time, regardless of object size. This commit applies the new pattern to buildMultipartSSES3Reader only; the SSE-KMS and SSE-C multipart readers retain their eager form for now and will be migrated in follow-up commits, since the same shape exists there too. Tests: - TestBuildMultipartSSES3Reader_LazyChunkFetch pins the new contract: zero chunks opened at construction, peak liveness == 1, all closed after drain. - TestBuildMultipartSSES3Reader_RejectsBadChunkBeforeAnyFetch (replaces ClosesAppendedOnError) asserts a malformed chunk in position N causes zero fetches for chunks 0..N -- the previous test pinned a weaker contract (cleanup after eager open). - TestBuildMultipartSSES3Reader_InvalidIVLength updated for the same reason: the fetch callback must NOT be invoked at all on a bad-IV chunk. - TestMultipartSSES3RealisticEndToEnd round-trips multiple parts encrypted the way putToFiler writes them (shared DEK + baseIV, partOffset=0, post-completion global offsets) and walks them through buildMultipartSSES3Reader. * fix(s3api): stream multipart SSE-KMS chunks lazily Apply the same fix as the previous commit to createMultipartSSEKMSDecryptedReaderDirect: per-chunk SSE-KMS metadata is validated upfront, but volume-server GETs fire lazily through lazyMultipartChunkReader. At most one chunk body is live at any time. This is the same eager-open-all-chunks shape that produced #8908's truncated GETs for SSE-S3; SSE-KMS multipart objects with many chunks were exposed to the same idle-keepalive failure mode under concurrent load. The wire format on disk is unchanged (same per-chunk metadata, same encrypted bytes, same object Extended attributes). Existing SSE-KMS multipart objects read back identically -- only when the volume-server GETs fire changes. * fix(s3api): stream multipart SSE-C chunks lazily Apply the same fix as the previous two commits to createMultipartSSECDecryptedReaderDirect: per-chunk SSE-C metadata is validated upfront (IV decode, IV length check, non-negative PartOffset), but the volume-server GET and CreateSSECDecryptedReader- WithOffset wrap fire lazily through lazyMultipartChunkReader. At most one chunk body is live at any time. This is the same eager-open-all-chunks shape that produced #8908's truncated GETs for SSE-S3; SSE-C multipart objects with many chunks were exposed to the same idle-keepalive failure mode under concurrent load. The pre-existing TODO note about CopyObject SSE-C PartOffset handling is preserved verbatim. The wire format on disk is unchanged (same per-chunk metadata, same encrypted bytes); existing SSE-C multipart objects read back identically. After this commit all three multipart SSE read paths (SSE-S3, SSE-KMS, SSE-C) share lazyMultipartChunkReader as their streaming engine. * test(s3): add Docker Registry-shape multipart SSE-S3 GET regression Pin the end-to-end fix for #8908 with a test that mirrors what Docker Registry actually does on pull: a 25-part * 5MB upload with bucket- default SSE-S3, then a full GET, then SHA-256 over the streamed body must match SHA-256 over the uploaded bytes. The eager-multipart-reader bug was specifically a streaming truncation under load: the response status was 200 with a Content-Length matching the object size, but the body short-circuited mid-stream because later chunks' volume-server connections had already been closed by keepalive. The hash check is the symptom Docker Registry surfaces ("Digest did not match"), so this is the most faithful regression we can pin without spinning up a registry. uploadAndVerifyMultipartSSEObject already byte-compares the GET body, but hashing on top is intentionally explicit -- it documents WHY the test exists, and matches the failure mode reported in the issue. * test(s3): add range-read coverage matrix across SSE modes and sizes Existing range-read coverage in test/s3/sse was scoped to small (<= 1MB) single-chunk objects, with one ad-hoc range case per SSE mode and one 129-byte boundary-crossing case in TestSSEMultipartUploadIntegration. Nothing exercised: - Range reads on single-PUT objects whose content crosses the 8MB internal chunk boundary (medium size class). - Range reads on multipart objects whose parts each span multiple internal chunks (large size class) -- the shape #8908 originally surfaced for full-object GETs and the most likely site of any future regression in per-chunk IV / PartOffset plumbing for partial reads. - A consistent range-pattern set applied uniformly across SSE modes, so any divergence between modes (SSE-C uses random IV + PartOffset; SSE-S3/KMS use base IV + offset) is comparable at a glance. TestSSERangeReadCoverageMatrix introduces a parameterized matrix: modes: no_sse, sse_c, sse_kms, sse_s3 sizes: small (256KB single chunk), medium (12MB single PUT crossing one internal boundary), large (5x9MB multipart, ~10 internal chunks, every part itself spans an 8MB boundary) ranges: single byte at 0, prefix 512B, single byte at last, suffix bytes=-100, open-ended bytes=N-, whole object, AES-block boundary 15-31, mid straddling one internal boundary (medium+large), mid spanning many internal boundaries (large only) Per case it asserts: body bytes equal the expected slice, Content-Length matches the range length, Content-Range matches start-end/total, and the SSE response headers match the mode. The sse_kms branch probes once with a 1-byte SSE-KMS PUT and t.Skip's the remaining sse_kms subtests with a clear reason if the local server has no KMS provider configured -- the default `weed mini` setup lacks one; the Makefile target `test-with-kms` provides one via OpenBao. Other modes always run. Verified locally: 75 subtests pass under no_sse / sse_c / sse_s3 against weed mini, sse_kms cleanly skipped. * test(s3): conform new test names to TestSSE*Integration so CI runs them The two tests added in the previous commits had names that did NOT match the patterns the test/s3/sse Makefile and .github/workflows/s3-sse-tests.yml use to discover SSE integration tests: - test/s3/sse/Makefile `test` target: TestSSE.*Integration - test/s3/sse/Makefile `test-multipart`: TestSSEMultipartUploadIntegration - .github/workflows/s3-sse-tests.yml: ...|.*Multipart.*Integration|.*RangeRequestsServerBehavior Result: SSE-KMS coverage I added to TestSSERangeReadCoverageMatrix and the Docker-Registry-shape multipart regression in TestSSES3MultipartManyChunks_DockerRegistryShape were silently invisible to CI even though the underlying test setup (start-seaweedfs-ci using s3-config-template.json with the embedded `local` KMS provider) already has SSE-KMS configured. Renames: TestSSERangeReadCoverageMatrix -> TestSSERangeReadIntegration TestSSES3MultipartManyChunks_... -> TestSSEMultipartManyChunksIntegration Both names now match `TestSSE.*Integration` (Makefile `test` target) and TestSSEMultipartManyChunksIntegration additionally matches `.*Multipart.*Integration` (CI's comprehensive subset). No behavior change; only the function names move. Verified locally against `weed mini` with s3-config-template.json: TestSSERangeReadIntegration runs 96 leaf subtests across 4 SSE modes (none, SSE-C, SSE-KMS, SSE-S3) x 3 size classes x 7-9 range patterns, all passing, 0 skipped. The probe-and-skip in the SSE-KMS arm now only fires for ad-hoc local setups that don't load any KMS provider; the project's standard test setup loads the local provider, so CI has full SSE-KMS range coverage. * fix(s3api): validate SSE-KMS chunk IV during prep, before any fetch Addresses CodeRabbit review on PR #9228: in createMultipartSSEKMSDecryptedReaderDirect the per-chunk SSE-KMS metadata was deserialized in the prep loop but the IV length was only validated later, inside CreateSSEKMSDecryptedReader, which runs from the wrap closure -- AFTER the chunk's volume-server fetch has already started. That weakens the new "reject malformed chunks before any fetch" contract for SSE-KMS specifically: a chunk with a missing/short/long IV would fire its HTTP GET, then fail mid-stream during decrypt. The fix moves the existing ValidateIV check into the prep loop, matching the SSE-S3 and SSE-C paths. Drive-by: extract the SSE-KMS prep loop into a free buildMultipartSSEKMSReader helper that mirrors buildMultipartSSES3Reader, so the new contract is unit-testable without an S3ApiServer. The exported method (createMultipartSSEKMSDecryptedReaderDirect) stays a thin caller, so behavior for production callers is unchanged. New tests in weed/s3api/s3api_multipart_ssekms_test.go pin the contract: - TestBuildMultipartSSEKMSReader_RejectsBadIVBeforeAnyFetch covers missing IV, empty IV, short IV, long IV. Each case asserts both that an error is returned AND that the fetch callback is never invoked. - TestBuildMultipartSSEKMSReader_RejectsMissingMetadataBeforeAnyFetch pins the analogous behavior when SseMetadata is nil on a chunk in position N: chunks 0..N-1 must not be fetched (the earlier eager implementation depended on a closeAppendedReaders cleanup path; the new contract is stronger -- nothing is opened in the first place). - TestBuildMultipartSSEKMSReader_RejectsUnparseableMetadataBeforeAnyFetch covers the JSON-unmarshal failure branch. - TestBuildMultipartSSEKMSReader_SortsByOffset smoke-tests the documented sort-by-offset contract by recording the order in which fetch is invoked. All four pass under `go test ./weed/s3api/`. Existing weed/s3api unit suite + the SSE integration suite (with the local KMS provider enabled via s3-config-template.json) continue to pass. * test(s3): address CodeRabbit nitpicks on range coverage matrix Three small follow-ups on the range-read coverage matrix from the previous commit, per CodeRabbit nitpicks on PR #9228: 1. Promote the body-length check from `assert.Equal` to `require.Equal` so a truncation regression -- the canonical #8908 failure mode -- aborts the subtest immediately. Previously the assertion logged a length mismatch and then `assertDataEqual` ran on differently-sized slices, producing a noisy byte-diff on top of the actual symptom. The redundant trailing `t.Fatalf` block becomes dead and is removed. 2. Broaden the SSE-KMS probe-skip heuristic. The probe previously produced the friendly "KMS provider not configured" message only for 5xx responses; KMS-misconfig surfaces also include 501 NotImplemented, 4xx KMS.NotConfigured, and error messages containing "KMS.NotConfigured" / "NotImplemented" / "not configured". The behaviour change is purely cosmetic (the caller t.Skip's on any non-empty reason either way) but the new diagnostic is more useful in CI logs. 3. Add `t.Parallel()` at the mode and size-class levels of the matrix. Each (mode, size) writes an independent object key under the shared bucket, with no cross-talk, so parallel execution is safe. Local wall time on the full matrix dropped from ~2.0s to ~1.1s (~45%); the savings scale with chunk count and CI machine concurrency. Verified locally against `weed mini` with s3-config-template.json: - go test ./weed/s3api/ -count=1 PASS - TestSSERangeReadIntegration -v 112 PASS, 0 SKIP - TestSSEMultipartUploadIntegration etc. PASS * fix(s3api): tighten lazy reader error path; unify SSE IV validation Three CodeRabbit nitpicks on PR #9228: 1. lazyMultipartChunkReader: mark finished on non-EOF Read errors The Read loop's three earlier failure paths (chunk index past end, fetch error, wrap error) all set l.finished = true before returning. The non-EOF Read path -- where l.current.Read itself errors mid-chunk -- did not, leaving l.current/l.closer set and l.finished = false. A caller that retried Read after an error would re-enter the same broken stream instead of advancing or giving up. Set l.finished = true on non-EOF Read error so post-error state is consistent across all four failure sites; Close() (which the GetObjectHandler defers) still releases the chunk body. 2. Unify IV-length validation across SSE-S3, SSE-KMS, SSE-C prep paths The previous commit moved SSE-KMS to the shared ValidateIV helper but left SSE-S3 and SSE-C with bespoke inline `len(...) != AESBlockSize` checks. All three are enforcing the same invariant; inconsistency obscures the symmetry. Move SSE-S3 and SSE-C to ValidateIV too, with the same `<algo> chunk <fileId> IV` name convention. Error message wording shifts from "<algo> chunk X has invalid IV length N (expected 16)" to ValidateIV's "invalid <algo> chunk X IV length: expected 16 bytes, got N". The substring "IV length" is preserved across both, so the existing TestBuildMultipartSSES3Reader_InvalidIVLength substring assertion is loosened to match either form. 3. TestBuildMultipartSSEKMSReader_SortsByOffset: verify full ordering The test previously drove Read() to observe fetch-call order, but CreateSSEKMSDecryptedReader requires a live KMS provider to unwrap the encrypted DEK -- unavailable in unit tests -- so the wrap closure failed on the first chunk and only one fetch was ever recorded. The test asserted only fetchOrder[0] == "c0", which is weaker than the comment promised. Switch to a static check: type-assert the returned reader to *lazyMultipartChunkReader (same package so unexported fields are accessible) and inspect the prepared chunks slice directly. This pins the entire [c0, c1, c2] sort order in one place, doesn't depend on KMS, and runs in zero fetch calls. The fetch closure now asserts it is never invoked during preparation. All weed/s3api unit tests pass; integration suite (with KMS provider configured via s3-config-template.json) passes. * test(s3): switch range coverage cleanup to t.Cleanup; tighten KMS probe Two CodeRabbit comments on PR #9228, both about test/s3/sse/s3_sse_range_coverage_test.go: 1. CRITICAL: defer + t.Parallel() race in TestSSERangeReadIntegration The test creates one bucket up front, then runs subtests that call t.Parallel() at the mode and size levels (added in |
||
|
Chris Lu
|
525900dfe4 |
fix(s3api): backfill multipart SSE-S3 metadata at completion (#9224)
* fix(s3api): backfill missing per-chunk SSE-S3 metadata at completion When a part of an SSE-S3 multipart upload lands with SseType=NONE on its chunks (e.g. a transient failure to apply SSE-S3 setup in PutObjectPart), the completed object inherits NONE-tagged chunks and detectPrimarySSEType then misses the chunked SSE-S3 encryption. The read path falls through to the unencrypted serve and GET returns ciphertext, producing the SHA mismatch reported in #8908. Recover at completion using the base IV and key data the upload directory recorded at CreateMultipartUpload: - extractMultipartSSES3Info validates upload-entry metadata up front and hard-fails completion if the base IV or key data are malformed; serializing chunk metadata we then could not decrypt is worse than rejecting the upload. - completedMultipartChunk re-derives a per-chunk IV from baseIV + chunk.Offset (matching what putToFiler would have written) and serializes per-chunk SSE-S3 metadata when the chunk has no tag. Existing per-chunk metadata is left alone; we cannot recover an already-derived IV from the upload-entry alone. The IV formula intentionally has no partNumber term: putToFiler hardcodes partOffset=0 when it calls handleSSES3MultipartEncryption for every part, so each chunk's encryption IV is calculateIVWithOffset(baseIV, chunk.Offset_part_local). PartOffsetMultiplier is defined in s3_constants but is not consumed by the encryption path. Adopting (partNumber-1)*PartOffsetMultiplier + chunk.Offset would produce IVs that fail to decrypt the bytes on disk - a stronger failure mode than the bug being fixed. Tests pin this: - TestCompletedMultipartChunkBackfilledIVDecryptsActualCiphertext runs the round trip across the encryption boundary: encrypt parts with CreateSSES3EncryptedReaderWithBaseIV (the call putToFiler uses), drop chunk metadata to reproduce #8908, backfill, decrypt with backfilled IV, assert plaintext intact. - TestCompletedMultipartChunkRejectsPartNumberMultiplierFormula constructs the IV the partNumber formula would produce and shows it does not decrypt the actual ciphertext. This commit covers the chunk-level recovery only. The companion fix for the object-level Extended attributes (SeaweedFSSSES3Key / X-Amz-Server-Side-Encryption) follows separately. * fix(s3api): backfill canonical SSE-S3 attributes onto multipart object The previous commit ensures every chunk of an SSE-S3 multipart upload carries SseType=SSE_S3 with a per-chunk IV, so the multipart-direct read path can decrypt. The completed object's Extended map can still miss the canonical pair detectPrimarySSEType and IsSSES3EncryptedInternal look at: - X-Amz-Server-Side-Encryption (the AmzServerSideEncryption header detectPrimarySSEType reads on inline / small-object reads) - x-seaweedfs-sse-s3-key (SeaweedFSSSES3Key, required by IsSSES3EncryptedInternal and by the read-path key lookup) When a part of the upload was written by a path that did not set those (the same #8908 race that produced the NONE chunks), copySSEHeadersFromFirstPart finds nothing to copy and the final entry ends up with only the multipart-init keys (SeaweedFSSSES3Encryption / BaseIV / KeyData). The read path then mis-detects the object as unencrypted. applyMultipartSSES3HeadersFromUploadEntry writes the canonical pair from the multipart-init metadata in all three completion paths (versioned, suspended, non-versioned), only when the keys are missing so a healthy first part still wins. extractMultipartSSES3Info already ran in prepareMultipartCompletionState, so the data is reused without re-decoding. Tests: TestApplyMultipartSSES3HeadersFromUploadEntry covers backfill, do-not-clobber, and nil-info no-op cases. * fix(s3api): drop double IV adjustment in SSE-KMS chunk view decrypt decryptSSEKMSChunkView was pre-adjusting the SSE-KMS chunk IV (calculateIVWithOffset(baseIV, ChunkOffset)) and then handing the adjusted IV to CreateSSEKMSDecryptedReader, which itself runs calculateIVWithOffset(IV, ChunkOffset) on whatever it receives. The offset was being applied twice for any chunk with a non-zero ChunkOffset, corrupting the keystream for range reads that cross multipart chunk boundaries. Pass the raw SSE-KMS key (with base IV and the original ChunkOffset field) into CreateSSEKMSDecryptedReader so the offset is applied exactly once, and remove the now-dead intra-block skip that was compensating for the double adjustment. Add an anti-test inside TestSSEKMSDecryptChunkView_RequiresOffsetAdjustment that decrypts the same ciphertext with a deliberately double-adjusted IV and asserts the output is corrupted, so any regression that re-introduces the double application fails the unit test. * test(s3): cover multipart SSE across chunk-spanning parts and ranges Adds an integration subtest "Multipart Parts Larger Than Internal Chunks Across SSE Types" to TestSSEMultipartUploadIntegration that exercises the end-to-end S3 path for the bugs fixed in this branch: - Two-part multipart upload with each part larger than the 8MB internal SeaweedFS chunk, so each part itself spans multiple underlying chunks. - Subtests for SSE-C, SSE-KMS, explicit SSE-S3, and bucket-default SSE-S3 - the four paths multipart parts can take through the SSE pipeline. - Each subtest does a full GET (verifying every byte and the response Content-Length / SSE response headers) plus a 129-byte range read straddling the 8MB internal chunk boundary, which is the path that produced the SSE-KMS double-IV corruption (fix in the previous commit) and the SSE-S3 chunk-tag loss (fix in the earlier commits). Factored the request shape behind multipartSSEOptions / uploadAndVerifyMultipartSSEObject so all four SSE flavors share the same upload+verify code; only the SSE-specific input/output configuration differs per subtest. * test(s3): abort orphan multipart uploads on test failure Address coderabbit nitpick on uploadAndVerifyMultipartSSEObject. The helper used require.NoError after CreateMultipartUpload, UploadPart and CompleteMultipartUpload, so a failure in any of those (or in the later GET / range read on a still-incomplete upload) called t.Fatal without aborting the in-flight MPU, leaving an orphan upload in the bucket. Harmless in CI where the data dir is wiped on shutdown, but a real annoyance when iterating locally and a textbook AWS S3 caveat in production. Register a t.Cleanup that calls AbortMultipartUpload unless a "completed" flag was set right after a successful CompleteMultipartUpload. Use context.Background for the abort call since the parent ctx may already be cancelled at cleanup time, and t.Logf the abort error rather than failing the test so the original failure remains visible in the run output. |
||
|
Chris Lu
|
e77f8ae204 |
fix(s3api): route STS GetFederationToken to STS handler (#9157) (#9167)
* fix(s3api): route STS GetFederationToken requests to STS handler (#9157) The STS GetFederationToken handler was implemented but never reachable. Three routing gaps sent requests to the S3/IAM path instead of STS: - No explicit mux route for Action=GetFederationToken in the URL query - iamMatcher did not exclude GetFederationToken, so authenticated POSTs with Action in the form body were matched and dispatched to IAM - UnifiedPostHandler only dispatched AssumeRole* and GetCallerIdentity to STS, leaving GetFederationToken to fall through to DoActions and return NotImplemented Add the missing route, the matcher exclusion, and the dispatch branch. Also wire TestSTS, TestAssumeRoleWithWebIdentity, and TestServiceAccount into the s3-iam-tests workflow as a new "sts" matrix entry. Before this change, none of test/s3/iam/s3_sts_get_federation_token_test.go's four test functions ran in CI, which is why this regression shipped. * test(iam): make orphaned STS/service-account tests pass under auth-enabled CI Follow-up to wiring STS tests into CI: fixes several pre-existing issues that made the newly-included tests fail locally. Server fixes: - weed/s3api/s3api_sts.go: handleGetFederationToken no longer 500s when the caller is a legacy S3-config identity (not in the IAM user store). Previously any GetPoliciesForUser error short-circuited to InternalError, which hard-failed every SigV4 caller using keys from -s3.config. - weed/s3api/s3api_embedded_iam.go: CreateServiceAccount now generates IDs in the sa:<parent>:<uuid> format required by credential.ValidateServiceAccountId. The old "sa-XXXXXXXX" format failed the persistence-layer regex and caused every CreateServiceAccount call to return 500 once a filer-backed credential store validated the ID. Test helpers: - test/s3/iam/s3_sts_assume_role_test.go: callSTSAPIWithSigV4 no longer sets req.Header["Host"]. aws-sdk-go v1 v4.Signer already signs Host from req.URL.Host, and a manual Host header made the signer emit host;host in SignedHeaders, producing SignatureDoesNotMatch. Updated missing_role_arn subtest to match the existing SeaweedFS behavior (user-context assumption). - test/s3/iam/s3_service_account_test.go: callIAMAPI now SigV4-signs requests when STS_TEST_{ACCESS,SECRET}_KEY env vars are set. Unsigned IAM writes otherwise fall through to the STS fallback and return InvalidAction. CI matrix: - .github/workflows/s3-iam-tests.yml: skip TestServiceAccountLifecycle/use_service_account_credentials only. The rest of the service-account suite passes; that one subtest depends on a separate credential-reload issue where new ABIA keys briefly register into accessKeyIdent but aren't persisted to the filer, so they vanish on the next reload. Out of scope for the #9157 GetFederationToken fix. * fix(credential): accept AWS IAM username chars in service-account IDs Gemini review on #9167 pointed out that ServiceAccountIdPattern's parent-user segment was more restrictive than an AWS IAM username: `[A-Za-z0-9_-]` vs. IAM's `[\w+=,.@-]`. Realistic usernames with `@`, `.`, `+`, `=`, or `,` (e.g. email-style principals) would fail validation at the filer store even though the embedded IAM API happily created them. Broaden the regex to `[A-Za-z0-9_+=,.@-]` (matching the AWS IAM spec at https://docs.aws.amazon.com/IAM/latest/APIReference/API_User.html) and add a table-driven test that locks the expansion in. * address PR review feedback on #9167 All five review items were valid; changes keyed to review bullets: - weed/s3api/s3api_sts.go: handleGetFederationToken no longer swallows arbitrary policy-lookup failures. Only credential.ErrUserNotFound is treated leniently (the legacy-config SigV4 path); any other error now returns InternalError so we don't mint tokens with an incomplete policy set. - weed/credential/grpc/grpc_identity.go: GetUser translates gRPC NotFound back to credential.ErrUserNotFound so errors.Is(...) above matches for gRPC-backed stores, not just memory/filer-direct. - weed/s3api/s3api_embedded_iam.go: CreateServiceAccount now validates the generated saId against credential.ValidateServiceAccountId before returning. Surfaces a client 400 with the offending ID instead of the opaque 500 that used to bubble up from the persistence layer. - weed/s3api/s3api_server_routing_test.go: seed a routing-test identity with a known AK/SK, sign TestRouting_GetFederationTokenAuthenticatedBody with aws-sdk-go v4.Signer so the request actually passes AuthSignatureOnly. Assert 503 ServiceUnavailable (from STSHandlers with no stsService) instead of just NotEqual(501) — 503 proves the dispatch reached STSHandlers.HandleSTSRequest. - test/s3/iam/s3_service_account_test.go: callIAMAPI signs with service="iam" instead of "s3" (SeaweedFS verifies against whichever service the client signed with, but "iam" is semantically correct). - weed/credential/validation_test.go: add positive rows for an uppercase parent (sa:ALICE:...) and a canonical hyphenated UUID suffix (sa:alice:123e4567-e89b-12d3-a456-426614174000). |
||
|
Chris Lu
|
d0a09ea178 |
fix(s3): honor ChecksumAlgorithm on presigned URL uploads (#9076)
* fix(s3): honor ChecksumAlgorithm on presigned URL uploads AWS SDK presigners hoist x-amz-sdk-checksum-algorithm (and related checksum headers) into the signed URL's query string, so servers must read either location. detectRequestedChecksumAlgorithm only looked at request headers, so presigned PUTs with ChecksumAlgorithm set validated and stored no additional checksum, and HEAD/GET never returned the x-amz-checksum-* header. Read these parameters from headers first, then fall back to a case-insensitive query-string lookup. Apply the same fallback when comparing an object-level checksum value against the computed one. Fixes #9075 * test(s3): presigned URL checksum integration tests (#9075) Adds test/s3/checksum with end-to-end coverage for flexible-checksum behavior on presigned URL uploads. Tests generate a presigned PUT URL with ChecksumAlgorithm set, upload the body with a plain http.Client (bypassing AWS SDK middleware so the server must honor the query-string hoisted x-amz-sdk-checksum-algorithm), then HEAD/GET with ChecksumMode=ENABLED and assert the stored x-amz-checksum-* header. Covers SHA256, SHA1, and a negative control with no checksum requested. Wires the new directory into s3-go-tests.yml as its own CI job. * perf(s3): parse presigned query once in detectRequestedChecksumAlgorithm Previously, each header fallback called getHeaderOrQuery, which re-parsed r.URL.Query() and allocated a new map on every invocation — up to eight times per PutObject request. Parse the raw query at most once per request (only when non-empty) and pass the pre-parsed url.Values into a new lookupHeaderOrQuery helper. Also drops a redundant strings.ToLower allocation in the case-insensitive query key scan (strings.EqualFold already handles ASCII case folding). Addresses review feedback from gemini-code-assist on PR #9076. * test(s3): honor credential env vars and add presigned upload timeout - init() now reads S3_ACCESS_KEY/S3_SECRET_KEY (and AWS_ACCESS_KEY_ID / AWS_SECRET_ACCESS_KEY / AWS_REGION fallbacks) so that `make test-with-server ACCESS_KEY=... SECRET_KEY=...` no longer authenticates with hardcoded defaults while the server has been started with different credentials. - uploadViaPresignedURL uses a dedicated http.Client with a 30s timeout instead of http.DefaultClient, so a stalled server fails fast in CI instead of blocking until the suite's global timeout fires. Addresses review feedback from coderabbitai on PR #9076. * test(s3): pass S3_PORT and credentials through to checksum tests - 'make test' now exports S3_ENDPOINT, S3_ACCESS_KEY, and S3_SECRET_KEY derived from the Makefile variables so the Go test process talks to the same endpoint/credentials that start-server was launched with. - start-server cleans up the background SeaweedFS process and PID file when the readiness poll times out, preventing stale port conflicts on subsequent runs. Addresses review feedback from coderabbitai on PR #9076. * ci(s3): raise checksum tests step timeout make test-with-server builds weed_binary, waits up to 90s for readiness, then runs go test -timeout=10m. The previous 12-minute step timeout only had ~2 minutes of headroom over the Go timeout, risking the Actions runner killing the step before tests reported a real failure. Bumps the job timeout from 15 to 20 minutes and the step timeout from 12 to 16 minutes, matching other S3 integration jobs. Addresses review feedback from coderabbitai on PR #9076. * perf(s3): thread pre-parsed query through putToFiler hot path Parse the request's query string once at the top of putToFiler and reuse the resulting url.Values for both the checksum-algorithm detection and the expected-checksum verification. Previously, the verification path called getHeaderOrQuery which re-parsed r.URL.Query() again on every PutObject, defeating the previous commit's single-parse goal. - Add parseRequestQuery + detectRequestedChecksumAlgorithmQ (the pre-parsed-query variant). detectRequestedChecksumAlgorithm is now a thin wrapper used by callers that do a single lookup. - putToFiler parses once and threads the result through both call sites. - Remove getHeaderOrQuery and update the unit test to use lookupHeaderOrQuery directly. Addresses follow-up review from gemini-code-assist on PR #9076. * test(s3): check io.ReadAll error in uploadViaPresignedURL helper * test(s3): drop SHA1 presigned test case The AWS SDK v2 presigner signs a Content-MD5 header at presign time for SHA1 PutObject requests even when no body is attached (the MD5 of the empty payload gets baked into the signed headers). Uploading the real body via a plain http.Client then trips SeaweedFS's MD5 validation and returns BadDigest — an SDK/presigner quirk, not a SeaweedFS bug. The SHA256 positive case already exercises the server-side query-hoisted algorithm path that issue #9075 is about, and the unit tests in weed/s3api cover each algorithm's header mapping. Drop the SHA1 integration case rather than chase SDK-specific workarounds. * test(s3): provide real Content-MD5 to presigned checksum test AWS SDK v2's flexible-checksum middleware signs a Content-MD5 header at presign time. There is no body to hash at that point, so it seeds the header with MD5 of the empty payload. When the real body is then PUT with a plain http.Client, SeaweedFS's server-side Content-MD5 verification correctly rejects the upload with BadDigest. Pre-compute the MD5 of the test body and thread it into PutObjectInput.ContentMD5 so the signed Content-MD5 matches the body that will actually be uploaded. The test still exercises the server-side path that reads X-Amz-Sdk-Checksum-Algorithm from the query string (the fix that PR #9076 is validating). * test(s3): send the signed Content-MD5 header on presigned upload uploadViaPresignedURL now accepts an extraHeaders map so callers can thread through headers that the presigner signed but the raw http request would otherwise omit. The SHA256 test passes the Content-MD5 it computed, matching what the presigner baked into the signature. Fixes SignatureDoesNotMatch seen in CI after the previous commit set ContentMD5 on the presign input without sending the corresponding header on the actual upload. * test(s3): build presigned URL with the raw v4 signer The AWS SDK v2 s3.PresignClient runs the flexible-checksum middleware for any PutObject input that carries ChecksumAlgorithm. That middleware injects a Content-MD5 header at presign time, and with no body present it seeds MD5-of-empty. Any subsequent upload of a non-empty body through a plain http.Client then trips SeaweedFS's Content-MD5 verification and returns BadDigest — not the code path that issue #9075 is about. Replace the PresignClient usage in the integration test with a direct call to v4.Signer.PresignHTTP, building a canonical URL whose query string already contains x-amz-sdk-checksum-algorithm=SHA256. This is exactly the shape of URL a browser/curl client would receive from any presigner that hoists the algorithm header, and it exercises the server-side fix from PR #9076 without dragging in SDK-specific middleware quirks. * test(s3): set X-Amz-Expires on presigned URL before signing v4.Signer.PresignHTTP does not add X-Amz-Expires on its own — the caller has to seed it into the request's query string so the signer includes it in the canonical query and the server accepts the presigned URL. Without it, SeaweedFS correctly returns AuthorizationQueryParametersError. Also adds a .gitignore for the make-managed test volume data, log file, and PID file so local `make test-with-server` runs do not leave artifacts tracked by git. Verified by running the integration tests locally: make test-with-server → both presigned checksum tests PASS. |
||
|
Chris Lu
|
10e7f0f2bc |
fix(shell): s3.user.provision handles existing users by attaching policy (#9040)
* fix(shell): s3.user.provision handles existing users by attaching policy Instead of erroring when the user already exists, the command now creates the policy and attaches it to the existing user via UpdateUser. Credentials are only generated and displayed for newly created users. * fix(shell): skip duplicate policy attachment in s3.user.provision Check if the policy is already attached before appending and calling UpdateUser, making repeated runs idempotent. * fix(shell): generate service account ID in s3.serviceaccount.create The command built a ServiceAccount proto without setting Id, which was rejected by credential.ValidateServiceAccountId on any real store. Now generates sa:<parent>:<uuid> matching the format used by the admin UI. * test(s3): integration tests for s3.* shell commands Adds TestShell* integration tests covering ~40 previously untested shell commands: user, accesskey, group, serviceaccount, anonymous, bucket, policy.attach/detach, config.show, and iam.export/import. Switches the test cluster's credential store from memory to filer_etc because the memory store silently drops groups and service accounts in LoadConfiguration/SaveConfiguration. * fix(shell): rollback policy on key generation failure in s3.user.provision If iam.GenerateRandomString or iam.GenerateSecretAccessKey fails after the policy was persisted, the policy would be left orphaned. Extracts the rollback logic into a local closure and invokes it on all failure paths after policy creation for consistency. * address PR review feedback for s3 shell tests and serviceaccount - s3.serviceaccount.create: use 16 bytes of randomness (hex-encoded) for the service account UUID instead of 4 bytes to eliminate collision risk - s3.serviceaccount.create: print the actual ID and drop the outdated "server-assigned" note (the ID is now client-generated) - tests: guard createdAK in accesskey rotate/delete subtests so sibling failures don't run invalid CLI calls - tests: requireContains/requireNotContains use t.Fatalf to fail fast - tests: Provision subtest asserts the "Attached policy" message on the second provision call for an existing user - tests: update extractServiceAccountID comment example to match the sa:<parent>:<uuid> format - tests: drop redundant saID empty-check (extractServiceAccountID fatals) * test(s3): use t.Fatalf for precondition check in serviceaccount test |
||
|
Chris Lu
|
e648c76bcf | go fmt | ||
|
Chris Lu
|
e21d7602c3 |
feat(iam): implement group inline policy actions (#8992)
* feat(iam): implement group inline policy actions Add PutGroupPolicy, GetGroupPolicy, DeleteGroupPolicy, and ListGroupPolicies to both embedded and standalone IAM servers. The standalone IAM stores group inline policies in a new GroupInlinePolicies field in the Policies JSON, mirroring the existing user inline policy pattern. DeleteGroup now also checks for inline policies before allowing deletion. * fix: address review feedback for group inline policies - Embedded IAM: return NotImplemented for group inline policies instead of silently succeeding as no-ops (Gemini + CodeRabbit) - Standalone IAM: recompute member actions after PutGroupPolicy and DeleteGroupPolicy (Gemini) - Add parameter validation for GroupName/PolicyName/PolicyDocument on PutGroupPolicy, DeleteGroupPolicy, ListGroupPolicies (Gemini) - Add UserName validation for ListUserPolicies in standalone IAM - Call cleanupGroupInlinePolicies from DeleteGroup (Gemini) - Migrate GroupInlinePolicies on group rename in UpdateGroup (CodeRabbit) - Fix integration test cleanup order (CodeRabbit) * fix: persist recomputed actions and improve error handling - Set changed=true for PutGroupPolicy/DeleteGroupPolicy in standalone IAM DoActions so recomputed member actions are persisted (Gemini critical) - Make cleanupGroupInlinePolicies accept policies parameter to avoid redundant I/O, return error (Gemini) - Make migrateGroupInlinePolicies return error, handle in caller (Gemini) * fix: include group policies in action recomputation Extend computeAllActionsForUser to also aggregate group inline policies and group managed policies when s3cfg is provided. Previously, group inline policies were stored but never reflected in member Identity.Actions. (CodeRabbit critical) * perf: use identity index in recomputeActionsForGroupMembers for O(N+M) * fix: skip group inline policy integration test on embedded IAM The embedded IAM returns NotImplemented for group inline policies. Skip TestIAMGroupInlinePolicy when running against embedded mode to avoid CI failures in the group integration test matrix. |
||
|
Chris Lu
|
45ee2ab4b9 |
feat(iam): implement ListUserPolicies API action (#8991)
* feat(iam): implement ListUserPolicies API action (#8987) Add ListUserPolicies support to both embedded and standalone IAM servers, resolving the NotImplemented error when calling `aws iam list-user-policies`. * fix: address review feedback for ListUserPolicies - Add handleImplicitUsername for ListUserPolicies in both IAM servers so omitting UserName defaults to the calling user (Gemini review) - Assert synthetic policy name in unit test (CodeRabbit) - Use require.True for error type assertion in integration test (CodeRabbit) |
||
|
Chris Lu
|
fbe758efa8 |
test: consolidate port allocation into shared test/testutil package (#8982)
* test: consolidate port allocation into shared test/testutil package Move duplicated port allocation logic from 15+ test files into a single shared package at test/testutil/. This fixes a port collision bug where independently allocated ports could overlap via the gRPC offset (port+10000), causing weed mini to reject the configuration. The shared package provides: - AllocatePorts: atomic allocation of N unique ports - AllocateMiniPorts/MustFreeMiniPorts: gRPC-offset-aware allocation that prevents port A+10000 == port B collisions - WaitForPort, WaitForService, FindBindIP, WriteIAMConfig, HasDocker * test: address review feedback and fix FUSE build - Revert fuse_integration change: it has its own go.mod and cannot import the shared testutil package - AllocateMiniPorts: hold all listeners open until the entire batch is allocated, preventing race conditions where other processes steal ports - HasDocker: add 5s context timeout to avoid hanging on stalled Docker - WaitForService: only treat 2xx HTTP status codes as ready * test: use global rand in AllocateMiniPorts for better seeding Go 1.20+ auto-seeds the global rand generator. Using it avoids identical sequences when multiple tests call at the same nanosecond. * test: revert WaitForService status code check S3 endpoints return non-2xx (e.g. 403) on bare GET requests, so requiring 2xx caused the S3 integration test to time out. Any HTTP response is sufficient proof that the service is running. * test: fix gofmt formatting in s3tables test files |
||
|
Chris Lu
|
a4753b6a3b |
S3: delay empty folder cleanup to prevent Spark write failures (#8970)
* S3: delay empty folder cleanup to prevent Spark write failures (#8963) Empty folders were being cleaned up within seconds, causing Apache Spark (s3a) writes to fail when temporary directories like _temporary/0/task_xxx/ were briefly empty. - Increase default cleanup delay from 5s to 2 minutes - Only process queue items that have individually aged past the delay (previously the entire queue was drained once any item triggered) - Make the delay configurable via filer.toml: [filer.options] s3.empty_folder_cleanup_delay = "2m" * test: increase cleanup wait timeout to match 2m delay The empty folder cleanup delay was increased to 2 minutes, so the Spark integration test needs to wait longer for temporary directories to disappear. * fix: eagerly clean parent directories after empty folder deletion After deleting an empty folder, immediately try to clean its parent rather than relying on cascading metadata events that each re-enter the 2-minute delay queue. This prevents multi-minute waits when cleaning nested temporary directory trees (e.g. Spark's _temporary hierarchy with 3+ levels would take 6m+ vs near-instant). Fixes the CI failure where lingering _temporary parent directories were not cleaned within the test's 3-minute timeout. |
||
|
Chris Lu
|
8fad85aed7 |
feat(s3): support WEED_S3_SSE_KEY env var for SSE-S3 KEK (#8904)
* feat(s3): support WEED_S3_SSE_KEY env var for SSE-S3 KEK Add support for providing the SSE-S3 Key Encryption Key (KEK) via the WEED_S3_SSE_KEY environment variable (hex-encoded 256-bit key). This avoids storing the master key in plaintext on the filer at /etc/s3/sse_kek. Key source priority: 1. WEED_S3_SSE_KEY environment variable (recommended) 2. Existing filer KEK at /etc/s3/sse_kek (backward compatible) 3. Auto-generate and save to filer (deprecated for new deployments) Existing deployments with a filer-stored KEK continue to work unchanged. A deprecation warning is logged when auto-generating a new filer KEK. * refactor(s3): derive KEK from any string via HKDF instead of requiring hex Accept any secret string in WEED_S3_SSE_KEY and derive a 256-bit key using HKDF-SHA256 instead of requiring a hex-encoded key. This is simpler for users — no need to generate hex, just set a passphrase. * feat(s3): add WEED_S3_SSE_KEK and WEED_S3_SSE_KEY env vars for KEK Two env vars for providing the SSE-S3 Key Encryption Key: - WEED_S3_SSE_KEK: hex-encoded, same format as /etc/s3/sse_kek. If the filer file also exists, they must match. - WEED_S3_SSE_KEY: any string, 256-bit key derived via HKDF-SHA256. Refuses to start if /etc/s3/sse_kek exists (must delete first). Only one may be set. Existing filer-stored KEKs continue to work. Auto-generating and storing new KEKs on filer is deprecated. * fix(s3): stop auto-generating KEK, fail only when SSE-S3 is used Instead of auto-generating a KEK and storing it on the filer when no key source is configured, simply leave SSE-S3 disabled. Encrypt and decrypt operations return a clear error directing the user to set WEED_S3_SSE_KEK or WEED_S3_SSE_KEY. * refactor(s3): move SSE-S3 KEK config to security.toml Move KEK configuration from standalone env vars to security.toml's new [sse_s3] section, following the same pattern as JWT keys and TLS certs. [sse_s3] kek = "" # hex-encoded 256-bit key (same format as /etc/s3/sse_kek) key = "" # any string, HKDF-derived Viper's WEED_ prefix auto-mapping provides env var support: WEED_SSE_S3_KEK and WEED_SSE_S3_KEY. All existing behavior is preserved: filer KEK fallback, mismatch detection, and HKDF derivation. * refactor(s3): rename SSE-S3 config keys to s3.sse.kek / s3.sse.key Use [s3.sse] section in security.toml, matching the existing naming convention (e.g. [s3.*]). Env vars: WEED_S3_SSE_KEK, WEED_S3_SSE_KEY. * fix(s3): address code review findings for SSE-S3 KEK - Don't hold mutex during filer retry loop (up to 20s of sleep). Lock only to write filerClient and superKey. - Remove dead generateAndSaveSuperKeyToFiler and unused constants. - Return error from deriveKeyFromSecret instead of ignoring it. - Fix outdated doc comment on InitializeWithFiler. - Use t.Setenv in tests instead of manual os.Setenv/Unsetenv. * fix(s3): don't block startup on filer errors when KEK is configured - When s3.sse.kek is set, a temporarily unreachable filer no longer prevents startup. The filer consistency check becomes best-effort with a warning. - Same treatment for s3.sse.key: filer unreachable logs a warning instead of failing. - Rewrite error messages to suggest migration instead of file deletion, avoiding the risk of orphaning encrypted data. Finding 3 (restore auto-generation) intentionally skipped — auto-gen was removed by design to avoid storing plaintext KEK on filer. * fix(test): set WEED_S3_SSE_KEY in SSE integration test server startup SSE-S3 no longer auto-generates a KEK, so integration tests must provide one. Set WEED_S3_SSE_KEY=test-sse-s3-key in all weed mini invocations in the test Makefile. |
||
|
Chris Lu
|
059bee683f |
feat(s3): add STS GetFederationToken support (#8891)
* feat(s3): add STS GetFederationToken support Implement the AWS STS GetFederationToken API, which allows long-term IAM users to obtain temporary credentials scoped down by an optional inline session policy. This is useful for server-side applications that mint per-user temporary credentials. Key behaviors: - Requires SigV4 authentication from a long-term IAM user - Rejects calls from temporary credentials (session tokens) - Name parameter (2-64 chars) identifies the federated user - DurationSeconds supports 900-129600 (15 min to 36 hours, default 12h) - Optional inline session policy for permission scoping - Caller's attached policies are embedded in the JWT token - Returns federated user ARN: arn:aws:sts::<account>:federated-user/<Name> No performance impact on the S3 hot path — credential vending is a separate control-plane operation, and all policy data is embedded in the stateless JWT token. * fix(s3): address GetFederationToken PR review feedback - Fix Name validation: max 32 chars (not 64) per AWS spec, add regex validation for [\w+=,.@-]+ character whitelist - Refactor parseDurationSeconds into parseDurationSecondsWithBounds to eliminate duplicated duration parsing logic - Add sts:GetFederationToken permission check via VerifyActionPermission mirroring the AssumeRole authorization pattern - Change GetPoliciesForUser to return ([]string, error) so callers fail closed on policy-resolution failures instead of silently returning nil - Move temporary-credentials rejection before SigV4 verification for early rejection and proper test coverage - Update tests: verify specific error message for temp cred rejection, add regex validation test cases (spaces, slashes rejected) * refactor(s3): use sts.Action* constants instead of hard-coded strings Replace hard-coded "sts:AssumeRole" and "sts:GetFederationToken" strings in VerifyActionPermission calls with sts.ActionAssumeRole and sts.ActionGetFederationToken package constants. * fix(s3): pass through sts: prefix in action resolver and merge policies Two fixes: 1. mapBaseActionToS3Format now passes through "sts:" prefix alongside "s3:" and "iam:", preventing sts:GetFederationToken from being rewritten to s3:sts:GetFederationToken in VerifyActionPermission. This also fixes the existing sts:AssumeRole permission checks. 2. GetFederationToken policy embedding now merges identity.PolicyNames (from SigV4 identity) with policies from the IAM manager (which may include group-attached policies), deduplicated via a map. Previously the IAM manager lookup was skipped when identity.PolicyNames was non-empty, causing group policies to be omitted from the token. * test(s3): add integration tests for sts: action passthrough and policy merge Action resolver tests: - TestMapBaseActionToS3Format_ServicePrefixPassthrough: verifies s3:, iam:, and sts: prefixed actions pass through unchanged while coarse actions (Read, Write) are mapped to S3 format - TestResolveS3Action_STSActionsPassthrough: verifies sts:AssumeRole, sts:GetFederationToken, sts:GetCallerIdentity pass through ResolveS3Action unchanged with both nil and real HTTP requests Policy merge tests: - TestGetFederationToken_GetPoliciesForUser: tests IAMManager.GetPoliciesForUser with no user store (error), missing user, user with policies, user without - TestGetFederationToken_PolicyMergeAndDedup: tests that identity.PolicyNames and IAM-manager-resolved policies are merged and deduplicated (SharedPolicy appears in both sources, result has 3 unique policies) - TestGetFederationToken_PolicyMergeNoManager: tests that when IAM manager is unavailable, identity.PolicyNames alone are embedded * test(s3): add end-to-end integration tests for GetFederationToken Add integration tests that call GetFederationToken using real AWS SigV4 signed HTTP requests against a running SeaweedFS instance, following the existing pattern in test/s3/iam/s3_sts_assume_role_test.go. Tests: - TestSTSGetFederationTokenValidation: missing name, name too short/long, invalid characters, duration too short/long, malformed policy, anonymous rejection (7 subtests) - TestSTSGetFederationTokenRejectTemporaryCredentials: obtains temp creds via AssumeRole then verifies GetFederationToken rejects them - TestSTSGetFederationTokenSuccess: basic success, custom 1h duration, 36h max duration with expiration time verification - TestSTSGetFederationTokenWithSessionPolicy: creates a bucket, obtains federated creds with GetObject-only session policy, verifies GetObject succeeds and PutObject is denied using the AWS SDK S3 client |
||
|
Chris Lu
|
a4b896a224 |
fix(s3): skip directories before marker in ListObjectVersions pagination (#8890)
* fix(s3): skip directories before marker in ListObjectVersions pagination ListObjectVersions was re-traversing the entire directory tree from the beginning on every paginated request, only skipping entries at the leaf level. For buckets with millions of objects in deep hierarchies, this caused exponentially slower responses as pagination progressed. Two optimizations: 1. Use keyMarker to compute a startFrom position at each directory level, skipping directly to the relevant entry instead of scanning from the beginning (mirroring how ListObjects uses marker descent). 2. Skip recursing into subdirectories whose keys are entirely before the keyMarker. Changes per-page cost from O(entries_before_marker) to O(tree_depth). * test(s3): add integration test for deep-hierarchy version listing pagination Adds TestVersioningPaginationDeepDirectoryHierarchy which creates objects across 20 subdirectories at depth 6 (mimicking Veeam 365 backup layout) and paginates through them with small maxKeys. Verifies correctness (no duplicates, sorted order, all objects found) and checks that later pages don't take dramatically longer than earlier ones — the symptom of the pre-fix re-traversal bug. Also tests delimiter+pagination interaction across subdirectories. * test(s3): strengthen deep-hierarchy pagination assertions - Replace timing warning (t.Logf) with a failing assertion (t.Errorf) so pagination regressions actually fail the test. - Replace generic count/uniqueness/sort checks on CommonPrefixes with exact equality against the expected prefix slice, catching wrong-but- sorted results. * test(s3): use allKeys for exact assertion in deep-hierarchy pagination test Wire the allKeys slice (previously unused dead code) into the version listing assertion, replacing generic count/uniqueness/sort checks with an exact equality comparison against the keys that were created. |
||
|
Chris Lu
|
98714b9f70 |
fix(test): address flaky S3 distributed lock integration test (#8888)
* fix(test): address flaky S3 distributed lock integration test Two root causes: 1. Lock ring convergence race: After waitForFilerCount(2) confirms the master sees both filers, there's a window where filer0's lock ring still only contains itself (master's LockRingUpdate broadcast is delayed by the 1s stabilization timer). During this window filer0 considers itself primary for ALL keys, so both filers can independently grant the same lock. Fix: Add waitForLockRingConverged() that acquires the same lock through both filers and verifies mutual exclusion before proceeding. 2. Hash function mismatch: ownerForObjectLock used util.HashStringToLong (MD5 + modulo) to predict lock owners, but the production DLM uses CRC32 consistent hashing via HashRing. This meant the test could pick keys that route to the same filer, not exercising the cross-filer coordination it intended to test. Fix: Use lock_manager.NewHashRing + GetPrimary() to match production routing exactly. * fix(test): verify lock denial reason in convergence check Ensure the convergence check only returns true when the second lock attempt is denied specifically because the lock is already owned, avoiding false positives from transient errors. * fix(test): check one key per primary filer in convergence wait A single arbitrary key can false-pass: if its real primary is the filer with the stale ring, mutual exclusion holds trivially because that filer IS the correct primary. Generate one test key per distinct primary using the same consistent-hash ring as production, so a stale ring on any filer is caught deterministically. |
||
|
Chris Lu
|
d5068b3ee6 | test: harden weed mini readiness checks | ||
|
Chris Lu
|
0884acd70c |
ci: add S3 mutation regression coverage (#8804)
* test/s3: stabilize distributed lock regression * ci: add S3 mutation regression workflow * test: fix delete regression readiness probe * test: address mutation regression review comments |
||
|
Chris Lu
|
0adb78bc6b |
s3api: make conditional mutations atomic and AWS-compatible (#8802)
* s3api: serialize conditional write finalization * s3api: add conditional delete mutation checks * s3api: enforce destination conditions for copy * s3api: revalidate multipart completion under lock * s3api: rollback failed put finalization hooks * s3api: report delete-marker version deletions * s3api: fix copy destination versioning edge cases * s3api: make versioned multipart completion idempotent * test/s3: cover conditional mutation regressions * s3api: rollback failed copy version finalization * s3api: resolve suspended delete conditions via latest entry * s3api: remove copy test null-version injection * s3api: reject out-of-order multipart completions * s3api: preserve multipart replay version metadata * s3api: surface copy destination existence errors * s3api: simplify delete condition target resolution * test/s3: make conditional delete assertions order independent * test/s3: add distributed lock gateway integration * s3api: fail closed multipart versioned completion * s3api: harden copy metadata and overwrite paths * s3api: create delete markers for suspended deletes * s3api: allow duplicate multipart completion parts |