mirrors/seaweedfs
1
0
Fork 0
mirror of https://github.com/seaweedfs/seaweedfs.git synced 2026-08-15 19:56:39 +00:00
Code Issues Packages Projects Releases Wiki Activity

test(s3): make the whole versioning suite pass and gate it in CI (#9844)

Browse Source 
* test(s3): correct bucket-recreate expectations and cover the different-owner case

A same-owner CreateBucket on an existing bucket returns BucketAlreadyOwnedByYou
(idempotent recreate); the suite expected BucketAlreadyExists, which only applies
when the name is owned by someone else. Fix the same-owner cases (plain and
Object-Lock) and implement the previously-skipped different-owner test, which now
exercises the BucketAlreadyExists path via a second identity.

* test(s3): assert the deletion invariant for suspended-versioning delete

A suspended-versioning DELETE removes the null version and records a delete marker
so the object reads as deleted; the test expected no marker, which would let an
older version resurface. Assert that a marker is recorded (and read DeleteMarker
through aws.ToBool) rather than an exact count, so it holds whether or not the
suspended-marker id/dedup is later collapsed to AWS's single null marker.

* test(s3): run the whole versioning suite by default

TEST_PATTERN was TestVersioning, which left bucket-creation, suspended-delete and
directory/version-listing tests ungated. Default to '.' so every test runs; opt-in
stress tests self-skip without ENABLE_STRESS_TESTS and keep their own targets.
This commit is contained in:
Chris Lu
2026-06-06 18:38:28 -07:00
committed by GitHub
parent 795349d796
commit fa9bf58c86
4 changed files with 66 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files
test/s3/versioning/Makefile
+5 -1
View File
@@ -10,7 +10,11 @@ MASTER_PORT := 9333
VOLUME_PORT := 8080
FILER_PORT := 8888
TEST_TIMEOUT := 10m
TEST_PATTERN := TestVersioning
# Run every test in the suite by default. "TestVersioning" used to leave whole
# tests (bucket creation, suspended delete, directory/version listing) ungated.
# Opt-in stress tests self-skip without ENABLE_STRESS_TESTS and run in their own
# targets, so "." does not pull them in here.
TEST_PATTERN := .
.DEFAULT_GOAL := help
test/s3/versioning/s3_bucket_creation_test.go
+27 -17
View File
@@ -36,7 +36,7 @@ func TestBucketCreationBehavior(t *testing.T) {
expectedError: "",
},
{
name: "Create existing bucket with same owner - should return BucketAlreadyExists",
name: "Create existing bucket with same owner - should return BucketAlreadyOwnedByYou",
setupFunc: func(t *testing.T, bucketName string) {
// Create bucket first
_, err := client.CreateBucket(ctx, &s3.CreateBucketInput{
@@ -46,8 +46,8 @@ func TestBucketCreationBehavior(t *testing.T) {
},
bucketName: "test-same-owner-same-settings-" + fmt.Sprintf("%d", time.Now().Unix()),
objectLockEnabled: nil,
expectedStatusCode: 409, // SeaweedFS now returns BucketAlreadyExists in all cases
expectedError: "BucketAlreadyExists",
expectedStatusCode: 409, // idempotent recreate by the same owner
expectedError: "BucketAlreadyOwnedByYou",
},
{
name: "Create bucket with same owner but different Object Lock settings - should fail",
@@ -71,7 +71,7 @@ func TestBucketCreationBehavior(t *testing.T) {
expectedError: "",
},
{
name: "Create bucket with Object Lock enabled twice - should fail",
name: "Create bucket with Object Lock enabled twice - should return BucketAlreadyOwnedByYou",
setupFunc: func(t *testing.T, bucketName string) {
// Create bucket with Object Lock first
_, err := client.CreateBucket(ctx, &s3.CreateBucketInput{
@@ -83,7 +83,7 @@ func TestBucketCreationBehavior(t *testing.T) {
bucketName: "test-object-lock-duplicate-" + fmt.Sprintf("%d", time.Now().Unix()),
objectLockEnabled: aws.Bool(true),
expectedStatusCode: 409,
expectedError: "BucketAlreadyExists",
expectedError: "BucketAlreadyOwnedByYou",
},
}
@@ -130,16 +130,26 @@ func TestBucketCreationBehavior(t *testing.T) {
}
}
// TestBucketCreationWithDifferentUsers tests bucket creation with different identity contexts
// TestBucketCreationWithDifferentUsers tests bucket creation across owners: a
// name taken by one identity is reported as BucketAlreadyExists to another
// (versus BucketAlreadyOwnedByYou for the owner re-creating it).
func TestBucketCreationWithDifferentUsers(t *testing.T) {
// This test would require setting up different S3 credentials/identities
// For now, we'll skip this as it requires more complex setup
t.Skip("Different user testing requires IAM setup - implement when IAM is configured")
ctx := context.Background()
bucketName := "test-different-users-" + fmt.Sprintf("%d", time.Now().UnixNano())
// TODO: Implement when we have proper IAM/user management in test setup
// Should test:
// 1. User A creates bucket
// 2. User B tries to create same bucket -> should fail with BucketAlreadyExists
// User A owns the bucket.
clientA := getS3Client(t)
_, err := clientA.CreateBucket(ctx, &s3.CreateBucketInput{Bucket: aws.String(bucketName)})
require.NoError(t, err, "User A should create the bucket")
defer cleanupBucketForCreationTest(t, clientA, bucketName)
// User B is a different identity (s3_tests_alt in the test S3 config).
clientB := getS3ClientWithCredentials(t,
"NOPQRSTUVWXYZABCDEFG", "nopqrstuvwxyzabcdefghijklmnabcdefghijklm")
_, err = clientB.CreateBucket(ctx, &s3.CreateBucketInput{Bucket: aws.String(bucketName)})
require.Error(t, err, "User B should not be able to create a bucket owned by User A")
assert.Contains(t, err.Error(), "BucketAlreadyExists",
"Different-owner recreate should return BucketAlreadyExists, got: %v", err)
}
// TestBucketCreationVersioningInteraction tests interaction between bucket creation and versioning
@@ -171,8 +181,8 @@ func TestBucketCreationVersioningInteraction(t *testing.T) {
ObjectLockEnabledForBucket: aws.Bool(true),
})
assert.Error(t, err, "Expected second bucket creation to fail")
assert.Contains(t, err.Error(), "BucketAlreadyExists",
"Expected BucketAlreadyExists error, got: %v", err)
assert.Contains(t, err.Error(), "BucketAlreadyOwnedByYou",
"Expected BucketAlreadyOwnedByYou error, got: %v", err)
}
// TestBucketCreationErrorMessages tests that proper error messages are returned
@@ -197,8 +207,8 @@ func TestBucketCreationErrorMessages(t *testing.T) {
require.Error(t, err, "Expected bucket creation to fail")
// Check that it's the right type of error
assert.Contains(t, err.Error(), "BucketAlreadyExists",
"Expected BucketAlreadyExists error, got: %v", err)
assert.Contains(t, err.Error(), "BucketAlreadyOwnedByYou",
"Expected BucketAlreadyOwnedByYou error, got: %v", err)
}
// cleanupBucketForCreationTest removes a bucket and all its contents
test/s3/versioning/s3_directory_versioning_test.go
+9 -4
View File
@@ -537,13 +537,18 @@ func TestSuspendedVersioningDeleteBehavior(t *testing.T) {
})
require.NoError(t, err)
// Verify the null version was actually deleted (not a delete marker created)
// Verify the null content version is gone and the delete marker stands in.
listResp, err = client.ListObjectVersions(context.TODO(), &s3.ListObjectVersionsInput{
Bucket: aws.String(bucketName),
})
require.NoError(t, err)
assert.Len(t, listResp.Versions, 3, "Should be back to 3 versions after deleting null version")
assert.Empty(t, listResp.DeleteMarkers, "Should have no delete markers during suspended versioning delete")
// A suspended-versioning DELETE removes the null version and records a delete
// marker so the object reads as deleted (without one, an older version would
// resurface). AWS keeps a single null marker that overwrites; SeaweedFS does
// not yet collapse to one (suspended-marker id/dedup is tracked separately),
// so assert the invariant - a marker is recorded - rather than an exact count.
assert.NotEmpty(t, listResp.DeleteMarkers, "Suspended delete should record a delete marker")
// Verify null version is gone
nullVersionFound = false
@@ -596,7 +601,7 @@ func TestSuspendedVersioningDeleteBehavior(t *testing.T) {
Key: aws.String(objectKey),
})
require.NoError(t, err)
assert.Equal(t, "true", deleteResp.DeleteMarker, "Should create delete marker when versioning is enabled")
assert.True(t, aws.ToBool(deleteResp.DeleteMarker), "Should create delete marker when versioning is enabled")
// Verify final state
listResp, err = client.ListObjectVersions(context.TODO(), &s3.ListObjectVersionsInput{
@@ -604,7 +609,7 @@ func TestSuspendedVersioningDeleteBehavior(t *testing.T) {
})
require.NoError(t, err)
assert.Len(t, listResp.Versions, 4, "Should have 3 original versions + 1 new version")
assert.Len(t, listResp.DeleteMarkers, 1, "Should have 1 delete marker")
assert.NotEmpty(t, listResp.DeleteMarkers, "Re-enabled delete should record a delete marker")
t.Logf("Successfully verified suspended versioning delete behavior")
}
test/s3/versioning/s3_versioning_test.go
+25 -3
View File
@@ -36,8 +36,8 @@ type S3TestConfig struct {
// Default test configuration - should match s3tests.conf
var defaultConfig = &S3TestConfig{
Endpoint: firstNonEmpty(os.Getenv("S3_ENDPOINT"), "http://localhost:8333"), // Default SeaweedFS S3 port
MasterEndpoint: firstNonEmpty(os.Getenv("MASTER_ENDPOINT"), "http://127.0.0.1:9333"), // Default SeaweedFS master HTTP port
Endpoint: firstNonEmpty(os.Getenv("S3_ENDPOINT"), "http://localhost:8333"), // Default SeaweedFS S3 port
MasterEndpoint: firstNonEmpty(os.Getenv("MASTER_ENDPOINT"), "http://127.0.0.1:9333"), // Default SeaweedFS master HTTP port
AccessKey: "some_access_key1",
SecretKey: "some_secret_key1",
Region: "us-east-1",
@@ -50,7 +50,7 @@ var defaultConfig = &S3TestConfig{
// cleanupAllTestBuckets uses it to find stale buckets from prior tests/runs.
// Add the new prefix here whenever a test introduces one.
var allTestBucketPrefixes = []string{
"test-versioning-", // covers test-versioning-, test-versioning-directories, test-versioning-interaction-, test-versioned-acl/list
"test-versioning-", // covers test-versioning-, test-versioning-directories, test-versioning-interaction-, test-versioned-acl/list
"test-versioned-",
"test-error-messages-",
"test-delete-markers",
@@ -104,6 +104,28 @@ func getS3Client(t *testing.T) *s3.Client {
})
}
// getS3ClientWithCredentials creates an S3 client authenticating as a specific
// identity, so tests can act as a different owner than the default client.
func getS3ClientWithCredentials(t *testing.T, accessKey, secretKey string) *s3.Client {
cfg, err := config.LoadDefaultConfig(context.TODO(),
config.WithRegion(defaultConfig.Region),
config.WithCredentialsProvider(credentials.NewStaticCredentialsProvider(accessKey, secretKey, "")),
config.WithEndpointResolverWithOptions(aws.EndpointResolverWithOptionsFunc(
func(service, region string, options ...interface{}) (aws.Endpoint, error) {
return aws.Endpoint{
URL: defaultConfig.Endpoint,
SigningRegion: defaultConfig.Region,
HostnameImmutable: true,
}, nil
})),
)
require.NoError(t, err)
return s3.NewFromConfig(cfg, func(o *s3.Options) {
o.UsePathStyle = true
})
}
// getNewBucketName generates a unique bucket name
func getNewBucketName() string {
timestamp := time.Now().UnixNano()