diff --git a/pkg/operator/ceph/object/bucket/provisioner.go b/pkg/operator/ceph/object/bucket/provisioner.go index b9a1eb82b..2af38c05c 100644 --- a/pkg/operator/ceph/object/bucket/provisioner.go +++ b/pkg/operator/ceph/object/bucket/provisioner.go @@ -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 diff --git a/pkg/operator/ceph/object/bucket/provisioner_test.go b/pkg/operator/ceph/object/bucket/provisioner_test.go index c4affef80..fa5388620 100644 --- a/pkg/operator/ceph/object/bucket/provisioner_test.go +++ b/pkg/operator/ceph/object/bucket/provisioner_test.go @@ -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) diff --git a/tests/integration/object/bucketowner/bucketowner.go b/tests/integration/object/bucketowner/bucketowner.go index e12237bcd..a1097463f 100644 --- a/tests/integration/object/bucketowner/bucketowner.go +++ b/tests/integration/object/bucketowner/bucketowner.go @@ -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) {