Tree:
fe6f8d737d
add-ec-vacuum
add-filer-iam-grpc
add-iam-grpc-management
add_fasthttp_client
add_remote_storage
adding-message-queue-integration-tests
adjust-fsck-cutoff-default
also-delete-parent-directory-if-empty
avoid_releasing_temp_file_on_write
changing-to-zap
collect-public-metrics
copilot/fix-helm-chart-installation
copilot/fix-s3-object-tagging-issue
copilot/make-renew-interval-configurable
copilot/make-renew-interval-configurable-again
copilot/sub-pr-7677
create-table-snapshot-api-design
data_query_pushdown
dependabot/maven/other/java/client/com.google.protobuf-protobuf-java-3.25.5
dependabot/maven/other/java/examples/org.apache.hadoop-hadoop-common-3.4.0
detect-and-plan-ec-tasks
do-not-retry-if-error-is-NotFound
ec-disk-type-support
enhance-erasure-coding
fasthttp
feature/iceberg-rest-catalog
feature/mini-port-detection
feature/modernize-s3-tests
feature/s3-multi-cert-support
filer1_maintenance_branch
fix-GetObjectLockConfigurationHandler
fix-bucket-name-case-7910
fix-helm-fromtoml-compatibility
fix-mount-http-parallelism
fix-mount-read-throughput-7504
fix-pr-7909
fix-s3-configure-consistency
fix-s3-object-tagging-issue-7589
fix-sts-session-token-7941
fix-versioning-listing-only
fix/windows-test-file-cleanup
ftp
gh-pages
iam-multi-file-migration
iam-permissions-and-api
improve-fuse-mount
improve-fuse-mount2
logrus
master
message_send
mount2
mq-subscribe
mq2
nfs-cookie-prefix-list-fixes
optimize-delete-lookups
original_weed_mount
pr-7412
pr/7984
pr/8140
raft-dual-write
random_access_file
refactor-needle-read-operations
refactor-volume-write
remote_overlay
remove-implicit-directory-handling
revert-5134-patch-1
revert-5819-patch-1
revert-6434-bugfix-missing-s3-audit
s3-remote-cache-singleflight
s3-select
s3tables-by-claude
sub
tcp_read
test-reverting-lock-table
test_udp
testing
testing-sdx-generation
tikv
track-mount-e2e
upgrade-versions-to-4.00
volume_buffered_writes
worker-execute-ec-tasks
0.72
0.72.release
0.73
0.74
0.75
0.76
0.77
0.90
0.91
0.92
0.93
0.94
0.95
0.96
0.97
0.98
0.99
1.00
1.01
1.02
1.03
1.04
1.05
1.06
1.07
1.08
1.09
1.10
1.11
1.12
1.14
1.15
1.16
1.17
1.18
1.19
1.20
1.21
1.22
1.23
1.24
1.25
1.26
1.27
1.28
1.29
1.30
1.31
1.32
1.33
1.34
1.35
1.36
1.37
1.38
1.40
1.41
1.42
1.43
1.44
1.45
1.46
1.47
1.48
1.49
1.50
1.51
1.52
1.53
1.54
1.55
1.56
1.57
1.58
1.59
1.60
1.61
1.61RC
1.62
1.63
1.64
1.65
1.66
1.67
1.68
1.69
1.70
1.71
1.72
1.73
1.74
1.75
1.76
1.77
1.78
1.79
1.80
1.81
1.82
1.83
1.84
1.85
1.86
1.87
1.88
1.90
1.91
1.92
1.93
1.94
1.95
1.96
1.97
1.98
1.99
1;70
2.00
2.01
2.02
2.03
2.04
2.05
2.06
2.07
2.08
2.09
2.10
2.11
2.12
2.13
2.14
2.15
2.16
2.17
2.18
2.19
2.20
2.21
2.22
2.23
2.24
2.25
2.26
2.27
2.28
2.29
2.30
2.31
2.32
2.33
2.34
2.35
2.36
2.37
2.38
2.39
2.40
2.41
2.42
2.43
2.47
2.48
2.49
2.50
2.51
2.52
2.53
2.54
2.55
2.56
2.57
2.58
2.59
2.60
2.61
2.62
2.63
2.64
2.65
2.66
2.67
2.68
2.69
2.70
2.71
2.72
2.73
2.74
2.75
2.76
2.77
2.78
2.79
2.80
2.81
2.82
2.83
2.84
2.85
2.86
2.87
2.88
2.89
2.90
2.91
2.92
2.93
2.94
2.95
2.96
2.97
2.98
2.99
3.00
3.01
3.02
3.03
3.04
3.05
3.06
3.07
3.08
3.09
3.10
3.11
3.12
3.13
3.14
3.15
3.16
3.18
3.19
3.20
3.21
3.22
3.23
3.24
3.25
3.26
3.27
3.28
3.29
3.30
3.31
3.32
3.33
3.34
3.35
3.36
3.37
3.38
3.39
3.40
3.41
3.42
3.43
3.44
3.45
3.46
3.47
3.48
3.50
3.51
3.52
3.53
3.54
3.55
3.56
3.57
3.58
3.59
3.60
3.61
3.62
3.63
3.64
3.65
3.66
3.67
3.68
3.69
3.71
3.72
3.73
3.74
3.75
3.76
3.77
3.78
3.79
3.80
3.81
3.82
3.83
3.84
3.85
3.86
3.87
3.88
3.89
3.90
3.91
3.92
3.93
3.94
3.95
3.96
3.97
3.98
3.99
4.00
4.01
4.02
4.03
4.04
4.05
4.06
4.07
dev
helm-3.65.1
v0.69
v0.70beta
v3.33
${ noResults }
166 Commits (fe6f8d737def32955aafc96a9627ff7089e85367)
| Author | SHA1 | Message | Date |
|---|---|---|---|
|
|
c106532b79 |
fix: prevent MiniClusterCtx race conditions in command shutdown
Capture global MiniClusterCtx into local variables before goroutine/select evaluation to prevent nil dereference/data race when context is reset to nil after nil check. Applied to filer, master, volume, and s3 commands. |
5 days ago |
|
|
2f155ee5ee
|
feat: Add S3 Tables support for Iceberg tabular data (#8147)
* s3tables: extract utility and filer operations to separate modules
- Move ARN parsing, path helpers, and metadata structures to utils.go
- Extract all extended attribute and filer operations to filer_ops.go
- Reduces code duplication and improves modularity
- Improves code organization and maintainability
* s3tables: split table bucket operations into focused modules
- Create bucket_create.go for CreateTableBucket operation
- Create bucket_get_list_delete.go for Get, List, Delete operations
- Related operations grouped for better maintainability
- Each file has a single, clear responsibility
- Improves code clarity and makes it easier to test
* s3tables: simplify handler by removing duplicate utilities
- Reduce handler.go from 370 to 195 lines (47% reduction)
- Remove duplicate ARN parsing and path helper functions
- Remove filer operation methods moved to filer_ops.go
- Remove metadata structure definitions moved to utils.go
- Keep handler focused on request routing and response formatting
- Maintains all functionality with improved code organization
* s3tables: complete s3tables package implementation
- namespace.go: namespace CRUD operations (310 lines)
- table.go: table CRUD operations with Iceberg schema support (409 lines)
- policy.go: resource policies and tagging operations (419 lines)
- types.go: request/response types and error definitions (290 lines)
- All handlers updated to use standalone utilities from utils.go
- All files follow single responsibility principle
* s3api: add S3 Tables integration layer
- Create s3api_tables.go to integrate S3 Tables with S3 API server
- Implement S3 Tables route matcher for X-Amz-Target header
- Register S3 Tables routes with API router
- Provide gRPC filer client interface for S3 Tables handlers
- All S3 Tables operations accessible via S3 API endpoint
* s3api: register S3 Tables routes in API server
- Add S3 Tables route registration in s3api_server.go registerRouter method
- Enable S3 Tables API operations to be routed through S3 API server
- Routes handled by s3api_tables.go integration layer
- Minimal changes to existing S3 API structure
* test: add S3 Tables test infrastructure
- Create setup.go with TestCluster and S3TablesClient definitions
- Create client.go with HTTP client methods for all operations
- Test utilities and client methods organized for reusability
- Foundation for S3 Tables integration tests
* test: add S3 Tables integration tests
- Comprehensive integration tests for all 23 S3 Tables operations
- Test cluster setup based on existing S3 integration tests
- Tests cover:
* Table bucket lifecycle (create, get, list, delete)
* Namespace operations
* Table CRUD with Iceberg schema
* Table bucket and table policies
* Resource tagging operations
- Ready for CI/CD pipeline integration
* ci: add S3 Tables integration tests to GitHub Actions
- Create new workflow for S3 Tables integration testing
- Add build verification job for s3tables package and s3api integration
- Add format checking for S3 Tables code
- Add go vet checks for code quality
- Workflow runs on all pull requests
- Includes test output logging and artifact upload on failure
* s3tables: add handler_ prefix to operation handler files
- Rename bucket_create.go → handler_bucket_create.go
- Rename bucket_get_list_delete.go → handler_bucket_get_list_delete.go
- Rename namespace.go → handler_namespace.go
- Rename table.go → handler_table.go
- Rename policy.go → handler_policy.go
Improves file organization by clearly identifying handler implementations.
No code changes, refactoring only.
* s3tables test: refactor to eliminate duplicate definitions
- Move all client methods to client.go
- Remove duplicate types/constants from s3tables_integration_test.go
- Keep setup.go for test infrastructure
- Keep integration test logic in s3tables_integration_test.go
- Clean up unused imports
- Test compiles successfully
* Delete client_methods.go
* s3tables: add bucket name validation and fix error handling
- Add isValidBucketName validation function for [a-z0-9_-] characters
- Validate bucket name characters match ARN parsing regex
- Fix error handling in WithFilerClient closure - properly check for lookup errors
- Add error handling for json.Marshal calls (metadata and tags)
- Improve error messages and logging
* s3tables: add error handling for json.Marshal calls
- Add error handling in handler_namespace.go (metadata marshaling)
- Add error handling in handler_table.go (metadata and tags marshaling)
- Add error handling in handler_policy.go (tag marshaling in TagResource and UntagResource)
- Return proper errors with context instead of silently ignoring failures
* s3tables: replace custom splitPath with stdlib functions
- Remove custom splitPath implementation (23 lines)
- Use filepath.Dir and filepath.Base from stdlib
- More robust and handles edge cases correctly
- Reduces code duplication
* s3tables: improve error handling specificity in ListTableBuckets
- Specifically check for 'not found' errors instead of catching all errors
- Return empty list only when directory doesn't exist
- Propagate other errors (network, permission) with context
- Prevents masking real errors
* s3api_tables: optimize action validation with map lookup
- Replace O(n) slice iteration with O(1) map lookup
- Move s3TablesActionsMap to package level
- Avoid recreating the map on every function call
- Improves performance for request validation
* s3tables: implement permission checking and authorization
- Add permissions.go with permission definitions and checks
- Define permissions for all 21 S3 Tables operations
- Add permission checking helper functions
- Add getPrincipalFromRequest to extract caller identity
- Implement access control in CreateTableBucket, GetTableBucket, DeleteTableBucket
- Return 403 Forbidden for unauthorized operations
- Only bucket owner can perform operations (extensible for future policies)
- Add AuthError type for authorization failures
* workflow: fix s3 tables tests path and working directory
The workflow was failing because it was running inside 'weed' directory,
but the tests are at the repository root. Removed working-directory
default and updated relative paths to weed source.
* workflow: remove emojis from echo statements
* test: format s3tables client.go
* workflow: fix go install path to ./weed
* ci: fail s3 tables tests if any command in pipeline fails
* s3tables: use path.Join for path construction and align namespace paths
* s3tables: improve integration test stability and error reporting
* s3tables: propagate request context to filer operations
* s3tables: clean up unused code and improve error response formatting
* Refine S3 Tables implementation to address code review feedback
- Standardize namespace representation to []string
- Improve listing logic with pagination and StartFromFileName
- Enhance error handling with sentinel errors and robust checks
- Add JSON encoding error logging
- Fix CI workflow to use gofmt -l
- Standardize timestamps in directory creation
- Validate single-level namespaces
* s3tables: further refinements to filer operations and utilities
- Add multi-segment namespace support to ARN parsing
- Refactor permission checking to use map lookup
- Wrap lookup errors with ErrNotFound in filer operations
- Standardize splitPath to use path package
* test: improve S3 Tables client error handling and cleanup
- Add detailed error reporting when decoding failure responses
- Remove orphaned comments and unused sections
* command: implement graceful shutdown for mini cluster
- Introduce MiniClusterCtx to coordinate shutdown across mini services
- Update Master, Volume, Filer, S3, and WebDAV servers to respect context cancellation
- Ensure all resources are cleaned up properly during test teardown
- Integrate MiniClusterCtx in s3tables integration tests
* s3tables: fix pagination and enhance error handling in list/delete operations
- Fix InclusiveStartFrom logic to ensure exclusive start on continued pages
- Prevent duplicates in bucket, namespace, and table listings
- Fail fast on listing errors during bucket and namespace deletion
- Stop swallowing errors in handleListTables and return proper HTTP error responses
* s3tables: align ARN formatting and optimize resource handling
- Update generateTableARN to match AWS S3 Tables specification
- Move defer r.Body.Close() to follow standard Go patterns
- Remove unused generateNamespaceARN helper
* command: fix stale error variable logging in filer serving goroutines
- Use local 'err' variable instead of stale 'e' from outer scope
- Applied to both TLS and non-TLS paths for local listener
* s3tables: implement granular authorization and refine error responses
- Remove mandatory ACTION_ADMIN at the router level
- Enforce granular permissions in bucket and namespace handlers
- Prioritize AccountID in ExtractPrincipalFromContext for ARN matching
- Distinguish between 404 (NoSuchBucket) and 500 (InternalError) in metadata lookups
- Clean up unused imports in s3api_tables.go
* test: refactor S3 Tables client for DRYness and multi-segment namespaces
- Implement doRequestAndDecode to eliminate HTTP boilerplate
- Update client API to accept []string for namespaces to support hierarchy
- Standardize error response decoding across all client methods
* test: update integration tests to match refactored S3 Tables client
- Pass namespaces as []string to support hierarchical structures
- Adapt test calls to new client API signatures
* s3tables: normalize filer errors and use standard helpers
- Migrate from custom ErrNotFound to filer_pb.ErrNotFound
- Use filer_pb.LookupEntry for automatic error normalization
- Normalize entryExists and attribute lookups
* s3tables: harden namespace validation and correct ARN parsing
- Prohibit path traversal (".", "..") and "/" in namespaces
- Restrict namespace characters to [a-z0-9_] for consistency
- Switch to url.PathUnescape for correct decoding of ARN path components
- Align ARN parsing regex with single-segment namespace validation
* s3tables: improve robustness, security, and error propagation in handlers
- Implement strict table name validation (prevention of path traversal and character enforcement)
- Add nil checks for entry.Entry in all listing loops to prevent panics
- Propagate backend errors instead of swallowing them or assuming 404
- Correctly map filer_pb.ErrNotFound to appropriate S3 error codes
- Standardize existence checks across bucket, namespace, and table handlers
* test: add miniClusterMutex to prevent race conditions
- Introduce sync.Mutex to protect global state (os.Args, os.Chdir)
- Ensure serialized initialization of the mini cluster runner
- Fix intermittent race conditions during parallel test execution
* s3tables: improve error handling and permission logic
- Update handleGetNamespace to distinguish between 404 and 500 errors
- Refactor CanManagePolicy to use CheckPermission for consistent enforcement
- Ensure empty identities are correctly handled in policy management checks
* s3tables: optimize regex usage and improve version token uniqueness
- Pre-compile regex patterns as package-level variables to avoid re-compilation overhead on every call
- Add a random component to version token generation to reduce collision probability under high concurrency
* s3tables: harden auth and error handling
- Add authorization checks to all S3 Tables handlers (policy, table ops) to enforce security
- Improve error handling to distinguish between NotFound (404) and InternalError (500)
- Fix directory FileMode usage in filer_ops
- Improve test randomness for version tokens
- Update permissions comments to acknowledge IAM gaps
* S3 Tables: fix gRPC stream loop handling for list operations
- Correctly handle io.EOF to terminate loops gracefully.
- Propagate other errors to prevent silent failures.
- Ensure all list results are processed effectively.
* S3 Tables: validate ARN namespace to prevent path traversal
- Enforce validation on decoded namespace in parseTableFromARN.
- Ensures path components are safe after URL unescaping.
* S3 Tables: secure API router with IAM authentication
- Wrap S3 Tables handler with authenticateS3Tables.
- Use AuthSignatureOnly to enforce valid credentials while delegating granular authorization to handlers.
- Prevent anonymous access to all S3 Tables endpoints.
* S3 Tables: fix gRPC stream loop handling in namespace handlers
- Correctly handle io.EOF in handleListNamespaces and handleDeleteNamespace.
- Propagate other errors to prevent silent failures or accidental data loss.
- Added necessary io import.
* S3 Tables: use os.ModeDir constant in filer_ops.go
- Replace magic number 1<<31 with os.ModeDir for better readability.
- Added necessary os import.
* s3tables: improve principal extraction using identity context
* s3tables: remove duplicate comment in permissions.go
* s3tables test: improve error reporting on decoding failure
* s3tables: implement validateTableName helper
* s3tables: add table name validation and 404 propagation to policy handlers
* s3tables: add table name validation and cleanup duplicated logic in table handlers
* s3tables: ensure root tables directory exists before bucket creation
* s3tables: implement token-based pagination for table buckets listing
* s3tables: implement token-based pagination for namespace listing
* s3tables: refine permission helpers to align with operation names
* s3tables: return 404 in handleDeleteNamespace if namespace not found
* s3tables: fix cross-namespace pagination in listTablesInAllNamespaces
* s3tables test: expose pagination parameters in client list methods
* s3tables test: update integration tests for new client API
* s3tables: use crypto/rand for secure version token generation
Replaced math/rand with crypto/rand to ensure version tokens are
cryptographically secure and unpredictable for optimistic concurrency control.
* s3tables: improve account ID handling and define missing error codes
Updated getPrincipalFromRequest to prioritize X-Amz-Account-ID header and
added getAccountID helper. Defined ErrVersionTokenMismatch and ErrCodeConflict
for better optimistic concurrency support.
* s3tables: update bucket handlers for multi-account support
Ensured bucket ownership is correctly attributed to the authenticated
account ID and updated ARNs to use the request-derived account ID. Added
standard S3 existence checks for bucket deletion.
* s3tables: update namespace handlers for multi-account support
Updated namespace creation to use authenticated account ID for ownership
and unified permission checks across all namespace operations to use the
correct account principal.
* s3tables: implement optimistic concurrency for table deletion
Added VersionToken validation to handleDeleteTable. Refactored table
listing to use request context for accurate ARN generation and fixed
cross-namespace pagination issues.
* s3tables: improve resource resolution and error mapping for policies and tagging
Refactored resolveResourcePath to return resource type, enabling accurate
NoSuchBucket vs NoSuchTable error codes. Added existence checks before
deleting policies.
* s3tables: enhance test robustness and resilience
Updated random string generation to use crypto/rand in s3tables tests.
Increased resilience of IAM distributed tests by adding "connection refused"
to retryable errors.
* s3tables: remove legacy principal fallback header
Removed the fallback to X-Amz-Principal in getPrincipalFromRequest as
S3 Tables is a new feature and does not require legacy header support.
* s3tables: remove unused ExtractPrincipalFromContext function
Removed the unused ExtractPrincipalFromContext utility and its
accompanying iam/utils import to keep the new s3tables codebase clean.
* s3tables: allow hyphens in namespace and table names
Relaxed regex validation in utils.go to support hyphens in S3 Tables
namespaces and table names, improving consistency with S3 bucket naming
and allowing derived names from services like S3 Storage Lens.
* s3tables: add isAuthError helper to handler.go
* s3tables: refactor permission checks to use resource owner in bucket handlers
* s3tables: refactor permission checks to use resource owner in namespace handlers
* s3tables: refactor permission checks to use resource owner in table handlers
* s3tables: refactor permission checks to use resource owner in policy and tagging handlers
* ownerAccountID
* s3tables: implement strict AWS-aligned name validation for buckets, namespaces, and tables
* s3tables: enforce strict resource ownership and implement result filtering for buckets
* s3tables: enforce strict resource ownership and implement result filtering for namespaces
* s3tables: enforce strict resource ownership and implement result filtering for tables
* s3tables: align getPrincipalFromRequest with account ID for IAM compatibility
* s3tables: fix inconsistent permission check in handleCreateTableBucket
* s3tables: improve pagination robustness and error handling in table listing handlers
* s3tables: refactor handleDeleteTableBucket to use strongly typed AuthError
* s3tables: align ARN regex patterns with S3 standards and refactor to constants
* s3tables: standardize access denied errors using ErrAccessDenied constant
* go fmt
* s3tables: fix double-write issue in handleListTables
Remove premature HTTP error writes from within WithFilerClient closure
to prevent duplicate status code responses. Error handling is now
consistently performed at the top level using isAuthError.
* s3tables: update bucket name validation message
Remove "underscores" from error message to accurately reflect that
bucket names only allow lowercase letters, numbers, and hyphens.
* s3tables: add table policy test coverage
Add comprehensive test coverage for table policy operations:
- Added PutTablePolicy, GetTablePolicy, DeleteTablePolicy methods to test client
- Implemented testTablePolicy lifecycle test validating Put/Get/Delete operations
- Verified error handling for missing policies
* follow aws spec
* s3tables: add request body size limiting
Add request body size limiting (10MB) to readRequestBody method:
- Define maxRequestBodySize constant to prevent unbounded reads
- Use io.LimitReader to enforce size limit
- Add explicit error handling for oversized requests
- Prevents potential DoS attacks via large request bodies
* S3 Tables API now properly enforces resource policies
addressing the critical security gap where policies were created but never evaluated.
* s3tables: Add upper bound validation for MaxTables parameter
MaxTables is user-controlled and influences gRPC ListEntries limits via
uint32(maxTables*2). Without an upper bound, very large values can overflow
uint32 or cause excessively large directory scans. Cap MaxTables to 1000 and
return InvalidRequest for out-of-range values, consistent with S3 MaxKeys
handling.
* s3tables: Add upper bound validation for MaxBuckets parameter
MaxBuckets is user-controlled and used in uint32(maxBuckets*2) for ListEntries.
Very large values can overflow uint32 or trigger overly expensive scans. Cap
MaxBuckets to 1000 and reject out-of-range values, consistent with MaxTables
handling and S3 MaxKeys validation elsewhere in the codebase.
* s3tables: Validate bucket name in parseBucketNameFromARN()
Enforce the same bucket name validation rules (length, characters, reserved
prefixes/suffixes) when extracting from ARN. This prevents accepting ARNs
that the system would never create and ensures consistency with
CreateTableBucket validation.
* s3tables: Fix parseTableFromARN() namespace and table name validation
- Remove dead URL unescape for namespace (regex [a-z0-9_]+ cannot contain
percent-escapes)
- Add URL decoding and validation of extracted table name via
validateTableName() to prevent callers from bypassing request validation
done in other paths
* s3tables: Rename tableMetadataInternal.Schema to Metadata
The field name 'Schema' was confusing given it holds a *TableMetadata struct
and serializes as 'metadata' in JSON. Rename to 'Metadata' for clarity and
consistency with the JSON tag and intended meaning.
* s3tables: Improve bucket name validation error message
Replace misleading character-only error message with generic 'invalid bucket
name'. The isValidBucketName() function checks multiple constraints beyond
character set (length, reserved prefixes/suffixes, start/end rules), so a
specific character message is inaccurate.
* s3tables: Separate permission checks for tagging and untagging
- Add CanTagResource() to check TagResource permission
- Add CanUntagResource() to check UntagResource permission
- Update CanManageTags() to check both operations (OR logic)
This prevents UntagResource from incorrectly checking 'ManageTags' permission
and ensures each operation validates the correct permission when per-operation
permissions are enforced.
* s3tables: Consolidate getPrincipalFromRequest and getAccountID into single method
Both methods had identical implementations - they return the account ID from
request header or fall back to handler's default. Remove the duplicate
getPrincipalFromRequest and use getAccountID throughout, with updated comment
explaining its dual role as both caller identity and principal for permission
checks.
* s3tables: Fetch bucket policy in handleListTagsForResource for permission evaluation
Update handleListTagsForResource to fetch and pass bucket policy to
CheckPermission, matching the behavior of handleTagResource/handleUntagResource.
This enables bucket-policy-based permission grants to be evaluated for
ListTagsForResource, not just ownership-based checks.
* s3tables: Extract resource owner and bucket extraction into helper method
Create extractResourceOwnerAndBucket() helper to consolidate the repeated pattern
of unmarshaling metadata and extracting bucket name from resource path. This
pattern was duplicated in handleTagResource, handleListTagsForResource, and
handleUntagResource. Update all three handlers to use the helper.
Also update remaining uses of getPrincipalFromRequest() (in handler_bucket_create,
handler_bucket_get_list_delete, handler_namespace) to use getAccountID() after
consolidating the two identical methods.
* s3tables: Add log message when cluster shutdown times out
The timeout path (2 second wait for graceful shutdown) was silent. Add a
warning log message when it occurs to help diagnose flaky test issues and
indicate when the mini cluster didn't shut down cleanly.
* s3tables: Use policy_engine wildcard matcher for complete IAM compatibility
Replace the custom suffix-only wildcard implementation in matchesActionPattern
and matchesPrincipal with the policy_engine.MatchesWildcard function from
PR #8052. This enables full wildcard support including:
- Middle wildcards: s3tables:Get*Table matches GetTable
- Question mark wildcards: Get? matches any single character
- Combined patterns: s3tables:*Table* matches any action containing 'Table'
Benefits:
- Code reuse: eliminates duplicate wildcard logic
- Complete IAM compatibility: supports all AWS wildcard patterns
- Performance: uses efficient O(n) backtracking algorithm
- Consistency: same wildcard behavior across S3 API and S3 Tables
Add comprehensive unit tests covering exact matches, suffix wildcards,
middle wildcards, question marks, and combined patterns for both action
and principal matching.
* go fmt
* s3tables: Fix vet error - remove undefined c.t reference in Stop()
The TestCluster.Stop() method doesn't have access to testing.T object.
Remove the log statement and keep the timeout handling comment for clarity.
The original intent (warning about shutdown timeout) is still captured in
the code comment explaining potential issues.
* clean up
* s3tables: Add t field to TestCluster for logging
Add *testing.T field to TestCluster struct and initialize it in
startMiniCluster. This allows Stop() to properly log warnings when
cluster shutdown times out. Includes the t field in the test cluster
initialization and restores the logging statement in Stop().
* s3tables: Fix bucket policy error handling in permission checks
Replace error-swallowing pattern where all errors from getExtendedAttribute
were ignored for bucket policy reads. Now properly distinguish between:
- ErrAttributeNotFound: Policy not found is expected; continue with empty policy
- Other errors: Return internal server error and stop processing
Applied fix to all bucket policy reads in:
- handleDeleteTableBucketPolicy (line 220)
- handleTagResource (line 313)
- handleUntagResource (line 405)
- handleListTagsForResource (line 488)
- And additional occurrences in closures
This prevents silent failures and ensures policy-related errors are surfaced
to callers rather than being silently ignored.
* s3tables: Pre-validate namespace to return 400 instead of 500
Move validateNamespace call outside of filerClient.WithFilerClient closure
so that validation errors return HTTP 400 (InvalidRequest) instead of 500
(InternalError).
Before: Validation error inside closure → treated as internal error → 500
After: Validation error before closure → handled as bad request → 400
This provides correct error semantics: namespace validation is an input
validation issue, not a server error.
* Update weed/s3api/s3tables/handler.go
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
* s3tables: Normalize action names to include service prefix
Add automatic normalization of operations to full IAM-style action names
(e.g., 's3tables:CreateTableBucket') in CheckPermission(). This ensures
policy statements using prefixed actions (s3tables:*) correctly match
operations evaluated by permission helpers.
Also fixes incorrect r.Context() passed to GetIdentityNameFromContext
which expects *http.Request. Now passes r directly.
* s3tables: Use policy framework for table creation authorization
Replace strict ownership check in CreateTable with policy-based authorization.
Now checks both namespace and bucket policies for CreateTable permission,
allowing delegation via resource policies while still respecting owner bypass.
Authorization logic:
- Namespace policy grants CreateTable → allowed
- Bucket policy grants CreateTable → allowed
- Otherwise → denied (even if same owner)
This enables cross-principal table creation via policies while maintaining
security through explicit allow/deny semantics.
* s3tables: Use policy framework for GetTable authorization
Replace strict ownership check with policy-based authorization in GetTable.
Now checks both table and bucket policies for GetTable permission, allowing
authorized non-owners to read table metadata.
Authorization logic:
- Table policy grants GetTable → allowed
- Bucket policy grants GetTable → allowed
- Otherwise → 404 NotFound (no access disclosed)
Maintains security through policy evaluation while enabling read delegation.
* s3tables: Generate ARNs using resource owner account ID
Change ARN generation to use resource OwnerAccountID instead of caller
identity (h.getAccountID(r)). This ensures ARNs are stable and consistent
regardless of which principal accesses the resource.
Updated generateTableBucketARN and generateTableARN function signatures
to accept ownerAccountID parameter. All call sites updated to pass the
resource owner's account ID from metadata.
This prevents ARN inconsistency issues when multiple principals have
access to the same resource via policies.
* s3tables: Fix remaining policy error handling in namespace and bucket handlers
Replace silent error swallowing (err == nil) with proper error distinction
for bucket policy reads. Now properly checks ErrAttributeNotFound and
propagates other errors as internal server errors.
Fixed 5 locations:
- handleCreateNamespace (policy fetch)
- handleDeleteNamespace (policy fetch)
- handleListNamespaces (policy fetch)
- handleGetNamespace (policy fetch)
- handleGetTableBucket (policy fetch)
This prevents masking of filer issues when policies cannot be read due
to I/O errors or other transient failures.
* ci: Pin GitHub Actions to commit SHAs for s3-tables-tests
Update all action refs to use pinned commit SHAs instead of floating tags:
- actions/checkout: @v6 → @8e8c483 (v4)
- actions/setup-go: @v6 → @0c52d54 (v5)
- actions/upload-artifact: @v6 → @65d8626 (v4)
Pinned SHAs improve reproducibility and reduce supply chain risk by
preventing accidental or malicious changes in action releases. Aligns
with repository conventions used in other workflows (e.g., go.yml).
* s3tables: Add resource ARN validation to policy evaluation
Implement resource-specific policy validation to prevent over-broad
permission grants. Add matchesResource and matchesResourcePattern functions
to validate statement Resource fields against specific resource ARNs.
Add new CheckPermissionWithResource function that includes resource ARN
validation, while keeping CheckPermission unchanged for backward compatibility.
This enables policies to grant access to specific resources only:
- statements with Resource: "arn:aws:s3tables:...:bucket/specific-bucket/*"
will only match when accessing that specific bucket
- statements without Resource field match all resources (implicit *)
- resource patterns support wildcards (* for any sequence, ? for single char)
For future use: Handlers can call CheckPermissionWithResource with the
target resource ARN to enforce resource-level access control.
* Revert "ci: Pin GitHub Actions to commit SHAs for s3-tables-tests"
This reverts commit
|
5 days ago |
|
|
580c2b4ad4 |
command: fix stale error variable logging in filer serving goroutines
- Use local 'err' variable instead of stale 'e' from outer scope - Applied to both TLS and non-TLS paths for local listener |
5 days ago |
|
|
01c17478ae |
command: implement graceful shutdown for mini cluster
- Introduce MiniClusterCtx to coordinate shutdown across mini services - Update Master, Volume, Filer, S3, and WebDAV servers to respect context cancellation - Ensure all resources are cleaned up properly during test teardown - Integrate MiniClusterCtx in s3tables integration tests |
5 days ago |
|
|
6bf088cec9
|
IAM Policy Management via gRPC (#8109)
* Add IAM gRPC service definition - Add GetConfiguration/PutConfiguration for config management - Add CreateUser/GetUser/UpdateUser/DeleteUser/ListUsers for user management - Add CreateAccessKey/DeleteAccessKey/GetUserByAccessKey for access key management - Methods mirror existing IAM HTTP API functionality * Add IAM gRPC handlers on filer server - Implement IamGrpcServer with CredentialManager integration - Handle configuration get/put operations - Handle user CRUD operations - Handle access key create/delete operations - All methods delegate to CredentialManager for actual storage * Wire IAM gRPC service to filer server - Add CredentialManager field to FilerOption and FilerServer - Import credential store implementations in filer command - Initialize CredentialManager from credential.toml if available - Register IAM gRPC service on filer gRPC server - Enable credential management via gRPC alongside existing filer services * Regenerate IAM protobuf with gRPC service methods * iam_pb: add Policy Management to protobuf definitions * credential: implement PolicyManager in credential stores * filer: implement IAM Policy Management RPCs * shell: add s3.policy command * test: add integration test for s3.policy * test: fix compilation errors in policy_test * pb * fmt * test * weed shell: add -policies flag to s3.configure This allows linking/unlinking IAM policies to/from identities directly from the s3.configure command. * test: verify s3.configure policy linking and fix port allocation - Added test case for linking policies to users via s3.configure - Implemented findAvailablePortPair to ensure HTTP and gRPC ports are both available, avoiding conflicts with randomized port assignments. - Updated assertion to match jsonpb output (policyNames) * credential: add StoreTypeGrpc constant * credential: add IAM gRPC store boilerplate * credential: implement identity methods in gRPC store * credential: implement policy methods in gRPC store * admin: use gRPC credential store for AdminServer This ensures that all IAM and policy changes made through the Admin UI are persisted via the Filer's IAM gRPC service instead of direct file manipulation. * shell: s3.configure use granular IAM gRPC APIs instead of full config patching * shell: s3.configure use granular IAM gRPC APIs * shell: replace deprecated ioutil with os in s3.policy * filer: use gRPC FailedPrecondition for unconfigured credential manager * test: improve s3.policy integration tests and fix error checks * ci: add s3 policy shell integration tests to github workflow * filer: fix LoadCredentialConfiguration error handling * credential/grpc: propagate unmarshal errors in GetPolicies * filer/grpc: improve error handling and validation * shell: use gRPC status codes in s3.configure * credential: document PutPolicy as create-or-replace * credential/postgres: reuse CreatePolicy in PutPolicy to deduplicate logic * shell: add timeout context and strictly enforce flags in s3.policy * iam: standardize policy content field naming in gRPC and proto * shell: extract slice helper functions in s3.configure * filer: map credential store errors to gRPC status codes * filer: add input validation for UpdateUser and CreateAccessKey * iam: improve validation in policy and config handlers * filer: ensure IAM service registration by defaulting credential manager * credential: add GetStoreName method to manager * test: verify policy deletion in integration test |
1 week ago |
|
|
6a9860098f
|
fix: correcting S3 nil cipher dereference in filer init (#7952)
Resolves the following error reported in #7949: ``` I0103 21:38:30.230662 s3.go:275 Starting S3 API Server with standard IAM panic: runtime error: invalid memory address or nil pointer dereference [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x38ca961] goroutine 102 [running]: github.com/seaweedfs/seaweedfs/weed/command.(*S3Options).startS3Server(0x7caf840) /go/src/github.com/seaweedfs/seaweedfs/weed/command/s3.go:295 +0x741 github.com/seaweedfs/seaweedfs/weed/command.runFiler.func1(...) /go/src/github.com/seaweedfs/seaweedfs/weed/command/filer.go:244 created by github.com/seaweedfs/seaweedfs/weed/command.runFiler in goroutine 1 /go/src/github.com/seaweedfs/seaweedfs/weed/command/filer.go:242 +0x353 ``` |
4 weeks ago |
|
|
1b1e5f69a2
|
Add TUS protocol support for resumable uploads (#7592)
* Add TUS protocol integration tests
This commit adds integration tests for the TUS (resumable upload) protocol
in preparation for implementing TUS support in the filer.
Test coverage includes:
- OPTIONS handler for capability discovery
- Basic single-request upload
- Chunked/resumable uploads
- HEAD requests for offset tracking
- DELETE for upload cancellation
- Error handling (invalid offsets, missing uploads)
- Creation-with-upload extension
- Resume after interruption simulation
Tests are skipped in short mode and require a running SeaweedFS cluster.
* Add TUS session storage types and utilities
Implements TUS upload session management:
- TusSession struct for tracking upload state
- Session creation with directory-based storage
- Session persistence using filer entries
- Session retrieval and offset updates
- Session deletion with chunk cleanup
- Upload completion with chunk assembly into final file
Session data is stored in /.uploads.tus/{upload-id}/ directory,
following the pattern used by S3 multipart uploads.
* Add TUS HTTP handlers
Implements TUS protocol HTTP handlers:
- tusHandler: Main entry point routing requests
- tusOptionsHandler: Capability discovery (OPTIONS)
- tusCreateHandler: Create new upload (POST)
- tusHeadHandler: Get upload offset (HEAD)
- tusPatchHandler: Upload data at offset (PATCH)
- tusDeleteHandler: Cancel upload (DELETE)
- tusWriteData: Upload data to volume servers
Features:
- Supports creation-with-upload extension
- Validates TUS protocol headers
- Offset conflict detection
- Automatic upload completion when size is reached
- Metadata parsing from Upload-Metadata header
* Wire up TUS protocol routes in filer server
Add TUS handler route (/.tus/) to the filer HTTP server.
The TUS route is registered before the catch-all route to ensure
proper routing of TUS protocol requests.
TUS protocol is now accessible at:
- OPTIONS /.tus/ - Capability discovery
- POST /.tus/{path} - Create upload
- HEAD /.tus/.uploads/{id} - Get offset
- PATCH /.tus/.uploads/{id} - Upload data
- DELETE /.tus/.uploads/{id} - Cancel upload
* Improve TUS integration test setup
Add comprehensive Makefile for TUS tests with targets:
- test-with-server: Run tests with automatic server management
- test-basic/chunked/resume/errors: Specific test categories
- manual-start/stop: For development testing
- debug-logs/status: For debugging
- ci-test: For CI/CD pipelines
Update README.md with:
- Detailed TUS protocol documentation
- All endpoint descriptions with headers
- Usage examples with curl commands
- Architecture diagram
- Comparison with S3 multipart uploads
Follows the pattern established by other tests in test/ folder.
* Fix TUS integration tests and creation-with-upload
- Fix test URLs to use full URLs instead of relative paths
- Fix creation-with-upload to refresh session before completing
- Fix Makefile to properly handle test cleanup
- Add FullURL helper function to TestCluster
* Add TUS protocol tests to GitHub Actions CI
- Add tus-tests.yml workflow that runs on PRs and pushes
- Runs when TUS-related files are modified
- Automatic server management for integration testing
- Upload logs on failure for debugging
* Make TUS base path configurable via CLI
- Add -tus.path CLI flag to filer command
- TUS is disabled by default (empty path)
- Example: -tus.path=/.tus to enable at /.tus endpoint
- Update test Makefile to use -tus.path flag
- Update README with TUS enabling instructions
* Rename -tus.path to -tusBasePath with default .tus
- Rename CLI flag from -tus.path to -tusBasePath
- Default to .tus (TUS enabled by default)
- Add -filer.tusBasePath option to weed server command
- Properly handle path prefix (prepend / if missing)
* Address code review comments
- Sort chunks by offset before assembling final file
- Use chunk.Offset directly instead of recalculating
- Return error on invalid file ID instead of skipping
- Require Content-Length header for PATCH requests
- Use fs.option.Cipher for encryption setting
- Detect MIME type from data using http.DetectContentType
- Fix concurrency group for push events in workflow
- Use os.Interrupt instead of Kill for graceful shutdown in tests
* fmt
* Address remaining code review comments
- Fix potential open redirect vulnerability by sanitizing uploadLocation path
- Add language specifier to README code block
- Handle os.Create errors in test setup
- Use waitForHTTPServer instead of time.Sleep for master/volume readiness
- Improve test reliability and debugging
* Address critical and high-priority review comments
- Add per-session locking to prevent race conditions in updateTusSessionOffset
- Stream data directly to volume server instead of buffering entire chunk
- Only buffer 512 bytes for MIME type detection, then stream remaining data
- Clean up session locks when session is deleted
* Fix race condition to work across multiple filer instances
- Store each chunk as a separate file entry instead of updating session JSON
- Chunk file names encode offset, size, and fileId for atomic storage
- getTusSession loads chunks from directory listing (atomic read)
- Eliminates read-modify-write race condition across multiple filers
- Remove in-memory mutex that only worked for single filer instance
* Address code review comments: fix variable shadowing, sniff size, and test stability
- Rename path variable to reqPath to avoid shadowing path package
- Make sniff buffer size respect contentLength (read at most contentLength bytes)
- Handle Content-Length < 0 in creation-with-upload (return error for chunked encoding)
- Fix test cluster: use temp directory for filer store, add startup delay
* Fix test stability: increase cluster stabilization delay to 5 seconds
The tests were intermittently failing because the volume server needed more
time to create volumes and register with the master. Increasing the delay
from 2 to 5 seconds fixes the flaky test behavior.
* Address PR review comments for TUS protocol support
- Fix strconv.Atoi error handling in test file (lines 386, 747)
- Fix lossy fileId encoding: use base64 instead of underscore replacement
- Add pagination support for ListDirectoryEntries in getTusSession
- Batch delete chunks instead of one-by-one in deleteTusSession
* Address additional PR review comments for TUS protocol
- Fix UploadAt timestamp: use entry.Crtime instead of time.Now()
- Remove redundant JSON content in chunk entry (metadata in filename)
- Refactor tusWriteData to stream in 4MB chunks to avoid OOM on large uploads
- Pass filer.Entry to parseTusChunkPath to preserve actual upload time
* Address more PR review comments for TUS protocol
- Normalize TUS path once in filer_server.go, store in option.TusPath
- Remove redundant path normalization from TUS handlers
- Remove goto statement in tusCreateHandler, simplify control flow
* Remove unnecessary mutexes in tusWriteData
The upload loop is sequential, so uploadErrLock and chunksLock are not needed.
* Rename updateTusSessionOffset to saveTusChunk
Remove unused newOffset parameter and rename function to better reflect its purpose.
* Improve TUS upload performance and add path validation
- Reuse operation.Uploader across sub-chunks for better connection reuse
- Guard against TusPath='/' to prevent hijacking all filer routes
* Address PR review comments for TUS protocol
- Fix critical chunk filename parsing: use strings.Cut instead of SplitN
to correctly handle base64-encoded fileIds that may contain underscores
- Rename tusPath to tusBasePath for naming consistency across codebase
- Add background garbage collection for expired TUS sessions (runs hourly)
- Improve error messages with %w wrapping for better debuggability
* Address additional TUS PR review comments
- Fix tusBasePath default to use leading slash (/.tus) for consistency
- Add chunk contiguity validation in completeTusUpload to detect gaps/overlaps
- Fix offset calculation to find maximum contiguous range from 0, not just last chunk
- Return 413 Request Entity Too Large instead of silently truncating content
- Document tusChunkSize rationale (4MB balances memory vs request overhead)
- Fix Makefile xargs portability by removing GNU-specific -r flag
- Add explicit -tusBasePath flag to integration test for robustness
- Fix README example to use /.uploads/tus path format
* Revert log_buffer changes (moved to separate PR)
* Minor style fixes from PR review
- Simplify tusBasePath flag description to use example format
- Add 'TUS upload' prefix to session not found error message
- Remove duplicate tusChunkSize comment
- Capitalize warning message for consistency
- Add grep filter to Makefile xargs for better empty input handling
|
2 months ago |
|
|
f41925b60b
|
Embed IAM API into S3 server (#7740)
* Embed IAM API into S3 server This change simplifies the S3 and IAM deployment by embedding the IAM API directly into the S3 server, following the patterns used by MinIO and Ceph RGW. Changes: - Add -iam flag to S3 server (enabled by default) - Create embedded IAM API handler in s3api package - Register IAM routes (POST to /) in S3 server when enabled - Deprecate standalone 'weed iam' command with warning Benefits: - Single binary, single port for both S3 and IAM APIs - Simpler deployment and configuration - Shared credential manager between S3 and IAM - Backward compatible: 'weed iam' still works with deprecation warning Usage: - weed s3 -port=8333 # S3 + IAM on same port (default) - weed s3 -iam=false # S3 only, disable embedded IAM - weed iam -port=8111 # Deprecated, shows warning * Fix nil pointer panic: add s3.iam flag to weed server command The enableIam field was not initialized when running S3 via 'weed server', causing a nil pointer dereference when checking *s3opt.enableIam. * Fix nil pointer panic: add s3.iam flag to weed filer command The enableIam field was not initialized when running S3 via 'weed filer -s3', causing a nil pointer dereference when checking *s3opt.enableIam. * Add integration tests for embedded IAM API Tests cover: - CreateUser, ListUsers, GetUser, UpdateUser, DeleteUser - CreateAccessKey, DeleteAccessKey, ListAccessKeys - CreatePolicy, PutUserPolicy, GetUserPolicy - Implicit username extraction from authorization header - Full user lifecycle workflow test These tests validate the embedded IAM API functionality that was added in the S3 server, ensuring IAM operations work correctly when served from the same port as S3. * Security: Use crypto/rand for IAM credential generation SECURITY FIX: Replace math/rand with crypto/rand for generating access keys and secret keys. Using math/rand is not cryptographically secure and can lead to predictable credentials. This change: 1. Replaces math/rand with crypto/rand in both: - weed/s3api/s3api_embedded_iam.go (embedded IAM) - weed/iamapi/iamapi_management_handlers.go (standalone IAM) 2. Removes the seededRand variable that was initialized with time-based seed (predictable) 3. Updates StringWithCharset/iamStringWithCharset to: - Use crypto/rand.Int() for secure random index generation - Return an error for proper error handling 4. Updates CreateAccessKey to handle the new error return 5. Updates DoActions handlers to propagate errors properly * Fix critical bug: DeleteUserPolicy was deleting entire user instead of policy BUG FIX: DeleteUserPolicy was incorrectly deleting the entire user identity from s3cfg.Identities instead of just clearing the user's inline policy (Actions). Before (wrong): s3cfg.Identities = append(s3cfg.Identities[:i], s3cfg.Identities[i+1:]...) After (correct): ident.Actions = nil Also: - Added proper iamDeleteUserPolicyResponse / DeleteUserPolicyResponse types - Fixed return type from iamPutUserPolicyResponse to iamDeleteUserPolicyResponse Affected files: - weed/s3api/s3api_embedded_iam.go (embedded IAM) - weed/iamapi/iamapi_management_handlers.go (standalone IAM) - weed/iamapi/iamapi_response.go (response types) * Add tests for DeleteUserPolicy to prevent regression Added two tests: 1. TestEmbeddedIamDeleteUserPolicy - Verifies that: - User is NOT deleted (identity still exists) - Credentials are NOT deleted - Only Actions (policy) are cleared to nil 2. TestEmbeddedIamDeleteUserPolicyUserNotFound - Verifies: - Returns 404 when user doesn't exist These tests ensure the bug fixed in the previous commit (deleting user instead of policy) doesn't regress. * Fix race condition: Add mutex lock to IAM DoActions The DoActions function performs a read-modify-write operation on the shared IAM configuration without any locking. This could lead to race conditions and data loss if multiple requests modify the IAM config concurrently. Added mutex lock at the start of DoActions in both: - weed/s3api/s3api_embedded_iam.go (embedded IAM) - weed/iamapi/iamapi_management_handlers.go (standalone IAM) The lock protects the entire read-modify-write cycle: 1. GetS3ApiConfiguration (read) 2. Modify s3cfg based on action 3. PutS3ApiConfiguration (write) * Fix action comparison and document CreatePolicy limitation 1. Replace reflect.DeepEqual with order-independent string slice comparison - Added iamStringSlicesEqual/stringSlicesEqual helper functions - Prevents duplicate policy statements when actions are in different order 2. Document CreatePolicy limitation in embedded IAM - Added TODO comment explaining that managed policies are not persisted - Users should use PutUserPolicy for inline policies 3. Fix deadlock in standalone IAM's CreatePolicy - Removed nested lock acquisition (DoActions already holds the lock) Files changed: - weed/s3api/s3api_embedded_iam.go - weed/iamapi/iamapi_management_handlers.go * Add rate limiting to embedded IAM endpoint Apply circuit breaker rate limiting to the IAM endpoint to prevent abuse. Also added request tracking for IAM operations. The IAM endpoint now follows the same pattern as other S3 endpoints: - track() for request metrics - s3a.iam.Auth() for authentication - s3a.cb.Limit() for rate limiting * Fix handleImplicitUsername to properly look up username from AccessKeyId According to AWS spec, when UserName is not specified in an IAM request, IAM should determine the username implicitly based on the AccessKeyId signing the request. Previously, the code incorrectly extracted s[2] (region field) from the SigV4 credential string as the username. This fix: 1. Extracts the AccessKeyId from s[0] of the credential string 2. Looks up the AccessKeyId in the credential store using LookupByAccessKey 3. Uses the identity's Name field as the username if found Also: - Added exported LookupByAccessKey wrapper method to IdentityAccessManagement - Updated tests to verify correct access key lookup behavior - Applied fix to both embedded IAM and standalone IAM implementations * Fix CreatePolicy to not trigger unnecessary save CreatePolicy validates the policy document and returns metadata but does not actually store the policy (SeaweedFS uses inline policies attached via PutUserPolicy). However, 'changed' was left as true, triggering an unnecessary save operation. Set changed = false after successful CreatePolicy validation in both embedded IAM and standalone IAM implementations. * Improve embedded IAM test quality - Remove unused mock types (mockCredentialManager, mockEmbeddedIamApi) - Use proto.Clone instead of proto.Merge for proper deep copy semantics - Replace brittle regex-based XML error extraction with proper XML unmarshalling - Remove unused regexp import - Add state and field assertions to tests: - CreateUser: verify username in response and user persisted in config - ListUsers: verify response contains expected users - GetUser: verify username in response - CreatePolicy: verify policy metadata in response - PutUserPolicy: verify actions were attached to user - CreateAccessKey: verify credentials in response and persisted in config * Remove shared test state and improve executeEmbeddedIamRequest - Remove package-level embeddedIamApi variable to avoid shared test state - Update executeEmbeddedIamRequest to accept API instance as parameter - Only call xml.Unmarshal when v != nil, making nil-v cases explicit - Return unmarshal error properly instead of always returning it - Update all tests to create their own EmbeddedIamApiForTest instance - Each test now has isolated state, preventing test interdependencies * Add comprehensive test coverage for embedded IAM Added tests for previously uncovered functions: - iamStringSlicesEqual: 0% → 100% - iamMapToStatementAction: 40% → 100% - iamMapToIdentitiesAction: 30% → 70% - iamHash: 100% - iamStringWithCharset: 85.7% - GetPolicyDocument: 75% → 100% - CreatePolicy: 91.7% → 100% - DeleteUser: 83.3% → 100% - GetUser: 83.3% → 100% - ListAccessKeys: 55.6% → 88.9% New test cases for helper functions, error handling, and edge cases. * Document IAM code duplication and reference GitHub issue #7747 Added comments to both IAM implementations noting the code duplication and referencing the tracking issue for future refactoring: - weed/s3api/s3api_embedded_iam.go (embedded IAM) - weed/iamapi/iamapi_management_handlers.go (standalone IAM) See: https://github.com/seaweedfs/seaweedfs/issues/7747 * Implement granular IAM authorization for self-service operations Previously, all IAM actions required ACTION_ADMIN permission, which was overly restrictive. This change implements AWS-like granular permissions: Self-service operations (allowed without admin for own resources): - CreateAccessKey (on own user) - DeleteAccessKey (on own user) - ListAccessKeys (on own user) - GetUser (on own user) - UpdateAccessKey (on own user) Admin-only operations: - CreateUser, DeleteUser, UpdateUser - PutUserPolicy, GetUserPolicy, DeleteUserPolicy - CreatePolicy - ListUsers - Operations on other users The new AuthIam middleware: 1. Authenticates the request (signature verification) 2. Parses the IAM Action and target UserName 3. For self-service actions, allows if user is operating on own resources 4. For all other actions or operations on other users, requires admin * Fix misleading comment in standalone IAM CreatePolicy The comment incorrectly stated that CreatePolicy only validates the policy document. In the standalone IAM server, CreatePolicy actually persists the policy via iama.s3ApiConfig.PutPolicies(). The changed flag is false because it doesn't modify s3cfg.Identities, not because nothing is stored. * Simplify IAM auth and add RequestId to responses - Remove redundant ACTION_ADMIN fallback in AuthIam: The action parameter in authRequest is for permission checking, not signature verification. If auth fails with ACTION_READ, it will fail with ACTION_ADMIN too. - Add SetRequestId() call before writing IAM responses for AWS compatibility. All IAM response structs embed iamCommonResponse which has SetRequestId(). * Address code review feedback for IAM implementation 1. auth_credentials.go: Add documentation warning that LookupByAccessKey returns internal pointers that should not be mutated. 2. iamapi_management_handlers.go & s3api_embedded_iam.go: Add input guards for StringWithCharset/iamStringWithCharset when length <= 0 or charset is empty to avoid runtime errors from rand.Int. 3. s3api_embedded_iam_test.go: Don't ignore xml.Marshal errors in test DoActions handler. Return proper error response if marshaling fails. 4. s3api_embedded_iam_test.go: Use obviously fake access key IDs (AKIATESTFAKEKEY*) to avoid CI secret scanner false positives. * Address code review feedback for IAM implementation (batch 2) 1. iamapi/iamapi_management_handlers.go: - Redact Authorization header log (security: avoid exposing signature) - Add nil-guard for iama.iam before LookupByAccessKey call 2. iamapi/iamapi_test.go: - Replace real-looking access keys with obviously fake ones (AKIATESTFAKEKEY*) to avoid CI secret scanner false positives 3. s3api/s3api_embedded_iam.go - CreateUser: - Validate UserName is not empty (return ErrCodeInvalidInputException) - Check for duplicate users (return ErrCodeEntityAlreadyExistsException) 4. s3api/s3api_embedded_iam.go - CreateAccessKey: - Return ErrCodeNoSuchEntityException if user doesn't exist - Removed implicit user creation behavior 5. s3api/s3api_embedded_iam.go - getActions: - Fix S3 ARN parsing for bucket/path patterns - Handle mybucket, mybucket/*, mybucket/path/* correctly - Return error if no valid actions found in policy 6. s3api/s3api_embedded_iam.go - handleImplicitUsername: - Redact Authorization header log - Add nil-guard for e.iam 7. s3api/s3api_embedded_iam.go - DoActions: - Reload in-memory IAM maps after credential mutations - Call LoadS3ApiConfigurationFromCredentialManager after save 8. s3api/auth_credentials.go - AuthSignatureOnly: - Add new signature-only authentication method - Bypasses S3 authorization checks for IAM operations - Used by AuthIam to properly separate signature verification from IAM-specific permission checks * Fix nil pointer dereference and error handling in IAM 1. AuthIam: Add nil check for identity after AuthSignatureOnly - AuthSignatureOnly can return nil identity with ErrNone for authTypePostPolicy or authTypeStreamingUnsigned - Now returns ErrAccessDenied if identity is nil 2. writeIamErrorResponse: Add missing error code cases - ErrCodeEntityAlreadyExistsException -> HTTP 409 Conflict - ErrCodeInvalidInputException -> HTTP 400 Bad Request 3. UpdateUser: Use consistent error handling - Changed from direct ErrInvalidRequest to writeIamErrorResponse - Now returns correct HTTP status codes based on error type * Add IAM config reload to standalone IAM server after mutations Match the behavior of embedded IAM (s3api_embedded_iam.go) by reloading the in-memory identity maps after persisting configuration changes. This ensures newly created access keys are visible to LookupByAccessKey immediately without requiring a server restart. * Minor improvements to test helpers and log masking 1. iamapi_test.go: Update mustMarshalJSON to use t.Helper() and t.Fatal() instead of panic() for better test diagnostics 2. s3api_embedded_iam.go: Mask access key in 'not found' log message to avoid exposing full access key IDs in logs * Mask access key in standalone IAM log message for consistency Match the embedded IAM version by masking the access key ID in the 'not found' log message (show only first 4 chars). |
2 months ago |
|
|
1ee03b6411 |
fmt
|
2 months ago |
|
|
2d06ddab41
|
Remove default concurrent upload/download limits for best performance (#7712)
Change all concurrentUploadLimitMB and concurrentDownloadLimitMB defaults from fixed values (64, 128, 256 MB) to 0 (unlimited). This removes artificial throttling that can limit throughput on high-performance systems, especially on all-flash setups with many cores. Files changed: - volume.go: concurrentUploadLimitMB 256->0, concurrentDownloadLimitMB 256->0 - server.go: filer/volume/s3 concurrent limits 64/128->0 - s3.go: concurrentUploadLimitMB 128->0 - filer.go: concurrentUploadLimitMB 128->0, s3.concurrentUploadLimitMB 128->0 Users can still set explicit limits if needed for resource management. |
2 months ago |
|
|
a9b3be416b
|
fix: initialize missing S3 options in filer to prevent nil pointer dereference (#7646)
* fix: initialize missing S3 options in filer to prevent nil pointer dereference Fixes #7644 When starting the S3 gateway from the filer, several S3Options fields were not being initialized, which could cause nil pointer dereferences during startup. This commit adds initialization for: - iamConfig: for advanced IAM configuration - metricsHttpPort: for Prometheus metrics endpoint - metricsHttpIp: for binding the metrics endpoint Also ensures metricsHttpIp defaults to bindIp when not explicitly set, matching the behavior of the standalone S3 server. This prevents the panic that was occurring in the s3.go:226 area when these pointer fields were accessed but never initialized. * fix: copy value instead of pointer for metricsHttpIp default Address review comment to avoid pointer aliasing. Copy the value instead of the pointer to prevent unexpected side effects if the bindIp value is modified later. |
2 months ago |
|
|
5167bbd2a9 |
Remove deprecated allowEmptyFolder CLI option
The allowEmptyFolder option is no longer functional because: 1. The code that used it was already commented out 2. Empty folder cleanup is now handled asynchronously by EmptyFolderCleaner The CLI flags are kept for backward compatibility but marked as deprecated and ignored. This removes: - S3ApiServerOption.AllowEmptyFolder field - The actual usage in s3api_object_handlers_list.go - Helm chart values and template references - References in test Makefiles and docker-compose files |
2 months ago |
|
|
3183a49698
|
fix: S3 downloads failing after idle timeout (#7626)
* fix: S3 downloads failing after idle timeout (#7618) The idle timeout was incorrectly terminating active downloads because read and write deadlines were managed independently. During a download, the server writes data but rarely reads, so the read deadline would expire even though the connection was actively being used. Changes: 1. Simplify to single Timeout field - since this is a 'no activity timeout' where any activity extends the deadline, separate read/write timeouts are unnecessary. Now uses SetDeadline() which sets both at once. 2. Implement proper 'no activity timeout' - any activity (read or write) now extends the deadline. The connection only times out when there's genuinely no activity in either direction. 3. Increase default S3 idleTimeout from 10s to 120s for additional safety margin when fetching chunks from slow storage backends. Fixes #7618 * Update weed/util/net_timeout.go Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> |
2 months ago |
|
|
edf0ef7a80
|
Filer, S3: Feature/add concurrent file upload limit (#7554)
* Support multiple filers for S3 and IAM servers with automatic failover
This change adds support for multiple filer addresses in the 'weed s3' and 'weed iam' commands, enabling high availability through automatic failover.
Key changes:
- Updated S3ApiServerOption.Filer to Filers ([]pb.ServerAddress)
- Updated IamServerOption.Filer to Filers ([]pb.ServerAddress)
- Modified -filer flag to accept comma-separated addresses
- Added getFilerAddress() helper methods for backward compatibility
- Updated all filer client calls to support multiple addresses
- Uses pb.WithOneOfGrpcFilerClients for automatic failover
Usage:
weed s3 -filer=localhost:8888,localhost:8889
weed iam -filer=localhost:8888,localhost:8889
The underlying FilerClient already supported multiple filers with health
tracking and automatic failover - this change exposes that capability
through the command-line interface.
* Add filer discovery: treat initial filers as seeds and discover peers from master
Enhances FilerClient to automatically discover additional filers in the same
filer group by querying the master server. This allows users to specify just
a few seed filers, and the client will discover all other filers in the cluster.
Key changes to wdclient/FilerClient:
- Added MasterClient, FilerGroup, and DiscoveryInterval fields
- Added thread-safe filer list management with RWMutex
- Implemented discoverFilers() background goroutine
- Uses cluster.ListExistingPeerUpdates() to query master for filers
- Automatically adds newly discovered filers to the list
- Added Close() method to clean up discovery goroutine
New FilerClientOption fields:
- MasterClient: enables filer discovery from master
- FilerGroup: specifies which filer group to discover
- DiscoveryInterval: how often to refresh (default 5 minutes)
Usage example:
masterClient := wdclient.NewMasterClient(...)
filerClient := wdclient.NewFilerClient(
[]pb.ServerAddress{"localhost:8888"}, // seed filers
grpcDialOption,
dataCenter,
&wdclient.FilerClientOption{
MasterClient: masterClient,
FilerGroup: "my-group",
},
)
defer filerClient.Close()
The initial filers act as seeds - the client discovers and adds all other
filers in the same group from the master. Discovered filers are added
dynamically without removing existing ones (relying on health checks for
unavailable filers).
* Address PR review comments: implement full failover for IAM operations
Critical fixes based on code review feedback:
1. **IAM API Failover (Critical)**:
- Replace pb.WithGrpcFilerClient with pb.WithOneOfGrpcFilerClients in:
* GetS3ApiConfigurationFromFiler()
* PutS3ApiConfigurationToFiler()
* GetPolicies()
* PutPolicies()
- Now all IAM operations support automatic failover across multiple filers
2. **Validation Improvements**:
- Add validation in NewIamApiServerWithStore() to require at least one filer
- Add validation in NewS3ApiServerWithStore() to require at least one filer
- Add warning log when no filers configured for credential store
3. **Error Logging**:
- Circuit breaker now logs when config load fails instead of silently ignoring
- Helps operators understand why circuit breaker limits aren't applied
4. **Code Quality**:
- Use ToGrpcAddress() for filer address in credential store setup
- More consistent with rest of codebase and future-proof
These changes ensure IAM operations have the same high availability guarantees
as S3 operations, completing the multi-filer failover implementation.
* Fix IAM manager initialization: remove code duplication, add TODO for HA
Addresses review comment on s3api_server.go:145
Changes:
- Remove duplicate code for getting first filer address
- Extract filerAddr variable once and reuse
- Add TODO comment documenting the HA limitation for IAM manager
- Document that loadIAMManagerFromConfig and NewS3IAMIntegration need
updates to support multiple filers for full HA
Note: This is a known limitation when using filer-backed IAM stores.
The interfaces need to be updated to accept multiple filer addresses.
For now, documenting this limitation clearly.
* Document credential store HA limitation with TODO
Addresses review comment on auth_credentials.go:149
Changes:
- Add TODO comment documenting that SetFilerClient interface needs update
for multi-filer support
- Add informative log message indicating HA limitation
- Document that this is a known limitation for filer-backed credential stores
The SetFilerClient interface currently only accepts a single filer address.
To properly support HA, the credential store interfaces need to be updated
to handle multiple filer addresses.
* Track current active filer in FilerClient for better HA
Add GetCurrentFiler() method to FilerClient that returns the currently
active filer based on the filerIndex which is updated on successful
operations. This provides better availability than always using the
first filer.
Changes:
- Add FilerClient.GetCurrentFiler() method that returns current active filer
- Update S3ApiServer.getFilerAddress() to use FilerClient's current filer
- Add fallback to first filer if FilerClient not yet initialized
- Document IAM limitation (doesn't have FilerClient access)
Benefits:
- Single-filer operations (URLs, ReadFilerConf, etc.) now use the
currently active/healthy filer
- Better distribution and failover behavior
- FilerClient's round-robin and health tracking automatically
determines which filer to use
* Document ReadFilerConf HA limitation in lifecycle handlers
Addresses review comment on s3api_bucket_handlers.go:880
Add comment documenting that ReadFilerConf uses the current active filer
from FilerClient (which is better than always using first filer), but
doesn't have built-in multi-filer failover.
Add TODO to update filer.ReadFilerConf to support multiple filers for
complete HA. For now, it uses the currently active/healthy filer tracked
by FilerClient which provides reasonable availability.
* Document multipart upload URL HA limitation
Addresses review comment on s3api_object_handlers_multipart.go:442
Add comment documenting that part upload URLs point to the current
active filer (tracked by FilerClient), which is better than always
using the first filer but still creates a potential point of failure
if that filer becomes unavailable during upload.
Suggest TODO solutions:
- Use virtual hostname/load balancer for filers
- Have S3 server proxy uploads to healthy filers
Current behavior provides reasonable availability by using the
currently active/healthy filer rather than being pinned to first filer.
* Document multipart completion Location URL limitation
Addresses review comment on filer_multipart.go:187
Add comment documenting that the Location URL in CompleteMultipartUpload
response points to the current active filer (tracked by FilerClient).
Note that clients should ideally use the S3 API endpoint rather than
this direct URL. If direct access is attempted and the specific filer
is unavailable, the request will fail.
Current behavior uses the currently active/healthy filer rather than
being pinned to the first filer, providing better availability.
* Make credential store use current active filer for HA
Update FilerEtcStore to use a function that returns the current active
filer instead of a fixed address, enabling high availability.
Changes:
- Add SetFilerAddressFunc() method to FilerEtcStore
- Store uses filerAddressFunc instead of fixed filerGrpcAddress
- withFilerClient() calls the function to get current active filer
- Keep SetFilerClient() for backward compatibility (marked deprecated)
- Update S3ApiServer to pass FilerClient.GetCurrentFiler to store
Benefits:
- Credential store now uses currently active/healthy filer
- Automatic failover when filer becomes unavailable
- True HA for credential operations
- Backward compatible with old SetFilerClient interface
This addresses the credential store limitation - no longer pinned to
first filer, uses FilerClient's tracked current active filer.
* Clarify multipart URL comments: filer address not used for uploads
Update comments to reflect that multipart upload URLs are not actually
used for upload traffic - uploads go directly to volume servers.
Key clarifications:
- genPartUploadUrl: Filer address is parsed out, only path is used
- CompleteMultipartUpload Location: Informational field per AWS S3 spec
- Actual uploads bypass filer proxy and go directly to volume servers
The filer address in these URLs is NOT a HA concern because:
1. Part uploads: URL is parsed for path, upload goes to volume servers
2. Location URL: Informational only, clients use S3 endpoint
This addresses the observation that S3 uploads don't go through filers,
only metadata operations do.
* Remove filer address from upload paths - pass path directly
Eliminate unnecessary filer address from upload URLs by passing file
paths directly instead of full URLs that get immediately parsed.
Changes:
- Rename genPartUploadUrl() → genPartUploadPath() (returns path only)
- Rename toFilerUrl() → toFilerPath() (returns path only)
- Update putToFiler() to accept filePath instead of uploadUrl
- Remove URL parsing code (no longer needed)
- Remove net/url import (no longer used)
- Keep old function names as deprecated wrappers for compatibility
Benefits:
- Cleaner code - no fake URL construction/parsing
- No dependency on filer address for internal operations
- More accurate naming (these are paths, not URLs)
- Eliminates confusion about HA concerns
This completely removes the filer address from upload operations - it was
never actually used for routing, only parsed for the path.
* Remove deprecated functions: use new path-based functions directly
Remove deprecated wrapper functions and update all callers to use the
new function names directly.
Removed:
- genPartUploadUrl() → all callers now use genPartUploadPath()
- toFilerUrl() → all callers now use toFilerPath()
- SetFilerClient() → removed along with fallback code
Updated:
- s3api_object_handlers_multipart.go: uploadUrl → filePath
- s3api_object_handlers_put.go: uploadUrl → filePath, versionUploadUrl → versionFilePath
- s3api_object_versioning.go: toFilerUrl → toFilerPath
- s3api_object_handlers_test.go: toFilerUrl → toFilerPath
- auth_credentials.go: removed SetFilerClient fallback
- filer_etc_store.go: removed deprecated SetFilerClient method
Benefits:
- Cleaner codebase with no deprecated functions
- All variable names accurately reflect that they're paths, not URLs
- Single interface for credential stores (SetFilerAddressFunc only)
All code now consistently uses the new path-based approach.
* Fix toFilerPath: remove URL escaping for raw file paths
The toFilerPath function should return raw file paths, not URL-escaped
paths. URL escaping was needed when the path was embedded in a URL
(old toFilerUrl), but now that we pass paths directly to putToFiler,
they should be unescaped.
This fixes S3 integration test failures:
- test_bucket_listv2_encoding_basic
- test_bucket_list_encoding_basic
- test_bucket_listv2_delimiter_whitespace
- test_bucket_list_delimiter_whitespace
The tests were failing because paths were double-encoded (escaped when
stored, then escaped again when listed), resulting in %252B instead of
%2B for '+' characters.
Root cause: When we removed URL parsing in putToFiler, we should have
also removed URL escaping in toFilerPath since paths are now used
directly without URL encoding/decoding.
* Add thread safety to FilerEtcStore and clarify credential store comments
Address review suggestions for better thread safety and code clarity:
1. **Thread Safety**: Add RWMutex to FilerEtcStore
- Protects filerAddressFunc and grpcDialOption from concurrent access
- Initialize() uses write lock when setting function
- SetFilerAddressFunc() uses write lock
- withFilerClient() uses read lock to get function and dial option
- GetPolicies() uses read lock to check if configured
2. **Improved Error Messages**:
- Prefix errors with "filer_etc:" for easier debugging
- "filer address not configured" → "filer_etc: filer address function not configured"
- "filer address is empty" → "filer_etc: filer address is empty"
3. **Clarified Comments**:
- auth_credentials.go: Clarify that initial setup is temporary
- Document that it's updated in s3api_server.go after FilerClient creation
- Remove ambiguity about when FilerClient.GetCurrentFiler is used
Benefits:
- Safe for concurrent credential operations
- Clear error messages for debugging
- Explicit documentation of initialization order
* Enable filer discovery: pass master addresses to FilerClient
Fix two critical issues:
1. **Filer Discovery Not Working**: Master client was not being passed to
FilerClient, so peer discovery couldn't work
2. **Credential Store Design**: Already uses FilerClient via GetCurrentFiler
function - this is the correct design for HA
Changes:
**Command (s3.go):**
- Read master addresses from GetFilerConfiguration response
- Pass masterAddresses to S3ApiServerOption
- Log master addresses for visibility
**S3ApiServerOption:**
- Add Masters []pb.ServerAddress field for discovery
**S3ApiServer:**
- Create MasterClient from Masters when available
- Pass MasterClient + FilerGroup to FilerClient via options
- Enable discovery with 5-minute refresh interval
- Log whether discovery is enabled or disabled
**Credential Store:**
- Already correctly uses filerClient.GetCurrentFiler via function
- This provides HA without tight coupling to FilerClient struct
- Function-based design is clean and thread-safe
Discovery Flow:
1. S3 command reads filer config → gets masters + filer group
2. S3ApiServer creates MasterClient from masters
3. FilerClient uses MasterClient to query for peer filers
4. Background goroutine refreshes peer list every 5 minutes
5. Credential store uses GetCurrentFiler to get active filer
Now filer discovery actually works! ��
* Use S3 endpoint in multipart Location instead of filer address
* Add multi-filer failover to ReadFilerConf
* Address CodeRabbit review: fix buffer reuse and improve lock safety
Address two code review suggestions:
1. **Fix buffer reuse in ReadFilerConfFromFilers**:
- Use local []byte data instead of shared buffer
- Prevents partial data from failed attempts affecting successful reads
- Creates fresh buffer inside callback for masterClient path
- More robust to future changes in read helpers
2. **Improve lock safety in FilerClient**:
- Add *WithHealth variants that accept health pointer
- Get health pointer while holding lock, then release before calling
- Eliminates potential for lock confusion (though no actual deadlock existed)
- Clearer separation: lock for data access, atomics for health ops
Changes:
- ReadFilerConfFromFilers: var data []byte, create buf inside callback
- shouldSkipUnhealthyFilerWithHealth(health *filerHealth)
- recordFilerSuccessWithHealth(health *filerHealth)
- recordFilerFailureWithHealth(health *filerHealth)
- Keep old functions for backward compatibility (marked deprecated)
- Update LookupVolumeIds to use WithHealth variants
Benefits:
- More robust multi-filer configuration reading
- Clearer lock vs atomic operation boundaries
- No lock held during health checks (even though atomics don't block)
- Better code organization and maintainability
* add constant
* Fix IAM manager and post policy to use current active filer
* Fix critical race condition and goroutine leak
* Update weed/s3api/filer_multipart.go
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
* Fix compilation error and address code review suggestions
Address remaining unresolved comments:
1. **Fix compilation error**: Add missing net/url import
- filer_multipart.go used url.PathEscape without import
- Added "net/url" to imports
2. **Fix Location URL formatting** (all 4 occurrences):
- Add missing slash between bucket and key
- Use url.PathEscape for bucket names
- Use urlPathEscape for object keys
- Handles special characters in bucket/key names
- Before: http://host/bucketkey
- After: http://host/bucket/key (properly escaped)
3. **Optimize discovery loop** (O(N*M) → O(N+M)):
- Use map for existing filers (O(1) lookup)
- Reduces time holding write lock
- Better performance with many filers
- Before: Nested loop for each discovered filer
- After: Build map once, then O(1) lookups
Changes:
- filer_multipart.go: Import net/url, fix all Location URLs
- filer_client.go: Use map for efficient filer discovery
Benefits:
- Compiles successfully
- Proper URL encoding (handles spaces, special chars)
- Faster discovery with less lock contention
- Production-ready URL formatting
* Fix race conditions and make Close() idempotent
Address CodeRabbit review #3512078995:
1. **Critical: Fix unsynchronized read in error message**
- Line 584 read len(fc.filerAddresses) without lock
- Race with refreshFilerList appending to slice
- Fixed: Take RLock to read length safely
- Prevents race detector warnings
2. **Important: Make Close() idempotent**
- Closing already-closed channel panics
- Can happen with layered cleanup in shutdown paths
- Fixed: Use sync.Once to ensure single close
- Safe to call Close() multiple times now
3. **Nitpick: Add warning for empty filer address**
- getFilerAddress() can return empty string
- Helps diagnose unexpected state
- Added: Warning log when no filers available
4. **Nitpick: Guard deprecated index-based helpers**
- shouldSkipUnhealthyFiler, recordFilerSuccess/Failure
- Accessed filerHealth without lock (races with discovery)
- Fixed: Take RLock and check bounds before array access
- Prevents index out of bounds and races
Changes:
- filer_client.go:
- Add closeDiscoveryOnce sync.Once field
- Use Do() in Close() for idempotent channel close
- Add RLock guards to deprecated index-based helpers
- Add bounds checking to prevent panics
- Synchronized read of filerAddresses length in error
- s3api_server.go:
- Add warning log when getFilerAddress returns empty
Benefits:
- No race conditions (passes race detector)
- No panic on double-close
- Better error diagnostics
- Safe with discovery enabled
- Production-hardened shutdown logic
* Fix hardcoded http scheme and add panic recovery
Address CodeRabbit review #3512114811:
1. **Major: Fix hardcoded http:// scheme in Location URLs**
- Location URLs always used http:// regardless of client connection
- HTTPS clients got http:// URLs (incorrect)
- Fixed: Detect scheme from request
- Check X-Forwarded-Proto header (for proxies) first
- Check r.TLS != nil for direct HTTPS
- Fallback to http for plain connections
- Applied to all 4 CompleteMultipartUploadResult locations
2. **Major: Add panic recovery to discovery goroutine**
- Long-running background goroutine could crash entire process
- Panic in refreshFilerList would terminate program
- Fixed: Add defer recover() with error logging
- Goroutine failures now logged, not fatal
3. **Note: Close() idempotency already implemented**
- Review flagged as duplicate issue
- Already fixed in commit
|
2 months ago |
|
|
605b3333c1
|
fix sftp start with filer (#7120)
* fix sftp start with filer * add bindIp |
6 months ago |
|
|
8fa1a69f8c
|
* Fix undefined http serve behaiver (#6943)
|
7 months ago |
|
|
bd4891a117 |
change version directory
|
8 months ago |
|
|
93aed187e9
|
Add SFTP Server Support (#6753)
* Add SFTP Server Support Signed-off-by: Mohamed Sekour <mohamed.sekour@exfo.com> * fix s3 tests and helm lint Signed-off-by: Mohamed Sekour <mohamed.sekour@exfo.com> * increase helm chart version * adjust version --------- Signed-off-by: Mohamed Sekour <mohamed.sekour@exfo.com> Co-authored-by: chrislu <chris.lu@gmail.com> |
9 months ago |
|
|
a2c5510ae1
|
Addendum to issue #6733 (#6756)
add s3.ip.bind command line parameter to filer |
9 months ago |
|
|
d424cbbcd5
|
fix: S3 Feature: please add s3.idleTimeout command line parameter #6746 (#6747)
ildeTimeout command line parameter for s3 |
9 months ago |
|
|
aa9121aa9d
|
Revert "[filer] void panic if s3opt.tlsCACertificate is nill" (#6694)
Revert "[filer] void panic if s3opt.tlsCACertificate is nill (#6685)"
This reverts commit
|
10 months ago |
|
|
4dca59ed27
|
[filer] void panic if s3opt.tlsCACertificate is nill (#6685)
* fix issues/6684 * add s3.cacert.file option for filer |
10 months ago |
|
|
95746f48e6
|
feat/fix: add s3.cacert.file/tlsVerifyClientCert to filer command (#6659)
This prevent crash filler with nil pointer dereference as s3 expect this parameters. New two parameters are added to filer command - copy of s3 parameters: - s3.cacert.file - path to the TLS CA certificate file - s3.tlsVerifyClientCert - whether to verify the client's certificate |
11 months ago |
|
|
75ef324533
|
Allow standalone webdav server to bind specific address (#6403)
* Allow standalone webdav to bind specific address * Rename bindIp to ipBind * Bind embedded webdav IP to its parent |
11 months ago |
|
|
bf2366ba3a
|
Fix KeyMaterial nil bug (#5849)
|
2 years ago |
|
|
e1f9336688
|
[filer] fix panic filer on webdav.maxMB (#5822)
fix panic filer on webdav.maxMB |
2 years ago |
|
|
b796c21fa9
|
Added loadSecurityConfigOnce (#5792)
|
2 years ago |
|
|
ec9e7493b3
|
-metricsIp cmd flag (#5773)
* Added/Updated: - Added metrics ip options for all servers; - Fixed a bug with the selection of the binIp or ip parameter for the metrics handler; * Fixed cmd flags |
2 years ago |
|
|
de60f383de
|
Added tls for http handlers (#5764)
* Added https handler for filer * Added example for security.toml |
2 years ago |
|
|
ac6fd36c06 |
using fixed bootstrap from peer filer
|
2 years ago |
|
|
1caa012a76
|
Fix Filer Sync Issue: 5455 (#5663)
|
2 years ago |
|
|
85d6d5371b
|
Disable filer UI in configuration (#5297)
* Add filer.ui.enabled configuration property * Add filer.expose_directory_metadata to config * Ammend commit * Remove ShowUI reference * Update all routes that allow directory metadata * Add cmd flag to server.go |
2 years ago |
|
|
d3688938d9 |
avoid nil
fix https://github.com/seaweedfs/seaweedfs/issues/5153 introduced from #5109 |
2 years ago |
|
|
06343f8976
|
Set allowed origins in config (#5109)
* Add a way to use a JWT in an HTTP only cookie If a JWT is not included in the Authorization header or a query string, attempt to get a JWT from an HTTP only cookie. * Added a way to specify allowed origins header from config * Removed unecessary log * Check list of domains from config or command flag * Handle default wildcard and change name of config value to cors |
2 years ago |
|
|
4ed06e9ba5
|
Pass correct Filer's gRPC port to S3 server. (#4972)
Fixes seaweedfs/seaweedfs#4971 |
2 years ago |
|
|
796b7508f3
|
Implement SRV lookups for filer (#4767)
|
2 years ago |
|
|
f0d3c874b6
|
Unix Socket listener for S3 server (#4621)
Add localSocket option to s3 server |
3 years ago |
|
|
4dd890d4a2
|
optional https port for s3 (#4482)
Co-authored-by: Konstantin Lebedev <9497591+kmlebedev@users.noreply.github.co> |
3 years ago |
|
|
1976ca9160
|
add -disk to filer command (#4247)
* add -disk to filer command * add diskType to filer.grpc * use filer.disk when filerWebDavOptions.disk is empty * add filer.disk to weed server command. --------- Co-authored-by: 三千院羽 <3000y@MacBook-Pro.lan> |
3 years ago |
|
|
469c959e6a |
add missing options for webdav
|
3 years ago |
|
|
4b0430e71d
|
[metrics] Add the ability to control bind ip (#4012)
|
3 years ago |
|
|
8efe1db01a
|
refactor(various): `Listner` -> `Listener` readability improvements (#3672)
* refactor(net_timeout): `listner` -> `listener` Signed-off-by: Ryan Russell <git@ryanrussell.org> * refactor(s3): `s3ApiLocalListner` -> `s3ApiLocalListener` Signed-off-by: Ryan Russell <git@ryanrussell.org> * refactor(filer): `localPublicListner` -> `localPublicListener` Signed-off-by: Ryan Russell <git@ryanrussell.org> * refactor(command): `masterLocalListner` -> `masterLocalListener` Signed-off-by: Ryan Russell <git@ryanrussell.org> * refactor(net_timeout): `ipListner` -> `ipListener` Signed-off-by: Ryan Russell <git@ryanrussell.org> Signed-off-by: Ryan Russell <git@ryanrussell.org> |
3 years ago |
|
|
19652c1b83
|
refactor(socket mount): Update socket mount pattern to `/tmp/seaweedf… (#3662)
|
3 years ago |
|
|
8c3040db81
|
avoid DATA RACE on S3Options.localFilerSocket (#3571)
* avoid DATA RACE on S3Options.localFilerSocket https://github.com/seaweedfs/seaweedfs/issues/3552 * copy localSocket |
3 years ago |
|
|
903a06a160
|
avoid data race access to startDelay (#3548)
|
3 years ago |
|
|
84ec68e11a
|
Add download speed limit support (#3408)
|
4 years ago |
|
|
4d08393b7c
|
filer prefer volume server in same data center (#3405)
* initial prefer same data center https://github.com/seaweedfs/seaweedfs/issues/3404 * GetDataCenter * prefer same data center for ReplicationSource * GetDataCenterId * remove glog |
4 years ago |
|
|
26dbc6c905 |
move to https://github.com/seaweedfs/seaweedfs
|
4 years ago |
|
|
af23e63e3f |
Improve filer command help, add supported filer store list
|
4 years ago |
|
|
b90d213a87 |
Change ui.deleteDir default to true
|
4 years ago |