This PR implements logic load/save persistent state information for storages
associated with volume servers, and reporting state changes back to masters
via heartbeat messages.
More work ensues!
See https://github.com/seaweedfs/seaweedfs/issues/7977 for details.
* Fix chown Input/output error on large file sets (Fixes#7911)
Implemented retry logic for MySQL/MariaDB backend to handle transient errors like deadlocks and timeouts.
* Fix syntax error: missing closing brace
* Refactor: Use %w for error wrapping and errors.As for extraction
* Fix: Disable retry logic inside transactions
There is a mistmatch in the conditionals for the definition and mounting of the `config-users` volume in the filer's template.
Volume definition:
```
{{- if and .Values.filer.s3.enabled .Values.filer.s3.enableAuth }}
```
Mount:
```
{{- if .Values.filer.s3.enableAuth }}
```
This leads to an invalid specification in the case where s3 is disabled but the enableAuth value is set to true, as it tries to mount in an undefined volume. I've fixed it here by adding the extra check to the latter conditional.
Fixes#7990
The issue was that the Charset constant used for generating secret keys
included the '/' character, which is URL-unsafe. When secret keys
containing '/' were used in HTTP requests, they would be URL-encoded,
causing a mismatch during signature verification.
Changes:
- Removed '/' from the Charset constant in weed/iam/constants.go
- Added TestGenerateSecretAccessKey_URLSafe to verify generated keys
don't contain URL-unsafe characters like '/' or '+'
This ensures all newly generated secret keys are URL-safe and will
work correctly with S3 authentication. Existing keys continue to work.
Previously, `weed mini` logic duplicated the credential loading process
by creating a temporary IAM config file from environment variables.
`auth_credentials.go` also had fallback logic to load these variables.
This change:
1. Updates `auth_credentials.go` to *always* check for and merge
AWS environment variable credentials (`AWS_ACCESS_KEY_ID`, etc.)
into the identity list. This ensures they are available regardless
of whether other configurations (static file or filer) are loaded.
2. Removes the redundant file creation logic from `weed/command/mini.go`.
3. Updates `weed mini` user messages to accurately reflect that
credentials are loaded from environment variables in-memory.
This results in a cleaner implementation where `weed/s3api` manages
all credential loading logic, and `weed mini` simply relies on it.
Refactored `NewIdentityAccessManagementWithStore` to remove mutual
exclusivity between static (file-based) and dynamic (filer-based)
configuration loading.
Previously, if a static config configuration was present (including the
legacy `IamConfig` option used by `weed mini`), it prevented loading
users from the filer.
Now, the system loads the static configuration first (if present), and
then *always* attempts to merge in the dynamic configuration from the
filer. This ensures that:
1. Static users (e.g. from `weed mini` env vars or `-s3.config`) are loaded and protected.
2. Dynamic users (e.g. created via Admin UI and stored in Filer) are also loaded and available.
`weed mini` sets the `-s3.iam.config` flag instead of `-s3.config`,
which populates `S3ApiServerOption.IamConfig`.
Previously, `NewIdentityAccessManagementWithStore` only checked
`option.Config`. This caused `weed mini` generated credentials (written
to a temp file passed via IamConfig) to be ignored, breaking S3 access
in mini mode even when environment variables were provided.
This change ensures we try to load the configuration from `IamConfig`
if `Config` is empty, restoring functionality for `weed mini`.
Fixed regression where AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY
environment variables were not being loaded as fallback credentials.
The issue was that configLoaded was set to true when filer call
succeeded, even if it returned an empty configuration. This blocked
the environment variable fallback logic.
Now only set configLoaded = true when we actually have loaded
identities, allowing env vars to work correctly in mini mode and
other scenarios where filer config is empty.
* fix(s3api): ensure static config file takes precedence over dynamic updates
When a static S3 configuration file is provided, avoid overwriting
the configuration from dynamic filer updates. This ensures the
documented "Highest Priority" for the configuration file is respected.
* refactor(s3api): implement merge-based static config with immutable identities
Static identities from config file are now immutable and protected from
dynamic updates. Dynamic identities (from admin panel) can be added and
updated without affecting static entries.
- Track identity names loaded from static config file
- Implement merge logic that preserves static identities
- Allow dynamic identities to be added or updated
- Remove blanket block on config file updates
* fix: address PR review comments for static config merge logic
Critical Bugs:
- Fix existingIdx always-false condition causing duplicate identities
- Fix race condition in static config initialization (move useStaticConfig inside mutex)
Security & Robustness:
- Add nil identity check in VerifyActionPermission to fail closed
- Mask access keys in STS validation logs to avoid exposing credentials
- Add nil guard for s3a.iam in subscription handler
Test Improvements:
- Add authCalled tracking to MockIAMIntegration for explicit verification
- Lower log level for static config messages to reduce noise
* fix: prevent duplicates and race conditions in merge logic
Data Integrity:
- Prevent service account credential duplicates on repeated merges
- Clean up stale accessKeyIdent entries when replacing identities
- Check existing credentials before appending
Concurrency Safety:
- Add synchronization to IsStaticConfig method
Test Improvements:
- Add mux route vars for proper GetBucketAndObject extraction
- Add STS session token header to trigger correct auth path
* Fix STS authorization in streaming/chunked uploads
During streaming/chunked uploads (SigV4 streaming), authorization happens
twice:
1. Initial authorization in authRequestWithAuthType() - works correctly
2. Second authorization in verifyV4Signature() - was failing for STS
The issue was that verifyV4Signature() only used identity.canDo() for
permission checks, which always denies STS identities (they have empty
Actions). This bypassed IAM authorization completely.
This commit makes verifyV4Signature() IAM-aware by adding the same
fallback logic used in authRequestWithAuthType():
- Traditional identities (with Actions) use legacy canDo() check
- STS/JWT identities (empty Actions) fall back to IAM authorization
Fixes: https://github.com/seaweedfs/seaweedfs/pull/7986#issuecomment-3723196038
* Add comprehensive unit tests for STS authorization in streaming uploads
Created test suite to verify that verifyV4Signature properly handles STS
identities by falling back to IAM authorization when shouldCheckPermissions
is true.
Tests cover:
- STS identities with IAM integration (allow and deny cases)
- STS identities without IAM integration (should deny)
- Traditional identities with Actions (canDo check)
- Permission check bypass when shouldCheckPermissions=false
- Specific streaming upload scenario from bug report
- Action determination based on HTTP method
All tests pass successfully.
* Refactor authorization logic to avoid duplication
Centralized the authorization logic into IdentityAccessManagement.VerifyActionPermission.
Updated auth_signature_v4.go and auth_credentials.go to use this new helper.
Updated tests to clarify that they mirror the centralized logic.
* Refactor tests to use VerifyActionPermission directly
Introduced IAMIntegration interface to facilitate mocking of internal IAM integration logic.
Updated IdentityAccessManagement to use the interface.
Updated tests to directy call VerifyActionPermission using a mocked IAM integration, eliminating duplicated logic in tests.
* fix(s3api): ensure static config file takes precedence and refactor tests
- Track if configuration was loaded from a static file using `useStaticConfig`.
- Ignore filer-based IAM updates when a static configuration is in use to respect "Highest Priority" rule.
- Refactor `TestVerifyV4SignatureWithSTSIdentity` to use `VerifyActionPermission` directly.
- Fix typed nil interface panic in authorization test.
* Fix: Add delimiter support to ListObjectVersions with proper truncation
- Implemented delimiter support to group keys into CommonPrefixes
- Fixed critical truncation bug: now merges versions and common prefixes into single sorted list before truncation
- Ensures total items never exceed MaxKeys (prevents infinite pagination loops)
- Properly sets NextKeyMarker and NextVersionIdMarker for pagination
- Added integration tests in test/s3/versioning/s3_versioning_delimiter_test.go
- Verified behavior matches S3 API specification
* Fix: Add delimiter support to ListObjectVersions with proper truncation
- Implemented delimiter support to group keys into CommonPrefixes
- Fixed critical truncation bug: now merges versions and common prefixes before truncation
- Added safety guard for maxKeys=0 to prevent panics
- Condensed verbose comments for better readability
- Added robust Go integration tests with nil checks for AWS SDK pointers
- Verified behavior matches S3 API specification
- Resolved compilation error in integration tests
- Refined pagination comments and ensured exclusive KeyMarker behavior
- Refactored listObjectVersions into helper methods for better maintainability
* Fix STS identity authorization by populating PolicyNames (#7985)
This commit fixes GitHub issue #7985 where STS-assumed identities
received empty identity.Actions, causing all S3 operations to be denied
even when the role had valid IAM policies attached.
Changes:
1. Populate PolicyNames field from sessionInfo.Policies in
validateSTSSessionToken() to enable IAM-based authorization for
STS identities
2. Fix bucket+objectKey path construction in canDo() method to include
proper slash separator between bucket and object key
3. Add comprehensive test suite to validate the fix and prevent
regression
The fix ensures that STS-assumed identities are properly authorized
through the IAM path when iamIntegration is available, allowing roles
with valid IAM policies to perform S3 operations as expected.
* Update STS identity tests to be more rigorous and use actual implementation path
* Fix regression in canDo() path concatenation
The previous fix blindly added a slash separator, which caused double
slashes when objectKey already started with a slash (common in existing
tests and some code paths). This broke TestCanDo and
TestObjectLevelListPermissions.
This commit updates the logic to only add the slash separator if
objectKey is not empty and does not already start with a slash.
This fixes the regressions while maintaining the fix for issue #7985.
* Refactor STS identity tests: extract helpers and simplify redundant logic
- Extracted setupTestSTSService and newTestIdentity helper functions
- Removed redundant if-else verification blocks that were already covered by assertions
- Cleaned up test cases to improve maintainability as suggested in code review.
* Add canDo() verification to STS identity tests
Address code review suggestion: verify that identities with empty
Actions correctly return false for canDo() checks, which confirms the
behavior that forces authorization to fall back to the IAM path.
* Simplify TestCanDoPathConstruction variable names
Rename expectedPath to fullPath and simplify logging/assertion logic
based on code review feedback.
* Refactor path construction and logging in canDo()
- Compute fullPath early and use it for logging to prevent double slashes
- Update TestCanDoPathConstruction to use robust path verification
- Add test case for objectKey with leading slash to ensure correct handling
* Implement Policy Attachment support for Object Store Users
- Added policy_names field to iam.proto and regenerated protos.
- Updated S3 API and IAM integration to support direct policy evaluation for users.
- Enhanced Admin UI to allow attaching policies to users via modals.
- Renamed 'policies' to 'policy_names' to clarify that it stores identifiers.
- Fixed syntax error in user_management.go.
* Fix policy dropdown not populating
The API returns {policies: [...]} but JavaScript was treating response as direct array.
Updated loadPolicies() to correctly access data.policies property.
* Add null safety checks for policy dropdowns
Added checks to prevent "undefined" errors when:
- Policy select elements don't exist
- Policy dropdowns haven't loaded yet
- User is being edited before policies are loaded
* Fix policy dropdown by using correct JSON field name
JSON response has lowercase 'name' field but JavaScript was accessing 'Name'.
Changed policy.Name to policy.name to match the IAMPolicy JSON structure.
* Fix policy names not being saved on user update
Changed condition from len(req.PolicyNames) > 0 to req.PolicyNames != nil
to ensure policy names are always updated when present in the request,
even if it's an empty array (to allow clearing policies).
* Add debug logging for policy names update flow
Added console.log in frontend and glog in backend to trace
policy_names data through the update process.
* Temporarily disable auto-reload for debugging
Commented out window.location.reload() so console logs are visible
when updating a user.
* Add detailed debug logging and alert for policy selection
Added console.log for each step and an alert to show policy_names value
to help diagnose why it's not being included in the request.
* Regenerate templ files for object_store_users
Ran templ generate to ensure _templ.go files are up to date with
the latest .templ changes including debug logging.
* Remove debug logging and restore normal functionality
Cleaned up temporary debug code (console.log and alert statements)
and re-enabled automatic page reload after user update.
* Add step-by-step alert debugging for policy update
Added 5 alert checkpoints to trace policy data through the update flow:
1. Check if policiesSelect element exists
2. Show selected policy values
3. Show userData.policy_names
4. Show full request body
5. Confirm server response
Temporarily disabled auto-reload to see alerts.
* Add version check alert on page load
Added alert on DOMContentLoaded to verify new JavaScript is being executed
and not cached by the browser.
* Compile templates using make
Ran make to compile all template files and install the weed binary.
* Add button click detection and make handleUpdateUser global
- Added inline alert on button click to verify click is detected
- Made handleUpdateUser a window-level function to ensure it's accessible
- Added alert at start of handleUpdateUser function
* Fix handleUpdateUser scope issue - remove duplicate definition
Removed duplicate function definition that was inside DOMContentLoaded.
Now handleUpdateUser is defined only once in global scope (line 383)
making it accessible when button onclick fires.
* Remove all duplicate handleUpdateUser definitions
Now handleUpdateUser is defined only once at the very top of the script
block (line 352), before DOMContentLoaded, ensuring it's available when
the button onclick fires.
* Add function existence check and error catching
Added alerts to check if handleUpdateUser is defined and wrapped
the function call in try-catch to capture any JavaScript errors.
Also added console.log statements to verify function definition.
* Simplify handleUpdateUser to non-async for testing
Removed async/await and added early return to test if function
can be called at all. This will help identify if async is causing
the issue.
* Add cache-control headers to prevent browser caching
Added no-cache headers to ShowObjectStoreUsers handler to prevent
aggressive browser caching of inline JavaScript in the HTML page.
* Fix syntax error - make handleUpdateUser async
Changed function back to async to fix 'await is only valid in async functions' error.
The cache-control headers are working - browser is now loading new code.
* Update version check to v3 to verify cache busting
Changed version alert to 'v3 - WITH EARLY RETURN' to confirm
the new code with early return statement is being loaded.
* Remove all debug code - clean implementation
Removed all alerts, console.logs, and test code.
Implemented clean policy update functionality with proper error handling.
* Add ETag header for cache-busting and update walkthrough
* Fix policy pre-selection in Edit User modal
- Updated admin.js editUser function to pre-select policies
- Root cause: duplicate editUser in admin.js overwrote inline version
- Added policy pre-selection logic to match inline template
- Verified working in browser: policies now pre-select correctly
* Fix policy persistence in handleUpdateUser
- Added policy_names field to userData payload in handleUpdateUser
- Policies were being lost because handleUpdateUser only sent email and actions
- Now collects selected policies from editPolicies dropdown
- Verified working: policies persist correctly across updates
* Fix XSS vulnerability in access keys display
- Escape HTML in access key display using escapeHtml utility
- Replace inline onclick handlers with data attributes
- Add event delegation for delete access key buttons
- Prevents script injection via malicious access key values
* Fix additional XSS vulnerabilities in user details display
- Escape HTML in actions badges (line 626)
- Escape HTML in policy_names badges (line 636)
- Prevents script injection via malicious action or policy names
* Fix XSS vulnerability in loadPolicies function
- Replace innerHTML string concatenation with DOM API
- Use createElement and textContent for safe policy name insertion
- Prevents script injection via malicious policy names
- Apply same pattern to both create and edit select elements
* Remove debug logging from UpdateObjectStoreUser
- Removed glog.V(0) debug statements
- Clean up temporary debugging code before production
* Remove duplicate handleUpdateUser function
- Removed inline handleUpdateUser that duplicated admin.js logic
- Removed debug console.log statement
- admin.js version is now the single source of truth
- Eliminates maintenance burden of keeping two versions in sync
* Refine user management and address code review feedback
- Preserve PolicyNames in UpdateUserPolicies
- Allow clearing actions in UpdateObjectStoreUser by checking for nil
- Remove version comment from object_store_users.templ
- Refactor loadPolicies for DRYness using cloneNode while keeping DOM API security
* IAM Authorization for Static Access Keys
* verified XSS Fixes in Templates
* fix div
* Enable writeback_cache and async_dio FUSE options
Fixes#7978
- Update mount_std.go to use EnableWriteback and EnableAsyncDio from go-fuse
- Add go.mod replace directive to use local go-fuse with capability support
- Remove temporary workaround that disabled these options
This enables proper FUSE kernel capability negotiation for writeback cache
and async direct I/O, improving performance for small writes and concurrent
direct I/O operations.
* Address PR review comments
- Remove redundant nil checks for writebackCache and asyncDio flags
- Update go.mod replace directive to use seaweedfs/go-fuse fork instead of local path
* Add TODO comment for go.mod replace directive
The replace directive must use a local path until seaweedfs/go-fuse#1 is merged.
After merge, this should be updated to use the proper version.
* Use seaweedfs/go-fuse v2.9.0 instead of local repository
Replace local path with seaweedfs/go-fuse v2.9.0 fork which includes
the writeback_cache and async_dio capability support.
* Use github.com/seaweedfs/go-fuse/v2 directly without replace directive
- Updated all imports to use github.com/seaweedfs/go-fuse/v2
- Removed replace directive from go.mod
- Using seaweedfs/go-fuse v2.0.0-20260106181308-87f90219ce09 which includes:
* writeback_cache and async_dio support
* Corrected module path
* Update to seaweedfs/go-fuse v2.9.1
Use v2.9.1 tag which includes the corrected module path
(github.com/seaweedfs/go-fuse/v2) along with writeback_cache
and async_dio support.
* opt: reduce ShardsInfo memory usage with bitmap and sorted slice
- Replace map[ShardId]*ShardInfo with sorted []ShardInfo slice
- Add ShardBits (uint32) bitmap for O(1) existence checks
- Use binary search for O(log n) lookups by shard ID
- Maintain sorted order for efficient iteration
- Add comprehensive unit tests and benchmarks
Memory savings:
- Map overhead: ~48 bytes per entry eliminated
- Pointers: 8 bytes per entry eliminated
- Total: ~56 bytes per shard saved
Performance improvements:
- Has(): O(1) using bitmap
- Size(): O(log n) using binary search (was O(1), acceptable tradeoff)
- Count(): O(1) using popcount on bitmap
- Iteration: Faster due to cache locality
* refactor: add methods to ShardBits type
- Add Has(), Set(), Clear(), and Count() methods to ShardBits
- Simplify ShardsInfo methods by using ShardBits methods
- Improves code readability and encapsulation
* opt: use ShardBits directly in ShardsCountFromVolumeEcShardInformationMessage
Avoid creating a full ShardsInfo object just to count shards.
Directly cast vi.EcIndexBits to ShardBits and use Count() method.
* opt: use strings.Builder in ShardsInfo.String() for efficiency
* refactor: change AsSlice to return []ShardInfo (values instead of pointers)
This completes the memory optimization by avoiding unnecessary pointer slices and potential allocations.
* refactor: rename ShardsCountFromVolumeEcShardInformationMessage to GetShardCount
* fix: prevent deadlock in Add and Subtract methods
Copy shards data from 'other' before releasing its lock to avoid
potential deadlock when a.Add(b) and b.Add(a) are called concurrently.
The previous implementation held other's lock while calling si.Set/Delete,
which acquires si's lock. This could deadlock if two goroutines tried to
add/subtract each other concurrently.
* opt: avoid unnecessary locking in constructor functions
ShardsInfoFromVolume and ShardsInfoFromVolumeEcShardInformationMessage
now build shards slice and bitmap directly without calling Set(), which
acquires a lock on every call. Since the object is local and not yet
shared, locking is unnecessary and adds overhead.
This improves performance during object construction.
* fix: rename 'copy' variable to avoid shadowing built-in function
The variable name 'copy' in TestShardsInfo_Copy shadowed the built-in
copy() function, which is confusing and bad practice. Renamed to 'siCopy'.
* opt: use math/bits.OnesCount32 and reorganize types
1. Replace manual popcount loop with math/bits.OnesCount32 for better
performance and idiomatic Go code
2. Move ShardSize type definition to ec_shards_info.go for better code
organization since it's primarily used there
* refactor: Set() now accepts ShardInfo for future extensibility
Changed Set(id ShardId, size ShardSize) to Set(shard ShardInfo) to
support future additions to ShardInfo without changing the API.
This makes the code more extensible as new fields can be added to
ShardInfo (e.g., checksum, location, etc.) without breaking the Set API.
* refactor: move ShardInfo and ShardSize to separate file
Created ec_shard_info.go to hold the basic shard types (ShardInfo and
ShardSize) for better code organization and separation of concerns.
* refactor: add ShardInfo constructor and helper functions
Added NewShardInfo() constructor and IsValid() method to better
encapsulate ShardInfo creation and validation. Updated code to use
the constructor for cleaner, more maintainable code.
* fix: update remaining Set() calls to use NewShardInfo constructor
Fixed compilation errors in storage and shell packages where Set() calls
were not updated to use the new NewShardInfo() constructor.
* fix: remove unreachable code in filer backup commands
Removed unreachable return statements after infinite loops in
filer_backup.go and filer_meta_backup.go to fix compilation errors.
* fix: rename 'new' variable to avoid shadowing built-in
Renamed 'new' to 'result' in MinusParityShards, Plus, and Minus methods
to avoid shadowing Go's built-in new() function.
* fix: update remaining test files to use NewShardInfo constructor
Fixed Set() calls in command_volume_list_test.go and
ec_rebalance_slots_test.go to use NewShardInfo() constructor.
Chris LuGitHubgemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
* Fix trust policy wildcard principal handling
This change fixes the trust policy validation to properly support
AWS-standard wildcard principals like {"Federated": "*"}.
Previously, the evaluatePrincipalValue() function would check for
context existence before evaluating wildcards, causing wildcard
principals to fail when the context key didn't exist. This forced
users to use the plain "*" workaround instead of the more specific
{"Federated": "*"} format.
Changes:
- Modified evaluatePrincipalValue() to check for "*" FIRST before
validating against context
- Added support for wildcards in principal arrays
- Added comprehensive tests for wildcard principal handling
- All existing tests continue to pass (no regressions)
This matches AWS IAM behavior where "*" in a principal field means
"allow any value" without requiring context validation.
Fixes: https://github.com/seaweedfs/seaweedfs/issues/7917
* Refactor: Move Principal matching to PolicyEngine
This refactoring consolidates all policy evaluation logic into the
PolicyEngine, improving code organization and eliminating duplication.
Changes:
- Added matchesPrincipal() and evaluatePrincipalValue() to PolicyEngine
- Added EvaluateTrustPolicy() method for direct trust policy evaluation
- Updated statementMatches() to check Principal field when present
- Made resource matching optional (trust policies don't have Resources)
- Simplified evaluateTrustPolicy() in iam_manager.go to delegate to PolicyEngine
- Removed ~170 lines of duplicate code from iam_manager.go
Benefits:
- Single source of truth for all policy evaluation
- Better code reusability and maintainability
- Consistent evaluation rules for all policy types
- Easier to test and debug
All tests pass with no regressions.
* Make PolicyEngine AWS-compatible and add unit tests
Changes:
1. AWS-Compatible Context Keys:
- Changed "seaweed:FederatedProvider" -> "aws:FederatedProvider"
- Changed "seaweed:AWSPrincipal" -> "aws:PrincipalArn"
- Changed "seaweed:ServicePrincipal" -> "aws:PrincipalServiceName"
- This ensures 100% AWS compatibility for trust policies
2. Added Comprehensive Unit Tests:
- TestPrincipalMatching: 8 test cases for Principal matching
- TestEvaluatePrincipalValue: 7 test cases for value evaluation
- TestTrustPolicyEvaluation: 6 test cases for trust policy evaluation
- TestGetPrincipalContextKey: 4 test cases for context key mapping
- Total: 25 new unit tests for PolicyEngine
All tests pass:
- Policy engine tests: 54 passed
- Integration tests: 9 passed
- Total: 63 tests passing
* Update context keys to standard AWS/OIDC formats
Replaced remaining seaweed: context keys with standard AWS and OIDC
keys to ensure 100% compatibility with AWS IAM policies.
Mappings:
- seaweed:TokenIssuer -> oidc:iss
- seaweed:Issuer -> oidc:iss
- seaweed:Subject -> oidc:sub
- seaweed:SourceIP -> aws:SourceIp
Also updated unit tests to reflect these changes.
All 63 tests pass successfully.
* Add advanced policy tests for variable substitution and conditions
Added comprehensive tests inspired by AWS IAM patterns:
- TestPolicyVariableSubstitution: Tests ${oidc:sub} variable in resources
- TestConditionWithNumericComparison: Tests sts:DurationSeconds condition
- TestMultipleConditionOperators: Tests combining StringEquals and StringLike
Results:
- TestMultipleConditionOperators: ✅ All 3 subtests pass
- Other tests reveal need for sts:DurationSeconds context population
These tests validate the PolicyEngine's ability to handle complex
AWS-compatible policy scenarios.
* Fix federated provider context and add DurationSeconds support
Changes:
- Use iss claim as aws:FederatedProvider (AWS standard)
- Add sts:DurationSeconds to trust policy evaluation context
- TestPolicyVariableSubstitution now passes ✅
Remaining work:
- TestConditionWithNumericComparison partially works (1/3 pass)
- Need to investigate NumericLessThanEquals evaluation
* Update trust policies to use issuer URL for AWS compatibility
Changed trust policy from using provider name ("test-oidc") to
using the issuer URL ("https://test-issuer.com") to match AWS
standard behavior where aws:FederatedProvider contains the OIDC
issuer URL.
Test Results:
- 10/12 test suites passing
- TestFullOIDCWorkflow: ✅ All subtests pass
- TestPolicyEnforcement: ✅ All subtests pass
- TestSessionExpiration: ✅ Pass
- TestPolicyVariableSubstitution: ✅ Pass
- TestMultipleConditionOperators: ✅ All subtests pass
Remaining work:
- TestConditionWithNumericComparison needs investigation
- One subtest in TestTrustPolicyValidation needs fix
* Fix S3 API tests for AWS compatibility
Updated all S3 API tests to use AWS-compatible context keys and
trust policy principals:
Changes:
- seaweed:SourceIP → aws:SourceIp (IP-based conditions)
- Federated: "test-oidc" → "https://test-issuer.com" (trust policies)
Test Results:
- TestS3EndToEndWithJWT: ✅ All 13 subtests pass
- TestIPBasedPolicyEnforcement: ✅ All 3 subtests pass
This ensures policies are 100% AWS-compatible and portable.
* Fix ValidateTrustPolicy for AWS compatibility
Updated ValidateTrustPolicy method to check for:
- OIDC: issuer URL ("https://test-issuer.com")
- LDAP: provider name ("test-ldap")
- Wildcard: "*"
Test Results:
- TestTrustPolicyValidation: ✅ All 3 subtests pass
This ensures trust policy validation uses the same AWS-compatible
principals as the PolicyEngine.
* Fix multipart and presigned URL tests for AWS compatibility
Updated trust policies in:
- s3_multipart_iam_test.go
- s3_presigned_url_iam_test.go
Changed "Federated": "test-oidc" → "https://test-issuer.com"
Test Results:
- TestMultipartIAMValidation: ✅ All 7 subtests pass
- TestPresignedURLIAMValidation: ✅ All 4 subtests pass
- TestPresignedURLGeneration: ✅ All 4 subtests pass
- TestPresignedURLExpiration: ✅ All 4 subtests pass
- TestPresignedURLSecurityPolicy: ✅ All 4 subtests pass
All S3 API tests now use AWS-compatible trust policies.
* Fix numeric condition evaluation and trust policy validation interface
Major updates to ensure robust AWS-compatible policy evaluation:
1. **Policy Engine**: Added support for `int` and `int64` types in `evaluateNumericCondition`, fixing issues where raw numbers in policy documents caused evaluation failures.
2. **Trust Policy Validation**: Updated `TrustPolicyValidator` interface and `STSService` to propagate `DurationSeconds` correctly during the double-validation flow (Validation -> STS -> Validation callback).
3. **IAM Manager**: Updated implementation to match the new interface and correctly pass `sts:DurationSeconds` context key.
Test Results:
- TestConditionWithNumericComparison: ✅ All 3 subtests pass
- All IAM and S3 integration tests pass (100%)
This resolves the final edge case with DurationSeconds numeric conditions.
* Fix MockTrustPolicyValidator interface and unreachable code warnings
Updates:
1. Updated MockTrustPolicyValidator.ValidateTrustPolicyForWebIdentity to match new interface signature with durationSeconds parameter
2. Removed unreachable code after infinite loops in filer_backup.go and filer_meta_backup.go to satisfy linter
Test Results:
- All STS tests pass ✅
- Build warnings resolved ✅
* Refactor matchesPrincipal to consolidate array handling logic
Consolidated duplicated logic for []interface{} and []string types by converting them to a unified []interface{} upfront.
* Fix malformed AWS docs URL in iam_manager.go comment
* dup
* Enhance IAM integration tests with negative cases and interface array support
Added test cases to TestTrustPolicyWildcardPrincipal to:
1. Verify rejection of roles when principal context does not match (negative test)
2. Verify support for principal arrays as []interface{} (simulating JSON unmarshaled roles)
* Fix syntax errors in filer_backup and filer_meta_backup
Restored missing closing braces for for-loops and re-added return statements.
The previous attempt to remove unreachable code accidentally broke the function structure.
Build now passes successfully.
* feat: add flags to disable WebDAV and Admin UI in weed mini
- Add -webdav flag (default: true) to optionally disable WebDAV server
- Add -admin.ui flag (default: true) to optionally disable Admin UI only (server still runs)
- Conditionally skip WebDAV service startup based on flag
- Pass disableUI flag to SetupRoutes to skip UI route registration
- Admin server still runs for gRPC and API access when UI is disabled
Addresses issue from https://github.com/seaweedfs/seaweedfs/pull/7833#issuecomment-3711924150
* refactor: use positive enableUI parameter instead of disableUI across admin server and handlers
* docs: update mini welcome message to list enabled components
* chore: remove unused welcomeMessageTemplate constant
* docs: split S3 credential message into separate sb.WriteString calls
* Fix flaky EC integration tests by collecting server logs on failure
The EC Integration Tests were experiencing flaky timeouts with errors like
"error reading from server: EOF" and master client reconnection attempts.
When tests failed, server logs were not collected, making debugging difficult.
Changes:
- Updated all test functions to use t.TempDir() instead of os.MkdirTemp()
and manual cleanup. t.TempDir() automatically preserves directories when
tests fail, ensuring logs are available for debugging.
- Modified GitHub Actions workflow to collect server logs from temp
directories when tests fail, including master.log and volume*.log files.
- Added explicit log collection step that searches for test temp directories
and copies them to artifacts for upload.
This will make debugging flaky test failures much easier by providing access
to the actual server logs showing what went wrong.
* Fix find command precedence in log collection
The -type d flag only applied to the first -name predicate because -o
has lower precedence than the implicit AND. Grouped the -name predicates
with escaped parentheses so -type d applies to all directory name patterns.
* fix: handle range requests on empty objects (size=0)
Range requests on empty objects were incorrectly being rejected with:
'invalid range start for ...: 0 >= 0'
The validation logic used 'startOffset >= totalSize' which failed when
both were 0, incorrectly rejecting valid range requests like bytes=0-1535
on 0-byte files.
Fix: Added special case handling before validation to properly return
416 Range Not Satisfiable for any range request on an empty object,
per RFC 7233.
Fixed at two locations (lines 873 and 1154) in s3api_object_handlers.go
* refactor: return 404 for directory objects, not 416
Per S3 semantics, GET requests on directory paths (without trailing "/")
should return 404 Not Found, not try to serve them as objects.
Updated fix to:
1. Check if entry.IsDirectory and return 404 (S3-compliant)
2. Only return 416 for true empty files (size=0, not directory)
This matches AWS S3 behavior where directories don't exist as objects
unless they're explicit directory markers ending with "/".
* reduce repeated info
* refactor: move directory check before range branching
This ensures that any Range header (including suffix ranges like bytes=-N)
on a directory path (without trailing slash) returns 404 (ErrNoSuchKey)
instead of potentially returning 416 or attempting to serve as an object.
Applied to both streamFromVolumeServers and streamFromVolumeServersWithSSE.
* refactoring
* Fix: prevent panic when swap file creation fails
* weed mount: fix race condition in swap file initialization
Ensure thread-safe access to sf.file and other state in NewSwapFileChunk
and FreeResource by using sf.chunkTrackingLock consistently. Also
set sf.file to nil after closing to prevent reuse.
* weed mount: improve swap directory creation logic
- Check error for os.MkdirAll and log it if it fails.
- Use 0700 permissions for the swap directory for better security.
- Improve error logging context.
* weed mount: add unit tests for swap file creation
Add tests to verify:
- Concurrent initialization of the swap file.
- Correct directory permissions (0700).
- Automatic directory recreation if deleted.
* weed mount: fix thread-safety in swap file unit tests
Use atomic.Uint32 to track failures within goroutines in
TestSwapFile_NewSwapFileChunk_Concurrent to avoid unsafe calls to
t.Errorf from multiple goroutines.
* weed mount: simplify swap file creation logic
Refactor the directory check and retry logic for better readability and
to avoid re-using the main error variable for directory creation errors.
Remove redundant error logging.
* weed mount: improve error checking in swap file tests
Explicitly check if NewSwapFileChunk returns nil to provide more
informative failures.
* weed mount: update DirtyPages interface to return error
Propagate errors from SaveDataAt when swap file creation fails. This
prevents potential panics in the write path.
* weed mount: handle AddPage errors in write paths
Update ChunkedDirtyPages and PageWriter to propagate errors and update
WFS.Write and WFS.CopyFileRange to return fuse.EIO on failure.
* weed mount: update swap directory creation error message
Change "recreate" to "create/recreate" to better reflect that this path
is also taken during the initial creation of the swap directory.
---------
Co-authored-by: lixiang58 <lixiang58@lenovo.com>
Co-authored-by: Chris Lu <chris.lu@gmail.com>
* fix: EC UI template error when viewing shard details
Fixed field name mismatch in volume.html where it was using .ShardDetails
instead of .Shards. Added a robust type conversion wrapper in templates.go
to handle int64 to uint64 conversion for bytesToHumanReadable.
Added regression test to ensure future stability.
* refactor: improve bytesToHumanReadable and test robustness
- Handled more integer types (uint32, int32, uint) in bytesToHumanReadable.
- Improved volume_test.go to verify both shards are formatted correctly.
* refactor: add bounds checking to bytesToHumanReadable
Added checks for negative values in signed integer types to avoid incorrect
formatting when converting to uint64.
Addressed feedback from coderabbitai.
* fix(iam): support both AWS standard and legacy IAM role ARN formats
Fix issue #7946 where SeaweedFS only recognized legacy IAM role ARN format
(arn:aws:iam::role/RoleName) but not the standard AWS format with account ID
(arn:aws:iam::ACCOUNT:role/RoleName). This was breaking EKS pod identity
integration which expects the standard format.
Changes:
- Update ExtractRoleNameFromArn() to handle both formats by searching for
'role/' marker instead of matching a fixed prefix
- Update ExtractRoleNameFromPrincipal() to clearly document both STS and IAM
formats it supports with or without account ID
- Simplify role ARN validation in validateRoleAssumptionForWebIdentity() and
validateRoleAssumptionForCredentials() to use the extraction function
- Add comprehensive test coverage with 25 test cases covering both formats
The fix maintains backward compatibility with legacy format while adding
support for standard AWS format with account ID.
Fixes: https://github.com/seaweedfs/seaweedfs/issues/7946
* docs: improve docstring coverage for ARN utility functions
- Add comprehensive package-level documentation
- Enhance ExtractRoleNameFromPrincipal docstring with parameter and return descriptions
- Enhance ExtractRoleNameFromArn docstring with detailed format documentation
- Add docstrings to test functions explaining test coverage
- Update all docstrings to 80%+ coverage for code review compliance
* refactor: improve ARN parsing code maintainability and error messages
- Define constants for ARN prefixes and markers (stsPrefix, stsAssumedRoleMarker, iamPrefix, iamRoleMarker)
- Replace hardcoded magic strings with named constants in ExtractRoleNameFromPrincipal and ExtractRoleNameFromArn
- Enhance error messages in sts_service.go to show expected ARN format when validation fails
- Error message now shows: 'arn:aws:iam::[ACCOUNT_ID:]role/ROLE_NAME' format
- Improves code readability and maintainability
- Facilitates future ARN format changes and debugging
* feat: add structured ARN type for better debugging and extensibility
Implements Option 2 (Structured ARN Type) from ARN handling comparison:
New Features:
- ARNInfo struct with Original, RoleName, AccountID, and Format fields
- ARNFormat enum (Legacy, Standard, Invalid) for type-safe format tracking
- ParseRoleARN() function for structured IAM role ARN parsing
- ParsePrincipalARN() function for structured STS/IAM principal parsing
Benefits:
- Better debugging: Can see original ARN, extracted components, and format type
- Extensible: Easy to add more fields (Region, Service, etc.) in future
- Type-safe: Format is an enum, not a string
- Backward compatible: Kept original string-based functions
STS Service Updates:
- Uses ParseRoleARN() for structured validation
- Logs ARN components at V(4) level for debugging (role, account, format)
- Better error context when validation fails
Test Coverage:
- 7 new tests for ParseRoleARN (legacy, standard, invalid formats)
- 7 new tests for ParsePrincipalARN (STS/IAM, legacy/standard)
- All 39 existing tests still pass
- Total: 53 ARN-related tests
Comparison with MinIO:
- More flexible: Supports both AWS formats (MinIO only supports MinIO format)
- Better tested: 53 tests vs MinIO's 8 tests
- Structured like MinIO but more practical for AWS use cases
* security: fix ARN parsing to prevent malicious ARN acceptance
Fix critical security vulnerability where malicious ARNs could bypass validation:
- ARNs like 'arn:aws:iam::123456789012:user/role/malicious' were incorrectly accepted
- The previous implementation used strings.Index to find 'role/' anywhere in the ARN
- This allowed non-role resource types to be accepted if they contained 'role/' in their path
Changes:
1. Updated ExtractRoleNameFromArn() to validate resource type is exactly 'role/'
2. Updated ExtractRoleNameFromPrincipal() to validate resource type is exactly 'assumed-role/'
3. Updated ParseRoleARN() to validate structure before extracting fields
4. Updated ParsePrincipalARN() to validate structure before extracting fields
5. Added 6 security test cases to prevent regression
The fix validates ARN structure by:
- Splitting on ':' to separate account ID from resource type
- Verifying resource type starts with exact marker ('role/' or 'assumed-role/')
- Only then extracting role name, account ID, and format
All 59 tests pass, including new security tests that verify malicious ARNs are rejected.
Fixes: GitHub Copilot review #3624499048
* test: add test cases for empty role names and improve validation
Address review feedback to improve edge case coverage:
1. Added test case for standard format with empty role name
- TestExtractRoleNameFromArn: arn:aws:iam::123456789012:role/
- TestParseRoleARN: arn:aws:iam::123456789012:role/
2. Added empty role name validation for STS ARNs in ParsePrincipalARN
- Now matches ParseRoleARN behavior
- Prevents ARNs like arn:aws:sts::assumed-role/ from having valid Format
3. Added test cases for empty STS role names
- TestParsePrincipalARN: arn:aws:sts::assumed-role/
- TestParsePrincipalARN: arn:aws:sts::123456789012:assumed-role/
All 65 tests pass (15 for ExtractRoleNameFromArn, 10 for ExtractRoleNameFromPrincipal,
8 for ParseRoleARN, 9 for ParsePrincipalARN, 4 security user ARNs, 2 security STS,
plus existing tests).
* refactor: simplify ARNInfo by removing Format enum
Remove ARNFormat enum (ARNFormatLegacy, ARNFormatStandard, ARNFormatInvalid)
as it's not needed for backward compatibility. Simplifications:
1. Removed ARNFormat type and all format constants
2. Removed Format field from ARNInfo struct
3. Validation now checks if RoleName is empty (simpler and clearer)
4. AccountID presence already distinguishes legacy (empty) from standard (non-empty) formats
5. Updated STS service to check RoleName emptiness instead of Format field
6. Improved debug logging to explicitly show "(legacy format)" or "(standard format)"
Benefits:
- Simpler code with fewer concepts
- AccountID field already provides format information
- Validation is clearer: empty RoleName = invalid ARN
- All 65 tests still pass
This change maintains the same functionality while reducing code complexity.
No backward compatibility concerns as the structured ARN parsing is new.
* test: add comprehensive edge case tests for ARN parsing
Add 4 new test functions covering:
- Multiple role markers in paths (e.g., role/role/name)
- Consecutive slashes in role paths (preserved as valid components)
- Special characters valid in AWS role names (+=,.@-_)
- Extremely long role names near AWS limits
These tests verify the parser's resilience to edge cases and ensure
proper handling of various valid role name formats and special characters.
Resolves the following error reported in #7949:
```
I0103 21:38:30.230662 s3.go:275 Starting S3 API Server with standard IAM
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x38ca961]
goroutine 102 [running]:
github.com/seaweedfs/seaweedfs/weed/command.(*S3Options).startS3Server(0x7caf840)
/go/src/github.com/seaweedfs/seaweedfs/weed/command/s3.go:295 +0x741
github.com/seaweedfs/seaweedfs/weed/command.runFiler.func1(...)
/go/src/github.com/seaweedfs/seaweedfs/weed/command/filer.go:244
created by github.com/seaweedfs/seaweedfs/weed/command.runFiler in goroutine 1
/go/src/github.com/seaweedfs/seaweedfs/weed/command/filer.go:242 +0x353
```
* fix(gcs): resolve credential conflict and improve backup logging
- Workaround GCS SDK's "multiple credential options" error by manually constructing an authenticated HTTP client.
- Include source entry path in filer backup error logs for better visibility on missing volumes/404s.
* fix: address PR review feedback
- Add nil check for EventNotification in getSourceKey
- Avoid reassigning google_application_credentials parameter in gcs_sink.go
* fix(gcs): return errors instead of calling glog.Fatalf in initialize
Adheres to Go best practices and allows for more graceful failure handling by callers.
* read from bind ip
* Add documentation for issue #7941 fix
* ensure auth
* rm FIX_ISSUE_7941.md
* Integrate STS session token validation into V4 signature verification
- Check for X-Amz-Security-Token header in verifyV4Signature
- Call validateSTSSessionToken for STS requests
- Skip regular access key lookup and expiration check for STS sessions
* Fix variable scoping in verifyV4Signature for STS session token validation
* Add ErrExpiredToken error for better AWS S3 compatibility with STS session tokens
* Support STS session token in query parameters for presigned URLs
* Fix nil pointer dereference in validateSTSSessionToken
* Enhance STS token validation with detailed error diagnostics and logging
* Fix missing credentials in STSSessionClaims.ToSessionInfo()
* test: Add comprehensive STS session claims validation tests
- TestSTSSessionClaimsToSessionInfo: Validates basic claims conversion
- TestSTSSessionClaimsToSessionInfoCredentialGeneration: Verifies credential generation
- TestSTSSessionClaimsToSessionInfoPreservesAllFields: Ensures all fields are preserved
- TestSTSSessionClaimsToSessionInfoEmptyFields: Tests handling of empty/nil fields
- TestSTSSessionClaimsToSessionInfoCredentialExpiration: Validates expiration handling
All tests pass with proper timing tolerance for credential generation.
* perf: Reuse CredentialGenerator instance for STS session claims
Optimize ToSessionInfo() to reuse a package-level defaultCredentialGenerator
instead of allocating a new CredentialGenerator on every call. This reduces
allocation overhead since this method is called frequently during signature
verification (potentially once per request).
The CredentialGenerator is stateless and deterministic, making it safe to
reuse across concurrent calls without synchronization.
* refactor: Surface credential generation errors and remove sensitive logging
Two improvements to error handling and security:
1. weed/iam/sts/session_claims.go:
- Add logging for credential generation failures in ToSessionInfo()
- Wrap errors with context (session ID) to aid debugging
- Use glog.Warningf() to surface errors instead of silently swallowing them
- Add fmt import for error wrapping
2. weed/s3api/auth_signature_v4.go:
- Remove debug logging of actual access key IDs (glog.V(2) call)
- Security improvement: avoid exposing sensitive access keys even at debug level
- Keep warning-level logging that shows only count of available keys
This ensures credential generation failures are observable while protecting
sensitive authentication material from logs.
* test: Verify deterministic credential generation in session claims tests
Update TestSTSSessionClaimsToSessionInfoCredentialGeneration to properly verify
deterministic credential generation:
- Remove misleading comment about 'randomness' - parts of credentials ARE deterministic
- Add assertions that AccessKeyId is identical for same SessionId (hash-based, deterministic)
- Add assertions that SessionToken is identical for same SessionId (hash-based, deterministic)
- Verify Expiration matches when SessionId is identical
- Document that SecretAccessKey is NOT deterministic (uses random.Read)
- Truncate expiresAt to second precision to avoid timing issues
This test now properly verifies that the deterministic components of credential
generation work correctly while acknowledging the cryptographic randomness of
the secret access key.
* test(sts): Assert credentials expiration relative to now in credential expiration tests
Replace wallclock assertions comparing tc.expiresAt to time.Now() (which only verified test setup)
with assertions that check sessionInfo.Credentials.Expiration relative to time.Now(), thus
exercising the code under test. Include clarifying comment for intent.
* feat(sts): Add IsExpired helpers and use them in expiration tests
- Add Credentials.IsExpired() and SessionInfo.IsExpired() in new file session_helpers.go.
- Update TestSTSSessionClaimsToSessionInfoCredentialExpiration to use helpers for clearer intent.
* test: revert test-only IsExpired helpers; restore direct expiration assertions
Remove session_helpers.go and update TestSTSSessionClaimsToSessionInfoCredentialExpiration to assert against sessionInfo.Credentials.Expiration directly as requested by reviewer.,
* fix(s3api): restore error return when access key not found
Critical fix: The previous cleanup of sensitive logging inadvertently removed
the error return statement when access key lookup fails. This caused the code
to continue and call isCredentialExpired() on nil pointer, crashing the server.
This explains EOF errors in CORS tests - server was panicking on requests
with invalid keys.
* fix(sts): make secret access key deterministic based on sessionId
CRITICAL FIX: The secret access key was being randomly generated, causing
signature verification failures when the same session token was used twice:
1. AssumeRoleWithWebIdentity generates random secret key X
2. Client signs request using secret key X
3. Server validates token, regenerates credentials via ToSessionInfo()
4. ToSessionInfo() calls generateSecretAccessKey(), which generates random key Y
5. Server tries to verify signature using key Y, but signature was made with X
6. Signature verification fails (SignatureDoesNotMatch)
Solution: Make generateSecretAccessKey() deterministic by using SHA256 hash
of 'secret-key:' + sessionId, just like generateAccessKeyId() already does.
This ensures:
- AssumeRoleWithWebIdentity generates deterministic secret key from sessionId
- ToSessionInfo() regenerates the same secret key from the same sessionId
- Client signature verification succeeds because keys match
Fixes: AWS SDK v2 CORS tests failing with 'ExpiredToken' errors
Affected files:
- weed/iam/sts/token_utils.go: Updated generateSecretAccessKey() signature
and implementation to be deterministic
- Updated GenerateTemporaryCredentials() to pass sessionId parameter
Tests: All 54 STS tests pass with this fix
* test(sts): add comprehensive secret key determinism test coverage
Updated tests to verify that secret access keys are now deterministic:
1. Updated TestSTSSessionClaimsToSessionInfoCredentialGeneration:
- Changed comment from 'NOT deterministic' to 'NOW deterministic'
- Added assertion that same sessionId produces identical secret key
- Explains why this is critical for signature verification
2. Added TestSecretAccessKeyDeterminism (new dedicated test):
- Verifies secret key is identical across multiple calls with same sessionId
- Verifies access key ID and session token are also identical
- Verifies different sessionIds produce different credentials
- Includes detailed comments explaining why determinism is critical
These tests ensure that the STS implementation correctly regenerates
deterministic credentials during signature verification. Without
determinism, signature verification would always fail because the
server would use different secret keys than the client used to sign.
* refactor(sts): add explicit zero-time expiration handling
Improved defensive programming in IsExpired() methods:
1. Credentials.IsExpired():
- Added explicit check for zero-time expiration (time.Time{})
- Treats uninitialized credentials as expired
- Prevents accidentally treating uninitialized creds as valid
2. SessionInfo.IsExpired():
- Added same explicit zero-time check
- Treats uninitialized sessions as expired
- Protects against bugs where sessions might not be properly initialized
This is important because time.Now().After(time.Time{}) returns true,
but explicitly checking for zero time makes the intent clear and helps
catch initialization bugs during code review and debugging.
* refactor(sts): remove unused IsExpired() helper functions
The session_helpers.go file contained two unused IsExpired() methods:
- Credentials.IsExpired()
- SessionInfo.IsExpired()
These were never called anywhere in the codebase. The actual expiration
checks use:
- isCredentialExpired() in weed/s3api/auth_credentials.go (S3 auth)
- Direct time.Now().After() checks
Removing unused code improves code clarity and reduces maintenance burden.
* fix(auth): pass STS session token to IAM authorization for V4 signature auth
CRITICAL FIX: Session tokens were not being passed to the authorization
check when using AWS Signature V4 authentication with STS credentials.
The bug:
1. AWS SDK sends request with X-Amz-Security-Token header (V4 signature)
2. validateSTSSessionToken validates the token, creates Identity with PrincipalArn
3. authorizeWithIAM only checked X-SeaweedFS-Session-Token (JWT auth header)
4. Since it was empty, fell into 'static V4' branch which set SessionToken = ''
5. AuthorizeAction returned ErrAccessDenied because SessionToken was empty
The fix (in authorizeWithIAM):
- Check X-SeaweedFS-Session-Token first (JWT auth)
- If empty, fallback to X-Amz-Security-Token header (V4 STS auth)
- If still empty, check X-Amz-Security-Token query param (presigned URLs)
- When session token is found with PrincipalArn, use 'STS V4 signature' path
- Only use 'static V4' path when there's no session token
This ensures:
- JWT Bearer auth with session tokens works (existing path)
- STS V4 signature auth with session tokens works (new path)
- Static V4 signature auth without session tokens works (existing path)
Logging updated to distinguish:
- 'JWT-based IAM authorization'
- 'STS V4 signature IAM authorization' (new)
- 'static V4 signature IAM authorization' (clarified)
* test(s3api): add comprehensive STS session token authorization test coverage
Added new test file auth_sts_v4_test.go with comprehensive tests for the
STS session token authorization fix:
1. TestAuthorizeWithIAMSessionTokenExtraction:
- Verifies X-SeaweedFS-Session-Token is extracted from JWT auth headers
- Verifies X-Amz-Security-Token is extracted from V4 STS auth headers
- Verifies X-Amz-Security-Token is extracted from query parameters (presigned URLs)
- Verifies JWT tokens take precedence when both are present
- Regression test for the bug where V4 STS tokens were not being passed to authorization
2. TestSTSSessionTokenIntoCredentials:
- Verifies STS credentials have all required fields (AccessKeyId, SecretAccessKey, SessionToken)
- Verifies deterministic generation from sessionId (same sessionId = same credentials)
- Verifies different sessionIds produce different credentials
- Critical for signature verification: same session must regenerate same secret key
3. TestActionConstantsForV4Auth:
- Verifies S3 action constants are available for authorization checks
- Ensures ACTION_READ, ACTION_WRITE, etc. are properly defined
These tests ensure that:
- V4 Signature auth with STS tokens properly extracts and uses session tokens
- Session tokens are prioritized correctly (JWT > X-Amz-Security-Token header > query param)
- STS credentials are deterministically generated for signature verification
- The fix for passing STS session tokens to authorization is properly covered
All 3 test functions pass (6 test cases total).
* refactor(s3api): improve code quality and performance
- Rename authorization path constants to avoid conflict with existing authType enum
- Replace nested if/else with clean switch statement in authorizeWithIAM()
- Add determineIAMAuthPath() helper for clearer intent and testability
- Optimize key counting in auth_signature_v4.go: remove unnecessary slice allocation
- Fix timing assertion in session_claims_test.go: use WithinDuration for symmetric tolerance
These changes improve code readability, maintainability, and performance while
maintaining full backward compatibility and test coverage.
* refactor(s3api): use typed iamAuthPath for authorization path constants
- Define iamAuthPath as a named string type (similar to existing authType enum)
- Update constants to use explicit type: iamAuthPathJWT, iamAuthPathSTS_V4, etc.
- Update determineIAMAuthPath() to return typed iamAuthPath
- Improves type safety and prevents accidental string value misuse
* Add documentation for issue #7941 fix
* rm FIX_ISSUE_7941.md
* Standardize -ip.bind flags to default to empty string and fall back to -ip option
- Change s3 command -ip.bind default logic to use -ip instead of localhost
- Change sftp command -ip.bind default to empty and fall back to 0.0.0.0
- Update help text for consistency
* Fix compilation error: add -ip flag to s3 command and update bindIp fallback
* Revert -ip flag addition for s3 command, set bindIp fallback to 0.0.0.0
* Update s3 command -ip.bind help text to reflect correct default behavior
* fix: directory incorrectly listed as object in S3 ListObjects
Regular directories (without MIME type) were only added to CommonPrefixes
when delimiter was exactly '/'. This caused directories to be silently
skipped for other delimiter values.
Changed the condition from 'delimiter == "/"' to 'delimiter != ""' to
ensure directories are correctly added to CommonPrefixes for any delimiter.
Fixes issue where directories like 'data/file.vhd' were being returned as
objects instead of prefixes in ListObjects responses.
* fix: complete the directory listing fix for all delimiters
Address reviewer feedback:
- Changed doListFilerEntries line 549 from 'delimiter != "/"' to 'delimiter == ""'
This ensures directories are yielded to the callback for ANY delimiter, not just "/"
- Parameterized test to verify fix works with multiple delimiters (/, _, :)
The previous fix only addressed line 260 but line 549 was still causing
recursion for non-"/" delimiters, preventing directories from being
added to CommonPrefixes.
* docs: update test comment to reflect multiple delimiters
Address reviewer feedback - clarify that the test verifies behavior
for any non-empty delimiter, not just '/'.
* docs: clarify test comment with delimiter examples
Add specific examples of delimiters ('/', '_', ':') to make it clear
that the test verifies behavior with multiple delimiter types.
* fix: revert line 549 to original logic, only line 260 needed changing
The fix for directories being listed as objects only required changing
line 260 from 'delimiter == "/"' to 'delimiter != ""'.
Line 549 should remain as 'delimiter != "/"' to allow recursion for
delimiters that don't exist in paths (e.g., delimiter=z for paths like
b/a/c). This is correct S3 behavior.
Updated test to only verify delimiter="/" since other delimiters should
recurse into directories to find actual files.
* docs: clarify test scope in directory listing test
* optimize: enable immediate EC shard reporting during startup
Ported the immediate EC shard reporting feature from Enterprise to Community version.
This allows the master to be notified about EC shards immediately during volume server startup,
instead of waiting for the first heartbeat.
Changes:
1. Updated NewStore to initialize notification channels BEFORE loading volumes (fixes potential nil panic).
2. Added ecShardNotifyHandler to report EC shards to NewEcShardsChan during startup.
3. Implemented non-blocking channel send for EC reporting to prevent deadlock when loading many EC shards (fixing the enterprise bug 17ac1290c).
4. Updated DiskLocation and EC loading logic to support the callback.
This optimization improves cluster state consistency and startup speed for EC-heavy clusters.
* optimize: report actual EC shard size during startup
* optimize: increase notification channel buffer size to 1024
* optimize: fix variable shadowing in store.go
* feat(iam): add TLS configuration support for OIDC provider
Adds tlsCaCert and tlsInsecureSkipVerify options to OIDC provider configuration to allow using custom CA certificates and skipping verification in development environments.
* fix: use SystemCertPool for custom CA and add security warning
- Use x509.SystemCertPool() to preserve trust in public CAs
- Add warning log when TLSInsecureSkipVerify is enabled
- Addresses code review feedback from gemini-code-assist
* docs: enhance TLS configuration field documentation
- Add explicit warning about TLSInsecureSkipVerify production usage
- Clarify TLSCACert is for custom/self-signed certificates
* security: enforce TLS 1.2 minimum version
- Set MinVersion to TLS 1.2 to prevent downgrade attacks
- Ensures secure communication with OIDC providers
* security: validate CA cert path is absolute
- Add filepath.IsAbs check before reading CA certificate
- Prevents reading unintended files from relative paths
- Fail fast on misconfigured paths
* Fix: Add -admin.grpc flag to worker for explicit gRPC port configuration
* Fix(helm): Add adminGrpcServer to worker configuration
* Refactor: Support host:port.grpcPort address format, revert -admin.grpc flag
* Helm: Conditionally append grpcPort to worker admin address
* weed/admin: fix "send on closed channel" panic in worker gRPC server
Make unregisterWorker connection-aware to prevent closing channels
belonging to newer connections.
* weed/worker: improve gRPC client stability and logging
- Fix goroutine leak in reconnection logic
- Refactor reconnection loop to exit on success and prevent busy-waiting
- Add session identification and enhanced logging to client handlers
- Use constant for internal reset action and remove unused variables
* weed/worker: fix worker state initialization and add lifecycle logs
- Revert workerState to use running boolean correctly
- Prevent handleStart failing by checking running state instead of startTime
- Add more detailed logs for worker startup events
This adds support for the new FUSE performance options to the 'weed fuse' command,
matching the functionality available in 'weed mount'.
Added options:
- writebackCache: Enable FUSE writeback cache for improved write performance
- asyncDio: Enable async direct I/O for better concurrency
- cacheSymlink: Enable symlink caching to reduce metadata lookups
- sys.novncache: (macOS only) Disable vnode name caching to avoid stale data
These options can now be used with mount -t weed:
mount -t weed fuse /mnt -o "filer=localhost:8888,writebackCache=true,asyncDio=true"
This ensures feature parity between 'weed mount' and 'weed fuse' commands.
* mount: add -asyncDio flag for async direct I/O
This adds support for async direct I/O via the -asyncDio flag.
Async DIO enables the FUSE_CAP_ASYNC_DIO capability, allowing the kernel
to perform direct I/O operations asynchronously. This improves concurrency
for applications that use O_DIRECT flag.
Benefits:
- Better concurrency for direct I/O operations
- Improved performance for applications using O_DIRECT
- Reduced blocking on I/O operations
Use cases:
- Database workloads that use direct I/O
- Applications that bypass page cache intentionally
- High-performance I/O scenarios
Implementation inspired by JuiceFS which enables this capability
for improved I/O performance.
Usage:
weed mount -filer=localhost:8888 -dir=/mnt/seaweedfs -asyncDio
* mount: add all remaining FUSE options (asyncDio, cacheSymlink, novncache)
This combines the remaining three FUSE mount options on top of the merged writebackCache PR:
1. asyncDio: Enable async direct I/O for better concurrency
2. cacheSymlink: Enable symlink caching to reduce metadata lookups
3. novncache: (macOS only) Disable vnode name caching to avoid stale data
All options use the function parameter 'option' instead of global 'mountOptions'.
* mount: add -asyncDio flag for async direct I/O
This adds support for async direct I/O via the -asyncDio flag.
Async DIO enables the FUSE_CAP_ASYNC_DIO capability, allowing the kernel
to perform direct I/O operations asynchronously. This improves concurrency
for applications that use O_DIRECT flag.
Benefits:
- Better concurrency for direct I/O operations
- Improved performance for applications using O_DIRECT
- Reduced blocking on I/O operations
Use cases:
- Database workloads that use direct I/O
- Applications that bypass page cache intentionally
- High-performance I/O scenarios
Implementation inspired by JuiceFS which enables this capability
for improved I/O performance.
Usage:
weed mount -filer=localhost:8888 -dir=/mnt/seaweedfs -asyncDio
* mount: add all remaining FUSE options (asyncDio, cacheSymlink, novncache)
This combines the remaining three FUSE mount options on top of the merged writebackCache PR:
1. asyncDio: Enable async direct I/O for better concurrency
2. cacheSymlink: Enable symlink caching to reduce metadata lookups
3. novncache: (macOS only) Disable vnode name caching to avoid stale data
All options use the function parameter 'option' instead of global 'mountOptions'.
* mount: add -asyncDio flag for async direct I/O
This adds support for async direct I/O via the -asyncDio flag.
Async DIO enables the FUSE_CAP_ASYNC_DIO capability, allowing the kernel
to perform direct I/O operations asynchronously. This improves concurrency
for applications that use O_DIRECT flag.
Benefits:
- Better concurrency for direct I/O operations
- Improved performance for applications using O_DIRECT
- Reduced blocking on I/O operations
Use cases:
- Database workloads that use direct I/O
- Applications that bypass page cache intentionally
- High-performance I/O scenarios
Implementation inspired by JuiceFS which enables this capability
for improved I/O performance.
Usage:
weed mount -filer=localhost:8888 -dir=/mnt/seaweedfs -asyncDio
* mount: add all remaining FUSE options (asyncDio, cacheSymlink, novncache)
This combines the remaining three FUSE mount options on top of the merged writebackCache PR:
1. asyncDio: Enable async direct I/O for better concurrency
2. cacheSymlink: Enable symlink caching to reduce metadata lookups
3. novncache: (macOS only) Disable vnode name caching to avoid stale data
All options use the function parameter 'option' instead of global 'mountOptions'.
2025-12-31 00:30:12 -08:00
Chris LuGitHubgemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
* mount: add -writebackCache flag for FUSE writeback caching
This adds support for FUSE writeback caching via the -writebackCache flag.
Writeback caching buffers writes in the kernel page cache before flushing
to the filesystem. This significantly improves performance for workloads
with many small writes by reducing the number of write syscalls.
Benefits:
- Improved write performance for small files (2-5x faster)
- Reduced latency for write-heavy workloads
- Better handling of bursty write patterns
Trade-offs:
- Data may be lost if system crashes before kernel flushes
- Not recommended for critical data without proper fsync usage
- Disabled by default for safety
Inspired by JuiceFS implementation which uses the same FUSE option.
Usage:
weed mount -filer=localhost:8888 -dir=/mnt/seaweedfs -writebackCache
* Apply suggestion from @gemini-code-assist[bot]
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>
When writing metadata logs to /topics/.system/log, the filer was not
respecting the disk type configuration from path-specific rules
(fs.configure). This caused volume assignment failures when volume
servers used a specific disk type (e.g., "ssd") because the assign
request defaulted to empty disk type.
The fix adds DiskType to the VolumeAssignRequest in the filer's
metadata log write path, ensuring that path-specific disk type
configurations are properly honored for internal system writes.
Fixes errors like:
"metadata log write failed /topics/.system/log/...: AssignVolume:
failed to find writable volumes for collection"
Signed-off-by: Charles Darke <s.cduk@toodevious.com>
Co-authored-by: Charles Darke <s.cduk@toodevious.com>
* Fix STS Access Denied for AssumeRoleWithWebIdentity (Issue #7917)
* Fix logging regression: ensure IAM status is logged even if STS is enabled
* Address PR feedback: fix duplicate log, clarify comments, add comprehensive routing tests
* Add edge case test: authenticated STS action routes to IAM (auth precedence)
* Fix: trim prefix slash in ListObjectVersionsHandler
* Add test for ListObjectVersions prefix handling
Test validates that prefix normalization works correctly with and without
leading slashes, ensuring the fix for /Veeam/Archive/ style prefixes.
* Simplify prefix test to validate normalization logic
The test now validates that the prefix normalization (TrimPrefix) works
correctly and that normalized prefixes match paths as expected. This is
a focused unit test that validates the core fix without requiring complex
mocking of the filer client.
* Enhance prefix test with full matchesPrefixFilter logic
Added test cases for directory traversal including:
- Directory matching with trailing slash
- canDescend logic for recursive directory search
- Full simulation of matchesPrefixFilter behavior
This provides more comprehensive coverage of the prefix normalization
fix and ensures it works correctly for both files and directories.
* filer.meta.backup: add -excludePaths flag to skip paths from backup
Add a new -excludePaths flag that accepts comma-separated path prefixes
to exclude from backup operations. This enables selective backup when
certain directories (e.g., legacy buckets) should be skipped.
Usage:
weed filer.meta.backup -filerDir=/buckets -excludePaths=/buckets/legacy1,/buckets/legacy2 -config=backup.toml
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* filer.meta.backup: address code review feedback for -excludePaths
Fixes based on CodeRabbit and Gemini review:
- Cache parsed exclude paths in struct (performance)
- TrimSpace and skip empty entries (handles "a,,b" and "a, b")
- Add trailing slash for directory boundary matching (prevents
/buckets/legacy matching /buckets/legacy_backup)
- Validate paths start with '/' and warn if not
- Log excluded paths at startup for debugging
- Fix rename handling: check both old and new paths, handle all
four combinations correctly
- Add docstring to shouldExclude()
- Update UsageLine and Long description with new flag
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* filer.meta.backup: address nitpick feedback
- Clarify directory boundary matching behavior in help text
- Add warning when root path '/' is excluded (would exclude everything)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* includePrefixes and excludePrefixes
---------
Co-authored-by: C Shaw <cliffshaw@users.noreply.github.com>
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Co-authored-by: Chris Lu <chris.lu@gmail.com>
* Fix critical authentication bypass vulnerability (#7912)
The isRequestPostPolicySignatureV4() function was incorrectly returning
true for ANY POST request with multipart/form-data content type,
causing all such requests to bypass authentication in authRequest().
This allowed unauthenticated access to S3 API endpoints, as reported
in issue #7912 where any credentials (or no credentials) were accepted.
The fix removes isRequestPostPolicySignatureV4() entirely, preventing
authTypePostPolicy from ever being set. PostPolicy signature verification
is still properly handled in PostPolicyBucketHandler via
doesPolicySignatureMatch().
Fixes#7912
* add AuthPostPolicy
* refactor
* Optimizing Auth Credentials
* Update auth_credentials.go
* Update auth_credentials.go
* s3: implement Bucket Owner Enforced for object ownership
Objects uploaded by service accounts (or any user) are now owned by
the bucket owner when the bucket has BucketOwnerEnforced ownership
policy (the modern AWS default since April 2023).
This provides a more intuitive ownership model where users expect
objects created by their service accounts to be owned by themselves.
- Modified setObjectOwnerFromRequest to check bucket ObjectOwnership
- When BucketOwnerEnforced: use bucket owner's account ID
- When ObjectWriter: use uploader's account ID (backward compatible)
* s3: add nil check and fix ownership logic hole
- Add nil check for bucketRegistry before calling GetBucketMetadata
- Fix logic hole where objects could be created without owner when
BucketOwnerEnforced is set but bucket owner is nil
- Refactor to ensure objects always have an owner by falling back
to uploader when bucket owner is unavailable
- Improve logging to distinguish between different fallback scenarios
Addresses code review feedback from Gemini on PR #7913
* s3: add comprehensive tests for object ownership logic
Add unit tests for setObjectOwnerFromRequest covering:
- BucketOwnerEnforced: uses bucket owner
- ObjectWriter: uses uploader
- BucketOwnerPreferred: uses uploader
- Nil owner fallback scenarios
- Bucket metadata errors
- Nil bucketRegistry
- Empty account ID handling
All 8 test cases pass, verifying correct ownership assignment
in all scenarios including edge cases.
* admin: add Service Account management UI
Add admin UI for managing service accounts:
New files:
- handlers/service_account_handlers.go - HTTP handlers
- dash/service_account_management.go - CRUD operations
- view/app/service_accounts.templ - UI template
Changes:
- dash/types.go - Add ServiceAccount and related types
- handlers/admin_handlers.go - Register routes and handlers
- view/layout/layout.templ - Add sidebar navigation link
Service accounts are stored as special identities with "sa:" prefix
in their name, using ABIA access key prefix. They can be created,
listed, enabled/disabled, and deleted through the admin UI.
Features:
- Create service accounts linked to parent users
- View and manage service account status
- Delete service accounts
- Service accounts inherit parent user permissions
Note: STS configuration is read-only (configured via JSON file).
Full STS integration requires changes from PR #7901.
* admin: use dropdown for parent user selection
Change the Parent User field from text input to dropdown when
creating a service account. The dropdown is populated with all
existing Object Store users.
Changes:
- Add AvailableUsers field to ServiceAccountsData type
- Populate available users in getServiceAccountsData handler
- Update template to use <select> element with user options
* admin: show secret access key on service account creation
Display both access key and secret access key when creating a
service account, with proper AWS CLI usage instructions.
Changes:
- Add SecretAccessKey field to ServiceAccount type (only populated on creation)
- Return secret key from CreateServiceAccount
- Add credentials modal with copy-to-clipboard buttons
- Show AWS CLI usage example with actual credentials
- Modal is non-dismissible until user confirms they saved credentials
The secret key is only shown once during creation for security.
After creation, only the access key ID is visible in the list.
* admin: address code review comments for service account management
- Persist creation dates in identity actions (createdAt:timestamp)
- Replace magic number slicing with len(accessKeyPrefix)
- Add bounds checking after strings.SplitN
- Use accessKeyPrefix constant instead of hardcoded "ABIA"
Creation dates are now stored as actions (e.g., "createdAt:1735473600")
and will persist across restarts. Helper functions getCreationDate()
and setCreationDate() manage the timestamp storage.
Addresses review comments from gemini-code-assist[bot] and coderabbitai[bot]
* admin: fix XSS vulnerabilities in service account details
Replace innerHTML with template literals with safe DOM creation.
The createSADetailsContent function now uses createElement and
textContent to prevent XSS attacks from malicious service account
data (id, description, parent_user, etc.).
Also added try-catch for date parsing to prevent exceptions on
malformed input.
Addresses security review comments from coderabbitai[bot]
* admin: add context.Context to service account management methods
Addressed PR #7902 review feedback:
1. All service account management methods now accept context.Context as
first parameter to enable cancellation, deadlines, and tracing
2. Removed all context.Background() calls
3. Updated handlers to pass c.Request.Context() from HTTP requests
Methods updated:
- GetServiceAccounts
- GetServiceAccountDetails
- CreateServiceAccount
- UpdateServiceAccount
- DeleteServiceAccount
- GetServiceAccountByAccessKey
Note: Creation date persistence was already implemented using the
createdAt:<timestamp> action pattern as suggested in the review.
* admin: fix render flow to prevent partial HTML writes
Fixed ShowServiceAccounts handler to render template to an in-memory
buffer first before writing to the response. This prevents partial HTML
writes followed by JSON error responses, which would result in invalid
mixed content.
Changes:
- Render to bytes.Buffer first
- Only write to c.Writer if render succeeds
- Use c.AbortWithStatus on error instead of attempting JSON response
- Prevents any additional headers/body writes after partial write
* admin: fix error handling, date validation, and event parameters
Addressed multiple code review issues:
1. Proper 404 vs 500 error handling:
- Added ErrServiceAccountNotFound sentinel error
- GetServiceAccountDetails now wraps errors with sentinel
- Handler uses errors.Is() to distinguish not-found from internal errors
- Returns 404 only for missing resources, 500 for other errors
- Logs internal errors before returning 500
2. Date validation in JavaScript:
- Validate expiration date before using it
- Check !isNaN(date.getTime()) to ensure valid date
- Return validation error if date is invalid
- Prevents invalid Date construction
3. Event parameter handling:
- copyToClipboard now accepts event parameter
- Updated onclick attributes to pass event object
- Prevents reliance on window.event
- More explicit and reliable event handling
* admin: replace deprecated execCommand with Clipboard API
Replaced deprecated document.execCommand('copy') with modern
navigator.clipboard.writeText() API for better security and UX.
Changes:
- Made copyToClipboard async to support Clipboard API
- Use navigator.clipboard.writeText() as primary method
- Fallback to execCommand if Clipboard API fails (older browsers)
- Added console warning when fallback is used
- Maintains same visual feedback behavior
* admin: improve security and UX for error handling
Addressed code review feedback:
1. Security: Remove sensitive error details from API responses
- CreateServiceAccount: Return generic error message
- UpdateServiceAccount: Return generic error message
- DeleteServiceAccount: Return generic error message
- Detailed errors still logged server-side via glog.Errorf()
- Prevents exposure of internal system details to clients
2. UX: Replace alert() with Bootstrap toast notifications
- Implemented showToast() function using Bootstrap 5 toasts
- Non-blocking, modern notification system
- Auto-dismiss after 5 seconds
- Proper HTML escaping to prevent XSS
- Toast container positioned at top-right
- Success (green) and error (red) variants
* admin: complete error handling improvements
Addressed remaining security review feedback:
1. GetServiceAccounts: Remove error details from response
- Log errors server-side via glog.Errorf()
- Return generic error message to client
2. UpdateServiceAccount & DeleteServiceAccount:
- Wrap not-found errors with ErrServiceAccountNotFound sentinel
- Enables proper 404 vs 500 distinction in handlers
3. Update & Delete handlers:
- Added errors.Is() check for ErrServiceAccountNotFound
- Return 404 for missing resources
- Return 500 for internal errors with logging
- Consistent with GetServiceAccountDetails behavior
All handlers now properly distinguish not-found (404) from internal
errors (500) and never expose sensitive error details to clients.
* admin: implement expiration support and improve code quality
Addressed final code review feedback:
1. Expiration Support:
- Added expiration helper functions (getExpiration, setExpiration)
- Implemented expiration in CreateServiceAccount
- Implemented expiration in UpdateServiceAccount
- Added Expiration field to ServiceAccount struct
- Parse and validate RFC3339 expiration dates
2. Constants for Magic Strings:
- Added StatusActive, StatusInactive constants
- Added disabledAction, serviceAccountPrefix constants
- Replaced all magic strings with constants throughout
- Improves maintainability and prevents typos
3. Helper Function to Reduce Duplication:
- Created identityToServiceAccount() helper
- Reduces code duplication across Get/Update/Delete methods
- Centralizes ServiceAccount struct building logic
4. Fixed time.Now() Fallback:
- Changed from time.Now() to time.Time{} for legacy accounts
- Prevents creation date from changing on each fetch
- UI can display zero time as "N/A" or blank
All code quality issues addressed!
* admin: fix StatusActive reference in handler
Use dash.StatusActive to properly reference the constant from the dash package.
* admin: regenerate templ files
Regenerated all templ Go files after recent template changes.
The AWS CLI usage example already uses proper <pre><code> formatting
which preserves line breaks for better readability.
* admin: add explicit white-space CSS to AWS CLI example
Added style="white-space: pre-wrap;" to the pre tag to ensure
line breaks are preserved and displayed correctly in all browsers.
This forces the browser to respect the newlines in the code block.
* admin: fix AWS CLI example to display on separate lines
Replaced pre/code block with individual div elements for each line.
This ensures each command displays on its own line regardless of
how templ processes whitespace. Each line is now a separate div
with font-monospace styling for code appearance.
* make
* admin: filter service accounts from parent user dropdown
Service accounts should not appear as selectable parent users when
creating new service accounts. Added filter to GetObjectStoreUsers()
to skip identities with "sa:" prefix, ensuring only actual IAM users
are shown in the parent user dropdown.
* admin: address code review feedback
- Use constants for magic strings in service account management
- Add Expiration field to service account responses
- Add nil checks and context propagation
- Improve templates (date validation, async clipboard, toast notifications)
* Update service_accounts_templ.go
* iam: add ServiceAccount protobuf schema
Add ServiceAccount message type to iam.proto with support for:
- Unique ID and parent user linkage
- Optional expiration timestamp
- Separate credentials (access key/secret)
- Action restrictions (subset of parent)
- Enable/disable status
This is the first step toward implementing issue #7744
(IAM Service Account Support).
* iam: add service account response types
Add IAM API response types for service account operations:
- ServiceAccountInfo struct for marshaling account details
- CreateServiceAccountResponse
- DeleteServiceAccountResponse
- ListServiceAccountsResponse
- GetServiceAccountResponse
- UpdateServiceAccountResponse
Also add type aliases in iamapi package for backwards compatibility.
Part of issue #7744 (IAM Service Account Support).
* iam: implement service account API handlers
Add CRUD operations for service accounts:
- CreateServiceAccount: Creates service account with ABIA key prefix
- DeleteServiceAccount: Removes service account and parent linkage
- ListServiceAccounts: Lists all or filtered by parent user
- GetServiceAccount: Retrieves service account details
- UpdateServiceAccount: Modifies status, description, expiration
Service accounts inherit parent user's actions by default and
support optional expiration timestamps.
Part of issue #7744 (IAM Service Account Support).
* sts: add AssumeRoleWithWebIdentity HTTP endpoint
Add STS API HTTP endpoint for AWS SDK compatibility:
- Create s3api_sts.go with HTTP handlers matching AWS STS spec
- Support AssumeRoleWithWebIdentity action with JWT token
- Return XML response with temporary credentials (AccessKeyId,
SecretAccessKey, SessionToken) matching AWS format
- Register STS route at POST /?Action=AssumeRoleWithWebIdentity
This enables AWS SDKs (boto3, AWS CLI, etc.) to obtain temporary
S3 credentials using OIDC/JWT tokens.
Part of issue #7744 (IAM Service Account Support).
* test: add service account and STS integration tests
Add integration tests for new IAM features:
s3_service_account_test.go:
- TestServiceAccountLifecycle: Create, Get, List, Update, Delete
- TestServiceAccountValidation: Error handling for missing params
s3_sts_test.go:
- TestAssumeRoleWithWebIdentityValidation: Parameter validation
- TestAssumeRoleWithWebIdentityWithMockJWT: JWT token handling
Tests skip gracefully when SeaweedFS is not running or when IAM
features are not configured.
Part of issue #7744 (IAM Service Account Support).
* iam: address code review comments
- Add constants for service account ID and key lengths
- Use strconv.ParseInt instead of fmt.Sscanf for better error handling
- Allow clearing descriptions by checking key existence in url.Values
- Replace magic numbers (12, 20, 40) with named constants
Addresses review comments from gemini-code-assist[bot]
* test: add proper error handling in service account tests
Use require.NoError(t, err) for io.ReadAll and xml.Unmarshal
to prevent silent failures and ensure test reliability.
Addresses review comment from gemini-code-assist[bot]
* test: add proper error handling in STS tests
Use require.NoError(t, err) for io.ReadAll and xml.Unmarshal
to prevent silent failures and ensure test reliability.
Repeated this fix throughout the file.
Addresses review comment from gemini-code-assist[bot] in PR #7901.
* iam: address additional code review comments
- Specific error code mapping for STS service errors
- Distinguish between Sender and Receiver error types in STS responses
- Add nil checks for credentials in List/GetServiceAccount
- Validate expiration date is in the future
- Improve integration test error messages (include response body)
- Add credential verification step in service account tests
Addresses remaining review comments from gemini-code-assist[bot] across multiple files.
* iam: fix shared slice reference in service account creation
Copy parent's actions to create an independent slice for the service
account instead of sharing the underlying array. This prevents
unexpected mutations when the parent's actions are modified later.
Addresses review comment from coderabbitai[bot] in PR #7901.
* iam: remove duplicate unused constant
Removed redundant iamServiceAccountKeyPrefix as ServiceAccountKeyPrefix
is already defined and used.
Addresses remaining cleanup task.
* sts: document limitation of string-based error mapping
Added TODO comment explaining that the current string-based error
mapping approach is fragile and should be replaced with typed errors
from the STS service in a future refactoring.
This addresses the architectural concern raised in code review while
deferring the actual implementation to a separate PR to avoid scope
creep in the current service account feature addition.
* iam: fix remaining review issues
- Add future-date validation for expiration in UpdateServiceAccount
- Reorder tests so credential verification happens before deletion
- Fix compilation error by using correct JWT generation methods
Addresses final review comments from coderabbitai[bot].
* iam: fix service account access key length
The access key IDs were incorrectly generated with 24 characters
instead of the AWS-standard 20 characters. This was caused by
generating 20 random characters and then prepending the 4-character
ABIA prefix.
Fixed by subtracting the prefix length from AccessKeyLength, so the
final key is: ABIA (4 chars) + random (16 chars) = 20 chars total.
This ensures compatibility with S3 clients that validate key length.
* test: add comprehensive service account security tests
Added comprehensive integration tests for service account functionality:
- TestServiceAccountS3Access: Verify SA credentials work for S3 operations
- TestServiceAccountExpiration: Test expiration date validation and enforcement
- TestServiceAccountInheritedPermissions: Verify parent-child relationship
- TestServiceAccountAccessKeyFormat: Validate AWS-compatible key format (ABIA prefix, 20 char length)
These tests ensure SeaweedFS service accounts are compatible with AWS
conventions and provide robust security coverage.
* iam: remove unused UserAccessKeyPrefix constant
Code cleanup to remove unused constants.
* iam: remove unused iamCommonResponse type alias
Code cleanup to remove unused type aliases.
* iam: restore and use UserAccessKeyPrefix constant
Restored UserAccessKeyPrefix constant and updated s3api tests to use it
instead of hardcoded strings for better maintainability and consistency.
* test: improve error handling in service account security tests
Added explicit error checking for io.ReadAll and xml.Unmarshal in
TestServiceAccountExpiration to ensure failures are reported correctly and
cleanup is performed only when appropriate. Also added logging for failed
responses.
* test: use t.Cleanup for reliable resource cleanup
Replaced defer with t.Cleanup to ensure service account cleanup runs even
when require.NoError fails. Also switched from manual error checking to
require.NoError for more idiomatic testify usage.
* iam: add CreatedBy field and optimize identity lookups
- Added createdBy parameter to CreateServiceAccount to track who created each service account
- Extract creator identity from request context using GetIdentityNameFromContext
- Populate created_by field in ServiceAccount protobuf
- Added findIdentityByName helper function to optimize identity lookups
- Replaced nested loops with O(n) helper function calls in CreateServiceAccount and DeleteServiceAccount
This addresses code review feedback for better auditing and performance.
* iam: prevent user deletion when service accounts exist
Following AWS IAM behavior, prevent deletion of users that have active
service accounts. This ensures explicit cleanup and prevents orphaned
service account resources with invalid ParentUser references.
Users must delete all associated service accounts before deleting the
parent user, providing safer resource management.
* sts: enhance TODO with typed error implementation guidance
Updated TODO comment with detailed implementation approach for replacing
string-based error matching with typed errors using errors.Is(). This
provides a clear roadmap for a follow-up PR to improve error handling
robustness and maintainability.
* iam: add operational limits for service account creation
Added AWS IAM-compatible safeguards to prevent resource exhaustion:
- Maximum 100 service accounts per user (LimitExceededException)
- Maximum 1000 character description length (InvalidInputException)
These limits prevent accidental or malicious resource exhaustion while
not impacting legitimate use cases.
* iam: add missing operational limit constants
Added MaxServiceAccountsPerUser and MaxDescriptionLength constants that
were referenced in the previous commit but not defined.
* iam: enforce service account expiration during authentication
CRITICAL SECURITY FIX: Expired service account credentials were not being
rejected during authentication, allowing continued access after expiration.
Changes:
- Added Expiration field to Credential struct
- Populate expiration when loading service accounts from configuration
- Check expiration in all authentication paths (V2 and V4 signatures)
- Return ErrExpiredToken for expired credentials
This ensures expired service accounts are properly rejected at authentication
time, matching AWS IAM behavior and preventing unauthorized access.
* iam: fix error code for expired service account credentials
Use ErrAccessDenied instead of non-existent ErrExpiredToken for expired
service account credentials. This provides appropriate access denial for
expired credentials while maintaining AWS-compatible error responses.
* iam: fix remaining ErrExpiredToken references
Replace all remaining instances of non-existent ErrExpiredToken with
ErrAccessDenied for expired service account credentials.
* iam: apply AWS-standard key format to user access keys
Updated CreateAccessKey to generate AWS-standard 20-character access keys
with AKIA prefix for regular users, matching the format used for service
accounts. This ensures consistency across all access key types and full
AWS compatibility.
- Access keys: AKIA + 16 random chars = 20 total (was 21 chars, no prefix)
- Secret keys: 40 random chars (was 42, now matches AWS standard)
- Uses AccessKeyLength and UserAccessKeyPrefix constants
* sts: replace fragile string-based error matching with typed errors
Implemented robust error handling using typed errors and errors.Is() instead
of fragile strings.Contains() matching. This decouples the HTTP layer from
service implementation details and prevents errors from being miscategorized
if error messages change.
Changes:
- Added typed error variables to weed/iam/sts/constants.go:
* ErrTypedTokenExpired
* ErrTypedInvalidToken
* ErrTypedInvalidIssuer
* ErrTypedInvalidAudience
* ErrTypedMissingClaims
- Updated STS service to wrap provider authentication errors with typed errors
- Replaced strings.Contains() with errors.Is() in HTTP layer for error checking
- Removed TODO comment as the improvement is now implemented
This makes error handling more maintainable and reliable.
* sts: eliminate all string-based error matching with provider-level typed errors
Completed the typed error implementation by adding provider-level typed errors
and updating provider implementations to return them. This eliminates ALL
fragile string matching throughout the entire error handling stack.
Changes:
- Added typed error definitions to weed/iam/providers/errors.go:
* ErrProviderTokenExpired
* ErrProviderInvalidToken
* ErrProviderInvalidIssuer
* ErrProviderInvalidAudience
* ErrProviderMissingClaims
- Updated OIDC provider to wrap JWT validation errors with typed provider errors
- Replaced strings.Contains() with errors.Is() in STS service for error mapping
- Complete error chain: Provider -> STS -> HTTP layer, all using errors.Is()
This provides:
- Reliable error classification independent of error message content
- Type-safe error checking throughout the stack
- No order-dependent string matching
- Maintainable error handling that won't break with message changes
* oidc: use jwt.ErrTokenExpired instead of string matching
Replaced the last remaining string-based error check with the JWT library's
exported typed error. This makes the error detection independent of error
message content and more robust against library updates.
Changed from:
strings.Contains(errMsg, "expired")
To:
errors.Is(err, jwt.ErrTokenExpired)
This completes the elimination of ALL string-based error matching throughout
the entire authentication stack.
* iam: add description length validation to UpdateServiceAccount
Fixed inconsistency where UpdateServiceAccount didn't validate description
length against MaxDescriptionLength, allowing operational limits to be
bypassed during updates.
Now validates that updated descriptions don't exceed 1000 characters,
matching the validation in CreateServiceAccount.
* iam: refactor expiration check into helper method
Extracted duplicated credential expiration check logic into a helper method
to reduce code duplication and improve maintainability.
Added Credential.isCredentialExpired() method and replaced 5 instances of
inline expiration checks across auth_signature_v2.go and auth_signature_v4.go.
* iam: address critical Copilot security and consistency feedback
Fixed three critical issues identified by Copilot code review:
1. SECURITY: Prevent loading disabled service account credentials
- Added check to skip disabled service accounts during credential loading
- Disabled accounts can no longer authenticate
2. Add DurationSeconds validation for STS AssumeRoleWithWebIdentity
- Enforce AWS-compatible range: 900-43200 seconds (15 min - 12 hours)
- Returns proper error for out-of-range values
3. Fix expiration update consistency in UpdateServiceAccount
- Added key existence check like Description field
- Allows explicit clearing of expiration by setting to empty string
- Distinguishes between "not updating" and "clearing expiration"
* sts: remove unused durationSecondsStr variable
Fixed build error from unused variable after refactoring duration parsing.
* iam: address remaining Copilot feedback and remove dead code
Completed remaining Copilot code review items:
1. Remove unused getPermission() method (dead code)
- Method was defined but never called anywhere
2. Improve slice modification safety in DeleteServiceAccount
- Replaced append-with-slice-operations with filter pattern
- Avoids potential issues from mutating slice during iteration
3. Fix route registration order
- Moved STS route registration BEFORE IAM route
- Prevents IAM route from intercepting STS requests
- More specific route (with query parameter) now registered first
* iam: improve expiration validation and test cleanup robustness
Addressed additional Copilot feedback:
1. Make expiration validation more explicit
- Added explicit check for negative values
- Added comment clarifying that 0 is allowed to clear expiration
- Improves code readability and intent
2. Fix test cleanup order in s3_service_account_test.go
- Track created service accounts in a slice
- Delete all service accounts before deleting parent user
- Prevents DeleteConflictException during cleanup
- More robust cleanup even if test fails mid-execution
Note: s3_service_account_security_test.go already had correct cleanup
order due to LIFO defer execution.
* test: remove redundant variable assignments
Removed duplicate assignments of createdSAId, createdAccessKeyId, and
createdSecretAccessKey on lines 148-150 that were already assigned on
lines 132-134.
* mount: let filer handle chunk deletion decision
Remove chunk deletion decision from FUSE mount's Unlink operation.
Previously, the mount decided whether to delete chunks based on
its locally cached entry's HardLinkCounter, which could be stale.
Now always pass isDeleteData=true and let the filer make the
authoritative decision based on its own data. This prevents
potential inconsistencies when:
- The FUSE mount's cached entry is stale
- Race conditions occur between multiple mounts
- Direct filer operations change hard link counts
* filer: check hard link counter before deleting chunks
When deleting an entry, only delete the underlying chunks if:
1. It is not a hard link
2. OR it is the last hard link (counter <= 1)
This protects against data loss when a client (like FUSE mount)
requests chunk deletion for a file that has multiple hard links.
* s3api: Add SOSAPI core implementation and tests
Implement Smart Object Storage API (SOSAPI) support for Veeam integration.
- Add s3api_sosapi.go with XML structures and handlers for system.xml and capacity.xml
- Implement virtual object detection and dynamic XML generation
- Add capacity retrieval via gRPC (to be optimized in follow-up)
- Include comprehensive unit tests covering detection, XML generation, and edge cases
This enables Veeam Backup & Replication to discover SeaweedFS capabilities and capacity.
* s3api: Integrate SOSAPI handlers into GetObject and HeadObject
Add early interception for SOSAPI virtual objects in GetObjectHandler and HeadObjectHandler.
- Check for SOSAPI objects (.system-*/system.xml, .system-*/capacity.xml) before normal processing
- Delegate to handleSOSAPIGetObject and handleSOSAPIHeadObject when detected
- Ensures virtual objects are served without hitting storage layer
* s3api: Allow anonymous access to SOSAPI virtual objects
Enable discovery of SOSAPI capabilities without requiring credentials.
- Modify AuthWithPublicRead to bypass auth for SOSAPI objects if bucket exists
- Supports Veeam's initial discovery phase before full IAM setup
- Validates bucket existence to prevent information disclosure
* s3api: Fix SOSAPI capacity retrieval to use proper master connection
Fix gRPC error by connecting directly to master servers instead of through filer.
- Use pb.WithOneOfGrpcMasterClients with s3a.option.Masters
- Matches pattern used in bucket_size_metrics.go
- Resolves "unknown service master_pb.Seaweed" error
- Gracefully handles missing master configuration
* Merge origin/master and implement robust SOSAPI capacity logic
- Resolved merge conflict in s3api_sosapi.go
- Replaced global Statistics RPC with VolumeList (topology) for accurate bucket-specific 'Used' calculation
- Added bucket quota support (report quota as Capacity if set)
- Implemented cluster-wide capacity calculation from topology when no quota
- Added unit tests for topology capacity and usage calculations
* s3api: Remove anonymous access to SOSAPI virtual objects
Reverts the implicit public access for system.xml and capacity.xml.
Requests to these objects now require standard S3 authentication,
unless the bucket has a public-read policy.
* s3api: Refactor SOSAPI handlers to use http.ServeContent
- Consolidate handleSOSAPIGetObject and handleSOSAPIHeadObject into serveSOSAPI
- Use http.ServeContent for standard Range, HEAD, and ETag handling
- Remove manual range request handler and reduce code duplication
* s3api: Unify SOSAPI request handling
- Replaced handleSOSAPIGetObject and handleSOSAPIHeadObject with single HandleSOSAPI function
- Updated call sites in s3api_object_handlers.go
- Simplifies logic and ensures consistent handling for both GET and HEAD requests via http.ServeContent
* s3api: Restore distinct SOSAPI GET/HEAD handlers
- Reverted unified handler to enforce distinct behavior for GET and HEAD
- GET: Supports Range requests via http.ServeContent
- HEAD: Explicitly ignores Range requests (matches MinIO behavior) and writes headers only
* s3api: Refactor SOSAPI handlers to eliminate duplication
- Extracted shared content generation logic into generateSOSAPIContent helper
- handleSOSAPIGetObject: Uses http.ServeContent (supports Range requests)
- handleSOSAPIHeadObject: Manually sets headers (no Range, no body)
- Maintains distinct behavior while following DRY principle
* s3api: Remove low-value SOSAPI tests
Removed tests that validate standard library behavior or trivial constant checks:
- TestSOSAPIConstants (string prefix/suffix checks)
- TestSystemInfoXMLRootElement (redundant with TestGenerateSystemXML)
- TestSOSAPIXMLContentType (tests httptest, not our code)
- TestHTTPTimeFormat (tests standard library)
- TestCapacityInfoXMLStruct (tests Go's XML marshaling)
Kept tests that validate actual business logic and edge cases.
* s3api: Use consistent S3-compliant error responses in SOSAPI
Replaced http.Error() with s3err.WriteErrorResponse() for internal errors
to ensure all SOSAPI errors return S3-compliant XML instead of plain text.
* s3api: Return error when no masters configured for SOSAPI capacity
Changed getCapacityInfo to return an error instead of silently returning
zero capacity when no master servers are configured. This helps surface
configuration issues rather than masking them.
* s3api: Use collection name with FilerGroup prefix for SOSAPI capacity
Fixed collectBucketUsageFromTopology to use s3a.getCollectionName(bucket)
instead of raw bucket name. This ensures collection comparisons match actual
volume collection names when FilerGroup prefix is configured.
* s3api: Apply PR review feedback for SOSAPI
- Renamed `bucket` parameter to `collectionName` in collectBucketUsageFromTopology for clarity
- Changed error checks from `==` to `errors.Is()` for better wrapped error handling
- Added `errors` import
* s3api: Avoid variable shadowing in SOSAPI capacity retrieval
Refactored `getCapacityInfo` to use distinct variable names for errors
to improve code clarity and avoid unintentional shadowing of the
return parameter.
Enable discovery of SOSAPI capabilities without requiring credentials.
- Modify AuthWithPublicRead to bypass auth for SOSAPI objects if bucket exists
- Supports Veeam's initial discovery phase before full IAM setup
- Validates bucket existence to prevent information disclosure
Add early interception for SOSAPI virtual objects in GetObjectHandler and HeadObjectHandler.
- Check for SOSAPI objects (.system-*/system.xml, .system-*/capacity.xml) before normal processing
- Delegate to handleSOSAPIGetObject and handleSOSAPIHeadObject when detected
- Ensures virtual objects are served without hitting storage layer
Implement Smart Object Storage API (SOSAPI) support for Veeam integration.
- Add s3api_sosapi.go with XML structures and handlers for system.xml and capacity.xml
- Implement virtual object detection and dynamic XML generation
- Add capacity retrieval via gRPC (to be optimized in follow-up)
- Include comprehensive unit tests covering detection, XML generation, and edge cases
This enables Veeam Backup & Replication to discover SeaweedFS capabilities and capacity.
* Fix: relax gRPC server keepalive enforcement to 20s to prevent 'too_many_pings' errors
* Refactor: introduce GrpcKeepAliveMinimumTime constant for clarity
* feat: Add probes to worker service
* feat: Add probes to worker service
* Merge branch 'master' into pr/7896
* refactor
---------
Co-authored-by: Chris Lu <chris.lu@gmail.com>
* S3: Fix Content-Encoding header not preserved (#7894)
The Content-Encoding header was not being returned in S3 GET/HEAD responses
because it wasn't being stored in metadata during PUT operations.
Root cause: The putToFiler function only stored a hardcoded list of standard
HTTP headers (Cache-Control, Expires, Content-Disposition) but was missing
Content-Encoding and Content-Language.
Fix: Added Content-Encoding and Content-Language to the list of standard
headers that are stored in entry.Extended during PUT operations.
This matches the behavior of ParseS3Metadata (used for multipart uploads)
and ensures consistency across all S3 operations.
Fixes#7894
* Update s3api_object_handlers_put.go
* Add S3 volume encryption support with -s3.encryptVolumeData flag
This change adds volume-level encryption support for S3 uploads, similar
to the existing -filer.encryptVolumeData option. Each chunk is encrypted
with its own auto-generated CipherKey when the flag is enabled.
Changes:
- Add -s3.encryptVolumeData flag to weed s3, weed server, and weed mini
- Wire Cipher option through S3ApiServer and ChunkedUploadOption
- Add integration tests for multi-chunk range reads with encryption
- Tests verify encryption works across chunk boundaries
Usage:
weed s3 -encryptVolumeData
weed server -s3 -s3.encryptVolumeData
weed mini -s3.encryptVolumeData
Integration tests:
go test -v -tags=integration -timeout 5m ./test/s3/sse/...
* Add GitHub Actions CI for S3 volume encryption tests
- Add test-volume-encryption target to Makefile that starts server with -s3.encryptVolumeData
- Add s3-volume-encryption job to GitHub Actions workflow
- Tests run with integration build tag and 10m timeout
- Server logs uploaded on failure for debugging
* Fix S3 client credentials to use environment variables
The test was using hardcoded credentials "any"/"any" but the Makefile
sets AWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEY to "some_access_key1"/
"some_secret_key1". Updated getS3Client() to read from environment
variables with fallback to "any"/"any" for manual testing.
* Change bucket creation errors from skip to fatal
Tests should fail, not skip, when bucket creation fails. This ensures
that credential mismatches and other configuration issues are caught
rather than silently skipped.
* Make copy and multipart test jobs fail instead of succeed
Changed exit 0 to exit 1 for s3-sse-copy-operations and s3-sse-multipart
jobs. These jobs document known limitations but should fail to ensure
the issues are tracked and addressed, not silently ignored.
* Hardcode S3 credentials to match Makefile
Changed from environment variables to hardcoded credentials
"some_access_key1"/"some_secret_key1" to match the Makefile
configuration. This ensures tests work reliably.
* fix Double Encryption
* fix Chunk Size Mismatch
* Added IsCompressed
* is gzipped
* fix copying
* only perform HEAD request when len(cipherKey) > 0
* Revert "Make copy and multipart test jobs fail instead of succeed"
This reverts commit bc34a7eb3c.
* fix security vulnerability
* fix security
* Update s3api_object_handlers_copy.go
* Update s3api_object_handlers_copy.go
* jwt to get content length
* fix: support standard HTTP headers in S3 multipart upload
* fix(s3api): validate standard HTTP headers correctly and avoid persisting Response-Content-Disposition
---------
Co-authored-by: steve.wei <coderushing@gmail.com>
* grpc: reduce client idle pings to avoid ENHANCE_YOUR_CALM (too_many_pings)
* test: use context.WithTimeout and pb constants for keepalive
* test(kafka): use separate dial and client contexts in NewDirectBrokerClient
* test(kafka): fix client context usage in NewDirectBrokerClient
* Fix issue #7880: Tasks use Volume IDs instead of ip:port
When volume servers are registered with custom IDs, tasks were attempting
to connect using the ID instead of the actual ip:port address, causing
connection failures.
Modified task detection logic in balance, erasure coding, and vacuum tasks
to resolve volume server IDs to their actual ip:port addresses using
ActiveTopology information.
* Use server addresses directly instead of translating from IDs
Modified VolumeHealthMetrics to include ServerAddress field populated
directly from topology DataNodeInfo.Address. Updated task detection
logic to use addresses directly without runtime lookups.
Changes:
- Added ServerAddress field to VolumeHealthMetrics
- Updated maintenance scanner to populate ServerAddress
- Modified task detection to use ServerAddress for Node fields
- Updated DestinationPlan to include TargetAddress
- Removed runtime address lookups in favor of direct address usage
* Address PR comments: add ServerAddress field, improve error handling
- Add missing ServerAddress field to VolumeHealthMetrics struct
- Add warning in vacuum detection when server not found in topology
- Improve error handling in erasure coding to abort task if sources missing
- Make vacuum task stricter by skipping if server not found in topology
* Refactor: Extract common address resolution logic into shared utility
- Created weed/worker/tasks/util/address.go with ResolveServerAddress function
- Updated balance, erasure_coding, and vacuum detection to use the shared utility
- Removed code duplication and improved maintainability
- Consistent error handling across all task types
* Fix critical issues in task address resolution
- Vacuum: Require topology availability and fail if server not found (no fallback to ID)
- Ensure all task types consistently fail early when topology is incomplete
- Prevent creation of tasks that would fail due to missing server addresses
* Address additional PR feedback
- Add validation for empty addresses in ResolveServerAddress
- Remove redundant serverAddress variable in vacuum detection
- Improve robustness of address resolution
* Improve error logging in vacuum detection
- Include actual error details in log message for better diagnostics
- Make error messages consistent with other task types
* add readonly user
* add args
* address comments
* avoid same user name
* Prevents timing attacks
* doc
---------
Co-authored-by: Chris Lu <chris.lu@gmail.com>
Changes:
Modified weed/command/fuse.go to add a function GetFuseCommandName to return the name of the fuse command.
Modified weed/weed.go to conditionally initialize the global HTTP client only if the command is not "fuse".
Modified weed/command/fuse_std.go to parse parameters and ensure the global HTTP client is initialized for the fuse command.
Tests:
Use /etc/fstab like:
fuse /repos fuse.weed filer=192.168.1.101:7202,filer.path=/hpc/repos,config_dir=/etc/seaweedfs/seaweedfs_01 0 0
fuse /opt/ohpc/pub fuse.weed filer=192.168.1.102:7202,filer.path=/hpc_cluster/pub,config_dir=/etc/seaweedfs/seaweedfs_02 0 0
Co-authored-by: zhangxl56 <zhangxl56@lenovo.com>
* Refactor S3 integration tests to use weed mini
* Fix weed mini flags for sse and parquet tests
* Fix IAM test startup: remove -iam.config flag from weed mini
* Enhance logging in IAM Makefile to debug startup failure
* Simplify weed mini flags and checks in S3 tests (IAM, Parquet, SSE, Copying)
* Simplify weed mini flags and checks in all S3 tests
* Fix IAM tests: use -s3.iam.config for weed mini
* Replace timeout command with portable loop in IAM Makefile
* Standardize portable loop-based readiness checks in all S3 Makefiles
* Define SERVER_DIR in retention Makefile
* Fix versioning and retention Makefiles: remove unsupported weed mini flags
* fix filer_group test
* fix cors
* emojis
* fix sse
* fix retention
* fixes
* fix
* fixes
* fix parquet
* fixes
* fix
* clean up
* avoid duplicated debug server
* Update .gitignore
* simplify
* clean up
* add credentials
* bind
* delay
* Update Makefile
* Update Makefile
* check ready
* delay
* update remote credentials
* Update Makefile
* clean up
* kill
* Update Makefile
* update credentials
* Replace removeDuplicateSlashes with NormalizeObjectKey
Use s3_constants.NormalizeObjectKey instead of removeDuplicateSlashes in most places
for consistency. NormalizeObjectKey handles both duplicate slash removal and ensures
the path starts with '/', providing more complete normalization.
* Fix double slash issues after NormalizeObjectKey
After using NormalizeObjectKey, object keys have a leading '/'. This commit ensures:
- getVersionedObjectDir strips leading slash before concatenation
- getEntry calls receive names without leading slash
- String concatenation with '/' doesn't create '//' paths
This prevents path construction errors like:
/buckets/bucket//object (wrong)
/buckets/bucket/object (correct)
* ensure object key leading "/"
* fix compilation
* fix: Strip leading slash from object keys in S3 API responses
After introducing NormalizeObjectKey, all internal object keys have a
leading slash. However, S3 API responses must return keys without
leading slashes to match AWS S3 behavior.
Fixed in three functions:
- addVersion: Strip slash for version list entries
- processRegularFile: Strip slash for regular file entries
- processExplicitDirectory: Strip slash for directory entries
This ensures ListObjectVersions and similar APIs return keys like
'bar' instead of '/bar', matching S3 API specifications.
* fix: Normalize keyMarker for consistent pagination comparison
The S3 API provides keyMarker without a leading slash (e.g., 'object-001'),
but after introducing NormalizeObjectKey, all internal object keys have
leading slashes (e.g., '/object-001').
When comparing keyMarker < normalizedObjectKey in shouldSkipObjectForMarker,
the ASCII value of '/' (47) is less than 'o' (111), causing all objects
to be incorrectly skipped during pagination. This resulted in page 2 and
beyond returning 0 results.
Fix: Normalize the keyMarker when creating versionCollector so comparisons
work correctly with normalized object keys.
Fixes pagination tests:
- TestVersioningPaginationOver1000Versions
- TestVersioningPaginationMultipleObjectsManyVersions
* refactor: Change NormalizeObjectKey to return keys without leading slash
BREAKING STRATEGY CHANGE:
Previously, NormalizeObjectKey added a leading slash to all object keys,
which required stripping it when returning keys to S3 API clients and
caused complexity in marker normalization for pagination.
NEW STRATEGY:
- NormalizeObjectKey now returns keys WITHOUT leading slash (e.g., 'foo/bar' not '/foo/bar')
- This matches the S3 API format directly
- All path concatenations now explicitly add '/' between bucket and object
- No need to strip slashes in responses or normalize markers
Changes:
1. Modified NormalizeObjectKey to strip leading slash instead of adding it
2. Fixed all path concatenations to use:
- BucketsPath + '/' + bucket + '/' + object
instead of:
- BucketsPath + '/' + bucket + object
3. Reverted response key stripping in:
- addVersion()
- processRegularFile()
- processExplicitDirectory()
4. Reverted keyMarker normalization in findVersionsRecursively()
5. Updated matchesPrefixFilter() to work with keys without leading slash
6. Fixed paths in handlers:
- s3api_object_handlers.go (GetObject, HeadObject, cacheRemoteObjectForStreaming)
- s3api_object_handlers_postpolicy.go
- s3api_object_handlers_tagging.go
- s3api_object_handlers_acl.go
- s3api_version_id.go (getVersionedObjectDir, getVersionIdFormat)
- s3api_object_versioning.go (getObjectVersionList, updateLatestVersionAfterDeletion)
All versioning tests pass including pagination stress tests.
* adjust format
* Update post policy tests to match new NormalizeObjectKey behavior
- Update TestPostPolicyKeyNormalization to expect keys without leading slashes
- Update TestNormalizeObjectKey to expect keys without leading slashes
- Update TestPostPolicyFilenameSubstitution to expect keys without leading slashes
- Update path construction in tests to use new pattern: BucketsPath + '/' + bucket + '/' + object
* Fix ListObjectVersions prefix filtering
Remove leading slash addition to prefix parameter to allow correct filtering
of .versions directories when listing object versions with a specific prefix.
The prefix parameter should match entry paths relative to bucket root.
Adding a leading slash was breaking the prefix filter for paginated requests.
Fixes pagination issue where second page returned 0 versions instead of
continuing with remaining versions.
* no leading slash
* Fix urlEscapeObject to add leading slash for filer paths
NormalizeObjectKey now returns keys without leading slashes to match S3 API format.
However, urlEscapeObject is used for filer paths which require leading slashes.
Add leading slash back after normalization to ensure filer paths are correct.
Fixes TestS3ApiServer_toFilerPath test failures.
* adjust tests
* normalize
* Fix: Normalize prefixes and markers in LIST operations using NormalizeObjectKey
Ensure consistent key normalization across all S3 operations (GET, PUT, LIST).
Previously, LIST operations were not applying the same normalization rules
(handling backslashes, duplicate slashes, leading slashes) as GET/PUT operations.
Changes:
- Updated normalizePrefixMarker() to call NormalizeObjectKey for both prefix and marker
- This ensures prefixes with leading slashes, backslashes, or duplicate slashes are
handled consistently with how object keys are normalized
- Fixes Parquet test failures where pads.write_dataset creates implicit directory
structures that couldn't be discovered by subsequent LIST operations
- Added TestPrefixNormalizationInList and TestListPrefixConsistency tests
All existing LIST tests continue to pass with the normalization improvements.
* Add debugging logging to LIST operations to track prefix normalization
* Fix: Remove leading slash addition from GetPrefix to work with NormalizeObjectKey
The NormalizeObjectKey function removes leading slashes to match S3 API format
(e.g., 'foo/bar' not '/foo/bar'). However, GetPrefix was adding a leading slash
back, which caused LIST operations to fail with incorrect path handling.
Now GetPrefix only normalizes duplicate slashes without adding a leading slash,
which allows NormalizeObjectKey changes to work correctly for S3 LIST operations.
All Parquet integration tests now pass (20/20).
* Fix: Handle object paths without leading slash in checkDirectoryObject
NormalizeObjectKey() removes the leading slash to match S3 API format.
However, checkDirectoryObject() was assuming the object path has a leading
slash when processing directory markers (paths ending with '/').
Now we ensure the object has a leading slash before processing it for
filer operations.
Fixes implicit directory marker test (explicit_dir/) while keeping
Parquet integration tests passing (20/20).
All tests pass:
- Implicit directory tests: 6/6
- Parquet integration tests: 20/20
* Fix: Handle explicit directory markers with trailing slashes
Explicit directory markers created with put_object(Key='dir/', ...) are stored
in the filer with the trailing slash as part of the name. The checkDirectoryObject()
function now checks for both:
1. Explicit directories: lookup with trailing slash preserved (e.g., 'explicit_dir/')
2. Implicit directories: lookup without trailing slash (e.g., 'implicit_dir')
This ensures both types of directory markers are properly recognized.
All tests pass:
- Implicit directory tests: 6/6 (including explicit directory marker test)
- Parquet integration tests: 20/20
* Fix: Preserve trailing slash in NormalizeObjectKey
NormalizeObjectKey now preserves trailing slashes when normalizing object keys.
This is important for explicit directory markers like 'explicit_dir/' which rely
on the trailing slash to be recognized as directory objects.
The normalization process:
1. Notes if trailing slash was present
2. Removes duplicate slashes and converts backslashes
3. Removes leading slash for S3 API format
4. Restores trailing slash if it was in the original
This ensures explicit directory markers created with put_object(Key='dir/', ...)
are properly normalized and can be looked up by their exact name.
All tests pass:
- Implicit directory tests: 6/6
- Parquet integration tests: 20/20
* clean object
* Fix: Don't restore trailing slash if result is empty
When normalizing paths that are only slashes (e.g., '///', '/'), the function
should return an empty string, not a single slash. The fix ensures we only
restore the trailing slash if the result is non-empty.
This fixes the 'just_slashes' test case:
- Input: '///'
- Expected: ''
- Previous: '/'
- Fixed: ''
All tests now pass:
- Unit tests: TestNormalizeObjectKey (13/13)
- Implicit directory tests: 6/6
- Parquet integration tests: 20/20
* prefixEndsOnDelimiter
* Update s3api_object_handlers_list.go
* Update s3api_object_handlers_list.go
* handle create directory
* Fix: Support object tagging in versioned buckets (Issue #7868)
This fix addresses the issue where setting tags on files in versioned buckets would fail with 'filer: no entry is found in filer store' error.
Changes:
- Updated GetObjectTaggingHandler to check versioning status and retrieve correct object versions
- Updated PutObjectTaggingHandler to properly locate and update tags on versioned objects
- Updated DeleteObjectTaggingHandler to delete tags from versioned objects
- Added proper handling for both specific versions and latest versions
- Added distinction between null versions (pre-versioning objects) and versioned objects
The fix follows the same versioning-aware pattern already implemented in ACL handlers.
Tests:
- Added comprehensive test suite for tagging operations on versioned buckets
- Tests cover PUT, GET, and DELETE tagging operations on specific versions and latest versions
- Tests verify tag isolation between different versions of the same object
* Fix: Ensure consistent directory path construction in tagging handlers
Changed directory path construction to match the pattern used in ACL handlers:
- Added missing '/' before object path when constructing .versions directory path
- This ensures compatibility with the filer's expected path structure
- Applied to both PutObjectTaggingHandler and DeleteObjectTaggingHandler
* Revert: Remove redundant slash in path construction - object already has leading slash from NormalizeObjectKey
* Fix: Remove redundant slashes in versioning path construction across handlers
- getVersionedObjectDir: object already starts with '/', no need for extra '/'
- ACL handlers: same pattern, fix both PutObjectAcl locations
- Ensures consistent path construction with object parameter normalization
* fix test compilation
* Add: Comprehensive ACL tests for versioned and non-versioned buckets
- Added s3_acl_versioning_test.go with 5 test cases covering:
* GetObjectAcl on versioned buckets
* GetObjectAcl on specific versions
* PutObjectAcl on versioned buckets
* PutObjectAcl on specific versions
* Independent ACL management across versions
These tests were missing and would have caught the path construction
issues we just fixed in the ACL handler. Tests validate that ACL
operations work correctly on both versioned and non-versioned objects.
* Fix: Correct tagging versioning test file formatting
* fix: Update AWS SDK endpoint config and improve cleanup to handle delete markers
- Replace deprecated EndpointResolverWithOptions with BaseEndpoint in AWS SDK v2 client configuration
- Update cleanupTestBucket to properly delete both object versions and delete markers
- Apply changes to both ACL and tagging test files for consistency
* Fix S3 multi-delete for versioned objects
The bug was in getVersionedObjectDir() which was constructing paths without
a slash between the bucket and object key:
BEFORE (WRONG): /buckets/mybucket{key}.versions
AFTER (FIXED): /buckets/mybucket/{key}/.versions
This caused version deletions to claim success but not actually delete files,
breaking S3 compatibility tests:
- test_versioning_multi_object_delete
- test_versioning_multi_object_delete_with_marker
- test_versioning_concurrent_multi_object_delete
- test_object_lock_multi_delete_object_with_retention
Added comprehensive test that reproduces the issue and verifies the fix.
* Remove emojis from test output
The worker deployment was incorrectly passing the admin gRPC port (33646)
to the -admin flag. However, the SeaweedFS worker command automatically
calculates the gRPC port by adding 10000 to the HTTP port provided.
This caused workers to attempt connection to port 43646 (33646 + 10000)
instead of the correct gRPC port 33646 (23646 + 10000).
Changes:
- Update worker-deployment.yaml to use admin.port instead of admin.grpcPort
- Workers now correctly connect to admin HTTP port, allowing the binary
to calculate the gRPC port automatically
Fixes workers failing with:
"dial tcp <admin-ip>:43646: connect: no route to host"
Related:
- Worker code: weed/pb/grpc_client_server.go:272 (grpcPort = port + 10000)
- Worker docs: weed/command/worker.go:36 (admin HTTP port + 10000)
* fix(iam): add support for fine-grained S3 actions in IAM policies
Add support for fine-grained S3 actions like s3:DeleteObject, s3:PutObject,
and other specific S3 actions in IAM policy mapping. Previously, only
coarse-grained action patterns (Put*, Get*, etc.) were supported, causing
IAM policies with specific actions to be rejected with 'not a valid action'
error.
Fixes issue #7864 part 2: s3:DeleteObject IAM action is now supported.
Changes:
- Extended MapToStatementAction() to handle fine-grained S3 actions
- Maps S3-specific actions to appropriate internal action constants
- Supports 30+ S3 actions including DeleteObject, PutObject, GetObject, etc.
* fix(s3api): correct resource ARN generation for subpath permissions
Fix convertSingleAction() to properly handle subpath patterns in legacy
actions. Previously, when a user was granted Write permission to a subpath
(e.g., Write:bucket/sub_path/*), the resource ARN was incorrectly generated,
causing DELETE operations to be denied even though s3:DeleteObject was
included in the Write action.
The fix:
- Extract bucket name and prefix path separately from patterns like
'bucket/prefix/*'
- Generate correct S3 ARN format: arn:aws:s3:::bucket/prefix/*
- Ensure all permission checks (Read, Write, List, Tagging, etc.) work
correctly with subpaths
- Support nested paths (e.g., bucket/a/b/c/*)
Fixes issue #7864 part 1: Write permission on subpath now allows DELETE.
Example:
- Permission: Write:mybucket/documents/*
- Objects can now be: PUT, DELETE, or ACL operations on mybucket/documents/*
- Objects outside this path are still denied
* test(s3api): add comprehensive tests for subpath permission handling
Add new test file with comprehensive tests for convertSingleAction():
1. TestConvertSingleActionDeleteObject: Verifies s3:DeleteObject is
included in Write actions (fixes issue #7864 part 2)
2. TestConvertSingleActionSubpath: Tests proper resource ARN generation
for different permission patterns:
- Bucket-level: Write:mybucket -> arn:aws:s3:::mybucket
- Wildcard: Write:mybucket/* -> arn:aws:s3:::mybucket/*
- Subpath: Write:mybucket/sub_path/* -> arn:aws:s3:::mybucket/sub_path/*
- Nested: Read:mybucket/documents/* -> arn:aws:s3:::mybucket/documents/*
3. TestConvertSingleActionSubpathDeleteAllowed: Specifically validates
that subpath Write permissions allow DELETE operations
4. TestConvertSingleActionNestedPaths: Tests deeply nested path handling
(e.g., bucket/a/b/c/*)
All tests pass and validate the fixes for issue #7864.
* fix: address review comments from PR #7865
- Fix critical bug: use parsed 'bucket' instead of 'resourcePattern' for GetObjectRetention, GetObjectLegalHold, and PutObjectLegalHold actions to avoid malformed ARNs like arn:aws:s3:::bucket/*/*
- Refactor large switch statement in MapToStatementAction() into a map-based lookup for better performance and maintainability
* fmt
* refactor: extract extractBucketAndPrefix helper and simplify convertSingleAction
- Extract extractBucketAndPrefix as a package-level function for better testability and reusability
- Remove unused bucketName parameter from convertSingleAction signature
- Update GetResourcesFromLegacyAction to use the extracted helper for consistent ARN generation
- Update all call sites in tests to match new function signature
- All tests pass and module compiles without errors
* fix: use extracted bucket variable consistently in all ARN generation branches
Replace resourcePattern with extracted bucket variable in else branches and bucket-level cases to avoid malformed ARNs like 'arn:aws:s3:::mybucket/*/*':
- Read case: bucket-level else branch
- Write case: bucket-level else branch
- Admin case: both bucket and object ARNs
- List case: bucket-level else branch
- GetBucketObjectLockConfiguration: bucket extraction
- PutBucketObjectLockConfiguration: bucket extraction
This ensures consistent ARN format: arn:aws:s3:::bucket or arn:aws:s3:::bucket/*
* fix: address remaining review comments from PR #7865
High priority fixes:
- Write action on bucket-level now generates arn:aws:s3:::mybucket/* instead of
arn:aws:s3:::mybucket to enable object-level S3 actions (s3:PutObject, s3:DeleteObject)
- GetResourcesFromLegacyAction now generates both bucket and object ARNs for /*
patterns to maintain backward compatibility with mixed action groups
Medium priority improvements:
- Remove unused 'bucket' field from TestConvertSingleActionSubpath test struct
- Update test to use assert.ElementsMatch instead of assert.Contains for more
comprehensive resource ARN validation
- Clarify test expectations with expectedResources slice instead of single expectedResource
All tests pass, compilation verified
* test: improve TestConvertSingleActionNestedPaths with comprehensive assertions
Update test to use assert.ElementsMatch for more robust resource ARN verification:
- Change struct from single expectedResource to expectedResources slice
- Update Read nested path test to expect both bucket and prefix ARNs
- Use assert.ElementsMatch to verify all generated resources match exactly
- Provides complete coverage for nested path handling
This matches the improvement pattern used in TestConvertSingleActionSubpath
* refactor: simplify S3 action map and improve resource ARN detection
- Refactor fineGrainedActionMap to use init() function for programmatic population of both prefixed (s3:Action) and unprefixed (Action) variants, eliminating 70+ duplicate entries
- Add buildObjectResourceArn() helper to eliminate duplicated resource ARN generation logic across switch cases
- Fix bucket vs object-level access detection: only use HasSuffix(/*) check instead of Contains('/') which incorrectly matched patterns like 'bucket/prefix' without wildcard
- Apply buildObjectResourceArn() consistently to Tagging, BypassGovernanceRetention, GetObjectRetention, PutObjectRetention, GetObjectLegalHold, and PutObjectLegalHold cases
* fmt
* fix: generate object-level ARNs for bucket-level read access
When bucket-level read access is granted (e.g., 'Read:mybucket'),
generate both bucket and object ARNs so that object-level actions
like s3:GetObject can properly authorize.
Similarly, in GetResourcesFromLegacyAction, bucket-level patterns
should generate both ARN levels for consistency with patterns that
include wildcards. This ensures that users with bucket-level
permissions can read objects, not just the bucket itself.
* fix: address Copilot code review comments
- Remove unused bucketName parameter from ConvertIdentityToPolicy signature
- Update all callers in examples.go and engine_test.go
- Bucket is now extracted from action string itself
- Update extractBucketAndPrefix documentation
- Add nested path example (bucket/a/b/c/*)
- Clarify that prefix can contain multiple path segments
- Make GetResourcesFromLegacyAction action-aware
- Different action types have different resource requirements
- List actions only need bucket ARN (bucket-only operations)
- Read/Write/Tagging actions need both bucket and object ARNs
- Aligns with convertSingleAction logic for consistency
All tests pass successfully
* test: add comprehensive tests for GetResourcesFromLegacyAction consistency
- Add TestGetResourcesFromLegacyAction to verify action-aware resource generation
- Validate consistency with convertSingleAction for all action types:
* List actions: bucket-only ARNs (s3:ListBucket is bucket-level operation)
* Read actions: both bucket and object ARNs
* Write actions: object-only ARNs (subpaths) or object ARNs (bucket-level)
* Admin actions: both bucket and object ARNs
- Update GetResourcesFromLegacyAction to generate Admin ARNs consistent with convertSingleAction
- All tests pass (35+ test cases across integration_test.go)
* refactor: eliminate code duplication in GetResourcesFromLegacyAction
- Simplify GetResourcesFromLegacyAction to delegate to convertSingleAction
- Eliminates ~50 lines of duplicated action-type-specific logic
- Ensures single source of truth for resource ARN generation
- Improves maintainability: changes to ARN logic only need to be made in one place
- All tests pass: any inconsistencies would be caught immediately
- Addresses Gemini Code Assist review comment about code duplication
* fix: remove fragile 'dummy' action type in CreatePolicyFromLegacyIdentity
- Replace hardcoded 'dummy:' prefix with proper representative action type
- Use first valid action type from the action list to determine resource requirements
- Ensures GetResourcesFromLegacyAction receives a valid action type
- Prevents silent failures when convertSingleAction encounters unknown action
- Improves code clarity: explains why representative action type is needed
- All tests pass: policy engine tests verify correct behavior
* security: prevent privilege escalation in Admin action subpath handling
- Admin action with subpath (e.g., Admin:bucket/admin/*) now correctly restricts
to the specified subpath instead of granting full bucket access
- If prefix exists: resources restricted to bucket + bucket/prefix/*
- If no prefix: full bucket access (unchanged behavior for root Admin)
- Added test case Admin_on_subpath to validate the security fix
- All 40+ policy engine tests pass
* refactor: address Copilot code review comments on S3 authorization
- Fix GetObjectTagging mapping: change from ACTION_READ to ACTION_TAGGING
(tagging operations should not be classified as general read operations)
- Enhance extractBucketAndPrefix edge case handling:
- Add input validation (reject empty strings, whitespace, slash-only)
- Normalize double slashes and trailing slashes
- Return empty bucket/prefix for invalid patterns
- Prevent generation of malformed ARNs
- Separate Read action from ListBucket (AWS S3 IAM semantics):
- ListBucket is a bucket-level operation, not object-level
- Read action now only includes s3:GetObject, s3:GetObjectVersion
- This aligns with AWS S3 IAM policy best practices
- Update buildObjectResourceArn to handle invalid bucket names gracefully:
- Return empty slice if bucket is empty after validation
- Prevents malformed ARN generation
- Add comprehensive TestExtractBucketAndPrefixEdgeCases with 8 test cases:
- Validates empty strings, whitespace, special characters
- Confirms proper normalization of double/trailing slashes
- Ensures robust parsing of nested paths
- Update existing tests to reflect removed ListBucket from Read action
All 40+ policy engine tests pass
* fix: aggregate resource ARNs from all action types in CreatePolicyFromLegacyIdentity
CRITICAL FIX: The previous implementation incorrectly used a single representative
action type to determine resource ARNs when multiple legacy actions targeted the
same resource pattern. This caused incorrect policy generation when action types
with different resource requirements (e.g., List vs Write) were grouped together.
Example of the bug:
- Input: List:mybucket/path/*, Write:mybucket/path/*
- Old behavior: Used only List's resources (bucket-level ARN)
- Result: Policy had Write actions (s3:PutObject) but only bucket ARN
- Consequence: s3:PutObject would be denied due to missing object-level ARN
Solution:
- Iterate through all action types for a given resource pattern
- For each action type, call GetResourcesFromLegacyAction to get required ARNs
- Aggregate all ARNs into a set to eliminate duplicates
- Use the merged set for the final policy statement
- Admin action short-circuits (always includes full permissions)
Example of correct behavior:
- Input: List:mybucket/path/*, Write:mybucket/path/*
- New behavior: Aggregates both List and Write resource requirements
- Result: Policy has Write actions with BOTH bucket and object-level ARNs
- Outcome: s3:PutObject works correctly on mybucket/path/*
Added TestCreatePolicyFromLegacyIdentityMultipleActions with 3 test cases:
1. List + Write on subpath: verifies bucket + object ARN aggregation
2. Read + Tagging on bucket: verifies action-specific ARN combinations
3. Admin with other actions: verifies Admin dominates resource ARNs
All 45+ policy engine tests pass
* fix: remove bucket-level ARN from Read action for consistency
ISSUE: The Read action was including bucket-level ARNs (arn:aws:s3:::bucket)
even though the only S3 actions in Read are s3:GetObject and s3:GetObjectVersion,
which are object-level operations. This created a mismatch between the actions
and resources in the policy statement.
ROOT CAUSE: s3:ListBucket was previously removed from the Read action, but the
bucket-level ARN was not removed, creating an inconsistency.
SOLUTION: Update Read action to only generate object-level ARNs using
buildObjectResourceArn, consistent with how Write and Tagging actions work.
This ensures:
- Read:mybucket generates arn:aws:s3:::mybucket/* (not bucket ARN)
- Read:bucket/prefix/* generates arn:aws:s3:::bucket/prefix/* (object-level only)
- Consistency: same actions, same resources, same logic across all object operations
Updated test expectations:
- TestConvertSingleActionSubpath: Read_on_subpath now expects only object ARN
- TestConvertSingleActionNestedPaths: Read nested path now expects only object ARN
- TestConvertIdentityToPolicy: Read resources now 1 instead of 2
- TestCreatePolicyFromLegacyIdentityMultipleActions: Read+Tagging aggregates to 1 ARN
All 45+ policy engine tests pass
* doc
* fmt
* fix: address Copilot code review on Read action consistency and missing S3 action mappings
- Clarify MapToStatementAction comment to reflect exact lookup (not pattern matching)
- Add missing S3 actions to baseS3ActionMap:
- ListBucketVersions, ListAllMyBuckets for bucket operations
- GetBucketCors, PutBucketCors, DeleteBucketCors for CORS
- GetBucketNotification, PutBucketNotification for notifications
- GetBucketObjectLockConfiguration, PutBucketObjectLockConfiguration for object lock
- GetObjectVersionTagging for version tagging
- GetObjectVersionAcl, PutBucketAcl for ACL operations
- PutBucketTagging, DeleteBucketTagging for bucket tagging
- Fix Read action scope inconsistency with GetActionMappings():
- Previously: only included GetObject, GetObjectVersion
- Now: includes full Read set (14 actions) from GetActionMappings
- Includes both bucket-level (ListBucket*, GetBucket*) and object-level (GetObject*) ARNs
- Bucket ARN enables ListBucket operations, object ARN enables GetObject operations
- Update all test expectations:
- TestConvertSingleActionSubpath: Read now returns 2 ARNs (bucket + objects)
- TestConvertSingleActionNestedPaths: Read nested path now includes bucket ARN
- TestGetResourcesFromLegacyAction: Read test cases updated for consistency
- TestCreatePolicyFromLegacyIdentityMultipleActions: Read_and_Tagging now returns 2 ARNs
- TestConvertIdentityToPolicy: Updated to expect 14 Read actions and 2 resources
Fixes: Inconsistency between convertSingleAction Read case and GetActionMappings function
* fmt
* fix: align convertSingleAction with GetActionMappings and add bucket validation
- Fix Write action: now includes all 16 actions from GetActionMappings (object and bucket operations)
- Includes PutBucketVersioning, PutBucketCors, PutBucketAcl, PutBucketTagging, etc.
- Generates both bucket and object ARNs to support bucket-level operations
- Fix List action: add ListAllMyBuckets from GetActionMappings
- Previously: ListBucket, ListBucketVersions
- Now: ListBucket, ListBucketVersions, ListAllMyBuckets
- Add bucket validation to prevent malformed ARNs with empty bucket
- Fix Tagging action: include bucket-level tagging operations
- Previously: only object-level (GetObjectTagging, PutObjectTagging, DeleteObjectTagging)
- Now: includes bucket-level (GetBucketTagging, PutBucketTagging, DeleteBucketTagging)
- Generates both bucket and object ARNs to support bucket-level operations
- Add bucket validation to prevent malformed ARNs:
- Admin: return error if bucket is empty
- List: generate empty resources if bucket is empty
- Tagging: check bucket before generating ARNs
- GetBucketObjectLockConfiguration, PutBucketObjectLockConfiguration: validate bucket
- Fix TrimRight issue in extractBucketAndPrefix:
- Change from strings.TrimRight(pattern, "/") to remove only one trailing slash
- Prevents loss of prefix when pattern has multiple trailing slashes
- Update all test cases:
- TestConvertSingleActionSubpath: Write now returns 16 actions and bucket+object ARNs
- TestConvertSingleActionNestedPaths: Write includes bucket ARN
- TestGetResourcesFromLegacyAction: Updated Write and Tagging expectations
- TestCreatePolicyFromLegacyIdentityMultipleActions: Updated action/resource counts
Fixes: Inconsistencies between convertSingleAction and GetActionMappings for Write/List/Tagging actions
* fmt
* fix: resolve ListMultipartUploads/ListParts mapping inconsistency and add action validation
- Fix ListMultipartUploads and ListParts mapping in helpers.go:
- Changed from ACTION_LIST to ACTION_WRITE for consistency with GetActionMappings
- These operations are part of the multipart write workflow and should map to Write action
- Prevents inconsistent behavior when same actions processed through different code paths
- Add documentation to clarify multipart operations in Write action:
- Explain why ListMultipartUploads and ListParts are part of Write permissions
- These are required for meaningful multipart upload workflow management
- Add action validation to CreatePolicyFromLegacyIdentity:
- Validates action format before processing using ValidateActionMapping
- Logs warnings for invalid actions instead of silently skipping them
- Provides clearer error messages when invalid action types are used
- Ensures users know when their intended permissions weren't applied
- Consistent with ConvertLegacyActions validation approach
Fixes: Inconsistent action type mappings and silent failure for invalid actions
* fix: restore TimeToFirstByte metric for S3 GetObject operations (issue #7869)
* fix(iam): add support for fine-grained S3 actions in IAM policies
Add support for fine-grained S3 actions like s3:DeleteObject, s3:PutObject,
and other specific S3 actions in IAM policy mapping. Previously, only
coarse-grained action patterns (Put*, Get*, etc.) were supported, causing
IAM policies with specific actions to be rejected with 'not a valid action'
error.
Fixes issue #7864 part 2: s3:DeleteObject IAM action is now supported.
Changes:
- Extended MapToStatementAction() to handle fine-grained S3 actions
- Maps S3-specific actions to appropriate internal action constants
- Supports 30+ S3 actions including DeleteObject, PutObject, GetObject, etc.
* fix(s3api): correct resource ARN generation for subpath permissions
Fix convertSingleAction() to properly handle subpath patterns in legacy
actions. Previously, when a user was granted Write permission to a subpath
(e.g., Write:bucket/sub_path/*), the resource ARN was incorrectly generated,
causing DELETE operations to be denied even though s3:DeleteObject was
included in the Write action.
The fix:
- Extract bucket name and prefix path separately from patterns like
'bucket/prefix/*'
- Generate correct S3 ARN format: arn:aws:s3:::bucket/prefix/*
- Ensure all permission checks (Read, Write, List, Tagging, etc.) work
correctly with subpaths
- Support nested paths (e.g., bucket/a/b/c/*)
Fixes issue #7864 part 1: Write permission on subpath now allows DELETE.
Example:
- Permission: Write:mybucket/documents/*
- Objects can now be: PUT, DELETE, or ACL operations on mybucket/documents/*
- Objects outside this path are still denied
* test(iam): add tests for fine-grained S3 action mappings
Extend TestMapToStatementAction with test cases for fine-grained S3 actions:
- s3:DeleteObject
- s3:PutObject
- s3:GetObject
- s3:ListBucket
- s3:PutObjectAcl
- s3:GetObjectAcl
Ensures the new action mapping support is working correctly.
* test(s3api): add comprehensive tests for subpath permission handling
Add new test file with comprehensive tests for convertSingleAction():
1. TestConvertSingleActionDeleteObject: Verifies s3:DeleteObject is
included in Write actions (fixes issue #7864 part 2)
2. TestConvertSingleActionSubpath: Tests proper resource ARN generation
for different permission patterns:
- Bucket-level: Write:mybucket -> arn:aws:s3:::mybucket
- Wildcard: Write:mybucket/* -> arn:aws:s3:::mybucket/*
- Subpath: Write:mybucket/sub_path/* -> arn:aws:s3:::mybucket/sub_path/*
- Nested: Read:mybucket/documents/* -> arn:aws:s3:::mybucket/documents/*
3. TestConvertSingleActionSubpathDeleteAllowed: Specifically validates
that subpath Write permissions allow DELETE operations
4. TestConvertSingleActionNestedPaths: Tests deeply nested path handling
(e.g., bucket/a/b/c/*)
All tests pass and validate the fixes for issue #7864.
* fix: address review comments from PR #7865
- Fix critical bug: use parsed 'bucket' instead of 'resourcePattern' for GetObjectRetention, GetObjectLegalHold, and PutObjectLegalHold actions to avoid malformed ARNs like arn:aws:s3:::bucket/*/*
- Refactor large switch statement in MapToStatementAction() into a map-based lookup for better performance and maintainability
* fmt
* refactor: extract extractBucketAndPrefix helper and simplify convertSingleAction
- Extract extractBucketAndPrefix as a package-level function for better testability and reusability
- Remove unused bucketName parameter from convertSingleAction signature
- Update GetResourcesFromLegacyAction to use the extracted helper for consistent ARN generation
- Update all call sites in tests to match new function signature
- All tests pass and module compiles without errors
* fix: use extracted bucket variable consistently in all ARN generation branches
Replace resourcePattern with extracted bucket variable in else branches and bucket-level cases to avoid malformed ARNs like 'arn:aws:s3:::mybucket/*/*':
- Read case: bucket-level else branch
- Write case: bucket-level else branch
- Admin case: both bucket and object ARNs
- List case: bucket-level else branch
- GetBucketObjectLockConfiguration: bucket extraction
- PutBucketObjectLockConfiguration: bucket extraction
This ensures consistent ARN format: arn:aws:s3:::bucket or arn:aws:s3:::bucket/*
* fix: address remaining review comments from PR #7865
High priority fixes:
- Write action on bucket-level now generates arn:aws:s3:::mybucket/* instead of
arn:aws:s3:::mybucket to enable object-level S3 actions (s3:PutObject, s3:DeleteObject)
- GetResourcesFromLegacyAction now generates both bucket and object ARNs for /*
patterns to maintain backward compatibility with mixed action groups
Medium priority improvements:
- Remove unused 'bucket' field from TestConvertSingleActionSubpath test struct
- Update test to use assert.ElementsMatch instead of assert.Contains for more
comprehensive resource ARN validation
- Clarify test expectations with expectedResources slice instead of single expectedResource
All tests pass, compilation verified
* test: improve TestConvertSingleActionNestedPaths with comprehensive assertions
Update test to use assert.ElementsMatch for more robust resource ARN verification:
- Change struct from single expectedResource to expectedResources slice
- Update Read nested path test to expect both bucket and prefix ARNs
- Use assert.ElementsMatch to verify all generated resources match exactly
- Provides complete coverage for nested path handling
This matches the improvement pattern used in TestConvertSingleActionSubpath
* refactor: simplify S3 action map and improve resource ARN detection
- Refactor fineGrainedActionMap to use init() function for programmatic population of both prefixed (s3:Action) and unprefixed (Action) variants, eliminating 70+ duplicate entries
- Add buildObjectResourceArn() helper to eliminate duplicated resource ARN generation logic across switch cases
- Fix bucket vs object-level access detection: only use HasSuffix(/*) check instead of Contains('/') which incorrectly matched patterns like 'bucket/prefix' without wildcard
- Apply buildObjectResourceArn() consistently to Tagging, BypassGovernanceRetention, GetObjectRetention, PutObjectRetention, GetObjectLegalHold, and PutObjectLegalHold cases
* fmt
* fix: generate object-level ARNs for bucket-level read access
When bucket-level read access is granted (e.g., 'Read:mybucket'),
generate both bucket and object ARNs so that object-level actions
like s3:GetObject can properly authorize.
Similarly, in GetResourcesFromLegacyAction, bucket-level patterns
should generate both ARN levels for consistency with patterns that
include wildcards. This ensures that users with bucket-level
permissions can read objects, not just the bucket itself.
* fix: address Copilot code review comments
- Remove unused bucketName parameter from ConvertIdentityToPolicy signature
- Update all callers in examples.go and engine_test.go
- Bucket is now extracted from action string itself
- Update extractBucketAndPrefix documentation
- Add nested path example (bucket/a/b/c/*)
- Clarify that prefix can contain multiple path segments
- Make GetResourcesFromLegacyAction action-aware
- Different action types have different resource requirements
- List actions only need bucket ARN (bucket-only operations)
- Read/Write/Tagging actions need both bucket and object ARNs
- Aligns with convertSingleAction logic for consistency
All tests pass successfully
* test: add comprehensive tests for GetResourcesFromLegacyAction consistency
- Add TestGetResourcesFromLegacyAction to verify action-aware resource generation
- Validate consistency with convertSingleAction for all action types:
* List actions: bucket-only ARNs (s3:ListBucket is bucket-level operation)
* Read actions: both bucket and object ARNs
* Write actions: object-only ARNs (subpaths) or object ARNs (bucket-level)
* Admin actions: both bucket and object ARNs
- Update GetResourcesFromLegacyAction to generate Admin ARNs consistent with convertSingleAction
- All tests pass (35+ test cases across integration_test.go)
* refactor: eliminate code duplication in GetResourcesFromLegacyAction
- Simplify GetResourcesFromLegacyAction to delegate to convertSingleAction
- Eliminates ~50 lines of duplicated action-type-specific logic
- Ensures single source of truth for resource ARN generation
- Improves maintainability: changes to ARN logic only need to be made in one place
- All tests pass: any inconsistencies would be caught immediately
- Addresses Gemini Code Assist review comment about code duplication
* fix: remove fragile 'dummy' action type in CreatePolicyFromLegacyIdentity
- Replace hardcoded 'dummy:' prefix with proper representative action type
- Use first valid action type from the action list to determine resource requirements
- Ensures GetResourcesFromLegacyAction receives a valid action type
- Prevents silent failures when convertSingleAction encounters unknown action
- Improves code clarity: explains why representative action type is needed
- All tests pass: policy engine tests verify correct behavior
* security: prevent privilege escalation in Admin action subpath handling
- Admin action with subpath (e.g., Admin:bucket/admin/*) now correctly restricts
to the specified subpath instead of granting full bucket access
- If prefix exists: resources restricted to bucket + bucket/prefix/*
- If no prefix: full bucket access (unchanged behavior for root Admin)
- Added test case Admin_on_subpath to validate the security fix
- All 40+ policy engine tests pass
* refactor: address Copilot code review comments on S3 authorization
- Fix GetObjectTagging mapping: change from ACTION_READ to ACTION_TAGGING
(tagging operations should not be classified as general read operations)
- Enhance extractBucketAndPrefix edge case handling:
- Add input validation (reject empty strings, whitespace, slash-only)
- Normalize double slashes and trailing slashes
- Return empty bucket/prefix for invalid patterns
- Prevent generation of malformed ARNs
- Separate Read action from ListBucket (AWS S3 IAM semantics):
- ListBucket is a bucket-level operation, not object-level
- Read action now only includes s3:GetObject, s3:GetObjectVersion
- This aligns with AWS S3 IAM policy best practices
- Update buildObjectResourceArn to handle invalid bucket names gracefully:
- Return empty slice if bucket is empty after validation
- Prevents malformed ARN generation
- Add comprehensive TestExtractBucketAndPrefixEdgeCases with 8 test cases:
- Validates empty strings, whitespace, special characters
- Confirms proper normalization of double/trailing slashes
- Ensures robust parsing of nested paths
- Update existing tests to reflect removed ListBucket from Read action
All 40+ policy engine tests pass
* fix: aggregate resource ARNs from all action types in CreatePolicyFromLegacyIdentity
CRITICAL FIX: The previous implementation incorrectly used a single representative
action type to determine resource ARNs when multiple legacy actions targeted the
same resource pattern. This caused incorrect policy generation when action types
with different resource requirements (e.g., List vs Write) were grouped together.
Example of the bug:
- Input: List:mybucket/path/*, Write:mybucket/path/*
- Old behavior: Used only List's resources (bucket-level ARN)
- Result: Policy had Write actions (s3:PutObject) but only bucket ARN
- Consequence: s3:PutObject would be denied due to missing object-level ARN
Solution:
- Iterate through all action types for a given resource pattern
- For each action type, call GetResourcesFromLegacyAction to get required ARNs
- Aggregate all ARNs into a set to eliminate duplicates
- Use the merged set for the final policy statement
- Admin action short-circuits (always includes full permissions)
Example of correct behavior:
- Input: List:mybucket/path/*, Write:mybucket/path/*
- New behavior: Aggregates both List and Write resource requirements
- Result: Policy has Write actions with BOTH bucket and object-level ARNs
- Outcome: s3:PutObject works correctly on mybucket/path/*
Added TestCreatePolicyFromLegacyIdentityMultipleActions with 3 test cases:
1. List + Write on subpath: verifies bucket + object ARN aggregation
2. Read + Tagging on bucket: verifies action-specific ARN combinations
3. Admin with other actions: verifies Admin dominates resource ARNs
All 45+ policy engine tests pass
* fix: remove bucket-level ARN from Read action for consistency
ISSUE: The Read action was including bucket-level ARNs (arn:aws:s3:::bucket)
even though the only S3 actions in Read are s3:GetObject and s3:GetObjectVersion,
which are object-level operations. This created a mismatch between the actions
and resources in the policy statement.
ROOT CAUSE: s3:ListBucket was previously removed from the Read action, but the
bucket-level ARN was not removed, creating an inconsistency.
SOLUTION: Update Read action to only generate object-level ARNs using
buildObjectResourceArn, consistent with how Write and Tagging actions work.
This ensures:
- Read:mybucket generates arn:aws:s3:::mybucket/* (not bucket ARN)
- Read:bucket/prefix/* generates arn:aws:s3:::bucket/prefix/* (object-level only)
- Consistency: same actions, same resources, same logic across all object operations
Updated test expectations:
- TestConvertSingleActionSubpath: Read_on_subpath now expects only object ARN
- TestConvertSingleActionNestedPaths: Read nested path now expects only object ARN
- TestConvertIdentityToPolicy: Read resources now 1 instead of 2
- TestCreatePolicyFromLegacyIdentityMultipleActions: Read+Tagging aggregates to 1 ARN
All 45+ policy engine tests pass
* doc
* fmt
* fix: address Copilot code review on Read action consistency and missing S3 action mappings
- Clarify MapToStatementAction comment to reflect exact lookup (not pattern matching)
- Add missing S3 actions to baseS3ActionMap:
- ListBucketVersions, ListAllMyBuckets for bucket operations
- GetBucketCors, PutBucketCors, DeleteBucketCors for CORS
- GetBucketNotification, PutBucketNotification for notifications
- GetBucketObjectLockConfiguration, PutBucketObjectLockConfiguration for object lock
- GetObjectVersionTagging for version tagging
- GetObjectVersionAcl, PutBucketAcl for ACL operations
- PutBucketTagging, DeleteBucketTagging for bucket tagging
- Fix Read action scope inconsistency with GetActionMappings():
- Previously: only included GetObject, GetObjectVersion
- Now: includes full Read set (14 actions) from GetActionMappings
- Includes both bucket-level (ListBucket*, GetBucket*) and object-level (GetObject*) ARNs
- Bucket ARN enables ListBucket operations, object ARN enables GetObject operations
- Update all test expectations:
- TestConvertSingleActionSubpath: Read now returns 2 ARNs (bucket + objects)
- TestConvertSingleActionNestedPaths: Read nested path now includes bucket ARN
- TestGetResourcesFromLegacyAction: Read test cases updated for consistency
- TestCreatePolicyFromLegacyIdentityMultipleActions: Read_and_Tagging now returns 2 ARNs
- TestConvertIdentityToPolicy: Updated to expect 14 Read actions and 2 resources
Fixes: Inconsistency between convertSingleAction Read case and GetActionMappings function
* fmt
* fix: align convertSingleAction with GetActionMappings and add bucket validation
- Fix Write action: now includes all 16 actions from GetActionMappings (object and bucket operations)
- Includes PutBucketVersioning, PutBucketCors, PutBucketAcl, PutBucketTagging, etc.
- Generates both bucket and object ARNs to support bucket-level operations
- Fix List action: add ListAllMyBuckets from GetActionMappings
- Previously: ListBucket, ListBucketVersions
- Now: ListBucket, ListBucketVersions, ListAllMyBuckets
- Add bucket validation to prevent malformed ARNs with empty bucket
- Fix Tagging action: include bucket-level tagging operations
- Previously: only object-level (GetObjectTagging, PutObjectTagging, DeleteObjectTagging)
- Now: includes bucket-level (GetBucketTagging, PutBucketTagging, DeleteBucketTagging)
- Generates both bucket and object ARNs to support bucket-level operations
- Add bucket validation to prevent malformed ARNs:
- Admin: return error if bucket is empty
- List: generate empty resources if bucket is empty
- Tagging: check bucket before generating ARNs
- GetBucketObjectLockConfiguration, PutBucketObjectLockConfiguration: validate bucket
- Fix TrimRight issue in extractBucketAndPrefix:
- Change from strings.TrimRight(pattern, "/") to remove only one trailing slash
- Prevents loss of prefix when pattern has multiple trailing slashes
- Update all test cases:
- TestConvertSingleActionSubpath: Write now returns 16 actions and bucket+object ARNs
- TestConvertSingleActionNestedPaths: Write includes bucket ARN
- TestGetResourcesFromLegacyAction: Updated Write and Tagging expectations
- TestCreatePolicyFromLegacyIdentityMultipleActions: Updated action/resource counts
Fixes: Inconsistencies between convertSingleAction and GetActionMappings for Write/List/Tagging actions
* fmt
* fix: resolve ListMultipartUploads/ListParts mapping inconsistency and add action validation
- Fix ListMultipartUploads and ListParts mapping in helpers.go:
- Changed from ACTION_LIST to ACTION_WRITE for consistency with GetActionMappings
- These operations are part of the multipart write workflow and should map to Write action
- Prevents inconsistent behavior when same actions processed through different code paths
- Add documentation to clarify multipart operations in Write action:
- Explain why ListMultipartUploads and ListParts are part of Write permissions
- These are required for meaningful multipart upload workflow management
- Add action validation to CreatePolicyFromLegacyIdentity:
- Validates action format before processing using ValidateActionMapping
- Logs warnings for invalid actions instead of silently skipping them
- Provides clearer error messages when invalid action types are used
- Ensures users know when their intended permissions weren't applied
- Consistent with ConvertLegacyActions validation approach
Fixes: Inconsistent action type mappings and silent failure for invalid actions
* fix: use keyed fields in struct literals
- Replace unsafe reflect.StringHeader/SliceHeader with safe unsafe.String/Slice (weed/query/sqltypes/unsafe.go)
- Add field names to Type_ScalarType struct literals (weed/mq/schema/schema_builder.go)
- Add Duration field name to FlexibleDuration struct literals across test files
- Add field names to bson.D struct literals (weed/filer/mongodb/mongodb_store_kv.go)
Fixes go vet warnings about unkeyed struct literals.
* fix: remove unreachable code
- Remove unreachable return statements after infinite for loops
- Remove unreachable code after if/else blocks where all paths return
- Simplify recursive logic by removing unnecessary for loop (inode_to_path.go)
- Fix Type_ScalarType literal to use enum value directly (schema_builder.go)
- Call onCompletionFn on stream error (subscribe_session.go)
Files fixed:
- weed/query/sqltypes/unsafe.go
- weed/mq/schema/schema_builder.go
- weed/mq/client/sub_client/connect_to_sub_coordinator.go
- weed/filer/redis3/ItemList.go
- weed/mq/client/agent_client/subscribe_session.go
- weed/mq/broker/broker_grpc_pub_balancer.go
- weed/mount/inode_to_path.go
- weed/util/skiplist/name_list.go
* fix: avoid copying lock values in protobuf messages
- Use proto.Merge() instead of direct assignment to avoid copying sync.Mutex in S3ApiConfiguration (iamapi_server.go)
- Add explicit comments noting that channel-received values are already copies before taking addresses (volume_grpc_client_to_master.go)
The protobuf messages contain sync.Mutex fields from the message state, which should not be copied.
Using proto.Merge() properly merges messages without copying the embedded mutex.
* fix: correct byte array size for uint32 bit shift operations
The generateAccountId() function only needs 4 bytes to create a uint32 value.
Changed from allocating 8 bytes to 4 bytes to match the actual usage.
This fixes go vet warning about shifting 8-bit values (bytes) by more than 8 bits.
* fix: ensure context cancellation on all error paths
In broker_client_subscribe.go, ensure subscriberCancel() is called on all error return paths:
- When stream creation fails
- When partition assignment fails
- When sending initialization message fails
This prevents context leaks when an error occurs during subscriber creation.
* fix: ensure subscriberCancel called for CreateFreshSubscriber stream.Send error
Ensure subscriberCancel() is called when stream.Send fails in CreateFreshSubscriber.
* ci: add go vet step to prevent future lint regressions
- Add go vet step to GitHub Actions workflow
- Filter known protobuf lock warnings (MessageState sync.Mutex)
These are expected in generated protobuf code and are safe
- Prevents accumulation of go vet errors in future PRs
- Step runs before build to catch issues early
* fix: resolve remaining syntax and logic errors in vet fixes
- Fixed syntax errors in filer_sync.go caused by missing closing braces
- Added missing closing brace for if block and function
- Synchronized fixes to match previous commits on branch
* fix: add missing return statements to daemon functions
- Add 'return false' after infinite loops in filer_backup.go and filer_meta_backup.go
- Satisfies declared bool return type signatures
- Maintains consistency with other daemon functions (runMaster, runFilerSynchronize, runWorker)
- While unreachable, explicitly declares the return satisfies function signature contract
* fix: add nil check for onCompletionFn in SubscribeMessageRecord
- Check if onCompletionFn is not nil before calling it
- Prevents potential panic if nil function is passed
- Matches pattern used in other callback functions
* docs: clarify unreachable return statements in daemon functions
- Add comments documenting that return statements satisfy function signature
- Explains that these returns follow infinite loops and are unreachable
- Improves code clarity for future maintainers
* feat(worker): add metrics HTTP server and debug profiling support
- Add -metricsPort flag to enable Prometheus metrics endpoint
- Add -metricsIp flag to configure metrics server bind address
- Implement /metrics endpoint for Prometheus-compatible metrics
- Implement /health endpoint for Kubernetes readiness/liveness probes
- Add -debug flag to enable pprof debugging server
- Add -debug.port flag to configure debug server port
- Fix stats package import naming conflict by using alias
- Update usage examples to show new flags
Fixes#7843
* feat(helm): add worker metrics and health check support
- Update worker readiness probe to use httpGet on /health endpoint
- Update worker liveness probe to use httpGet on /health endpoint
- Add metricsPort flag to worker command in deployment template
- Support both httpGet and tcpSocket probe types for backward compatibility
- Update values.yaml with health check configuration
This enables Kubernetes pod lifecycle management for worker components through
proper health checks on the new metrics HTTP endpoint.
* feat(mini): align all services to share single debug and metrics servers
- Disable S3's separate debug server in mini mode (port 6060 now shared by all)
- Add metrics server startup to embedded worker for health monitoring
- All services now share the single metrics port (9327) and single debug port (6060)
- Consistent pattern with master, filer, volume, webdav services
* fix(worker): fix variable shadowing in health check handler
- Rename http.ResponseWriter parameter from 'w' to 'rw' to avoid shadowing
the outer 'w *worker.Worker' parameter
- Prevents potential bugs if future code tries to use worker state in handler
- Improves code clarity and follows Go best practices
* fix(worker): remove unused worker parameter in metrics server
- Change 'w *worker.Worker' parameter to '_' as it's not used
- Clarifies intent that parameter is intentionally unused
- Follows Go best practices and improves code clarity
* fix(helm): fix trailing backslash syntax errors in worker command
- Fix conditional backslash placement to prevent shell syntax errors
- Only add backslash when metricsPort OR extraArgs are present
- Prevents worker pod startup failures due to malformed command arguments
- Ensures proper shell command parsing regardless of configuration state
* refactor(worker): use standard stats.StartMetricsServer for consistency
- Replace custom metrics server implementation with stats.StartMetricsServer
to match pattern used in master, volume, s3, filer_sync components
- Simplifies code and improves maintainability
- Uses glog.Fatal for errors (consistent with other SeaweedFS components)
- Remove unused net/http and prometheus/promhttp imports
- Automatically provides /metrics and /health endpoints via standard implementation
fix: consolidate Helm chart release with container image build
Resolve issue #7855 by consolidating the Helm chart release workflow
with the container image build workflow. This ensures perfect alignment:
1. Container images are built and pushed to GHCR
2. Images are copied from GHCR to Docker Hub
3. Helm chart is published only after step 2 completes
Previously, the Helm chart was published immediately on tag push before
images were available in Docker Hub, causing deployment failures.
Changes:
- Added helm-release job to container_release_unified.yml that depends
on copy-to-dockerhub job
- Removed helm_chart_release.yml workflow (consolidated into unified release)
Benefits:
- No race conditions between image push and chart publication
- Users can deploy immediately after release
- Single source of truth for release process
- Clearer job dependencies and execution flow
* fix: use path instead of filepath to handle urls in weed admin file browser
* test: add comprehensive tests for file browser path handling
- Test breadcrumb generation for various path scenarios
- Test path handling with forward slashes (URL compatibility)
- Test parent path calculation for Windows compatibility
- Test file extension handling using path.Ext
- Test bucket path detection logic
These tests verify that the switch from filepath to path package works
correctly and handles URLs properly across all platforms.
* refactor: simplify fullPath construction using path.Join
Replace verbose manual path construction with path.Join which:
- Handles trailing slashes automatically
- Is more concise and readable
- Is more robust for edge cases
* fix: normalize path in ShowFileBrowser and rename generateBreadcrumbs parameter
Critical fix:
- Add util.CleanWindowsPath() normalization to path parameter in ShowFileBrowser
handler, matching the pattern used in other file operation handlers
(lines 273, 464)
- This ensures Windows-style backslashes are converted to forward slashes
before processing, fixing path handling issues on Windows
Consistency improvement:
- Rename path parameter to dir in generateBreadcrumbs function
- Aligns with parameter rename in GetFileBrowser for consistent naming
throughout the file
* test: improve coverage for Windows path handling and production code behavior
Address reviewer feedback by enhancing test quality:
1. Improved test documentation:
- Added clear comments explaining what each test validates
- Clarified that some tests validate expected behavior vs production code
- Documented the Windows path normalization flow
2. Enhanced actual production code testing:
- TestGenerateBreadcrumbs: Calls actual production function
- TestBreadcrumbPathFormatting: Validates production output format
- TestDirectoryNavigation: Integration-style test for complete flow
3. Added new test functions for better coverage:
- TestPathJoinHandlesEdgeCases: Verifies path.Join behavior
- TestWindowsPathNormalizationBehavior: Documents expected normalization
- TestDirectoryNavigation: Complete navigation flow test
4. Improved test organization:
- Fixed duplicate field naming issues
- Better test names for clarity
- More comprehensive edge case coverage
These improvements ensure the fix for issue #7628 (Windows path handling)
is properly validated across the complete flow from handler to path logic.
* test: use actual util.CleanWindowsPath function in Windows path normalization test
Address reviewer feedback by testing the actual production function:
- Import util package for CleanWindowsPath
- Call the real util.CleanWindowsPath() instead of reimplementing logic
- Ensures test validates actual implementation, not just expected behavior
- Added more test cases for edge cases (simple path, deep nesting)
This change validates that the Windows path normalization in the
ShowFileBrowser handler (handlers/file_browser_handlers.go:64)
works correctly with the actual util.CleanWindowsPath function.
* style: fix indentation in TestPathJoinHandlesEdgeCases
Align t.Errorf statement inside the if block with proper indentation.
The error message now correctly aligns with the if block body,
maintaining consistent indentation throughout the function.
* test: restore backslash validation check in TestPathJoinHandlesEdgeCases
---------
Co-authored-by: Chris Lu <chris.lu@gmail.com>
* refactor: Convert versioning to three-state string model matching AWS S3
- Change VersioningEnabled bool to VersioningStatus string in S3Bucket struct
- Add GetVersioningStatus() function returning empty string (never enabled), 'Enabled', or 'Suspended'
- Update StoreVersioningInExtended() to delete key instead of setting 'Suspended'
- Ensures Admin UI and S3 API use consistent versioning state representation
* fix: Add validation for bucket quota and Object Lock configuration
- Prevent buckets with quota enabled but size=0 (validation check)
- Fix Object Lock mode handling to only pass mode when setDefaultRetention is true
- Ensures proper extended attribute storage for Object Lock configuration
- Matches AWS S3 behavior for Object Lock setup
* feat: Handle versioned objects in bucket details view
- Recognize .versions directories as versioned objects in listBucketObjects()
- Extract size and mtime from extended attribute metadata (ExtLatestVersionSizeKey, ExtLatestVersionMtimeKey)
- Add length validation (8 bytes) before parsing extended attribute byte arrays
- Update GetBucketDetails() and GetS3Buckets() to use new GetVersioningStatus()
- Properly display versioned objects without .versions suffix in bucket details
* ui: Update bucket management UI to show three-state versioning and Object Lock
- Change versioning display from binary (Enabled/Disabled) to three-state (Not configured/Enabled/Suspended)
- Update Object Lock display to show 'Not configured' instead of 'Disabled'
- Fix bucket details modal to use bucket.versioning_status instead of bucket.versioning_enabled
- Update displayBucketDetails() JavaScript to handle three versioning states
* chore: Regenerate template code for bucket UI changes
- Generated from updated s3_buckets.templ
- Reflects three-state versioning and Object Lock UI improvements
The GrpcMaxConnectionAge and GrpcMaxConnectionAgeGrace constants have a troubled
history - they were removed in 2022 due to gRPC issues, reverted later, and
recently re-added. However, they are not essential to the core worker reconnection
fix which was solved through proper goroutine ordering.
The Docker Swarm DNS handling mentioned in the comments is not critical, and
these parameters have caused problems in the past. Removing them simplifies
the configuration without losing functionality.
* feat: add automatic port detection and fallback for mini command
- Added port availability detection using TCP binding tests
- Implemented port fallback mechanism searching for available ports
- Support for both HTTP and gRPC port handling
- IP-aware port checking using actual service bind address
- Dual-interface verification (specific IP and wildcard 0.0.0.0)
- All services (Master, Volume, Filer, S3, WebDAV, Admin) auto-reallocate to available ports
- Enables multiple mini instances to run simultaneously without conflicts
* fix: use actual bind IP for service health checks
- Previously health checks were hardcoded to localhost (127.0.0.1)
- This caused failures when services bind to actual IP (e.g., 10.21.153.8)
- Now health checks use the same IP that services are binding to
- Fixes Volume and other service health check failures on non-localhost IPs
* refactor: improve port detection logic and remove gRPC handling duplication
- findAvailablePortOnIP now returns 0 on failure instead of unavailable port
Allows callers to detect when port finding fails and handle appropriately
- Remove duplicate gRPC port handling from ensureAllPortsAvailableOnIP
All gRPC port logic is now centralized in initializeGrpcPortsOnIP
- Log final port configuration only after all ports are finalized
Both HTTP and gRPC ports are now correctly initialized before logging
- Add error logging when port allocation fails
Makes debugging easier when ports can't be found
* refactor: fix race condition and clean up port detection code
- Convert parallel HTTP port checks to sequential to prevent race conditions
where multiple goroutines could allocate the same available port
- Remove unused 'sync' import since WaitGroup is no longer used
- Add documentation to localhost wrapper functions explaining they are
kept for backwards compatibility and future use
- All gRPC port logic is now exclusively handled in initializeGrpcPortsOnIP
eliminating any duplication in ensureAllPortsAvailableOnIP
* refactor: address code review comments - constants, helper function, and cleanup
- Define GrpcPortOffset constant (10000) to replace magic numbers throughout
the code for better maintainability and consistency
- Extract bindIp determination logic into getBindIp() helper function
to eliminate code duplication between runMini and startMiniServices
- Remove redundant 'calculatedPort = calculatedPort' assignment that had no effect
- Update all gRPC port calculations to use GrpcPortOffset constant
(lines 489, 886 and the error logging at line 501)
* refactor: remove unused wrapper functions and update documentation
- Remove unused localhost wrapper functions that were never called:
- isPortOpen() - wrapper around isPortOpenOnIP with hardcoded 127.0.0.1
- findAvailablePort() - wrapper around findAvailablePortOnIP with hardcoded 127.0.0.1
- ensurePortAvailable() - wrapper around ensurePortAvailableOnIP with hardcoded 127.0.0.1
- ensureAllPortsAvailable() - wrapper around ensureAllPortsAvailableOnIP with hardcoded 127.0.0.1
Since this is new functionality with no backwards compatibility concerns,
these wrapper functions were not needed. The comments claiming they were
'kept for future use or backwards compatibility' are no longer valid.
- Update documentation to reference GrpcPortOffset constant instead of hardcoded 10000:
- Update comment in ensureAllPortsAvailableOnIP to use GrpcPortOffset
- Update admin.port.grpc flag help text to reference GrpcPortOffset
Note: getBindIp() is actually being used and should be retained (contrary to
the review comment suggesting it was unused - it's called in both runMini
and startMiniServices functions)
* refactor: prevent HTTP/gRPC port collisions and improve error handling
- Add upfront reservation of all calculated gRPC ports before allocating HTTP ports
to prevent collisions where an HTTP port allocation could use a port that will
later be needed for a gRPC port calculation.
Example scenario that is now prevented:
- Master HTTP reallocated from 9333 to 9334 (original in use)
- Filer HTTP search finds 19334 available and assigns it
- Master gRPC calculated as 9334 + GrpcPortOffset = 19334 → collision!
Now: reserved gRPC ports are tracked upfront and HTTP port search skips them.
- Improve admin server gRPC port fallback error handling:
- Change from silent V(1) verbose log to Warningf to make the error visible
- Update comment to clarify this indicates a problem in the port initialization sequence
- Add explanation that the fallback calculation may cause bind failure
- Update ensureAllPortsAvailableOnIP comment to clarify it avoids reserved ports
* fix: enforce reserved ports in HTTP allocation and improve admin gRPC fallback
Critical fixes for port allocation safety:
1. Make findAvailablePortOnIP and ensurePortAvailableOnIP aware of reservedPorts:
- Add reservedPorts map parameter to both functions
- findAvailablePortOnIP now skips reserved ports when searching for alternatives
- ensurePortAvailableOnIP passes reservedPorts through to findAvailablePortOnIP
- This prevents HTTP ports from being allocated to ports reserved for gRPC
2. Update ensureAllPortsAvailableOnIP to pass reservedPorts:
- Pass the reservedPorts map to ensurePortAvailableOnIP calls
- Maintains the map updates (delete/add) for accuracy as ports change
3. Replace blind admin gRPC port fallback with proper availability checks:
- Previous code just calculated *miniAdminOptions.port + GrpcPortOffset
- New code checks both the calculated port and finds alternatives if needed
- Uses the same availability checking logic as initializeGrpcPortsOnIP
- Properly logs the fallback process and any port changes
- Will fail gracefully if no available ports found (consistent with other services)
These changes eliminate two critical vulnerabilities:
- HTTP port allocation can no longer accidentally claim gRPC ports
- Admin gRPC port fallback no longer blindly uses an unchecked port
* fix: prevent gRPC port collisions during multi-service fallback allocation
Critical fix for gRPC port allocation safety across multiple services:
Problem: When multiple services need gRPC port fallback allocation in sequence
(e.g., Master gRPC unavailable → finds alternative, then Filer gRPC unavailable
→ searches from calculated port), there was no tracking of previously allocated
gRPC ports. This could allow two services to claim the same port.
Scenario that is now prevented:
- Master gRPC: calculated 19333 unavailable → finds 19334 → assigns 19334
- Filer gRPC: calculated 18888 unavailable → searches from 18889, might land on
19334 if consecutive ports in range are unavailable (especially with custom
port configurations or in high-port-contention environments)
Solution:
- Add allocatedGrpcPorts map to track gRPC ports allocated within the function
- Check allocatedGrpcPorts before using calculated port for each service
- Pass allocatedGrpcPorts to findAvailablePortOnIP when finding fallback ports
- Add allocatedGrpcPorts[port] = true after each successful allocation
- This ensures no two services can allocate the same gRPC port
The fix handles both:
1. Calculated gRPC ports (when grpcPort == 0)
2. Explicitly set gRPC ports (when user provides -service.port.grpc value)
While default port spacing makes collision unlikely, this fix is essential for:
- Custom port configurations
- High-contention environments
- Edge cases with many unavailable consecutive ports
- Correctness and safety guarantees
* feat: enforce hard-fail behavior for explicitly specified ports
When users explicitly specify a port via command-line flags (e.g., -s3.port=8333),
the server should fail immediately if the port is unavailable, rather than silently
falling back to an alternative port. This prevents user confusion and makes misconfiguration
failures obvious.
Changes:
- Modified ensurePortAvailableOnIP() to check if a port was explicitly passed via isFlagPassed()
- If an explicit port is unavailable, return error instead of silently allocating alternative
- Updated ensureAllPortsAvailableOnIP() to handle the returned error and fail startup
- Modified runMini() to check error from ensureAllPortsAvailableOnIP() and return false on failure
- Default ports (not explicitly specified) continue to fallback to available alternatives
This ensures:
- Explicit ports: fail if unavailable (e.g., -s3.port=8333 fails if 8333 is taken)
- Default ports: fallback to alternatives (e.g., s3.port without flag falls back to 8334 if 8333 taken)
* fix: accurate error messages for explicitly specified unavailable ports
When a port is explicitly specified via CLI flags but is unavailable, the error message
now correctly reports the originally requested port instead of reporting a fallback port
that was calculated internally.
The issue was that the config file applied after CLI flag parsing caused isFlagPassed()
to return true for ports loaded from the config file (since flag.Visit() was called during
config file application), incorrectly marking them as explicitly specified.
Solution: Capture which port flags were explicitly passed on the CLI BEFORE the config file
is applied, storing them in the explicitPortFlags map. This preserves the accurate
distinction between user-specified ports and defaults/config-file ports.
Example:
- User runs: weed mini -dir=. -s3.port=22
- Now correctly shows: 'port 22 for S3 (specified by flag s3.port) is not available'
- Previously incorrectly showed: 'port 8334 for S3...' (some calculated fallback)
* fix: respect explicitly specified ports and prevent config file override
When a port is explicitly specified via CLI flags (e.g., -s3.port=8333),
the config file options should NOT override it. Previously, config file
options would be applied if the flag value differed from default, but
this check wasn't sufficient to prevent override in all cases.
Solution: Check the explicitPortFlags map before applying any config file
port options. If a port was explicitly passed on the CLI, skip applying
the config file option for that port.
This ensures:
- Explicit ports take absolute precedence over config file ports
- Config file ports are only used if port wasn't specified on CLI
- Example: 'weed mini -s3.port=8333' will use 8333, never the config file value
* fix: don't print usage on port allocation error
When a port allocation fails (e.g., explicit port is unavailable), exit
immediately without showing the usage example. This provides cleaner
error output when the error is expected (port conflict).
* refactor: clean up code quality issues
Remove no-op assignment (calculatedPort = calculatedPort) that had no effect.
The variable already holds the correct value when no alternative port is
found.
Improve documentation for the defensive gRPC port initialization fallback
in startAdminServer. While this code shouldn't execute in normal flow
because ensureAllPortsAvailableOnIP is called earlier in runMini, the
fallback handles edge cases where port initialization may have been skipped
or failed silently due to configuration changes or error handling paths.
* fix: improve worker reconnection robustness and prevent handleOutgoing hang
- Add dedicated streamFailed signaling channel to abort registration waits early when stream dies
- Add per-connection regWait channel to route RegistrationResponse separately from shared incoming channel, avoiding race where other consumers steal the response
- Refactor handleOutgoing() loop to use select on streamExit/errCh, ensuring old handlers exit cleanly on reconnect (prevents stale senders competing with new stream)
- Buffer msgCh to reduce shutdown edge cases
- Add cleanup of streamFailed and regWait channels on reconnect/disconnect
- Fixes registration timeout and potential stream lifecycle hangs on aggressive server max_age recycling
* fix: prevent deadlock when stream error occurs - make cmds send non-blocking
If managerLoop is blocked (e.g., waiting on regWait), a blocking send to cmds
will deadlock handleIncoming. Make the send non-blocking to prevent this.
* fix: address code review comments on mini.go port allocation
- Remove flawed fallback gRPC port initialization and convert to fatal error
(ensures port initialization issues are caught immediately instead of silently
failing with an empty reserved ports map)
- Extract common port validation logic to eliminate duplication between
calculated and explicitly set gRPC port handling
* Fix critical race condition and improve error handling in worker client
- Capture channel pointers before checking for nil (prevents TOCTOU race with reconnect)
- Use async fallback goroutine for cmds send to prevent error loss when manager is busy
- Consistently close regWait channel on disconnect (matches streamFailed behavior)
- Complete cleanup of channels on failed registration
- Improve error messages for clarity (replace 'timeout' with 'failed' where appropriate)
* Add debug logging for registration response routing
Add glog.V(3) and glog.V(2) logs to track successful and dropped registration
responses in handleIncoming, helping diagnose registration issues in production.
* Update weed/worker/client.go
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* Ensure stream errors are never lost by using async fallback
When handleIncoming detects a stream error, queue ActionStreamError to managerLoop
with non-blocking send. If managerLoop is busy and cmds channel is full, spawn an
async goroutine to queue the error asynchronously. This ensures the manager is
always notified of stream failures, preventing the connection from remaining in an
inconsistent state (connected=true while stream is dead).
* Refactor handleOutgoing to eliminate duplicate error handling code
Extract error handling and cleanup logic into helper functions to avoid duplication
in nested select statements. This improves maintainability and reduces the risk of
inconsistencies when updating error handling logic.
* Prevent goroutine leaks by adding timeouts to blocking cmds sends
Add 2-second timeouts to both handleStreamError and the async fallback goroutine
when sending ActionStreamError to cmds channel. This prevents the handleOutgoing
and handleIncoming goroutines from blocking indefinitely if the managerLoop is
no longer receiving (e.g., during shutdown), preventing resource leaks.
* Properly close regWait channel in reconnect to prevent resource leaks
Close the regWait channel before setting it to nil in reconnect(), matching the
pattern used in handleDisconnect(). This ensures any goroutines waiting on this
channel during reconnection are properly signaled, preventing them from hanging.
* Use non-blocking async pattern in handleOutgoing error reporting
Refactor handleStreamError to use non-blocking send with async fallback goroutine,
matching the pattern used in handleIncoming. This allows handleOutgoing to exit
immediately when errors occur rather than blocking for up to 2 seconds, improving
responsiveness and consistency across handlers.
* fix: drain regWait channel before closing to prevent message loss
- Add drain loop before closing regWait in reconnect() cleanup
- Add drain loop before closing regWait in handleDisconnect() cleanup
- Ensures no pending RegistrationResponse messages are lost during channel closure
* docs: add comments explaining regWait buffered channel design
- Document that regWait buffer size 1 prevents race conditions
- Explain non-blocking send pattern between sendRegistration and handleIncoming
- Clarify timing of registration response handling in handleIncoming
* fix: improve error messages and channel handling in sendRegistration
- Clarify error message when stream fails before registration sent
- Use two-value receive form to properly detect closed channels
- Better distinguish between closed channel and nil value scenarios
* refactor: extract drain and close channel logic into helper function
- Create drainAndCloseRegWaitChannel() helper to eliminate code duplication
- Replace 3 copies of drain-and-close logic with single function call
- Improves maintainability and consistency across cleanup paths
---------
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* Fix: Eliminate duplicate versioned objects in S3 list operations
- Move versioned directory processing outside of pagination loop to process only once
- Add deduplication during .versions directory collection phase
- Fix directory handling to not add directories to results in recursive mode
- Directly add versioned entries to contents array instead of using callback
Fixes issue where AWS S3 list operations returned duplicated versioned objects
(e.g., 1000 duplicate entries from 4 unique objects). Now correctly returns only
the unique logical entries without duplication.
Verified with:
aws s3api list-objects --endpoint-url http://localhost:8333 --bucket pm-itatiaiucu-01
Returns exactly 4 entries (ClientInfo.xml and Repository from 2 Veeam backup folders)
* Refactor: Process .versions directories immediately when encountered
Instead of collecting .versions directories and processing them after the
pagination loop, process them immediately when encountered during traversal.
Benefits:
- Simpler code: removed versionedDirEntry struct and collection array
- More efficient: no need to store and iterate through collected entries
- Same O(V) complexity but with less memory overhead
- Clearer logic: processing happens in one pass during traversal
Since each .versions directory is only visited once during recursive
traversal (we never traverse into them), there's no need for deferred
processing or deduplication.
* Add comprehensive tests for versioned objects list
- TestListObjectsWithVersionedObjects: Tests listing with various delimiters
- TestVersionedObjectsNoDuplication: Core test validating no 250x duplication
- TestVersionedObjectsWithDeleteMarker: Tests delete marker filtering
- TestVersionedObjectsMaxKeys: Tests pagination with versioned objects
- TestVersionsDirectoryNotTraversed: Ensures .versions never traversed
- Fix existing test signature to match updated doListFilerEntries
* style: Fix formatting alignment in versioned objects tests
* perf: Optimize path extraction using string indexing
Replace multiple strings.Split/Join calls with efficient strings.Index
slicing to extract bucket-relative path from directory string.
Reduces unnecessary allocations and improves performance in versioned
objects listing path construction.
* refactor: Address code review feedback from Gemini Code Assist
1. Fix misleading comment about versioned directory processing location.
Versioned directories are processed immediately in doListFilerEntries,
not deferred to ListObjectsV1Handler.
2. Simplify path extraction logic using explicit bucket path construction
instead of index-based string slicing for better readability and
maintainability.
3. Add clarifying comment to test callback explaining why production logic
is duplicated - necessary because listFilerEntries is not easily testable
with filer client injection.
* fmt
* refactor: Address code review feedback from Copilot
- Fix misleading comment about versioned directory processing location
(note that processing happens within doListFilerEntries, not at top level)
- Add maxKeys validation checks in all test callbacks for consistency
- Add maxKeys check before calling eachEntryFn for versioned objects
- Improve test documentation to clarify testing approach and avoid apologetic tone
* refactor: Address code review feedback from Gemini Code Assist
- Remove redundant maxKeys check before eachEntryFn call on line 541
(the loop already checks maxKeys <= 0 at line 502, ensuring quota exists)
- Fix pagination pattern consistency in all test callbacks
- TestVersionedObjectsNoDuplication: Use cursor.maxKeys <= 0 check and decrement
- TestVersionedObjectsWithDeleteMarker: Use cursor.maxKeys <= 0 check and decrement
- TestVersionsDirectoryNotTraversed: Use cursor.maxKeys <= 0 check and decrement
- Ensures consistent pagination logic across all callbacks matching production behavior
* refactor: Address code review suggestions for code quality
- Adjust log verbosity from V(5) to V(4) for file additions to reduce noise
while maintaining useful debug output during troubleshooting
- Remove unused isRecursive parameter from doListFilerEntries function
signature and all call sites (not used for any logic decisions)
- Consolidate redundant comments about versioned directory handling
to reduce documentation duplication
These changes improve code maintainability and clarity.
* fmt
* refactor: Add pagination test and optimize stream processing
- Add comprehensive test validation to TestVersionedObjectsMaxKeys
that verifies truncation is correctly set when maxKeys is exhausted
with more entries available, ensuring proper pagination state
- Optimize stream processing in doListFilerEntries by using 'break'
instead of 'continue' when quota is exhausted (cursor.maxKeys <= 0)
This avoids receiving and discarding entries from the stream when
we've already reached the requested limit, improving efficiency
Fixes#7230
When a cluster goes down during file replication, the chunk upload process
would fail after a limited number of retries. Once the remote cluster came
back online, those failed uploads were never retried, leaving the clusters
out-of-sync.
This change enables the RetryForever flag in the UploadOption when
replicating chunks between filers. This ensures that upload operations
will keep retrying indefinitely, and once the remote cluster comes back
online, the pending uploads will automatically succeed.
Users no longer need to manually run fs.meta.save and fs.meta.load as
a workaround for out-of-sync clusters.
* feat: add automatic port detection and fallback for mini command
- Added port availability detection using TCP binding tests
- Implemented port fallback mechanism searching for available ports
- Support for both HTTP and gRPC port handling
- IP-aware port checking using actual service bind address
- Dual-interface verification (specific IP and wildcard 0.0.0.0)
- All services (Master, Volume, Filer, S3, WebDAV, Admin) auto-reallocate to available ports
- Enables multiple mini instances to run simultaneously without conflicts
* fix: use actual bind IP for service health checks
- Previously health checks were hardcoded to localhost (127.0.0.1)
- This caused failures when services bind to actual IP (e.g., 10.21.153.8)
- Now health checks use the same IP that services are binding to
- Fixes Volume and other service health check failures on non-localhost IPs
* refactor: improve port detection logic and remove gRPC handling duplication
- findAvailablePortOnIP now returns 0 on failure instead of unavailable port
Allows callers to detect when port finding fails and handle appropriately
- Remove duplicate gRPC port handling from ensureAllPortsAvailableOnIP
All gRPC port logic is now centralized in initializeGrpcPortsOnIP
- Log final port configuration only after all ports are finalized
Both HTTP and gRPC ports are now correctly initialized before logging
- Add error logging when port allocation fails
Makes debugging easier when ports can't be found
* refactor: fix race condition and clean up port detection code
- Convert parallel HTTP port checks to sequential to prevent race conditions
where multiple goroutines could allocate the same available port
- Remove unused 'sync' import since WaitGroup is no longer used
- Add documentation to localhost wrapper functions explaining they are
kept for backwards compatibility and future use
- All gRPC port logic is now exclusively handled in initializeGrpcPortsOnIP
eliminating any duplication in ensureAllPortsAvailableOnIP
* refactor: address code review comments - constants, helper function, and cleanup
- Define GrpcPortOffset constant (10000) to replace magic numbers throughout
the code for better maintainability and consistency
- Extract bindIp determination logic into getBindIp() helper function
to eliminate code duplication between runMini and startMiniServices
- Remove redundant 'calculatedPort = calculatedPort' assignment that had no effect
- Update all gRPC port calculations to use GrpcPortOffset constant
(lines 489, 886 and the error logging at line 501)
* refactor: remove unused wrapper functions and update documentation
- Remove unused localhost wrapper functions that were never called:
- isPortOpen() - wrapper around isPortOpenOnIP with hardcoded 127.0.0.1
- findAvailablePort() - wrapper around findAvailablePortOnIP with hardcoded 127.0.0.1
- ensurePortAvailable() - wrapper around ensurePortAvailableOnIP with hardcoded 127.0.0.1
- ensureAllPortsAvailable() - wrapper around ensureAllPortsAvailableOnIP with hardcoded 127.0.0.1
Since this is new functionality with no backwards compatibility concerns,
these wrapper functions were not needed. The comments claiming they were
'kept for future use or backwards compatibility' are no longer valid.
- Update documentation to reference GrpcPortOffset constant instead of hardcoded 10000:
- Update comment in ensureAllPortsAvailableOnIP to use GrpcPortOffset
- Update admin.port.grpc flag help text to reference GrpcPortOffset
Note: getBindIp() is actually being used and should be retained (contrary to
the review comment suggesting it was unused - it's called in both runMini
and startMiniServices functions)
* refactor: prevent HTTP/gRPC port collisions and improve error handling
- Add upfront reservation of all calculated gRPC ports before allocating HTTP ports
to prevent collisions where an HTTP port allocation could use a port that will
later be needed for a gRPC port calculation.
Example scenario that is now prevented:
- Master HTTP reallocated from 9333 to 9334 (original in use)
- Filer HTTP search finds 19334 available and assigns it
- Master gRPC calculated as 9334 + GrpcPortOffset = 19334 → collision!
Now: reserved gRPC ports are tracked upfront and HTTP port search skips them.
- Improve admin server gRPC port fallback error handling:
- Change from silent V(1) verbose log to Warningf to make the error visible
- Update comment to clarify this indicates a problem in the port initialization sequence
- Add explanation that the fallback calculation may cause bind failure
- Update ensureAllPortsAvailableOnIP comment to clarify it avoids reserved ports
* fix: enforce reserved ports in HTTP allocation and improve admin gRPC fallback
Critical fixes for port allocation safety:
1. Make findAvailablePortOnIP and ensurePortAvailableOnIP aware of reservedPorts:
- Add reservedPorts map parameter to both functions
- findAvailablePortOnIP now skips reserved ports when searching for alternatives
- ensurePortAvailableOnIP passes reservedPorts through to findAvailablePortOnIP
- This prevents HTTP ports from being allocated to ports reserved for gRPC
2. Update ensureAllPortsAvailableOnIP to pass reservedPorts:
- Pass the reservedPorts map to ensurePortAvailableOnIP calls
- Maintains the map updates (delete/add) for accuracy as ports change
3. Replace blind admin gRPC port fallback with proper availability checks:
- Previous code just calculated *miniAdminOptions.port + GrpcPortOffset
- New code checks both the calculated port and finds alternatives if needed
- Uses the same availability checking logic as initializeGrpcPortsOnIP
- Properly logs the fallback process and any port changes
- Will fail gracefully if no available ports found (consistent with other services)
These changes eliminate two critical vulnerabilities:
- HTTP port allocation can no longer accidentally claim gRPC ports
- Admin gRPC port fallback no longer blindly uses an unchecked port
* fix: prevent gRPC port collisions during multi-service fallback allocation
Critical fix for gRPC port allocation safety across multiple services:
Problem: When multiple services need gRPC port fallback allocation in sequence
(e.g., Master gRPC unavailable → finds alternative, then Filer gRPC unavailable
→ searches from calculated port), there was no tracking of previously allocated
gRPC ports. This could allow two services to claim the same port.
Scenario that is now prevented:
- Master gRPC: calculated 19333 unavailable → finds 19334 → assigns 19334
- Filer gRPC: calculated 18888 unavailable → searches from 18889, might land on
19334 if consecutive ports in range are unavailable (especially with custom
port configurations or in high-port-contention environments)
Solution:
- Add allocatedGrpcPorts map to track gRPC ports allocated within the function
- Check allocatedGrpcPorts before using calculated port for each service
- Pass allocatedGrpcPorts to findAvailablePortOnIP when finding fallback ports
- Add allocatedGrpcPorts[port] = true after each successful allocation
- This ensures no two services can allocate the same gRPC port
The fix handles both:
1. Calculated gRPC ports (when grpcPort == 0)
2. Explicitly set gRPC ports (when user provides -service.port.grpc value)
While default port spacing makes collision unlikely, this fix is essential for:
- Custom port configurations
- High-contention environments
- Edge cases with many unavailable consecutive ports
- Correctness and safety guarantees
* feat: enforce hard-fail behavior for explicitly specified ports
When users explicitly specify a port via command-line flags (e.g., -s3.port=8333),
the server should fail immediately if the port is unavailable, rather than silently
falling back to an alternative port. This prevents user confusion and makes misconfiguration
failures obvious.
Changes:
- Modified ensurePortAvailableOnIP() to check if a port was explicitly passed via isFlagPassed()
- If an explicit port is unavailable, return error instead of silently allocating alternative
- Updated ensureAllPortsAvailableOnIP() to handle the returned error and fail startup
- Modified runMini() to check error from ensureAllPortsAvailableOnIP() and return false on failure
- Default ports (not explicitly specified) continue to fallback to available alternatives
This ensures:
- Explicit ports: fail if unavailable (e.g., -s3.port=8333 fails if 8333 is taken)
- Default ports: fallback to alternatives (e.g., s3.port without flag falls back to 8334 if 8333 taken)
* fix: accurate error messages for explicitly specified unavailable ports
When a port is explicitly specified via CLI flags but is unavailable, the error message
now correctly reports the originally requested port instead of reporting a fallback port
that was calculated internally.
The issue was that the config file applied after CLI flag parsing caused isFlagPassed()
to return true for ports loaded from the config file (since flag.Visit() was called during
config file application), incorrectly marking them as explicitly specified.
Solution: Capture which port flags were explicitly passed on the CLI BEFORE the config file
is applied, storing them in the explicitPortFlags map. This preserves the accurate
distinction between user-specified ports and defaults/config-file ports.
Example:
- User runs: weed mini -dir=. -s3.port=22
- Now correctly shows: 'port 22 for S3 (specified by flag s3.port) is not available'
- Previously incorrectly showed: 'port 8334 for S3...' (some calculated fallback)
* fix: respect explicitly specified ports and prevent config file override
When a port is explicitly specified via CLI flags (e.g., -s3.port=8333),
the config file options should NOT override it. Previously, config file
options would be applied if the flag value differed from default, but
this check wasn't sufficient to prevent override in all cases.
Solution: Check the explicitPortFlags map before applying any config file
port options. If a port was explicitly passed on the CLI, skip applying
the config file option for that port.
This ensures:
- Explicit ports take absolute precedence over config file ports
- Config file ports are only used if port wasn't specified on CLI
- Example: 'weed mini -s3.port=8333' will use 8333, never the config file value
* fix: don't print usage on port allocation error
When a port allocation fails (e.g., explicit port is unavailable), exit
immediately without showing the usage example. This provides cleaner
error output when the error is expected (port conflict).
* fix: increase worker registration timeout for reconnections
Increase the worker registration timeout from 10 seconds to 30 seconds.
The 10-second timeout was too aggressive for reconnections when the admin
server might be busy processing other operations. Reconnecting workers need
more time to:
1. Re-establish the gRPC connection
2. Send the registration message
3. Wait for the admin server to process and respond
This prevents spurious "registration timeout" errors during long-running
mini instances when brief network hiccups or admin server load cause delays.
* refactor: clean up code quality issues
Remove no-op assignment (calculatedPort = calculatedPort) that had no effect.
The variable already holds the correct value when no alternative port is
found.
Improve documentation for the defensive gRPC port initialization fallback
in startAdminServer. While this code shouldn't execute in normal flow
because ensureAllPortsAvailableOnIP is called earlier in runMini, the
fallback handles edge cases where port initialization may have been skipped
or failed silently due to configuration changes or error handling paths.
* fix: prevent panic on close of closed channel in worker client reconnection
- Use idiomatic Go pattern of setting channels to nil after closing instead of flags
- Extract repeated safe-close logic into safeCloseChannel() helper method
- Call safeCloseChannel() in attemptConnection(), reconnect(), and handleDisconnect()
- In safeCloseChannel(), check if channel is not nil, close it, and set to nil
- Also set streamExit to nil in attemptConnection() when registration fails
- This follows Go best practices for channel management and prevents double-close panics
- Improved code maintainability by eliminating duplication
* fix: prevent panic on close of closed channel in worker client reconnection
- Use idiomatic Go pattern of setting channels to nil after closing instead of flags
- Extract repeated safe-close logic into safeCloseChannel() helper method
- Call safeCloseChannel() in attemptConnection(), reconnect(), and handleDisconnect()
- In safeCloseChannel(), check if channel is not nil, close it, and set to nil
- Also set streamExit to nil in attemptConnection() when registration fails
- Document thread-safety assumptions: function is safe in current usage (serialized
in managerLoop) but would need synchronization if used in concurrent contexts
- This follows Go best practices for channel management and prevents double-close panics
- Improved code maintainability by eliminating duplication
* feat: auto-configure optimal volume size limit based on available disk space
- Add calculateOptimalVolumeSizeMB() function with OS-independent disk detection
- Reuses existing stats.NewDiskStatus() which works across Linux, macOS, Windows, BSD, Solaris
- Algorithm: available disk / 100, rounded up to nearest power of 2 (64MB, 128MB, 256MB, 512MB, 1024MB)
- Volume size capped to maximum of 1GB (1024MB) for better stability
- Minimum volume size is 64MB
- Uses efficient bits.Len() for power-of-2 rounding instead of floating-point operations
- Only auto-calculates volume size if user didn't specify a custom value via -master.volumeSizeLimitMB
- Respects user-specified values without override
- Master logs whether value was auto-calculated or user-specified
- Welcome message displays the configured volume size with correct format string ordering
- Removed unused autoVolumeSizeMB variable (logging handles source tracking)
Fixes: #0
* Refactor: Consolidate volume size constants and use robust flag detection for mini mode
This commit addresses all code review feedback on the auto-optimal volume size feature:
1. **Consolidate hardcoded defaults into package-level constants**
- Moved minVolumeSizeMB=64 and maxVolumeSizeMB=1024 from local function-scope
constants to package-level constants for consistency and maintainability
- All three volume size constants (min, default, max) now defined in one place
2. **Implement robust flag detection using flag.Visit()**
- Added isFlagPassed() helper function using flag.Visit() to check if a CLI
flag was explicitly passed on the command line
- Replaces the previous implementation that checked if current value equals
default (which could incorrectly assume user intent if default was specified)
- Now correctly detects user override regardless of the actual value
3. **Restructure power-of-2 rounding logic for clarity**
- Changed from 'only round if above min threshold' to 'always round to power-of-2
first, then apply min/max constraints'
- More robust: works correctly even if min/max constants are adjusted in future
- Clearer intent: all non-zero values go through consistent rounding logic
4. **Fix import ordering**
- Added 'flag' import (aliased to fla9 package) to support isFlagPassed()
- Added 'math/bits' import to support power-of-2 rounding
Benefits:
- Better code organization with all volume size limits in package constants
- Correct user override detection that doesn't rely on value equality checks
- More maintainable rounding logic that's easier to understand and modify
- Consistent with SeaweedFS conventions (uses fla9 package like other commands)
* fix: Address code review feedback for volume size calculation
This commit resolves three code review comments for better code quality and robustness:
1. **Handle comma-separated directories in -dir flag**
- The -dir flag accepts comma-separated list of directories, but the volume size
calculation was passing the entire string to util.ResolvePath()
- Now splits on comma and uses the first directory for disk space calculation
- Added explanatory comment about the multi-directory support
- Ensures the optimal size calculation works correctly in all scenarios
2. **Change disk detection failure from verbose log to warning**
- When disk status cannot be determined, the warning is now logged via
glog.Warningf() instead of glog.V(1).Infof()
- Makes the event visible in default logs without requiring verbose mode
- Better alerting for operators about fallback to default values
3. **Avoid recalculating availableMB/100 and define bytesPerMB constant**
- Added bytesPerMB = 1024*1024 constant for clarity and reusability
- Replaced hardcoded (1024 * 1024) with bytesPerMB constant
- Store availableMB/100 in initialOptimalMB variable to avoid recalculation
- Log message now references initialOptimalMB instead of recalculating
- Improves maintainability and reduces redundant computation
All three changes maintain the same logic while improving code quality and
robustness as requested by the reviewer.
* fix: Address rounding logic, logging clarity, and disk capacity measurement issues
This commit resolves three additional code review comments to improve robustness
and clarity of the volume size calculation:
1. **Fix power-of-2 rounding logic for edge cases**
- The previous condition 'if optimalMB > 0' created a bug: when optimalMB=1,
bits.Len(0)=0, resulting in 1<<0=1, which is below minimum (64MB)
- Changed to explicitly handle zero case first: 'if optimalMB == 0'
- Separate zero-handling from power-of-2 rounding ensures correct behavior:
* optimalMB=0 → set to minVolumeSizeMB (64)
* optimalMB>=1 → apply power-of-2 rounding
- Then apply min/max constraints unconditionally
- More explicit and easier to reason about correctness
2. **Use total disk capacity instead of free space for stable configuration**
- Changed from diskStatus.Free (available space) to diskStatus.All (total capacity)
- Free space varies based on current disk usage at startup time
- This caused inconsistent volume sizes: same disk could get different sizes
depending on how full it is when the service starts
- Using total capacity ensures predictable, stable configuration across restarts
- Better aligns with the intended behavior of sizing based on disk capacity
- Added explanatory comments about why total capacity is more appropriate
3. **Improve log message clarity and accuracy**
- Updated message to clearly show:
* 'total disk capacity' instead of vague 'available disk'
* 'capacity/100 before rounding' to match actual calculation
* 'clamped to [min,max]' instead of 'capped to max' to show both bounds
* Includes min and max values in log for context
- More accurate and helpful for operators troubleshooting volume sizing
These changes ensure the volume size calculation is both correct and predictable.
* feat: Save mini configuration to file for persistence and documentation
This commit adds persistent configuration storage for the 'weed mini' command,
saving all non-default parameters to a JSON configuration file for:
1. **Configuration Documentation**
- All parameters actually passed on the command line are saved
- Provides a clear record of the running configuration
- Useful for auditing and understanding how the system is configured
2. **Persistence of Auto-Calculated Values**
- The auto-calculated optimal volume size (master.volumeSizeLimitMB) is saved
with a note indicating it was auto-calculated
- On restart, if the auto-calculated value exists, it won't be recalculated
- Users can delete the auto-calculated entry to force recalculation on next startup
- Provides stable, predictable configuration across restarts
3. **Configuration File Location**
- Saved to: <data-folder>/.seaweedfs/mini.config.json
- Uses the first directory from comma-separated -dir list
- Directory is created automatically if it doesn't exist
- JSON format for easy parsing and manual editing
4. **Implementation Details**
- Uses flag.Visit() to collect only explicitly passed flags
- Distinguishes between user-specified and auto-calculated values
- Includes helpful notes in the JSON file
- Graceful handling of save errors (logs warnings, doesn't fail startup)
The configuration file includes all parameters such as:
- IP and port settings (master, filer, volume, admin)
- Data directories and metadata folders
- Replication and collection settings
- S3 and IAM configurations
- Performance tuning parameters (concurrency limits, timeouts, etc.)
- Auto-calculated volume size (if applicable)
Example mini.config.json output:
{
"debug": "true",
"dir": "/data/seaweedfs",
"master.port": "9333",
"filer.port": "8888",
"volume.port": "9340",
"master.volumeSizeLimitMB.auto": "256",
"_note_auto_calculated": "This value was auto-calculated. Remove it to recalculate on next startup."
}
This allows operators to:
- Review what configuration was active
- Replicate the configuration on other systems
- Understand the startup behavior
- Control when auto-calculation occurs
* refactor: Change configuration file format to match command-line options format
Update the saved configuration format from JSON to shell-compatible options format
that matches how options are expected to be passed on the command line.
Configuration file: .seaweedfs/mini.options
Format: Each line contains a command-line option in the format -name=value
Benefits:
- Format is compatible with shell scripts and can be sourced
- Can be easily converted to command-line options
- Human-readable and editable
- Values with spaces are properly quoted
- Includes helpful comments explaining auto-calculated values
- Directly usable with weed mini command
The file can be used in multiple ways:
1. Extract options: cat .seaweedfs/mini.options | grep -v '^#' | tr '\n' ' '
2. Inline in command: weed mini \$(cat .seaweedfs/mini.options | grep -v '^#')
3. Manual review: cat .seaweedfs/mini.options
* refactor: Save mini.options directly to -dir folder
* docs: Update PR description with accurate algorithm and examples
Update the function documentation comments to accurately reflect the implemented
algorithm and provide real-world examples with actual calculated outputs.
Changes:
- Clarify that algorithm uses total disk capacity (not free space)
- Document exact calculation: capacity/100, round to power of 2, clamp to [64,1024]
- Add realistic examples showing input disk sizes and resulting volume sizes:
* 10GB disk → 64MB (minimum)
* 100GB disk → 64MB (minimum)
* 1TB disk → 64MB (minimum)
* 6.4TB disk → 64MB
* 12.8TB disk → 128MB
* 100TB disk → 1024MB (maximum)
* 1PB disk → 1024MB (maximum)
- Include note that values are rounded to next power of 2 and capped at 1GB
This helps users understand the volume size calculation and predict what size
will be set for their specific disk configurations.
* feat: integrate configuration file loading into mini startup
- Load mini.options file at startup if it exists
- Apply loaded configuration options before normal initialization
- CLI flags override file-based configuration
- Exclude 'dir' option from being saved (environment-specific)
- Configuration file format: option=value without leading dashes
- Auto-calculated volume size persists with recalculation marker
* Add 'weed mini' command for S3 beginners and small/dev use cases
This new command simplifies starting SeaweedFS by combining all components
in one process with optimized settings for development and small deployments.
Features:
- Starts master, volume, filer, S3, WebDAV, and admin in one command
- Volume size limit: 64MB (optimized for small files)
- Volume max: 0 (auto-configured based on free disk space)
- Pre-stop seconds: 1 (faster shutdown for development)
- Master peers: none (single master mode by default)
- Includes admin UI with one worker for maintenance tasks
- Clean, user-friendly startup message with all endpoint URLs
Usage:
weed mini # Use default temp directory
weed mini -dir=/data # Custom data directory
This makes it much easier for:
- Developers getting started with SeaweedFS
- Testing and development workflows
- Learning S3 API with SeaweedFS
- Small deployments that don't need complex clustering
* Change default volume server port to 9340 to avoid popular port 8080
* Fix nil pointer dereference by initializing all required volume server fields
Added missing VolumeServerOptions field initializations:
- id, publicUrl, diskType
- maintenanceMBPerSecond, ldbTimeout
- concurrentUploadLimitMB, concurrentDownloadLimitMB
- pprof, idxFolder
- inflightUploadDataTimeout, inflightDownloadDataTimeout
- hasSlowRead, readBufferSizeMB
This resolves the panic that occurred when starting the volume server.
* Fix multiple nil pointer dereferences in mini command
Added missing field initializations for:
- Master options: raftHashicorp, raftBootstrap, telemetryUrl, telemetryEnabled
- Filer options: filerGroup, saveToFilerLimit, concurrentUploadLimitMB,
concurrentFileUploadLimit, localSocket, showUIDirectoryDelete,
downloadMaxMBps, diskType, allowedOrigins, exposeDirectoryData, tusBasePath
- Volume options: id, publicUrl, diskType, maintenanceMBPerSecond, ldbTimeout,
concurrentUploadLimitMB, concurrentDownloadLimitMB, pprof, idxFolder,
inflightUploadDataTimeout, inflightDownloadDataTimeout, hasSlowRead, readBufferSizeMB
- WebDAV options: tlsPrivateKey, tlsCertificate, filerRootPath
- Admin options: master
These initializations are required to avoid runtime panics when starting components.
* Fix remaining S3 option nil pointers in mini command
* Update mini command: 256MB volume size and add S3 access instructions for beginners
* mini: set default master.volumeSizeLimitMB to 128MB and update help/banner text
* mini: shorten S3 help text to a concise pointer to docs/Admin UI
* mini: remove duplicated component bullet list, use concise sentence
* mini: tidy help alignment and update example usage
* mini: default -dir to current directory
* mini: load initial S3 credentials from env and write IAM config
* mini: use AWS env vars for initial S3 creds; instruct to create via Admin UI if absent
* Improve startup synchronization with channel-based coordination
- Replace fragile time.Sleep delays with robust channel-based synchronization
- Implement proper service dependency ordering (Master → Volume → Filer → S3/WebDAV/Admin)
- Add sync.WaitGroup for goroutine coordination
- Add startup readiness logging for better visibility
- Implement 10-second timeout for admin server startup
- Remove arbitrary sleep delays for faster, more reliable startup
- Services now start deterministically based on dependencies, not timing
This makes the startup process more reliable and eliminates race conditions on slow systems or under load.
* Refactor service startup logic for better maintainability
Extract service startup into dedicated helper functions:
- startMiniServices(): Orchestrates all service startup with dependency coordination
- startServiceWithCoordination(): Starts services with readiness signaling
- startServiceWithoutReady(): Starts services without readiness signaling
- startS3Service(): Encapsulates S3 initialization logic
Benefits:
- Reduced code duplication in runMini()
- Clearer separation of concerns
- Easier to add new services or modify startup sequence
- More testable code structure
- Improved readability with explicit service names and logging
* Remove unused serviceStartupInfo struct type
- Delete the serviceStartupInfo struct that was defined but never used
- Improves code clarity by removing dead code
- All service startup is now handled directly by helper functions
* Preserve existing IAM config file instead of truncating
- Use os.Stat to check if IAM config file already exists
- Only create and write configuration if file doesn't exist
- Log appropriate messages for each case:
* File exists: skip writing, preserve existing config
* File absent: create with os.OpenFile and write new config
* Stat error: log error without overwriting
- Set *miniIamConfig only when new file is successfully created
- Use os.O_CREATE|os.O_WRONLY flags for safe file creation
- Handles file operations with proper error checking and cleanup
* Fix CodeQL security issue: prevent logging of sensitive S3 credentials
- Add createdInitialIAM flag to track when initial IAM config is created from env vars
- Set flag in startS3Service() when new IAM config is successfully written
- Update welcome message to inform user of credential creation without exposing secrets
- Print only the username (mini) and config file location to user
- Never print access keys or secret keys in clear text
- Maintain security while keeping user informed of what was created
- Addresses CodeQL finding: Clear-text logging of sensitive information
* Fix three code review issues in weed mini command
1. Fix deadlock in service startup coordination:
- Run blocking service functions (startMaster, startFiler, etc.) in separate goroutines
- This allows readyChan to be closed and prevents indefinite blocking
- Services now start concurrently instead of sequentially blocking the coordinator
2. Use shared grace.StartDebugServer for consistency:
- Replace inline debug server startup with grace.StartDebugServer
- Improves code consistency with other commands (master, filer, etc.)
- Removes net/http import which is no longer needed
3. Simplify IAM config file cleanup with defer:
- Use 'defer f.Close()' instead of multiple f.Close() calls
- Ensures file is closed regardless of which code path is taken
- Improves robustness and code clarity
* fmt
* Fix: Remove misleading 'service is ready' logs
The previous fix removed 'go' from service function calls but left misleading
'service is ready' log messages. The service helpers now correctly:
- Call fn() directly (blocking) instead of 'go fn()' (non-blocking)
- Remove the 'service is ready' message that was printed before the service
actually started running
- Services run as blocking goroutines within the coordinator goroutine,
which keeps them alive while the program runs
- The readiness channels still work correctly because they're closed when
the coordinator finishes waiting for dependencies
* Update mini.go
* Fix four code review issues in weed mini command
1. Use restrictive file permissions (0600) for IAM config:
- Changed from 0644 to 0600 when creating iam_config.json
- Prevents world-readable access to sensitive AWS credentials
- Protects AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY
2. Remove unused sync.WaitGroup:
- Removed WaitGroup that was never waited on
- All services run as blocking goroutines in the coordinator
- Main goroutine blocks indefinitely with select{}
- Removes unnecessary complexity without changing behavior
3. Merge service startup helper functions:
- Combined startServiceWithCoordination and startServiceWithoutReady
- Made readyChan optional (nil for services without readiness signaling)
- Reduces code duplication and improves maintainability
- Both services now use single startServiceWithCoordination function
4. Fix admin server readiness check:
- Removed misleading timeout channel that never closed at startup
- Replaced with simple 2-second sleep before worker startup
- startAdminServer() blocks indefinitely, so channel would only close on shutdown
- Explicit sleep is clearer about the startup coordination intent
* Fix three code quality issues in weed mini command
1. Define volume configuration as named constants:
- Added miniVolumeMaxDataVolumeCounts = "0"
- Added miniVolumeMinFreeSpace = "1"
- Added miniVolumeMinFreeSpacePercent = "1"
- Removed local variable assignments in Volume startup
- Improves maintainability and documents configuration intent
2. Fix deadlock in startServiceWithCoordination:
- Changed from 'defer close(readyChan)' with blocking fn() to running fn() in goroutine
- Close readyChan immediately after launching service goroutine
- Prevents deadlock where fn() never returns, blocking defer execution
- Allows dependent services to start without waiting for blocking call
3. Improve admin server readiness check:
- Replaced fixed 2-second sleep with polling the gRPC port
- Polls up to 20 times (10 seconds total) with 500ms intervals
- Uses net.DialTimeout to check if port is available
- Properly handles IPv6 addresses using net.JoinHostPort
- Logs progress and warnings about connection status
- More robust than sleep against server startup timing variations
4. Add net import for network operations (IPv6 support)
Also fixed IAM config file close error handling to properly check error
from f.Close() and log any failures, preventing silent data loss on NFS.
* Document environment variable setup for S3 credentials
Updated welcome message to explain two ways to create S3 credentials:
1. Environment variables (recommended for quick setup):
- Set AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY
- Run 'weed mini -dir=/data'
- Creates initial 'mini' user credentials automatically
2. Admin UI (for managing multiple users and policies):
- Open http://localhost:23646 (Admin UI)
- Add identities to create new S3 credentials
This gives users clear guidance on the easiest way to get started with S3
credentials while also explaining the more advanced option for multiple users.
* Print welcome message after all services are running
Moved the welcome message printing from immediately after startMiniServices()
to after all services have been started and are ready. This ensures users see
the welcome message only after startup is complete, not mixed with startup logs.
Changes:
- Extract welcome message logic into printWelcomeMessage() function
- Call printWelcomeMessage() after startMiniServices() completes
- Change message from 'are starting' to 'are running and ready to use'
- This provides cleaner startup output without interleaved logs
* Wait for all services to complete before printing welcome message
The welcome message should only appear after all services are fully running and
the worker is connected. This prevents the message from appearing too early before
startup logs complete.
Changes:
- Pass allServicesReady channel through startMiniServices()
- Add adminReadyChan to track when admin/worker startup completes
- Signal allServicesReady when admin service is fully ready
- Wait for allServicesReady in runMini() before printing welcome message
- This ensures clean output: startup logs first, then welcome message once ready
Now the user sees all startup activity, then a clear welcome message when
everything is truly ready to use.
* Fix welcome message timing: print after worker is fully started
The welcome message was printing too early because allServicesReady was being
closed when the Admin service goroutine started, not when it actually completed.
The Admin service launches startMiniAdminWithWorker() which is a blocking call
that doesn't return until the worker is fully connected.
Now allServicesReady is passed through to startMiniWorker() which closes it
after the worker successfully starts and connects to the admin server.
This ensures the welcome message only appears after:
- Master is ready
- Volume server is ready
- Filer is ready
- S3 service is ready
- WebDAV service is ready
- Admin server is ready
- Worker is connected and running
All startup logs appear first, then the clean welcome message at the end.
* Wait for S3 and WebDAV services to be ready before showing welcome message
The welcome message was printing before S3 and WebDAV servers had fully
initialized. Now the readiness flow is:
1. Master → ready
2. Volume → ready
3. Filer → ready
4. S3 → ready (signals s3ReadyChan)
5. WebDAV → ready (signals webdavReadyChan)
6. Admin/Worker → starts, then waits for both S3 and WebDAV
7. Welcome message prints (all services truly ready)
Changes:
- Add s3ReadyChan and webdavReadyChan to service startup
- Pass S3 and WebDAV ready channels through to Admin service
- Admin/Worker waits for both S3 and WebDAV before closing allServicesReady
- This ensures welcome message appears only when all services are operational
* Admin service should wait for Filer, S3, and WebDAV to be ready
Admin service depends on Filer being operational since it uses the filer
for credential storage. It also makes sense to wait for S3 and WebDAV
since they are user-facing services that should be ready before Admin.
Updated dependencies:
- Admin now waits for: Master, Filer, S3, WebDAV
- This ensures all critical services are operational before Admin starts
- Welcome message will print only after all services including Admin are ready
* Add initialization delay for S3 and WebDAV services
S3 and WebDAV servers need extra time to fully initialize and start listening
after their service functions are launched. Added a 1-second delay after
launching S3 and WebDAV goroutines before signaling readiness.
This ensures the welcome message doesn't print until both services have
emitted their startup logs and are actually serving requests.
* Increase service initialization wait times for more reliable startup
- Increase S3 and WebDAV initialization delay from 1s to 2s to ensure they emit startup logs before welcome message
- Add 1s initialization delay for Filer to ensure it's listening
- Increase admin gRPC polling timeout from 10s to 20s to ensure admin server is fully ready
- This ensures welcome message prints only after all services are fully initialized and ready to accept requests
* Increase service wait time to 10 seconds for reliable startup
All services now wait 10 seconds after launching to ensure they are fully initialized and ready before signaling readiness to dependent services. This ensures the welcome message prints only after all services have fully started.
* Replace fixed 10s delay with intelligent port polling for service readiness
Instead of waiting a fixed 10 seconds for each service, now polls the service
port to check if it's actually accepting connections. This eliminates unnecessary
waiting and allows services to signal readiness as soon as they're ready.
- Polls each service port with up to 30 attempts (6 seconds total)
- Each attempt waits 200ms before retrying
- Stops polling immediately once service is ready
- Falls back gracefully if service is unknown
- Significantly faster startup sequence while maintaining reliability
* Replace channel-based coordination with HTTP pinging for service readiness
Instead of using channels to coordinate service startup, now uses HTTP GET requests
to ping each service endpoint to check if it's ready to accept connections.
Key changes:
- Removed all readiness channels (masterReadyChan, volumeReadyChan, etc.)
- Simplified startMiniServices to use sequential HTTP polling for each service
- startMiniService now just starts the service with logging
- waitForServiceReady uses HTTP client to ping service endpoints (max 6 seconds)
- waitForAdminServerReady uses HTTP GET to check admin server availability
- startMiniAdminWithWorker and startMiniWorker simplified without channel parameters
Benefits:
- Cleaner, more straightforward code
- HTTP pinging is more reliable than TCP port probing
- Services signal readiness through their HTTP endpoints
- Eliminates channel synchronization complexity
* log level
* Remove overly specific comment from volume size limit in welcome message
The '(good for small files)' comment is too limiting. The 128MB volume size
limit works well for general use cases, not just small files. Simplified the
message to just show the value.
* Ensure allServicesReady channel is always closed via defer
Add 'defer close(allServicesReady)' at the start of startMiniAdminWithWorker
to guarantee the channel is closed on ALL exit paths (normal and error).
This prevents the caller waiting on <-allServicesReady from ever hanging,
while removing the explicit close() at the successful end prevents panic
from double-close.
This makes the code more robust by:
- Guaranteeing channel closure even if worker setup fails
- Eliminating the possibility of caller hanging on errors
- Following Go defer patterns for resource cleanup
* Enhance health check polling for more robust service coordination
The service startup already uses HTTP health checks via waitForServiceReady()
to verify services are actually accepting connections. This commit improves
the health check implementation:
Changes:
- Elevated success logging to Info level so users see when services become ready
- Improved error messages to clarify that health check timeouts are not fatal
- Services continue startup even if health checks timeout (they may still work)
- Consistent handling of health check results across all services
This provides better visibility into service startup while maintaining the
existing robust coordination via HTTP pinging rather than just TCP port checks.
* Implement stricter error handling for robust mini server startup
Apply all PR review feedback to ensure the mini server fails fast and clearly
when critical components cannot start:
Changes:
1. Remove redundant miniVolumeMinFreeSpacePercent constant
- Simplified util.MustParseMinFreeSpace() call to use single parameter
2. Make service readiness checks fatal errors:
- Master, Volume, Filer, S3, WebDAV health check failures now return errors
- Prevents partially-functional servers from running
- Caller can handle errors gracefully instead of continuing with broken state
3. Make admin server readiness fatal:
- Admin gRPC availability is critical for worker startup
- Use glog.Fatalf to terminate with clear error message
4. Improve IAM config error handling:
- Treat all file operation failures (stat, open, write, close) as fatal
- Prevents silent failures in S3 credential setup
- User gets immediate feedback instead of authentication issues later
5. Use glog.Fatalf for critical worker setup errors:
- Failed to create worker directory, task directories, or worker instance
- Failed to create admin client or start worker
- Ensures mini server doesn't run in broken state
This ensures deterministic startup: services succeed completely or fail with
clear, actionable error messages for the user.
* Make health checks non-fatal for graceful degradation and improve IAM file handling
Address PR feedback to make the mini command more resilient for development:
1. Make health check failures non-fatal
- Master, Volume, Filer, S3, WebDAV health checks now log warnings but allow startup
- Services may still work even if health check endpoints aren't immediately available
- Aligns with intent of a dev-focused tool that should be forgiving of timing issues
- Only prevents startup if startup coordination or critical errors occur
2. Improve IAM config file handling
- Refactored to guarantee file is always closed using separate error variables
- Opens file once and handles write/close errors independently
- Maintains strict error handling while improving code clarity
- All file operation failures still cause fatal errors (as intended)
This makes startup more graceful while maintaining critical error handling for
fundamental failures like missing directories or configuration errors.
* Fix code quality issues in weed mini command
- Fix pointer aliasing: use value copy (*miniBindIp = *miniIp) instead of pointer assignment
- Remove misleading error return from waitForServiceReady() function
- Simplify health check callers to call waitForServiceReady() directly without error handling
- Remove redundant S3 option assignments already set in init() block
- Remove unused allServicesReady parameter from startMiniWorker() function
* Refactor welcome message to use template strings and add startup delay
- Convert welcome message to constant template strings for cleaner code
- Separate credentials instructions into dedicated constant
- Add 500ms delay after worker startup to allow full initialization before welcome message
- Improves output cleanliness by avoiding log interleaving with welcome message
* Fix code style issues in weed mini command
- Fix indentation in IAM config block (lines 424-432) to align with surrounding code
- Remove unused adminServerDone channel that was created but never read
* Address code review feedback for robustness and resource management
- Use defer f.Close() for IAM file handling to ensure file is closed in all code paths, preventing potential file descriptor leaks
- Use 127.0.0.1 instead of *miniIp for service readiness checks to ensure checks always target localhost, improving reliability in environments with firewalls or complex network configurations
- Simplify error handling in waitForAdminServerReady by using single error return instead of separate write/close error variables
* Fix function declaration formatting
- Separate closing brace of startS3Service from startMiniAdminWithWorker declaration with blank line
- Move comment to proper position above function declaration
- Run gofmt for consistent formatting
* Fix IAM config pointer assignment when file already exists
- Add missing *miniIamConfig = iamPath assignment when IAM config file already exists
- Ensures S3 service is properly pointed to the existing IAM configuration
- Retains logging to inform user that existing configuration is being preserved
* Improve pointer assignments and worker synchronization
- Simplify grpcPort and dataDir pointer assignments by directly dereferencing and assigning values instead of taking address of local variables
- Replace time.Sleep(500ms) with proper TCP-based polling to wait for worker gRPC port readiness
- Add waitForWorkerReady function that polls worker's gRPC port with max 6-second timeout
- Add net package import for TCP connection checks
- Improves code idiomaticity and synchronization robustness
* Refactor and simplify error handling for maintainability
- Remove unused error return from startMiniServices (always returned nil)
- Update runMini caller to not expect error from startMiniServices
- Refactor init() into component-specific helper functions:
* initMiniCommonFlags() for common options
* initMiniMasterFlags() for master server options
* initMiniFilerFlags() for filer server options
* initMiniVolumeFlags() for volume server options
* initMiniS3Flags() for S3 server options
* initMiniWebDAVFlags() for WebDAV server options
* initMiniAdminFlags() for admin server options
- Significantly improves code readability and maintainability
- Each component's flags are now in dedicated, focused functions
* Make lock_manager.RenewInterval configurable in LiveLock
- Add renewInterval field to LiveLock struct
- Modify StartLongLivedLock to accept renewInterval parameter
- Update all call sites to pass lock_manager.RenewInterval
- Default to lock_manager.RenewInterval if zero is passed
* S3 metrics: reduce collection interval to half of bucketSizeMetricsInterval
Since S3 metrics collection is not critical, check more frequently but
only collect when holding the distributed lock. This allows faster
detection of any issues while avoiding overhead on non-leader instances.
* Remove unused lock_manager import from bucket_size_metrics.go
* Refactor: Make lockTTL the primary parameter, derive renewInterval from it
Instead of configurable renew interval, lockTTL is now the input parameter.
The renewal interval is automatically derived as lockTTL / 2, ensuring that
locks are renewed well before expiration.
Changes:
- Replace renewInterval parameter with lockTTL
- Rename LiveLock.renewInterval field to lockTTL
- Calculate renewInterval as lockTTL / 2 inside the goroutine
- Update all call sites to pass lockTTL values
- Simplify sleep logic to use consistent renewInterval for both states
This approach is more intuitive and guarantees safe renewal windows.
* When locked, renew more aggressively to actively keep the lock
When holding the lock, sleep for renewInterval/2 to renew more frequently.
When seeking the lock, sleep for renewInterval to retry with normal frequency.
This ensures we actively maintain lock ownership while being less aggressive
when competing for the lock.
* Simplify: use consistent renewInterval for all lock states
Since renewInterval is already lockTTL / 2, there's no need to differentiate
between locked and unlocked states. Both use the same interval for consistency.
* Adjust sleep intervals for different lock states
- Locked instances sleep for renewInterval (lockTTL/2) to renew the lock
- Unlocked instances sleep for 5*renewInterval (2.5*lockTTL) to retry acquisition less frequently
* s3api: fix bucket-root listing w/ delimiter
* test: improve mock robustness for bucket-root listing test
- Make testListEntriesStream implement interface explicitly without embedding
- Add prefix filtering logic to testFilerClient to simulate real filer behavior
- Special-case prefix='/' to not filter for bucket root compatibility
- Add required imports for metadata and strings packages
This addresses review comments about test mock brittleness and accuracy.
* test: add clarifying comment for mock filtering behavior
Add detailed comment explaining which ListEntriesRequest parameters
are implemented (Prefix) vs ignored (Limit, StartFromFileName, etc.)
in the test mock to improve code documentation and future maintenance.
* logging
* less logs
* less check if already locked
2025-12-20 00:59:10 -08:00
607 changed files with 23573 additions and 6973 deletions
* [Quick Start with weed mini](#quick-start-with-weed-mini)
* [Quick Start for S3 API on Docker](#quick-start-for-s3-api-on-docker)
* [Quick Start with Single Binary](#quick-start-with-single-binary)
* [Quick Start SeaweedFS S3 on AWS](#quick-start-seaweedfs-s3-on-aws)
* [Introduction](#introduction)
* [Features](#features)
* [Additional Features](#additional-features)
@@ -78,6 +78,32 @@ Table of Contents
# Quick Start #
## Quick Start with weed mini ##
The easiest way to get started with SeaweedFS for development and testing:
* Download the latest binary from https://github.com/seaweedfs/seaweedfs/releases and unzip a single binary file `weed` or `weed.exe`.
Example:
```bash
# remove quarantine on macOS
# xattr -d com.apple.quarantine ./weed
exportAWS_ACCESS_KEY_ID=admin
exportAWS_SECRET_ACCESS_KEY=key
./weed mini -dir=/data
```
This single command starts a complete SeaweedFS setup with:
- **Master UI**: http://localhost:9333
- **Volume Server**: http://localhost:9340
- **Filer UI**: http://localhost:8888
- **S3 Endpoint**: http://localhost:8333
- **WebDAV**: http://localhost:7333
- **Admin UI**: http://localhost:23646
Perfect for development, testing, learning SeaweedFS, and single node deployments!
## Quick Start for S3 API on Docker ##
`docker run -p 8333:8333 chrislusf/seaweedfs server -s3`
@@ -85,13 +111,10 @@ Table of Contents
## Quick Start with Single Binary ##
* Download the latest binary from https://github.com/seaweedfs/seaweedfs/releases and unzip a single binary file `weed` or `weed.exe`. Or run `go install github.com/seaweedfs/seaweedfs/weed@latest`.
*`export AWS_ACCESS_KEY_ID=admin ; export AWS_SECRET_ACCESS_KEY=key` as the admin credentials to access the object store.
* Run `weed server -dir=/some/data/dir -s3` to start one master, one volume server, one filer, and one S3 gateway.
* Run `weed server -dir=/some/data/dir -s3` to start one master, one volume server, one filer, and one S3 gateway. The difference with `weed mini` is that `weed mini` can auto configure based on the single host environment, while `weed server` requires manual configuration and are designed for production use.
Also, to increase capacity, just add more volume servers by running `weed volume -dir="/some/data/dir2" -master="<master_host>:9333" -port=8081` locally, or on a different machine, or on thousands of machines. That is it!
## Quick Start SeaweedFS S3 on AWS ##
* Setup fast production-ready [SeaweedFS S3 on AWS with cloudformation](https://aws.amazon.com/marketplace/pp/prodview-nzelz5gprlrjc)
# Introduction #
SeaweedFS is a simple and highly scalable distributed file system. There are two objectives:
# Server-Side Encryption with Customer-Provided Keys (SSE-C) Implementation
This document describes the implementation of SSE-C support in SeaweedFS, addressing the feature request from [GitHub Discussion #5361](https://github.com/seaweedfs/seaweedfs/discussions/5361).
## Overview
SSE-C allows clients to provide their own encryption keys for server-side encryption of objects stored in SeaweedFS. The server encrypts the data using the customer-provided AES-256 key but does not store the key itself - only an MD5 hash of the key for validation purposes.
## Implementation Details
### Architecture
The SSE-C implementation follows a transparent encryption/decryption pattern:
1.**Upload (PUT/POST)**: Data is encrypted with the customer key before being stored
2.**Download (GET/HEAD)**: Encrypted data is decrypted on-the-fly using the customer key
3.**Metadata Storage**: Only the encryption algorithm and key MD5 are stored as metadata
### Key Components
#### 1. Constants and Headers (`weed/s3api/s3_constants/header.go`)
- Added AWS-compatible SSE-C header constants
- Support for both regular and copy-source SSE-C headers
This implementation follows the [AWS S3 SSE-C specification](https://docs.aws.amazon.com/AmazonS3/latest/userguide/ServerSideEncryptionCustomerKeys.html) for maximum compatibility with existing S3 clients and tools.
## Performance Impact
- **Encryption Overhead**: Minimal CPU impact with efficient AES-CTR streaming
- **Memory Usage**: Constant memory usage via streaming encryption/decryption
- **Storage Overhead**: 16 bytes per object for IV storage
$(MAKE)test||(echo"❌ Tests failed, showing server logs:"&&echo"=== Last 50 lines of server logs ==="&& tail -50 weed-test.log &&echo"=== End of server logs ==="&&exit 1)
$(MAKE)test||(echo"❌ Tests failed, showing server logs:"&&echo"=== Last 50 lines of server logs ==="&& tail -50 weed-server.log &&echo"=== End of server logs ==="&&exit 1)
This document summarizes the cross-filesystem compatibility testing between **s3fs** and **PyArrow native S3 filesystem** implementations when working with SeaweedFS.
## Test Purpose
Verify that Parquet files written using one filesystem implementation (s3fs or PyArrow native S3) can be correctly read using the other implementation, confirming true file format compatibility.
## Test Methodology
### Test Matrix
The test performs the following combinations:
1. **Write with s3fs → Read with PyArrow native S3**
2. **Write with PyArrow native S3 → Read with s3fs**
For each direction, the test:
- Creates a sample PyArrow table with multiple data types (int64, string, float64, bool)
- Writes the Parquet file using one filesystem implementation
- Reads the Parquet file using the other filesystem implementation
- Verifies data integrity by comparing:
- Row counts
- Schema equality
- Data contents (after sorting by ID to handle row order differences)
Large file testing requires adequate volume capacity in SeaweedFS. When run with default volume settings (50MB max size), tests may encounter capacity issues with the number of large test files created simultaneously.
**Recommendation**: For large file testing, increase `VOLUME_MAX_SIZE_MB` in the Makefile or run tests with `TEST_QUICK=1` for development/validation purposes.
## Key Findings
### ✅ Full Compatibility Confirmed
**Files written with s3fs and PyArrow native S3 filesystem are fully compatible and can be read by either implementation.**
This confirms that:
1. **Identical Parquet Format**: Both s3fs and PyArrow native S3 use the same underlying PyArrow library to generate Parquet files, resulting in identical file formats at the binary level.
2. **S3 API Compatibility**: SeaweedFS's S3 implementation handles both filesystem backends correctly, with proper:
- Object creation (PutObject)
- Object reading (GetObject)
- Directory handling (implicit directories)
- Multipart uploads (for larger files)
3. **Metadata Consistency**: File metadata, schemas, and data integrity are preserved across both write and read operations regardless of which filesystem implementation is used.
## Implementation Details
### Common Write Path
Both implementations use PyArrow's `pads.write_dataset()` function:
- `BUCKET_NAME`: Bucket name (default: `test-parquet-bucket`)
- `TEST_QUICK`: Run only small tests (default: `0`, set to `1` for quick mode)
## Conclusion
The cross-filesystem compatibility tests demonstrate that **Parquet files written via s3fs and PyArrow native S3 filesystem are completely interchangeable**. This validates that:
1. The Parquet file format is implementation-agnostic
2. SeaweedFS's S3 API correctly handles both filesystem backends
3. Users have full flexibility in choosing their preferred filesystem implementation
This compatibility is a testament to:
- PyArrow's consistent file format generation
- SeaweedFS's robust S3 API implementation
- Proper handling of S3 semantics (especially implicit directories)
This document provides a deep technical analysis of the s3fs compatibility issue with PyArrow Parquet datasets on SeaweedFS, and the solution implemented to resolve it.
## Root Cause
When PyArrow writes datasets using `write_dataset()`, it creates implicit directory structures by writing files without explicit directory markers. However, some S3 workflows may create 0-byte directory markers.
### The Problem
1. **PyArrow writes dataset files** without creating explicit directory objects
2. **s3fs calls HEAD** on the directory path to check if it exists
3. **If HEAD returns 200** with `Content-Length: 0`, s3fs interprets it as a file (not a directory)
4. **PyArrow fails** when trying to read, reporting "Parquet file size is 0 bytes"
### AWS S3 Behavior
AWS S3 returns **404 Not Found** for implicit directories (directories that only exist because they have children but no explicit marker object). This allows s3fs to fall back to LIST operations to detect the directory.
## The Solution
### Implementation
Modified the S3 API HEAD handler in `weed/s3api/s3api_object_handlers.go` to:
1. **Check if object ends with `/`**: Explicit directory markers return 200 as before
2. **Check if object has children**: If a 0-byte object has children in the filer, treat it as an implicit directory
3. **Return 404 for implicit directories**: This matches AWS S3 behavior and triggers s3fs's LIST fallback
### Code Changes
The fix is implemented in the `HeadObjectHandler` function with logic to:
- Detect implicit directories by checking for child entries
- Return 404 (NoSuchKey) for implicit directories
- Preserve existing behavior for explicit directory markers and regular files
## Performance Considerations
### Optimization: Child Check Cache
- Child existence checks are performed via filer LIST operations
- Results could be cached for frequently accessed paths
- Trade-off between consistency and performance
### Impact
- Minimal performance impact for normal file operations
- Slight overhead for HEAD requests on implicit directories (one additional LIST call)
- Overall improvement in PyArrow compatibility outweighs minor performance cost
This document compares how MinIO handles directory markers versus SeaweedFS's implementation, and explains the different approaches to S3 directory semantics.
## MinIO's Approach
MinIO handles implicit directories similarly to AWS S3:
1. **No explicit directory objects**: Directories are implicit, defined only by object key prefixes
2. **HEAD on directory returns 404**: Consistent with AWS S3 behavior
3. **LIST operations reveal directories**: Directories are discovered through delimiter-based LIST operations
4. **Automatic prefix handling**: MinIO automatically recognizes prefixes as directories
### MinIO Implementation Details
- Uses in-memory metadata for fast prefix lookups
- Optimized for LIST operations with common delimiter (`/`)
- No persistent directory objects in storage layer
- Directories "exist" as long as they contain objects
## SeaweedFS Approach
SeaweedFS uses a filer-based approach with real directory entries:
### Before the Fix
1. **Explicit directory objects**: Could create 0-byte objects as directory markers
2. **HEAD returns 200**: Even for implicit directories
3. **Caused s3fs issues**: s3fs interpreted 0-byte HEAD responses as empty files
### After the Fix
1. **Hybrid approach**: Supports both explicit markers (with `/` suffix) and implicit directories
2. **HEAD returns 404 for implicit directories**: Matches AWS S3 and MinIO behavior
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.