Skip to content

Commit dd45492

Browse files
committed
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 <[email protected]>
1 parent 1e4f788 commit dd45492

File tree

6 files changed

+87
-28
lines changed

6 files changed

+87
-28
lines changed

src/pybind/mgr/dashboard/controllers/rgw.py

Lines changed: 38 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
from ..services.ceph_service import CephService
1717
from ..services.rgw_client import _SYNC_GROUP_ID, NoRgwDaemonsException, \
1818
RgwClient, RgwMultisite, RgwMultisiteAutomation
19+
from ..services.rgw_iam import RgwAccounts
1920
from ..services.service import RgwServiceManager, wait_for_daemon_to_start
2021
from ..tools import json_str_to_object, str_to_bool
2122
from . import APIDoc, APIRouter, BaseController, CreatePermission, \
@@ -399,6 +400,15 @@ def _append_bid(self, bucket):
399400
if bucket['tenant'] else bucket['bucket']
400401
return bucket
401402

403+
def _get_owner(self, owner):
404+
accounts = RgwAccounts().get_accounts()
405+
406+
# if the owner is present in the accounts list,
407+
# then the bucket is owned by an account.
408+
# hence we will use dashboard user to fetch the
409+
# bucket info
410+
return owner if owner not in accounts else RgwServiceManager.user
411+
402412
def _get_versioning(self, owner, daemon_name, bucket_name):
403413
rgw_client = RgwClient.instance(owner, daemon_name)
404414
return rgw_client.get_bucket_versioning(bucket_name)
@@ -542,19 +552,20 @@ def get(self, bucket, daemon_name=None):
542552
bucket_name = RgwBucket.get_s3_bucket_name(result['bucket'],
543553
result['tenant'])
544554

555+
owner = self._get_owner(result['owner'])
545556
# Append the versioning configuration.
546-
versioning = self._get_versioning(result['owner'], daemon_name, bucket_name)
547-
encryption = self._get_encryption(bucket_name, daemon_name, result['owner'])
557+
versioning = self._get_versioning(owner, daemon_name, bucket_name)
558+
encryption = self._get_encryption(bucket_name, daemon_name, owner)
548559
result['encryption'] = encryption['Status']
549560
result['versioning'] = versioning['Status']
550561
result['mfa_delete'] = versioning['MfaDelete']
551-
result['bucket_policy'] = self._get_policy(bucket_name, daemon_name, result['owner'])
552-
result['acl'] = self._get_acl(bucket_name, daemon_name, result['owner'])
553-
result['replication'] = self._get_replication(bucket_name, result['owner'], daemon_name)
554-
result['lifecycle'] = self._get_lifecycle(bucket_name, daemon_name, result['owner'])
562+
result['bucket_policy'] = self._get_policy(bucket_name, daemon_name, owner)
563+
result['acl'] = self._get_acl(bucket_name, daemon_name, owner)
564+
result['replication'] = self._get_replication(bucket_name, owner, daemon_name)
565+
result['lifecycle'] = self._get_lifecycle(bucket_name, daemon_name, owner)
555566

556567
# Append the locking configuration.
557-
locking = self._get_locking(result['owner'], daemon_name, bucket_name)
568+
locking = self._get_locking(owner, daemon_name, bucket_name)
558569
result.update(locking)
559570

560571
return self._append_bid(result)
@@ -599,7 +610,7 @@ def create(self, bucket, uid, zonegroup=None, placement_target=None,
599610
raise DashboardException(e, http_status_code=500, component='rgw')
600611

601612
@allow_empty_body
602-
def set(self, bucket, bucket_id, uid, versioning_state=None,
613+
def set(self, bucket, bucket_id, uid=None, versioning_state=None,
603614
encryption_state='false', encryption_type=None, key_id=None,
604615
mfa_delete=None, mfa_token_serial=None, mfa_token_pin=None,
605616
lock_mode=None, lock_retention_period_days=None,
@@ -609,23 +620,27 @@ def set(self, bucket, bucket_id, uid, versioning_state=None,
609620
encryption_state = str_to_bool(encryption_state)
610621
if replication is not None:
611622
replication = str_to_bool(replication)
612-
# When linking a non-tenant-user owned bucket to a tenanted user, we
613-
# need to prefix bucket name with '/'. e.g. photos -> /photos
614-
if '$' in uid and '/' not in bucket:
615-
bucket = '/{}'.format(bucket)
616-
617-
# Link bucket to new user:
618-
result = self.proxy(daemon_name,
619-
'PUT',
620-
'bucket', {
621-
'bucket': bucket,
622-
'bucket-id': bucket_id,
623-
'uid': uid
624-
},
625-
json_response=False)
623+
624+
result = None
625+
if uid:
626+
# When linking a non-tenant-user owned bucket to a tenanted user, we
627+
# need to prefix bucket name with '/'. e.g. photos -> /photos
628+
if '$' in uid and '/' not in bucket:
629+
bucket = '/{}'.format(bucket)
630+
631+
# Link bucket to new user:
632+
result = self.proxy(daemon_name,
633+
'PUT',
634+
'bucket', {
635+
'bucket': bucket,
636+
'bucket-id': bucket_id,
637+
'uid': uid
638+
},
639+
json_response=False)
626640

627641
uid_tenant = uid[:uid.find('$')] if uid.find('$') >= 0 else None
628642
bucket_name = RgwBucket.get_s3_bucket_name(bucket, uid_tenant)
643+
uid = self._get_owner(uid)
629644

630645
locking = self._get_locking(uid, daemon_name, bucket_name)
631646
if versioning_state:
@@ -659,7 +674,7 @@ def set(self, bucket, bucket_id, uid, versioning_state=None,
659674
self._set_lifecycle(bucket_name, lifecycle, daemon_name, uid)
660675
else:
661676
self._delete_lifecycle(bucket_name, daemon_name, uid)
662-
return self._append_bid(result)
677+
return self._append_bid(result) if result else None
663678

664679
def delete(self, bucket, purge_objects='true', daemon_name=None):
665680
return self.proxy(daemon_name, 'DELETE', 'bucket', {

src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-bucket-form/rgw-bucket-form.component.html

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,14 @@
9191
<span class="invalid-feedback"
9292
*ngIf="bucketForm.showError('owner', frm, 'required')"
9393
i18n>This field is required.</span>
94+
<cd-alert-panel
95+
type="info"
96+
*ngIf="bucketForm.get('owner').disabled"
97+
spacingClass="me-1 mt-1"
98+
i18n>
99+
The bucket is owned by an account. UI does not support changing
100+
the ownership of bucket owned by an account.
101+
</cd-alert-panel>
94102
</div>
95103
</div>
96104

src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-bucket-form/rgw-bucket-form.component.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,14 @@ export class RgwBucketFormComponent extends CdForm implements OnInit, AfterViewC
269269
}
270270
this.bucketForm.setValue(value);
271271
if (this.editing) {
272+
// temporary fix until the s3 account management is implemented in
273+
// the frontend. Disable changing the owner of the bucket in case
274+
// its owned by the account.
275+
// @TODO: Introduce account selection for a bucket.
276+
if (!this.owners.includes(value['owner'])) {
277+
this.owners.push(value['owner']);
278+
this.bucketForm.get('owner').disable();
279+
}
272280
this.isVersioningAlreadyEnabled = this.isVersioningEnabled;
273281
this.isMfaDeleteAlreadyEnabled = this.isMfaDeleteEnabled;
274282
this.setMfaDeleteValidators();
@@ -327,11 +335,15 @@ export class RgwBucketFormComponent extends CdForm implements OnInit, AfterViewC
327335
// Edit
328336
const versioning = this.getVersioningStatus();
329337
const mfaDelete = this.getMfaDeleteStatus();
338+
// make the owner empty if the field is disabled.
339+
// this ensures the bucket doesn't gets updated with owner when
340+
// the bucket is owned by the account.
341+
const owner = this.bucketForm.get('owner').disabled === true ? '' : values['owner'];
330342
this.rgwBucketService
331343
.update(
332344
values['bid'],
333345
values['id'],
334-
values['owner'],
346+
owner,
335347
versioning,
336348
values['encryption_enabled'],
337349
values['encryption_type'],

src/pybind/mgr/dashboard/openapi.yaml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11193,7 +11193,6 @@ paths:
1119311193
type: string
1119411194
required:
1119511195
- bucket_id
11196-
- uid
1119711196
type: object
1119811197
responses:
1119911198
'200':
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
from subprocess import SubprocessError
2+
from typing import List
3+
4+
from .. import mgr
5+
from ..exceptions import DashboardException
6+
7+
8+
class RgwAccounts:
9+
def send_rgw_cmd(self, command: List[str]):
10+
try:
11+
exit_code, out, err = mgr.send_rgwadmin_command(command)
12+
13+
if exit_code != 0:
14+
raise DashboardException(msg=err,
15+
http_status_code=500,
16+
component='rgw')
17+
return out
18+
19+
except SubprocessError as e:
20+
raise DashboardException(e, component='rgw')
21+
22+
def get_accounts(self):
23+
get_accounts_cmd = ['account', 'list']
24+
return self.send_rgw_cmd(get_accounts_cmd)

src/pybind/mgr/dashboard/services/service.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,8 @@ def wait_for_daemon_to_start(service_name, timeout=30):
101101

102102

103103
class RgwServiceManager:
104+
user = 'dashboard'
105+
104106
def find_available_port(self, starting_port=80):
105107
orch = OrchClient.instance()
106108
daemons = [d.to_dict() for d in orch.services.list_daemons(daemon_type='rgw')]
@@ -172,7 +174,6 @@ def _get_user_keys(self, user: str, realm: Optional[str] = None) -> Tuple[str, s
172174

173175
def configure_rgw_credentials(self):
174176
logger.info('Configuring dashboard RGW credentials')
175-
user = 'dashboard'
176177
realms = []
177178
access_key = ''
178179
secret_key = ''
@@ -186,15 +187,15 @@ def configure_rgw_credentials(self):
186187
realm_access_keys = {}
187188
realm_secret_keys = {}
188189
for realm in realms:
189-
realm_access_key, realm_secret_key = self._get_user_keys(user, realm)
190+
realm_access_key, realm_secret_key = self._get_user_keys(self.user, realm)
190191
if realm_access_key:
191192
realm_access_keys[realm] = realm_access_key
192193
realm_secret_keys[realm] = realm_secret_key
193194
if realm_access_keys:
194195
access_key = json.dumps(realm_access_keys)
195196
secret_key = json.dumps(realm_secret_keys)
196197
else:
197-
access_key, secret_key = self._get_user_keys(user)
198+
access_key, secret_key = self._get_user_keys(self.user)
198199

199200
assert access_key and secret_key
200201
Settings.RGW_API_ACCESS_KEY = access_key

0 commit comments

Comments
 (0)