fix bug: management section id not match the key defined in capabilit…

…ies (#6421) * fix management section id not match the key defined in capabilities Signed-off-by: Yulong Ruan <[email protected]> * add descriptions on how the PR solved the problme Signed-off-by: Yulong Ruan <[email protected]> --------- Signed-off-by: Yulong Ruan <[email protected]>
opensearch-project · Aug 7, 2024 · a65a8aa · a65a8aa
1 parent aa807e5
commit a65a8aa
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 16 deletions.
diff --git a/src/plugins/management/common/contants.ts b/src/plugins/management/common/contants.ts
@@ -29,3 +29,17 @@
  */
 
 export const MANAGEMENT_APP_ID = 'management';
+export const DEFAULT_MANAGEMENT_CAPABILITIES = {
+  management: {
+    /*
+     * Management settings correspond to management section/link ids, and should not be changed
+     * without also updating those definitions.
+     */
+    opensearchDashboards: {
+      settings: true,
+      indexPatterns: true,
+      objects: true,
+      dataSources: true,
+    },
+  },
+};
diff --git a/src/plugins/management/public/management_sections_service.test.ts b/src/plugins/management/public/management_sections_service.test.ts
@@ -28,6 +28,7 @@
  * under the License.
  */
 
+import { DEFAULT_MANAGEMENT_CAPABILITIES } from '../common/contants';
 import {
   ManagementSectionsService,
   getSectionsServiceStartPrivate,
@@ -105,4 +106,31 @@ describe('ManagementService', () => {
       ]
     `);
   });
+
+  it('should disable apps register in predefined opensearchDashboards section', () => {
+    // The management capabilities has `opensearchDashboards` as the key
+    const originalDataSourcesCapability =
+      DEFAULT_MANAGEMENT_CAPABILITIES.management.opensearchDashboards.dataSources;
+
+    const setup = managementService.setup();
+
+    // The predefined opensearchDashboards section has id `opensearch-dashboards` which
+    // doesn't match the capability id `opensearchDashboards`
+    setup.section.opensearchDashboards.registerApp({
+      id: 'dataSources',
+      title: 'Data source',
+      mount: jest.fn(),
+    });
+
+    // Now set dataSources to capability to false should disable
+    // the dataSources app registered in opensearchDashboards section
+    DEFAULT_MANAGEMENT_CAPABILITIES.management.opensearchDashboards.dataSources = false;
+
+    managementService.start({ capabilities: DEFAULT_MANAGEMENT_CAPABILITIES });
+    expect(
+      setup.section.opensearchDashboards.apps.find((app) => app.id === 'dataSources')?.enabled
+    ).toBe(false);
+
+    DEFAULT_MANAGEMENT_CAPABILITIES.management.opensearchDashboards.dataSources = originalDataSourcesCapability;
+  });
 });
diff --git a/src/plugins/management/public/management_sections_service.ts b/src/plugins/management/public/management_sections_service.ts
@@ -51,6 +51,22 @@ const [getSectionsServiceStartPrivate, setSectionsServiceStartPrivate] = createG
   ManagementSectionsStartPrivate
 >('SectionsServiceStartPrivate');
 
+/**
+ * The management capabilities has `opensearchDashboards` as the key
+ * While when registering the opensearchDashboards section, the section id is `opensearch-dashboards`
+ * as defined in {@link ManagementSectionId.OpenSearchDashboards} and section id is used as the capability
+ * id. Here we have a mapping so that the section id opensearch-dashboards can mapping correctly back to the
+ * capability id: opensearchDashboards
+ *
+ * Why not directly change the capability id to opensearch-dashboards?
+ * The issue was introduced in https://github.com/opensearch-project/OpenSearch-Dashboards/pull/260
+ * Since then, the capability id `opensearchDashboards` has been used by plugins, having a mapping here
+ * is for backward compatibility
+ */
+const MANAGEMENT_ID_TO_CAPABILITIES: Record<string, string> = {
+  'opensearch-dashboards': 'opensearchDashboards',
+};
+
 export { getSectionsServiceStartPrivate };
 
 export class ManagementSectionsService {
@@ -94,8 +110,9 @@ export class ManagementSectionsService {
 
   start({ capabilities }: SectionsServiceStartDeps) {
     this.getAllSections().forEach((section) => {
-      if (capabilities.management.hasOwnProperty(section.id)) {
-        const sectionCapabilities = capabilities.management[section.id];
+      const capabilityId = MANAGEMENT_ID_TO_CAPABILITIES[section.id] || section.id;
+      if (capabilities.management.hasOwnProperty(capabilityId)) {
+        const sectionCapabilities = capabilities.management[capabilityId];
         section.apps.forEach((app) => {
           if (sectionCapabilities.hasOwnProperty(app.id) && sectionCapabilities[app.id] !== true) {
             app.disable();

diff --git a/src/plugins/management/server/capabilities_provider.ts b/src/plugins/management/server/capabilities_provider.ts
@@ -28,17 +28,6 @@
  * under the License.
  */
 
-export const capabilitiesProvider = () => ({
-  management: {
-    /*
-     * Management settings correspond to management section/link ids, and should not be changed
-     * without also updating those definitions.
-     */
-    opensearchDashboards: {
-      settings: true,
-      indexPatterns: true,
-      objects: true,
-      dataSources: true,
-    },
-  },
-});
+import { DEFAULT_MANAGEMENT_CAPABILITIES } from '../common/contants';
+
+export const capabilitiesProvider = () => DEFAULT_MANAGEMENT_CAPABILITIES;