From: Naman Munet Date: Mon, 7 Jul 2025 09:26:49 +0000 (+0530) Subject: mgr/dashboard: differentiate account users from rgw users in bucket form X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=c5b5408c5be8c64280de368093509ad4ef8e28ec;p=ceph.git mgr/dashboard: differentiate account users from rgw users in bucket form fixes: https://tracker.ceph.com/issues/71523 commit includes: 1) Added checkbox to select account user and another dropdown to show account users 2) Also fixed bucket replication as it was throwing error for 'invalidBucketARN' Signed-off-by: Naman Munet --- diff --git a/src/pybind/mgr/dashboard/controllers/rgw.py b/src/pybind/mgr/dashboard/controllers/rgw.py index 90a438a972e23..de843d70aa442 100755 --- a/src/pybind/mgr/dashboard/controllers/rgw.py +++ b/src/pybind/mgr/dashboard/controllers/rgw.py @@ -626,13 +626,21 @@ class RgwBucket(RgwRESTController): bucket = '/{}'.format(bucket) # Link bucket to new user: + params = { + 'bucket': bucket, + 'bucket-id': bucket_id, + } + + accounts = RgwAccounts().get_accounts() + if uid in accounts: + # If the bucket is owned by an account, we need to use the account-id + # instead of uid. + params['account-id'] = uid + else: + params['uid'] = uid result = self.proxy(daemon_name, 'PUT', - 'bucket', { - 'bucket': bucket, - 'bucket-id': bucket_id, - 'uid': uid - }, + 'bucket', params, json_response=False) uid_tenant = uid[:uid.find('$')] if uid.find('$') >= 0 else None @@ -811,16 +819,24 @@ class RgwUser(RgwRESTController): and len(set(edit_permissions).intersection(set(permissions[Scope.RGW]))) > 0 @EndpointDoc("Display RGW Users", + parameters={ + 'detailed': (bool, "If true, returns complete user details for each user. " + "If false, returns only the list of usernames.") + }, responses={200: RGW_USER_SCHEMA}) - def list(self, daemon_name=None): - # type: (Optional[str]) -> List[str] - users = [] # type: List[str] + def list(self, daemon_name=None, detailed: bool = False): + detailed = str_to_bool(detailed) + users = [] # type: List[Union[str, Dict[str, Any]]] marker = None while True: params = {} # type: dict if marker: params['marker'] = marker result = self.proxy(daemon_name, 'GET', 'user?list', params) + if detailed: + for user in result['keys']: + users.append(self._get(user, daemon_name=daemon_name, stats=False)) + return users users.extend(result['keys']) if not result['truncated']: break 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 05432220722b1..3dc7779f91483 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 @@ -67,38 +67,67 @@ - -
- Owner - - - - - - This field is required. - - - The bucket is owned by an account. UI does not support changing - the ownership of bucket owned by an account. - -
+ + @if(accounts.length && accountUsers.length > 0){ + Select account user + + } + + @if (bucketForm.get('isAccountOwner').value) { + +
+ Account user + + + + + + This field is required. + + + The bucket is owned by an account. UI does not support changing + the ownership of bucket owned by an account. + +
+ } @else { + +
+ Owner + + + + + + This field is required. + +
+ }
diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-bucket-form/rgw-bucket-form.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-bucket-form/rgw-bucket-form.component.spec.ts index f72573e12a43e..f07609a22b9e3 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-bucket-form/rgw-bucket-form.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/rgw-bucket-form/rgw-bucket-form.component.spec.ts @@ -23,6 +23,7 @@ import { CheckboxModule, SelectModule } from 'carbon-components-angular'; import { NO_ERRORS_SCHEMA } from '@angular/core'; import { By } from '@angular/platform-browser'; import { LoadingStatus } from '~/app/shared/forms/cd-form'; +import { RgwUserAccountsService } from '~/app/shared/api/rgw-user-accounts.service'; describe('RgwBucketFormComponent', () => { let component: RgwBucketFormComponent; @@ -31,6 +32,7 @@ describe('RgwBucketFormComponent', () => { let getPlacementTargetsSpy: jasmine.Spy; let rgwBucketServiceGetSpy: jasmine.Spy; let enumerateSpy: jasmine.Spy; + let accountListSpy: jasmine.Spy; let formHelper: FormHelper; let childComponent: RgwRateLimitComponent; @@ -55,6 +57,7 @@ describe('RgwBucketFormComponent', () => { rgwBucketServiceGetSpy = spyOn(rgwBucketService, 'get'); getPlacementTargetsSpy = spyOn(TestBed.inject(RgwSiteService), 'get'); enumerateSpy = spyOn(TestBed.inject(RgwUserService), 'enumerate'); + accountListSpy = spyOn(TestBed.inject(RgwUserAccountsService), 'list'); formHelper = new FormHelper(component.bucketForm); }); @@ -85,6 +88,7 @@ describe('RgwBucketFormComponent', () => { }; getPlacementTargetsSpy.and.returnValue(observableOf(payload)); enumerateSpy.and.returnValue(observableOf([])); + accountListSpy.and.returnValue(observableOf([])); fixture.detectChanges(); expect(component.zonegroup).toBe(payload.zonegroup); 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 468718214432a..b3b6f0f35fb1e 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 @@ -16,7 +16,7 @@ import * as xml2js from 'xml2js'; import { RgwBucketService } from '~/app/shared/api/rgw-bucket.service'; import { RgwSiteService } from '~/app/shared/api/rgw-site.service'; import { RgwUserService } from '~/app/shared/api/rgw-user.service'; -import { ActionLabelsI18n, URLVerbs } from '~/app/shared/constants/app.constants'; +import { ActionLabelsI18n, AppConstants, URLVerbs } from '~/app/shared/constants/app.constants'; import { Icons } from '~/app/shared/enum/icons.enum'; import { NotificationType } from '~/app/shared/enum/notification-type.enum'; import { CdForm } from '~/app/shared/forms/cd-form'; @@ -41,6 +41,8 @@ import { TextAreaXmlFormatterService } from '~/app/shared/services/text-area-xml import { RgwRateLimitComponent } from '../rgw-rate-limit/rgw-rate-limit.component'; import { RgwRateLimitConfig } from '../models/rgw-rate-limit'; import { ModalCdsService } from '~/app/shared/services/modal-cds.service'; +import { RgwUserAccountsService } from '~/app/shared/api/rgw-user-accounts.service'; +import { RgwUser } from '../models/rgw-user'; @Component({ selector: 'cd-rgw-bucket-form', @@ -55,7 +57,9 @@ export class RgwBucketFormComponent extends CdForm implements OnInit, AfterViewC bucketForm: CdFormGroup; editing = false; - owners: string[] = null; + owners: RgwUser[] = null; + accounts: string[] = []; + accountUsers: RgwUser[] = []; kmsProviders: string[] = null; action: string; resource: string; @@ -104,7 +108,8 @@ export class RgwBucketFormComponent extends CdForm implements OnInit, AfterViewC public actionLabels: ActionLabelsI18n, private readonly changeDetectorRef: ChangeDetectorRef, private rgwMultisiteService: RgwMultisiteService, - private rgwDaemonService: RgwDaemonService + private rgwDaemonService: RgwDaemonService, + private rgwAccountsService: RgwUserAccountsService ) { super(); this.editing = this.router.url.startsWith(`/rgw/bucket/${URLVerbs.EDIT}`); @@ -137,7 +142,14 @@ export class RgwBucketFormComponent extends CdForm implements OnInit, AfterViewC ? [] : [CdValidators.bucketName(), CdValidators.bucketExistence(false, this.rgwBucketService)] ], - owner: [null, [Validators.required]], + owner: [ + null, + [ + CdValidators.requiredIf({ + isAccountOwner: false + }) + ] + ], kms_provider: ['vault'], 'placement-target': [null], versioning: [null], @@ -169,13 +181,23 @@ export class RgwBucketFormComponent extends CdForm implements OnInit, AfterViewC lifecycle: ['{}', CdValidators.jsonOrXml()], grantee: [Grantee.Owner, [Validators.required]], aclPermission: [[aclPermission.FullControl], [Validators.required]], - replication: [false] + replication: [false], + isAccountOwner: [false], + accountUser: [ + null, + [ + CdValidators.requiredIf({ + isAccountOwner: true + }) + ] + ] }); } ngOnInit() { const promises = { - owners: this.rgwUserService.enumerate() + owners: this.rgwUserService.enumerate(true), + accounts: this.rgwAccountsService.list() }; this.multisiteStatus$ = this.rgwMultisiteService.status(); this.isDefaultZoneGroup$ = this.rgwDaemonService.selectedDaemon$.pipe( @@ -217,8 +239,9 @@ export class RgwBucketFormComponent extends CdForm implements OnInit, AfterViewC forkJoin(promises).subscribe((data: any) => { // Get the list of possible owners. - this.owners = (data.owners).sort(); - + this.accounts = data.accounts; + this.accountUsers = data.owners.filter((owner: RgwUser) => owner.account_id); + this.owners = data.owners.filter((owner: RgwUser) => !owner.account_id); // Get placement targets: if (data['getPlacementTargets']) { const placementTargets = data['getPlacementTargets']; @@ -269,13 +292,21 @@ 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 + // 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(); + const ownersList = data.owners.map((owner: RgwUser) => owner.uid); + if (!ownersList.includes(value['owner'])) { + // creating dummy user object to show the account owner + // here value['owner] is the account user id + const user = Object.assign( + { uid: value['owner'] }, + ownersList.find((owner: RgwUser) => owner.uid === AppConstants.defaultUser) + ); + this.accountUsers.push(user); + this.bucketForm.get('isAccountOwner').setValue(true); + this.bucketForm.get('isAccountOwner').disable(); + this.bucketForm.get('accountUser').setValue(value['owner']); + this.bucketForm.get('accountUser').disable(); } this.isVersioningAlreadyEnabled = this.isVersioningEnabled; this.isMfaDeleteAlreadyEnabled = this.isMfaDeleteEnabled; @@ -340,7 +371,19 @@ export class RgwBucketFormComponent extends CdForm implements OnInit, AfterViewC // 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']; + let owner; + if (this.bucketForm.get('accountUser').disabled) { + // If the bucket is owned by the account, then set the owner as account user. + owner = ''; + } else if (values['isAccountOwner']) { + const accountUser: RgwUser = this.accountUsers.filter( + (user) => values['accountUser'] === user.uid + )[0]; + owner = accountUser?.account_id ?? values['owner']; + } else { + owner = values['owner']; + } + this.rgwBucketService .update( values['bid'], @@ -376,11 +419,12 @@ export class RgwBucketFormComponent extends CdForm implements OnInit, AfterViewC } ); } else { + const owner = values['isAccountOwner'] ? values['accountUser'] : values['owner']; // Add this.rgwBucketService .create( values['bid'], - values['owner'], + owner, this.zonegroup, values['placement-target'], values['lock_enabled'], @@ -421,7 +465,8 @@ export class RgwBucketFormComponent extends CdForm implements OnInit, AfterViewC * Scenario 2: Changing the bucket owner from non-tenanted to tenanted * Scenario 3: Keeping the owner(tenanted) same and changing only rate limit */ - const owner = this.bucketForm.getValue('owner'); + // in case of creating bucket with account user, owner will be empty + const owner = this.bucketForm.getValue('owner') || ''; const bidInput = this.bucketForm.getValue('bid'); let bid: string; diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-user.service.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-user.service.spec.ts index 7884f2385f22d..e833c5a6dde0f 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-user.service.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-user.service.spec.ts @@ -34,7 +34,9 @@ describe('RgwUserService', () => { service.list().subscribe((resp) => { result = resp; }); - const req = httpTesting.expectOne(`api/rgw/user?${RgwHelper.DAEMON_QUERY_PARAM}`); + const req = httpTesting.expectOne( + `api/rgw/user?${RgwHelper.DAEMON_QUERY_PARAM}&detailed=false` + ); expect(req.request.method).toBe('GET'); req.flush([]); expect(result).toEqual([]); @@ -45,7 +47,7 @@ describe('RgwUserService', () => { service.list().subscribe((resp) => { result = resp; }); - let req = httpTesting.expectOne(`api/rgw/user?${RgwHelper.DAEMON_QUERY_PARAM}`); + let req = httpTesting.expectOne(`api/rgw/user?${RgwHelper.DAEMON_QUERY_PARAM}&detailed=false`); expect(req.request.method).toBe('GET'); req.flush(['foo', 'bar']); @@ -62,7 +64,9 @@ describe('RgwUserService', () => { it('should call enumerate', () => { service.enumerate().subscribe(); - const req = httpTesting.expectOne(`api/rgw/user?${RgwHelper.DAEMON_QUERY_PARAM}`); + const req = httpTesting.expectOne( + `api/rgw/user?${RgwHelper.DAEMON_QUERY_PARAM}&detailed=false` + ); expect(req.request.method).toBe('GET'); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-user.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-user.service.ts index ecedcdc4e4152..c691085a4aab6 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-user.service.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-user.service.ts @@ -41,9 +41,10 @@ export class RgwUserService { * Get the list of usernames. * @return {Observable} */ - enumerate() { + enumerate(detailed: boolean = false) { return this.rgwDaemonService.request((params: HttpParams) => { - return this.http.get(this.url, { params: params }); + params = params.append('detailed', detailed); + return this.http.get(this.url, { params }); }); } diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/constants/app.constants.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/constants/app.constants.ts index 05423c6ed08d9..2514f1f87cd63 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/constants/app.constants.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/constants/app.constants.ts @@ -5,6 +5,7 @@ import { environment } from '~/environments/environment'; export class AppConstants { public static readonly organization = 'ceph'; public static readonly projectName = 'Ceph Dashboard'; + public static readonly defaultUser = 'dashboard'; public static readonly license = 'Free software (LGPL 2.1).'; public static readonly copyright = 'Copyright(c) ' + environment.year + ' Ceph contributors.'; public static readonly cephLogo = 'assets/Ceph_Logo.svg'; diff --git a/src/pybind/mgr/dashboard/openapi.yaml b/src/pybind/mgr/dashboard/openapi.yaml index 8c509673086de..04c976c2c6952 100755 --- a/src/pybind/mgr/dashboard/openapi.yaml +++ b/src/pybind/mgr/dashboard/openapi.yaml @@ -13816,6 +13816,13 @@ paths: name: daemon_name schema: type: string + - default: false + description: If true, returns complete user details for each user. If false, + returns only the list of usernames. + in: query + name: detailed + schema: + type: boolean responses: '200': content: diff --git a/src/pybind/mgr/dashboard/services/rgw_client.py b/src/pybind/mgr/dashboard/services/rgw_client.py index 68744389e90a4..052b344d90080 100755 --- a/src/pybind/mgr/dashboard/services/rgw_client.py +++ b/src/pybind/mgr/dashboard/services/rgw_client.py @@ -1097,7 +1097,7 @@ class RgwClient(RestClient): destination = ET.SubElement(rule, 'Destination') bucket = ET.SubElement(destination, 'Bucket') - bucket.text = bucket_name + bucket.text = 'arn:aws:s3:::'f'{bucket_name}' replication_config = ET.tostring(root, encoding='utf-8', method='xml').decode()