mgr/dashboard : fix-non-versioning-bucket-issue

fixes : https://tracker.ceph.com/issues/76109
Signed-off-by: Abhishek Desai <abhishek.desai1@ibm.com>

diff --git a/qa/tasks/mgr/dashboard/test_rgw.py b/qa/tasks/mgr/dashboard/test_rgw.py
index 96936f55047f8fa7e670bd71dd9da8b2402fdcac..3f385f79a27f29c45e6553d1a9a21e68ef52eea7 100644 (file)
--- a/qa/tasks/mgr/dashboard/test_rgw.py
+++ b/qa/tasks/mgr/dashboard/test_rgw.py
@@ -204,7 +204,7 @@ class RgwBucketTest(RgwTestCase):
         self.assertEqual(data['bucket'], 'teuth-test-bucket')
         self.assertEqual(data['owner'], 'admin')
         self.assertEqual(data['placement_rule'], 'default-placement')
-        self.assertEqual(data['versioning'], 'Suspended')
+        self.assertEqual(data['versioning'], 'Off')
 
         # Update bucket: change owner, enable versioning.
         self._put(
@@ -312,7 +312,7 @@ class RgwBucketTest(RgwTestCase):
         # Get the bucket.
         data = _verify_tenant_bucket('teuth-test-bucket', 'testx', 'teuth-test-user')
         self.assertEqual(data['placement_rule'], 'default-placement')
-        self.assertEqual(data['versioning'], 'Suspended')
+        self.assertEqual(data['versioning'], 'Off')
 
         # Update bucket: different user with different tenant, enable versioning.
         self._put(

diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/models/rgw-bucket-versioning.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/models/rgw-bucket-versioning.ts
index 51048c65e7b39ec0040418822b49c8a745440fbf..77160c0c5cba563e33ea0013a3cfdbd48ce8adea 100644 (file)
--- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/models/rgw-bucket-versioning.ts
+++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/rgw/models/rgw-bucket-versioning.ts
@@ -1,4 +1,5 @@
 export enum RgwBucketVersioning {
   ENABLED = 'Enabled',
-  SUSPENDED = 'Suspended'
+  SUSPENDED = 'Suspended',
+  OFF = 'Off'
 }

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 512f0462c19054d3540c738b3de0d9d8a269c2ec..6a1ed657572140a3321964ffa1702347cdb17cf8 100644 (file)
--- 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
@@ -68,6 +68,7 @@ export class RgwBucketFormComponent extends CdForm implements OnInit, AfterViewC
   placementTargets: object[] = [];
   isVersioningAlreadyEnabled = false;
   isMfaDeleteAlreadyEnabled = false;
+  initialVersioningStatus: string | null = null;
   icons = Icons;
   kmsConfigured = false;
   s3Configured = false;
@@ -267,6 +268,9 @@ export class RgwBucketFormComponent extends CdForm implements OnInit, AfterViewC
 
         if (data['getBid']) {
           const bidResp = data['getBid'];
+          if (this.editing) {
+            this.initialVersioningStatus = bidResp['versioning'] ?? null;
+          }
           // Get the default values (incl. the values from disabled fields).
           const defaults = _.clone(this.bucketForm.getRawValue());
 
@@ -526,8 +530,14 @@ export class RgwBucketFormComponent extends CdForm implements OnInit, AfterViewC
     mfaTokenPinControl.updateValueAndValidity();
   }
 
-  getVersioningStatus() {
-    return this.isVersioningEnabled ? RgwBucketVersioning.ENABLED : RgwBucketVersioning.SUSPENDED;
+  getVersioningStatus(): string {
+    if (this.isVersioningEnabled) {
+      return RgwBucketVersioning.ENABLED;
+    }
+    if (this.editing && this.initialVersioningStatus === RgwBucketVersioning.OFF) {
+      return '';
+    }
+    return RgwBucketVersioning.SUSPENDED;
   }
 
   getMfaDeleteStatus() {

diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-bucket.service.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-bucket.service.spec.ts
index 6375cf3d4eacbcf2be19efb607dce31c058953e5..d09ca9ae8f2e8bee1938226ad3d6eec7311e4165 100644 (file)
--- a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-bucket.service.spec.ts
+++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-bucket.service.spec.ts
@@ -95,7 +95,35 @@ describe('RgwBucketService', () => {
       )
       .subscribe();
     const req = httpTesting.expectOne(
-      `api/rgw/bucket/foo?${RgwHelper.DAEMON_QUERY_PARAM}&bucket_id=bar&uid=baz&versioning_state=Enabled&encryption_state=true&encryption_type=aws%253Akms&key_id=qwerty1&mfa_delete=Enabled&mfa_token_serial=1&mfa_token_pin=223344&lock_mode=GOVERNANCE&lock_retention_period_days=10&tags=null&bucket_policy=null&canned_acl=private&replication=true&lifecycle=null`
+      `api/rgw/bucket/foo?${RgwHelper.DAEMON_QUERY_PARAM}&bucket_id=bar&uid=baz&encryption_state=true&encryption_type=aws%253Akms&key_id=qwerty1&mfa_delete=Enabled&mfa_token_serial=1&mfa_token_pin=223344&lock_mode=GOVERNANCE&lock_retention_period_days=10&tags=null&bucket_policy=null&canned_acl=private&replication=true&lifecycle=null&versioning_state=Enabled`
+    );
+    expect(req.request.method).toBe('PUT');
+  });
+
+  it('should call update without versioning_state when empty', () => {
+    service
+      .update(
+        'foo',
+        'bar',
+        'baz',
+        '',
+        true,
+        'aws:kms',
+        'qwerty1',
+        'Enabled',
+        '1',
+        '223344',
+        'GOVERNANCE',
+        '10',
+        null,
+        null,
+        'private',
+        'true',
+        null
+      )
+      .subscribe();
+    const req = httpTesting.expectOne(
+      `api/rgw/bucket/foo?${RgwHelper.DAEMON_QUERY_PARAM}&bucket_id=bar&uid=baz&encryption_state=true&encryption_type=aws%253Akms&key_id=qwerty1&mfa_delete=Enabled&mfa_token_serial=1&mfa_token_pin=223344&lock_mode=GOVERNANCE&lock_retention_period_days=10&tags=null&bucket_policy=null&canned_acl=private&replication=true&lifecycle=null`
     );
     expect(req.request.method).toBe('PUT');
   });

diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-bucket.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-bucket.service.ts
index 5f82e30fc081ec93a8e7a236d9ff9c36c3d3d4a9..cb7f7023d39f15fbd0cd6c9e4748aa2ead3ed440 100644 (file)
--- a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-bucket.service.ts
+++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/rgw-bucket.service.ts
@@ -170,10 +170,9 @@ export class RgwBucketService extends ApiClient {
     lifecycle: string
   ) {
     return this.rgwDaemonService.request((params: HttpParams) => {
-      params = params.appendAll({
+      const paramsObject: Record<string, string> = {
         bucket_id: bucketId,
         uid: uid,
-        versioning_state: versioningState,
         encryption_state: String(encryptionState),
         encryption_type: encryptionType,
         key_id: keyId,
@@ -187,7 +186,11 @@ export class RgwBucketService extends ApiClient {
         canned_acl: cannedAcl,
         replication: replication,
         lifecycle: lifecycle
-      });
+      };
+      if (versioningState) {
+        paramsObject['versioning_state'] = versioningState;
+      }
+      params = params.appendAll(paramsObject);
       return this.http.put(`${this.url}/${bucket}`, null, { params: params });
     });
   }

diff --git a/src/pybind/mgr/dashboard/services/rgw_client.py b/src/pybind/mgr/dashboard/services/rgw_client.py
index 881cb0c63723d9236b369c43acb300f3811c136d..644df47f21320202c307a26dafa5b17dd160ccb1 100755 (executable)
--- a/src/pybind/mgr/dashboard/services/rgw_client.py
+++ b/src/pybind/mgr/dashboard/services/rgw_client.py
@@ -599,8 +599,12 @@ class RgwClient(RestClient):
         """
         # pylint: disable=unused-argument
         result = request()
-        if 'Status' not in result:
-            result['Status'] = 'Suspended'
+        if not isinstance(result, dict):
+            result = {}
+        # RGW omits Status when versioning has never been configured (CLI/radosgw-admin
+        # reports "off"). That must not be shown as "Suspended", which means versioning was enabled
+        if not result.get('Status'):
+            result['Status'] = 'Off'
         if 'MfaDelete' not in result:
             result['MfaDelete'] = 'Disabled'
         return result