seaweedfs

Commit Graph

Author	SHA1	Message	Date
Chris Lu	1b13324fb7	fix: skip log files with deleted volumes in filer backup (#7692 ) fix: skip log files with deleted volumes in filer backup (#3720) When filer.backup or filer.meta.backup resumes after being stopped, it may encounter persisted log files stored on volumes that have since been deleted (via volume.deleteEmpty -force). Previously, this caused the backup to get stuck in an infinite retry loop with 'volume X not found' errors. This fix catches 'volume not found' errors when reading log files and skips the problematic file instead of failing. The backup will now: - Log a warning about the missing volume - Skip the problematic log file - Continue with the next log file, allowing progress The VolumeNotFoundPattern regex was already defined but never used - this change puts it to use. Fixes #3720	4 days ago
Chris Lu	80c7de8d76	Helm Charts: add admin and worker to helm charts (#7688 ) * add admin and worker to helm charts * workers are stateless, admin is stateful * removed the duplicate admin-deployment.yaml * address comments * address comments * purge * Update README.md * Update k8s/charts/seaweedfs/templates/admin/admin-ingress.yaml Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * address comments * address comments * supports Kubernetes versions from v1.14 to v1.30+, ensuring broad compatibility * add probe for workers * address comments * add a todo * chore: trigger CI * use port name for probes in admin statefulset * fix: remove trailing blank line in values.yaml * address code review feedback - Quote admin credentials in shell command to handle special characters - Remove unimplemented capabilities (remote, replication) from worker defaults - Add security note about admin password character restrictions --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>	5 days ago
Chris Lu	0ede657a34	fix: return error on size mismatch in ReadNeedleMeta for consistency (#7687 ) * fix: return error on size mismatch in ReadNeedleMeta for consistency When ReadNeedleMeta encounters a size mismatch at offset >= MaxPossibleVolumeSize, it previously just continued without returning an error, potentially using wrong data. This fix makes ReadNeedleMeta consistent with ReadBytes (needle_read.go), which properly returns an error in both cases: - ErrorSizeMismatch when offset < MaxPossibleVolumeSize (to trigger retry at offset+32GB) - A descriptive error when offset >= MaxPossibleVolumeSize (after retry failed) Fixes #7673 * refactor: use more accurate error message for size mismatch	5 days ago
Chris Lu	5c27522507	fix: prevent empty .vif files from ec.decode causing parse errors (#7686 ) * fix: prevent empty .vif files from ec.decode causing parse errors When ec.decode copies .vif files from EC shard nodes, if a source node doesn't have the .vif file, an empty .vif file was created on the target node. This caused volume.configure.replication to fail with 'proto: syntax error' when trying to parse the empty file. This fix: 1. In writeToFile: Remove empty files when no data was written (source file was not found) to avoid leaving corrupted empty files 2. In MaybeLoadVolumeInfo: Handle empty .vif files gracefully by treating them as non-existent, allowing the system to create a proper one Fixes #7666 * refactor: remove redundant dst.Close() and add error logging Address review feedback: - Remove redundant dst.Close() call since defer already handles it - Add error logging for os.Remove() failure	5 days ago
Chen Pu	40eee23be9	mount: fix weed inode nlookup do not equel kernel inode nlookup (#7682 ) * mount: fix weed inode nlookup do not equel kernel inode nlookup * mount: add underflow protection for nlookup decrement in Forget * mount: use consistent == 0 check for uint64 nlookup * Update weed/mount/inode_to_path.go Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * mount: snapshot data before unlock in Forget to avoid using deleted InodeEntry --------- Co-authored-by: chrislu <chris.lu@gmail.com> Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>	5 days ago
Chris Lu	b4e2cca204	s3api: remove redundant auth verification in getRequestDataReader (#7685 ) * s3api: remove redundant auth verification in getRequestDataReader The handlers PutObjectHandler and PutObjectPartHandler are already wrapped with s3a.iam.Auth() middleware which performs signature verification via authRequest() before the handler is invoked. The signature verification for authTypeSignedV2, authTypePresignedV2, authTypePresigned, and authTypeSigned in getRequestDataReader was therefore redundant. The newChunkedReader() call for streaming auth types is kept as it's needed to parse the chunked transfer encoding and extract the actual data. Fixes #7683 * simplify switch to if statement for single condition	5 days ago
Chris Lu	d6d893c8c3	s3: add s3:ExistingObjectTag condition support for bucket policies (#7677 ) * s3: add s3:ExistingObjectTag condition support in policy engine Add support for s3:ExistingObjectTag/<tag-key> condition keys in bucket policies, allowing access control based on object tags. Changes: - Add ObjectEntry field to PolicyEvaluationArgs (entry.Extended metadata) - Update EvaluateConditions to handle s3:ExistingObjectTag/<key> format - Extract tag value from entry metadata using X-Amz-Tagging-<key> prefix This enables policies like: { "Condition": { "StringEquals": { "s3:ExistingObjectTag/status": ["public"] } } } Fixes: https://github.com/seaweedfs/seaweedfs/issues/7447 * s3: update EvaluatePolicy to accept object entry for tag conditions Update BucketPolicyEngine.EvaluatePolicy to accept objectEntry parameter (entry.Extended metadata) for evaluating tag-based policy conditions. Changes: - Add objectEntry parameter to EvaluatePolicy method - Update callers in auth_credentials.go and s3api_bucket_handlers.go - Pass nil for objectEntry in auth layer (entry fetched later in handlers) For tag-based conditions to work, handlers should call EvaluatePolicy with the object's entry.Extended after fetching the entry from filer. * s3: add tests for s3:ExistingObjectTag policy conditions Add comprehensive tests for object tag-based policy conditions: - TestExistingObjectTagCondition: Basic tag matching scenarios - Matching/non-matching tag values - Missing tags, no tags, empty tags - Multiple tags with one matching - TestExistingObjectTagConditionMultipleTags: Multiple tag conditions - Both tags match - Only one tag matches - TestExistingObjectTagDenyPolicy: Deny policies with tag conditions - Default allow without tag - Deny when specific tag present * s3: document s3:ExistingObjectTag support and feature status Update policy engine documentation: - Add s3:ExistingObjectTag/<tag-key> to supported condition keys - Add 'Object Tag-Based Access Control' section with examples - Add 'Feature Status' section with implemented and planned features Planned features for future implementation: - s3:RequestObjectTag/<key> - s3:RequestObjectTagKeys - s3:x-amz-server-side-encryption - Cross-account access * Implement tag-based policy re-check in handlers - Add checkPolicyWithEntry helper to S3ApiServer for handlers to re-check policy after fetching object entry (for s3:ExistingObjectTag conditions) - Add HasPolicyForBucket method to policy engine for efficient check - Integrate policy re-check in GetObjectHandler after entry is fetched - Integrate policy re-check in HeadObjectHandler after entry is fetched - Update auth_credentials.go comments to explain two-phase evaluation - Update documentation with supported operations for tag-based conditions This implements 'Approach 1' where handlers re-check the policy with the object entry after fetching it, allowing tag-based conditions to be properly evaluated. * Add integration tests for s3:ExistingObjectTag conditions - Add TestCheckPolicyWithEntry: tests checkPolicyWithEntry helper with various tag scenarios (matching tags, non-matching tags, empty entry, nil entry) - Add TestCheckPolicyWithEntryNoPolicyForBucket: tests early return when no policy - Add TestCheckPolicyWithEntryNilPolicyEngine: tests nil engine handling - Add TestCheckPolicyWithEntryDenyPolicy: tests deny policies with tag conditions - Add TestHasPolicyForBucket: tests HasPolicyForBucket method These tests cover the Phase 2 policy evaluation with object entry metadata, ensuring tag-based conditions are properly evaluated. * Address code review nitpicks - Remove unused extractObjectTags placeholder function (engine.go) - Add clarifying comment about s3:ExistingObjectTag/<key> evaluation - Consolidate duplicate tag-based examples in README - Factor out tagsToEntry helper to package level in tests * Address code review feedback - Fix unsafe type assertions in GetObjectHandler and HeadObjectHandler when getting identity from context (properly handle type assertion failure) - Extract getConditionContextValue helper to eliminate duplicated logic between EvaluateConditions and EvaluateConditionsLegacy - Ensure consistent handling of missing condition keys (always return empty slice) * Fix GetObjectHandler to match HeadObjectHandler pattern Add safety check for nil objectEntryForSSE before tag-based policy evaluation, ensuring tag-based conditions are always evaluated rather than silently skipped if entry is unexpectedly nil. Addresses review comment from Copilot. * Fix HeadObject action name in docs for consistency Change 'HeadObject' to 's3:HeadObject' to match other action names. * Extract recheckPolicyWithObjectEntry helper to reduce duplication Move the repeated identity extraction and policy re-check logic from GetObjectHandler and HeadObjectHandler into a shared helper method. * Add validation for empty tag key in s3:ExistingObjectTag condition Prevent potential issues with malformed policies containing s3:ExistingObjectTag/ (empty tag key after slash).	5 days ago
Chris Lu	d5f21fd8ba	fix: add missing backslash for volume extraArgs in helm chart (#7676 ) Fixes #7467 The -mserver argument line in volume-statefulset.yaml was missing a trailing backslash, which prevented extraArgs from being passed to the weed volume process. Also: - Extracted master server list generation logic into shared helper templates in _helpers.tpl for better maintainability - Updated all occurrences of deprecated -mserver flag to -master across docker-compose files, test files, and documentation	5 days ago
Chris Lu	cea12ba3c4	fix: prevent makeslice panic in ReadNeedleMeta with corrupted needle (#7675 ) * fix: prevent makeslice panic in ReadNeedleMeta with corrupted needle When a needle's DataSize in the .dat file is corrupted to a very large value, the calculation of metaSize can become negative, causing a panic with 'makeslice: len out of range' when creating the metadata slice. This fix adds validation to check if metaSize is negative before creating the slice, returning a descriptive error instead of panicking. Fixes #7475 * Update weed/storage/needle/needle_read_page.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>	5 days ago
Chris Lu	9196696278	mount: add mutex to DirectoryHandle to fix race condition (#7674 ) * mount: add mutex to DirectoryHandle to fix race condition When using Ganesha NFS on top of FUSE mount, ls operations would hang forever on directories with hundreds of files. This was caused by a race condition in DirectoryHandle where multiple concurrent readdir operations could modify shared state (entryStream, entryStreamOffset, isFinished) without synchronization. The fix adds a mutex to DirectoryHandle and holds it for the entire duration of doReadDirectory. This serializes concurrent readdir calls on the same handle, which is the correct behavior for a directory handle and fixes the race condition. Key changes: - Added sync.Mutex to DirectoryHandle struct - Lock the mutex at the start of doReadDirectory - This ensures thread-safe access to entryStream and other state The lock is per-handle (not global), so different directories can still be listed concurrently. Only concurrent operations on the same directory handle are serialized. Fixes: https://github.com/seaweedfs/seaweedfs/issues/7672 * mount: add mutex to DirectoryHandle to fix race condition When using Ganesha NFS on top of FUSE mount, ls operations would hang forever on directories with hundreds of files. This was caused by a race condition in DirectoryHandle where multiple concurrent readdir operations could modify shared state (entryStream, entryStreamOffset, isFinished) without synchronization. The fix adds a mutex to DirectoryHandle and holds it for the entire duration of doReadDirectory. This serializes concurrent readdir calls on the same handle, which is the correct behavior for a directory handle and fixes the race condition. Key changes: - Added sync.Mutex to DirectoryHandle struct - Lock the mutex at the start of doReadDirectory - Optimized reset() to reuse slice capacity and allow GC of old entries The lock is per-handle (not global), so different directories can still be listed concurrently. Only concurrent operations on the same directory handle are serialized. Fixes: https://github.com/seaweedfs/seaweedfs/issues/7672	5 days ago
Chris Lu	ff4855dcbe	sts: limit session duration to incoming token's exp claim (#7670 ) * sts: limit session duration to incoming token's exp claim This fixes the issue where AssumeRoleWithWebIdentity would issue sessions that outlive the source identity token's expiration. For use cases like GitLab CI Jobs where the ID Token has an exp claim limited to the CI job's timeout, the STS session should not exceed that expiration. Changes: - Add TokenExpiration field to ExternalIdentity struct - Extract exp/iat/nbf claims in OIDC provider's ValidateToken - Pass token expiration from Authenticate to ExternalIdentity - Modify calculateSessionDuration to cap at source token's exp - Add comprehensive tests for the new behavior Fixes: https://github.com/seaweedfs/seaweedfs/discussions/7653 * refactor: reduce duplication in time claim extraction Use a loop over claim names instead of repeating the same extraction logic three times for exp, iat, and nbf claims. * address review: add defense-in-depth for expired tokens - Handle already-expired tokens defensively with 1 minute minimum duration - Enforce MaxSessionLength from config as additional cap - Fix potential nil dereference in test mock - Add test case for expired token scenario * remove issue reference from test * fix: remove early return to ensure MaxSessionLength is always checked	6 days ago
Chris Lu	772459f93c	fix: restore volume mount when VolumeConfigure fails (#7669 ) * fix: restore volume mount when VolumeConfigure fails When volume.configure.replication command fails (e.g., due to corrupted .vif file), the volume was left unmounted and the master was already notified that the volume was deleted, causing the volume to disappear. This fix attempts to re-mount the volume when ConfigureVolume fails, restoring the volume state and preventing data loss. Fixes #7666 * include mount restore error in response message	6 days ago
Chris Lu	086ab3e28c	Fix webhook duplicate deliveries and POST to GET conversion (#7668 ) * Fix webhook duplicate deliveries and POST to GET conversion Fixes #7667 This commit addresses two critical issues with the webhook notification system: 1. Duplicate webhook deliveries based on worker count 2. POST requests being converted to GET when following redirects Issue 1: Multiple webhook deliveries ------------------------------------ Problem: The webhook queue was creating multiple handlers (one per worker) that all subscribed to the same topic. With Watermill's gochannel, each handler creates a separate subscription, and all subscriptions receive their own copy of every message, resulting in duplicate webhook calls equal to the worker count. Solution: Use a single handler instead of multiple handlers to ensure each webhook event is sent only once, regardless of worker configuration. Issue 2: POST to GET conversion with intelligent redirect handling ------------------------------------------------------------------ Problem: When webhook endpoints returned redirects (301/302/303), Go's default HTTP client would automatically follow them and convert POST requests to GET requests per HTTP specification. Solution: Implement intelligent redirect handling that: - Prevents automatic redirects to preserve POST method - Manually follows redirects by recreating POST requests - Caches the final redirect destination for performance - Invalidates cache and retries on failures (network or HTTP errors) - Provides automatic recovery from cached endpoint failures Benefits: - Webhooks are now sent exactly once per event - POST method is always preserved through redirects - Reduced latency through redirect destination caching - Automatic failover when cached destinations become unavailable - Thread-safe concurrent webhook delivery Testing: - Added TestQueueNoDuplicateWebhooks to verify single delivery - Added TestHttpClientFollowsRedirectAsPost for redirect handling - Added TestHttpClientUsesCachedRedirect for caching behavior - Added cache invalidation tests for error scenarios - All 18 webhook tests pass successfully * Address code review comments - Add maxWebhookRetryDepth constant to avoid magic number - Extract cache invalidation logic into invalidateCache() helper method - Fix redirect handling to properly follow redirects even on retry attempts - Remove misleading comment about nWorkers controlling handler parallelism - Fix test assertions to match actual execution flow - Remove trailing whitespace in test file All tests passing. * Refactor: use setFinalURL() instead of invalidateCache() Replace invalidateCache() with more explicit setFinalURL() function. This is cleaner as it makes the intent clear - we're setting the URL (either to a value or to empty string to clear it), rather than having a separate function just for clearing. No functional changes, all tests passing. * Add concurrent webhook delivery using nWorkers configuration Webhooks were previously sent sequentially (one-by-one), which could be a performance bottleneck for high-throughput scenarios. Now nWorkers configuration is properly used to control concurrent webhook delivery. Implementation: - Added semaphore channel (buffered to nWorkers capacity) - handleWebhook acquires semaphore slot before sending (blocks if at capacity) - Releases slot after webhook completes - Allows up to nWorkers concurrent webhook HTTP requests Benefits: - Improved throughput for slow webhook endpoints - nWorkers config now has actual purpose (was validated but unused) - Default 5 workers provides good balance - Configurable from 1-100 workers based on needs Example performance improvement: - Before: 500ms webhook latency = ~2 webhooks/sec max - After (5 workers): 500ms latency = ~10 webhooks/sec - After (10 workers): 500ms latency = ~20 webhooks/sec All tests passing. * Replace deprecated AddNoPublisherHandler with AddConsumerHandler AddNoPublisherHandler is deprecated in Watermill. Use AddConsumerHandler instead, which is the current recommended API for handlers that only consume messages without publishing. No functional changes, all tests passing. * Drain response bodies to enable HTTP connection reuse Added drainBody() calls in all code paths to ensure response bodies are consumed before returning. This is critical for HTTP keep-alive connection reuse. Without draining: - Connections are closed after each request - New TCP handshake + TLS handshake for every webhook - Higher latency and resource usage With draining: - Connections are reused via HTTP keep-alive - Significant performance improvement for repeated webhooks - Lower latency (no handshake overhead) - Reduced resource usage Implementation: - Added drainBody() helper that reads up to 1MB (prevents memory issues) - Drain on success path (line 161) - Drain on error responses before retry (lines 119, 152) - Drain on redirect responses before following (line 118) - Already had drainResponse() for network errors (line 99) All tests passing. * Use existing CloseResponse utility instead of custom drainBody Replaced custom drainBody() function with the existing util_http.CloseResponse() utility which is already used throughout the codebase. This provides: - Consistent behavior with rest of the codebase - Better logging (logs bytes drained via CountingReader) - Full body drainage (not limited to 1MB) - Cleaner code (no duplication) CloseResponse properly drains and closes the response body to enable HTTP keep-alive connection reuse. All tests passing. * Fix: Don't overwrite original error when draining response Before: err was being overwritten by drainResponse() result After: Use drainErr to avoid losing the original client.Do() error This was a subtle bug where if drainResponse() succeeded (returned nil), we would lose the original network error and potentially return a confusing error message. All tests passing. * Optimize HTTP client: reuse client and remove redundant timeout 1. Reuse single http.Client instance instead of creating new one per request - Reduces allocation overhead - More efficient for high-volume webhooks 2. Remove redundant timeout configuration - Before: timeout set on both context AND http.Client - After: timeout only on context (cleaner, context fires first anyway) Performance benefits: - Reduced GC pressure (fewer client allocations) - Better connection pooling (single transport instance) - Cleaner code (no redundancy) All tests passing.	6 days ago
Lisandro Pin	ca1ad9c4c2	Nit: have `ec.encode` exit immediately if no volumes are processed. (#7654 ) * Nit: have `ec.encode` exit immediately if no volumes are processed. * Update weed/shell/command_ec_encode.go Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --------- Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>	6 days ago
chrislu	1856eaca03	Update notification.toml	6 days ago
chrislu	95f6a2bc13	Added a complete webhook configuration example	6 days ago
Trim21	0a0eb21b86	fix random volume ids in master.html (#7655 )	6 days ago
chrislu	805950b401	4.02	6 days ago
Chris Lu	cadb2eeb05	fix: ARM v7 alignment issue for 64-bit atomic operations (#7652 ) Fixes #7643 Reordered filerHealth struct fields to ensure int64 field comes first, guaranteeing 8-byte alignment required for atomic operations on 32-bit ARM architectures (ARMv7, as used in OpenWRT).	6 days ago
Chris Lu	982aae6d53	SFTP: support reloading user store on HUP signal (#7651 ) Fixes #7650 This change enables the SFTP server to reload the user store configuration (sftp_userstore.json) when a HUP signal is sent to the process, without requiring a service restart. Changes: - Add Reload() method to FileStore to re-read users from disk - Add Reload() method to SFTPService to handle reload requests - Register reload hook with grace.OnReload() in sftp command This allows administrators to add users or change access policies dynamically by editing the user store file and sending a HUP signal (e.g., 'systemctl reload seaweedfs' or 'kill -HUP <pid>').	6 days ago
Chris Lu	f5c0bcafa3	s3: fix ListBuckets not showing buckets created by authenticated users (#7648 ) * s3: fix ListBuckets not showing buckets created by authenticated users Fixes #7647 ## Problem Users with proper Admin permissions could create buckets but couldn't list them. The issue occurred because ListBucketsHandler was not wrapped with the Auth middleware, so the authenticated identity was never set in the request context. ## Root Cause - PutBucketHandler uses iam.Auth() middleware which sets identity in context - ListBucketsHandler did NOT use iam.Auth() middleware - Without the middleware, GetIdentityNameFromContext() returned empty string - Bucket ownership checks failed because no identity was present ## Changes 1. Wrap ListBucketsHandler with iam.Auth() middleware (s3api_server.go) 2. Update ListBucketsHandler to get identity from context (s3api_bucket_handlers.go) 3. Add lookupByIdentityName() helper method (auth_credentials.go) 4. Add comprehensive test TestListBucketsIssue7647 (s3api_bucket_handlers_test.go) ## Testing - All existing tests pass (1348 tests in s3api package) - New test TestListBucketsIssue7647 validates the fix - Verified admin users can see their created buckets - Verified admin users can see all buckets - Verified backward compatibility maintained * s3: fix ListBuckets for JWT/Keycloak authentication The previous fix broke JWT/Keycloak authentication because JWT identities are created on-the-fly and not stored in the iam.identities list. The lookupByIdentityName() would return nil for JWT users. Solution: Store the full Identity object in the request context, not just the name. This allows ListBucketsHandler to retrieve the complete identity for all authentication types (SigV2, SigV4, JWT, Anonymous). Changes: - Add SetIdentityInContext/GetIdentityFromContext in s3_constants/header.go - Update Auth middleware to store full identity in context - Update ListBucketsHandler to retrieve identity from context first, with fallback to lookup for backward compatibility * s3: optimize lookupByIdentityName to O(1) using map Address code review feedback: Use a map for O(1) lookups instead of O(N) linear scan through identities list. Changes: - Add nameToIdentity map to IdentityAccessManagement struct - Populate map in loadS3ApiConfiguration (consistent with accessKeyIdent pattern) - Update lookupByIdentityName to use map lookup instead of loop This improves performance when many identities are configured and aligns with the existing pattern used for accessKeyIdent lookups. * s3: address code review feedback on nameToIdentity and logging Address two code review points: 1. Wire nameToIdentity into env-var fallback path - The AWS env-var fallback in NewIdentityAccessManagementWithStore now populates nameToIdentity map along with accessKeyIdent - Keeps all identity lookup maps in sync - Avoids potential issues if handlers rely on lookupByIdentityName 2. Improve access key lookup logging - Reduce log verbosity: V(1) -> V(2) for failed lookups - Truncate access keys in logs (show first 4 chars + **) - Include key length for debugging - Prevents credential exposure in production logs - Reduces log noise from misconfigured clients fmt * s3: refactor truncation logic and improve error handling Address additional code review feedback: 1. DRY principle: Extract key truncation logic into local function - Define truncate() helper at function start - Reuse throughout lookupByAccessKey - Eliminates code duplication 2. Enhanced security: Mask very short access keys - Keys <= 4 chars now show as '***' instead of full key - Prevents any credential exposure even for short keys - Consistent masking across all log statements 3. Improved robustness: Add warning log for type assertion failure - Log unexpected type when identity context object is wrong type - Helps debug potential middleware or context issues - Better production diagnostics 4. Documentation: Add comment about future optimization opportunity - Note potential for lightweight identity view in context - Suggests credential-free view for better data minimization - Documents design decision for future maintainers	6 days ago
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.	6 days ago
Chris Lu	5d53edb93b	Optimize database connection pool settings for MySQL and PostgreSQL (#7645 ) - Reduce connection_max_idle from 100 to 10 (PostgreSQL) and from 2 to 10 (MySQL) - Reduce connection_max_open from 100 to 50 for all database stores - Set connection_max_lifetime_seconds to 300 (5 minutes) to force connection recycling This prevents 'cannot assign requested address' errors under high load by: 1. Limiting the number of concurrent connections to reduce ephemeral port usage 2. Forcing connection recycling to prevent stale connections and port exhaustion 3. Reducing idle connections to minimize resource consumption Fixes #6887	7 days ago
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	1 week ago
Chris Lu	55f0fbf364	s3: optimize DELETE by skipping lock check for buckets without Object Lock (#7642 ) This optimization avoids an expensive filer gRPC call for every DELETE operation on buckets that don't have Object Lock enabled. Before this change, enforceObjectLockProtections() would always call getObjectEntry() to fetch object metadata to check for retention/legal hold, even for buckets that never had Object Lock configured. Changes: 1. Add early return in enforceObjectLockProtections() if bucket has no Object Lock config or bucket doesn't exist 2. Add isObjectLockEnabled() helper function to check if a bucket has Object Lock configured 3. Fix validateObjectLockHeaders() to check ObjectLockConfig instead of just versioningEnabled - this ensures object-lock headers are properly rejected on buckets without Object Lock enabled, which aligns with AWS S3 semantics 4. Make bucket creation with Object Lock atomic - set Object Lock config in the same CreateEntry call as bucket creation, preventing race conditions where bucket exists without Object Lock enabled 5. Properly handle Object Lock setup failures during bucket creation - if StoreObjectLockConfigurationInExtended fails, roll back the bucket creation and return an error instead of leaving a bucket without the requested Object Lock configuration This significantly improves DELETE latency for non-Object-Lock buckets, which is the common case (lockCheck time reduced from 1-10ms to ~1µs).	1 week ago
Chris Lu	9c266fac29	fix: CompleteMultipartUpload fails for uploads with more than 1000 parts (#7641 ) When completing a multipart upload, the code was listing parts with limit=0, which relies on the server's DirListingLimit default. In 'weed server' mode, this defaults to 1000, causing uploads with more than 1000 parts to fail with InvalidPart error. For a 38GB file with 8MB parts (AWS CLI default), this results in ~4564 parts, far exceeding the 1000 limit. Fix: Use explicit limit of MaxS3MultipartParts+1 (10001) to ensure all parts are listed regardless of server configuration. Fixes #7638	1 week ago
Chris Lu	28ac536280	fix: normalize Windows backslash paths in weed admin file uploads (#7636 ) fix: normalize Windows backslash paths in file uploads When uploading files from a Windows client to a Linux server, file paths containing backslashes were not being properly interpreted as directory separators. This caused files intended for subdirectories to be created in the root directory with backslashes in their filenames. Changes: - Add util.CleanWindowsPath and util.CleanWindowsPathBase helper functions in weed/util/fullpath.go for reusable path normalization - Use path.Join/path.Clean/path.Base instead of filepath equivalents for URL path semantics (filepath is OS-specific) - Apply normalization in weed admin handlers and filer upload parsing Fixes #7628	1 week ago
Chris Lu	89b6deaefa	fix: Use mime.FormatMediaType for RFC 6266 compliant Content-Disposition (#7635 ) Updated Content-Disposition header generation to use mime.FormatMediaType from the standard library, which properly handles non-ASCII characters and special characters per RFC 6266. Changes: - weed/server/common.go: Updated adjustHeaderContentDisposition to use mime.FormatMediaType instead of manual escaping with fileNameEscaper - weed/operation/upload_content.go: Updated multipart form Content-Disposition to use mime.FormatMediaType - weed/server/volume_server_handlers_read.go: Removed unused fileNameEscaper This ensures correct filename display for international users across filer downloads and file uploads. Fixes #7634	1 week ago
Chris Lu	f1384108e8	fix: Admin UI file browser uses https.client TLS config for filer communication (#7633 ) * fix: Admin UI file browser uses https.client TLS config for filer communication When filer is configured with HTTPS (https.filer section in security.toml), the Admin UI file browser was still using plain HTTP for file uploads, downloads, and viewing. This caused TLS handshake errors: 'http: TLS handshake error: client sent an HTTP request to an HTTPS server' This fix: - Updates FileBrowserHandlers to use the HTTPClient from weed/util/http/client which properly loads TLS configuration from https.client section - The HTTPClient automatically uses HTTPS when https.client.enabled=true - All file operations (upload, download, view) now respect TLS configuration - Falls back to plain HTTP if TLS client creation fails Fixes #7631 * fix: Address code review comments - Fix fallback client Transport wiring (properly assign transport to http.Client) - Use per-operation timeouts instead of unified 60s timeout: - uploadFileToFiler: 60s (for large file uploads) - ViewFile: 30s (original timeout) - isLikelyTextFile: 10s (original timeout) * fix: Proxy file downloads through Admin UI for mTLS support The DownloadFile function previously used browser redirect, which would fail when filer requires mutual TLS (client certificates) since the browser doesn't have these certificates. Now the Admin UI server proxies the download, using its TLS-aware HTTP client with the configured client certificates, then streams the response to the browser. * fix: Ensure HTTP response body is closed on non-200 responses In ViewFile, the response body was only closed on 200 OK paths, which could leak connections on non-200 responses. Now the body is always closed via defer immediately after checking err == nil, before checking the status code. * refactor: Extract fetchFileContent helper to reduce nesting in ViewFile Extracted the deeply nested file fetch logic (7+ levels) into a separate fetchFileContent helper method. This improves readability while maintaining the same TLS-aware behavior and error handling. * refactor: Use idiomatic Go error handling in fetchFileContent Changed fetchFileContent to return (string, error) instead of (content string, reason string) for idiomatic Go error handling. This enables error wrapping and standard 'if err != nil' checks. Also improved error messages to be more descriptive for debugging, including the HTTP status code and response body on non-200 responses. * refactor: Extract newClientWithTimeout helper to reduce code duplication - Added newClientWithTimeout() helper method that creates a temporary http.Client with the specified timeout, reusing the TLS transport - Updated uploadFileToFiler, fetchFileContent, DownloadFile, and isLikelyTextFile to use the new helper - Improved error message in DownloadFile to include response body for better debuggability (consistent with fetchFileContent) * fix: Address CodeRabbit review comments - Fix connection leak in isLikelyTextFile: ensure resp.Body.Close() is called even when status code is not 200 - Use http.NewRequestWithContext in DownloadFile so the filer request is cancelled when the client disconnects, improving resource cleanup * fix: Escape Content-Disposition filename per RFC 2616 Filenames containing quotes, backslashes, or special characters could break the Content-Disposition header or cause client-side parsing issues. Now properly escapes these characters before including in the header. * fix: Handle io.ReadAll errors when reading error response bodies In fetchFileContent and DownloadFile, the error from io.ReadAll was ignored when reading the filer's error response body. Now properly handles these errors to provide complete error messages. * fix: Fail fast when TLS client creation fails If TLS is enabled (https.client.enabled=true) but misconfigured, fail immediately with glog.Fatalf rather than silently falling back to plain HTTP. This prevents confusing runtime errors when the filer only accepts HTTPS connections. * fix: Use mime.FormatMediaType for RFC 6266 compliant Content-Disposition Replace manual escaping with mime.FormatMediaType which properly handles non-ASCII characters and special characters per RFC 6266, ensuring correct filename display for international users.	1 week ago
msementsov	c0dad091f1	Separate vacuum speed from replication speed (#7632 )	1 week ago
Chris Lu	4cc6a2a4e5	fix: Admin UI user creation fails before filer discovery (#7624 ) (#7625 ) * fix: Admin UI user creation fails before filer discovery (#7624) The credential manager's filer address function was not configured quickly enough after admin server startup, causing 'filer address function not configured' errors when users tried to create users immediately. Changes: - Use exponential backoff (200ms -> 5s) instead of fixed 5s polling for faster filer discovery on startup - Improve error messages to be more user-friendly and actionable Fixes #7624 * Add more debug logging to help diagnose filer discovery issues * fix: Use dynamic filer address function to eliminate race condition Instead of using a goroutine to wait for filer discovery before setting the filer address function, we now set a dynamic function immediately that returns the current filer address whenever it's called. This eliminates the race condition where users could create users before the goroutine completed, and provides clearer error messages when no filer is available. The dynamic function is HA-aware - it automatically returns whatever filer is currently available, adapting to filer failovers.	1 week ago
Chris Lu	5c1de633cb	mount: improve read throughput with parallel chunk fetching (#7627 ) * filer: remove lock contention during chunk download This addresses issue #7504 where a single weed mount FUSE instance does not fully utilize node network bandwidth when reading large files. The SingleChunkCacher was holding a mutex during the entire HTTP download, causing readers to block until the download completed. This serialized chunk reads even when multiple goroutines were downloading in parallel. Changes: - Add sync.Cond to SingleChunkCacher for efficient waiting - Move HTTP download outside the critical section in startCaching() - Use condition variable in readChunkAt() to wait for download completion - Add isComplete flag to track download state Now multiple chunk downloads can proceed truly in parallel, and readers wait efficiently using the condition variable instead of blocking on a mutex held during I/O operations. Ref: #7504 * filer: parallel chunk fetching within doReadAt This addresses issue #7504 by enabling parallel chunk downloads within a single read operation. Previously, doReadAt() processed chunks sequentially in a loop, meaning each chunk had to be fully downloaded before the next one started. This left significant network bandwidth unused when chunks resided on different volume servers. Changes: - Collect all chunk read tasks upfront - Use errgroup to fetch multiple chunks in parallel - Each chunk reads directly into its correct buffer position - Limit concurrency to prefetchCount (min 4) to avoid overwhelming the system - Handle gaps and zero-filling before parallel fetch - Trigger prefetch after parallel reads complete For a read spanning N chunks on different volume servers, this can now utilize up to N times the bandwidth of a single connection. Ref: #7504 * http: direct buffer read to reduce memory copies This addresses issue #7504 by reducing memory copy overhead during chunk downloads. Previously, RetriedFetchChunkData used ReadUrlAsStream which: 1. Allocated a 64KB intermediate buffer 2. Read data in 64KB chunks 3. Called a callback to copy each chunk to the destination For a 16MB chunk, this meant 256 copy operations plus the callback overhead. Profiling showed significant time spent in memmove. Changes: - Add readUrlDirectToBuffer() that reads directly into the destination - Add retriedFetchChunkDataDirect() for unencrypted, non-gzipped chunks - Automatically use direct read path when possible (cipher=nil, gzip=false) - Use http.NewRequestWithContext for proper cancellation For unencrypted chunks (the common case), this eliminates the intermediate buffer entirely, reading HTTP response bytes directly into the final destination buffer. Ref: #7504 * address review comments - Use channel (done) instead of sync.Cond for download completion signaling This integrates better with context cancellation patterns - Remove redundant groupErr check in reader_at.go (errors are already captured in task.err) - Remove buggy URL encoding logic from retriedFetchChunkDataDirect (The existing url.PathEscape on full URL is a pre-existing bug that should be fixed separately) * address review comments (round 2) - Return io.ErrUnexpectedEOF when HTTP response is truncated This prevents silent data corruption from incomplete reads - Simplify errgroup error handling by using g.Wait() error directly Remove redundant task.err field and manual error aggregation loop - Define minReadConcurrency constant instead of magic number 4 Improves code readability and maintainability Note: Context propagation to startCaching() is intentionally NOT changed. The downloaded chunk is a shared resource that may be used by multiple readers. Using context.Background() ensures the download completes even if one reader cancels, preventing data loss for other waiting readers. * http: inject request ID for observability in direct read path Add request_id.InjectToRequest() call to readUrlDirectToBuffer() for consistency with ReadUrlAsStream path. This ensures full-chunk reads carry the same tracing/correlation headers for server logs and metrics. * filer: consistent timestamp handling in sequential read path Use max(ts, task.chunk.ModifiedTsNs) in sequential path to match parallel path behavior. Also update ts before error check so that on failure, the returned timestamp reflects the max of all chunks processed so far. * filer: document why context.Background() is used in startCaching Add comment explaining the intentional design decision: the downloaded chunk is a shared resource that may be used by multiple concurrent readers. Using context.Background() ensures the download completes even if one reader cancels, preventing errors for other waiting readers. * filer: propagate context for reader cancellation Address review comment: pass context through ReadChunkAt call chain so that a reader can cancel its wait for a download. The key distinction is: - Download uses context.Background() - shared resource, always completes - Reader wait uses request context - can be cancelled individually If a reader cancels, it stops waiting and returns ctx.Err(), but the download continues to completion for other readers waiting on the same chunk. This properly handles the shared resource semantics while still allowing individual reader cancellation. * filer: use defer for close(done) to guarantee signal on panic Move close(s.done) to a defer statement at the start of startCaching() to ensure the completion signal is always sent, even if an unexpected panic occurs. This prevents readers from blocking indefinitely. * filer: remove unnecessary code - Remove close(s.cacheStartedCh) in destroy() - the channel is only used for one-time synchronization, closing it provides no benefit - Remove task := task loop variable capture - Go 1.22+ fixed loop variable semantics, this capture is no longer necessary (go.mod specifies Go 1.24.0) * filer: restore fallback to chunkCache when cacher returns no data Fix critical issue where ReadChunkAt would return 0,nil immediately if SingleChunkCacher couldn't provide data for the requested offset, without trying the chunkCache fallback. Now if cacher.readChunkAt returns n=0 and err=nil, we fall through to try chunkCache. * filer: add comprehensive tests for ReaderCache Tests cover: - Context cancellation while waiting for download - Fallback to chunkCache when cacher returns n=0, err=nil - Multiple concurrent readers waiting for same chunk - Partial reads at different offsets - Downloader cleanup when exceeding cache limit - Done channel signaling (no hangs on completion) * filer: prioritize done channel over context cancellation If data is already available (done channel closed), return it even if the reader's context is also cancelled. This avoids unnecessary errors when the download has already completed. * filer: add lookup error test and document test limitations Add TestSingleChunkCacherLookupError to test error handling when lookup fails. Document that full HTTP integration tests for SingleChunkCacher require global HTTP client initialization which is complex in unit tests. The download path is tested via FUSE integration tests. * filer: add tests that exercise SingleChunkCacher concurrency logic Add tests that use blocking lookupFileIdFn to exercise the actual SingleChunkCacher wait/cancellation logic: - TestSingleChunkCacherContextCancellationDuringLookup: tests reader cancellation while lookup is blocked - TestSingleChunkCacherMultipleReadersWaitForDownload: tests multiple readers waiting on the same download - TestSingleChunkCacherOneReaderCancelsOthersContinue: tests that when one reader cancels, other readers continue waiting These tests properly exercise the done channel wait/cancel logic without requiring HTTP calls - the blocking lookup simulates a slow download.	1 week ago
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>	1 week ago
Chris Lu	f9b4a4c396	fix: check freeEcSlot before evacuating EC shards to prevent data loss (#7621 ) * fix: check freeEcSlot before evacuating EC shards to prevent data loss Related to #7619 The moveAwayOneEcVolume function was missing the freeEcSlot check that exists in other EC shard placement functions. This could cause EC shards to be moved to volume servers that have no capacity, resulting in: 1. 0-byte shard files when disk is full 2. Data loss when source shards are deleted after 'successful' copy Changes: - Add freeEcSlot check before attempting to move EC shards - Sort destinations by both shard count and free slots - Refresh topology during evacuation to get updated slot counts - Log when nodes are skipped due to no free slots - Update freeEcSlot count after successful moves * fix: clarify comment wording per CodeRabbit review The comment stated 'after each move' but the code executes before calling moveAwayOneEcVolume. Updated to 'before moving each EC volume' for accuracy. * fix: collect topology once and track capacity changes locally Remove the topology refresh within the loop as it gives a false sense of correctness - the refreshed topology could still be stale (minutes old). Instead, we: 1. Collect topology once at the start 2. Track capacity changes ourselves via freeEcSlot decrement after each move This is more accurate because we know exactly what moves we've made, rather than relying on potentially stale topology refreshes. * fix: ensure partial EC volume moves are reported as failures Set hasMoved=false when a shard fails to move, even if previous shards succeeded. This prevents the caller from incorrectly assuming the entire volume was evacuated, which could lead to data loss if the source server is decommissioned based on this incorrect status. * fix: also reset hasMoved on moveMountedShardToEcNode error Same issue as the previous fix: if moveMountedShardToEcNode fails after some shards succeeded, hasMoved would incorrectly be true. Ensure partial moves are always reported as failures.	1 week ago
Chris Lu	fdb888729b	fix: properly handle errors in writeToFile to prevent 0-byte EC shards (#7620 ) Fixes #7619 The writeToFile function had two critical bugs that could cause data loss during EC shard evacuation when the destination disk is full: Bug 1: When os.OpenFile fails (e.g., disk full), the error was silently ignored and nil was returned. This caused the caller to think the copy succeeded. Bug 2: When dst.Write fails (e.g., 'no space left on device'), the error was completely ignored because the return value was not checked. When evacuating EC shards to a full volume server (especially on BTRFS): 1. OpenFile may succeed (creates 0-byte file inode) 2. Write fails with 'no space left on device' 3. Errors were ignored, function returned nil 4. Caller thinks copy succeeded and deletes source shard 5. Result: 0-byte shard on destination, data loss! This fix ensures both errors are properly returned, preventing data loss. Added unit tests to verify the fix.	1 week ago
Chris Lu	716f21fbd3	s3: support STREAMING-AWS4-HMAC-SHA256-PAYLOAD-TRAILER for signed chunked uploads with checksums (#7623 ) * s3: support STREAMING-AWS4-HMAC-SHA256-PAYLOAD-TRAILER for signed chunked uploads with checksums When AWS SDK v2 clients upload with both chunked encoding and checksum validation enabled, they use the x-amz-content-sha256 header value of STREAMING-AWS4-HMAC-SHA256-PAYLOAD-TRAILER instead of the simpler STREAMING-AWS4-HMAC-SHA256-PAYLOAD. This caused the chunked reader to not be properly activated, resulting in chunk-signature metadata being stored as part of the file content. Changes: - Add streamingSignedPayloadTrailer constant for the new header value - Update isRequestSignStreamingV4() to recognize this header - Update newChunkedReader() to handle this streaming type - Update calculateSeedSignature() to accept this header - Add unit test for signed streaming upload with trailer Fixes issue where Quarkus/AWS SDK v2 uploads with checksum validation resulted in corrupted file content containing chunk-signature data. * address review comments: add trailer signature to test, fix constant alignment * test: separate canonical trailer text (\n) from on-wire format (\r\n) * test: add negative test for invalid trailer signature * refactor: check HTTP method first in streaming auth checks (fail-fast) * test: handle crc32 Write error return for completeness * refactor: extract createTrailerStreamingRequest helper to reduce test duplication * fmt * docs: clarify test comment about trailer signature validation status * refactor: calculate chunk data length dynamically instead of hardcoding * Update weed/s3api/chunked_reader_v4_test.go Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * fix: use current time for signatures instead of hardcoded past date --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>	1 week ago
Chris Lu	a5ab05ec03	fix: S3 GetObject/HeadObject with PartNumber should return object ETag, not part ETag (#7622 ) AWS S3 behavior: when calling GetObject or HeadObject with the PartNumber query parameter, the ETag header should still return the complete object's ETag (e.g., 'abc123-4' for a 4-part multipart upload), not the individual part's ETag. The previous implementation incorrectly overrode the ETag with the part's ETag, causing test_multipart_get_part to fail. This fix removes the ETag override logic while keeping: - x-amz-mp-parts-count header (correct) - Content-Length adjusted to part size (correct) - Range calculation for part boundaries (correct)	1 week ago
chrislu	8d110b29dd	fmt	1 week ago
Chris Lu	39ba19eea6	filer: async empty folder cleanup via metadata events (#7614 ) * filer: async empty folder cleanup via metadata events Implements asynchronous empty folder cleanup when files are deleted in S3. Key changes: 1. EmptyFolderCleaner - New component that handles folder cleanup: - Uses consistent hashing (LockRing) to determine folder ownership - Each filer owns specific folders, avoiding duplicate cleanup work - Debounces delete events (10s delay) to batch multiple deletes - Caches rough folder counts to skip unnecessary checks - Cancels pending cleanup when new files are created - Handles both file and subdirectory deletions 2. Integration with metadata events: - Listens to both local and remote filer metadata events - Processes create/delete/rename events to track folder state - Only processes folders under /buckets/<bucket>/... 3. Removed synchronous empty folder cleanup from S3 handlers: - DeleteObjectHandler no longer calls DoDeleteEmptyParentDirectories - DeleteMultipleObjectsHandler no longer tracks/cleans directories - Cleanup now happens asynchronously via metadata events Benefits: - Non-blocking: S3 delete requests return immediately - Coordinated: Only one filer (the owner) cleans each folder - Efficient: Batching and caching reduce unnecessary checks - Event-driven: Folder deletion triggers parent folder check automatically * filer: add CleanupQueue data structure for deduplicated folder cleanup CleanupQueue uses a linked list for FIFO ordering and a hashmap for O(1) deduplication. Processing is triggered when: - Queue size reaches maxSize (default 1000), OR - Oldest item exceeds maxAge (default 10 minutes) Key features: - O(1) Add, Remove, Pop, Contains operations - Duplicate folders are ignored (keeps original position/time) - Testable with injectable time function - Thread-safe with mutex protection * filer: use CleanupQueue for empty folder cleanup Replace timer-per-folder approach with queue-based processing: - Use CleanupQueue for deduplication and ordered processing - Process queue when full (1000 items) or oldest item exceeds 10 minutes - Background processor checks queue every 10 seconds - Remove from queue on create events to cancel pending cleanup Benefits: - Bounded memory: queue has max size, not unlimited timers - Efficient: O(1) add/remove/contains operations - Batch processing: handle many folders efficiently - Better for high-volume delete scenarios * filer: CleanupQueue.Add moves duplicate to back with updated time When adding a folder that already exists in the queue: - Remove it from its current position - Add it to the back of the queue - Update the queue time to current time This ensures that folders with recent delete activity are processed later, giving more time for additional deletes to occur. * filer: CleanupQueue uses event time and inserts in sorted order Changes: - Add() now takes eventTime parameter instead of using current time - Insert items in time-sorted order (oldest at front) to handle out-of-order events - When updating duplicate with newer time, reposition to maintain sort order - Ignore updates with older time (keep existing later time) This ensures proper ordering when processing events from distributed filers where event arrival order may not match event occurrence order. * filer: remove unused CleanupQueue functions (SetNowFunc, GetAll) Removed test-only functions: - SetNowFunc: tests now use real time with past event times - GetAll: tests now use Pop() to verify order Kept functions used in production: - Peek: used in filer_notify_read.go - OldestAge: used in empty_folder_cleaner.go logging * filer: initialize cache entry on first delete/create event Previously, roughCount was only updated if the cache entry already existed, but entries were only created during executeCleanup. This meant delete/create events before the first cleanup didn't track the count. Now create the cache entry on first event, so roughCount properly tracks all changes from the start. * filer: skip adding to cleanup queue if roughCount > 0 If the cached roughCount indicates there are still items in the folder, don't bother adding it to the cleanup queue. This avoids unnecessary queue entries and reduces wasted cleanup checks. * filer: don't create cache entry on create event Only update roughCount if the folder is already being tracked. New folders don't need tracking until we see a delete event. * filer: move empty folder cleanup to its own package - Created weed/filer/empty_folder_cleanup package - Defined FilerOperations interface to break circular dependency - Added CountDirectoryEntries method to Filer - Exported IsUnderPath and IsUnderBucketPath helper functions * filer: make isUnderPath and isUnderBucketPath private These helpers are only used within the empty_folder_cleanup package.	1 week ago
Chris Lu	e361daa754	fix: SFTP HomeDir path translation for user operations (#7611 ) * fix: SFTP HomeDir path translation for user operations When users have a non-root HomeDir (e.g., '/sftp/user'), their SFTP operations should be relative to that directory. Previously, when a user uploaded to '/' via SFTP, the path was not translated to their home directory, causing 'permission denied for / for permission write'. This fix adds a toAbsolutePath() method that implements chroot-like behavior where the user's HomeDir becomes their root. All file and directory operations now translate paths through this method. Example: User with HomeDir='/sftp/user' uploading to '/' now correctly maps to '/sftp/user'. Fixes: https://github.com/seaweedfs/seaweedfs/issues/7470 * test: add SFTP integration tests Add comprehensive integration tests for the SFTP server including: - HomeDir path translation tests (verifies fix for issue #7470) - Basic file upload/download operations - Directory operations (mkdir, rmdir, list) - Large file handling (1MB test) - File rename operations - Stat/Lstat operations - Path edge cases (trailing slashes, .., unicode filenames) - Admin root access verification The test framework starts a complete SeaweedFS cluster with: - Master server - Volume server - Filer server - SFTP server with test user credentials Test users are configured in testdata/userstore.json: - admin: HomeDir=/ with full access - testuser: HomeDir=/sftp/testuser with access to home - readonly: HomeDir=/public with read-only access * fix: correct SFTP HomeDir path translation and add CI Fix path.Join issue where paths starting with '/' weren't joined correctly. path.Join('/sftp/user', '/file') returns '/file' instead of '/sftp/user/file'. Now we strip the leading '/' before joining. Test improvements: - Update go.mod to Go 1.24 - Fix weed binary discovery to prefer local build over PATH - Add stabilization delay after service startup - All 8 SFTP integration tests pass locally Add GitHub Actions workflow for SFTP tests: - Runs on push/PR affecting sftpd code or tests - Tests HomeDir path translation, file ops, directory ops - Covers issue #7470 fix verification * security: update golang.org/x/crypto to v0.45.0 Addresses security vulnerability in golang.org/x/crypto < 0.45.0 * security: use proper SSH host key verification in tests Replace ssh.InsecureIgnoreHostKey() with ssh.FixedHostKey() that verifies the server's host key matches the known test key we generated. This addresses CodeQL warning go/insecure-hostkeycallback. Also updates go.mod to specify go 1.24.0 explicitly. * security: fix path traversal vulnerability in SFTP toAbsolutePath The previous implementation had a critical security vulnerability: - Path traversal via '../..' could escape the HomeDir chroot jail - Absolute paths were not correctly prefixed with HomeDir The fix: 1. Concatenate HomeDir with userPath directly, then clean 2. Add security check to ensure final path stays within HomeDir 3. If traversal detected, safely return HomeDir instead Also adds path traversal prevention tests to verify the fix. * fix: address PR review comments 1. Fix SkipCleanup check to use actual test config instead of default - Added skipCleanup field to SftpTestFramework struct - Store config.SkipCleanup during Setup() - Use f.skipCleanup in Cleanup() instead of DefaultTestConfig() 2. Fix path prefix check false positive in mkdir - Changed from strings.HasPrefix(absPath, fs.user.HomeDir) - To: absPath == fs.user.HomeDir \|\| strings.HasPrefix(absPath, fs.user.HomeDir+"/") - Prevents matching partial directory names (e.g., /sftp/username when HomeDir is /sftp/user) * fix: check write permission on parent dir for mkdir Aligns makeDir's permission check with newFileWriter for consistency. To create a directory, a user needs write permission on the parent directory, not mkdir permission on the new directory path. * fix: refine SFTP path traversal logic and tests 1. Refine toAbsolutePath: - Use path.Join with strings.TrimPrefix for idiomatic path construction - Return explicit error on path traversal attempt instead of clamping - Updated all call sites to handle the error 2. Add Unit Tests: - Added sftp_server_test.go to verify toAbsolutePath logic - Covers normal paths, root path, and various traversal attempts 3. Update Integration Tests: - Updated PathTraversalPrevention test to reflect that standard SFTP clients sanitize paths before sending. The test now verifies successful containment within the jail rather than blocking (since the server receives a clean path). - The server-side blocking is verified by the new unit tests. 4. Makefile: - Removed -v from default test target * fix: address PR comments on tests and makefile 1. Enhanced Unit Tests: - Added edge cases (empty path, multiple slashes, trailing slash) to sftp_server_test.go 2. Makefile Improvements: - Added 'all' target as default entry point 3. Code Clarity: - Added comment to mkdir permission check explaining defensive nature of HomeDir check * fix: address PR review comments on permissions and tests 1. Security: - Added write permission check on target directory in renameEntry 2. Logging: - Changed dispatch log verbosity from V(0) to V(1) 3. Testing: - Updated Makefile .PHONY targets - Added unit test cases for empty/root HomeDir behavior in toAbsolutePath * fix: set SFTP starting directory to virtual root 1. Critical Fix: - Changed sftp.WithStartDirectory from fs.user.HomeDir to '/' - Prevents double-prefixing when toAbsolutePath translates paths - Users now correctly start at their virtual root which maps to HomeDir 2. Test Improvements: - Use pointer for homeDir in tests for clearer nil vs empty distinction * fix: clean HomeDir at config load time Clean HomeDir path when loading users from JSON config. This handles trailing slashes and other path anomalies at the source, ensuring consistency throughout the codebase and avoiding repeated cleaning on every toAbsolutePath call. * test: strengthen assertions and add error checking in SFTP tests 1. Add error checking for cleanup operations in TestWalk 2. Strengthen cwd assertion to expect '/' explicitly in TestCurrentWorkingDirectory 3. Add error checking for cleanup in PathTraversalPrevention test	2 weeks ago
Lisandro Pin	d59cc1b09f	Fix handling of fixed read-only volumes for `volume.check.disk`. (#7612 ) There's unfortunatley no way to tell whether a volume is flagged read-only because it got full, or because it is faulty. To address this, modify the check logic so all read-only volumes are processed; if no changes are written (i.e. the volume is healthy) it is kept as read-only. Volumes which are modified in this process are deemed fixed, and switched to writable.	2 weeks ago
Xiao Wei	3bcadc9f90	fix: update getVersioningState to signal non-existent buckets with Er… (#7613 ) * fix: update getVersioningState to signal non-existent buckets with ErrNotFound This change modifies the getVersioningState function to return filer_pb.ErrNotFound when a requested bucket does not exist, allowing callers to handle the situation appropriately, such as auto-creating the bucket in PUT handlers. This improves error handling and clarity in the API's behavior regarding bucket existence. * Update weed/s3api/s3api_bucket_config.go Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --------- Co-authored-by: 洪晓威 <xiaoweihong@deepglint.com> Co-authored-by: Chris Lu <chrislusf@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	e9da64f62a	fix: volume server healthz now checks local conditions only (#7610 ) This fixes issue #6823 where a single volume server shutdown would cause other healthy volume servers to fail their health checks and get restarted by Kubernetes, causing a cascading failure. Previously, the healthz handler checked if all replicated volumes could reach their remote replicas via GetWritableRemoteReplications(). When a volume server went down, the master would remove it from the volume location list. Other volume servers would then fail their healthz checks because they couldn't find all required replicas, causing Kubernetes to restart them. The healthz endpoint now only checks local conditions: 1. Is the server shutting down? 2. Is the server heartbeating with the master? This follows the principle that a health check should only verify the health of THIS server, not the overall cluster state. Fixes #6823	2 weeks ago
Chris Lu	5ed0b00fb9	Support separate volume server ID independent of RPC bind address (#7609 ) * pb: add id field to Heartbeat message for stable volume server identification This adds an 'id' field to the Heartbeat protobuf message that allows volume servers to identify themselves independently of their IP:port address. Ref: https://github.com/seaweedfs/seaweedfs/issues/7487 * storage: add Id field to Store struct Add Id field to Store struct and include it in CollectHeartbeat(). The Id field provides a stable volume server identity independent of IP:port. Ref: https://github.com/seaweedfs/seaweedfs/issues/7487 * topology: support id-based DataNode identification Update GetOrCreateDataNode to accept an id parameter for stable node identification. When id is provided, the DataNode can maintain its identity even when its IP address changes (e.g., in Kubernetes pod reschedules). For backward compatibility: - If id is provided, use it as the node ID - If id is empty, fall back to ip:port Ref: https://github.com/seaweedfs/seaweedfs/issues/7487 * volume: add -id flag for stable volume server identity Add -id command line flag to volume server that allows specifying a stable identifier independent of the IP address. This is useful for Kubernetes deployments with hostPath volumes where pods can be rescheduled to different nodes while the persisted data remains on the original node. Usage: weed volume -id=node-1 -ip=10.0.0.1 ... If -id is not specified, it defaults to ip:port for backward compatibility. Fixes https://github.com/seaweedfs/seaweedfs/issues/7487 * server: add -volume.id flag to weed server command Support the -volume.id flag in the all-in-one 'weed server' command, consistent with the standalone 'weed volume' command. Usage: weed server -volume.id=node-1 ... Ref: https://github.com/seaweedfs/seaweedfs/issues/7487 * topology: add test for id-based DataNode identification Test the key scenarios: 1. Create DataNode with explicit id 2. Same id with different IP returns same DataNode (K8s reschedule) 3. IP/PublicUrl are updated when node reconnects with new address 4. Different id creates new DataNode 5. Empty id falls back to ip:port (backward compatibility) Ref: https://github.com/seaweedfs/seaweedfs/issues/7487 * pb: add address field to DataNodeInfo for proper node addressing Previously, DataNodeInfo.Id was used as the node address, which worked when Id was always ip:port. Now that Id can be an explicit string, we need a separate Address field for connection purposes. Changes: - Add 'address' field to DataNodeInfo protobuf message - Update ToDataNodeInfo() to populate the address field - Update NewServerAddressFromDataNode() to use Address (with Id fallback) - Fix LookupEcVolume to use dn.Url() instead of dn.Id() Ref: https://github.com/seaweedfs/seaweedfs/issues/7487 * fix: trim whitespace from volume server id and fix test - Trim whitespace from -id flag to treat ' ' as empty - Fix store_load_balancing_test.go to include id parameter in NewStore call Ref: https://github.com/seaweedfs/seaweedfs/issues/7487 * refactor: extract GetVolumeServerId to util package Move the volume server ID determination logic to a shared utility function to avoid code duplication between volume.go and rack.go. Ref: https://github.com/seaweedfs/seaweedfs/issues/7487 * fix: improve transition logic for legacy nodes - Use exact ip:port match instead of net.SplitHostPort heuristic - Update GrpcPort and PublicUrl during transition for consistency - Remove unused net import Ref: https://github.com/seaweedfs/seaweedfs/issues/7487 * fix: add id normalization and address change logging - Normalize id parameter at function boundary (trim whitespace) - Log when DataNode IP:Port changes (helps debug K8s pod rescheduling) Ref: https://github.com/seaweedfs/seaweedfs/issues/7487	2 weeks ago
Chris Lu	51841a2e04	fix: skip cookie validation for EC volume deletion when SkipCookieCheck is set (#7608 ) fix: EC volume deletion issues Fixes #7489 1. Skip cookie check for EC volume deletion when SkipCookieCheck is set When batch deleting files from EC volumes with SkipCookieCheck=true (e.g., orphan file cleanup), the cookie is not available. The deletion was failing with 'unexpected cookie 0' because DeleteEcShardNeedle always validated the cookie. 2. Optimize doDeleteNeedleFromAtLeastOneRemoteEcShards to return early Return immediately when a deletion succeeds, instead of continuing to try all parity shards unnecessarily. 3. Remove useless log message that always logged nil error The log at V(1) was logging err after checking it was nil. Regression introduced in commit `7bdae5172` (Jan 3, 2023) when EC batch delete support was added.	2 weeks ago
Chris Lu	4f038820dc	Add disk-aware EC rebalancing (#7597 ) * Add placement package for EC shard placement logic - Consolidate EC shard placement algorithm for reuse across shell and worker tasks - Support multi-pass selection: racks, then servers, then disks - Include proper spread verification and scoring functions - Comprehensive test coverage for various cluster topologies * Make ec.balance disk-aware for multi-disk servers - Add EcDisk struct to track individual disks on volume servers - Update EcNode to maintain per-disk shard distribution - Parse disk_id from EC shard information during topology collection - Implement pickBestDiskOnNode() for selecting best disk per shard - Add diskDistributionScore() for tie-breaking node selection - Update all move operations to specify target disk in RPC calls - Improves shard balance within multi-disk servers, not just across servers * Use placement package in EC detection for consistent disk-level placement - Replace custom EC disk selection logic with shared placement package - Convert topology DiskInfo to placement.DiskCandidate format - Use SelectDestinations() for multi-rack/server/disk spreading - Convert placement results back to topology DiskInfo for task creation - Ensures EC detection uses same placement logic as shell commands * Make volume server evacuation disk-aware - Use pickBestDiskOnNode() when selecting evacuation target disk - Specify target disk in evacuation RPC requests - Maintains balanced disk distribution during server evacuations * Rename PlacementConfig to PlacementRequest for clarity PlacementRequest better reflects that this is a request for placement rather than a configuration object. This improves API semantics. * Rename DefaultConfig to DefaultPlacementRequest Aligns with the PlacementRequest type naming for consistency * Address review comments from Gemini and CodeRabbit Fix HIGH issues: - Fix empty disk discovery: Now discovers all disks from VolumeInfos, not just from EC shards. This ensures disks without EC shards are still considered for placement. - Fix EC shard count calculation in detection.go: Now correctly filters by DiskId and sums actual shard counts using ShardBits.ShardIdCount() instead of just counting EcShardInfo entries. Fix MEDIUM issues: - Add disk ID to evacuation log messages for consistency with other logging - Remove unused serverToDisks variable in placement.go - Fix comment that incorrectly said 'ascending' when sorting is 'descending' * add ec tests * Update ec-integration-tests.yml * Update ec_integration_test.go * Fix EC integration tests CI: build weed binary and update actions - Add 'Build weed binary' step before running tests - Update actions/setup-go from v4 to v6 (Node20 compatibility) - Update actions/checkout from v2 to v4 (Node20 compatibility) - Move working-directory to test step only * Add disk-aware EC rebalancing integration tests - Add TestDiskAwareECRebalancing test with multi-disk cluster setup - Test EC encode with disk awareness (shows disk ID in output) - Test EC balance with disk-level shard distribution - Add helper functions for disk-level verification: - startMultiDiskCluster: 3 servers x 4 disks each - countShardsPerDisk: track shards per disk per server - calculateDiskShardVariance: measure distribution balance - Verify no single disk is overloaded with shards	2 weeks ago
Lisandro Pin	ebb06a3908	Mutex command output writes for `volume.check.disk`. (#7605 ) Prevents potential screen garbling when operations are parallelized .Also simplifies logging by automatically adding newlines on output, if necessary. Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com>	2 weeks ago
Lisandro Pin	ee775825bc	Parallelize read-only volume check pass for `volume.check.disk`. (#7602 )	2 weeks ago
Chris Lu	733ca8e6df	Fix SSE-S3 copy: preserve encryption metadata and set chunk SSE type (#7598 ) * Fix SSE-S3 copy: preserve encryption metadata and set chunk SSE type Fixes GitHub #7562: Copying objects between encrypted buckets was failing. Root causes: 1. processMetadataBytes was re-adding SSE headers from source entry, undoing the encryption header filtering. Now uses dstEntry.Extended which is already filtered. 2. SSE-S3 streaming copy returned nil metadata. Now properly generates and returns SSE-S3 destination metadata (SeaweedFSSSES3Key, AES256 header) via ExecuteStreamingCopyWithMetadata. 3. Chunks created during streaming copy didn't have SseType set. Now sets SseType and per-chunk SseMetadata with chunk-specific IVs for SSE-S3, enabling proper decryption on GetObject. * Address review: make SSE-S3 metadata serialization failures fatal errors - In executeEncryptCopy: return error instead of just logging if SerializeSSES3Metadata fails - In createChunkFromData: return error if chunk SSE-S3 metadata serialization fails This ensures objects/chunks are never created without proper encryption metadata, preventing unreadable/corrupted data. * fmt * Refactor: reuse function names instead of creating WithMetadata variants - Change ExecuteStreamingCopy to return (EncryptionSpec, error) directly - Remove ExecuteStreamingCopyWithMetadata wrapper - Change executeStreamingReencryptCopy to return (EncryptionSpec, error) - Remove executeStreamingReencryptCopyWithMetadata wrapper - Update callers to ignore encryption spec with _ where not needed * Add TODO documenting large file SSE-S3 copy limitation The streaming copy approach encrypts the entire stream with a single IV but stores data in chunks with per-chunk IVs. This causes decryption issues for large files. Small inline files work correctly. This is a known architectural issue that needs separate work to fix. * Use chunk-by-chunk encryption for SSE-S3 copy (consistent with SSE-C/SSE-KMS) Instead of streaming encryption (which had IV mismatch issues for multi-chunk files), SSE-S3 now uses the same chunk-by-chunk approach as SSE-C and SSE-KMS: 1. Extended copyMultipartCrossEncryption to handle SSE-S3: - Added SSE-S3 source decryption in copyCrossEncryptionChunk - Added SSE-S3 destination encryption with per-chunk IVs - Added object-level metadata generation for SSE-S3 destinations 2. Updated routing in executeEncryptCopy/executeDecryptCopy/executeReencryptCopy to use copyMultipartCrossEncryption for all SSE-S3 scenarios 3. Removed streaming copy functions (shouldUseStreamingCopy, executeStreamingReencryptCopy) as they're no longer used 4. Added large file (1MB) integration test to verify chunk-by-chunk copy works This ensures consistent behavior across all SSE types and fixes data corruption that occurred with large files in the streaming copy approach. * fmt * fmt * Address review: fail explicitly if SSE-S3 metadata is missing Instead of silently ignoring missing SSE-S3 metadata (which could create unreadable objects), now explicitly fail the copy operation with a clear error message if: - First chunk is missing - First chunk doesn't have SSE-S3 type - First chunk has empty SSE metadata - Deserialization fails * Address review: improve comment to reflect full scope of chunk creation * Address review: fail explicitly if baseIV is empty for SSE-S3 chunk encryption If DestinationIV is not set when encrypting SSE-S3 chunks, the chunk would be created without SseMetadata, causing GetObject decryption to fail later. Now fails explicitly with a clear error message. Note: calculateIVWithOffset returns ([]byte, int) not ([]byte, error) - the int is a skip amount for intra-block alignment, not an error code. * Address review: handle 0-byte files in SSE-S3 copy For 0-byte files, there are no chunks to get metadata from. Generate an IV for the object-level metadata to ensure even empty files are properly marked as SSE-S3 encrypted. Also validate that we don't have a non-empty file with no chunks (which would indicate an internal error).	2 weeks ago
Chris Lu	099a351f3b	Fix issue #6847 : S3 chunked encoding includes headers in stored content (#7595 ) * Fix issue #6847: S3 chunked encoding includes headers in stored content - Add hasTrailer flag to s3ChunkedReader to track trailer presence - Update state transition logic to properly handle trailers in unsigned streaming - Enhance parseChunkChecksum to handle multiple trailer lines - Skip checksum verification for unsigned streaming uploads - Add test case for mixed format handling (unsigned headers with signed chunks) - Remove redundant CRLF reading in trailer processing This fixes the issue where chunk-signature and x-amz headers were appearing in stored file content when using chunked encoding with newer AWS SDKs. * Fix checksum validation for unsigned streaming uploads - Always validate checksum for data integrity regardless of signing - Correct checksum value in test case - Addresses PR review feedback about checksum verification * Add warning log when multiple checksum headers found in trailer - Log a warning when multiple valid checksum headers appear in trailers - Uses last checksum header as suggested by CodeRabbit reviewer - Improves debugging for edge cases with multiple checksum algorithms * Improve trailer parsing robustness in parseChunkChecksum - Remove redundant trimTrailingWhitespace call since readChunkLine already trims - Use bytes.TrimSpace for both key and value to handle whitespace around colon separator - Follows HTTP header specifications for optional whitespace around separators - Addresses Gemini Code Assist review feedback	2 weeks ago

1 2 3 4 5 ...

7785 Commits (1b13324fb79a8dcbc00044b90c357c41c2cc70c7)