mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-07-22 16:03:20 +00:00
filer-conditional-create
182 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Chris Lu
|
3d872a1416 |
fix(filer): load -s3.config static identities into the filer's CredentialManager (#9537)
When weed filer started its embedded S3 gateway with -s3 -s3.config, only the S3 server loaded the s3.json static identities — the filer's own CredentialManager stayed empty, so the IAM gRPC service backing the admin UI and weed shell returned only dynamic users. Mirror the wiring weed server already does and hand the same config path to the filer. |
||
|
Chris Lu
|
62821964dd |
filer/iam-grpc: make admin Bearer auth opt-in (fixes #9509) (#9514)
PR #9442 made the filer refuse to register the IAM gRPC service unless jwt.filer_signing.key was set in security.toml, which broke the admin UI Users/Groups/Policies pages for every deployment that ships without a security.toml — weed mini, plain Helm, vanilla weed filer. The Users tab returns Unimplemented and the page is unusable. Issues #9504, #9505 and #9509 all trace to this gap. The rest of the filer's gRPC surface is unauthenticated by default; treat IAM the same way. The service now always registers, and the auth gate is a no-op when no signing key is configured. When the key is set, every RPC still requires an admin-signed Bearer token, matching the post-#9442 behaviour. Operators who expose the filer gRPC port beyond a trusted network should set the key on both filer and admin. The admin client (IamGrpcStore.withIamClient) already skips attaching the authorization metadata when its key is empty, so no changes there. |
||
|
Chris Lu
|
5e8f99f40a |
filer: require admin-signed JWT on the IAM gRPC service (#9442)
Every IAM RPC (CreateUser, PutPolicy, CreateAccessKey, ...) now requires a Bearer token in the authorization metadata, signed with the filer write-signing key. The service refuses to register on a filer that has no jwt.filer_signing.key set, so the unauthenticated default is gone: operators who use these RPCs must configure the key and attach a token on every call. Bearer scheme matching is case-insensitive (RFC 6750), every handler nil-checks req before dereferencing it, and tests now cover the expired-token path. |
||
|
Chris Lu
|
d605feb403 |
refactor(command): expand "~" in all path-style CLI flags (#9306)
* refactor(command): expand "~" in all path-style CLI flags Many of weed's path-bearing flags (-s3.config, -s3.iam.config, -admin.dataDir, -webdav.cacheDir, -volume.dir.idx, TLS cert/key files, profile output paths, mount cache dirs, sftp key files, ...) were never run through util.ResolvePath, so a value like "~/iam.json" was used literally. Tilde only worked when the shell expanded it, which silently fails for the common -flag=~/path form (bash leaves the tilde literal in --opt=~/path). - Extend util.ResolvePath to also handle "~user" / "~user/rest", matching shell tilde expansion. Add unit tests. - Apply util.ResolvePath at the top of each shared start* function (s3, webdav, sftp) so mini/server/filer/standalone callers all inherit it; resolve at the few one-off use sites (mount cache dirs, volume idx folder, mini admin.dataDir, profile paths). - Drop the duplicate expandHomeDir helper from admin.go in favor of the now-equivalent util.ResolvePath. * fixup: handle comma-separated -dir flags for tilde expansion `weed mini -dir`, `weed server -dir`, and `weed volume -dir` accept comma-separated paths (`dir[,dir]...`). Calling util.ResolvePath on the whole string mishandled multi-folder values with tilde, e.g. "~/d1,~/d2" would resolve as if "d1,~/d2" were a single subpath. - Add util.ResolveCommaSeparatedPaths: split on ",", run each entry through ResolvePath, rejoin. Short-circuits when no "~" present. - Use it for *miniDataFolders (mini.go), *volumeDataFolders (server.go), and resolve each entry of v.folders in-place (volume.go) so all downstream consumers see resolved paths. - Add 7-case TestResolveCommaSeparatedPaths covering empty, single, multiple, and mixed inputs. * address PR review: metaFolder + Windows backslash - master.go: resolve *m.metaFolder at the top of runMaster so util.FullPath(*m.metaFolder) on the next line sees an expanded path. Drop the now-redundant ResolvePath in TestFolderWritable. - server.go: same treatment for *masterOptions.metaFolder, paired with the existing cpu/mem profile resolves. Drop the redundant inner ResolvePath at TestFolderWritable. - file_util.go: ResolvePath now accepts filepath.Separator as a separator after the tilde, so "~\\data" works on Windows. Other platforms keep current behaviour (backslash stays literal because it is a valid filename character in usernames and paths). - file_util_test.go: add two cases using filepath.Separator that exercise the new code path on Windows and remain a no-op on Unix. * address PR review: resolve "~" in remaining command path flags Comprehensive sweep of path-bearing flags across every weed subcommand, applying util.ResolvePath in-place at the top of each run* function so all downstream consumers see expanded paths. - webdav.go: resolve *wo.cacheDir at the top of startWebDav so mini/server/filer/standalone callers all inherit it. - mount_std.go: cpu/mem profile paths. - filer_sync.go: cpu/mem profile paths. - mq_broker.go: cpu/mem profile paths. - benchmark.go: cpuprofile output path. - backup.go: -dir resolved once at runBackup; drop the duplicated inline ResolvePath in NewVolume calls. - compact.go: -dir resolved at runCompact; drop inline ResolvePath. - export.go: -dir and -o resolved at runExport; drop inline ResolvePath in LoadFromIdx and ScanVolumeFile. - download.go: -dir resolved at runDownload; drop inline. - update.go: -dir resolved at runUpdate so filepath.Join uses the expanded path; drop inline ResolvePath in TestFolderWritable. - scaffold.go: -output expanded before filepath.Join. - worker.go: -workingDir expanded before being passed to runtime. * address PR review: resolve option-struct paths at run* entry points server.go:381 propagates s3Options.config to filerOptions.s3ConfigFile *before* startS3Server runs, which meant the filer-side code saw the unresolved tilde-prefixed pointer. Same pattern for webdavOptions and sftpOptions (and equivalent in mini.go / filer.go). The fix: hoist resolution from the shared start* functions up to the run* entry points, where every shared pointer is set up before any propagation happens. - s3.go, webdav.go, sftp.go: extract a resolvePaths() method on each Options struct that runs every path field through util.ResolvePath in-place. Idempotent. - runS3, runWebDav, runSftp: call the standalone struct's resolvePaths before starting metrics / loading security config. - runServer, runMini, runFiler: call resolvePaths on every embedded options struct, plus resolve loose flags (serverIamConfig, miniS3Config, miniIamConfig, miniMasterOptions.metaFolder, and filer's defaultLevelDbDirectory) so they're expanded before any pointer copy or use. - Drop the now-redundant inline ResolvePath at filer's defaultLevelDbDirectory composition. * address PR review: re-resolve mini -dir post-config, cover misc paths - mini.go: applyConfigFileOptions can overwrite -dir with a literal ~/data from mini.options. Re-resolve *miniDataFolders after the config-file apply, alongside the other path resolves, so the mini filer no longer ends up with a literal ~/data/filerldb2. - benchmark.go: resolve *b.idListFile (-list). - filer_sync.go: resolve *syncOptions.aSecurity / .bSecurity (-a.security / -b.security) before LoadClientTLSFromFile. - filer_cat.go: resolve *filerCat.output (-o) before os.OpenFile. - admin.go: drop trailing blank line at EOF (git diff --check). * address PR review: resolve -a.security/-b.security/-config before use Three follow-up fixes: - filer_sync.go: the -a.security / -b.security resolves were placed *after* LoadClientTLSFromFile / LoadHTTPClientFromFile were called, so weed filer.sync -a.security=~/a.toml still passed the literal tilde path. Hoist the resolves above the security-loading block so TLS clients see expanded paths. - filer_sync_verify.go: same flag pair was never resolved at all in the verify command; resolve at the top of runFilerSyncVerify. - filer_meta_backup.go: -config (the backup_filer.toml path) was passed directly to viper. Resolve at the top of runFilerMetaBackup. - mini.go: master.dir defaulted to the entire comma-joined miniDataFolders. With weed mini -dir=~/d1,~/d2 (or any multi-dir setup), TestFolderWritable then stat'd the joined string instead of a single directory. Default to the first entry via StringSplit to mirror the disk-space calculation a few lines below, and drop the now-redundant ResolvePath in TestFolderWritable. |
||
 Lars Lehtonen
|
29e14f89f1 |
fix(weed/command) address unhandled errors (#9208)
* fix(weed/command) address unhandled errors * fix(command): don't log graceful-shutdown sentinels; plug response-body leak - s3: Serve on unix socket treated http.ErrServerClosed as fatal; now excluded like the other Serve/ServeTLS paths in this file. - mq_agent, mq_broker: filter grpc.ErrServerStopped so clean shutdown doesn't log as an error. - worker_runtime: the added decodeErr early-continue skipped resp.Body.Close(); drop it since the existing check below already surfaces the decode error. - mount_std: the pre-mount Unmount commonly fails when nothing is mounted; demote to V(1) Infof. - fuse_std: tidy panic message to match sibling cases. * fix(mq_broker): filter grpc.ErrServerStopped on localhost listener The localhost listener goroutine logged any Serve error unconditionally, which includes grpc.ErrServerStopped on graceful shutdown. Match the main listener's check so clean stops don't surface as errors. --------- Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
Chris Lu
|
7f67995c24 |
chore(filer): remove -mount.p2p flag; registry is always on (#9183)
The filer-side mount peer registry (tier 1 of peer chunk sharing) was gated behind -mount.p2p (default true). Idle cost is negligible — a tiny in-memory map plus a 60s sweeper — so the opt-out is not worth the surface area. Removes the flag from weed filer, weed server (-filer.mount.p2p), and weed mini, and always constructs the registry in NewFilerServer. Also drops the now-dead nil guards in MountRegister/MountList/sweeper and the TestMountRegister_DisabledIsNoOp case. |
||
|
Chris Lu
|
9ae905e456 |
feat(security): hot-reload HTTPS certs without restart (k8s cert-manager) (#9181)
* feat(security): hot-reload HTTPS certs for master/volume/filer/webdav/admin S3 and filer already use a refreshing pemfile provider for their HTTPS cert, so rotated certificates (e.g. from k8s cert-manager) are picked up without a restart. Master, volume, webdav, and admin, however, passed cert/key paths straight to ServeTLS/ListenAndServeTLS and loaded once at startup — rotating those certs required a pod restart. Add a small helper NewReloadingServerCertificate in weed/security that wraps pemfile.Provider and returns a tls.Config.GetCertificate closure, then wire it into the four remaining HTTPS entry points. httpdown now also calls ServeTLS when TLSConfig carries a GetCertificate/Certificates but CertFile/KeyFile are empty, so volume server can pre-populate TLSConfig. A unit test exercises the rotation path (write cert, rotate on disk, assert the callback returns the new cert) with a short refresh window. * refactor(security): route filer/s3 HTTPS through the shared cert reloader Before: filer.go and s3.go each kept a *certprovider.Provider on the options struct plus a duplicated GetCertificateWithUpdate method. Both were loading pemfile themselves. Behaviorally they already reloaded, but the logic was duplicated two ways and neither path was shared with the newly-added master/volume/webdav/admin wiring. After: both use security.NewReloadingServerCertificate like the other servers. The per-struct certProvider field and GetCertificateWithUpdate method are removed, along with the now-unused certprovider and pemfile imports. Net: -32 lines, one code path for all HTTPS cert reloading. No behavior change — the refresh window, cache, and handshake contract are identical (the helper wraps the same pemfile.NewProvider). * feat(security): hot-reload HTTPS client certs for mount/backup/upload/etc The HTTP client in weed/util/http/client loaded the mTLS client cert once at startup via tls.LoadX509KeyPair. That left every long-lived HTTPS client process (weed mount, backup, filer.copy, filer→volume, s3→filer/volume) unable to pick up a rotated client cert without a restart — even though the same cert-manager setup was already rotating the server side fine. Swap the client cert loader for a tls.Config.GetClientCertificate callback backed by the same refreshing pemfile provider. New TLS handshakes pick up the rotated cert; in-flight pooled connections keep their old cert and drop as normal transport churn happens. To keep this reusable from both server and client TLS code without an import cycle (weed/security already imports weed/util/http/client for LoadHTTPClientFromFile), extract the pemfile wrapper into a new weed/security/certreload subpackage. weed/security keeps its thin NewReloadingServerCertificate wrapper. The existing unit test moves with the implementation. gRPC mTLS was already handled by security.LoadServerTLS / LoadClientTLS; this PR does not change any gRPC paths. MQ broker, MQ agent, Kafka gateway, and FUSE mount control plane are gRPC-only and therefore already rotate. CA bundles (ClientCAs / RootCAs / grpc.ca) are still loaded once — noted as a known limitation in the wiki. * fix(security): address PR review feedback on cert reloader Bots (gemini-code-assist + coderabbit) flagged three real issues and a couple of nits. Addressing them here: 1. KeyMaterial used context.Background(). The grpc pemfile provider's KeyMaterial blocks until material arrives or the context deadline expires; with Background() a slow disk could hang the TLS handshake indefinitely. Switched both the server and client callbacks to use hello.Context() / cri.Context() so a stuck read is bounded by the handshake timeout. 2. Admin server loaded TLS inside the serve goroutine. If the cert was bad, the goroutine returned but startAdminServer kept blocking on <-ctx.Done() with no listener, making the process look healthy with nothing bound. Moved TLS setup to run before the goroutine starts and propagate errors via fmt.Errorf; also captures the provider and defers Close(). 3. HTTP client discarded the certprovider.Provider from NewClientGetCertificate. That leaked the refresh goroutine, and NewHttpClientWithTLS had a worse case where a CA-file failure after provider creation orphaned the provider entirely. Added a certProvider field and a Close() method on HTTPClient, and made the constructors close the provider on subsequent error paths. 4. Server-side paths (master/volume/filer/s3/webdav/admin) now retain the provider. filer and webdav run ServeTLS synchronously, so a plain defer works. master/volume/s3 dispatch goroutines and return while the server keeps running, so they hook Close() into grace.OnInterrupt. 5. Test: certreload_test now tolerates transient read/parse errors during file rotation (writeSelfSigned rewrites cert before key) and reports the last error only if the deadline expires. No user-visible behavior change for the happy path. * test(tls): add end-to-end HTTPS cert rotation integration test Boots a real `weed master` with HTTPS enabled, captures the leaf cert served at TLS handshake time, atomically rewrites the cert/key files on disk (the same rename-in-place pattern kubelet does when it swaps a cert-manager Secret), and asserts that a subsequent TLS handshake observes the rotated leaf — with no process restart, no SIGHUP, no reloader sidecar. Verifies the full path: on-disk change → pemfile refresh tick → provider.KeyMaterial → tls.Config.GetCertificate → server TLS handshake. Runtime is ~1s by exposing the reloader's refresh window as an env var (WEED_TLS_CERT_REFRESH_INTERVAL) and setting it to 500ms for the test. The same env var is user-facing — documented in the wiki — so operators running short-lived certs (Vault, cert-manager with duration: 24h, etc.) can tighten the rotation-pickup window without a rebuild. Defaults to 5h to preserve prior behavior. security.CredRefreshingInterval is kept for API compatibility but now aliases certreload.DefaultRefreshInterval so the same env controls both gRPC mTLS and HTTPS reload. * ci(tls): wire the TLS rotation integration test into GitHub Actions Mirrors the existing vacuum-integration-tests.yml shape: Ubuntu runner, Go 1.25, build weed, run `go test` in test/tls_rotation, upload master logs on failure. 10-minute job timeout; the test itself finishes in about a second because WEED_TLS_CERT_REFRESH_INTERVAL is set to 500ms inside the test. Runs on every push to master and on every PR to master. * fix(tls): address follow-up PR review comments Three new comments on the integration test + volume shutdown path: 1. Test: peekServerCert was swallowing every dial/handshake error, which meant waitForCert's "last err: <nil>" fatal message lost all diagnostic value. Thread errors back through: peekServerCert now returns (*x509.Certificate, error), and waitForCert records the latest error so a CI flake points at the actual cause (master didn't come up, handshake rejected, CA pool mismatch, etc.). 2. Test: set HOME=<tempdir> on the master subprocess. Viper today registers the literal path "$HOME/.seaweedfs" without env expansion, so a developer's ~/.seaweedfs/security.toml is accidentally invisible — the test was relying on that. Pinning HOME is belt-and-braces against a future viper upgrade that does expand env vars. 3. volume.go: startClusterHttpService's provider close was registered via grace.OnInterrupt, which fires on SIGTERM but NOT on the v.shutdownCtx.Done() path used by mini / integration tests. The pemfile refresh goroutine leaked in that shutdown path. Now the helper returns a close func and the caller invokes it on BOTH shutdown paths for parity. Also add MinVersion: TLS 1.2 to the test's tls.Config to quiet the ast-grep static-analysis nit — zero-risk since the pool only trusts our in-memory CA. Test runs clean 3/3. |
||
|
Chris Lu
|
e24a443b17 |
peer chunk sharing 2/8: filer mount registry (#9131)
* proto: define MountRegister/MountList and MountPeer service Adds the wire types for peer chunk sharing between weed mount clients: * filer.proto: MountRegister / MountList RPCs so each mount can heartbeat its peer-serve address into a filer-hosted registry, and refresh the list of peers. Tiny payload; the filer stores only O(fleet_size) state. * mount_peer.proto (new): ChunkAnnounce / ChunkLookup RPCs for the mount-to-mount chunk directory. Each fid's directory entry lives on an HRW-assigned mount; announces and lookups route to that mount. No behavior yet — later PRs wire the RPCs into the filer and mount. See design-weed-mount-peer-chunk-sharing.md for the full design. * filer: add mount-server registry behind -peer.registry.enable Implements tier 1 of the peer chunk sharing design: an in-memory registry of live weed mount servers, keyed by peer address, refreshed by MountRegister heartbeats and served by MountList. * weed/filer/peer_registry.go: thread-safe map with TTL eviction; lazy sweep on List plus a background sweeper goroutine for bounded memory. * weed/server/filer_grpc_server_peer.go: MountRegister / MountList RPC handlers. When -peer.registry.enable is false (the default), both RPCs are silent no-ops so probing older filers is harmless. * -peer.registry.enable flag on weed filer; FilerOption.PeerRegistryEnabled wires it through. Phase 1 is single-filer (no cross-filer replication of the registry); mounts that fail over to another filer will re-register on the next heartbeat, so the registry self-heals within one TTL cycle. Part of the peer-chunk-sharing design; no behavior change at runtime until a later PR enables the flag on both filer and mount. * filer: nil-safe peerRegistryEnable + registry hardening Addresses review feedback on PR #9131. * Fix: nil pointer deref in the mini cluster. FilerOptions instances constructed outside weed/command/filer.go (e.g. miniFilerOptions in mini.go) do not populate peerRegistryEnable, so dereferencing the pointer panics at Filer startup. Use the same `nil && deref` idiom already used for distributedLock / writebackCache. * Hardening (gemini review): registry now enforces three invariants: - empty peer_addr is silently rejected (no client-controlled sentinel mass-inserts) - TTL is capped at 1 hour so a runaway client cannot pin entries - new-entry count is capped at 10000 to bound memory; renewals of existing entries are always honored, so a full registry still heartbeats its existing members correctly Covered by new unit tests. * filer: rename -peer.registry.enable flag to -mount.p2p Per review feedback: the old name "peer.registry.enable" leaked the implementation ("registry") into the CLI surface. "mount.p2p" is shorter and describes what it actually controls — whether this filer participates in mount-to-mount peer chunk sharing. Flag renames (all three keep default=true, idle cost is near-zero): -peer.registry.enable -> -mount.p2p (weed filer) -filer.peer.registry.enable -> -filer.mount.p2p (weed mini, weed server) Internal variable names (mountPeerRegistryEnable, MountPeerRegistry) keep their longer form — they describe the component, not the knob. * filer: MountList returns DataCenter + List uses RLock Two review follow-ups on the mount peer registry: * weed/server/filer_grpc_server_mount_peer.go: MountList was dropping the DataCenter on the wire. The whole point of carrying DC separately from Rack is letting the mount-side fetcher re-rank peers by the two-level locality hierarchy (same-rack > same-DC > cross-DC); without DC in the response every remote peer collapsed to "unknown locality." * weed/filer/mount_peer_registry.go: List() was taking a write lock so it could lazy-delete expired entries inline. But MountList is a read-heavy RPC hit on every mount's 30 s refresh loop, and Sweep is already wired as the sole reclamation path (same pattern as the mount-side PeerDirectory). Switch List to RLock + filter, let Sweep do the map mutation, so concurrent MountList callers don't serialize on each other. Test updated to reflect the new contract (List no longer mutates the map; Sweep is what drops expired entries). |
||
|
Chris Lu
|
9d15705c16 |
fix(mini): shut down admin/s3/webdav/filer before volume/master on Ctrl+C (#9112)
* fix(mini): shut down admin/s3/webdav/filer before volume/master on Ctrl+C Interrupts fired grace hooks in registration order, so master (started first) shut down before its clients, producing heartbeat-canceled errors and masterClient reconnection noise during weed mini shutdown. Admin/s3/ webdav had no interrupt hooks at all and were killed at os.Exit. - grace: execute interrupt hooks in LIFO (defer-style) order so later- started services tear down first. - filer: consolidate the three separate interrupt hooks (gRPC / HTTP / DB) into one that runs in order, so filer shutdown stays correct independent of FIFO/LIFO semantics. - mini: add MiniClientsShutdownCtx (separate from test-facing MiniClusterCtx) plus an OnMiniClientsShutdown helper. Admin, S3, WebDAV and the maintenance worker observe it; runMini registers a cancel hook after startup so under LIFO it fires first and waits up to 10s on a WaitGroup for those services to drain before filer, volume, and master shut down. Resulting order on Ctrl+C: admin/s3/webdav/worker -> filer (gRPC -> HTTP -> DB) -> volume -> master. * refactor(mini): group mini-client shutdown into one state struct The first pass spread the shutdown plumbing across three globals (MiniClientsShutdownCtx, miniClientsWg, cancelMiniClients) and two ctx-derivation sites (OnMiniClientsShutdown and startMiniAdminWithWorker). Group into a private miniClientsState (ctx/cancel/wg) rebuilt per runMini invocation, and chain its ctx from MiniClusterCtx so clients only observe one signal. Tests that cancel MiniClusterCtx still trigger client shutdown via parent-child propagation. - resetMiniClients() installs fresh state at the top of runMini, so in-process test reruns don't inherit stale ctx/wg. - onMiniClientsShutdown(fn) replaces the exported OnMiniClientsShutdown and only observes one ctx. - trackMiniClient() replaces the manual wg.Add/Done dance for the admin goroutine. - miniClientsCtx() gives the admin startup a ctx without re-deriving. - triggerMiniClientsShutdown(timeout) is the interrupt hook body. No behaviour change; existing tests pass. * refactor: generalize shutdown ctx as an option, not a mini-specific helper Several service files (s3, webdav, filer, master, volume) observed the mini-specific MiniClusterCtx or called onMiniClientsShutdown directly. That leaked mini orchestration into code that also runs under weed s3, weed webdav, weed filer, weed master, and weed volume standalone. Replace with a generic `shutdownCtx context.Context` field on each service's Options struct. When non-nil, the server watches it and shuts down gracefully; when nil (standalone), the shutdown path is a no-op. Mini wires the contexts up from a single place (runMini): - miniMasterOptions/miniOptions.v/miniFilerOptions.shutdownCtx = MiniClusterCtx (drives test-triggered teardown) - miniS3Options/miniWebDavOptions.shutdownCtx = miniClientsCtx() (drives Ctrl+C teardown before filer/volume/master) All knowledge of MiniClusterCtx now lives in mini.go. * fix(mini): stop worker before clients ctx so admin shutdown isn't blocked Symptom on Ctrl+C of a clean weed mini: mini's Shutting down admin/s3/ webdav hook sat for 10s then logged "timed out". Admin had started its shutdown but was blocked inside StopWorkerGrpcServer's GracefulStop, waiting for the still-connected worker stream. That in turn left filer clients connected and cascaded into filer's own 10s gRPC graceful-stop timeout. Two causes, both fixed: 1. worker.Stop() deadlocked on clean shutdown. It sent ActionStop (which makes managerLoop `break out` and exit), then called getTaskLoad() which sends to the same unbuffered cmd channel — no receiver, hangs forever. Reorder Stop() to snapshot the admin client and drain tasks BEFORE sending ActionStop, and call Disconnect() via the local snapshot afterwards. 2. Worker's taskRequestLoop raced with Disconnect(): RequestTask reads from c.incoming, which Disconnect closes, yielding a nil response and a panic on response.Message. Handle the closed channel explicitly. 3. Mini now has a preCancel phase (beforeMiniClientsShutdown) that runs synchronously BEFORE the clients ctx is cancelled. Register worker shutdown there so admin's worker-gRPC GracefulStop finds the worker already disconnected and returns immediately, instead of waiting on a stream that is about to close anyway. Observed shutdown of a clean mini: admin/s3/webdav down in <10ms; full process exit in ~11s (the remaining 10s is a pre-existing filer gRPC graceful-stop timeout, not cascaded from the clients tier). * feat(mini): cap filer gRPC graceful stop at 1s under weed mini Full weed mini shutdown was ~11s on a clean exit, dominated by the filer's default 10s gRPC GracefulStop timeout while background SubscribeLocalMetadata streams drained. Expose the timeout as a FilerOptions.gracefulStopTimeout field (default 10s for standalone weed filer) and set it to 1s in mini. Clean weed mini shutdown now takes ~2s. |
||
|
Chris Lu
|
eaf561e86c |
perf(s3): add optional shared in-memory chunk cache for GET (#9069)
Adds the -s3.cacheCapacityMB flag (default 0, disabled) that attaches
an in-memory chunk_cache.ChunkCacheInMemory to the server-wide
ReaderCache introduced in the previous commit. When enabled,
completed chunks are deposited into the shared cache as they are
downloaded, so concurrent and repeat GETs of the same object hit
memory instead of re-fetching chunks from volume servers.
When 0 (the default) the shared ReaderCache still runs — it just
attaches a nil chunk cache, so behaviour matches the previous commit
exactly. No behaviour change for clusters that don't opt in.
Disk-backed TieredChunkCache was evaluated and rejected: its
synchronous SetChunk writes regressed cold reads ~12x on loopback
because the chunk fetchers block on local disk I/O that is *slower*
than the TCP volume-server fetch it is supposed to accelerate.
Memory-only avoids that.
Flag registered in all four S3 flag sites (s3.go, server.go,
filer.go, mini.go) per the comment on command.S3Options. The chunk
size used to convert CacheSizeMB → entry count is encapsulated in
the s3ChunkCacheChunkSizeMB constant so it's easy to grep and
revisit if the filer default chunk size changes.
Measured on weed mini + 1 GiB random object over loopback, single
curl on a presigned URL:
cacheCapacityMB=0 (off): cold ~2900, warm ~2900 MB/s
cacheCapacityMB=4096: cold ~2790, warm ~5050 MB/s (+70%)
|
||
 os-pradipbabar
|
9cae95d749 |
fix(filer): prevent data corruption during graceful shutdown (#9037)
* fix: wait for in-flight uploads to complete before filer shutdown Prevents data corruption when SIGTERM is received during active uploads. The filer now waits for all in-flight operations to complete before calling the underlying shutdown logic. This affects all deployment types (Kubernetes, Docker, systemd) and fixes corruption issues during rolling updates, certificate rotation, and manual restarts. Changes: - Add FilerServer.Shutdown() method with upload wait logic - Update grace.OnInterrupt hook to use new shutdown method Fixes data corruption reported by production users during pod restarts. * fix: implement graceful shutdown for gRPC and HTTP servers, ensuring in-flight uploads complete * fix: address review comments on graceful shutdown - Add 10s timeout to gRPC GracefulStop to prevent indefinite blocking from long-lived streams (falls back to Stop on timeout) - Reduce HTTP/HTTPS shutdown timeout from 25s to 15s to fit within Kubernetes default 30s termination grace period - Move fs.Shutdown() (database close) after Serve() returns instead of a separate hook to eliminate race where main goroutine exits before the shutdown hook runs * fix: shut down all HTTP servers before filer database close Address remaining review comments: - Shut down auxiliary HTTP servers (Unix socket, local listener) during graceful shutdown so they can't serve write traffic after the main server stops - Register fs.Shutdown() as a grace.OnInterrupt hook to guarantee it completes before os.Exit(0), fixing the race between the grace goroutine and the main goroutine - Use sync.Once to ensure fs.Shutdown() runs exactly once regardless of whether shutdown is signal-driven or context-driven (MiniCluster) --------- Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
Chris Lu
|
d1823d3784 |
fix(s3): include static identities in listing operations (#8903)
* fix(s3): include static identities in listing operations Static identities loaded from -s3.config file were only stored in the S3 API server's in-memory state. Listing operations (s3.configure shell command, aws iam list-users) queried the credential manager which only returned dynamic identities from the backend store. Register static identities with the credential manager after loading so they are included in LoadConfiguration and ListUsers results, and filtered out before SaveConfiguration to avoid persisting them to the dynamic store. Fixes https://github.com/seaweedfs/seaweedfs/discussions/8896 * fix: avoid mutating caller's config and defensive copies - SaveConfiguration: use shallow struct copy instead of mutating the caller's config.Identities field - SetStaticIdentities: skip nil entries to avoid panics - GetStaticIdentities: defensively copy PolicyNames slice to avoid aliasing the original * fix: filter nil static identities and sync on config reload - SetStaticIdentities: filter nil entries from the stored slice (not just from staticNames) to prevent panics in LoadConfiguration/ListUsers - Extract updateCredentialManagerStaticIdentities helper and call it from both startup and the grace.OnReload handler so the credential manager's static snapshot stays current after config file reloads * fix: add mutex for static identity fields and fix ListUsers for store callers - Add sync.RWMutex to protect staticIdentities/staticNames against concurrent reads during config reload - Revert CredentialManager.ListUsers to return only store users, since internal callers (e.g. DeletePolicy) look up each user in the store and fail on non-existent static entries - Merge static usernames in the filer gRPC ListUsers handler instead, via the new GetStaticUsernames method - Fix CI: TestIAMPolicyManagement/managed_policy_crud_lifecycle was failing because DeletePolicy iterated static users that don't exist in the store * fix: show static identities in admin UI and weed shell The admin UI and weed shell s3.configure command query the filer's credential manager via gRPC, which is a separate instance from the S3 server's credential manager. Static identities were only registered on the S3 server's credential manager, so they never appeared in the filer's responses. - Add CredentialManager.LoadS3ConfigFile to parse a static S3 config file and register its identities - Add FilerOptions.s3ConfigFile so the filer can load the same static config that the S3 server uses - Wire s3ConfigFile through in weed mini and weed server modes - Merge static usernames in filer gRPC ListUsers handler - Add CredentialManager.GetStaticUsernames helper - Add sync.RWMutex to protect concurrent access to static identity fields - Avoid importing weed/filer from weed/credential (which pulled in filer store init() registrations and broke test isolation) - Add docker/compose/s3_static_users_example.json * fix(admin): make static users read-only in admin UI Static users loaded from the -s3.config file should not be editable or deletable through the admin UI since they are managed via the config file. - Add IsStatic field to ObjectStoreUser, set from credential manager - Hide edit, delete, and access key buttons for static users in the users table template - Show a "static" badge next to static user names - Return 403 Forbidden from UpdateUser and DeleteUser API handlers when the target user is a static identity * fix(admin): show details for static users GetObjectStoreUserDetails called credentialManager.GetUser which only queries the dynamic store. For static users this returned ErrUserNotFound. Fall back to GetStaticIdentity when the store lookup fails. * fix(admin): load static S3 identities in admin server The admin server has its own credential manager (gRPC store) which is a separate instance from the S3 server's and filer's. It had no static identity data, so IsStaticIdentity returned false (edit/delete buttons shown) and GetStaticIdentity returned nil (details page failed). Pass the -s3.config file path through to the admin server and call LoadS3ConfigFile on its credential manager, matching the approach used for the filer. * fix: use protobuf is_static field instead of passing config file path The previous approach passed -s3.config file path to every component (filer, admin). This is wrong because the admin server should not need to know about S3 config files. Instead, add an is_static field to the Identity protobuf message. The field is set when static identities are serialized (in GetStaticIdentities and LoadS3ConfigFile). Any gRPC client that loads configuration via GetConfiguration automatically sees which identities are static, without needing the config file. - Add is_static field (tag 8) to iam_pb.Identity proto message - Set IsStatic=true in GetStaticIdentities and LoadS3ConfigFile - Admin GetObjectStoreUsers reads identity.IsStatic from proto - Admin IsStaticUser helper loads config via gRPC to check the flag - Filer GetUser gRPC handler falls back to GetStaticIdentity - Remove s3ConfigFile from AdminOptions and NewAdminServer signature |
||
|
Chris Lu
|
efbed39e25 |
S3: map canned ACL to file permissions and add configurable default file mode (#8886)
* S3: map canned ACL to file permissions and add configurable default file mode S3 uploads were hardcoded to 0660 regardless of ACL headers. Now the X-Amz-Acl header maps to Unix file permissions per-object: - public-read, authenticated-read, bucket-owner-read → 0644 - public-read-write → 0666 - private, bucket-owner-full-control → 0660 Also adds -defaultFileMode / -s3.defaultFileMode flag to set a server-wide default when no ACL header is present. Closes #8874 * Address review feedback for S3 file mode feature - Extract hardcoded 0660 to defaultFileMode constant - Change parseDefaultFileMode to return error instead of calling Fatalf - Add -s3.defaultFileMode flag to filer.go and mini.go (was missing) - Add doc comment to S3Options about updating all four flag sites - Add TestResolveFileMode with 10 test cases covering ACL mapping, server default, and priority ordering |
||
|
Chris Lu
|
2eaf98a7a2 |
Use Unix sockets for gRPC in mini mode (#8856)
* Use Unix sockets for gRPC between co-located services in mini mode In `weed mini`, all services run in one process. Previously, inter-service gRPC traffic (volume↔master, filer↔master, S3↔filer, worker↔admin, etc.) went through TCP loopback. This adds a gRPC Unix socket registry in the pb package: mini mode registers a socket path per gRPC port at startup, each gRPC server additionally listens on its socket, and GrpcDial transparently routes to the socket via WithContextDialer when a match is found. Standalone commands (weed master, weed filer, etc.) are unaffected since no sockets are registered. TCP listeners are kept for external clients. * Handle Serve error and clean up socket file in ServeGrpcOnLocalSocket Log non-expected errors from grpcServer.Serve (ignoring grpc.ErrServerStopped) and always remove the Unix socket file when Serve returns, ensuring cleanup on Stop/GracefulStop. |
||
 blitt001
|
3d81d5bef7 |
Fix S3 signature verification behind reverse proxies (#8444)
* Fix S3 signature verification behind reverse proxies When SeaweedFS is deployed behind a reverse proxy (e.g. nginx, Kong, Traefik), AWS S3 Signature V4 verification fails because the Host header the client signed with (e.g. "localhost:9000") differs from the Host header SeaweedFS receives on the backend (e.g. "seaweedfs:8333"). This commit adds a new -s3.externalUrl parameter (and S3_EXTERNAL_URL environment variable) that tells SeaweedFS what public-facing URL clients use to connect. When set, SeaweedFS uses this host value for signature verification instead of the Host header from the incoming request. New parameter: -s3.externalUrl (flag) or S3_EXTERNAL_URL (environment variable) Example: -s3.externalUrl=http://localhost:9000 Example: S3_EXTERNAL_URL=https://s3.example.com The environment variable is particularly useful in Docker/Kubernetes deployments where the external URL is injected via container config. The flag takes precedence over the environment variable when both are set. At startup, the URL is parsed and default ports are stripped to match AWS SDK behavior (port 80 for HTTP, port 443 for HTTPS), so "http://s3.example.com:80" and "http://s3.example.com" are equivalent. Bugs fixed: - Default port stripping was removed by a prior PR, causing signature mismatches when clients connect on standard ports (80/443) - X-Forwarded-Port was ignored when X-Forwarded-Host was not present - Scheme detection now uses proper precedence: X-Forwarded-Proto > TLS connection > URL scheme > "http" - Test expectations for standard port stripping were incorrect - expectedHost field in TestSignatureV4WithForwardedPort was declared but never actually checked (self-referential test) * Add Docker integration test for S3 proxy signature verification Docker Compose setup with nginx reverse proxy to validate that the -s3.externalUrl parameter (or S3_EXTERNAL_URL env var) correctly resolves S3 signature verification when SeaweedFS runs behind a proxy. The test uses nginx proxying port 9000 to SeaweedFS on port 8333, with X-Forwarded-Host/Port/Proto headers set. SeaweedFS is configured with -s3.externalUrl=http://localhost:9000 so it uses "localhost:9000" for signature verification, matching what the AWS CLI signs with. The test can be run with aws CLI on the host or without it by using the amazon/aws-cli Docker image with --network host. Test covers: create-bucket, list-buckets, put-object, head-object, list-objects-v2, get-object, content round-trip integrity, delete-object, and delete-bucket — all through the reverse proxy. * Create s3-proxy-signature-tests.yml * fix CLI * fix CI * Update s3-proxy-signature-tests.yml * address comments * Update Dockerfile * add user * no need for fuse * Update s3-proxy-signature-tests.yml * debug * weed mini * fix health check * health check * fix health checking --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
Chris Lu
|
bd4e7ff14e |
command: fix s3 panic in filer command (#8208)
command: fix s3 panic in filer command due to uninitialized options This fixes a nil pointer dereference panic when starting the S3 server via the `weed filer` command by correctly initializing `iamReadOnly` and `portIceberg` flags. Relates to #8200 |
||
 Chris Lu
|
c106532b79 |
fix: prevent MiniClusterCtx race conditions in command shutdown
Capture global MiniClusterCtx into local variables before goroutine/select evaluation to prevent nil dereference/data race when context is reset to nil after nil check. Applied to filer, master, volume, and s3 commands. |
||
|
Chris Lu
|
580c2b4ad4 |
command: fix stale error variable logging in filer serving goroutines
- Use local 'err' variable instead of stale 'e' from outer scope - Applied to both TLS and non-TLS paths for local listener |
||
|
Chris Lu
|
01c17478ae |
command: implement graceful shutdown for mini cluster
- Introduce MiniClusterCtx to coordinate shutdown across mini services - Update Master, Volume, Filer, S3, and WebDAV servers to respect context cancellation - Ensure all resources are cleaned up properly during test teardown - Integrate MiniClusterCtx in s3tables integration tests |
||
|
Chris Lu
|
6bf088cec9 |
IAM Policy Management via gRPC (#8109)
* Add IAM gRPC service definition - Add GetConfiguration/PutConfiguration for config management - Add CreateUser/GetUser/UpdateUser/DeleteUser/ListUsers for user management - Add CreateAccessKey/DeleteAccessKey/GetUserByAccessKey for access key management - Methods mirror existing IAM HTTP API functionality * Add IAM gRPC handlers on filer server - Implement IamGrpcServer with CredentialManager integration - Handle configuration get/put operations - Handle user CRUD operations - Handle access key create/delete operations - All methods delegate to CredentialManager for actual storage * Wire IAM gRPC service to filer server - Add CredentialManager field to FilerOption and FilerServer - Import credential store implementations in filer command - Initialize CredentialManager from credential.toml if available - Register IAM gRPC service on filer gRPC server - Enable credential management via gRPC alongside existing filer services * Regenerate IAM protobuf with gRPC service methods * iam_pb: add Policy Management to protobuf definitions * credential: implement PolicyManager in credential stores * filer: implement IAM Policy Management RPCs * shell: add s3.policy command * test: add integration test for s3.policy * test: fix compilation errors in policy_test * pb * fmt * test * weed shell: add -policies flag to s3.configure This allows linking/unlinking IAM policies to/from identities directly from the s3.configure command. * test: verify s3.configure policy linking and fix port allocation - Added test case for linking policies to users via s3.configure - Implemented findAvailablePortPair to ensure HTTP and gRPC ports are both available, avoiding conflicts with randomized port assignments. - Updated assertion to match jsonpb output (policyNames) * credential: add StoreTypeGrpc constant * credential: add IAM gRPC store boilerplate * credential: implement identity methods in gRPC store * credential: implement policy methods in gRPC store * admin: use gRPC credential store for AdminServer This ensures that all IAM and policy changes made through the Admin UI are persisted via the Filer's IAM gRPC service instead of direct file manipulation. * shell: s3.configure use granular IAM gRPC APIs instead of full config patching * shell: s3.configure use granular IAM gRPC APIs * shell: replace deprecated ioutil with os in s3.policy * filer: use gRPC FailedPrecondition for unconfigured credential manager * test: improve s3.policy integration tests and fix error checks * ci: add s3 policy shell integration tests to github workflow * filer: fix LoadCredentialConfiguration error handling * credential/grpc: propagate unmarshal errors in GetPolicies * filer/grpc: improve error handling and validation * shell: use gRPC status codes in s3.configure * credential: document PutPolicy as create-or-replace * credential/postgres: reuse CreatePolicy in PutPolicy to deduplicate logic * shell: add timeout context and strictly enforce flags in s3.policy * iam: standardize policy content field naming in gRPC and proto * shell: extract slice helper functions in s3.configure * filer: map credential store errors to gRPC status codes * filer: add input validation for UpdateUser and CreateAccessKey * iam: improve validation in policy and config handlers * filer: ensure IAM service registration by defaulting credential manager * credential: add GetStoreName method to manager * test: verify policy deletion in integration test |
||
 Taylor Jasko
|
6a9860098f |
fix: correcting S3 nil cipher dereference in filer init (#7952)
Resolves the following error reported in #7949: ``` I0103 21:38:30.230662 s3.go:275 Starting S3 API Server with standard IAM panic: runtime error: invalid memory address or nil pointer dereference [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x38ca961] goroutine 102 [running]: github.com/seaweedfs/seaweedfs/weed/command.(*S3Options).startS3Server(0x7caf840) /go/src/github.com/seaweedfs/seaweedfs/weed/command/s3.go:295 +0x741 github.com/seaweedfs/seaweedfs/weed/command.runFiler.func1(...) /go/src/github.com/seaweedfs/seaweedfs/weed/command/filer.go:244 created by github.com/seaweedfs/seaweedfs/weed/command.runFiler in goroutine 1 /go/src/github.com/seaweedfs/seaweedfs/weed/command/filer.go:242 +0x353 ``` |
||
|
Chris Lu
|
1b1e5f69a2 |
Add TUS protocol support for resumable uploads (#7592)
* Add TUS protocol integration tests
This commit adds integration tests for the TUS (resumable upload) protocol
in preparation for implementing TUS support in the filer.
Test coverage includes:
- OPTIONS handler for capability discovery
- Basic single-request upload
- Chunked/resumable uploads
- HEAD requests for offset tracking
- DELETE for upload cancellation
- Error handling (invalid offsets, missing uploads)
- Creation-with-upload extension
- Resume after interruption simulation
Tests are skipped in short mode and require a running SeaweedFS cluster.
* Add TUS session storage types and utilities
Implements TUS upload session management:
- TusSession struct for tracking upload state
- Session creation with directory-based storage
- Session persistence using filer entries
- Session retrieval and offset updates
- Session deletion with chunk cleanup
- Upload completion with chunk assembly into final file
Session data is stored in /.uploads.tus/{upload-id}/ directory,
following the pattern used by S3 multipart uploads.
* Add TUS HTTP handlers
Implements TUS protocol HTTP handlers:
- tusHandler: Main entry point routing requests
- tusOptionsHandler: Capability discovery (OPTIONS)
- tusCreateHandler: Create new upload (POST)
- tusHeadHandler: Get upload offset (HEAD)
- tusPatchHandler: Upload data at offset (PATCH)
- tusDeleteHandler: Cancel upload (DELETE)
- tusWriteData: Upload data to volume servers
Features:
- Supports creation-with-upload extension
- Validates TUS protocol headers
- Offset conflict detection
- Automatic upload completion when size is reached
- Metadata parsing from Upload-Metadata header
* Wire up TUS protocol routes in filer server
Add TUS handler route (/.tus/) to the filer HTTP server.
The TUS route is registered before the catch-all route to ensure
proper routing of TUS protocol requests.
TUS protocol is now accessible at:
- OPTIONS /.tus/ - Capability discovery
- POST /.tus/{path} - Create upload
- HEAD /.tus/.uploads/{id} - Get offset
- PATCH /.tus/.uploads/{id} - Upload data
- DELETE /.tus/.uploads/{id} - Cancel upload
* Improve TUS integration test setup
Add comprehensive Makefile for TUS tests with targets:
- test-with-server: Run tests with automatic server management
- test-basic/chunked/resume/errors: Specific test categories
- manual-start/stop: For development testing
- debug-logs/status: For debugging
- ci-test: For CI/CD pipelines
Update README.md with:
- Detailed TUS protocol documentation
- All endpoint descriptions with headers
- Usage examples with curl commands
- Architecture diagram
- Comparison with S3 multipart uploads
Follows the pattern established by other tests in test/ folder.
* Fix TUS integration tests and creation-with-upload
- Fix test URLs to use full URLs instead of relative paths
- Fix creation-with-upload to refresh session before completing
- Fix Makefile to properly handle test cleanup
- Add FullURL helper function to TestCluster
* Add TUS protocol tests to GitHub Actions CI
- Add tus-tests.yml workflow that runs on PRs and pushes
- Runs when TUS-related files are modified
- Automatic server management for integration testing
- Upload logs on failure for debugging
* Make TUS base path configurable via CLI
- Add -tus.path CLI flag to filer command
- TUS is disabled by default (empty path)
- Example: -tus.path=/.tus to enable at /.tus endpoint
- Update test Makefile to use -tus.path flag
- Update README with TUS enabling instructions
* Rename -tus.path to -tusBasePath with default .tus
- Rename CLI flag from -tus.path to -tusBasePath
- Default to .tus (TUS enabled by default)
- Add -filer.tusBasePath option to weed server command
- Properly handle path prefix (prepend / if missing)
* Address code review comments
- Sort chunks by offset before assembling final file
- Use chunk.Offset directly instead of recalculating
- Return error on invalid file ID instead of skipping
- Require Content-Length header for PATCH requests
- Use fs.option.Cipher for encryption setting
- Detect MIME type from data using http.DetectContentType
- Fix concurrency group for push events in workflow
- Use os.Interrupt instead of Kill for graceful shutdown in tests
* fmt
* Address remaining code review comments
- Fix potential open redirect vulnerability by sanitizing uploadLocation path
- Add language specifier to README code block
- Handle os.Create errors in test setup
- Use waitForHTTPServer instead of time.Sleep for master/volume readiness
- Improve test reliability and debugging
* Address critical and high-priority review comments
- Add per-session locking to prevent race conditions in updateTusSessionOffset
- Stream data directly to volume server instead of buffering entire chunk
- Only buffer 512 bytes for MIME type detection, then stream remaining data
- Clean up session locks when session is deleted
* Fix race condition to work across multiple filer instances
- Store each chunk as a separate file entry instead of updating session JSON
- Chunk file names encode offset, size, and fileId for atomic storage
- getTusSession loads chunks from directory listing (atomic read)
- Eliminates read-modify-write race condition across multiple filers
- Remove in-memory mutex that only worked for single filer instance
* Address code review comments: fix variable shadowing, sniff size, and test stability
- Rename path variable to reqPath to avoid shadowing path package
- Make sniff buffer size respect contentLength (read at most contentLength bytes)
- Handle Content-Length < 0 in creation-with-upload (return error for chunked encoding)
- Fix test cluster: use temp directory for filer store, add startup delay
* Fix test stability: increase cluster stabilization delay to 5 seconds
The tests were intermittently failing because the volume server needed more
time to create volumes and register with the master. Increasing the delay
from 2 to 5 seconds fixes the flaky test behavior.
* Address PR review comments for TUS protocol support
- Fix strconv.Atoi error handling in test file (lines 386, 747)
- Fix lossy fileId encoding: use base64 instead of underscore replacement
- Add pagination support for ListDirectoryEntries in getTusSession
- Batch delete chunks instead of one-by-one in deleteTusSession
* Address additional PR review comments for TUS protocol
- Fix UploadAt timestamp: use entry.Crtime instead of time.Now()
- Remove redundant JSON content in chunk entry (metadata in filename)
- Refactor tusWriteData to stream in 4MB chunks to avoid OOM on large uploads
- Pass filer.Entry to parseTusChunkPath to preserve actual upload time
* Address more PR review comments for TUS protocol
- Normalize TUS path once in filer_server.go, store in option.TusPath
- Remove redundant path normalization from TUS handlers
- Remove goto statement in tusCreateHandler, simplify control flow
* Remove unnecessary mutexes in tusWriteData
The upload loop is sequential, so uploadErrLock and chunksLock are not needed.
* Rename updateTusSessionOffset to saveTusChunk
Remove unused newOffset parameter and rename function to better reflect its purpose.
* Improve TUS upload performance and add path validation
- Reuse operation.Uploader across sub-chunks for better connection reuse
- Guard against TusPath='/' to prevent hijacking all filer routes
* Address PR review comments for TUS protocol
- Fix critical chunk filename parsing: use strings.Cut instead of SplitN
to correctly handle base64-encoded fileIds that may contain underscores
- Rename tusPath to tusBasePath for naming consistency across codebase
- Add background garbage collection for expired TUS sessions (runs hourly)
- Improve error messages with %w wrapping for better debuggability
* Address additional TUS PR review comments
- Fix tusBasePath default to use leading slash (/.tus) for consistency
- Add chunk contiguity validation in completeTusUpload to detect gaps/overlaps
- Fix offset calculation to find maximum contiguous range from 0, not just last chunk
- Return 413 Request Entity Too Large instead of silently truncating content
- Document tusChunkSize rationale (4MB balances memory vs request overhead)
- Fix Makefile xargs portability by removing GNU-specific -r flag
- Add explicit -tusBasePath flag to integration test for robustness
- Fix README example to use /.uploads/tus path format
* Revert log_buffer changes (moved to separate PR)
* Minor style fixes from PR review
- Simplify tusBasePath flag description to use example format
- Add 'TUS upload' prefix to session not found error message
- Remove duplicate tusChunkSize comment
- Capitalize warning message for consistency
- Add grep filter to Makefile xargs for better empty input handling
|
||
|
Chris Lu
|
f41925b60b |
Embed IAM API into S3 server (#7740)
* Embed IAM API into S3 server This change simplifies the S3 and IAM deployment by embedding the IAM API directly into the S3 server, following the patterns used by MinIO and Ceph RGW. Changes: - Add -iam flag to S3 server (enabled by default) - Create embedded IAM API handler in s3api package - Register IAM routes (POST to /) in S3 server when enabled - Deprecate standalone 'weed iam' command with warning Benefits: - Single binary, single port for both S3 and IAM APIs - Simpler deployment and configuration - Shared credential manager between S3 and IAM - Backward compatible: 'weed iam' still works with deprecation warning Usage: - weed s3 -port=8333 # S3 + IAM on same port (default) - weed s3 -iam=false # S3 only, disable embedded IAM - weed iam -port=8111 # Deprecated, shows warning * Fix nil pointer panic: add s3.iam flag to weed server command The enableIam field was not initialized when running S3 via 'weed server', causing a nil pointer dereference when checking *s3opt.enableIam. * Fix nil pointer panic: add s3.iam flag to weed filer command The enableIam field was not initialized when running S3 via 'weed filer -s3', causing a nil pointer dereference when checking *s3opt.enableIam. * Add integration tests for embedded IAM API Tests cover: - CreateUser, ListUsers, GetUser, UpdateUser, DeleteUser - CreateAccessKey, DeleteAccessKey, ListAccessKeys - CreatePolicy, PutUserPolicy, GetUserPolicy - Implicit username extraction from authorization header - Full user lifecycle workflow test These tests validate the embedded IAM API functionality that was added in the S3 server, ensuring IAM operations work correctly when served from the same port as S3. * Security: Use crypto/rand for IAM credential generation SECURITY FIX: Replace math/rand with crypto/rand for generating access keys and secret keys. Using math/rand is not cryptographically secure and can lead to predictable credentials. This change: 1. Replaces math/rand with crypto/rand in both: - weed/s3api/s3api_embedded_iam.go (embedded IAM) - weed/iamapi/iamapi_management_handlers.go (standalone IAM) 2. Removes the seededRand variable that was initialized with time-based seed (predictable) 3. Updates StringWithCharset/iamStringWithCharset to: - Use crypto/rand.Int() for secure random index generation - Return an error for proper error handling 4. Updates CreateAccessKey to handle the new error return 5. Updates DoActions handlers to propagate errors properly * Fix critical bug: DeleteUserPolicy was deleting entire user instead of policy BUG FIX: DeleteUserPolicy was incorrectly deleting the entire user identity from s3cfg.Identities instead of just clearing the user's inline policy (Actions). Before (wrong): s3cfg.Identities = append(s3cfg.Identities[:i], s3cfg.Identities[i+1:]...) After (correct): ident.Actions = nil Also: - Added proper iamDeleteUserPolicyResponse / DeleteUserPolicyResponse types - Fixed return type from iamPutUserPolicyResponse to iamDeleteUserPolicyResponse Affected files: - weed/s3api/s3api_embedded_iam.go (embedded IAM) - weed/iamapi/iamapi_management_handlers.go (standalone IAM) - weed/iamapi/iamapi_response.go (response types) * Add tests for DeleteUserPolicy to prevent regression Added two tests: 1. TestEmbeddedIamDeleteUserPolicy - Verifies that: - User is NOT deleted (identity still exists) - Credentials are NOT deleted - Only Actions (policy) are cleared to nil 2. TestEmbeddedIamDeleteUserPolicyUserNotFound - Verifies: - Returns 404 when user doesn't exist These tests ensure the bug fixed in the previous commit (deleting user instead of policy) doesn't regress. * Fix race condition: Add mutex lock to IAM DoActions The DoActions function performs a read-modify-write operation on the shared IAM configuration without any locking. This could lead to race conditions and data loss if multiple requests modify the IAM config concurrently. Added mutex lock at the start of DoActions in both: - weed/s3api/s3api_embedded_iam.go (embedded IAM) - weed/iamapi/iamapi_management_handlers.go (standalone IAM) The lock protects the entire read-modify-write cycle: 1. GetS3ApiConfiguration (read) 2. Modify s3cfg based on action 3. PutS3ApiConfiguration (write) * Fix action comparison and document CreatePolicy limitation 1. Replace reflect.DeepEqual with order-independent string slice comparison - Added iamStringSlicesEqual/stringSlicesEqual helper functions - Prevents duplicate policy statements when actions are in different order 2. Document CreatePolicy limitation in embedded IAM - Added TODO comment explaining that managed policies are not persisted - Users should use PutUserPolicy for inline policies 3. Fix deadlock in standalone IAM's CreatePolicy - Removed nested lock acquisition (DoActions already holds the lock) Files changed: - weed/s3api/s3api_embedded_iam.go - weed/iamapi/iamapi_management_handlers.go * Add rate limiting to embedded IAM endpoint Apply circuit breaker rate limiting to the IAM endpoint to prevent abuse. Also added request tracking for IAM operations. The IAM endpoint now follows the same pattern as other S3 endpoints: - track() for request metrics - s3a.iam.Auth() for authentication - s3a.cb.Limit() for rate limiting * Fix handleImplicitUsername to properly look up username from AccessKeyId According to AWS spec, when UserName is not specified in an IAM request, IAM should determine the username implicitly based on the AccessKeyId signing the request. Previously, the code incorrectly extracted s[2] (region field) from the SigV4 credential string as the username. This fix: 1. Extracts the AccessKeyId from s[0] of the credential string 2. Looks up the AccessKeyId in the credential store using LookupByAccessKey 3. Uses the identity's Name field as the username if found Also: - Added exported LookupByAccessKey wrapper method to IdentityAccessManagement - Updated tests to verify correct access key lookup behavior - Applied fix to both embedded IAM and standalone IAM implementations * Fix CreatePolicy to not trigger unnecessary save CreatePolicy validates the policy document and returns metadata but does not actually store the policy (SeaweedFS uses inline policies attached via PutUserPolicy). However, 'changed' was left as true, triggering an unnecessary save operation. Set changed = false after successful CreatePolicy validation in both embedded IAM and standalone IAM implementations. * Improve embedded IAM test quality - Remove unused mock types (mockCredentialManager, mockEmbeddedIamApi) - Use proto.Clone instead of proto.Merge for proper deep copy semantics - Replace brittle regex-based XML error extraction with proper XML unmarshalling - Remove unused regexp import - Add state and field assertions to tests: - CreateUser: verify username in response and user persisted in config - ListUsers: verify response contains expected users - GetUser: verify username in response - CreatePolicy: verify policy metadata in response - PutUserPolicy: verify actions were attached to user - CreateAccessKey: verify credentials in response and persisted in config * Remove shared test state and improve executeEmbeddedIamRequest - Remove package-level embeddedIamApi variable to avoid shared test state - Update executeEmbeddedIamRequest to accept API instance as parameter - Only call xml.Unmarshal when v != nil, making nil-v cases explicit - Return unmarshal error properly instead of always returning it - Update all tests to create their own EmbeddedIamApiForTest instance - Each test now has isolated state, preventing test interdependencies * Add comprehensive test coverage for embedded IAM Added tests for previously uncovered functions: - iamStringSlicesEqual: 0% → 100% - iamMapToStatementAction: 40% → 100% - iamMapToIdentitiesAction: 30% → 70% - iamHash: 100% - iamStringWithCharset: 85.7% - GetPolicyDocument: 75% → 100% - CreatePolicy: 91.7% → 100% - DeleteUser: 83.3% → 100% - GetUser: 83.3% → 100% - ListAccessKeys: 55.6% → 88.9% New test cases for helper functions, error handling, and edge cases. * Document IAM code duplication and reference GitHub issue #7747 Added comments to both IAM implementations noting the code duplication and referencing the tracking issue for future refactoring: - weed/s3api/s3api_embedded_iam.go (embedded IAM) - weed/iamapi/iamapi_management_handlers.go (standalone IAM) See: https://github.com/seaweedfs/seaweedfs/issues/7747 * Implement granular IAM authorization for self-service operations Previously, all IAM actions required ACTION_ADMIN permission, which was overly restrictive. This change implements AWS-like granular permissions: Self-service operations (allowed without admin for own resources): - CreateAccessKey (on own user) - DeleteAccessKey (on own user) - ListAccessKeys (on own user) - GetUser (on own user) - UpdateAccessKey (on own user) Admin-only operations: - CreateUser, DeleteUser, UpdateUser - PutUserPolicy, GetUserPolicy, DeleteUserPolicy - CreatePolicy - ListUsers - Operations on other users The new AuthIam middleware: 1. Authenticates the request (signature verification) 2. Parses the IAM Action and target UserName 3. For self-service actions, allows if user is operating on own resources 4. For all other actions or operations on other users, requires admin * Fix misleading comment in standalone IAM CreatePolicy The comment incorrectly stated that CreatePolicy only validates the policy document. In the standalone IAM server, CreatePolicy actually persists the policy via iama.s3ApiConfig.PutPolicies(). The changed flag is false because it doesn't modify s3cfg.Identities, not because nothing is stored. * Simplify IAM auth and add RequestId to responses - Remove redundant ACTION_ADMIN fallback in AuthIam: The action parameter in authRequest is for permission checking, not signature verification. If auth fails with ACTION_READ, it will fail with ACTION_ADMIN too. - Add SetRequestId() call before writing IAM responses for AWS compatibility. All IAM response structs embed iamCommonResponse which has SetRequestId(). * Address code review feedback for IAM implementation 1. auth_credentials.go: Add documentation warning that LookupByAccessKey returns internal pointers that should not be mutated. 2. iamapi_management_handlers.go & s3api_embedded_iam.go: Add input guards for StringWithCharset/iamStringWithCharset when length <= 0 or charset is empty to avoid runtime errors from rand.Int. 3. s3api_embedded_iam_test.go: Don't ignore xml.Marshal errors in test DoActions handler. Return proper error response if marshaling fails. 4. s3api_embedded_iam_test.go: Use obviously fake access key IDs (AKIATESTFAKEKEY*) to avoid CI secret scanner false positives. * Address code review feedback for IAM implementation (batch 2) 1. iamapi/iamapi_management_handlers.go: - Redact Authorization header log (security: avoid exposing signature) - Add nil-guard for iama.iam before LookupByAccessKey call 2. iamapi/iamapi_test.go: - Replace real-looking access keys with obviously fake ones (AKIATESTFAKEKEY*) to avoid CI secret scanner false positives 3. s3api/s3api_embedded_iam.go - CreateUser: - Validate UserName is not empty (return ErrCodeInvalidInputException) - Check for duplicate users (return ErrCodeEntityAlreadyExistsException) 4. s3api/s3api_embedded_iam.go - CreateAccessKey: - Return ErrCodeNoSuchEntityException if user doesn't exist - Removed implicit user creation behavior 5. s3api/s3api_embedded_iam.go - getActions: - Fix S3 ARN parsing for bucket/path patterns - Handle mybucket, mybucket/*, mybucket/path/* correctly - Return error if no valid actions found in policy 6. s3api/s3api_embedded_iam.go - handleImplicitUsername: - Redact Authorization header log - Add nil-guard for e.iam 7. s3api/s3api_embedded_iam.go - DoActions: - Reload in-memory IAM maps after credential mutations - Call LoadS3ApiConfigurationFromCredentialManager after save 8. s3api/auth_credentials.go - AuthSignatureOnly: - Add new signature-only authentication method - Bypasses S3 authorization checks for IAM operations - Used by AuthIam to properly separate signature verification from IAM-specific permission checks * Fix nil pointer dereference and error handling in IAM 1. AuthIam: Add nil check for identity after AuthSignatureOnly - AuthSignatureOnly can return nil identity with ErrNone for authTypePostPolicy or authTypeStreamingUnsigned - Now returns ErrAccessDenied if identity is nil 2. writeIamErrorResponse: Add missing error code cases - ErrCodeEntityAlreadyExistsException -> HTTP 409 Conflict - ErrCodeInvalidInputException -> HTTP 400 Bad Request 3. UpdateUser: Use consistent error handling - Changed from direct ErrInvalidRequest to writeIamErrorResponse - Now returns correct HTTP status codes based on error type * Add IAM config reload to standalone IAM server after mutations Match the behavior of embedded IAM (s3api_embedded_iam.go) by reloading the in-memory identity maps after persisting configuration changes. This ensures newly created access keys are visible to LookupByAccessKey immediately without requiring a server restart. * Minor improvements to test helpers and log masking 1. iamapi_test.go: Update mustMarshalJSON to use t.Helper() and t.Fatal() instead of panic() for better test diagnostics 2. s3api_embedded_iam.go: Mask access key in 'not found' log message to avoid exposing full access key IDs in logs * Mask access key in standalone IAM log message for consistency Match the embedded IAM version by masking the access key ID in the 'not found' log message (show only first 4 chars). |
||
|
chrislu
|
1ee03b6411 | fmt | ||
|
Chris Lu
|
2d06ddab41 |
Remove default concurrent upload/download limits for best performance (#7712)
Change all concurrentUploadLimitMB and concurrentDownloadLimitMB defaults from fixed values (64, 128, 256 MB) to 0 (unlimited). This removes artificial throttling that can limit throughput on high-performance systems, especially on all-flash setups with many cores. Files changed: - volume.go: concurrentUploadLimitMB 256->0, concurrentDownloadLimitMB 256->0 - server.go: filer/volume/s3 concurrent limits 64/128->0 - s3.go: concurrentUploadLimitMB 128->0 - filer.go: concurrentUploadLimitMB 128->0, s3.concurrentUploadLimitMB 128->0 Users can still set explicit limits if needed for resource management. |
||
|
Chris Lu
|
a9b3be416b |
fix: initialize missing S3 options in filer to prevent nil pointer dereference (#7646)
* fix: initialize missing S3 options in filer to prevent nil pointer dereference Fixes #7644 When starting the S3 gateway from the filer, several S3Options fields were not being initialized, which could cause nil pointer dereferences during startup. This commit adds initialization for: - iamConfig: for advanced IAM configuration - metricsHttpPort: for Prometheus metrics endpoint - metricsHttpIp: for binding the metrics endpoint Also ensures metricsHttpIp defaults to bindIp when not explicitly set, matching the behavior of the standalone S3 server. This prevents the panic that was occurring in the s3.go:226 area when these pointer fields were accessed but never initialized. * fix: copy value instead of pointer for metricsHttpIp default Address review comment to avoid pointer aliasing. Copy the value instead of the pointer to prevent unexpected side effects if the bindIp value is modified later. |
||
|
chrislu
|
5167bbd2a9 |
Remove deprecated allowEmptyFolder CLI option
The allowEmptyFolder option is no longer functional because: 1. The code that used it was already commented out 2. Empty folder cleanup is now handled asynchronously by EmptyFolderCleaner The CLI flags are kept for backward compatibility but marked as deprecated and ignored. This removes: - S3ApiServerOption.AllowEmptyFolder field - The actual usage in s3api_object_handlers_list.go - Helm chart values and template references - References in test Makefiles and docker-compose files |
||
|
Chris Lu
|
3183a49698 |
fix: S3 downloads failing after idle timeout (#7626)
* fix: S3 downloads failing after idle timeout (#7618) The idle timeout was incorrectly terminating active downloads because read and write deadlines were managed independently. During a download, the server writes data but rarely reads, so the read deadline would expire even though the connection was actively being used. Changes: 1. Simplify to single Timeout field - since this is a 'no activity timeout' where any activity extends the deadline, separate read/write timeouts are unnecessary. Now uses SetDeadline() which sets both at once. 2. Implement proper 'no activity timeout' - any activity (read or write) now extends the deadline. The connection only times out when there's genuinely no activity in either direction. 3. Increase default S3 idleTimeout from 10s to 120s for additional safety margin when fetching chunks from slow storage backends. Fixes #7618 * Update weed/util/net_timeout.go Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> |
||
|
Chris Lu
|
edf0ef7a80 |
Filer, S3: Feature/add concurrent file upload limit (#7554)
* Support multiple filers for S3 and IAM servers with automatic failover
This change adds support for multiple filer addresses in the 'weed s3' and 'weed iam' commands, enabling high availability through automatic failover.
Key changes:
- Updated S3ApiServerOption.Filer to Filers ([]pb.ServerAddress)
- Updated IamServerOption.Filer to Filers ([]pb.ServerAddress)
- Modified -filer flag to accept comma-separated addresses
- Added getFilerAddress() helper methods for backward compatibility
- Updated all filer client calls to support multiple addresses
- Uses pb.WithOneOfGrpcFilerClients for automatic failover
Usage:
weed s3 -filer=localhost:8888,localhost:8889
weed iam -filer=localhost:8888,localhost:8889
The underlying FilerClient already supported multiple filers with health
tracking and automatic failover - this change exposes that capability
through the command-line interface.
* Add filer discovery: treat initial filers as seeds and discover peers from master
Enhances FilerClient to automatically discover additional filers in the same
filer group by querying the master server. This allows users to specify just
a few seed filers, and the client will discover all other filers in the cluster.
Key changes to wdclient/FilerClient:
- Added MasterClient, FilerGroup, and DiscoveryInterval fields
- Added thread-safe filer list management with RWMutex
- Implemented discoverFilers() background goroutine
- Uses cluster.ListExistingPeerUpdates() to query master for filers
- Automatically adds newly discovered filers to the list
- Added Close() method to clean up discovery goroutine
New FilerClientOption fields:
- MasterClient: enables filer discovery from master
- FilerGroup: specifies which filer group to discover
- DiscoveryInterval: how often to refresh (default 5 minutes)
Usage example:
masterClient := wdclient.NewMasterClient(...)
filerClient := wdclient.NewFilerClient(
[]pb.ServerAddress{"localhost:8888"}, // seed filers
grpcDialOption,
dataCenter,
&wdclient.FilerClientOption{
MasterClient: masterClient,
FilerGroup: "my-group",
},
)
defer filerClient.Close()
The initial filers act as seeds - the client discovers and adds all other
filers in the same group from the master. Discovered filers are added
dynamically without removing existing ones (relying on health checks for
unavailable filers).
* Address PR review comments: implement full failover for IAM operations
Critical fixes based on code review feedback:
1. **IAM API Failover (Critical)**:
- Replace pb.WithGrpcFilerClient with pb.WithOneOfGrpcFilerClients in:
* GetS3ApiConfigurationFromFiler()
* PutS3ApiConfigurationToFiler()
* GetPolicies()
* PutPolicies()
- Now all IAM operations support automatic failover across multiple filers
2. **Validation Improvements**:
- Add validation in NewIamApiServerWithStore() to require at least one filer
- Add validation in NewS3ApiServerWithStore() to require at least one filer
- Add warning log when no filers configured for credential store
3. **Error Logging**:
- Circuit breaker now logs when config load fails instead of silently ignoring
- Helps operators understand why circuit breaker limits aren't applied
4. **Code Quality**:
- Use ToGrpcAddress() for filer address in credential store setup
- More consistent with rest of codebase and future-proof
These changes ensure IAM operations have the same high availability guarantees
as S3 operations, completing the multi-filer failover implementation.
* Fix IAM manager initialization: remove code duplication, add TODO for HA
Addresses review comment on s3api_server.go:145
Changes:
- Remove duplicate code for getting first filer address
- Extract filerAddr variable once and reuse
- Add TODO comment documenting the HA limitation for IAM manager
- Document that loadIAMManagerFromConfig and NewS3IAMIntegration need
updates to support multiple filers for full HA
Note: This is a known limitation when using filer-backed IAM stores.
The interfaces need to be updated to accept multiple filer addresses.
For now, documenting this limitation clearly.
* Document credential store HA limitation with TODO
Addresses review comment on auth_credentials.go:149
Changes:
- Add TODO comment documenting that SetFilerClient interface needs update
for multi-filer support
- Add informative log message indicating HA limitation
- Document that this is a known limitation for filer-backed credential stores
The SetFilerClient interface currently only accepts a single filer address.
To properly support HA, the credential store interfaces need to be updated
to handle multiple filer addresses.
* Track current active filer in FilerClient for better HA
Add GetCurrentFiler() method to FilerClient that returns the currently
active filer based on the filerIndex which is updated on successful
operations. This provides better availability than always using the
first filer.
Changes:
- Add FilerClient.GetCurrentFiler() method that returns current active filer
- Update S3ApiServer.getFilerAddress() to use FilerClient's current filer
- Add fallback to first filer if FilerClient not yet initialized
- Document IAM limitation (doesn't have FilerClient access)
Benefits:
- Single-filer operations (URLs, ReadFilerConf, etc.) now use the
currently active/healthy filer
- Better distribution and failover behavior
- FilerClient's round-robin and health tracking automatically
determines which filer to use
* Document ReadFilerConf HA limitation in lifecycle handlers
Addresses review comment on s3api_bucket_handlers.go:880
Add comment documenting that ReadFilerConf uses the current active filer
from FilerClient (which is better than always using first filer), but
doesn't have built-in multi-filer failover.
Add TODO to update filer.ReadFilerConf to support multiple filers for
complete HA. For now, it uses the currently active/healthy filer tracked
by FilerClient which provides reasonable availability.
* Document multipart upload URL HA limitation
Addresses review comment on s3api_object_handlers_multipart.go:442
Add comment documenting that part upload URLs point to the current
active filer (tracked by FilerClient), which is better than always
using the first filer but still creates a potential point of failure
if that filer becomes unavailable during upload.
Suggest TODO solutions:
- Use virtual hostname/load balancer for filers
- Have S3 server proxy uploads to healthy filers
Current behavior provides reasonable availability by using the
currently active/healthy filer rather than being pinned to first filer.
* Document multipart completion Location URL limitation
Addresses review comment on filer_multipart.go:187
Add comment documenting that the Location URL in CompleteMultipartUpload
response points to the current active filer (tracked by FilerClient).
Note that clients should ideally use the S3 API endpoint rather than
this direct URL. If direct access is attempted and the specific filer
is unavailable, the request will fail.
Current behavior uses the currently active/healthy filer rather than
being pinned to the first filer, providing better availability.
* Make credential store use current active filer for HA
Update FilerEtcStore to use a function that returns the current active
filer instead of a fixed address, enabling high availability.
Changes:
- Add SetFilerAddressFunc() method to FilerEtcStore
- Store uses filerAddressFunc instead of fixed filerGrpcAddress
- withFilerClient() calls the function to get current active filer
- Keep SetFilerClient() for backward compatibility (marked deprecated)
- Update S3ApiServer to pass FilerClient.GetCurrentFiler to store
Benefits:
- Credential store now uses currently active/healthy filer
- Automatic failover when filer becomes unavailable
- True HA for credential operations
- Backward compatible with old SetFilerClient interface
This addresses the credential store limitation - no longer pinned to
first filer, uses FilerClient's tracked current active filer.
* Clarify multipart URL comments: filer address not used for uploads
Update comments to reflect that multipart upload URLs are not actually
used for upload traffic - uploads go directly to volume servers.
Key clarifications:
- genPartUploadUrl: Filer address is parsed out, only path is used
- CompleteMultipartUpload Location: Informational field per AWS S3 spec
- Actual uploads bypass filer proxy and go directly to volume servers
The filer address in these URLs is NOT a HA concern because:
1. Part uploads: URL is parsed for path, upload goes to volume servers
2. Location URL: Informational only, clients use S3 endpoint
This addresses the observation that S3 uploads don't go through filers,
only metadata operations do.
* Remove filer address from upload paths - pass path directly
Eliminate unnecessary filer address from upload URLs by passing file
paths directly instead of full URLs that get immediately parsed.
Changes:
- Rename genPartUploadUrl() → genPartUploadPath() (returns path only)
- Rename toFilerUrl() → toFilerPath() (returns path only)
- Update putToFiler() to accept filePath instead of uploadUrl
- Remove URL parsing code (no longer needed)
- Remove net/url import (no longer used)
- Keep old function names as deprecated wrappers for compatibility
Benefits:
- Cleaner code - no fake URL construction/parsing
- No dependency on filer address for internal operations
- More accurate naming (these are paths, not URLs)
- Eliminates confusion about HA concerns
This completely removes the filer address from upload operations - it was
never actually used for routing, only parsed for the path.
* Remove deprecated functions: use new path-based functions directly
Remove deprecated wrapper functions and update all callers to use the
new function names directly.
Removed:
- genPartUploadUrl() → all callers now use genPartUploadPath()
- toFilerUrl() → all callers now use toFilerPath()
- SetFilerClient() → removed along with fallback code
Updated:
- s3api_object_handlers_multipart.go: uploadUrl → filePath
- s3api_object_handlers_put.go: uploadUrl → filePath, versionUploadUrl → versionFilePath
- s3api_object_versioning.go: toFilerUrl → toFilerPath
- s3api_object_handlers_test.go: toFilerUrl → toFilerPath
- auth_credentials.go: removed SetFilerClient fallback
- filer_etc_store.go: removed deprecated SetFilerClient method
Benefits:
- Cleaner codebase with no deprecated functions
- All variable names accurately reflect that they're paths, not URLs
- Single interface for credential stores (SetFilerAddressFunc only)
All code now consistently uses the new path-based approach.
* Fix toFilerPath: remove URL escaping for raw file paths
The toFilerPath function should return raw file paths, not URL-escaped
paths. URL escaping was needed when the path was embedded in a URL
(old toFilerUrl), but now that we pass paths directly to putToFiler,
they should be unescaped.
This fixes S3 integration test failures:
- test_bucket_listv2_encoding_basic
- test_bucket_list_encoding_basic
- test_bucket_listv2_delimiter_whitespace
- test_bucket_list_delimiter_whitespace
The tests were failing because paths were double-encoded (escaped when
stored, then escaped again when listed), resulting in %252B instead of
%2B for '+' characters.
Root cause: When we removed URL parsing in putToFiler, we should have
also removed URL escaping in toFilerPath since paths are now used
directly without URL encoding/decoding.
* Add thread safety to FilerEtcStore and clarify credential store comments
Address review suggestions for better thread safety and code clarity:
1. **Thread Safety**: Add RWMutex to FilerEtcStore
- Protects filerAddressFunc and grpcDialOption from concurrent access
- Initialize() uses write lock when setting function
- SetFilerAddressFunc() uses write lock
- withFilerClient() uses read lock to get function and dial option
- GetPolicies() uses read lock to check if configured
2. **Improved Error Messages**:
- Prefix errors with "filer_etc:" for easier debugging
- "filer address not configured" → "filer_etc: filer address function not configured"
- "filer address is empty" → "filer_etc: filer address is empty"
3. **Clarified Comments**:
- auth_credentials.go: Clarify that initial setup is temporary
- Document that it's updated in s3api_server.go after FilerClient creation
- Remove ambiguity about when FilerClient.GetCurrentFiler is used
Benefits:
- Safe for concurrent credential operations
- Clear error messages for debugging
- Explicit documentation of initialization order
* Enable filer discovery: pass master addresses to FilerClient
Fix two critical issues:
1. **Filer Discovery Not Working**: Master client was not being passed to
FilerClient, so peer discovery couldn't work
2. **Credential Store Design**: Already uses FilerClient via GetCurrentFiler
function - this is the correct design for HA
Changes:
**Command (s3.go):**
- Read master addresses from GetFilerConfiguration response
- Pass masterAddresses to S3ApiServerOption
- Log master addresses for visibility
**S3ApiServerOption:**
- Add Masters []pb.ServerAddress field for discovery
**S3ApiServer:**
- Create MasterClient from Masters when available
- Pass MasterClient + FilerGroup to FilerClient via options
- Enable discovery with 5-minute refresh interval
- Log whether discovery is enabled or disabled
**Credential Store:**
- Already correctly uses filerClient.GetCurrentFiler via function
- This provides HA without tight coupling to FilerClient struct
- Function-based design is clean and thread-safe
Discovery Flow:
1. S3 command reads filer config → gets masters + filer group
2. S3ApiServer creates MasterClient from masters
3. FilerClient uses MasterClient to query for peer filers
4. Background goroutine refreshes peer list every 5 minutes
5. Credential store uses GetCurrentFiler to get active filer
Now filer discovery actually works! ��
* Use S3 endpoint in multipart Location instead of filer address
* Add multi-filer failover to ReadFilerConf
* Address CodeRabbit review: fix buffer reuse and improve lock safety
Address two code review suggestions:
1. **Fix buffer reuse in ReadFilerConfFromFilers**:
- Use local []byte data instead of shared buffer
- Prevents partial data from failed attempts affecting successful reads
- Creates fresh buffer inside callback for masterClient path
- More robust to future changes in read helpers
2. **Improve lock safety in FilerClient**:
- Add *WithHealth variants that accept health pointer
- Get health pointer while holding lock, then release before calling
- Eliminates potential for lock confusion (though no actual deadlock existed)
- Clearer separation: lock for data access, atomics for health ops
Changes:
- ReadFilerConfFromFilers: var data []byte, create buf inside callback
- shouldSkipUnhealthyFilerWithHealth(health *filerHealth)
- recordFilerSuccessWithHealth(health *filerHealth)
- recordFilerFailureWithHealth(health *filerHealth)
- Keep old functions for backward compatibility (marked deprecated)
- Update LookupVolumeIds to use WithHealth variants
Benefits:
- More robust multi-filer configuration reading
- Clearer lock vs atomic operation boundaries
- No lock held during health checks (even though atomics don't block)
- Better code organization and maintainability
* add constant
* Fix IAM manager and post policy to use current active filer
* Fix critical race condition and goroutine leak
* Update weed/s3api/filer_multipart.go
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
* Fix compilation error and address code review suggestions
Address remaining unresolved comments:
1. **Fix compilation error**: Add missing net/url import
- filer_multipart.go used url.PathEscape without import
- Added "net/url" to imports
2. **Fix Location URL formatting** (all 4 occurrences):
- Add missing slash between bucket and key
- Use url.PathEscape for bucket names
- Use urlPathEscape for object keys
- Handles special characters in bucket/key names
- Before: http://host/bucketkey
- After: http://host/bucket/key (properly escaped)
3. **Optimize discovery loop** (O(N*M) → O(N+M)):
- Use map for existing filers (O(1) lookup)
- Reduces time holding write lock
- Better performance with many filers
- Before: Nested loop for each discovered filer
- After: Build map once, then O(1) lookups
Changes:
- filer_multipart.go: Import net/url, fix all Location URLs
- filer_client.go: Use map for efficient filer discovery
Benefits:
- Compiles successfully
- Proper URL encoding (handles spaces, special chars)
- Faster discovery with less lock contention
- Production-ready URL formatting
* Fix race conditions and make Close() idempotent
Address CodeRabbit review #3512078995:
1. **Critical: Fix unsynchronized read in error message**
- Line 584 read len(fc.filerAddresses) without lock
- Race with refreshFilerList appending to slice
- Fixed: Take RLock to read length safely
- Prevents race detector warnings
2. **Important: Make Close() idempotent**
- Closing already-closed channel panics
- Can happen with layered cleanup in shutdown paths
- Fixed: Use sync.Once to ensure single close
- Safe to call Close() multiple times now
3. **Nitpick: Add warning for empty filer address**
- getFilerAddress() can return empty string
- Helps diagnose unexpected state
- Added: Warning log when no filers available
4. **Nitpick: Guard deprecated index-based helpers**
- shouldSkipUnhealthyFiler, recordFilerSuccess/Failure
- Accessed filerHealth without lock (races with discovery)
- Fixed: Take RLock and check bounds before array access
- Prevents index out of bounds and races
Changes:
- filer_client.go:
- Add closeDiscoveryOnce sync.Once field
- Use Do() in Close() for idempotent channel close
- Add RLock guards to deprecated index-based helpers
- Add bounds checking to prevent panics
- Synchronized read of filerAddresses length in error
- s3api_server.go:
- Add warning log when getFilerAddress returns empty
Benefits:
- No race conditions (passes race detector)
- No panic on double-close
- Better error diagnostics
- Safe with discovery enabled
- Production-hardened shutdown logic
* Fix hardcoded http scheme and add panic recovery
Address CodeRabbit review #3512114811:
1. **Major: Fix hardcoded http:// scheme in Location URLs**
- Location URLs always used http:// regardless of client connection
- HTTPS clients got http:// URLs (incorrect)
- Fixed: Detect scheme from request
- Check X-Forwarded-Proto header (for proxies) first
- Check r.TLS != nil for direct HTTPS
- Fallback to http for plain connections
- Applied to all 4 CompleteMultipartUploadResult locations
2. **Major: Add panic recovery to discovery goroutine**
- Long-running background goroutine could crash entire process
- Panic in refreshFilerList would terminate program
- Fixed: Add defer recover() with error logging
- Goroutine failures now logged, not fatal
3. **Note: Close() idempotency already implemented**
- Review flagged as duplicate issue
- Already fixed in commit
|
||
|
Chris Lu
|
605b3333c1 |
fix sftp start with filer (#7120)
* fix sftp start with filer * add bindIp |
||
 zuzuviewer
|
8fa1a69f8c | * Fix undefined http serve behaiver (#6943) | ||
|
chrislu
|
bd4891a117 | change version directory | ||
 Mohamed Sekour
|
93aed187e9 |
Add SFTP Server Support (#6753)
* Add SFTP Server Support Signed-off-by: Mohamed Sekour <mohamed.sekour@exfo.com> * fix s3 tests and helm lint Signed-off-by: Mohamed Sekour <mohamed.sekour@exfo.com> * increase helm chart version * adjust version --------- Signed-off-by: Mohamed Sekour <mohamed.sekour@exfo.com> Co-authored-by: chrislu <chris.lu@gmail.com> |
||
 ARibster
|
a2c5510ae1 |
Addendum to issue #6733 (#6756)
add s3.ip.bind command line parameter to filer |
||
|
ARibster
|
d424cbbcd5 |
fix: S3 Feature: please add s3.idleTimeout command line parameter #6746 (#6747)
ildeTimeout command line parameter for s3 |
||
 Konstantin Lebedev
|
aa9121aa9d |
Revert "[filer] void panic if s3opt.tlsCACertificate is nill" (#6694)
Revert "[filer] void panic if s3opt.tlsCACertificate is nill (#6685)"
This reverts commit
|
||
|
Konstantin Lebedev
|
4dca59ed27 |
[filer] void panic if s3opt.tlsCACertificate is nill (#6685)
* fix issues/6684 * add s3.cacert.file option for filer |
||
 Karol Będkowski
|
95746f48e6 |
feat/fix: add s3.cacert.file/tlsVerifyClientCert to filer command (#6659)
This prevent crash filler with nil pointer dereference as s3 expect this parameters. New two parameters are added to filer command - copy of s3 parameters: - s3.cacert.file - path to the TLS CA certificate file - s3.tlsVerifyClientCert - whether to verify the client's certificate |
||
 Weihao Jiang
|
75ef324533 |
Allow standalone webdav server to bind specific address (#6403)
* Allow standalone webdav to bind specific address * Rename bindIp to ipBind * Bind embedded webdav IP to its parent |
||
 qinguoyi
|
bf2366ba3a | Fix KeyMaterial nil bug (#5849) | ||
|
Konstantin Lebedev
|
e1f9336688 |
[filer] fix panic filer on webdav.maxMB (#5822)
fix panic filer on webdav.maxMB |
||
 vadimartynov
|
b796c21fa9 | Added loadSecurityConfigOnce (#5792) | ||
|
vadimartynov
|
ec9e7493b3 |
-metricsIp cmd flag (#5773)
* Added/Updated: - Added metrics ip options for all servers; - Fixed a bug with the selection of the binIp or ip parameter for the metrics handler; * Fixed cmd flags |
||
|
vadimartynov
|
de60f383de |
Added tls for http handlers (#5764)
* Added https handler for filer * Added example for security.toml |
||
|
chrislu
|
ac6fd36c06 | using fixed bootstrap from peer filer | ||
 MeenakshiSachdeva
|
1caa012a76 | Fix Filer Sync Issue: 5455 (#5663) | ||
 jerebear12
|
85d6d5371b |
Disable filer UI in configuration (#5297)
* Add filer.ui.enabled configuration property * Add filer.expose_directory_metadata to config * Ammend commit * Remove ShowUI reference * Update all routes that allow directory metadata * Add cmd flag to server.go |
||
|
chrislu
|
d3688938d9 |
avoid nil
fix https://github.com/seaweedfs/seaweedfs/issues/5153 introduced from #5109 |
||
|
jerebear12
|
06343f8976 |
Set allowed origins in config (#5109)
* Add a way to use a JWT in an HTTP only cookie If a JWT is not included in the Authorization header or a query string, attempt to get a JWT from an HTTP only cookie. * Added a way to specify allowed origins header from config * Removed unecessary log * Check list of domains from config or command flag * Handle default wildcard and change name of config value to cors |
||
 tired-engineer
|
4ed06e9ba5 |
Pass correct Filer's gRPC port to S3 server. (#4972)
Fixes seaweedfs/seaweedfs#4971 |