From 9fccfb0e6f3e0e2cbc4c61c4d8c4b0e342fc6f69 Mon Sep 17 00:00:00 2001 From: Jiffin Tony Thottan Date: Wed, 27 May 2020 17:58:08 +0530 Subject: [PATCH 1/4] ceph: introduce new function for cleaning resources Combining DeleteUser() and DeleteBucket() into single function Signed-off-by: Jiffin Tony Thottan --- .../ceph/object/bucket/rgw-handlers.go | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/pkg/operator/ceph/object/bucket/rgw-handlers.go b/pkg/operator/ceph/object/bucket/rgw-handlers.go index 63a756359..6e197706f 100644 --- a/pkg/operator/ceph/object/bucket/rgw-handlers.go +++ b/pkg/operator/ceph/object/bucket/rgw-handlers.go @@ -122,3 +122,38 @@ func (p *Provisioner) genUserName() (genName string, err error) { } return genName, nil } + +// Delete the user and bucket created by OBC with help of radosgw-admin commands +// If delete user failed, error is no longer returned since its permission is +// already revoked and hence user is no longer able to access the bucket +// Empty string is passed for bucketName only if user needs to be removed, ex Revoke() +func (p *Provisioner) deleteOBCResource(bucketName string) error { + + logger.Infof("deleting Ceph user %s and bucket %q", p.cephUserName, bucketName) + if len(p.cephUserName) > 0 { + _, errCode, _ := cephObject.DeleteUser(p.objectContext, p.cephUserName) + + if errCode == cephObject.RGWErrorNone { + logger.Infof("user %q successfully deleted", p.cephUserName) + } else if errCode == cephObject.RGWErrorNotFound { + // opinion: "not found" is not an error + logger.Infof("user %q does not exist", p.cephUserName) + } else { + logger.Warningf("failed to delete user %q: errCode: %d", p.cephUserName, errCode) + } + } + if len(bucketName) > 0 { + // delete bucket with purge option to remove all objects + errCode, err := cephObject.DeleteBucket(p.objectContext, bucketName, true) + + if errCode == cephObject.RGWErrorNone { + logger.Infof("bucket %q successfully deleted", p.bucketName) + } else if errCode == cephObject.RGWErrorNotFound { + // opinion: "not found" is not an error + logger.Infof("bucket %q does not exist", p.bucketName) + } else { + return errors.Wrapf(err, "failed to delete bucket %q: errCode: %d", bucketName, errCode) + } + } + return nil +} From 07f5aa7a191568b3c29bfbdb37ae1d7815fe7902 Mon Sep 17 00:00:00 2001 From: Jiffin Tony Thottan Date: Wed, 27 May 2020 22:33:31 +0530 Subject: [PATCH 2/4] ceph: remove deleteCephUser() and deleteBucket() Replacing deleteCephUser() and deleteBucket() with deleteOBCResource() Signed-off-by: Jiffin Tony Thottan --- .../ceph/object/bucket/provisioner.go | 11 +---- .../ceph/object/bucket/rgw-handlers.go | 40 ------------------- 2 files changed, 2 insertions(+), 49 deletions(-) diff --git a/pkg/operator/ceph/object/bucket/provisioner.go b/pkg/operator/ceph/object/bucket/provisioner.go index aa902fde6..4b5876db1 100644 --- a/pkg/operator/ceph/object/bucket/provisioner.go +++ b/pkg/operator/ceph/object/bucket/provisioner.go @@ -197,11 +197,7 @@ func (p Provisioner) Delete(ob *bktv1alpha1.ObjectBucket) error { return err } - err = p.deleteCephUser(p.cephUserName) - if err != nil { - return err - } - err = p.deleteBucket(p.bucketName) + err = p.deleteOBCResource(p.bucketName) if err != nil { return err } @@ -294,10 +290,7 @@ func (p Provisioner) Revoke(ob *bktv1alpha1.ObjectBucket) error { } // finally, delete unlinked user - err = p.deleteCephUser(p.cephUserName) - if err != nil { - return err - } + p.deleteOBCResource("") return nil } diff --git a/pkg/operator/ceph/object/bucket/rgw-handlers.go b/pkg/operator/ceph/object/bucket/rgw-handlers.go index 6e197706f..edda8bd17 100644 --- a/pkg/operator/ceph/object/bucket/rgw-handlers.go +++ b/pkg/operator/ceph/object/bucket/rgw-handlers.go @@ -7,26 +7,6 @@ import ( cephObject "github.com/rook/rook/pkg/operator/ceph/object" ) -// The bucket (and objects) could be deleted via an s3 iterator, but since radosgw -// supports the `buket rm` command, this is the approch we'll use. -func (p *Provisioner) deleteBucket(bktName string) error { - - // delete bucket with purge option to remove all objects - errCode, err := cephObject.DeleteBucket(p.objectContext, p.bucketName, true) - if errCode == cephObject.RGWErrorNone { - logger.Infof("Bucket %s successfully deleted", bktName) - return nil - } - - // opinion: "not found" is not an error - if errCode == cephObject.RGWErrorNotFound { - logger.Infof("Bucket %s does not exist", bktName) - return nil - } - - return errors.Wrapf(err, "failed to delete bucket %q: errCode: %d", bktName, errCode) -} - func (p *Provisioner) bucketExists(name string) (bool, error) { _, errCode, err := cephObject.GetBucket(p.objectContext, name) if errCode != 0 { @@ -83,26 +63,6 @@ func (p *Provisioner) createCephUser(username string) (accKey string, secKey str return *u.AccessKey, *u.SecretKey, nil } -// Delete the Ceph user associated to the passed-in bucket. -func (p *Provisioner) deleteCephUser(username string) error { - - logger.Infof("deleting Ceph user %s for bucket %q", username, p.bucketName) - _, errCode, err := cephObject.DeleteUser(p.objectContext, username) - - if errCode == cephObject.RGWErrorNone { - logger.Infof("User %s successfully deleted", username) - return nil - } - - // opinion: "not found" is not an error - if errCode == cephObject.RGWErrorNotFound { - logger.Infof("User %s does not exist", username) - return nil - } - - return errors.Wrapf(err, "failed to delete user %q: errCode: %d", username, errCode) -} - // returns "" if unable to generate a unique name. func (p *Provisioner) genUserName() (genName string, err error) { const ( From c0e23e0e16337865964d8c1b9ce28c0041498faf Mon Sep 17 00:00:00 2001 From: Jiffin Tony Thottan Date: Tue, 19 May 2020 16:14:12 +0530 Subject: [PATCH 3/4] ceph: clean up resources incase of failure When obc creation fails in Grant() or Provision() the resources such as rgw user or bucket created while provisioning need to remove. Signed-off-by: Jiffin Tony Thottan --- pkg/operator/ceph/object/bucket/provisioner.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pkg/operator/ceph/object/bucket/provisioner.go b/pkg/operator/ceph/object/bucket/provisioner.go index 4b5876db1..3870aec9e 100644 --- a/pkg/operator/ceph/object/bucket/provisioner.go +++ b/pkg/operator/ceph/object/bucket/provisioner.go @@ -94,11 +94,13 @@ func (p Provisioner) Provision(options *apibkt.BucketOptions) (*bktv1alpha1.Obje if err != nil { err = errors.Wrapf(err, "error creating bucket %q", p.bucketName) logger.Errorf(err.Error()) + p.deleteOBCResource("") return nil, err } _, errCode, err := cephObject.SetQuotaUserBucketMax(p.objectContext, p.cephUserName, maxBuckets) if errCode > 0 { + p.deleteOBCResource(p.bucketName) return nil, err } logger.Infof("set user %q bucket max to %d", p.cephUserName, maxBuckets) @@ -131,21 +133,25 @@ func (p Provisioner) Grant(options *apibkt.BucketOptions) (*bktv1alpha1.ObjectBu // need to quota into -1 for restricting creation of new buckets in rgw _, _, err = cephObject.SetQuotaUserBucketMax(p.objectContext, p.cephUserName, -1) if err != nil { + p.deleteOBCResource("") return nil, err } // get the bucket's owner via the bucket metadata stats, _, err := cephObject.GetBucket(p.objectContext, p.bucketName) if err != nil { + p.deleteOBCResource("") return nil, errors.Wrapf(err, "could not get bucket stats (bucket: %s)", p.bucketName) } objectUser, _, err := cephObject.GetUser(p.objectContext, stats.Owner) if err != nil { + p.deleteOBCResource("") return nil, errors.Wrapf(err, "could not get user (user: %s)", stats.Owner) } s3svc, err := NewS3Agent(*objectUser.AccessKey, *objectUser.SecretKey, p.getObjectStoreEndpoint()) if err != nil { + p.deleteOBCResource("") return nil, err } @@ -154,6 +160,7 @@ func (p Provisioner) Grant(options *apibkt.BucketOptions) (*bktv1alpha1.ObjectBu if err != nil { if aerr, ok := err.(awserr.Error); ok { if aerr.Code() != "NoSuchBucketPolicy" { + p.deleteOBCResource("") return nil, err } } @@ -175,6 +182,7 @@ func (p Provisioner) Grant(options *apibkt.BucketOptions) (*bktv1alpha1.ObjectBu logger.Infof("PutBucketPolicy output: %v", out) if err != nil { + p.deleteOBCResource("") return nil, err } // returned ob with connection info From 96e3d1529e8e01989b849dde5f84657f016c9b3a Mon Sep 17 00:00:00 2001 From: Jiffin Tony Thottan Date: Tue, 9 Jun 2020 16:27:33 +0530 Subject: [PATCH 4/4] ceph: remove unlinking user in revoke api for obc UnlinkUser works if bucket owner is obc user, otherwise it result in mismatch. Signed-off-by: Jiffin Tony Thottan --- pkg/operator/ceph/object/bucket/provisioner.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/pkg/operator/ceph/object/bucket/provisioner.go b/pkg/operator/ceph/object/bucket/provisioner.go index 3870aec9e..17d8efca5 100644 --- a/pkg/operator/ceph/object/bucket/provisioner.go +++ b/pkg/operator/ceph/object/bucket/provisioner.go @@ -292,12 +292,7 @@ func (p Provisioner) Revoke(ob *bktv1alpha1.ObjectBucket) error { } } - _, _, err = cephObject.UnlinkUser(p.objectContext, p.cephUserName, p.bucketName) - if err != nil { - return err - } - - // finally, delete unlinked user + // finally, delete the user p.deleteOBCResource("") return nil }