From 525bbd7ac5bae418aa957d47b6921d5421f8a191 Mon Sep 17 00:00:00 2001 From: Nizamudeen A Date: Fri, 18 Oct 2024 20:50:33 +0530 Subject: [PATCH] mgr/dashboard: fix bucket get for s3 account owned bucket **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 (cherry picked from commit dd4549238e525d371414538bb43e8333af19cb96) Conflicts: src/pybind/mgr/dashboard/controllers/rgw.py - remove imports for multisite-automation src/pybind/mgr/dashboard/services/service.py - this file is not part of squid. hence the user parameter is added to rgw_client.py instead where it originally was --- src/pybind/mgr/dashboard/controllers/rgw.py | 64 ++++++++++++------- .../rgw-bucket-form.component.html | 8 +++ .../rgw-bucket-form.component.ts | 14 +++- src/pybind/mgr/dashboard/openapi.yaml | 1 - .../mgr/dashboard/services/rgw_client.py | 7 +- src/pybind/mgr/dashboard/services/rgw_iam.py | 24 +++++++ 6 files changed, 89 insertions(+), 29 deletions(-) create mode 100644 src/pybind/mgr/dashboard/services/rgw_iam.py diff --git a/src/pybind/mgr/dashboard/controllers/rgw.py b/src/pybind/mgr/dashboard/controllers/rgw.py index 0fc0dc1d884e0..a1a0d86107d6e 100755 --- a/src/pybind/mgr/dashboard/controllers/rgw.py +++ b/src/pybind/mgr/dashboard/controllers/rgw.py @@ -14,7 +14,9 @@ from ..rest_client import RequestException from ..security import Permission, Scope from ..services.auth import AuthManager, JwtManager from ..services.ceph_service import CephService -from ..services.rgw_client import _SYNC_GROUP_ID, NoRgwDaemonsException, RgwClient, RgwMultisite +from ..services.rgw_client import _RGW_USER, _SYNC_GROUP_ID, \ + NoRgwDaemonsException, RgwClient, RgwMultisite +from ..services.rgw_iam import RgwAccounts from ..tools import json_str_to_object, str_to_bool from . import APIDoc, APIRouter, BaseController, CreatePermission, \ CRUDCollectionMethod, CRUDEndpoint, DeletePermission, Endpoint, \ @@ -358,6 +360,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 _RGW_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) @@ -501,19 +512,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) @@ -558,7 +570,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, @@ -568,23 +580,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: @@ -618,7 +634,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', { diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-bucket-form/rgw-bucket-form.component.html b/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-bucket-form/rgw-bucket-form.component.html index b25d47fecf33a..9365268c8bba0 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-bucket-form/rgw-bucket-form.component.html +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-bucket-form/rgw-bucket-form.component.html @@ -91,6 +91,14 @@ This field is required. + + The bucket is owned by an account. UI does not support changing + the ownership of bucket owned by an account. + diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-bucket-form/rgw-bucket-form.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-bucket-form/rgw-bucket-form.component.ts index cc2b5206517e0..b77d20cfac345 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-bucket-form/rgw-bucket-form.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-bucket-form/rgw-bucket-form.component.ts @@ -267,6 +267,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(); @@ -325,11 +333,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'], diff --git a/src/pybind/mgr/dashboard/openapi.yaml b/src/pybind/mgr/dashboard/openapi.yaml index e9950f823a756..a2888f7701590 100644 --- a/src/pybind/mgr/dashboard/openapi.yaml +++ b/src/pybind/mgr/dashboard/openapi.yaml @@ -10716,7 +10716,6 @@ paths: type: string required: - bucket_id - - uid type: object responses: '200': diff --git a/src/pybind/mgr/dashboard/services/rgw_client.py b/src/pybind/mgr/dashboard/services/rgw_client.py index f8ce523e8cae0..58787c1d9d998 100755 --- a/src/pybind/mgr/dashboard/services/rgw_client.py +++ b/src/pybind/mgr/dashboard/services/rgw_client.py @@ -34,6 +34,8 @@ _SYNC_GROUP_ID = 'dashboard_admin_group' _SYNC_FLOW_ID = 'dashboard_admin_flow' _SYNC_PIPE_ID = 'dashboard_admin_pipe' +_RGW_USER = 'dashboard' + class NoRgwDaemonsException(Exception): def __init__(self): @@ -257,7 +259,6 @@ def _get_user_keys(user: str, realm: Optional[str] = None) -> Tuple[str, str]: def configure_rgw_credentials(): logger.info('Configuring dashboard RGW credentials') - user = 'dashboard' realms = [] access_key = '' secret_key = '' @@ -271,7 +272,7 @@ def configure_rgw_credentials(): realm_access_keys = {} realm_secret_keys = {} for realm in realms: - realm_access_key, realm_secret_key = _get_user_keys(user, realm) + realm_access_key, realm_secret_key = _get_user_keys(_RGW_USER, realm) if realm_access_key: realm_access_keys[realm] = realm_access_key realm_secret_keys[realm] = realm_secret_key @@ -279,7 +280,7 @@ def configure_rgw_credentials(): access_key = json.dumps(realm_access_keys) secret_key = json.dumps(realm_secret_keys) else: - access_key, secret_key = _get_user_keys(user) + access_key, secret_key = _get_user_keys(_RGW_USER) assert access_key and secret_key Settings.RGW_API_ACCESS_KEY = access_key diff --git a/src/pybind/mgr/dashboard/services/rgw_iam.py b/src/pybind/mgr/dashboard/services/rgw_iam.py new file mode 100644 index 0000000000000..dbf00df25e0bb --- /dev/null +++ b/src/pybind/mgr/dashboard/services/rgw_iam.py @@ -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) -- 2.39.5