Skip to content

Commit c5b5408

Browse files
author
Naman Munet
committed
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 <[email protected]>
1 parent 665ffb9 commit c5b5408

File tree

9 files changed

+170
-63
lines changed

9 files changed

+170
-63
lines changed

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

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -626,13 +626,21 @@ def set(self, bucket, bucket_id, uid=None, versioning_state=None,
626626
bucket = '/{}'.format(bucket)
627627

628628
# Link bucket to new user:
629+
params = {
630+
'bucket': bucket,
631+
'bucket-id': bucket_id,
632+
}
633+
634+
accounts = RgwAccounts().get_accounts()
635+
if uid in accounts:
636+
# If the bucket is owned by an account, we need to use the account-id
637+
# instead of uid.
638+
params['account-id'] = uid
639+
else:
640+
params['uid'] = uid
629641
result = self.proxy(daemon_name,
630642
'PUT',
631-
'bucket', {
632-
'bucket': bucket,
633-
'bucket-id': bucket_id,
634-
'uid': uid
635-
},
643+
'bucket', params,
636644
json_response=False)
637645

638646
uid_tenant = uid[:uid.find('$')] if uid.find('$') >= 0 else None
@@ -811,16 +819,24 @@ def _keys_allowed():
811819
and len(set(edit_permissions).intersection(set(permissions[Scope.RGW]))) > 0
812820

813821
@EndpointDoc("Display RGW Users",
822+
parameters={
823+
'detailed': (bool, "If true, returns complete user details for each user. "
824+
"If false, returns only the list of usernames.")
825+
},
814826
responses={200: RGW_USER_SCHEMA})
815-
def list(self, daemon_name=None):
816-
# type: (Optional[str]) -> List[str]
817-
users = [] # type: List[str]
827+
def list(self, daemon_name=None, detailed: bool = False):
828+
detailed = str_to_bool(detailed)
829+
users = [] # type: List[Union[str, Dict[str, Any]]]
818830
marker = None
819831
while True:
820832
params = {} # type: dict
821833
if marker:
822834
params['marker'] = marker
823835
result = self.proxy(daemon_name, 'GET', 'user?list', params)
836+
if detailed:
837+
for user in result['keys']:
838+
users.append(self._get(user, daemon_name=daemon_name, stats=False))
839+
return users
824840
users.extend(result['keys'])
825841
if not result['truncated']:
826842
break

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

Lines changed: 61 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -67,38 +67,67 @@
6767
</ng-template>
6868
</div>
6969

70-
<!-- Owner -->
71-
<div class="form-item">
72-
<cds-select label="Owner"
73-
for="owner"
74-
formControlName="owner"
75-
name="owner"
76-
id="owner"
77-
[invalidText]="ownerError"
78-
[invalid]="!bucketForm.controls.owner.valid && bucketForm.controls.owner.dirty"
79-
cdRequiredField="Owner"
80-
i18n>Owner
81-
<option *ngIf="owners === null"
82-
[ngValue]="null">Loading...</option>
83-
<option *ngIf="owners !== null"
84-
value="">-- Select a user --</option>
85-
<option *ngFor="let owner of owners"
86-
[value]="owner">{{ owner }}</option>
87-
</cds-select>
88-
<ng-template #ownerError>
89-
<span class="invalid-feedback"
90-
*ngIf="bucketForm.showError('owner', frm, 'required')"
91-
i18n>This field is required.</span>
92-
</ng-template>
93-
<cd-alert-panel
94-
type="info"
95-
*ngIf="bucketForm.get('owner').disabled"
96-
spacingClass="me-1 mt-1"
97-
i18n>
98-
The bucket is owned by an account. UI does not support changing
99-
the ownership of bucket owned by an account.
100-
</cd-alert-panel>
101-
</div>
70+
<!-- Accounts -->
71+
@if(accounts.length && accountUsers.length > 0){
72+
<cds-checkbox formControlName="isAccountOwner"
73+
i18n>Select account user
74+
</cds-checkbox>
75+
}
76+
77+
@if (bucketForm.get('isAccountOwner').value) {
78+
<!-- Account user -->
79+
<div class="form-item">
80+
<cds-select label="Account user"
81+
id="acc_user"
82+
formControlName="accountUser"
83+
[invalidText]="accountUserError"
84+
[invalid]="!bucketForm.controls.accountUser.valid && bucketForm.controls.accountUser.dirty"
85+
cdRequiredField="Account user"
86+
i18n>Account user
87+
<option *ngIf="accountusers === null"
88+
[ngValue]="null">Loading...</option>
89+
<option *ngIf="accountusers !== null"
90+
value="">-- Select a user --</option>
91+
<option *ngFor="let user of accountUsers"
92+
[value]="user.uid">{{ user.uid }}</option>
93+
</cds-select>
94+
<ng-template #accountUserError>
95+
<span class="invalid-feedback"
96+
*ngIf="bucketForm.showError('accountUser', frm, 'required')"
97+
i18n>This field is required.</span>
98+
</ng-template>
99+
<cd-alert-panel
100+
type="info"
101+
*ngIf="bucketForm.get('accountUser').disabled"
102+
i18n>
103+
The bucket is owned by an account. UI does not support changing
104+
the ownership of bucket owned by an account.
105+
</cd-alert-panel>
106+
</div>
107+
} @else {
108+
<!-- Owner -->
109+
<div class="form-item">
110+
<cds-select label="Owner"
111+
formControlName="owner"
112+
id="owner"
113+
[invalidText]="ownerError"
114+
[invalid]="!bucketForm.controls.owner.valid && bucketForm.controls.owner.dirty"
115+
cdRequiredField="Owner"
116+
i18n>Owner
117+
<option *ngIf="owners === null"
118+
[ngValue]="null">Loading...</option>
119+
<option *ngIf="owners !== null"
120+
value="">-- Select a user --</option>
121+
<option *ngFor="let owner of owners"
122+
[value]="owner.uid">{{ owner.uid }}</option>
123+
</cds-select>
124+
<ng-template #ownerError>
125+
<span class="invalid-feedback"
126+
*ngIf="bucketForm.showError('owner', frm, 'required')"
127+
i18n>This field is required.</span>
128+
</ng-template>
129+
</div>
130+
}
102131

103132
<!-- Versioning -->
104133
<fieldset *ngIf="editing">

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import { CheckboxModule, SelectModule } from 'carbon-components-angular';
2323
import { NO_ERRORS_SCHEMA } from '@angular/core';
2424
import { By } from '@angular/platform-browser';
2525
import { LoadingStatus } from '~/app/shared/forms/cd-form';
26+
import { RgwUserAccountsService } from '~/app/shared/api/rgw-user-accounts.service';
2627

2728
describe('RgwBucketFormComponent', () => {
2829
let component: RgwBucketFormComponent;
@@ -31,6 +32,7 @@ describe('RgwBucketFormComponent', () => {
3132
let getPlacementTargetsSpy: jasmine.Spy;
3233
let rgwBucketServiceGetSpy: jasmine.Spy;
3334
let enumerateSpy: jasmine.Spy;
35+
let accountListSpy: jasmine.Spy;
3436
let formHelper: FormHelper;
3537
let childComponent: RgwRateLimitComponent;
3638

@@ -55,6 +57,7 @@ describe('RgwBucketFormComponent', () => {
5557
rgwBucketServiceGetSpy = spyOn(rgwBucketService, 'get');
5658
getPlacementTargetsSpy = spyOn(TestBed.inject(RgwSiteService), 'get');
5759
enumerateSpy = spyOn(TestBed.inject(RgwUserService), 'enumerate');
60+
accountListSpy = spyOn(TestBed.inject(RgwUserAccountsService), 'list');
5861
formHelper = new FormHelper(component.bucketForm);
5962
});
6063

@@ -85,6 +88,7 @@ describe('RgwBucketFormComponent', () => {
8588
};
8689
getPlacementTargetsSpy.and.returnValue(observableOf(payload));
8790
enumerateSpy.and.returnValue(observableOf([]));
91+
accountListSpy.and.returnValue(observableOf([]));
8892
fixture.detectChanges();
8993

9094
expect(component.zonegroup).toBe(payload.zonegroup);

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

Lines changed: 62 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import * as xml2js from 'xml2js';
1616
import { RgwBucketService } from '~/app/shared/api/rgw-bucket.service';
1717
import { RgwSiteService } from '~/app/shared/api/rgw-site.service';
1818
import { RgwUserService } from '~/app/shared/api/rgw-user.service';
19-
import { ActionLabelsI18n, URLVerbs } from '~/app/shared/constants/app.constants';
19+
import { ActionLabelsI18n, AppConstants, URLVerbs } from '~/app/shared/constants/app.constants';
2020
import { Icons } from '~/app/shared/enum/icons.enum';
2121
import { NotificationType } from '~/app/shared/enum/notification-type.enum';
2222
import { CdForm } from '~/app/shared/forms/cd-form';
@@ -41,6 +41,8 @@ import { TextAreaXmlFormatterService } from '~/app/shared/services/text-area-xml
4141
import { RgwRateLimitComponent } from '../rgw-rate-limit/rgw-rate-limit.component';
4242
import { RgwRateLimitConfig } from '../models/rgw-rate-limit';
4343
import { ModalCdsService } from '~/app/shared/services/modal-cds.service';
44+
import { RgwUserAccountsService } from '~/app/shared/api/rgw-user-accounts.service';
45+
import { RgwUser } from '../models/rgw-user';
4446

4547
@Component({
4648
selector: 'cd-rgw-bucket-form',
@@ -55,7 +57,9 @@ export class RgwBucketFormComponent extends CdForm implements OnInit, AfterViewC
5557

5658
bucketForm: CdFormGroup;
5759
editing = false;
58-
owners: string[] = null;
60+
owners: RgwUser[] = null;
61+
accounts: string[] = [];
62+
accountUsers: RgwUser[] = [];
5963
kmsProviders: string[] = null;
6064
action: string;
6165
resource: string;
@@ -104,7 +108,8 @@ export class RgwBucketFormComponent extends CdForm implements OnInit, AfterViewC
104108
public actionLabels: ActionLabelsI18n,
105109
private readonly changeDetectorRef: ChangeDetectorRef,
106110
private rgwMultisiteService: RgwMultisiteService,
107-
private rgwDaemonService: RgwDaemonService
111+
private rgwDaemonService: RgwDaemonService,
112+
private rgwAccountsService: RgwUserAccountsService
108113
) {
109114
super();
110115
this.editing = this.router.url.startsWith(`/rgw/bucket/${URLVerbs.EDIT}`);
@@ -137,7 +142,14 @@ export class RgwBucketFormComponent extends CdForm implements OnInit, AfterViewC
137142
? []
138143
: [CdValidators.bucketName(), CdValidators.bucketExistence(false, this.rgwBucketService)]
139144
],
140-
owner: [null, [Validators.required]],
145+
owner: [
146+
null,
147+
[
148+
CdValidators.requiredIf({
149+
isAccountOwner: false
150+
})
151+
]
152+
],
141153
kms_provider: ['vault'],
142154
'placement-target': [null],
143155
versioning: [null],
@@ -169,13 +181,23 @@ export class RgwBucketFormComponent extends CdForm implements OnInit, AfterViewC
169181
lifecycle: ['{}', CdValidators.jsonOrXml()],
170182
grantee: [Grantee.Owner, [Validators.required]],
171183
aclPermission: [[aclPermission.FullControl], [Validators.required]],
172-
replication: [false]
184+
replication: [false],
185+
isAccountOwner: [false],
186+
accountUser: [
187+
null,
188+
[
189+
CdValidators.requiredIf({
190+
isAccountOwner: true
191+
})
192+
]
193+
]
173194
});
174195
}
175196

176197
ngOnInit() {
177198
const promises = {
178-
owners: this.rgwUserService.enumerate()
199+
owners: this.rgwUserService.enumerate(true),
200+
accounts: this.rgwAccountsService.list()
179201
};
180202
this.multisiteStatus$ = this.rgwMultisiteService.status();
181203
this.isDefaultZoneGroup$ = this.rgwDaemonService.selectedDaemon$.pipe(
@@ -217,8 +239,9 @@ export class RgwBucketFormComponent extends CdForm implements OnInit, AfterViewC
217239

218240
forkJoin(promises).subscribe((data: any) => {
219241
// Get the list of possible owners.
220-
this.owners = (<string[]>data.owners).sort();
221-
242+
this.accounts = data.accounts;
243+
this.accountUsers = data.owners.filter((owner: RgwUser) => owner.account_id);
244+
this.owners = data.owners.filter((owner: RgwUser) => !owner.account_id);
222245
// Get placement targets:
223246
if (data['getPlacementTargets']) {
224247
const placementTargets = data['getPlacementTargets'];
@@ -269,13 +292,21 @@ export class RgwBucketFormComponent extends CdForm implements OnInit, AfterViewC
269292
}
270293
this.bucketForm.setValue(value);
271294
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
295+
// Disable changing the owner of the bucket in case
274296
// 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();
297+
const ownersList = data.owners.map((owner: RgwUser) => owner.uid);
298+
if (!ownersList.includes(value['owner'])) {
299+
// creating dummy user object to show the account owner
300+
// here value['owner] is the account user id
301+
const user = Object.assign(
302+
{ uid: value['owner'] },
303+
ownersList.find((owner: RgwUser) => owner.uid === AppConstants.defaultUser)
304+
);
305+
this.accountUsers.push(user);
306+
this.bucketForm.get('isAccountOwner').setValue(true);
307+
this.bucketForm.get('isAccountOwner').disable();
308+
this.bucketForm.get('accountUser').setValue(value['owner']);
309+
this.bucketForm.get('accountUser').disable();
279310
}
280311
this.isVersioningAlreadyEnabled = this.isVersioningEnabled;
281312
this.isMfaDeleteAlreadyEnabled = this.isMfaDeleteEnabled;
@@ -340,7 +371,19 @@ export class RgwBucketFormComponent extends CdForm implements OnInit, AfterViewC
340371
// make the owner empty if the field is disabled.
341372
// this ensures the bucket doesn't gets updated with owner when
342373
// the bucket is owned by the account.
343-
const owner = this.bucketForm.get('owner').disabled === true ? '' : values['owner'];
374+
let owner;
375+
if (this.bucketForm.get('accountUser').disabled) {
376+
// If the bucket is owned by the account, then set the owner as account user.
377+
owner = '';
378+
} else if (values['isAccountOwner']) {
379+
const accountUser: RgwUser = this.accountUsers.filter(
380+
(user) => values['accountUser'] === user.uid
381+
)[0];
382+
owner = accountUser?.account_id ?? values['owner'];
383+
} else {
384+
owner = values['owner'];
385+
}
386+
344387
this.rgwBucketService
345388
.update(
346389
values['bid'],
@@ -376,11 +419,12 @@ export class RgwBucketFormComponent extends CdForm implements OnInit, AfterViewC
376419
}
377420
);
378421
} else {
422+
const owner = values['isAccountOwner'] ? values['accountUser'] : values['owner'];
379423
// Add
380424
this.rgwBucketService
381425
.create(
382426
values['bid'],
383-
values['owner'],
427+
owner,
384428
this.zonegroup,
385429
values['placement-target'],
386430
values['lock_enabled'],
@@ -421,7 +465,8 @@ export class RgwBucketFormComponent extends CdForm implements OnInit, AfterViewC
421465
* Scenario 2: Changing the bucket owner from non-tenanted to tenanted
422466
* Scenario 3: Keeping the owner(tenanted) same and changing only rate limit
423467
*/
424-
const owner = this.bucketForm.getValue('owner');
468+
// in case of creating bucket with account user, owner will be empty
469+
const owner = this.bucketForm.getValue('owner') || '';
425470
const bidInput = this.bucketForm.getValue('bid');
426471

427472
let bid: string;

src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-user.service.spec.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,9 @@ describe('RgwUserService', () => {
3434
service.list().subscribe((resp) => {
3535
result = resp;
3636
});
37-
const req = httpTesting.expectOne(`api/rgw/user?${RgwHelper.DAEMON_QUERY_PARAM}`);
37+
const req = httpTesting.expectOne(
38+
`api/rgw/user?${RgwHelper.DAEMON_QUERY_PARAM}&detailed=false`
39+
);
3840
expect(req.request.method).toBe('GET');
3941
req.flush([]);
4042
expect(result).toEqual([]);
@@ -45,7 +47,7 @@ describe('RgwUserService', () => {
4547
service.list().subscribe((resp) => {
4648
result = resp;
4749
});
48-
let req = httpTesting.expectOne(`api/rgw/user?${RgwHelper.DAEMON_QUERY_PARAM}`);
50+
let req = httpTesting.expectOne(`api/rgw/user?${RgwHelper.DAEMON_QUERY_PARAM}&detailed=false`);
4951
expect(req.request.method).toBe('GET');
5052
req.flush(['foo', 'bar']);
5153

@@ -62,7 +64,9 @@ describe('RgwUserService', () => {
6264

6365
it('should call enumerate', () => {
6466
service.enumerate().subscribe();
65-
const req = httpTesting.expectOne(`api/rgw/user?${RgwHelper.DAEMON_QUERY_PARAM}`);
67+
const req = httpTesting.expectOne(
68+
`api/rgw/user?${RgwHelper.DAEMON_QUERY_PARAM}&detailed=false`
69+
);
6670
expect(req.request.method).toBe('GET');
6771
});
6872

src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-user.service.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,10 @@ export class RgwUserService {
4141
* Get the list of usernames.
4242
* @return {Observable<string[]>}
4343
*/
44-
enumerate() {
44+
enumerate(detailed: boolean = false) {
4545
return this.rgwDaemonService.request((params: HttpParams) => {
46-
return this.http.get(this.url, { params: params });
46+
params = params.append('detailed', detailed);
47+
return this.http.get(this.url, { params });
4748
});
4849
}
4950

0 commit comments

Comments
 (0)