Tree: eb054eb1f7

add-admin-and-worker-to-helm-charts

add-ec-vacuum

add_fasthttp_client

add_remote_storage

adding-message-queue-integration-tests

also-delete-parent-directory-if-empty

avoid_releasing_temp_file_on_write

changing-to-zap

collect-public-metrics

copilot/fix-helm-chart-installation

copilot/fix-s3-object-tagging-issue

create-table-snapshot-api-design

data_query_pushdown

dependabot/maven/other/java/client/com.google.protobuf-protobuf-java-3.25.5

dependabot/maven/other/java/examples/org.apache.hadoop-hadoop-common-3.4.0

detect-and-plan-ec-tasks

do-not-retry-if-error-is-NotFound

ec-disk-type-support

enhance-erasure-coding

fasthttp

feature/tus-protocol

filer1_maintenance_branch

fix-GetObjectLockConfigurationHandler

fix-mount-http-parallelism

fix-s3-object-tagging-issue-7589

fix-versioning-listing-only

ftp

gh-pages

improve-fuse-mount

improve-fuse-mount2

logrus

master

message_send

mount2

mq-subscribe

mq2

original_weed_mount

pr-7412

random_access_file

refactor-needle-read-operations

refactor-volume-write

remote_overlay

revert-5134-patch-1

revert-5819-patch-1

revert-6434-bugfix-missing-s3-audit

s3-select

sub

tcp_read

test-reverting-lock-table

test_udp

testing

testing-sdx-generation

tikv

track-mount-e2e

upgrade-versions-to-4.00

volume_buffered_writes

worker-execute-ec-tasks

0.72

0.72.release

0.73

0.74

0.75

0.76

0.77

0.90

0.91

0.92

0.93

0.94

0.95

0.96

0.97

0.98

0.99

1.00

1.01

1.02

1.03

1.04

1.05

1.06

1.07

1.08

1.09

1.10

1.11

1.12

1.14

1.15

1.16

1.17

1.18

1.19

1.20

1.21

1.22

1.23

1.24

1.25

1.26

1.27

1.28

1.29

1.30

1.31

1.32

1.33

1.34

1.35

1.36

1.37

1.38

1.40

1.41

1.42

1.43

1.44

1.45

1.46

1.47

1.48

1.49

1.50

1.51

1.52

1.53

1.54

1.55

1.56

1.57

1.58

1.59

1.60

1.61

1.61RC

1.62

1.63

1.64

1.65

1.66

1.67

1.68

1.69

1.70

1.71

1.72

1.73

1.74

1.75

1.76

1.77

1.78

1.79

1.80

1.81

1.82

1.83

1.84

1.85

1.86

1.87

1.88

1.90

1.91

1.92

1.93

1.94

1.95

1.96

1.97

1.98

1.99

1;70

2.00

2.01

2.02

2.03

2.04

2.05

2.06

2.07

2.08

2.09

2.10

2.11

2.12

2.13

2.14

2.15

2.16

2.17

2.18

2.19

2.20

2.21

2.22

2.23

2.24

2.25

2.26

2.27

2.28

2.29

2.30

2.31

2.32

2.33

2.34

2.35

2.36

2.37

2.38

2.39

2.40

2.41

2.42

2.43

2.47

2.48

2.49

2.50

2.51

2.52

2.53

2.54

2.55

2.56

2.57

2.58

2.59

2.60

2.61

2.62

2.63

2.64

2.65

2.66

2.67

2.68

2.69

2.70

2.71

2.72

2.73

2.74

2.75

2.76

2.77

2.78

2.79

2.80

2.81

2.82

2.83

2.84

2.85

2.86

2.87

2.88

2.89

2.90

2.91

2.92

2.93

2.94

2.95

2.96

2.97

2.98

2.99

3.00

3.01

3.02

3.03

3.04

3.05

3.06

3.07

3.08

3.09

3.10

3.11

3.12

3.13

3.14

3.15

3.16

3.18

3.19

3.20

3.21

3.22

3.23

3.24

3.25

3.26

3.27

3.28

3.29

3.30

3.31

3.32

3.33

3.34

3.35

3.36

3.37

3.38

3.39

3.40

3.41

3.42

3.43

3.44

3.45

3.46

3.47

3.48

3.50

3.51

3.52

3.53

3.54

3.55

3.56

3.57

3.58

3.59

3.60

3.61

3.62

3.63

3.64

3.65

3.66

3.67

3.68

3.69

3.71

3.72

3.73

3.74

3.75

3.76

3.77

3.78

3.79

3.80

3.81

3.82

3.83

3.84

3.85

3.86

3.87

3.88

3.89

3.90

3.91

3.92

3.93

3.94

3.95

3.96

3.97

3.98

3.99

4.00

4.01

dev

helm-3.65.1

v0.69

v0.70beta

v3.33

Branches Tags

${ item.name }

Create tag ${ searchTerm }

Create branch ${ searchTerm }

from 'eb054eb1f7'

${ noResults }

Commit Graph

686 Commits (eb054eb1f7d49b9c245ed9dbb801c7b14afd0ea6)

Author	SHA1	Message	Date
chrislu	eb054eb1f7	err on duplicated tag key	2 weeks ago
chrislu	08314b4688	multiple header values	2 weeks ago
chrislu	13b31de902	surfacing invalid X-Amz-Tagging as a client error	2 weeks ago
chrislu	bbdd7903f8	refactor or remove obsolete logic on filer	2 weeks ago
chrislu	b056540488	reuse	2 weeks ago
chrislu	e10cd469d9	remove obsolete	2 weeks ago
chrislu	6a7b887957	remove SeaweedFSIsDirectoryKey	2 weeks ago
chrislu	539206bde8	set identity to request context	2 weeks ago
Chris Lu	a9fefcd22c	S3: list owned buckets (#7519) * list owned buckets * simplify * add unit tests * no-owner buckets * set identity id * fallback to request header if iam is not enabled * refactor to test * fix comparing * fix security vulnerability * Update s3api_bucket_handlers.go * Update s3api_bucket_handlers.go * Update s3api_bucket_handlers.go	2 weeks ago
chrislu	7c82e1a3d1	Update s3api_bucket_handlers.go	2 weeks ago
chrislu	ebaeaa6892	Update s3api_bucket_handlers.go	2 weeks ago
chrislu	5890109812	Update s3api_bucket_handlers.go	2 weeks ago
chrislu	7f814f8772	fix security vulnerability	2 weeks ago
chrislu	ec69e25d7b	fix comparing	2 weeks ago
chrislu	fec3088fb1	refactor to test	2 weeks ago
chrislu	b72df84361	fallback to request header if iam is not enabled	2 weeks ago
chrislu	611a2db4e3	set identity id	2 weeks ago
chrislu	2a5e5049c7	no-owner buckets	2 weeks ago
chrislu	53dfc43d2b	add unit tests	2 weeks ago
chrislu	c81ff673a1	simplify	2 weeks ago
chrislu	b02f865183	list owned buckets	2 weeks ago
Chris Lu	c1b8d4bf0d	S3: adds FilerClient to use cached volume id (#7518) * adds FilerClient to use cached volume id * refactor: MasterClient embeds vidMapClient to eliminate ~150 lines of duplication - Create masterVolumeProvider that implements VolumeLocationProvider - MasterClient now embeds vidMapClient instead of maintaining duplicate cache logic - Removed duplicate methods: LookupVolumeIdsWithFallback, getStableVidMap, etc. - MasterClient still receives real-time updates via KeepConnected streaming - Updates call inherited addLocation/deleteLocation from vidMapClient - Benefits: DRY principle, shared singleflight, cache chain logic reused - Zero behavioral changes - only architectural improvement * refactor: mount uses FilerClient for efficient volume location caching - Add configurable vidMap cache size (default: 5 historical snapshots) - Add FilerClientOption struct for clean configuration * GrpcTimeout: default 5 seconds (prevents hanging requests) * UrlPreference: PreferUrl or PreferPublicUrl * CacheSize: number of historical vidMap snapshots (for volume moves) - NewFilerClient uses option struct for better API extensibility - Improved error handling in filerVolumeProvider.LookupVolumeIds: * Distinguish genuine 'not found' from communication failures * Log volumes missing from filer response * Return proper error context with volume count * Document that filer Locations lacks Error field (unlike master) - FilerClient.GetLookupFileIdFunction() handles URL preference automatically - Mount (WFS) creates FilerClient with appropriate options - Benefits for weed mount: * Singleflight: Deduplicates concurrent volume lookups * Cache history: Old volume locations available briefly when volumes move * Configurable cache depth: Tune for different deployment environments * Battle-tested vidMap cache with cache chain * Better concurrency handling with timeout protection * Improved error visibility and debugging - Old filer.LookupFn() kept for backward compatibility - Performance improvement for mount operations with high concurrency * fix: prevent vidMap swap race condition in LookupFileIdWithFallback - Hold vidMapLock.RLock() during entire vm.LookupFileId() call - Prevents resetVidMap() from swapping vidMap mid-operation - Ensures atomic access to the current vidMap instance - Added documentation warnings to getStableVidMap() about swap risks - Enhanced withCurrentVidMap() documentation for clarity This fixes a subtle race condition where: 1. Thread A: acquires lock, gets vm pointer, releases lock 2. Thread B: calls resetVidMap(), swaps vc.vidMap 3. Thread A: calls vm.LookupFileId() on old/stale vidMap While the old vidMap remains valid (in cache chain), holding the lock ensures we consistently use the current vidMap for the entire operation. * fix: FilerClient supports multiple filer addresses for high availability Critical fix: FilerClient now accepts []ServerAddress instead of single address - Prevents mount failure when first filer is down (regression fix) - Implements automatic failover to remaining filers - Uses round-robin with atomic index tracking (same pattern as WFS.WithFilerClient) - Retries all configured filers before giving up - Updates successful filer index for future requests Changes: - NewFilerClient([]pb.ServerAddress, ...) instead of (pb.ServerAddress, ...) - filerVolumeProvider references FilerClient for failover access - LookupVolumeIds tries all filers with util.Retry pattern - Mount passes all option.FilerAddresses for HA - S3 wraps single filer in slice for API consistency This restores the high availability that existed in the old implementation where mount would automatically failover between configured filers. * fix: restore leader change detection in KeepConnected stream loop Critical fix: Leader change detection was accidentally removed from the streaming loop - Master can announce leader changes during an active KeepConnected stream - Without this check, client continues talking to non-leader until connection breaks - This can lead to stale data or operational errors The check needs to be in TWO places: 1. Initial response (lines 178-187): Detect redirect on first connect 2. Stream loop (lines 203-209): Detect leader changes during active stream Restored the loop check that was accidentally removed during refactoring. This ensures the client immediately reconnects to new leader when announced. * improve: address code review findings on error handling and documentation 1. Master provider now preserves per-volume errors - Surface detailed errors from master (e.g., misconfiguration, deletion) - Return partial results with aggregated errors using errors.Join - Callers can now distinguish specific volume failures from general errors - Addresses issue of losing vidLoc.Error details 2. Document GetMaster initialization contract - Add comprehensive documentation explaining blocking behavior - Clarify that KeepConnectedToMaster must be started first - Provide typical initialization pattern example - Prevent confusing timeouts during warm-up 3. Document partial results API contract - LookupVolumeIdsWithFallback explicitly documents partial results - Clear examples of how to handle result + error combinations - Helps prevent callers from discarding valid partial results 4. Add safeguards to legacy filer.LookupFn - Add deprecation warning with migration guidance - Implement simple 10,000 entry cache limit - Log warning when limit reached - Recommend wdclient.FilerClient for new code - Prevents unbounded memory growth in long-running processes These changes improve API clarity and operational safety while maintaining backward compatibility. * fix: handle partial results correctly in LookupVolumeIdsWithFallback callers Two callers were discarding partial results by checking err before processing the result map. While these are currently single-volume lookups (so partial results aren't possible), the code was fragile and would break if we ever batched multiple volumes together. Changes: - Check result map FIRST, then conditionally check error - If volume is found in result, use it (ignore errors about other volumes) - If volume is NOT found and err != nil, include error context with %w - Add defensive comments explaining the pattern for future maintainers This makes the code: 1. Correct for future batched lookups 2. More informative (preserves underlying error details) 3. Consistent with filer_grpc_server.go which already handles this correctly Example: If looking up ["1", "2", "999"] and only 999 fails, callers looking for volumes 1 or 2 will succeed instead of failing unnecessarily. * improve: address remaining code review findings 1. Lazy initialize FilerClient in mount for proxy-only setups - Only create FilerClient when VolumeServerAccess != "filerProxy" - Avoids wasted work when all reads proxy through filer - filerClient is nil for proxy mode, initialized for direct access 2. Fix inaccurate deprecation comment in filer.LookupFn - Updated comment to reflect current behavior (10k bounded cache) - Removed claim of "unbounded growth" after adding size limit - Still directs new code to wdclient.FilerClient for better features 3. Audit all MasterClient usages for KeepConnectedToMaster - Verified all production callers start KeepConnectedToMaster early - Filer, Shell, Master, Broker, Benchmark, Admin all correct - IAM creates MasterClient but never uses it (harmless) - Test code doesn't need KeepConnectedToMaster (mocks) All callers properly follow the initialization pattern documented in GetMaster(), preventing unexpected blocking or timeouts. * fix: restore observability instrumentation in MasterClient During the refactoring, several important stats counters and logging statements were accidentally removed from tryConnectToMaster. These are critical for monitoring and debugging the health of master client connections. Restored instrumentation: 1. stats.MasterClientConnectCounter("total") - tracks all connection attempts 2. stats.MasterClientConnectCounter(FailedToKeepConnected) - when KeepConnected stream fails 3. stats.MasterClientConnectCounter(FailedToReceive) - when Recv() fails in loop 4. stats.MasterClientConnectCounter(Failed) - when overall gprcErr occurs 5. stats.MasterClientConnectCounter(OnPeerUpdate) - when peer updates detected Additionally restored peer update logging: - "+ filer@host noticed group.type address" for node additions - "- filer@host noticed group.type address" for node removals - Only logs updates matching the client's FilerGroup for noise reduction This information is valuable for: - Monitoring cluster health and connection stability - Debugging cluster membership changes - Tracking master failover and reconnection patterns - Identifying network issues between clients and masters No functional changes - purely observability restoration. * improve: implement gRPC-aware retry for FilerClient volume lookups The previous implementation used util.Retry which only retries errors containing the string "transport". This is insufficient for handling the full range of transient gRPC errors. Changes: 1. Added isRetryableGrpcError() to properly inspect gRPC status codes - Retries: Unavailable, DeadlineExceeded, ResourceExhausted, Aborted - Falls back to string matching for non-gRPC network errors 2. Replaced util.Retry with custom retry loop - 3 attempts with exponential backoff (1s, 1.5s, 2.25s) - Tries all N filers on each attempt (N*3 total attempts max) - Fast-fails on non-retryable errors (NotFound, PermissionDenied, etc.) 3. Improved logging - Shows both filer attempt (x/N) and retry attempt (y/3) - Logs retry reason and wait time for debugging Benefits: - Better handling of transient gRPC failures (server restarts, load spikes) - Faster failure for permanent errors (no wasted retries) - More informative logs for troubleshooting - Maintains existing HA failover across multiple filers Example: If all 3 filers return Unavailable (server overload): - Attempt 1: try all 3 filers, wait 1s - Attempt 2: try all 3 filers, wait 1.5s - Attempt 3: try all 3 filers, fail Example: If filer returns NotFound (volume doesn't exist): - Attempt 1: try all 3 filers, fast-fail (no retry) * fmt * improve: add circuit breaker to skip known-unhealthy filers The previous implementation tried all filers on every failure, including known-unhealthy ones. This wasted time retrying permanently down filers. Problem scenario (3 filers, filer0 is down): - Last successful: filer1 (saved as filerIndex=1) - Next lookup when filer1 fails: Retry 1: filer1(fail) → filer2(fail) → filer0(fail, wastes 5s timeout) Retry 2: filer1(fail) → filer2(fail) → filer0(fail, wastes 5s timeout) Retry 3: filer1(fail) → filer2(fail) → filer0(fail, wastes 5s timeout) Total wasted: 15 seconds on known-bad filer! Solution: Circuit breaker pattern - Track consecutive failures per filer (atomic int32) - Skip filers with 3+ consecutive failures - Re-check unhealthy filers every 30 seconds - Reset failure count on success New behavior: - filer0 fails 3 times → marked unhealthy - Future lookups skip filer0 for 30 seconds - After 30s, re-check filer0 (allows recovery) - If filer0 succeeds, reset failure count to 0 Benefits: 1. Avoids wasting time on known-down filers 2. Still sticks to last healthy filer (via filerIndex) 3. Allows recovery (30s re-check window) 4. No configuration needed (automatic) Implementation details: - filerHealth struct tracks failureCount (atomic) + lastFailureTime - shouldSkipUnhealthyFiler(): checks if we should skip this filer - recordFilerSuccess(): resets failure count to 0 - recordFilerFailure(): increments count, updates timestamp - Logs when skipping unhealthy filers (V(2) level) Example with circuit breaker: - filer0 down, saved filerIndex=1 (filer1 healthy) - Lookup 1: filer1(ok) → Done (0.01s) - Lookup 2: filer1(fail) → filer2(ok) → Done, save filerIndex=2 (0.01s) - Lookup 3: filer2(fail) → skip filer0 (unhealthy) → filer1(ok) → Done (0.01s) Much better than wasting 15s trying filer0 repeatedly! * fix: OnPeerUpdate should only process updates for matching FilerGroup Critical bug: The OnPeerUpdate callback was incorrectly moved outside the FilerGroup check when restoring observability instrumentation. This caused clients to process peer updates for ALL filer groups, not just their own. Problem: Before: mc.OnPeerUpdate only called for update.FilerGroup == mc.FilerGroup Bug: mc.OnPeerUpdate called for ALL updates regardless of FilerGroup Impact: - Multi-tenant deployments with separate filer groups would see cross-group updates (e.g., group A clients processing group B updates) - Could cause incorrect cluster membership tracking - OnPeerUpdate handlers (like Filer's DLM ring updates) would receive irrelevant updates from other groups Example scenario: Cluster has two filer groups: "production" and "staging" Production filer connects with FilerGroup="production" Incorrect behavior (bug): - Receives "staging" group updates - Incorrectly adds staging filers to production DLM ring - Cross-tenant data access issues Correct behavior (fixed): - Only receives "production" group updates - Only adds production filers to production DLM ring - Proper isolation between groups Fix: Moved mc.OnPeerUpdate(update, time.Now()) back INSIDE the FilerGroup check where it belongs, matching the original implementation. The logging and stats counter were already correctly scoped to matching FilerGroup, so they remain inside the if block as intended. * improve: clarify Aborted error handling in volume lookups Added documentation and logging to address the concern that codes.Aborted might not always be retryable in all contexts. Context-specific justification for treating Aborted as retryable: Volume location lookups (LookupVolume RPC) are simple, read-only operations: - No transactions - No write conflicts - No application-level state changes - Idempotent (safe to retry) In this context, Aborted is most likely caused by: - Filer restarting/recovering (transient) - Connection interrupted mid-request (transient) - Server-side resource cleanup (transient) NOT caused by: - Application-level conflicts (no writes) - Transaction failures (no transactions) - Logical errors (read-only lookup) Changes: 1. Added detailed comment explaining the context-specific reasoning 2. Added V(1) logging when treating Aborted as retryable - Helps detect misclassification if it occurs - Visible in verbose logs for troubleshooting 3. Split switch statement for clarity (one case per line) If future analysis shows Aborted should not be retried, operators will now have visibility via logs to make that determination. The logging provides evidence for future tuning decisions. Alternative approaches considered but not implemented: - Removing Aborted entirely (too conservative for read-only ops) - Message content inspection (adds complexity, no known patterns yet) - Different handling per RPC type (premature optimization) * fix: IAM server must start KeepConnectedToMaster for masterClient usage The IAM server creates and uses a MasterClient but never started KeepConnectedToMaster, which could cause blocking if IAM config files have chunks requiring volume lookups. Problem flow: NewIamApiServerWithStore() → creates masterClient → ❌ NEVER starts KeepConnectedToMaster GetS3ApiConfigurationFromFiler() → filer.ReadEntry(iama.masterClient, ...) → StreamContent(masterClient, ...) if file has chunks → masterClient.GetLookupFileIdFunction() → GetMaster(ctx) ← BLOCKS indefinitely waiting for connection! While IAM config files (identity & policies) are typically small and stored inline without chunks, the code path exists and would block if the files ever had chunks. Fix: Start KeepConnectedToMaster in background goroutine right after creating masterClient, following the documented pattern: mc := wdclient.NewMasterClient(...) go mc.KeepConnectedToMaster(ctx) This ensures masterClient is usable if ReadEntry ever needs to stream chunked content from volume servers. Note: This bug was dormant because IAM config files are small (<256 bytes) and SeaweedFS stores small files inline in Entry.Content, not as chunks. The bug would only manifest if: - IAM config grew > 256 bytes (inline threshold) - Config was stored as chunks on volume servers - ReadEntry called StreamContent - GetMaster blocked indefinitely Now all 9 production MasterClient instances correctly follow the pattern. * fix: data race on filerHealth.lastFailureTime in circuit breaker The circuit breaker tracked lastFailureTime as time.Time, which was written in recordFilerFailure and read in shouldSkipUnhealthyFiler without synchronization, causing a data race. Data race scenario: Goroutine 1: recordFilerFailure(0) health.lastFailureTime = time.Now() // ❌ unsynchronized write Goroutine 2: shouldSkipUnhealthyFiler(0) time.Since(health.lastFailureTime) // ❌ unsynchronized read → RACE DETECTED by -race detector Fix: Changed lastFailureTime from time.Time to int64 (lastFailureTimeNs) storing Unix nanoseconds for atomic access: Write side (recordFilerFailure): atomic.StoreInt64(&health.lastFailureTimeNs, time.Now().UnixNano()) Read side (shouldSkipUnhealthyFiler): lastFailureNs := atomic.LoadInt64(&health.lastFailureTimeNs) if lastFailureNs == 0 { return false } // Never failed lastFailureTime := time.Unix(0, lastFailureNs) time.Since(lastFailureTime) > 30*time.Second Benefits: - Atomic reads/writes (no data race) - Efficient (int64 is 8 bytes, always atomic on 64-bit systems) - Zero value (0) naturally means "never failed" - No mutex needed (lock-free circuit breaker) Note: sync/atomic was already imported for failureCount, so no new import needed. * fix: create fresh timeout context for each filer retry attempt The timeout context was created once at function start and reused across all retry attempts, causing subsequent retries to run with progressively shorter (or expired) deadlines. Problem flow: Line 244: timeoutCtx, cancel := context.WithTimeout(ctx, 5s) defer cancel() Retry 1, filer 0: client.LookupVolume(timeoutCtx, ...) ← 5s available ✅ Retry 1, filer 1: client.LookupVolume(timeoutCtx, ...) ← 3s left Retry 1, filer 2: client.LookupVolume(timeoutCtx, ...) ← 0.5s left Retry 2, filer 0: client.LookupVolume(timeoutCtx, ...) ← EXPIRED! ❌ Result: Retries always fail with DeadlineExceeded, defeating the purpose of retries. Fix: Moved context.WithTimeout inside the per-filer loop, creating a fresh timeout context for each attempt: for x := 0; x < n; x++ { timeoutCtx, cancel := context.WithTimeout(ctx, fc.grpcTimeout) err := pb.WithGrpcFilerClient(..., func(client) { resp, err := client.LookupVolume(timeoutCtx, ...) ... }) cancel() // Clean up immediately after call } Benefits: - Each filer attempt gets full fc.grpcTimeout (default 5s) - Retries actually have time to complete - No context leaks (cancel called after each attempt) - More predictable timeout behavior Example with fix: Retry 1, filer 0: fresh 5s timeout ✅ Retry 1, filer 1: fresh 5s timeout ✅ Retry 2, filer 0: fresh 5s timeout ✅ Total max time: 3 retries × 3 filers × 5s = 45s (plus backoff) Note: The outer ctx (from caller) still provides overall cancellation if the caller cancels or times out the entire operation. * fix: always reset vidMap cache on master reconnection The previous refactoring removed the else block that resets vidMap when the first message from a newly connected master is not a VolumeLocation. Problem scenario: 1. Client connects to master-1 and builds vidMap cache 2. Master-1 fails, client connects to master-2 3. First message from master-2 is a ClusterNodeUpdate (not VolumeLocation) 4. Old code: vidMap is reset and updated ✅ 5. New code: vidMap is NOT reset ❌ 6. Result: Client uses stale cache from master-1 → data access errors Example flow with bug: Connect to master-2 First message: ClusterNodeUpdate {filer.x added} → No resetVidMap() call → vidMap still has master-1's stale volume locations → Client reads from wrong volume servers → 404 errors Fix: Restored the else block that resets vidMap when first message is not a VolumeLocation: if resp.VolumeLocation != nil { // ... check leader, reset, and update ... } else { // First message is ClusterNodeUpdate or other type // Must still reset to avoid stale data mc.resetVidMap() } This ensures the cache is always cleared when establishing a new master connection, regardless of what the first message type is. Root cause: During the vidMapClient refactoring, this else block was accidentally dropped, making failover behavior fragile and non-deterministic (depends on which message type arrives first from the new master). Impact: - High severity for master failover scenarios - Could cause read failures, 404s, or wrong data access - Only manifests when first message is not VolumeLocation * fix: goroutine and connection leak in IAM server shutdown The IAM server's KeepConnectedToMaster goroutine used context.Background(), which is non-cancellable, causing the goroutine and its gRPC connections to leak on server shutdown. Problem: go masterClient.KeepConnectedToMaster(context.Background()) - context.Background() never cancels - KeepConnectedToMaster goroutine runs forever - gRPC connection to master stays open - No way to stop cleanly on server shutdown Result: Resource leaks when IAM server is stopped Fix: 1. Added shutdownContext and shutdownCancel to IamApiServer struct 2. Created cancellable context in NewIamApiServerWithStore: shutdownCtx, shutdownCancel := context.WithCancel(context.Background()) 3. Pass shutdownCtx to KeepConnectedToMaster: go masterClient.KeepConnectedToMaster(shutdownCtx) 4. Added Shutdown() method to invoke cancel: func (iama *IamApiServer) Shutdown() { if iama.shutdownCancel != nil { iama.shutdownCancel() } } 5. Stored masterClient reference on IamApiServer for future use Benefits: - Goroutine stops cleanly when Shutdown() is called - gRPC connections are closed properly - No resource leaks on server restart/stop - Shutdown() is idempotent (safe to call multiple times) Usage (for future graceful shutdown): iamServer, _ := iamapi.NewIamApiServer(...) defer iamServer.Shutdown() // or in signal handler: sigChan := make(chan os.Signal, 1) signal.Notify(sigChan, syscall.SIGTERM, syscall.SIGINT) go func() { <-sigChan iamServer.Shutdown() os.Exit(0) }() Note: Current command implementations (weed/command/iam.go) don't have shutdown paths yet, but this makes IAM server ready for proper lifecycle management when that infrastructure is added. * refactor: remove unnecessary KeepMasterClientConnected wrapper in filer The Filer.KeepMasterClientConnected() method was an unnecessary wrapper that just forwarded to MasterClient.KeepConnectedToMaster(). This wrapper added no value and created inconsistency with other components that call KeepConnectedToMaster directly. Removed: filer.go:178-180 func (fs *Filer) KeepMasterClientConnected(ctx context.Context) { fs.MasterClient.KeepConnectedToMaster(ctx) } Updated caller: filer_server.go:181 - go fs.filer.KeepMasterClientConnected(context.Background()) + go fs.filer.MasterClient.KeepConnectedToMaster(context.Background()) Benefits: - Consistent with other components (S3, IAM, Shell, Mount) - Removes unnecessary indirection - Clearer that KeepConnectedToMaster runs in background goroutine - Follows the documented pattern from MasterClient.GetMaster() Note: shell/commands.go was verified and already correctly starts KeepConnectedToMaster in a background goroutine (shell_liner.go:51): go commandEnv.MasterClient.KeepConnectedToMaster(ctx) * fix: use client ID instead of timeout for gRPC signature parameter The pb.WithGrpcFilerClient signature parameter is meant to be a client identifier for logging and tracking (added as 'sw-client-id' gRPC metadata in streaming mode), not a timeout value. Problem: timeoutMs := int32(fc.grpcTimeout.Milliseconds()) // 5000 (5 seconds) err := pb.WithGrpcFilerClient(false, timeoutMs, filerAddress, ...) - Passing timeout (5000ms) as signature/client ID - Misuse of API: signature should be a unique client identifier - Timeout is already handled by timeoutCtx passed to gRPC call - Inconsistent with other callers (all use 0 or proper client ID) How WithGrpcFilerClient uses signature parameter: func WithGrpcClient(..., signature int32, ...) { if streamingMode && signature != 0 { md := metadata.New(map[string]string{"sw-client-id": fmt.Sprintf("%d", signature)}) ctx = metadata.NewOutgoingContext(ctx, md) } ... } It's for client identification, not timeout control! Fix: 1. Added clientId int32 field to FilerClient struct 2. Initialize with rand.Int31() in NewFilerClient for unique ID 3. Removed timeoutMs variable (and misleading comment) 4. Use fc.clientId in pb.WithGrpcFilerClient call Before: err := pb.WithGrpcFilerClient(false, timeoutMs, ...) ^^^^^^^^^ Wrong! (5000) After: err := pb.WithGrpcFilerClient(false, fc.clientId, ...) ^^^^^^^^^^^^ Correct! (random int31) Benefits: - Correct API usage (signature = client ID, not timeout) - Timeout still works via timeoutCtx (unchanged) - Consistent with other pb.WithGrpcFilerClient callers - Enables proper client tracking on filer side via gRPC metadata - Each FilerClient instance has unique ID for debugging Examples of correct usage elsewhere: weed/iamapi/iamapi_server.go:145 pb.WithGrpcFilerClient(false, 0, ...) weed/command/s3.go:215 pb.WithGrpcFilerClient(false, 0, ...) weed/shell/commands.go:110 pb.WithGrpcFilerClient(streamingMode, 0, ...) All use 0 (or a proper signature), not a timeout value. * fix: add timeout to master volume lookup to prevent indefinite blocking The masterVolumeProvider.LookupVolumeIds method was using the context directly without a timeout, which could cause it to block indefinitely if the master is slow to respond or unreachable. Problem: err := pb.WithMasterClient(false, p.masterClient.GetMaster(ctx), ...) resp, err := client.LookupVolume(ctx, &master_pb.LookupVolumeRequest{...}) - No timeout on gRPC call to master - Could block indefinitely if master is unresponsive - Inconsistent with FilerClient which uses 5s timeout - This is a fallback path (cache miss) but still needs protection Scenarios where this could hang: 1. Master server under heavy load (slow response) 2. Network issues between client and master 3. Master server hung or deadlocked 4. Master in process of shutting down Fix: timeoutCtx, cancel := context.WithTimeout(ctx, 5*time.Second) defer cancel() err := pb.WithMasterClient(false, p.masterClient.GetMaster(timeoutCtx), ...) resp, err := client.LookupVolume(timeoutCtx, &master_pb.LookupVolumeRequest{...}) Benefits: - Prevents indefinite blocking on master lookup - Consistent with FilerClient timeout pattern (5 seconds) - Faster failure detection when master is unresponsive - Caller's context still honored (timeout is in addition, not replacement) - Improves overall system resilience Note: 5 seconds is a reasonable default for volume lookups: - Long enough for normal master response (~10-50ms) - Short enough to fail fast on issues - Matches FilerClient's grpcTimeout default * purge * refactor: address code review feedback on comments and style Fixed several code quality issues identified during review: 1. Corrected backoff algorithm description in filer_client.go: - Changed "Exponential backoff" to "Multiplicative backoff with 1.5x factor" - The formula waitTime * 3/2 produces 1s, 1.5s, 2.25s, not exponential 2^n - More accurate terminology prevents confusion 2. Removed redundant nil check in vidmap_client.go: - After the for loop, node is guaranteed to be non-nil - Loop either returns early or assigns non-nil value to node - Simplified: if node != nil { node.cache.Store(nil) } → node.cache.Store(nil) 3. Added startup logging to IAM server for consistency: - Log when master client connection starts - Matches pattern in S3ApiServer (line 100 in s3api_server.go) - Improves operational visibility during startup - Added missing glog import 4. Fixed indentation in filer/reader_at.go: - Lines 76-91 had incorrect indentation (extra tab level) - Line 93 also misaligned - Now properly aligned with surrounding code 5. Updated deprecation comment to follow Go convention: - Changed "DEPRECATED:" to "Deprecated:" (standard Go format) - Tools like staticcheck and IDEs recognize the standard format - Enables automated deprecation warnings in tooling - Better developer experience All changes are cosmetic and do not affect functionality. * fmt * refactor: make circuit breaker parameters configurable in FilerClient The circuit breaker failure threshold (3) and reset timeout (30s) were hardcoded, making it difficult to tune the client's behavior in different deployment environments without modifying the code. Problem: func shouldSkipUnhealthyFiler(index int32) bool { if failureCount < 3 { // Hardcoded threshold return false } if time.Since(lastFailureTime) > 30*time.Second { // Hardcoded timeout return false } } Different environments have different needs: - High-traffic production: may want lower threshold (2) for faster failover - Development/testing: may want higher threshold (5) to tolerate flaky networks - Low-latency services: may want shorter reset timeout (10s) - Batch processing: may want longer reset timeout (60s) Solution: 1. Added fields to FilerClientOption: - FailureThreshold int32 (default: 3) - ResetTimeout time.Duration (default: 30s) 2. Added fields to FilerClient: - failureThreshold int32 - resetTimeout time.Duration 3. Applied defaults in NewFilerClient with option override: failureThreshold := int32(3) resetTimeout := 30 * time.Second if opt.FailureThreshold > 0 { failureThreshold = opt.FailureThreshold } if opt.ResetTimeout > 0 { resetTimeout = opt.ResetTimeout } 4. Updated shouldSkipUnhealthyFiler to use configurable values: if failureCount < fc.failureThreshold { ... } if time.Since(lastFailureTime) > fc.resetTimeout { ... } Benefits: ✓ Tunable for different deployment environments ✓ Backward compatible (defaults match previous hardcoded values) ✓ No breaking changes to existing code ✓ Better maintainability and flexibility Example usage: // Aggressive failover for low-latency production fc := wdclient.NewFilerClient(filers, dialOpt, dc, &wdclient.FilerClientOption{ FailureThreshold: 2, ResetTimeout: 10 * time.Second, }) // Tolerant of flaky networks in development fc := wdclient.NewFilerClient(filers, dialOpt, dc, &wdclient.FilerClientOption{ FailureThreshold: 5, ResetTimeout: 60 * time.Second, }) * retry parameters * refactor: make retry and timeout parameters configurable Made retry logic and gRPC timeouts configurable across FilerClient and MasterClient to support different deployment environments and network conditions. Problem 1: Hardcoded retry parameters in FilerClient waitTime := time.Second // Fixed at 1s maxRetries := 3 // Fixed at 3 attempts waitTime = waitTime * 3 / 2 // Fixed 1.5x multiplier Different environments have different needs: - Unstable networks: may want more retries (5) with longer waits (2s) - Low-latency production: may want fewer retries (2) with shorter waits (500ms) - Batch processing: may want exponential backoff (2x) instead of 1.5x Problem 2: Hardcoded gRPC timeout in MasterClient timeoutCtx, cancel := context.WithTimeout(ctx, 5*time.Second) Master lookups may need different timeouts: - High-latency cross-region: may need 10s timeout - Local network: may use 2s timeout for faster failure detection Solution for FilerClient: 1. Added fields to FilerClientOption: - MaxRetries int (default: 3) - InitialRetryWait time.Duration (default: 1s) - RetryBackoffFactor float64 (default: 1.5) 2. Added fields to FilerClient: - maxRetries int - initialRetryWait time.Duration - retryBackoffFactor float64 3. Updated LookupVolumeIds to use configurable values: waitTime := fc.initialRetryWait maxRetries := fc.maxRetries for retry := 0; retry < maxRetries; retry++ { ... waitTime = time.Duration(float64(waitTime) * fc.retryBackoffFactor) } Solution for MasterClient: 1. Added grpcTimeout field to MasterClient (default: 5s) 2. Initialize in NewMasterClient with 5 * time.Second default 3. Updated masterVolumeProvider to use p.masterClient.grpcTimeout Benefits: ✓ Tunable for different network conditions and deployment scenarios ✓ Backward compatible (defaults match previous hardcoded values) ✓ No breaking changes to existing code ✓ Consistent configuration pattern across FilerClient and MasterClient Example usage: // Fast-fail for low-latency production with stable network fc := wdclient.NewFilerClient(filers, dialOpt, dc, &wdclient.FilerClientOption{ MaxRetries: 2, InitialRetryWait: 500 * time.Millisecond, RetryBackoffFactor: 2.0, // Exponential backoff GrpcTimeout: 2 * time.Second, }) // Patient retries for unstable network or batch processing fc := wdclient.NewFilerClient(filers, dialOpt, dc, &wdclient.FilerClientOption{ MaxRetries: 5, InitialRetryWait: 2 * time.Second, RetryBackoffFactor: 1.5, GrpcTimeout: 10 * time.Second, }) Note: MasterClient timeout is currently set at construction time and not user-configurable via NewMasterClient parameters. Future enhancement could add a MasterClientOption struct similar to FilerClientOption. * fix: rename vicCacheLock to vidCacheLock for consistency Fixed typo in variable name for better code consistency and readability. Problem: vidCache := make(map[string]*filer_pb.Locations) var vicCacheLock sync.RWMutex // Typo: vic instead of vid vicCacheLock.RLock() locations, found := vidCache[vid] vicCacheLock.RUnlock() The variable name 'vicCacheLock' is inconsistent with 'vidCache'. Both should use 'vid' prefix (volume ID) not 'vic'. Fix: Renamed all 5 occurrences: - var vicCacheLock → var vidCacheLock (line 56) - vicCacheLock.RLock() → vidCacheLock.RLock() (line 62) - vicCacheLock.RUnlock() → vidCacheLock.RUnlock() (line 64) - vicCacheLock.Lock() → vidCacheLock.Lock() (line 81) - vicCacheLock.Unlock() → vidCacheLock.Unlock() (line 91) Benefits: ✓ Consistent variable naming convention ✓ Clearer intent (volume ID cache lock) ✓ Better code readability ✓ Easier code navigation * fix: use defer cancel() with anonymous function for proper context cleanup Fixed context cancellation to use defer pattern correctly in loop iteration. Problem: for x := 0; x < n; x++ { timeoutCtx, cancel := context.WithTimeout(ctx, fc.grpcTimeout) err := pb.WithGrpcFilerClient(...) cancel() // Only called on normal return, not on panic } Issues with original approach: 1. If pb.WithGrpcFilerClient panics, cancel() is never called → context leak 2. If callback returns early (though unlikely here), cleanup might be missed 3. Not following Go best practices for context.WithTimeout usage Problem with naive defer in loop: for x := 0; x < n; x++ { timeoutCtx, cancel := context.WithTimeout(ctx, fc.grpcTimeout) defer cancel() // ❌ WRONG: All defers accumulate until function returns } In Go, defer executes when the surrounding *function* returns, not when the loop iteration ends. This would accumulate n deferred cancel() calls and leak contexts until LookupVolumeIds returns. Solution: Wrap in anonymous function for x := 0; x < n; x++ { err := func() error { timeoutCtx, cancel := context.WithTimeout(ctx, fc.grpcTimeout) defer cancel() // ✅ Executes when anonymous function returns (per iteration) return pb.WithGrpcFilerClient(...) }() } Benefits: ✓ Context always cancelled, even on panic ✓ defer executes after each iteration (not accumulated) ✓ Follows Go best practices for context.WithTimeout ✓ No resource leaks during retry loop execution ✓ Cleaner error handling Reference: Go documentation for context.WithTimeout explicitly shows: ctx, cancel := context.WithTimeout(...) defer cancel() This is the idiomatic pattern that should always be followed. * Can't use defer directly in loop * improve: add data center preference and URL shuffling for consistent performance Added missing data center preference and load distribution (URL shuffling) to ensure consistent performance and behavior across all code paths. Problem 1: PreferPublicUrl path missing DC preference and shuffling Location: weed/wdclient/filer_client.go lines 184-192 The custom PreferPublicUrl implementation was simply iterating through locations and building URLs without considering: 1. Data center proximity (latency optimization) 2. Load distribution across volume servers Before: for _, loc := range locations { url := loc.PublicUrl if url == "" { url = loc.Url } fullUrls = append(fullUrls, "http://"+url+"/"+fileId) } return fullUrls, nil After: var sameDcUrls, otherDcUrls []string dataCenter := fc.GetDataCenter() for _, loc := range locations { url := loc.PublicUrl if url == "" { url = loc.Url } httpUrl := "http://" + url + "/" + fileId if dataCenter != "" && dataCenter == loc.DataCenter { sameDcUrls = append(sameDcUrls, httpUrl) } else { otherDcUrls = append(otherDcUrls, httpUrl) } } rand.Shuffle(len(sameDcUrls), ...) rand.Shuffle(len(otherDcUrls), ...) fullUrls = append(sameDcUrls, otherDcUrls...) Problem 2: Cache miss path missing URL shuffling Location: weed/wdclient/vidmap_client.go lines 95-108 The cache miss path (fallback lookup) was missing URL shuffling, while the cache hit path (vm.LookupFileId) already shuffles URLs. This inconsistency meant: - Cache hit: URLs shuffled → load distributed - Cache miss: URLs not shuffled → first server always hit Before: var sameDcUrls, otherDcUrls []string // ... build URLs ... fullUrls = append(sameDcUrls, otherDcUrls...) return fullUrls, nil After: var sameDcUrls, otherDcUrls []string // ... build URLs ... rand.Shuffle(len(sameDcUrls), ...) rand.Shuffle(len(otherDcUrls), ...) fullUrls = append(sameDcUrls, otherDcUrls...) return fullUrls, nil Benefits: ✓ Reduced latency by preferring same-DC volume servers ✓ Even load distribution across all volume servers ✓ Consistent behavior between cache hit/miss paths ✓ Consistent behavior between PreferUrl and PreferPublicUrl ✓ Matches behavior of existing vidMap.LookupFileId implementation Impact on performance: - Lower read latency (same-DC preference) - Better volume server utilization (load spreading) - No single volume server becomes a hotspot Note: Added math/rand import to vidmap_client.go for shuffle support. * Update weed/wdclient/masterclient.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * improve: call IAM server Shutdown() for best-effort cleanup Added call to iamApiServer.Shutdown() to ensure cleanup happens when possible, and documented the limitations of the current approach. Problem: The Shutdown() method was defined in IamApiServer but never called anywhere, meaning the KeepConnectedToMaster goroutine would continue running even when the IAM server stopped, causing resource leaks. Changes: 1. Store iamApiServer instance in weed/command/iam.go - Changed: _, iamApiServer_err := iamapi.NewIamApiServer(...) - To: iamApiServer, iamApiServer_err := iamapi.NewIamApiServer(...) 2. Added defer call for best-effort cleanup - defer iamApiServer.Shutdown() - This will execute if startIamServer() returns normally 3. Added logging in Shutdown() method - Log when shutdown is triggered for visibility 4. Documented limitations and future improvements - Added note that defer only works for normal function returns - SeaweedFS commands don't currently have signal handling - Suggested future enhancement: add SIGTERM/SIGINT handling Current behavior: - ✓ Cleanup happens if HTTP server fails to start (glog.Fatalf path) - ✓ Cleanup happens if Serve() returns with error (unlikely) - ✗ Cleanup does NOT happen on SIGTERM/SIGINT (process killed) The last case is a limitation of the current command architecture - all SeaweedFS commands (s3, filer, volume, master, iam) lack signal handling for graceful shutdown. This is a systemic issue that affects all services. Future enhancement: To properly handle SIGTERM/SIGINT, the command layer would need: sigChan := make(chan os.Signal, 1) signal.Notify(sigChan, syscall.SIGTERM, syscall.SIGINT) go func() { httpServer.Serve(listener) // Non-blocking }() <-sigChan glog.V(0).Infof("Received shutdown signal") iamApiServer.Shutdown() httpServer.Shutdown(context.Background()) This would require refactoring the command structure for all services, which is out of scope for this change. Benefits of current approach: ✓ Best-effort cleanup (better than nothing) ✓ Proper cleanup in error paths ✓ Documented for future improvement ✓ Consistent with how other SeaweedFS services handle lifecycle * data racing in test --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>	2 weeks ago
Chris Lu	5f77f87335	S3: S3 Object Retention API to include XML namespace support (#7517) * Refactor S3 Object Retention API to include XML namespace support and improve compatibility with Veeam. Updated XML tags to remove hardcoded namespaces and added test cases for retention and legal hold configurations without namespaces. * Added XMLNS field setting in both places	2 weeks ago
Chris Lu	6281e62d7f	S3: JWT generation for volume server authentication (#7514) * Refactor JWT generation for volume server authentication to use centralized function from filer package, improving code clarity and reducing redundancy. * Update s3api_object_handlers.go	2 weeks ago
Chris Lu	ca84a8a713	S3: Directly read write volume servers (#7481) * Lazy Versioning Check, Conditional SSE Entry Fetch, HEAD Request Optimization * revert Reverted the conditional versioning check to always check versioning status Reverted the conditional SSE entry fetch to always fetch entry metadata Reverted the conditional versioning check to always check versioning status Reverted the conditional SSE entry fetch to always fetch entry metadata * Lazy Entry Fetch for SSE, Skip Conditional Header Check * SSE-KMS headers are present, this is not an SSE-C request (mutually exclusive) * SSE-C is mutually exclusive with SSE-S3 and SSE-KMS * refactor * Removed Premature Mutual Exclusivity Check * check for the presence of the X-Amz-Server-Side-Encryption header * not used * fmt * directly read write volume servers * HTTP Range Request Support * set header * md5 * copy object * fix sse * fmt * implement sse * sse continue * fixed the suffix range bug (bytes=-N for "last N bytes") * debug logs * Missing PartsCount Header * profiling * url encoding * test_multipart_get_part * headers * debug * adjust log level * handle part number * Update s3api_object_handlers.go * nil safety * set ModifiedTsNs * remove * nil check * fix sse header * same logic as filer * decode values * decode ivBase64 * s3: Fix SSE decryption JWT authentication and streaming errors Critical fix for SSE (Server-Side Encryption) test failures: 1. **JWT Authentication Bug** (Root Cause): - Changed from GenJwtForFilerServer to GenJwtForVolumeServer - S3 API now uses correct JWT when directly reading from volume servers - Matches filer's authentication pattern for direct volume access - Fixes 'unexpected EOF' and 500 errors in SSE tests 2. **Streaming Error Handling**: - Added error propagation in getEncryptedStreamFromVolumes goroutine - Use CloseWithError() to properly communicate stream failures - Added debug logging for streaming errors 3. **Response Header Timing**: - Removed premature WriteHeader(http.StatusOK) call - Let Go's http package write status automatically on first write - Prevents header lock when errors occur during streaming 4. **Enhanced SSE Decryption Debugging**: - Added IV/Key validation and logging for SSE-C, SSE-KMS, SSE-S3 - Better error messages for missing or invalid encryption metadata - Added glog.V(2) debugging for decryption setup This fixes SSE integration test failures where encrypted objects could not be retrieved due to volume server authentication failures. The JWT bug was causing volume servers to reject requests, resulting in truncated/empty streams (EOF) or internal errors. * s3: Fix SSE multipart upload metadata preservation Critical fix for SSE multipart upload test failures (SSE-C and SSE-KMS): **Root Cause - Incomplete SSE Metadata Copying**: The old code only tried to copy 'SeaweedFSSSEKMSKey' from the first part to the completed object. This had TWO bugs: 1. **Wrong Constant Name** (Key Mismatch Bug): - Storage uses: SeaweedFSSSEKMSKeyHeader = 'X-SeaweedFS-SSE-KMS-Key' - Old code read: SeaweedFSSSEKMSKey = 'x-seaweedfs-sse-kms-key' - Result: SSE-KMS metadata was NEVER copied → 500 errors 2. **Missing SSE-C and SSE-S3 Headers**: - SSE-C requires: IV, Algorithm, KeyMD5 - SSE-S3 requires: encrypted key data + standard headers - Old code: copied nothing for SSE-C/SSE-S3 → decryption failures **Fix - Complete SSE Header Preservation**: Now copies ALL SSE headers from first part to completed object: - SSE-C: SeaweedFSSSEIV, CustomerAlgorithm, CustomerKeyMD5 - SSE-KMS: SeaweedFSSSEKMSKeyHeader, AwsKmsKeyId, ServerSideEncryption - SSE-S3: SeaweedFSSSES3Key, ServerSideEncryption Applied consistently to all 3 code paths: 1. Versioned buckets (creates version file) 2. Suspended versioning (creates main object with null versionId) 3. Non-versioned buckets (creates main object) **Why This Is Correct**: The headers copied EXACTLY match what putToFiler stores during part upload (lines 496-521 in s3api_object_handlers_put.go). This ensures detectPrimarySSEType() can correctly identify encrypted multipart objects and trigger inline decryption with proper metadata. Fixes: TestSSEMultipartUploadIntegration (SSE-C and SSE-KMS subtests) * s3: Add debug logging for versioning state diagnosis Temporary debug logging to diagnose test_versioning_obj_plain_null_version_overwrite_suspended failure. Added glog.V(0) logging to show: 1. setBucketVersioningStatus: when versioning status is changed 2. PutObjectHandler: what versioning state is detected (Enabled/Suspended/none) 3. PutObjectHandler: which code path is taken (putVersionedObject vs putSuspendedVersioningObject) This will help identify if: - The versioning status is being set correctly in bucket config - The cache is returning stale/incorrect versioning state - The switch statement is correctly routing to suspended vs enabled handlers * s3: Enhanced versioning state tracing for suspended versioning diagnosis Added comprehensive logging across the entire versioning state flow: PutBucketVersioningHandler: - Log requested status (Enabled/Suspended) - Log when calling setBucketVersioningStatus - Log success/failure of status change setBucketVersioningStatus: - Log bucket and status being set - Log when config is updated - Log completion with error code updateBucketConfig: - Log versioning state being written to cache - Immediate cache verification after Set - Log if cache verification fails getVersioningState: - Log bucket name and state being returned - Log if object lock forces VersioningEnabled - Log errors This will reveal: 1. If PutBucketVersioning(Suspended) is reaching the handler 2. If the cache update succeeds 3. What state getVersioningState returns during PUT 4. Any cache consistency issues Expected to show why bucket still reports 'Enabled' after 'Suspended' call. * s3: Add SSE chunk detection debugging for multipart uploads Added comprehensive logging to diagnose why TestSSEMultipartUploadIntegration fails: detectPrimarySSEType now logs: 1. Total chunk count and extended header count 2. All extended headers with 'sse'/'SSE'/'encryption' in the name 3. For each chunk: index, SseType, and whether it has metadata 4. Final SSE type counts (SSE-C, SSE-KMS, SSE-S3) This will reveal if: - Chunks are missing SSE metadata after multipart completion - Extended headers are copied correctly from first part - The SSE detection logic is working correctly Expected to show if chunks have SseType=0 (none) or proper SSE types set. * s3: Trace SSE chunk metadata through multipart completion and retrieval Added end-to-end logging to track SSE chunk metadata lifecycle: **During Multipart Completion (filer_multipart.go)**: 1. Log finalParts chunks BEFORE mkFile - shows SseType and metadata 2. Log versionEntry.Chunks INSIDE mkFile callback - shows if mkFile preserves SSE info 3. Log success after mkFile completes **During GET Retrieval (s3api_object_handlers.go)**: 1. Log retrieved entry chunks - shows SseType and metadata after retrieval 2. Log detected SSE type result This will reveal at which point SSE chunk metadata is lost: - If finalParts have SSE metadata but versionEntry.Chunks don't → mkFile bug - If versionEntry.Chunks have SSE metadata but retrieved chunks don't → storage/retrieval bug - If chunks never have SSE metadata → multipart completion SSE processing bug Expected to show chunks with SseType=NONE during retrieval even though they were created with proper SseType during multipart completion. * s3: Fix SSE-C multipart IV base64 decoding bug **Critical Bug Found**: SSE-C multipart uploads were failing because: Root Cause: - entry.Extended[SeaweedFSSSEIV] stores base64-encoded IV (24 bytes for 16-byte IV) - SerializeSSECMetadata expects raw IV bytes (16 bytes) - During multipart completion, we were passing base64 IV directly → serialization error Error Message: "Failed to serialize SSE-C metadata for chunk in part X: invalid IV length: expected 16 bytes, got 24" Fix: - Base64-decode IV before passing to SerializeSSECMetadata - Added error handling for decode failures Impact: - SSE-C multipart uploads will now correctly serialize chunk metadata - Chunks will have proper SSE metadata for decryption during GET This fixes the SSE-C subtest of TestSSEMultipartUploadIntegration. SSE-KMS still has a separate issue (error code 23) being investigated. * fixes * kms sse * handle retry if not found in .versions folder and should read the normal object * quick check (no retries) to see if the .versions/ directory exists * skip retry if object is not found * explicit update to avoid sync delay * fix map update lock * Remove fmt.Printf debug statements * Fix SSE-KMS multipart base IV fallback to fail instead of regenerating * fmt * Fix ACL grants storage logic * header handling * nil handling * range read for sse content * test range requests for sse objects * fmt * unused code * upload in chunks * header case * fix url * bucket policy error vs bucket not found * jwt handling * fmt * jwt in request header * Optimize Case-Insensitive Prefix Check * dead code * Eliminated Unnecessary Stream Prefetch for Multipart SSE * range sse * sse * refactor * context * fmt * fix type * fix SSE-C IV Mismatch * Fix Headers Being Set After WriteHeader * fix url parsing * propergate sse headers * multipart sse-s3 * aws sig v4 authen * sse kms * set content range * better errors * Update s3api_object_handlers_copy.go * Update s3api_object_handlers.go * Update s3api_object_handlers.go * avoid magic number * clean up * Update s3api_bucket_policy_handlers.go * fix url parsing * context * data and metadata both use background context * adjust the offset * SSE Range Request IV Calculation * adjust logs * IV relative to offset in each part, not the whole file * collect logs * offset * fix offset * fix url * logs * variable * jwt * Multipart ETag semantics: conditionally set object-level Md5 for single-chunk uploads only. * sse * adjust IV and offset * multipart boundaries * ensures PUT and GET operations return consistent ETags * Metadata Header Case * CommonPrefixes Sorting with URL Encoding * always sort * remove the extra PathUnescape call * fix the multipart get part ETag * the FileChunk is created without setting ModifiedTsNs * Sort CommonPrefixes lexicographically to match AWS S3 behavior * set md5 for multipart uploads * prevents any potential data loss or corruption in the small-file inline storage path * compiles correctly * decryptedReader will now be properly closed after use * Fixed URL encoding and sort order for CommonPrefixes * Update s3api_object_handlers_list.go * SSE-x Chunk View Decryption * Different IV offset calculations for single-part vs multipart objects * still too verbose in logs * less logs * ensure correct conversion * fix listing * nil check * minor fixes * nil check * single character delimiter * optimize * range on empty object or zero-length * correct IV based on its position within that part, not its position in the entire object * adjust offset * offset Fetch FULL encrypted chunk (not just the range) Adjust IV by PartOffset/ChunkOffset only Decrypt full chunk Skip in the DECRYPTED stream to reach OffsetInChunk * look breaking * refactor * error on no content * handle intra-block byte skipping * Incomplete HTTP Response Error Handling * multipart SSE * Update s3api_object_handlers.go * address comments * less logs * handling directory * Optimized rejectDirectoryObjectWithoutSlash() to avoid unnecessary lookups * Revert "handling directory" This reverts commit 3a335f0ac3. * constant * Consolidate nil entry checks in GetObjectHandler * add range tests * Consolidate redundant nil entry checks in HeadObjectHandler * adjust logs * SSE type * large files * large files Reverted the plain-object range test * ErrNoEncryptionConfig * Fixed SSERangeReader Infinite Loop Vulnerability * Fixed SSE-KMS Multipart ChunkReader HTTP Body Leak * handle empty directory in S3, added PyArrow tests * purge unused code * Update s3_parquet_test.py * Update requirements.txt * According to S3 specifications, when both partNumber and Range are present, the Range should apply within the selected part's boundaries, not to the full object. * handle errors * errors after writing header * https * fix: Wait for volume assignment readiness before running Parquet tests The test-implicit-dir-with-server test was failing with an Internal Error because volume assignment was not ready when tests started. This fix adds a check that attempts a volume assignment and waits for it to succeed before proceeding with tests. This ensures that: 1. Volume servers are registered with the master 2. Volume growth is triggered if needed 3. The system can successfully assign volumes for writes Fixes the timeout issue where boto3 would retry 4 times and fail with 'We encountered an internal error, please try again.' * sse tests * store derived IV * fix: Clean up gRPC ports between tests to prevent port conflicts The second test (test-implicit-dir-with-server) was failing because the volume server's gRPC port (18080 = VOLUME_PORT + 10000) was still in use from the first test. The cleanup code only killed HTTP port processes, not gRPC port processes. Added cleanup for gRPC ports in all stop targets: - Master gRPC: MASTER_PORT + 10000 (19333) - Volume gRPC: VOLUME_PORT + 10000 (18080) - Filer gRPC: FILER_PORT + 10000 (18888) This ensures clean state between test runs in CI. * add import * address comments * docs: Add placeholder documentation files for Parquet test suite Added three missing documentation files referenced in test/s3/parquet/README.md: 1. TEST_COVERAGE.md - Documents 43 total test cases (17 Go unit tests, 6 Python integration tests, 20 Python end-to-end tests) 2. FINAL_ROOT_CAUSE_ANALYSIS.md - Explains the s3fs compatibility issue with PyArrow, the implicit directory problem, and how the fix works 3. MINIO_DIRECTORY_HANDLING.md - Compares MinIO's directory handling approach with SeaweedFS's implementation Each file contains: - Title and overview - Key technical details relevant to the topic - TODO sections for future expansion These placeholder files resolve the broken README links and provide structure for future detailed documentation. * clean up if metadata operation failed * Update s3_parquet_test.py * clean up * Update Makefile * Update s3_parquet_test.py * Update Makefile * Handle ivSkip for non-block-aligned offsets * Update README.md * stop volume server faster * stop volume server in 1 second * different IV for each chunk in SSE-S3 and SSE-KMS * clean up if fails * testing upload * error propagation * fmt * simplify * fix copying * less logs * endian * Added marshaling error handling * handling invalid ranges * error handling for adding to log buffer * fix logging * avoid returning too quickly and ensure proper cleaning up * Activity Tracking for Disk Reads * Cleanup Unused Parameters * Activity Tracking for Kafka Publishers * Proper Test Error Reporting * refactoring * less logs * less logs * go fmt * guard it with if entry.Attributes.TtlSec > 0 to match the pattern used elsewhere. * Handle bucket-default encryption config errors explicitly for multipart * consistent activity tracking * obsolete code for s3 on filer read/write handlers * Update weed/s3api/s3api_object_handlers_list.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>	2 weeks ago
Chris Lu	fa8df6e42b	S3: Lazy Versioning Check, Conditional SSE Entry Fetch, HEAD Request Optimization (#7480) * Lazy Versioning Check, Conditional SSE Entry Fetch, HEAD Request Optimization * revert Reverted the conditional versioning check to always check versioning status Reverted the conditional SSE entry fetch to always fetch entry metadata Reverted the conditional versioning check to always check versioning status Reverted the conditional SSE entry fetch to always fetch entry metadata * Lazy Entry Fetch for SSE, Skip Conditional Header Check * SSE-KMS headers are present, this is not an SSE-C request (mutually exclusive) * SSE-C is mutually exclusive with SSE-S3 and SSE-KMS * refactor * Removed Premature Mutual Exclusivity Check * check for the presence of the X-Amz-Server-Side-Encryption header * not used * fmt	3 weeks ago
chrislu	e154bfe163	minor	3 weeks ago
Chris Lu	4e73cc778c	S3: add context aware action resolution (#7479) * add context aware action resolution * isAnonymous * add s3 action resolver * refactor * correct action name * no need for action copy object * Simplify by removing the method-action mismatch path * use PUT instead of DELETE action * refactor * constants * versionId vs versions * address comments * comment * adjust messages * ResolveS3Action * address comments * refactor * simplify * more checks * not needed * simplify	3 weeks ago
chrislu	5b9a526310	adjust comment	3 weeks ago
Chris Lu	2a9d4d1e23	Refactor data structure (#7472) * refactor to avoids circular dependency * converts a policy.PolicyDocument to policy_engine.PolicyDocument * convert numeric types to strings * Update weed/s3api/policy_conversion.go Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * refactoring * not skipping numeric and boolean values in arrays * avoid nil * edge cases * handling conversion failure The handling of unsupported types in convertToString could lead to silent policy alterations. The conversion of map-based principals in convertPrincipal is too generic and could misinterpret policies. * concise * fix doc * adjust warning * recursion * return errors * reject empty principals * better error message --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>	3 weeks ago
Chris Lu	508d06d9a5	S3: Enforce bucket policy (#7471) * evaluate policies during authorization * cache bucket policy * refactor * matching with regex special characters * Case Sensitivity, pattern cache, Dead Code Removal * Fixed Typo, Restored []string Case, Added Cache Size Limit * hook up with policy engine * remove old implementation * action mapping * validate * if not specified, fall through to IAM checks * fmt * Fail-close on policy evaluation errors * Explicit `Allow` bypasses IAM checks * fix error message * arn:seaweed => arn:aws * remove legacy support * fix tests * Clean up bucket policy after this test * fix for tests * address comments * security fixes * fix tests * temp comment out	3 weeks ago
Chris Lu	bf8e4f40e6	S3: Perf related (#7463) * reduce checks * s3 object lookup optimization * Only check versioning configuration if client requests * Consolidate SSE Entry Lookups * optimize * revert optimization for versioned objects * Removed: getObjectEntryForSSE() function * refactor * Refactoring: Added fetchObjectEntryRequired * avoid refetching * return early if not found * reuse objects from conditional check * clear cache when creating bucket	4 weeks ago
Konstantin Lebedev	084b377f87	do delete expired entries on s3 list request (#7426) * do delete expired entries on s3 list request https://github.com/seaweedfs/seaweedfs/issues/6837 * disable delete expires s3 entry in filer * pass opt allowDeleteObjectsByTTL to all servers * delete on get and head * add lifecycle expiration s3 tests * fix opt allowDeleteObjectsByTTL for server * fix test lifecycle expiration * fix IsExpired * fix locationPrefix for updateEntriesTTL * fix s3tests * resolv coderabbitai * GetS3ExpireTime on filer * go mod * clear TtlSeconds for volume * move s3 delete expired entry to filer * filer delete meta and data * del unusing func removeExpiredObject * test s3 put * test s3 put multipart * allowDeleteObjectsByTTL by default * fix pipline tests * rm dublicate SeaweedFSExpiresS3 * revert expiration tests * fix updateTTL * rm log * resolv comment * fix delete version object * fix S3Versioning * fix delete on FindEntry * fix delete chunks * fix sqlite not support concurrent writes/reads * move deletion out of listing transaction; delete entries and empty folders * Revert "fix sqlite not support concurrent writes/reads" This reverts commit 5d5da14e0e. * clearer handling on recursive empty directory deletion * handle listing errors * strut copying * reuse code to delete empty folders * use iterative approach with a queue to avoid recursive WithFilerClient calls * stop a gRPC stream from the client-side callback is to return a specific error, e.g., io.EOF * still issue UpdateEntry when the flag must be added * errors join * join path * cleaner * add context, sort directories by depth (deepest first) to avoid redundant checks * batched operation, refactoring * prevent deleting bucket * constant * reuse code * more logging * refactoring * s3 TTL time * Safety check --------- Co-authored-by: chrislu <chris.lu@gmail.com>	4 weeks ago
Chris Lu	498ac8903f	S3: prevent deleting buckets with object locking (#7434) * prevent deleting buckets with object locking * addressing comments * Update s3api_bucket_handlers.go * address comments * early return * refactor * simplify * constant * go fmt	1 month ago
Chris Lu	bdc20d1c1e	S3: load bucket object locking configuration if not found in cache (#7422) * load bucket object locking configuration if not found in cache * fix cache building, more specific error, add back metrics	1 month ago
Chris Lu	b7e3284fc5	S3: fix TestSignedStreamingUploadInvalidSignature test (#7421) * Added continue statements after all state transitions in the state machine to ensure immediate state processing * simplify * remove redundant continue clause * ensure wrong signature	1 month ago
Chris Lu	9f07bca9cc	Fix IPv6 host header formatting to match AWS SDK behavior (#7414) * Add nginx reverse proxy documentation for S3 API Fixes #7407 Add comprehensive documentation and example configuration for using nginx as a reverse proxy with SeaweedFS S3 API while maintaining AWS Signature V4 authentication compatibility. Changes: - Add docker/nginx/README.md with detailed setup guide - Add docker/nginx/s3-example.conf with working configuration - Update docker/nginx/proxy.conf with important S3 notes The documentation covers: - Critical requirements for AWS Signature V4 authentication - Common mistakes and why they break S3 authentication - Complete working nginx configurations - Debugging tips and troubleshooting - Performance tuning recommendations * Fix IPv6 host header formatting to match AWS SDK behavior Follow-up to PR #7403 When a default port (80 for HTTP, 443 for HTTPS) is stripped from an IPv6 address, the square brackets should also be removed to match AWS SDK behavior for S3 signature calculation. Reference: https://github.com/aws/aws-sdk-go-v2/blob/main/aws/signer/internal/v4/host.go The AWS SDK's stripPort function explicitly removes brackets when returning an IPv6 address without a port. Changes: - Update extractHostHeader to strip brackets from IPv6 addresses when no port or default port is used - Update test expectations to match AWS SDK behavior - Add detailed comments explaining the AWS SDK compatibility requirement This ensures S3 signature validation works correctly with IPv6 addresses behind reverse proxies, matching AWS S3 canonical request format. Fixes the issue raised in PR #7403 comment: https://github.com/seaweedfs/seaweedfs/pull/7403#issuecomment-3471105438 * Update docker/nginx/README.md Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * Add nginx reverse proxy documentation for S3 API Fixes #7407 Add comprehensive documentation and example configuration for using nginx as a reverse proxy with SeaweedFS S3 API while maintaining AWS Signature V4 authentication compatibility. Changes: - Add docker/nginx/README.md with detailed setup guide - Add docker/nginx/s3-example.conf with working configuration - Update docker/nginx/proxy.conf with important S3 notes The documentation covers: - Critical requirements for AWS Signature V4 authentication - Common mistakes and why they break S3 authentication - Complete working nginx configurations - Debugging tips and troubleshooting - Performance tuning recommendations Fix IPv6 host header formatting to match AWS SDK behavior Follow-up to PR #7403 When a default port (80 for HTTP, 443 for HTTPS) is stripped from an IPv6 address, the square brackets should also be removed to match AWS SDK behavior for S3 signature calculation. Reference: https://github.com/aws/aws-sdk-go-v2/blob/main/aws/signer/internal/v4/host.go The AWS SDK's stripPort function explicitly removes brackets when returning an IPv6 address without a port. Changes: - Update extractHostHeader to strip brackets from IPv6 addresses when no port or default port is used - Update test expectations to match AWS SDK behavior - Add detailed comments explaining the AWS SDK compatibility requirement This ensures S3 signature validation works correctly with IPv6 addresses behind reverse proxies, matching AWS S3 canonical request format. Fixes the issue raised in PR #7403 comment: https://github.com/seaweedfs/seaweedfs/pull/7403#issuecomment-3471105438 * Revert "Merge branch 'fix-ipv6-brackets-default-port' of https://github.com/seaweedfs/seaweedfs into fix-ipv6-brackets-default-port" This reverts commit cca3f3985f, reversing changes made to 2b8f9de78e. --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>	1 month ago
Chris Lu	d00a2a8707	Fix S3 bucket policy ARN validation to accept AWS ARNs and simplified formats (#7409) * Fix S3 bucket policy ARN validation to accept AWS ARNs and simplified formats Fixes #7252 The bucket policy validation was rejecting valid AWS-style ARNs and simplified resource formats, causing validation failures with the error 'resource X does not match bucket X' even when they were identical strings. Changes: - Updated validateResourceForBucket() to accept three formats: 1. AWS-style ARNs: arn:aws:s3:::bucket-name[/*|/path] 2. SeaweedFS ARNs: arn:seaweed:s3:::bucket-name[/*|/path] 3. Simplified formats: bucket-name[/*|/path] - Added comprehensive test coverage for all three formats - Added specific test cases from issue #7252 to prevent regression This ensures compatibility with standard AWS S3 bucket policies while maintaining support for SeaweedFS-specific ARN format. * Refactor validateResourceForBucket to reduce code duplication Simplified the validation logic by stripping ARN prefixes first, then performing validation on the remaining resource path. This reduces code duplication and improves maintainability while maintaining identical functionality. Addresses review feedback from Gemini Code Assist. * Use strings.CutPrefix for cleaner ARN prefix stripping Replace strings.HasPrefix checks with strings.CutPrefix for more idiomatic Go code. This function is available in Go 1.20+ and provides cleaner conditional logic with the combined check and extraction. Addresses review feedback from Gemini Code Assist.	1 month ago
zuzuviewer	7e624d5355	* Fix s3 auth with proxy request (#7403) * * Fix s3 auth with proxy request * * 6649 Add unit test for signature v4 * address comments * fix for tests * ipv6 * address comments * setting scheme Works for both cases (direct HTTPS and behind proxy) * trim for ipv6 * Corrected Scheme Precedence Order * trim * accurate --------- Co-authored-by: chrislu <chris.lu@gmail.com> Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com>	1 month ago
Chris Lu	eaecd8328b	S3: add fallback for CORS (#7404) * add fallback for cors * refactor * expose aws headers * add fallback to test * refactor * Only falls back to global config when there's explicitly no bucket-level config. * fmt * Update s3_cors_http_test.go * refactoring	1 month ago
Taehyung Lim	d4790cb8e6	s3: fix if-match error (#7277) * s3: fix if-match error * add more checks * minor * minor --------- Co-authored-by: chrislu <chris.lu@gmail.com> Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com>	1 month ago
Chris Lu	85bd593936	S3: adjust for loading credentials (#7400) * adjust for loading credentials * Update weed/s3api/auth_credentials_test.go Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * simplify --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>	1 month ago
Nial	20e0d91037	IAM: add support for advanced IAM config file to server command (#7317) * IAM: add support for advanced IAM config file to server command * Add support for advanced IAM config file in S3 options * Fix S3 IAM config handling to simplify checks for configuration presence * simplify * simplify again * copy the value * const --------- Co-authored-by: chrislu <chris.lu@gmail.com> Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com>	1 month ago
Chris Lu	7d26c8838f	S3: auth supports X-Forwarded-Host and X-Forwarded-Port (#7398) * add fix and tests * address comments * idiomatic * ipv6	1 month ago
chrislu	b7ba6785a2	go fmt	1 month ago
Tom Crasset	824dcac3bf	s3: combine all signature verification checks into a single function (#7330)	1 month ago
Chris Lu	263e891da0	Clients to volume server requires JWT tokens for all read operations (#7376) * [Admin UI] Login not possible due to securecookie error * avoid 404 favicon * Update weed/admin/dash/auth_middleware.go Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * address comments * avoid variable over shadowing * log session save error * When jwt.signing.read.key is enabled in security.toml, the volume server requires JWT tokens for all read operations. * reuse fileId * refactor --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>	1 month ago
Konstantin Lebedev	bf58c5a688	fix: Use a mixed of virtual and path styles within a single subdomain (#7357) * fix: Use a mixed of virtual and path styles within a single subdomain * address comments * add tests --------- Co-authored-by: chrislu <chris.lu@gmail.com> Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com>	1 month ago
Chris Lu	0abf70061b	S3 API: Fix SSE-S3 decryption on object download (#7366) * S3 API: Fix SSE-S3 decryption on object download Fixes #7363 This commit adds missing SSE-S3 decryption support when downloading objects from SSE-S3 encrypted buckets. Previously, SSE-S3 encrypted objects were returned in their encrypted form, causing data corruption and hash mismatches. Changes: - Updated detectPrimarySSEType() to detect SSE-S3 encrypted objects by examining chunk metadata and distinguishing SSE-S3 from SSE-KMS - Added SSE-S3 handling in handleSSEResponse() to route to new handler - Implemented handleSSES3Response() for both single-part and multipart SSE-S3 encrypted objects with proper decryption - Implemented createMultipartSSES3DecryptedReader() for multipart objects with per-chunk decryption using stored IVs - Updated addSSEHeadersToResponse() to include SSE-S3 response headers The fix follows the existing SSE-C and SSE-KMS patterns, using the envelope encryption architecture where each object's DEK is encrypted with the KEK stored in the filer. * Add comprehensive tests for SSE-S3 decryption - TestSSES3EncryptionDecryption: basic encryption/decryption - TestSSES3IsRequestInternal: request detection - TestSSES3MetadataSerialization: metadata serialization/deserialization - TestDetectPrimarySSETypeS3: SSE type detection for various scenarios - TestAddSSES3HeadersToResponse: response header validation - TestSSES3EncryptionWithBaseIV: multipart encryption with base IV - TestSSES3WrongKeyDecryption: wrong key error handling - TestSSES3KeyGeneration: key generation and uniqueness - TestSSES3VariousSizes: encryption/decryption with various data sizes - TestSSES3ResponseHeaders: response header correctness - TestSSES3IsEncryptedInternal: metadata-based encryption detection - TestSSES3InvalidMetadataDeserialization: error handling for invalid metadata - TestGetSSES3Headers: header generation - TestProcessSSES3Request: request processing - TestGetSSES3KeyFromMetadata: key extraction from metadata - TestSSES3EnvelopeEncryption: envelope encryption correctness - TestValidateSSES3Key: key validation All tests pass successfully, providing comprehensive coverage for the SSE-S3 decryption fix. * Address PR review comments 1. Fix resource leak in createMultipartSSES3DecryptedReader: - Wrap decrypted reader with closer to properly release resources - Ensure underlying chunkReader is closed when done 2. Handle mixed-encryption objects correctly: - Check chunk encryption type before attempting decryption - Pass through non-SSE-S3 chunks unmodified - Log encryption type for debugging 3. Improve SSE type detection logic: - Add explicit case for aws:kms algorithm - Handle unknown algorithms gracefully - Better documentation for tie-breaking precedence 4. Document tie-breaking behavior: - Clarify that mixed encryption indicates potential corruption - Explicit precedence order: SSE-C > SSE-KMS > SSE-S3 These changes address high-severity resource management issues and improve robustness when handling edge cases and mixed-encryption scenarios. * Fix IV retrieval for small/inline SSE-S3 encrypted files Critical bug fix: The previous implementation only looked for the IV in chunk metadata, which would fail for small files stored inline (without chunks). Changes: - Check object-level metadata (sseS3Key.IV) first for inline files - Fallback to first chunk metadata only if object-level IV not found - Improved error message to indicate both locations were checked This ensures small SSE-S3 encrypted files (stored inline in entry.Content) can be properly decrypted, as their IV is stored in the object-level SeaweedFSSSES3Key metadata rather than in chunk metadata. Fixes the high-severity issue identified in PR review. * Clean up unused SSE metadata helper functions Remove legacy SSE metadata helper functions that were never fully implemented or used: Removed unused functions: - StoreSSECMetadata() / GetSSECMetadata() - StoreSSEKMSMetadata() / GetSSEKMSMetadata() - StoreSSES3Metadata() / GetSSES3Metadata() - IsSSEEncrypted() - GetSSEAlgorithm() Removed unused constants: - MetaSSEAlgorithm - MetaSSECKeyMD5 - MetaSSEKMSKeyID - MetaSSEKMSEncryptedKey - MetaSSEKMSContext - MetaSSES3KeyID These functions were from an earlier design where IV and other metadata would be stored in common entry.Extended keys. The actual implementations use type-specific serialization: - SSE-C: Uses StoreIVInMetadata()/GetIVFromMetadata() directly for IV - SSE-KMS: Serializes entire SSEKMSKey structure as JSON (includes IV) - SSE-S3: Serializes entire SSES3Key structure as JSON (includes IV) This follows Option A: SSE-S3 uses envelope encryption pattern like SSE-KMS, where IV is stored within the serialized key metadata rather than in a separate metadata field. Kept functions still in use: - StoreIVInMetadata() - Used by SSE-C - GetIVFromMetadata() - Used by SSE-C and streaming copy - MetaSSEIV constant - Used by SSE-C All tests pass after cleanup. * Rename SSE metadata functions to clarify SSE-C specific usage Renamed functions and constants to explicitly indicate they are SSE-C specific, improving code clarity: Renamed: - MetaSSEIV → MetaSSECIV - StoreIVInMetadata() → StoreSSECIVInMetadata() - GetIVFromMetadata() → GetSSECIVFromMetadata() Updated all usages across: - s3api_key_rotation.go - s3api_streaming_copy.go - s3api_object_handlers_copy.go - s3_sse_copy_test.go - s3_sse_test_utils_test.go Rationale: These functions are exclusively used by SSE-C for storing/retrieving the IV in entry.Extended metadata. SSE-KMS and SSE-S3 use different approaches (IV stored in serialized key structures), so the generic names were misleading. The new names make it clear these are part of the SSE-C implementation. All tests pass. * Add integration tests for SSE-S3 end-to-end encryption/decryption These integration tests cover the complete encrypt->store->decrypt cycle that was missing from the original test suite. They would have caught the IV retrieval bug for inline files. Tests added: - TestSSES3EndToEndSmallFile: Tests inline files (10, 50, 256 bytes) * Specifically tests the critical IV retrieval path for inline files * This test explicitly checks the bug we fixed where inline files couldn't retrieve their IV from object-level metadata - TestSSES3EndToEndChunkedFile: Tests multipart encrypted files * Verifies per-chunk metadata serialization/deserialization * Tests that each chunk can be independently decrypted with its own IV - TestSSES3EndToEndWithDetectPrimaryType: Tests type detection * Verifies inline vs chunked SSE-S3 detection * Ensures SSE-S3 is distinguished from SSE-KMS Note: Full HTTP handler tests (PUT -> GET through actual handlers) would require a complete mock server with filer connections, which is complex. These tests focus on the critical decrypt path and data flow. Why these tests are important: - Unit tests alone don't catch integration issues - The IV retrieval bug existed because there was no end-to-end test - These tests simulate the actual storage/retrieval flow - They verify the complete encryption architecture works correctly All tests pass. * Fix TestValidateSSES3Key expectations to match actual implementation The ValidateSSES3Key function only validates that the key struct is not nil, but doesn't validate the Key field contents or size. The test was expecting validation that doesn't exist. Updated test cases: - Nil key struct → should error (correct) - Valid key → should not error (correct) - Invalid key size → should not error (validation doesn't check this) - Nil key bytes → should not error (validation doesn't check this) Added comments to clarify what the current validation actually checks. This matches the behavior of ValidateSSEKMSKey and ValidateSSECKey which also only check for nil struct, not field contents. All SSE tests now pass. * Improve ValidateSSES3Key to properly validate key contents Enhanced the validation function from only checking nil struct to comprehensive validation of all key fields: Validations added: 1. Key bytes not nil 2. Key size exactly 32 bytes (SSES3KeySize) 3. Algorithm must be "AES256" (SSES3Algorithm) 4. Key ID must not be empty 5. IV length must be 16 bytes if set (optional - set during encryption) Test improvements (10 test cases): - Nil key struct - Valid key without IV - Valid key with IV - Invalid key size (too small) - Invalid key size (too large) - Nil key bytes - Empty key ID - Invalid algorithm - Invalid IV length - Empty IV (allowed - set during encryption) This matches the robustness of SSE-C and SSE-KMS validation and will catch configuration errors early rather than failing during encryption/decryption. All SSE tests pass. * Replace custom string helper functions with strings.Contains Address Gemini Code Assist review feedback: - Remove custom contains() and findSubstring() helper functions - Use standard library strings.Contains() instead - Add strings import This makes the code more idiomatic and easier to maintain by using the standard library instead of reimplementing functionality. Changes: - Added "strings" to imports - Replaced contains(err.Error(), tc.errorMsg) with strings.Contains(err.Error(), tc.errorMsg) - Removed 15 lines of custom helper code All tests pass. * filer fix reading and writing SSE-S3 headers * filter out seaweedfs internal headers * Update weed/s3api/s3api_object_handlers.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update weed/s3api/s3_validation_utils.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update s3api_streaming_copy.go * remove fallback * remove redundant check * refactor * remove extra object fetching * in case object is not found * Correct Version Entry for SSE Routing * Proper Error Handling for SSE Entry Fetching * Eliminated All Redundant Lookups * Removed brittle “exactly 5 successes/failures” assertions. Added invariant checks total recorded attempts equals request count, successes never exceed capacity, failures cover remaining attempts, final AvailableSpace matches capacity - successes. * refactor * fix test * Fixed Broken Fallback Logic * refactor * Better Error for Encryption Type Mismatch * refactor --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>	1 month ago
Chris Lu	f7bd75ef3b	S3: Avoid in-memory map concurrent writes in SSE-S3 key manager (#7358) * Fix concurrent map writes in SSE-S3 key manager This commit fixes issue #7352 where parallel uploads to SSE-S3 enabled buckets were causing 'fatal error: concurrent map writes' crashes. The SSES3KeyManager struct had an unsynchronized map that was being accessed from multiple goroutines during concurrent PUT operations. Changes: - Added sync.RWMutex to SSES3KeyManager struct - Protected StoreKey() with write lock - Protected GetKey() with read lock - Updated GetOrCreateKey() with proper read/write locking pattern including double-check to prevent race conditions All existing SSE tests pass successfully. Fixes #7352 * Improve SSE-S3 key manager with envelope encryption Replace in-memory key storage with envelope encryption using a super key (KEK). Instead of storing DEKs in a map, the key manager now: - Uses a randomly generated 256-bit super key (KEK) - Encrypts each DEK with the super key using AES-GCM - Stores the encrypted DEK in object metadata - Decrypts the DEK on-demand when reading objects Benefits: - Eliminates unbounded memory growth from caching DEKs - Provides better security with authenticated encryption (AES-GCM) - Follows envelope encryption best practices (similar to AWS KMS) - No need for mutex-protected map lookups on reads - Each object's encrypted DEK is self-contained in its metadata This approach matches the design pattern used in the local KMS provider and is more suitable for production use. * Persist SSE-S3 KEK in filer for multi-server support Store the SSE-S3 super key (KEK) in the filer at /.seaweedfs/s3/kek instead of generating it per-server. This ensures: 1. **Multi-server consistency**: All S3 API servers use the same KEK 2. **Persistence across restarts**: KEK survives server restarts 3. **Centralized management**: KEK stored in filer, accessible to all servers 4. **Automatic initialization**: KEK is created on first startup if it doesn't exist The KEK is: - Stored as hex-encoded bytes in filer - Protected with file mode 0600 (read/write for owner only) - Located in /.seaweedfs/s3/ directory (mode 0700) - Loaded on S3 API server startup - Reused across all S3 API server instances This matches the architecture of centralized configuration in SeaweedFS and enables proper SSE-S3 support in multi-server deployments. * Change KEK storage location to /etc/s3/kek Move SSE-S3 KEK from /.seaweedfs/s3/kek to /etc/s3/kek for better organization and consistency with other SeaweedFS configuration files. The /etc directory is the standard location for configuration files in SeaweedFS. * use global sse-se key manager when copying * Update volume_growth_reservation_test.go * Rename KEK file to sse_kek for clarity Changed /etc/s3/kek to /etc/s3/sse_kek to make it clear this key is specifically for SSE-S3 encryption, not for other KMS purposes. This improves clarity and avoids potential confusion with the separate KMS provider system used for SSE-KMS. * Use constants for SSE-S3 KEK directory and file name Refactored to use named constants instead of string literals: - SSES3KEKDirectory = "/etc/s3" - SSES3KEKParentDir = "/etc" - SSES3KEKDirName = "s3" - SSES3KEKFileName = "sse_kek" This improves maintainability and makes it easier to change the storage location if needed in the future. * Address PR review: Improve error handling and robustness Addresses review comments from https://github.com/seaweedfs/seaweedfs/pull/7358#pullrequestreview-3367476264 Critical fixes: 1. Distinguish between 'not found' and other errors when loading KEK - Only generate new KEK if ErrNotFound - Fail fast on connectivity/permission errors to prevent data loss - Prevents creating new KEK that would make existing data undecryptable 2. Make SSE-S3 initialization failure fatal - Return error instead of warning when initialization fails - Prevents server from running in broken state 3. Improve directory creation error handling - Only ignore 'file exists' errors - Fail on permission/connectivity errors These changes ensure the SSE-S3 key manager is robust against transient errors and prevents accidental data loss. * Fix KEK path conflict with /etc/s3 file Changed KEK storage from /etc/s3/sse_kek to /etc/seaweedfs/s3_sse_kek to avoid conflict with the circuit breaker config at /etc/s3. The /etc/s3 path is used by CircuitBreakerConfigDir and may exist as a file (circuit_breaker.json), causing the error: 'CreateEntry /etc/s3/sse_kek: /etc/s3 should be a directory' New KEK location: /etc/seaweedfs/s3_sse_kek This uses the seaweedfs subdirectory which is more appropriate for internal SeaweedFS configuration files. Fixes startup failure when /etc/s3 exists as a file. * Revert KEK path back to /etc/s3/sse_kek Changed back from /etc/seaweedfs/s3_sse_kek to /etc/s3/sse_kek as requested. The /etc/s3 directory will be created properly when it doesn't exist. * Fix directory creation with proper ModeDir flag Set FileMode to uint32(0755 | os.ModeDir) when creating /etc/s3 directory to ensure it's created as a directory, not a file. Without the os.ModeDir flag, the entry was being created as a file, which caused the error 'CreateEntry: /etc/s3 is a file' when trying to create the KEK file inside it. Uses 0755 permissions (rwxr-xr-x) for the directory and adds os import for os.ModeDir constant.	1 month ago