feedc0de/my-rook-config
1
0
Fork 0
forked from rook/rook
Code Pull Requests Activity

object: fix user quotas being overwritten when obc bucketOwner is set

Browse Source 
Resolves https://github.com/rook/rook/issues/16417

Signed-off-by: Joshua Hoblitt <josh@hoblitt.com>
This commit is contained in:
Joshua Hoblitt
2025-11-06 16:31:12 -07:00
parent 7fe438b5cb
commit 61cbe4fc23
3 changed files with 75 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/operator/ceph/object/bucket/provisioner.go
+27 -13
View File
@@ -151,7 +151,7 @@ func (p Provisioner) Provision(options *apibkt.BucketOptions) (*bktv1alpha1.Obje
logger.Infof("set user %q bucket max to %d", p.cephUserName, singleBucketQuota)
}
err = p.setAdditionalSettings(additionalConfig)
err = p.setAdditionalSettings(bucket)
if err != nil {
return nil, errors.Wrapf(err, "failed to set additional settings for OBC %q in NS %q associated with CephObjectStore %q in NS %q", options.ObjectBucketClaim.Name, options.ObjectBucketClaim.Namespace, p.objectStoreName, p.clusterInfo.Namespace)
}
@@ -205,7 +205,7 @@ func (p Provisioner) Grant(options *apibkt.BucketOptions) (*bktv1alpha1.ObjectBu
}
// setting quota limit if it is enabled
err = p.setAdditionalSettings(additionalConfig)
err = p.setAdditionalSettings(bucket)
if err != nil {
return nil, errors.Wrapf(err, "failed to set additional settings for OBC %q in NS %q associated with CephObjectStore %q in NS %q", options.ObjectBucketClaim.Name, options.ObjectBucketClaim.Namespace, p.objectStoreName, p.clusterInfo.Namespace)
}
@@ -609,23 +609,23 @@ func (p *Provisioner) populateDomainAndPort(sc *storagev1.StorageClass) error {
}
// Check for additional options mentioned in OBC and set them accordingly
func (p *Provisioner) setAdditionalSettings(additionalConfig *additionalConfigSpec) error {
err := p.setUserQuota(additionalConfig)
func (p *Provisioner) setAdditionalSettings(bucket *bucket) error {
err := p.setUserQuota(bucket)
if err != nil {
return errors.Wrap(err, "failed to set user quota")
}
err = p.setBucketQuota(additionalConfig)
err = p.setBucketQuota(bucket)
if err != nil {
return errors.Wrap(err, "failed to set bucket quota")
}
err = p.setBucketPolicy(additionalConfig)
err = p.setBucketPolicy(bucket)
if err != nil {
return errors.Wrap(err, "failed to set bucket policy")
}
err = p.setBucketLifecycle(additionalConfig)
err = p.setBucketLifecycle(bucket)
if err != nil {
return errors.Wrap(err, "failed to set bucket lifecycle")
}
@@ -633,7 +633,15 @@ func (p *Provisioner) setAdditionalSettings(additionalConfig *additionalConfigSp
return nil
}
func (p *Provisioner) setUserQuota(additionalConfig *additionalConfigSpec) error {
func (p *Provisioner) setUserQuota(bucket *bucket) error {
additionalConfig := bucket.additionalConfig
if additionalConfig.bucketOwner != nil {
// when an explicit bucket owner is set, we do not manage user quotas
logger.Debugf("Skipping user level quotas for OBC %q as bucketOwner is set", bucket.options.ObjectBucketClaim.Name)
return nil
}
liveQuota, err := p.adminOpsClient.GetUserQuota(p.clusterInfo.Context, admin.QuotaSpec{UID: p.cephUserName})
if err != nil {
return errors.Wrapf(err, "failed to fetch user %q", p.cephUserName)
@@ -684,12 +692,14 @@ func (p *Provisioner) setUserQuota(additionalConfig *additionalConfigSpec) error
return nil
}
func (p *Provisioner) setBucketQuota(additionalConfig *additionalConfigSpec) error {
bucket, err := p.adminOpsClient.GetBucketInfo(p.clusterInfo.Context, admin.Bucket{Bucket: p.bucketName})
func (p *Provisioner) setBucketQuota(bucket *bucket) error {
additionalConfig := bucket.additionalConfig
bkt, err := p.adminOpsClient.GetBucketInfo(p.clusterInfo.Context, admin.Bucket{Bucket: p.bucketName})
if err != nil {
return errors.Wrapf(err, "failed to fetch bucket %q", p.bucketName)
}
liveQuota := bucket.BucketQuota
liveQuota := bkt.BucketQuota
// Copy only the fields that are actively managed by the provisioner to
// prevent passing back undesirable combinations of fields. It is
@@ -737,7 +747,9 @@ func (p *Provisioner) setBucketQuota(additionalConfig *additionalConfigSpec) err
return nil
}
func (p *Provisioner) setBucketPolicy(additionalConfig *additionalConfigSpec) error {
func (p *Provisioner) setBucketPolicy(bucket *bucket) error {
additionalConfig := bucket.additionalConfig
svc := p.s3Agent.Client
var livePolicy *string
@@ -784,7 +796,9 @@ func (p *Provisioner) setBucketPolicy(additionalConfig *additionalConfigSpec) er
return nil
}
func (p *Provisioner) setBucketLifecycle(additionalConfig *additionalConfigSpec) error {
func (p *Provisioner) setBucketLifecycle(bucket *bucket) error {
additionalConfig := bucket.additionalConfig
svc := p.s3Agent.Client
var liveLc *s3.GetBucketLifecycleConfigurationOutput
pkg/operator/ceph/object/bucket/provisioner_test.go
+29 -18
View File
@@ -208,7 +208,8 @@ func TestProvisioner_setUserQuota(t *testing.T) {
p := newProvisioner(t, &getResult, &getSeen, &putSeen)
p.setBucketName("bob")
err := p.setUserQuota(&additionalConfigSpec{})
bucket := &bucket{additionalConfig: &additionalConfigSpec{}}
err := p.setUserQuota(bucket)
assert.NoError(t, err)
assert.Len(t, getSeen[userPath], 1)
@@ -227,7 +228,8 @@ func TestProvisioner_setUserQuota(t *testing.T) {
p := newProvisioner(t, &getResult, &getSeen, &putSeen)
p.setBucketName("bob")
err := p.setUserQuota(&additionalConfigSpec{})
bucket := &bucket{additionalConfig: &additionalConfigSpec{}}
err := p.setUserQuota(bucket)
assert.NoError(t, err)
assert.Len(t, getSeen[userPath], 1)
@@ -247,9 +249,10 @@ func TestProvisioner_setUserQuota(t *testing.T) {
p := newProvisioner(t, &getResult, &getSeen, &putSeen)
p.setBucketName("bob")
err := p.setUserQuota(&additionalConfigSpec{
bucket := &bucket{additionalConfig: &additionalConfigSpec{
maxSize: aws.Int64(2),
})
}}
err := p.setUserQuota(bucket)
assert.NoError(t, err)
assert.Len(t, getSeen[userPath], 1)
@@ -270,9 +273,10 @@ func TestProvisioner_setUserQuota(t *testing.T) {
p := newProvisioner(t, &getResult, &getSeen, &putSeen)
p.setBucketName("bob")
err := p.setUserQuota(&additionalConfigSpec{
bucket := &bucket{additionalConfig: &additionalConfigSpec{
maxObjects: aws.Int64(2),
})
}}
err := p.setUserQuota(bucket)
assert.NoError(t, err)
assert.Len(t, getSeen[userPath], 1)
@@ -293,10 +297,11 @@ func TestProvisioner_setUserQuota(t *testing.T) {
p := newProvisioner(t, &getResult, &getSeen, &putSeen)
p.setBucketName("bob")
err := p.setUserQuota(&additionalConfigSpec{
bucket := &bucket{additionalConfig: &additionalConfigSpec{
maxObjects: aws.Int64(2),
maxSize: aws.Int64(3),
})
}}
err := p.setUserQuota(bucket)
assert.NoError(t, err)
assert.Len(t, getSeen[userPath], 1)
@@ -318,10 +323,11 @@ func TestProvisioner_setUserQuota(t *testing.T) {
p := newProvisioner(t, &getResult, &getSeen, &putSeen)
p.setBucketName("bob")
err := p.setUserQuota(&additionalConfigSpec{
bucket := &bucket{additionalConfig: &additionalConfigSpec{
maxObjects: aws.Int64(12),
maxSize: aws.Int64(13),
})
}}
err := p.setUserQuota(bucket)
assert.NoError(t, err)
assert.Len(t, getSeen[userPath], 1)
@@ -394,7 +400,8 @@ func TestProvisioner_setBucketQuota(t *testing.T) {
p := newProvisioner(t, &getResult, &getSeen, &putSeen)
p.setBucketName("bob")
err := p.setBucketQuota(&additionalConfigSpec{})
bucket := &bucket{additionalConfig: &additionalConfigSpec{}}
err := p.setBucketQuota(bucket)
assert.NoError(t, err)
assert.Len(t, getSeen[bucketPath], 1)
@@ -413,7 +420,8 @@ func TestProvisioner_setBucketQuota(t *testing.T) {
p := newProvisioner(t, &getResult, &getSeen, &putSeen)
p.setBucketName("bob")
err := p.setBucketQuota(&additionalConfigSpec{})
bucket := &bucket{additionalConfig: &additionalConfigSpec{}}
err := p.setBucketQuota(bucket)
assert.NoError(t, err)
assert.Len(t, getSeen[bucketPath], 1)
@@ -433,9 +441,10 @@ func TestProvisioner_setBucketQuota(t *testing.T) {
p := newProvisioner(t, &getResult, &getSeen, &putSeen)
p.setBucketName("bob")
err := p.setBucketQuota(&additionalConfigSpec{
bucket := &bucket{additionalConfig: &additionalConfigSpec{
bucketMaxObjects: aws.Int64(4),
})
}}
err := p.setBucketQuota(bucket)
assert.NoError(t, err)
assert.Len(t, getSeen[bucketPath], 1)
@@ -456,9 +465,10 @@ func TestProvisioner_setBucketQuota(t *testing.T) {
p := newProvisioner(t, &getResult, &getSeen, &putSeen)
p.setBucketName("bob")
err := p.setBucketQuota(&additionalConfigSpec{
bucket := &bucket{additionalConfig: &additionalConfigSpec{
bucketMaxSize: aws.Int64(5),
})
}}
err := p.setBucketQuota(bucket)
assert.NoError(t, err)
assert.Len(t, getSeen[bucketPath], 1)
@@ -479,10 +489,11 @@ func TestProvisioner_setBucketQuota(t *testing.T) {
p := newProvisioner(t, &getResult, &getSeen, &putSeen)
p.setBucketName("bob")
err := p.setBucketQuota(&additionalConfigSpec{
bucket := &bucket{additionalConfig: &additionalConfigSpec{
bucketMaxObjects: aws.Int64(14),
bucketMaxSize: aws.Int64(15),
})
}}
err := p.setBucketQuota(bucket)
assert.NoError(t, err)
assert.Len(t, getSeen[bucketPath], 1)
tests/integration/object/bucketowner/bucketowner.go
+19 -9
View File
@@ -36,6 +36,7 @@ import (
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
storagev1 "k8s.io/api/storage/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/util/intstr"
@@ -112,6 +113,7 @@ var (
},
}
// test user without any quotas set
osu1 = cephv1.CephObjectStoreUser{
ObjectMeta: metav1.ObjectMeta{
Name: defaultName + "-user1",
@@ -123,6 +125,7 @@ var (
},
}
// test user with quotas set
osu2 = cephv1.CephObjectStoreUser{
ObjectMeta: metav1.ObjectMeta{
Name: defaultName + "-user2",
@@ -131,6 +134,11 @@ var (
Spec: cephv1.ObjectStoreUserSpec{
Store: objectStore.Name,
ClusterNamespace: objectStore.Namespace,
Quotas: &cephv1.ObjectUserQuotaSpec{
MaxBuckets: func(i int) *int { return &i }(1111),
MaxSize: resource.NewQuantity(2222, resource.DecimalSI),
MaxObjects: func(i int64) *int64 { return &i }(3333),
},
},
}
@@ -353,12 +361,13 @@ func TestObjectBucketClaimBucketOwner(t *testing.T, k8sh *utils.K8sHelper, insta
// obc should not modify pre-existing users
t.Run(fmt.Sprintf("no user quota was set on %q", osu1.Name), func(t *testing.T) {
liveQuota, err := adminClient.GetUserQuota(ctx, admin.QuotaSpec{UID: osu1.Name})
liveUser, err := adminClient.GetUser(ctx, admin.User{ID: osu1.Name})
require.NoError(t, err)
assert.False(t, *liveQuota.Enabled)
assert.Equal(t, int64(-1), *liveQuota.MaxSize)
assert.Equal(t, int64(-1), *liveQuota.MaxObjects)
assert.Equal(t, int(1000), *liveUser.MaxBuckets)
assert.False(t, *liveUser.UserQuota.Enabled)
assert.Equal(t, int64(-1), *liveUser.UserQuota.MaxSize)
assert.Equal(t, int64(-1), *liveUser.UserQuota.MaxObjects)
})
t.Run(fmt.Sprintf("create CephObjectStoreUser %q", osu2.Name), func(t *testing.T) {
@@ -439,13 +448,14 @@ func TestObjectBucketClaimBucketOwner(t *testing.T, k8sh *utils.K8sHelper, insta
})
// obc should not modify pre-existing users
t.Run(fmt.Sprintf("no user quota was set on %q", osu2.Name), func(t *testing.T) {
liveQuota, err := adminClient.GetUserQuota(ctx, admin.QuotaSpec{UID: osu2.Name})
t.Run(fmt.Sprintf("existing user quota on %q has not changed", osu2.Name), func(t *testing.T) {
liveUser, err := adminClient.GetUser(ctx, admin.User{ID: osu2.Name})
require.NoError(t, err)
assert.False(t, *liveQuota.Enabled)
assert.Equal(t, int64(-1), *liveQuota.MaxSize)
assert.Equal(t, int64(-1), *liveQuota.MaxObjects)
assert.Equal(t, int(1111), *liveUser.MaxBuckets)
assert.True(t, *liveUser.UserQuota.Enabled)
assert.Equal(t, int64(2222), *liveUser.UserQuota.MaxSize)
assert.Equal(t, int64(3333), *liveUser.UserQuota.MaxObjects)
})
t.Run(fmt.Sprintf("remove obc %q bucketOwner", obc1.Name), func(t *testing.T) {