Merge pull request ceph#64362 from rhcs-dashboard/edit-bucket-with-account

afreen23 · web-flow · commit df7f3e1c57bf · 2025-07-14T12:44:49.000+05:30
mgr/dashboard: differentiate account users from rgw users in bucket form

Reviewed-by: Afreen Misbah &lt;afreen@ibm.com&gt;
Reviewed-by: Nizamudeen A &lt;nia@redhat.com&gt;
Reviewed-by: Ankush Behl &lt;cloudbehl@gmail.com&gt;
diff --git a/src/pybind/mgr/dashboard/controllers/rgw.py b/src/pybind/mgr/dashboard/controllers/rgw.py
@@ -626,13 +626,21 @@ def set(self, bucket, bucket_id, uid=None, versioning_state=None,
                 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 @@ def _keys_allowed():
             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
@@ -67,38 +67,67 @@
       </ng-template>
     </div>
 
-    <!-- Owner -->
-    <div class="form-item">
-      <cds-select label="Owner"
-                  for="owner"
-                  formControlName="owner"
-                  name="owner"
-                  id="owner"
-                  [invalidText]="ownerError"
-                  [invalid]="!bucketForm.controls.owner.valid && bucketForm.controls.owner.dirty"
-                  cdRequiredField="Owner"
-                  i18n>Owner
-        <option *ngIf="owners === null"
-                [ngValue]="null">Loading...</option>
-        <option *ngIf="owners !== null"
-                value="">-- Select a user --</option>
-        <option *ngFor="let owner of owners"
-                [value]="owner">{{ owner }}</option>
-      </cds-select>
-      <ng-template #ownerError>
-        <span class="invalid-feedback"
-              *ngIf="bucketForm.showError('owner', frm, 'required')"
-              i18n>This field is required.</span>
-      </ng-template>
-      <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>
+    <!-- Accounts -->
+    @if(accounts.length && accountUsers.length > 0){
+      <cds-checkbox formControlName="isAccountOwner"
+                    i18n>Select account user
+      </cds-checkbox>
+    }
+
+    @if (bucketForm.get('isAccountOwner').value) {
+      <!-- Account user -->
+      <div class="form-item">
+        <cds-select label="Account user"
+                    id="acc_user"
+                    formControlName="accountUser"
+                    [invalidText]="accountUserError"
+                    [invalid]="!bucketForm.controls.accountUser.valid && bucketForm.controls.accountUser.dirty"
+                    cdRequiredField="Account user"
+                    i18n>Account user
+          <option *ngIf="accountusers === null"
+                  [ngValue]="null">Loading...</option>
+          <option *ngIf="accountusers !== null"
+                  value="">-- Select a user --</option>
+          <option *ngFor="let user of accountUsers"
+                  [value]="user.uid">{{ user.uid }}</option>
+        </cds-select>
+        <ng-template #accountUserError>
+          <span class="invalid-feedback"
+                *ngIf="bucketForm.showError('accountUser', frm, 'required')"
+                i18n>This field is required.</span>
+        </ng-template>
+        <cd-alert-panel
+          type="info"
+          *ngIf="bucketForm.get('accountUser').disabled"
+          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>
+    } @else {
+      <!-- Owner -->
+      <div class="form-item">
+        <cds-select label="Owner"
+                    formControlName="owner"
+                    id="owner"
+                    [invalidText]="ownerError"
+                    [invalid]="!bucketForm.controls.owner.valid && bucketForm.controls.owner.dirty"
+                    cdRequiredField="Owner"
+                    i18n>Owner
+          <option *ngIf="owners === null"
+                  [ngValue]="null">Loading...</option>
+          <option *ngIf="owners !== null"
+                  value="">-- Select a user --</option>
+          <option *ngFor="let owner of owners"
+                  [value]="owner.uid">{{ owner.uid }}</option>
+        </cds-select>
+        <ng-template #ownerError>
+          <span class="invalid-feedback"
+                *ngIf="bucketForm.showError('owner', frm, 'required')"
+                i18n>This field is required.</span>
+        </ng-template>
+      </div>
+    }
 
     <!-- Versioning -->
     <fieldset *ngIf="editing">
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
@@ -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
@@ -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 = (<string[]>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
@@ -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
@@ -41,9 +41,10 @@ export class RgwUserService {
    * Get the list of usernames.
    * @return {Observable<string[]>}
    */
-  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
diff --git a/src/pybind/mgr/dashboard/openapi.yaml b/src/pybind/mgr/dashboard/openapi.yaml
diff --git a/src/pybind/mgr/dashboard/services/rgw_client.py b/src/pybind/mgr/dashboard/services/rgw_client.py
