* test: cover listings spanning a run of retracted keys
A listing drops entries whose current version is a delete marker. When a run
of consecutive entries drops out, the page being filled can come back empty,
and an empty page is easily mistaken for the end of the listing — everything
after the run then never appears and the caller is told those objects do not
exist.
Backup repositories produce exactly this shape: a batch of keys under one
prefix is retracted while writing continues under the next.
Covers a retracted run before live keys and between live keys, walked with
page sizes smaller than the run so at least one page is filled entirely from
entries that get dropped, plus the version view of the same namespace where
every version and every delete marker must still be reported.
* test: sweep every page size in both walks and paginate the version listing
A listing on a versioned bucket is served from metadata cached on the
.versions directory entry so the whole listing is a single scan. The cache
carried size, mtime, ETag, owner and the delete-marker flag but not the
storage class, so newListEntry found none and fell back to STANDARD.
The result was that HEAD and the listings disagreed about the same object:
HEAD reported the class the object was stored with, while ListObjectsV2 and
ListObjectVersions reported STANDARD for every object. Clients that filter or
tier on storage class act on the listing.
Caches the class alongside the other listing fields, clears it with them, and
copies it in the routed RECOMPUTE_LATEST path so both finalize paths agree.
* test: compare ListObjects and ListObjectVersions over the same namespace
The two listings walk the same tree through separate code paths, so a client
navigating by versioned listings can see a different namespace than one
navigating by plain listings, and concludes keys are missing that are plainly
there. Testing each path on its own never catches that; only comparing them
does, and nothing compared them.
Asserts both report identical current keys and identical common prefixes
across a backup-shaped tree: nested prefixes, a prefix naming an object
exactly, a key that is simultaneously an object and the parent of other keys,
a partial key fragment, and a prefix matching nothing.
The version view is reduced to what a plain listing reports — latest versions
that are not delete markers — so the comparison is like for like.
* test: guard against truncated pages and cover the delete-marker path
* test: pin verb parity on lock-arbitration keys through acquire and release
Backup clients arbitrate repository ownership by writing and retracting small
keys under a fixed prefix and re-probing them, each probe using a different
verb. They trust those verbs to agree; a key reported present by one and
absent by another makes the client either spin or declare the repository
corrupt, and neither shows up as an error on the storage side because each
individual answer is locally correct.
The keys are written and immediately deleted by version id, which is the cycle
that empties a version container, so parity is asserted on both sides of the
delete and across repeated re-acquire cycles where residue accumulates.
Reports which verbs disagreed rather than just failing.
* test: run the reacquire cycle on every lock key, and drain probe bodies
* test: pin that an unusable version id is refused, never resolved
A version id containing a path separator, or "." / "..", can never name a
stored version. Resolving one to the null or latest version instead would let
a caller destroy a live version by asking for one that does not exist, on a
bucket configured for immutability.
The guard exists today and holds on every verb; it had no test. Pins two
properties: such a request is refused with a client error rather than a 5xx
(a 5xx invites endless retries of something that can never succeed), and the
version that does exist survives every refused request.
* test: require exactly 400 for an unusable version id
* test: cover delete idempotency on versioned object-locked buckets
Backup clients probe and retract lock keys continuously, so they routinely
delete keys and versions that are already gone, and they batch those deletes
alongside keys that do exist. S3 makes all of that succeed; returning an error
turns ordinary lock arbitration into a job failure.
The behaviour is correct today but had no coverage, and it runs through the
object-lock retention check, which is the most likely place for a missing
object to start being reported as an error.
Covers: deleting a key that never existed, deleting a well-formed version id
that names nothing (twice, and without disturbing the version that does
exist), and a batch whose middle key is missing — every requested key must
come back under its own name rather than silently taking another row's slot.
* test: verify the deletes actually took effect, not just that they returned
* s3: allow copying an object onto itself in a versioned bucket
The copy writes a new version instead of overwriting in place, which is how an
earlier version is restored. Buckets with versioning off or suspended keep
rejecting a self-copy that changes nothing.
* s3: cover the suspended-versioning self-copy rejection
Suspended versioning overwrites the null version in place, so a self-copy that
changes nothing stays rejected. Pin that alongside the never-versioned case.
* s3: keep the list marker exclusive for versioned objects
A versioned object lives in a "<key>.versions" directory, so the entry name
never matched the marker and start-after/marker returned the marker key itself.
* s3: match the list marker against the raw entry name too
A backend that echoes the marker it was given returns the ".versions" directory
name, which no longer matched once the comparison used the object name alone.
Cover both, and unit test each half.
* ci: move the fusermount3 repair into a composite action
Three copies of the same block were already drifting apart, and the
target comes from PATH: only ever add setuid root to a root-owned,
non-symlink binary under the system bin paths, and say why otherwise.
* test: say that the process exited in the wait errors
"process exit status 1 before ... accepted connections" is missing its
verb. Also mark the SIGTERM return discarded - it fails with
os.ErrProcessDone exactly when the select below already handles it.
* ci: prefer the distro fusermount3 over escalating a shadow copy
The shadowing /usr/local/bin/fusermount3 is not root-owned either, so
setting its setuid bit would have handed root to a binary the runner
user owns - the repair now symlinks the distro one earlier in PATH and
touches nothing, keeping the in-place chmod for a root-owned binary with
no distro alternative. A setuid bit only grants root when root owns the
file, so accept an existing one only then.
* ci: run the FUSE workflows when the shared action changes
Their paths filters listed each workflow file but not the composite
action all three now call.
* ci: restore the setuid bit on a shadowed fusermount3
Newer ubuntu-22.04 runner images carry a source-built fusermount3 in
/usr/local/bin that shadows the distro one in PATH and is not setuid
root. go-fuse looks the helper up through PATH, so every unprivileged
mount fails with "mount failed: Operation not permitted".
* test: fail a fuse test as soon as its mount process dies
A mount that cannot mount at all exits within a second, but the harness
still waited out the 30s readiness timeout and then reported "mount
point not ready within timeout", leaving the real cause buried in the
log tail. Watch the child processes and report their exit instead.
* mount: report a failed mount without a goroutine dump
A mount failure is an environment problem - no /dev/fuse, fusermount not
setuid, stale mount point - and the all-goroutine stack dump Fatalf adds
buries the one line that says so.
* s3: list the buckets an attached IAM policy grants
ListBuckets served an identity authorized by an attached IAM policy only
the buckets it had created itself. A user granted s3:ListBucket on a
bucket someone else provisioned could GetObject and ListObjectsV2 against
it, but the bucket never showed up in the listing any S3 client uses to
build its bucket picker.
The owner-index fast path is only valid for an identity whose grants name
every bucket it can reach, and the routing check assumed a policy could
never be enumerated. Read the names out of the policy instead: statements
that allow s3:ListBucket on a concrete bucket ARN become candidates, and
the per-bucket permission re-check still decides what is listed. A policy
that can reach a bucket it does not name -- a wildcard resource, a policy
variable, a NotResource, an STS session policy -- falls back to the full
scan, which evaluates the policy per bucket.
* s3: share the attached policy name lookup
authorizeWithIAM and the ListBuckets enumeration both built an identity's
policy names the same way, its own plus the ones from its enabled groups.
Pull that into one helper so group eligibility is decided in a single place.
* s3: read policy actions the way the IAM authorizer matches them
The IAM authorizer matches action names case-insensitively, so a policy
granting "S3:LISTBUCKET" or "S3:*" authorizes a list. The ListBuckets
classifier read those actions with the local case-sensitive matcher and
found no grant, so once the owner index was ready the buckets that policy
allows dropped out of the listing.
Match the action the looser way in the classifier: case-insensitive, and
true for any pattern holding a policy variable. Over-matching only costs a
candidate the per-bucket permission check then rejects, while under-matching
hides a bucket the caller can read.
* s3: infer a multipart grant in any case
The classifier matches action patterns case-insensitively but looked the
requested action up in a canonical-cased set, so "S3:UPLOADPART" missed the
s3:PutObject inference that the authorizer makes. Key the set for lookup in
lower case, matching how the IAM authorizer holds it.
Deleting the only object under a prefix in a versioned bucket writes a
delete marker and keeps the version history, so the filer directory
survives with nothing a current-version listing would return. A delimited
ListObjects kept reporting that path in CommonPrefixes, because the
prefixes come from the directory tree rather than from the keys, while a
listing scoped inside the prefix correctly came back empty.
Probe a directory before reporting it: one that holds entries but no key
the listing returns is neither a CommonPrefix nor a path the
trailing-slash probe answers for. Empty directories keep the meaning they
have today, and the probe only runs for buckets with versioning
configured, the only ones that can reach this state.
The copying and tagging tests force-drop each bucket's collection at the
master so volume slots are freed deterministically between tests. But the
copy-tests CI job runs its master on 9336 and the tagging Makefile on 9338,
while the tests default to 9333 — the cleanup dialed a dead port and quietly
no-oped. Each test bucket then grows 7 volumes against -volume.max=100, and
whenever async deletion lagged the data node ran out of slots and PutObject
500ed with "No writable volumes and no free volumes left".
Set MASTER_ENDPOINT where the master port is non-default: the copy-tests
workflow step, and the copying/tagging Makefiles (derived from MASTER_PORT).
* Add GitHub Actions workflow for codespell on master
* Add rudimentary codespell config
* Tune codespell config: skip generated code, ignore camelCase, whitelist domain terms
Add camelCase/PascalCase regex to ignore common Go/Rust/JS identifiers
like allLocations, publishErr, ReadInside, FlushInterval. Also skip
templ-generated *_templ.go files, and whitelist a handful of
short/domain-specific words (visibles, fo, te, ser, bject, unparseable,
keep-alives, tread, anc, ue) that show up as false positives across the
tree.
Co-Authored-By: Claude Code 2.1.217 / Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Fix ambiguous typos and protect false positives
Fixes typos that codespell reports with multiple candidate suggestions
(so `codespell -w` cannot auto-apply them), plus one inline pragma and
one config entry to protect legitimate identifiers.
Manual fixes (single correct answer chosen from context):
- pattens -> patterns (5x) in filer/upload/shell flag help strings
- finded -> found (2x) in tarantool storage.lua comment
- spacify -> specify (2x) in helm chart values.yaml comment
- wether -> whether in skiplist.go docstring
- simpe -> simple in mq schema test case name
False-positive protection:
- Add `//codespell:ignore` next to `source GET's` (possessive of HTTP
verb) in s3api_object_handlers_copy_stream.go
- Whitelist `auther` in .codespellrc — it's a local variable meaning
"authenticator" in weed/security/tls.go, not a typo of "author".
Co-Authored-By: Claude Code 2.1.217 / Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Extend codespell ignore list: .git-meta path and thirdparty groupId
Also skip `.git-meta` (scratch dir for commit messages that may contain
typo words verbatim) and whitelist `thirdparty` — it appears as the
literal Maven groupId `org.apache.hadoop.thirdparty` in hdfs3 poms
and cannot be renamed.
Co-Authored-By: Claude Code 2.1.217 / Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* [DATALAD RUNCMD] Fix non-ambiguous typos with codespell -w
Auto-applied fixes to the 44 remaining single-suggestion typos across
docs, comments, log messages, tests, config, and one Java pom.
=== Do not change lines below ===
{
"chain": [],
"cmd": "uvx codespell -w",
"exit": 0,
"extra_inputs": [],
"inputs": [],
"outputs": [],
"pwd": "."
}
^^^ Do not change lines above ^^^
* Revert breaking codespell fixes; whitelist unknwon and atleast
Two of the auto-applied `codespell -w` fixes were false positives that
would break the build/tests:
- go.mod: `github.com/unknwon/goconfig` is a real Go module path — the
upstream author's GitHub handle is literally `unknwon`. Renaming to
`unknown` would fail dependency resolution.
- test/benchmark/fuse_db/bin/{sqlite_verify.py,run_mysql.sh,run_sqlite.sh}:
`atleast` is a literal CLI mode value (a string constant compared and
passed as a positional argument). Rewriting to `at least` splits it
into two arguments and breaks the mode check.
Reverted those files and whitelisted both words in .codespellrc so
future runs won't re-suggest the same broken fixes.
Co-Authored-By: Claude Code 2.1.217 / Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Code 2.1.217 / Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Commit 8bff3b32 changed BatchDelete to keep processing after a cookie
mismatch but left the integration test asserting the old early-break
behavior, breaking Volume Server Integration Tests (grpc - Shard 1) on
master. Align the test with the new semantics and port the same
break->continue to the Rust volume server, which runs the same suite
via VOLUME_SERVER_IMPL=rust.
The FUSE harness picks the filer HTTP port and lets "weed mount" derive the
filer gRPC port as HTTP+10000. freePort kept the HTTP port under the Linux
ephemeral floor (32768) but not the derived gRPC port, which ranged up to
42000. When the gRPC port landed above the floor an outbound connection could
transiently hold it, so mini relocated its filer gRPC port while mount kept
dialing HTTP+10000, timing the mount out.
Cap the HTTP port at 22000 so the gRPC sibling stays at or below 32000, and
verify both ports are bindable before returning.
* sftp: match permission paths on path-component boundaries
Permission checks compared paths with raw string prefixes, so a
permission entry for /tenants/alice also matched sibling paths such as
/tenants/alice-archive, letting a scoped user read and overwrite
another tenant's files. The home directory containment check had the
same flaw. Route both through pathWithin, which cleans the paths and
requires exact equality or a separator-delimited descendant.
* sftp: clean permission path into a local when ranking matches
* admin: move volume-server read JWT helper into dash
The Iceberg data preview page needs the same per-fileId read token the
file browser uses when streaming chunks from volume servers.
Claude-Session: https://claude.ai/code/session_015n3oKLTjnPjcnZtfigNKur
* admin: add Iceberg table data preview page
The admin UI browses the Iceberg catalog down to table details but not
the data itself. Add a Browse Data page per table that walks the
selected snapshot's manifests and shows sample rows from its Parquet
data files, plus the data file list with per-file preview, a snapshot
switcher, and a row limit selector.
Rows are read through a ranged ReaderAt over stream-content so only
the Parquet footer and needed pages are fetched, with the volume read
JWT applied when configured. Iceberg locations resolve into /buckets
with traversal guards, and the file parameter must match a
manifest-listed data file. Snapshots with delete files get a warning
that raw rows are shown.
Claude-Session: https://claude.ai/code/session_015n3oKLTjnPjcnZtfigNKur
* admin: integration test for Iceberg catalog and data preview pages
Starts a weed mini cluster with the admin UI, creates a table bucket,
namespace, and tables via the S3 Tables manager, uploads real Parquet
files via S3, writes manifests and snapshots with iceberg-go, and
asserts on the rendered pages: catalog browsing, table details,
current and historical snapshot previews, per-file preview, row
limits, unknown snapshot and file errors, and a metadata-less table.
Claude-Session: https://claude.ai/code/session_015n3oKLTjnPjcnZtfigNKur
* admin: write Iceberg preview chunk reads straight into the caller slice
ReadAt wrapped the caller's buffer in a bytes.Buffer, which would
silently allocate a fresh backing array and drop bytes if it ever grew.
Copy directly into the destination slice and reject negative offsets so
the ReaderAt contract holds.
Claude-Session: https://claude.ai/code/session_015n3oKLTjnPjcnZtfigNKur
* admin: link to snapshot history when the preview switcher truncates
The snapshot switcher caps at 25 entries; add a trailing item pointing
at the table details page so older snapshots stay reachable.
Claude-Session: https://claude.ai/code/session_015n3oKLTjnPjcnZtfigNKur
* test: hoist mini cluster context assignment out of the goroutine
Set MiniClusterCtx before launching the cluster goroutine and clear it
in stop(), so the assignment is not buried in the command loop.
Claude-Session: https://claude.ai/code/session_015n3oKLTjnPjcnZtfigNKur
* 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.
glog flags (-v, -logtostderr) are registered on weed's global flagset,
so passing them after the subcommand name kills the process at flag
parsing: flag provided but not defined: -logtostderr. The old stat-based
mount readiness probe masked this — TestWriteBufferCap silently ran
against the bare local directory and passed. The device-ID readiness
check now surfaces the dead mount as a not-ready timeout.
Move glog flags into MountGlobalOptions, emitted before the subcommand,
and do the same for the EnableDebug verbosity flag on mini and mount.
* fix(master): let the growth initiator wait for the growth it triggered
The growth-in-flight shed also fired on the request that initiated the
growth: it sets the pending flag right before the shed check, so a
cold-start assign enqueued growth and immediately failed itself with
"volume growth in progress". With no concurrent assigns around to pick
up the freshly grown volume, a single writer against an empty cluster
never completes a write despite ample free space.
Claim the pending flag with a compare-and-swap so exactly one request
becomes the initiator, triggering growth at most once, and let it wait
for that growth to land. Everyone else still sheds retryably instead of
pinning a goroutine: followers behind an in-flight growth, an initiator
whose growth concluded without yielding a writable volume, and an
initiator whose growth outlives the 10s wait budget, which previously
surfaced a non-retryable error (gRPC Unknown, HTTP 406) even though a
retry would have succeeded moments later.
* fix(master): stop assign waits when the request is cancelled
The assign retry loops slept through client cancellation, keeping a
goroutine spinning for the rest of the 10s budget after the caller had
gone; StreamAssign also ran assigns on a background context detached
from the stream. Wait on the request context and pass the stream
context through.
* topology: drop the unconditional grow-request setter
Growth is only claimed through AddGrowRequestIfAbsent's compare-and-swap
now; keeping the raw Store(true) around invites the check-then-set race
back.
* test: cover cold-start first write with a real cluster
Boot a fresh master plus three empty volume servers and require the very
first assign - HTTP and gRPC, each on a cold volume layout, no client
retries - to complete a write. The assign that triggers volume growth
must wait for it rather than answering "volume growth in progress";
unit tests stub the topology, so only a real cluster exercises the
assign-grow-wait path end to end.
* test: keep mini-allocated ports below the ephemeral floor
Allocated ports could land in 32768-55000, so a transient outbound dial
during mini startup (volume->master gRPC, etc.) could grab an allocated
port as its source port before the filer bound it, failing with
"bind: address already in use". Cap the range so port+GrpcPortOffset
stays under 32768.
* test: derive mini port cap from the ephemeral floor constant
Name the 32768 floor once and compute miniPortMax as floor-GrpcPortOffset
so the cap tracks the offset; reuse the constant in the regression test.
waitForMount probed the mount point with stat+ReadDir, which a bare local
directory also passes, so Setup could return before the weed mount process
finished mounting. Tests then wrote to the local disk underneath the mount
point; when the mount activated it shadowed those files, producing the
intermittent TestConcurrentFileOperations/ConcurrentReadWrite ENOENT with
an empty ReadDir. Require the mount point's device ID to differ from its
parent's before reporting ready.
* ci: add per-process memory sampler for perf jobs
Samples VmRSS once a second into a CSV and records peak VmHWM per process
on stop. Linux only; reads /proc/<pid>/status.
* ci: run perf benchmarks on the Rust volume server and report memory
Matrix the throughput and S3 jobs over go/rust volume servers, using a
standalone master (plus filer for S3) and swapping only the volume binary
so the two are directly comparable. Sample peak RSS in every job and surface
it per impl in the run summary.
* ci: harden mem sampler arg handling and peak fallback
Guard against missing args under set -u, and fall back to the max RSS
sampled when a process exits before VmHWM can be read.
* 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.
* sts: enforce session-policy explicit deny during role chaining
A chained AssumeRole caller authenticates with an STS session token whose
inline session policy can explicitly deny sts:AssumeRole. The deny check only
evaluated the caller's named policies, so such a session could still chain into
any role its trust policy admits. Validate the session token in the deny check
and honor an explicit Deny in the inline session policy too.
* test(sts): integration coverage for AssumeRole authorization
Add an end-to-end AssumeRole authorization test (real weed mini + boto3):
a non-admin caller assumes a role its trust policy admits, an explicit
identity-side deny is blocked, and a session policy's explicit deny blocks
role chaining.
* sts: skip OIDC tokens and reject revoked sessions in the chaining deny check
Review follow-ups on the session-policy deny check:
- Guard session validation with !isOIDCToken so a bearer token our STS service
cannot validate does not error into a false deny.
- Reject a revoked session before evaluating its policy, restoring the
revocation enforcement the AssumeRole path lost when it stopped routing
through IsActionAllowed.
* build(deps): bump com.fasterxml.jackson.core:jackson-databind
Bumps [com.fasterxml.jackson.core:jackson-databind](https://github.com/FasterXML/jackson) from 2.18.6 to 2.22.0.
- [Commits](https://github.com/FasterXML/jackson/commits)
---
updated-dependencies:
- dependency-name: com.fasterxml.jackson.core:jackson-databind
dependency-version: 2.22.0
dependency-type: direct:production
...
Signed-off-by: dependabot[bot] <support@github.com>
* build(deps): pin jackson-annotations to its own 2.22 version
jackson-annotations dropped the patch digit in 2.20 and releases on its
own line, so 2.22.0 does not exist. Sharing jackson.version broke
dependency resolution; give it a dedicated property.
---------
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Chris Lu <chris.lu@gmail.com>
* 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>
* test: add FUSE database load/durability/perf benchmark
Runs MySQL (InnoDB) and SQLite with ~1GB datadirs on a SeaweedFS FUSE
mount. Two parts:
- durability: normal shutdown, kill -9, and crash-during-write all keep
every fsync-committed row (verified by integrity check + row count +
contiguous prefix + per-row CRC).
- performance: FUSE vs the same local disk. fsync/commit latency is the
dominant cost (~0.13ms -> ~1.18ms), so small transactions run ~9-12x
slower while bulk loads and warm reads stay close.
Harness is path-independent (runtime under $SEAWEED_BENCH_WORK) and only
touches its own processes on non-default ports.
* test/benchmark/fuse_db: portable to Linux + crash-safe progress
- export MYSQL_BIN; mysql_bench.py falls back to PATH when unset
- unmount via fusermount/fusermount3 (non-root Linux), then umount/diskutil
- atomic progress write (tmp+fsync+rename); treat empty progress file as 0
- reuse a single PRNG in the perf probe so RNG init doesn't skew timings
* test/benchmark/fuse_db: validate inputs, add subprocess timeout
- mysql_bench.py: 1800s timeout on mysql CLI calls; reject db names that
aren't plain identifiers (interpolated into SQL)
- sqlite_gen.py / sqlite_verify.py: allowlist journal/synchronous modes and
the verify mode so a typo can't silently weaken durability or relax checks
- run_mysql.sh: durable atomic progress write (tmp+fsync+rename), matching
sqlite_gen.py; quote $LB in both crash-test verify calls
* 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
* fix(ec): never delete recoverable shards on startup/reconcile (size-direction + byte-exact .dat)
EC startup validation and the cross-disk reconcile could delete the only
copy of distributed-EC shards whenever a non-empty .dat sat beside them.
This is the same data-loss class as the empty-.dat-stub fix, now for a
real (non-empty) stale or partial .dat.
validateEcVolume: the discriminating signal is the shard size relative to
the .dat's full encode, not the shard count.
- shards smaller than expected: an interrupted local encode left partial
shards and the .dat is the complete source -> reclaim the .dat.
- shards equal to expected: a valid (or still-distributing) EC volume ->
keep; the shards may be the only copy.
- shards larger than expected: the .dat is the stale/partial side (e.g. an
interrupted decode left a half-written .dat next to the real shards) ->
keep.
Previously any size mismatch, a low shard count beside a .dat, or a
transient stat error returned "delete", wiping sole-copy shards. Now every
ambiguity (size mismatch in either direction, inconsistent shard sizes,
transient I/O error, partial shard set) keeps the data; only a credible
full source .dat with no partial set to lose is reclaimed.
handleFoundEcxFile: a shard load failure (corrupt/locked .ecx, EMFILE
during a mass restart, transient I/O) no longer deletes the EC files when a
.dat exists -- it only unloads and keeps the files for retry. All deletion
authority now flows through validateEcVolume.
pruneIncompleteEcWithSiblingDat: count shards NODE-WIDE (a set split across
sibling disks summing to >= dataShards is independently recoverable and is
left alone), and require the sibling .dat to byte-exactly match the size
.vif recorded at encode time before deleting -- the prior "at least this
big, or bigger than a superblock" gate could trust a stale .dat and wipe
sole-copy shards. EC encode records the source size in .vif, so this gate
works for real volumes; older volumes without it fail safe (kept).
Rust volume server mirrors all of the above: size-direction + keep-on-
ambiguity in validate_ec_volume, keep-on-load-failure in
handle_found_ecx_file, and the node-wide + byte-exact gate in the prune.
The Rust validate/prune paths now resolve the data-shard count from the
volume's own .vif instead of hardcoding 10+4, so custom-ratio volumes are
not mis-sized and wrongly deleted on reboot.
Existing tests that encoded the old (unsafe) "delete on low count / size
mismatch" behavior are updated to the safe expectation, and new regression
tests cover the partial-decode-.dat-keeps-shards and transient-error-keeps
cases (Go and Rust); they fail on the pre-fix code.
* fix(ec): record DatFileSize in planted EC .vif for the prune test; trim comments
The multi-disk lifecycle e2e test planted a partial EC leftover with an
empty .vif, so the byte-exact prune gate (which a real encoded volume
satisfies via its recorded source size) kept it instead of cleaning up.
Record DatFileSize + the EC ratio in the planted .vif, matching production.
Also condense the verbose comments added in this change to the repo's
concise style.
* fix(storage): never let an empty .dat delete healthy distributed EC shards
A leftover empty .dat stub (a phantom from the pre-fix loader; zero
needles) next to a distributed EC volume's local shards made startup
classify the volume as an interrupted local encode: validateEcVolume
requires >= dataShards local shards when a .dat is present, fails with
the 1-2 shards a distributed volume keeps per disk, and the cleanup
deletes those shards -- the only copies of that part of the volume.
Repeated across restart waves this destroys enough shards cluster-wide
to make the volume unrecoverable.
Go:
- loadExistingVolume: hoist the empty-stub sweep above the EC presence
checks. Previously the .vif-next-to-.ecx guard returned before the
sweep ever ran, so exactly the dangerous layout (stub + .ecx + local
shards) kept its stub and then lost its shards in loadAllEcShards.
- validateEcVolume / checkDatFileExists: treat a .dat <= a superblock
(zero needles) as absent. An empty .dat cannot be the encode source,
so it must never gate shard deletion; this also covers stubs without
a .vif, which the sweep cannot prove are EC leftovers.
Rust mirror (seaweed-volume): the same gate in validate_ec_volume and
check_dat_file_exists (the Rust sweep already ran before validation);
the volume-load skip keeps a plain existence check so fresh,
needle-less volumes still load.
Regression tests in Go and Rust reproduce the production layout (a
zero-byte .dat beside .ecx/.ecj and two shards of a 10+4 volume, with
and without a .vif) and fail without the fix with the shards deleted.
* fix(ec): gate source volume deletion on a recoverable shard set
After EC encode, the shell command and the (plugin) worker task refused
to delete the source volume unless every shard was present, and aborted
otherwise -- leaving the source .dat next to live shards, exactly the
mixed state the startup cleanup mishandles.
Replace the full-set requirement with a recoverability gate shared by
both callers (RequireRecoverableShardSet): deleting a non-empty source
.dat requires at least dataShards distinct shards cluster-wide. Below
that the source is kept and the encode fails as before. A degraded but
recoverable set (>= dataShards, < total) now proceeds with a warning
instead of aborting: the missing shards can be rebuilt from the
survivors, while keeping the source would preserve the dangerous mixed
state. Empty stub replicas are still swept unguarded (OnlyEmpty) -- an
empty .dat has nothing to lose.
dataShards/totalShards stay parameters so enterprise custom EC ratios
share the helper verbatim.
* test(ec): use recoverable shard verification gate
* 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.
* feat(ec): add encode_ts_ns to EC shard metadata and the shard read RPC
EcShardConfig and VolumeEcShardReadRequest gain an int64 encode_ts_ns
(encode time in unix nanos). It rides in .vif and the read request so a
read can be scoped to the encode run that produced the index.
* fix(ec): stamp each encode and reject cross-run shard reads
Generate stamps EncodeTsNs into the volume's .vif. Reads carry it to the
shard's owning volume (resolved together via FindEcVolumeWithShard, so a
multi-disk server validates the disk that actually serves the bytes) and
reject a shard from a different encode run, recovering from parity. A
zero on either side (pre-upgrade volume) skips the guard.
* fix(ec): stamp the encode identity on the worker-generated .vif
The worker-local encode path now writes EncodeTsNs (and the resolved EC
ratio) into the .vif, so the read guard is not silently off for volumes
encoded by the maintenance worker.
* fix(ec): wipe stale EC artifacts before re-encoding
VolumeEcShardsGenerate evicts any in-memory EcVolume for the volume and
removes its on-disk shard/index/sidecar files before writing fresh ones,
so a retried encode never builds on a partial prior run and the unlink
frees the inodes instead of leaving open fds serving old bytes.
* fix(ec): unmount EC shards across all disks
UnmountEcShards walked only the first disk holding the shard, leaving a
duplicate copy mounted on a sibling disk (split-disk reconciled volumes)
still serving and heartbeating. Traverse every disk and emit one
deletion delta per disk.
* fix(ec): delete orphan shards without a local .ecx
deleteEcShardIdsForEachLocation gated shard-file removal on a local .ecx,
so it could not clean an orphan .ecNN left by a failed copy on a disk
with no index. Delete the requested shard files unconditionally; the
index-file (.ecx/.ecj/.vif) routing stays gated as before.
* fix(ec): clear stale EC shards cluster-wide before re-encoding
ec.encode unmounts and deletes EC shards for the target volumes on every
node before regenerating: fatal for the shards the topology reports
(mounted leftovers), best-effort for the rest (a sweep that catches
unmounted failed-copy orphans). A down node is a no-op.
* fix(ec): don't nil EC fds on close so reads can't race eviction
A reader resolves an EcVolume/shard under the lock then reads after it is
released, so an eviction that nils ecxFile/ecdFile would race that read
and panic. Close the fds without nilling the fields: the field is now
write-once (no data race) and a concurrent read hits a closed fd, getting
a clean error that the caller recovers from parity.
* fix(ec): wipe stale EC artifacts on every disk and surface failures
The pre-encode wipe only deleted beside the source volume, so a stale
shard on a sibling disk survived and could be mounted against the new
index at reconcile. Sweep every disk. Removal also ignored os.Remove
errors, reporting a failed cleanup as success and letting a stale shard
join the next generation; surface the first real failure (treating
already-gone as success) from removeStaleEcArtifacts and the shard delete.
* fix(ec): log when a local shard is skipped for a different encode run
The cross-run guard returned errShardNotLocal, indistinguishable in logs
from a genuinely-absent shard. Add a V(1) line naming both EncodeTsNs so
operators can tell "wrong encode generation" from "shard not here".
* fix(ec): surface metadata removal failures in the shard delete path
deleteEcShardIdsForEachLocation still dropped os.Remove errors on the
.ecx/.ecj/.vif/sidecar cleanup. A surviving stale .ecx is the orphan-index
condition this path prevents, so route those through removeFileIfExists and
return the first real failure instead of reporting cleanup as success.
* fix(ec): fail orphan cleanup when a reachable node's delete fails
The pre-encode orphan sweep swallowed every error for unreported (node,
volume) pairs. That is only safe for an unreachable node, which cannot
receive this encode's new generation. A reachable node whose delete
genuinely failed (permission/IO) keeps an orphan shard that a later copy
re-stamps with the new run's volume-level .vif identity, so the read guard
would accept stale data. Surface those; stay best-effort only for
unreachable nodes (gRPC Unavailable / no status).
* fix(ec): guard ecjFile under its lock in the EC delete path
EcVolume.Close nils ecjFile under ecjFileAccessLock; a delete that resolved
its .ecx lookup before a concurrent eviction (the generate-time
UnloadEcVolume) could then reach the journal append with a nil fd. Bail
with a clear "volume closed" error under the lock instead.
* fix(ec): reject an unstamped shard when the caller has an encode identity
The read guard required both identities nonzero, so a current (stamped)
caller accepted a holder with identity 0 and could be served a stale
pre-upgrade shard. Reject when the caller is stamped and the holder
differs (including unstamped); stay lenient only when the caller itself
has no identity (pre-upgrade reader). A skipped shard recovers from parity.
* fix(ec): full-teardown delete so cluster cleanup wipes a whole generation
The pre-encode cluster sweep deleted only the listed canonical shards on
remote nodes, leaving index/sidecar (and, on builds with versioned
generations, those too) behind. Add a full_teardown flag to
VolumeEcShardsDelete that evicts the volume and wipes every EC artifact for
it on every disk via removeStaleEcArtifacts; the shell and worker pre-encode
cleanup paths set it. Other delete callers (balance/decode/repair) are
unchanged.
* fix(ec): take ecjFileAccessLock before the nil-check in Sync and Close
Sync and Close read ev.ecjFile before acquiring ecjFileAccessLock while
Close nils it under the lock, a data race on the field. Take the lock
first, then nil-check inside, in both.
* fix(ec): acknowledge full_teardown so a pre-upgrade server can't fake success
An old volume server silently ignores full_teardown and returns success
for an ordinary delete, so the caller wrongly believes the generation was
wiped and copies a fresh gen-0 onto an unwiped node. Echo full_teardown_done
in the response; the worker destination cleanup fails when it is absent, and
the shell cluster sweep fails for a reported (mounted) leftover while staying
best-effort for an unreported node. encode_ts_ns stays an accepted transient
(an old server just skips the new read guard, no regression).
* fix(ec): fail the pre-encode sweep for any reachable node that can't ack teardown
A reachable pre-upgrade server ignores full_teardown and returns success
without wiping an orphan, which a later copy then folds into the new
generation. Treat a missing full_teardown_done ack as fatal for every
reachable node (best-effort only for a gRPC-unreachable one), not just for
topology-reported pairs.
* fix(ec): return the served shard identity and validate it client-side
The encode identity was only enforced server-side, so a pre-upgrade server
ignored the request field and served bytes unchecked. Echo the served
shard's EncodeTsNs on every read response chunk and have the client reject a
mismatch (including 0 from an old server), so the guard holds regardless of
server version; a rejected read recovers from parity.
* fix(ec): reject a short/empty remote shard read instead of serving zeros
doReadRemoteEcShardInterval accepted an immediate EOF or a short stream and
returned success with a partly zero-filled, unvalidated buffer (the server
stamps the identity only on chunks that carry bytes). A non-deleted interval
must arrive whole: require n == len(buf), exempting the is_deleted
short-circuit (n=0), matching readLocalEcShardInterval's local check. A short
read now fails so the caller recovers from parity.
* test(ec): fake volume server echoes the full_teardown acknowledgement
The worker now fails a teardown delete that isn't acknowledged (so a
pre-upgrade server can't silently skip the wipe). The fake server's no-op
VolumeEcShardsDelete returned an empty response, which the worker read as a
skipped teardown and aborted the encode. Echo full_teardown_done.
* feat(ec): mirror the encode-run identity guard + full_teardown into the Rust volume server
The Go volume server stamps an encode-run identity (encode_ts_ns) into the .vif
and rejects a read served from a shard of a different run; full_teardown wipes a
whole generation and acknowledges it. The Rust volume server had none of it.
Mirror the shared logic: load encode_ts_ns from the .vif onto the EcVolume,
stamp it on every read response, and reject a request/response mismatch on both
the server and the distributed-read client (recovering from parity); handle
full_teardown by evicting the volume and wiping every EC artifact on each disk,
echoing full_teardown_done so the caller can detect a server that ignored it.
* fix(ec): remove a stale .vif on full teardown of a shard-only node
A shard copy installs shards + .ecx before .vif, so an interrupted copy after a
teardown could mount the new files under the previous run's identity / version /
shard ratio / dat_file_size carried by the surviving .vif. Remove .vif during
full teardown, gated on .idx absence so a source-volume holder keeps its live
.vif. In Rust this lives in a teardown-only helper so the reconcile / load-
fallback paths (which share the base removal) still preserve .vif.
* fix(ec): treat a missing teardown ack as fatal, not as an unreachable node
isNodeUnreachable returned true for any non-gRPC-status error, so a reachable
pre-upgrade server's missing full_teardown_done ack (a plain error) was
classified unreachable and the unreported pair was silently skipped. Classify
only a real codes.Unavailable as unreachable, and wrap the missing ack in a
sentinel the sweep treats as fatal regardless. A genuinely down node still
surfaces as Unavailable from the RPC and stays best-effort.
* fix(ec): reject a short shard read in the local EC needle reader
read_ec_shard_needle ignored the byte count from shard.read_at and appended the
whole pre-sized buffer, so a truncated shard's zero-filled tail passed the later
length check and parsed as garbage. Require n == buf.len() per interval, erroring
on a short read like the local interval reader already does.
* fix(ec): probe reachability before skipping a node that returns Unavailable
The pre-encode sweep skipped any node whose teardown delete returned
codes.Unavailable, but a reachable volume server in maintenance mode also
returns that code for the maintenance-gated delete, so its stale EC files were
left behind on a node that can still receive the new generation. Confirm with a
non-maintenance-gated empty-target Ping: skip only when the node fails the probe
too (genuinely unreachable).
* fix(ec): use try_exists for the teardown .vif .idx guard
The teardown-only .vif removal gated on Path::exists(), which returns false on a
permission/IO stat error, so a stat failure on a present .idx would read as a
shard-only node and delete the live source volume's .vif. Gate on
try_exists() == Ok(false) instead, preserving the sidecar on any stat error.
* fix(ec): only skip a sweep node when a Ping confirms it is transport-down
The pre-encode sweep skipped a node whenever its teardown delete and a liveness
Ping both failed, but it treated ANY Ping error as down — an application-level
Internal/ResourceExhausted, or Unimplemented from a pre-Ping server, left a
reachable node's stale generation in place. Classify the Ping tri-state and skip
only when it transport-fails with codes.Unavailable; a reachable or inconclusive
node stays fatal.
* fix(ec): exclude sweep-skipped nodes from the encode's rebalance
The pre-encode sweep skips a genuinely-down node best-effort, but the rebalance
then recollected the current topology — a node that recovered between the two
could become a copy target and receive the new generation while still holding
its stale, never-cleared shards. Have the sweep return the skipped set and
exclude those nodes from the rebalance for this encode, so a node we could not
clean cannot receive the new generation. Standalone ec.balance is unaffected.
* fix(ec): re-sweep recovered nodes before generation so they aren't stranded
A node skipped as down by the pre-encode sweep is excluded from the rebalance,
but it can recover and become the generation host — mounting all shards locally,
then being excluded from distribution. Union-only verification accepts all
shards on one node and deletes the originals: a single point of failure. Re-sweep
the skipped nodes just before generation; one whose teardown now succeeds leaves
the skipped set and rebalances normally, while a node still down stays skipped.
* fix(ec): abort the encode if a selected source is still skipped after re-sweep
The re-sweep un-skips a recovered node, but the source was selected before it and
a node can stay down through the re-sweep then recover just in time to be the
generation host — mounting all shards locally while still excluded from the
rebalance, which union-only verification accepts before deleting the originals.
Abort the encode when a selected source remains skipped after the re-sweep.
* fix(ec): batch delete returns retriable 503 when a volume became EC mid-batch
If a volume is not EC at the batch-delete classification but is encoded to EC and
its .dat deleted before the regular-volume mutation, the mutation returns an exact
"not found" that the filer chunk-GC treats as completed, dropping the delete.
Recheck EC presence under the mutation lock and return a retriable 503 with the
"try again" token so the filer requeues it onto the EC path.
* fix(ec): recheck EC state before the regular batch-delete mutation
ec.encode mounts EC shards (copied from the .dat) before deleting the originals,
so a volume can be EC while its .dat still exists. The batch delete only rechecked
EC after a NotFound, so a successful regular-volume delete in that window wrote a
tombstone to the soon-removed .dat — the delete was lost and the needle resurrected
from the pre-tombstone shards. Recheck has_ec_volume under the write lock before
delete_volume_needle and return a retriable 503 so the filer requeues onto the EC path.
* fix(volume): make the metrics push test independent of test order
test_push_metrics_once asserted the pushed body contains the request-counter
family without ever touching the counter — a CounterVec with no children emits
nothing, so the assertion only held when another test had already created a
labelset in the shared registry. Create one in the test itself.
* ci(s3tables): drop docker pre-pull from Lakekeeper job
The lakekeeper repro is pure Go against the local weed binary; the job
kept failing on Docker Hub timeouts pulling python:3 and localstack
images the test never runs. Also drop the stale python-in-docker
comments left from the old harness.
* ci(s3tables): serve python:3 from GHA cache in the STS job
Retried pulls still die when both mirror.gcr.io and registry-1.docker.io
are unreachable from the runner. Cache the saved image tarball under a
weekly key: an exact hit skips the registry entirely, a miss pulls fresh
and refreshes the cache, and a stale tarball from a previous week is the
fallback when Docker Hub is down.
* ci(spark): pre-pull the spark tag the test actually runs
The workflow warmed apache/spark:3.5.8 with retries while the
testcontainers setup runs apache/spark:3.5.1, so the real image was
pulled at test time with no retry at all.
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.
- 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.
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.
* 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.
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).
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.
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.
Chris LuandGitHub
Konstantin LebedevandGitHub