mirrors/seaweedfs
1
0
Fork 0
mirror of https://github.com/seaweedfs/seaweedfs.git synced 2026-08-16 04:06:44 +00:00
Code Issues Packages Projects Releases Wiki Activity

s3: report the effective ownership when a bucket has none stored (#10591)

Browse Source 
* s3: report the effective ownership when a bucket has none stored

GetBucketOwnershipControls read Seaweed-X-Amz-Ownership straight out of the
bucket entry, so a bucket that never had one written reported an empty
ObjectOwnership. The object write path defaults the same missing attribute to
BucketOwnerEnforced, so the API contradicted the behavior it describes.

Resolve the stored value through one helper both readers share, and let
PutBucketOwnershipControls persist unconditionally so setting the default
value still gives DeleteBucketOwnershipControls something to remove.

* test: cover the bucket ownership controls round trip

Pins the behaviors the ownership default fix depends on: a bucket that never
had ownership controls written reports BucketOwnerEnforced, and putting that
same value on such a bucket still persists it, so the delete that follows has
something to remove. The put-then-delete case gets its own bucket -- run after
an ObjectWriter put, it would pass against an implementation that skips only
the initial write.

The acl workflow already runs this package against a live weed mini, so it
needs no wiring.
This commit is contained in:
Chris Lu
2026-08-05 13:15:28 -07:00
committed by GitHub
parent 69aa6d7adc
commit f09bc14165
7 changed files with 156 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files
test/s3/acl/s3_ownership_controls_test.go
+93
View File
@@ -0,0 +1,93 @@
package acl
import (
"context"
"strings"
"testing"
"time"
"github.com/aws/aws-sdk-go-v2/aws"
"github.com/aws/aws-sdk-go-v2/service/s3"
"github.com/aws/aws-sdk-go-v2/service/s3/types"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func createOwnershipTestBucket(t *testing.T, client *s3.Client) string {
bucketName := "test-ownership-" + strings.ToLower(strings.ReplaceAll(time.Now().Format("2006-01-02-15-04-05.000"), ":", "-"))
_, err := client.CreateBucket(context.TODO(), &s3.CreateBucketInput{
Bucket: aws.String(bucketName),
})
require.NoError(t, err)
return bucketName
}
func getObjectOwnership(t *testing.T, client *s3.Client, bucketName string) types.ObjectOwnership {
t.Helper()
resp, err := client.GetBucketOwnershipControls(context.TODO(), &s3.GetBucketOwnershipControlsInput{
Bucket: aws.String(bucketName),
})
require.NoError(t, err)
require.NotNil(t, resp.OwnershipControls)
require.Len(t, resp.OwnershipControls.Rules, 1)
return resp.OwnershipControls.Rules[0].ObjectOwnership
}
func putObjectOwnership(t *testing.T, client *s3.Client, bucketName string, ownership types.ObjectOwnership) {
t.Helper()
_, err := client.PutBucketOwnershipControls(context.TODO(), &s3.PutBucketOwnershipControlsInput{
Bucket: aws.String(bucketName),
OwnershipControls: &types.OwnershipControls{
Rules: []types.OwnershipControlsRule{{ObjectOwnership: ownership}},
},
})
require.NoError(t, err)
}
func TestGetBucketOwnershipControlsDefault(t *testing.T) {
client := getS3Client(t)
bucketName := createOwnershipTestBucket(t, client)
defer cleanupTestBucket(t, client, bucketName)
assert.Equal(t, types.ObjectOwnershipBucketOwnerEnforced, getObjectOwnership(t, client, bucketName))
}
func TestBucketOwnershipControlsLifecycle(t *testing.T) {
client := getS3Client(t)
bucketName := createOwnershipTestBucket(t, client)
defer cleanupTestBucket(t, client, bucketName)
putObjectOwnership(t, client, bucketName, types.ObjectOwnershipObjectWriter)
assert.Equal(t, types.ObjectOwnershipObjectWriter, getObjectOwnership(t, client, bucketName))
putObjectOwnership(t, client, bucketName, types.ObjectOwnershipBucketOwnerEnforced)
assert.Equal(t, types.ObjectOwnershipBucketOwnerEnforced, getObjectOwnership(t, client, bucketName))
_, err := client.DeleteBucketOwnershipControls(context.TODO(), &s3.DeleteBucketOwnershipControlsInput{
Bucket: aws.String(bucketName),
})
require.NoError(t, err)
assert.Equal(t, types.ObjectOwnershipBucketOwnerEnforced, getObjectOwnership(t, client, bucketName))
}
// A bucket with nothing stored already reports BucketOwnerEnforced, so this put has
// to persist anyway or the delete below finds nothing to remove.
func TestPutBucketOwnershipControlsDefaultOnNewBucket(t *testing.T) {
client := getS3Client(t)
bucketName := createOwnershipTestBucket(t, client)
defer cleanupTestBucket(t, client, bucketName)
putObjectOwnership(t, client, bucketName, types.ObjectOwnershipBucketOwnerEnforced)
_, err := client.DeleteBucketOwnershipControls(context.TODO(), &s3.DeleteBucketOwnershipControlsInput{
Bucket: aws.String(bucketName),
})
require.NoError(t, err)
_, err = client.DeleteBucketOwnershipControls(context.TODO(), &s3.DeleteBucketOwnershipControlsInput{
Bucket: aws.String(bucketName),
})
require.Error(t, err)
assert.Contains(t, err.Error(), "OwnershipControlsNotFound")
}
weed/s3api/bucket_metadata.go
+5 -9
View File
@@ -101,7 +101,7 @@ func buildBucketMetadata(accountManager AccountManager, entry *filer_pb.Entry) *
IsTableBucket: s3tables.IsTableBucketEntry(entry),
//Default ownership: OwnershipBucketOwnerEnforced, which means Acl is disabled
ObjectOwnership: s3_constants.OwnershipBucketOwnerEnforced,
ObjectOwnership: s3_constants.DefaultOwnershipForExists,
// Default owner: `AccountAdmin`
Owner: &s3.Owner{
@@ -111,15 +111,11 @@ func buildBucketMetadata(accountManager AccountManager, entry *filer_pb.Entry) *
}
if entry.Extended != nil {
//ownership control
ownership, ok := entry.Extended[s3_constants.ExtOwnershipKey]
if ok {
ownership := string(ownership)
valid := s3_constants.ValidateOwnership(ownership)
if valid {
bucketMetadata.ObjectOwnership = ownership
} else {
glog.Warningf("Invalid ownership: %s, bucket: %s", ownership, bucketMetadata.Name)
if ownership, ok := entry.Extended[s3_constants.ExtOwnershipKey]; ok {
if !s3_constants.ValidateOwnership(string(ownership)) {
glog.Warningf("Invalid ownership: %s, bucket: %s", string(ownership), bucketMetadata.Name)
}
bucketMetadata.ObjectOwnership = s3_constants.EffectiveOwnership(string(ownership))
}
//access control policy
weed/s3api/s3_constants/acp_ownership.go
+9
View File
@@ -16,3 +16,12 @@ func ValidateOwnership(ownership string) bool {
return true
}
}
// EffectiveOwnership resolves a stored ownership setting to the one that governs
// the bucket: absent or invalid behaves as BucketOwnerEnforced.
func EffectiveOwnership(ownership string) string {
if !ValidateOwnership(ownership) {
return DefaultOwnershipForExists
}
return ownership
}
weed/s3api/s3_constants/acp_ownership_test.go
+24
View File
@@ -0,0 +1,24 @@
package s3_constants
import "testing"
func TestEffectiveOwnership(t *testing.T) {
cases := []struct {
name string
input string
want string
}{
{name: "unset", input: "", want: OwnershipBucketOwnerEnforced},
{name: "invalid", input: "Bogus", want: OwnershipBucketOwnerEnforced},
{name: "object writer", input: OwnershipObjectWriter, want: OwnershipObjectWriter},
{name: "bucket owner preferred", input: OwnershipBucketOwnerPreferred, want: OwnershipBucketOwnerPreferred},
{name: "bucket owner enforced", input: OwnershipBucketOwnerEnforced, want: OwnershipBucketOwnerEnforced},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
if got := EffectiveOwnership(tc.input); got != tc.want {
t.Fatalf("EffectiveOwnership(%q) = %q, want %q", tc.input, got, tc.want)
}
})
}
}
weed/s3api/s3api_bucket_config.go
+1 -1
View File
@@ -696,7 +696,7 @@ func (s3a *S3ApiServer) getBucketOwnership(bucket string) (string, s3err.ErrorCo
return "", errCode
}
return config.Ownership, s3err.ErrNone
return s3_constants.EffectiveOwnership(config.Ownership), s3err.ErrNone
}
// setBucketOwnership sets the ownership setting for a bucket
weed/s3api/s3api_bucket_handlers.go
+2 -11
View File
@@ -1276,21 +1276,12 @@ func (s3a *S3ApiServer) PutBucketOwnershipControls(w http.ResponseWriter, r *htt
return
}
// Check if ownership needs to be updated
currentOwnership, errCode := s3a.getBucketOwnership(bucket)
if errCode != s3err.ErrNone {
// Persist even when it matches the implicit default, so a later delete has something to remove.
if errCode := s3a.setBucketOwnership(bucket, ownership); errCode != s3err.ErrNone {
s3err.WriteErrorResponse(w, r, errCode)
return
}
if currentOwnership != ownership {
errCode = s3a.setBucketOwnership(bucket, ownership)
if errCode != s3err.ErrNone {
s3err.WriteErrorResponse(w, r, errCode)
return
}
}
if printOwnership {
result := &s3.PutBucketOwnershipControlsInput{
OwnershipControls: &v,
weed/s3api/s3api_bucket_handlers_misc_test.go
+22
View File
@@ -152,6 +152,28 @@ func TestPutBucketOwnershipControlsRejectsRuleWithoutObjectOwnership(t *testing.
}
}
func TestGetBucketOwnershipControlsDefaultsToBucketOwnerEnforced(t *testing.T) {
ownerID := AccountAdmin.Id
s3a := newMiscTestServer(t, "b")
s3a.bucketRegistry = NewBucketRegistry(nil)
s3a.bucketRegistry.setMetadataCache(&BucketMetaData{
Name: "b",
Owner: &s3.Owner{ID: &ownerID},
})
req := newBucketRequest(http.MethodGet, "b", "ownershipControls=", "")
req.Header.Set(s3_constants.AmzAccountId, AccountAdmin.Id)
rec := httptest.NewRecorder()
s3a.GetBucketOwnershipControls(rec, req)
if rec.Code != http.StatusOK {
t.Fatalf("status = %d, want %d, body=%s", rec.Code, http.StatusOK, rec.Body.String())
}
if !strings.Contains(rec.Body.String(), "<ObjectOwnership>"+s3_constants.OwnershipBucketOwnerEnforced+"</ObjectOwnership>") {
t.Fatalf("body missing default ownership: %s", rec.Body.String())
}
}
func TestGetBucketAccelerateConfiguration(t *testing.T) {
s3a := newMiscTestServer(t, "b")
req := newBucketRequest(http.MethodGet, "b", "accelerate=", "")