Browse Source

address comments

pull/7160/head
chrislu 2 months ago
parent
commit
9d4d131043
  1. 7
      test/s3/iam/DISTRIBUTED.md
  2. 7
      test/s3/iam/STS_DISTRIBUTED.md
  3. 39
      test/s3/iam/s3_iam_distributed_test.go
  4. 4
      weed/s3api/s3_multipart_iam.go
  5. 2
      weed/s3api/s3_multipart_iam_test.go

7
test/s3/iam/DISTRIBUTED.md

@ -227,8 +227,8 @@ time weed filer.cat /seaweedfs/iam/roles/TestRole.json
- **Implement alerts** for IAM storage issues - **Implement alerts** for IAM storage issues
### Capacity Planning ### Capacity Planning
- **Estimate IAM data size**: Roles + Policies + Sessions
- **Plan for growth**: Active sessions scale with user count
- **Estimate IAM data size**: Roles + Policies (no sessions - stateless JWT)
- **Plan for growth**: Role and policy count scales with user/application count
- **Monitor filer disk usage**: `/seaweedfs/iam/` path - **Monitor filer disk usage**: `/seaweedfs/iam/` path
- **Set up log rotation**: For IAM audit logs - **Set up log rotation**: For IAM audit logs
@ -264,8 +264,7 @@ time weed filer.cat /seaweedfs/iam/roles/TestRole.json
│ │ /seaweedfs/ │ │ │ │ /seaweedfs/ │ │
│ │ iam/ │ │ │ │ iam/ │ │
│ │ ├─roles/ │ │ │ │ ├─roles/ │ │
│ │ ├─policies/ │ │
│ │ └─sessions/ │ │
│ │ └─policies/ │ │
│ └───────────────┘ │ │ └───────────────┘ │
└───────────────────┘ └───────────────────┘

7
test/s3/iam/STS_DISTRIBUTED.md

@ -289,11 +289,8 @@ User Request → Load Balancer → Any S3 Gateway Instance
Extract JWT Session Token Extract JWT Session Token
Validate with TokenGenerator
(Same signing key on all instances)
Retrieve Session from Filer
(Shared session store)
Validate JWT Token
(Self-contained - no external storage needed)
Check Permissions Check Permissions
(Shared policy engine) (Shared policy engine)

39
test/s3/iam/s3_iam_distributed_test.go

@ -178,16 +178,16 @@ func TestS3IAMDistributedTests(t *testing.T) {
var errorList []error var errorList []error
var transientErrors []error var transientErrors []error
var seriousErrors []error var seriousErrors []error
for err := range errors { for err := range errors {
errorList = append(errorList, err) errorList = append(errorList, err)
errorMsg := err.Error() errorMsg := err.Error()
// Categorize errors: transient vs serious // Categorize errors: transient vs serious
if strings.Contains(errorMsg, "timeout") ||
strings.Contains(errorMsg, "connection reset") ||
strings.Contains(errorMsg, "temporary failure") ||
strings.Contains(errorMsg, "TooManyRequests") {
if strings.Contains(errorMsg, "timeout") ||
strings.Contains(errorMsg, "connection reset") ||
strings.Contains(errorMsg, "temporary failure") ||
strings.Contains(errorMsg, "TooManyRequests") {
transientErrors = append(transientErrors, err) transientErrors = append(transientErrors, err)
} else { } else {
seriousErrors = append(seriousErrors, err) seriousErrors = append(seriousErrors, err)
@ -206,7 +206,7 @@ func TestS3IAMDistributedTests(t *testing.T) {
t.Logf(" Failed operations: %d (%.1f%% error rate)", len(errorList), errorRate*100) t.Logf(" Failed operations: %d (%.1f%% error rate)", len(errorList), errorRate*100)
t.Logf(" Serious errors: %d (%.1f%% rate)", len(seriousErrors), seriousErrorRate*100) t.Logf(" Serious errors: %d (%.1f%% rate)", len(seriousErrors), seriousErrorRate*100)
t.Logf(" Transient errors: %d (%.1f%% rate)", len(transientErrors), transientErrorRate*100) t.Logf(" Transient errors: %d (%.1f%% rate)", len(transientErrors), transientErrorRate*100)
if len(seriousErrors) > 0 { if len(seriousErrors) > 0 {
t.Logf(" First serious error: %v", seriousErrors[0]) t.Logf(" First serious error: %v", seriousErrors[0])
} }
@ -215,14 +215,25 @@ func TestS3IAMDistributedTests(t *testing.T) {
} }
} }
// STRICT CONCURRENCY TESTING: Use much lower error rate thresholds
// Serious errors (race conditions, deadlocks, etc.) should be near zero
if seriousErrorRate > 0.01 { // Max 1% serious error rate
t.Errorf("❌ Serious error rate too high: %.1f%% (>1%%). This indicates potential race conditions, deadlocks, or other concurrency bugs that must be fixed.", seriousErrorRate*100)
} else if errorRate > 0.05 { // Max 5% total error rate (including transient)
t.Errorf("❌ Total error rate too high: %.1f%% (>5%%). While some transient errors are acceptable, this rate suggests system instability under concurrent load.", errorRate*100)
// STRICT CONCURRENCY TESTING: Use appropriate thresholds for the operation count
// For totalOperations=6, error thresholds need to be adjusted to avoid unreachable conditions
// Serious errors (race conditions, deadlocks) should be zero or very rare
maxSeriousErrors := 1 // Allow 1 serious error max (16.7%) for small test size
if len(seriousErrors) > maxSeriousErrors {
t.Errorf("❌ Too many serious errors: %d (%.1f%%) - max allowed: %d. This indicates potential race conditions, deadlocks, or other concurrency bugs.",
len(seriousErrors), seriousErrorRate*100, maxSeriousErrors)
} else if len(seriousErrors) > 0 {
t.Logf("⚠️ %d serious error detected (%.1f%%) - within threshold but should be investigated", len(seriousErrors), seriousErrorRate*100)
}
// For total errors, allow more flexibility with small operation counts
maxTotalErrors := 2 // Allow 2 total errors max (33.3%) for small test size
if len(errorList) > maxTotalErrors {
t.Errorf("❌ Too many total errors: %d (%.1f%%) - max allowed: %d. This suggests system instability under concurrent load.",
len(errorList), errorRate*100, maxTotalErrors)
} else if len(errorList) > 0 { } else if len(errorList) > 0 {
t.Logf("⚠️ Concurrent operations completed with acceptable error rate: %.1f%% (mostly transient errors)", errorRate*100)
t.Logf("⚠️ Concurrent operations completed with %d errors (%.1f%%) - within acceptable range for testing", len(errorList), errorRate*100)
} else { } else {
t.Logf("✅ All concurrent operations completed successfully - excellent concurrency handling!") t.Logf("✅ All concurrent operations completed successfully - excellent concurrency handling!")
} }

4
weed/s3api/s3_multipart_iam.go

@ -289,7 +289,9 @@ func determineMultipartS3Action(operation MultipartOperation) Action {
case MultipartOpListParts: case MultipartOpListParts:
return s3_constants.ACTION_LIST_PARTS return s3_constants.ACTION_LIST_PARTS
default: default:
return s3_constants.ACTION_READ // Default fallback
// Fail closed for unmapped operations to prevent unintended access
glog.Errorf("unmapped multipart operation: %s", operation)
return "s3:InternalErrorUnknownMultipartAction" // Non-existent action ensures denial
} }
} }

2
weed/s3api/s3_multipart_iam_test.go

@ -266,7 +266,7 @@ func TestMultipartS3ActionMapping(t *testing.T) {
{MultipartOpAbort, s3_constants.ACTION_ABORT_MULTIPART}, {MultipartOpAbort, s3_constants.ACTION_ABORT_MULTIPART},
{MultipartOpList, s3_constants.ACTION_LIST_MULTIPART_UPLOADS}, {MultipartOpList, s3_constants.ACTION_LIST_MULTIPART_UPLOADS},
{MultipartOpListParts, s3_constants.ACTION_LIST_PARTS}, {MultipartOpListParts, s3_constants.ACTION_LIST_PARTS},
{MultipartOperation("unknown"), s3_constants.ACTION_READ}, // Default fallback
{MultipartOperation("unknown"), "s3:InternalErrorUnknownMultipartAction"}, // Fail-closed for security
} }
for _, tt := range tests { for _, tt := range tests {

Loading…
Cancel
Save