]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
mgr/dashboard: fix bucket get for s3 account owned bucket 60399/head
authorNizamudeen A <nia@redhat.com>
Fri, 18 Oct 2024 15:20:33 +0000 (20:50 +0530)
committerNizamudeen A <nia@redhat.com>
Tue, 22 Oct 2024 13:48:39 +0000 (19:18 +0530)
**Issue 1:**

When a bucket is created with a user that is owner by the account user,
it fails to fetch the info for the bucket because the owner of that
bucket is considered as owned by the account rather than the user.

dashboard api try to fetch some info for the bucket on the basis of the
bucket owner which in this case fails since its not a real user.

To keep the existing behavior as it is and fix the current issue, I am
doing a check to make sure the user id exists or not. if it doesn't
exist, we goes on to fetch using the dashboard user.

**Issue 2**
Editing the bucket owner by the account is broken. So disabling the
ownership change for the bucket owned by account until we have user
account management for rgw in the dashboard

Fixes: https://tracker.ceph.com/issues/68622
Signed-off-by: Nizamudeen A <nia@redhat.com>
src/pybind/mgr/dashboard/controllers/rgw.py
src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-bucket-form/rgw-bucket-form.component.html
src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-bucket-form/rgw-bucket-form.component.ts
src/pybind/mgr/dashboard/openapi.yaml
src/pybind/mgr/dashboard/services/rgw_iam.py [new file with mode: 0644]
src/pybind/mgr/dashboard/services/service.py

index 2e6e466f97b19b14484073d5f8f774e7aeee1710..9d2576747947395b5102a0dea69ee89c53a3a5f7 100755 (executable)
@@ -16,6 +16,7 @@ from ..services.auth import AuthManager, JwtManager
 from ..services.ceph_service import CephService
 from ..services.rgw_client import _SYNC_GROUP_ID, NoRgwDaemonsException, \
     RgwClient, RgwMultisite, RgwMultisiteAutomation
+from ..services.rgw_iam import RgwAccounts
 from ..services.service import RgwServiceManager, wait_for_daemon_to_start
 from ..tools import json_str_to_object, str_to_bool
 from . import APIDoc, APIRouter, BaseController, CreatePermission, \
@@ -399,6 +400,15 @@ class RgwBucket(RgwRESTController):
                 if bucket['tenant'] else bucket['bucket']
         return bucket
 
+    def _get_owner(self, owner):
+        accounts = RgwAccounts().get_accounts()
+
+        # if the owner is present in the accounts list,
+        # then the bucket is owned by an account.
+        # hence we will use dashboard user to fetch the
+        # bucket info
+        return owner if owner not in accounts else RgwServiceManager.user
+
     def _get_versioning(self, owner, daemon_name, bucket_name):
         rgw_client = RgwClient.instance(owner, daemon_name)
         return rgw_client.get_bucket_versioning(bucket_name)
@@ -542,19 +552,20 @@ class RgwBucket(RgwRESTController):
         bucket_name = RgwBucket.get_s3_bucket_name(result['bucket'],
                                                    result['tenant'])
 
+        owner = self._get_owner(result['owner'])
         # Append the versioning configuration.
-        versioning = self._get_versioning(result['owner'], daemon_name, bucket_name)
-        encryption = self._get_encryption(bucket_name, daemon_name, result['owner'])
+        versioning = self._get_versioning(owner, daemon_name, bucket_name)
+        encryption = self._get_encryption(bucket_name, daemon_name, owner)
         result['encryption'] = encryption['Status']
         result['versioning'] = versioning['Status']
         result['mfa_delete'] = versioning['MfaDelete']
-        result['bucket_policy'] = self._get_policy(bucket_name, daemon_name, result['owner'])
-        result['acl'] = self._get_acl(bucket_name, daemon_name, result['owner'])
-        result['replication'] = self._get_replication(bucket_name, result['owner'], daemon_name)
-        result['lifecycle'] = self._get_lifecycle(bucket_name, daemon_name, result['owner'])
+        result['bucket_policy'] = self._get_policy(bucket_name, daemon_name, owner)
+        result['acl'] = self._get_acl(bucket_name, daemon_name, owner)
+        result['replication'] = self._get_replication(bucket_name, owner, daemon_name)
+        result['lifecycle'] = self._get_lifecycle(bucket_name, daemon_name, owner)
 
         # Append the locking configuration.
-        locking = self._get_locking(result['owner'], daemon_name, bucket_name)
+        locking = self._get_locking(owner, daemon_name, bucket_name)
         result.update(locking)
 
         return self._append_bid(result)
@@ -599,7 +610,7 @@ class RgwBucket(RgwRESTController):
             raise DashboardException(e, http_status_code=500, component='rgw')
 
     @allow_empty_body
-    def set(self, bucket, bucket_id, uid, versioning_state=None,
+    def set(self, bucket, bucket_id, uid=None, versioning_state=None,
             encryption_state='false', encryption_type=None, key_id=None,
             mfa_delete=None, mfa_token_serial=None, mfa_token_pin=None,
             lock_mode=None, lock_retention_period_days=None,
@@ -609,23 +620,27 @@ class RgwBucket(RgwRESTController):
         encryption_state = str_to_bool(encryption_state)
         if replication is not None:
             replication = str_to_bool(replication)
-        # When linking a non-tenant-user owned bucket to a tenanted user, we
-        # need to prefix bucket name with '/'. e.g. photos -> /photos
-        if '$' in uid and '/' not in bucket:
-            bucket = '/{}'.format(bucket)
-
-        # Link bucket to new user:
-        result = self.proxy(daemon_name,
-                            'PUT',
-                            'bucket', {
-                                'bucket': bucket,
-                                'bucket-id': bucket_id,
-                                'uid': uid
-                            },
-                            json_response=False)
+
+        result = None
+        if uid:
+            # When linking a non-tenant-user owned bucket to a tenanted user, we
+            # need to prefix bucket name with '/'. e.g. photos -> /photos
+            if '$' in uid and '/' not in bucket:
+                bucket = '/{}'.format(bucket)
+
+            # Link bucket to new user:
+            result = self.proxy(daemon_name,
+                                'PUT',
+                                'bucket', {
+                                    'bucket': bucket,
+                                    'bucket-id': bucket_id,
+                                    'uid': uid
+                                },
+                                json_response=False)
 
         uid_tenant = uid[:uid.find('$')] if uid.find('$') >= 0 else None
         bucket_name = RgwBucket.get_s3_bucket_name(bucket, uid_tenant)
+        uid = self._get_owner(uid)
 
         locking = self._get_locking(uid, daemon_name, bucket_name)
         if versioning_state:
@@ -659,7 +674,7 @@ class RgwBucket(RgwRESTController):
             self._set_lifecycle(bucket_name, lifecycle, daemon_name, uid)
         else:
             self._delete_lifecycle(bucket_name, daemon_name, uid)
-        return self._append_bid(result)
+        return self._append_bid(result) if result else None
 
     def delete(self, bucket, purge_objects='true', daemon_name=None):
         return self.proxy(daemon_name, 'DELETE', 'bucket', {
index f77526be779b2549c2d5804692c589440e5d9b56..9c07182a0e597eb0e77a5e899442445dd6abf298 100644 (file)
             <span class="invalid-feedback"
                   *ngIf="bucketForm.showError('owner', frm, 'required')"
                   i18n>This field is required.</span>
+            <cd-alert-panel
+              type="info"
+              *ngIf="bucketForm.get('owner').disabled"
+              spacingClass="me-1 mt-1"
+              i18n>
+                The bucket is owned by an account. UI does not support changing
+                the ownership of bucket owned by an account.
+            </cd-alert-panel>
           </div>
         </div>
 
index d82c71e3cf74f1c80d1cc31b4d9221d73576cb84..53a1ac442c5300d4ac7c34a1b75a788a5c733644 100644 (file)
@@ -269,6 +269,14 @@ export class RgwBucketFormComponent extends CdForm implements OnInit, AfterViewC
           }
           this.bucketForm.setValue(value);
           if (this.editing) {
+            // temporary fix until the s3 account management is implemented in
+            // the frontend. Disable changing the owner of the bucket in case
+            // its owned by the account.
+            // @TODO: Introduce account selection for a bucket.
+            if (!this.owners.includes(value['owner'])) {
+              this.owners.push(value['owner']);
+              this.bucketForm.get('owner').disable();
+            }
             this.isVersioningAlreadyEnabled = this.isVersioningEnabled;
             this.isMfaDeleteAlreadyEnabled = this.isMfaDeleteEnabled;
             this.setMfaDeleteValidators();
@@ -327,11 +335,15 @@ export class RgwBucketFormComponent extends CdForm implements OnInit, AfterViewC
       // Edit
       const versioning = this.getVersioningStatus();
       const mfaDelete = this.getMfaDeleteStatus();
+      // make the owner empty if the field is disabled.
+      // this ensures the bucket doesn't gets updated with owner when
+      // the bucket is owned by the account.
+      const owner = this.bucketForm.get('owner').disabled === true ? '' : values['owner'];
       this.rgwBucketService
         .update(
           values['bid'],
           values['id'],
-          values['owner'],
+          owner,
           versioning,
           values['encryption_enabled'],
           values['encryption_type'],
index aedee7e493db4a24d223c4265e476b875157dbbe..b464344e27a2f76cc7a590529b25704ccd9d01ac 100644 (file)
@@ -11193,7 +11193,6 @@ paths:
                   type: string
               required:
               - bucket_id
-              - uid
               type: object
       responses:
         '200':
diff --git a/src/pybind/mgr/dashboard/services/rgw_iam.py b/src/pybind/mgr/dashboard/services/rgw_iam.py
new file mode 100644 (file)
index 0000000..dbf00df
--- /dev/null
@@ -0,0 +1,24 @@
+from subprocess import SubprocessError
+from typing import List
+
+from .. import mgr
+from ..exceptions import DashboardException
+
+
+class RgwAccounts:
+    def send_rgw_cmd(self, command: List[str]):
+        try:
+            exit_code, out, err = mgr.send_rgwadmin_command(command)
+
+            if exit_code != 0:
+                raise DashboardException(msg=err,
+                                         http_status_code=500,
+                                         component='rgw')
+            return out
+
+        except SubprocessError as e:
+            raise DashboardException(e, component='rgw')
+
+    def get_accounts(self):
+        get_accounts_cmd = ['account', 'list']
+        return self.send_rgw_cmd(get_accounts_cmd)
index 41fcc4c44461340c5994044f3539a0af03f06990..9b789c0c85929b279fc5a91afca71532764fe02c 100644 (file)
@@ -101,6 +101,8 @@ def wait_for_daemon_to_start(service_name, timeout=30):
 
 
 class RgwServiceManager:
+    user = 'dashboard'
+
     def find_available_port(self, starting_port=80):
         orch = OrchClient.instance()
         daemons = [d.to_dict() for d in orch.services.list_daemons(daemon_type='rgw')]
@@ -172,7 +174,6 @@ class RgwServiceManager:
 
     def configure_rgw_credentials(self):
         logger.info('Configuring dashboard RGW credentials')
-        user = 'dashboard'
         realms = []
         access_key = ''
         secret_key = ''
@@ -186,7 +187,7 @@ class RgwServiceManager:
                 realm_access_keys = {}
                 realm_secret_keys = {}
                 for realm in realms:
-                    realm_access_key, realm_secret_key = self._get_user_keys(user, realm)
+                    realm_access_key, realm_secret_key = self._get_user_keys(self.user, realm)
                     if realm_access_key:
                         realm_access_keys[realm] = realm_access_key
                         realm_secret_keys[realm] = realm_secret_key
@@ -194,7 +195,7 @@ class RgwServiceManager:
                     access_key = json.dumps(realm_access_keys)
                     secret_key = json.dumps(realm_secret_keys)
             else:
-                access_key, secret_key = self._get_user_keys(user)
+                access_key, secret_key = self._get_user_keys(self.user)
 
             assert access_key and secret_key
             Settings.RGW_API_ACCESS_KEY = access_key