Handle data sources and advanced settings as global object. (#313)

* feat: POC implementation Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: add some comment Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: revert dependency Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: update comment Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: address one TODO Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: address TODO Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: add unit test Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: some special logic on specific operation Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: add integration test Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: declare workspaces as empty array for advanced settings Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: unified workspaces parameters when parsing from router Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: improve code coverage Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: declare workspaces as null Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: use unified types Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: update comment Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: remove null Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: address comments Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: use request app to store request workspace id Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: use app state to store request workspace id Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: remove workspaces when listing data sources Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: remove useless code change Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: throw error if the type is not allowed Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: add unit test Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: add integration test Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: change the implementation Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: remove useless change Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: remove useless change Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: add integration test Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * fix: unit test Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: add error message Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * fix: integration test Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * fix: integration test Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: remove useless change Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: add test case and add restrict on create method Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: change type Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: change comment Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: optimize test Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * refactor: move logic to conflict check wrapper Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: remove useless change Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * fix: unit test Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * fix: unit test Signed-off-by: SuZhou-Joe <suzhou@amazon.com> --------- Signed-off-by: SuZhou-Joe <suzhou@amazon.com>
ruanyl · Apr 18, 2024 · a821167 · a821167
1 parent d5bda77
commit a821167
Show file tree

Hide file tree

Showing 3 changed files with 280 additions and 4 deletions.
diff --git a/...aved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts b/...aved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts
@@ -5,14 +5,30 @@
 
 import { SavedObject } from 'src/core/types';
 import { isEqual } from 'lodash';
+import packageInfo from '../../../../../../package.json';
 import * as osdTestServer from '../../../../../core/test_helpers/osd_server';
+import { DATA_SOURCE_SAVED_OBJECT_TYPE } from '../../../../data_source/common';
 
 const dashboard: Omit<SavedObject, 'id'> = {
   type: 'dashboard',
   attributes: {},
   references: [],
 };
 
+const dataSource: Omit<SavedObject, 'id'> = {
+  type: DATA_SOURCE_SAVED_OBJECT_TYPE,
+  attributes: {
+    title: 'test data source',
+  },
+  references: [],
+};
+
+const advancedSettings: Omit<SavedObject, 'id'> = {
+  type: 'config',
+  attributes: {},
+  references: [],
+};
+
 interface WorkspaceAttributes {
   id: string;
   name?: string;
@@ -32,6 +48,9 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test',
       adjustTimeout: (t: number) => jest.setTimeout(t),
       settings: {
         osd: {
+          data_source: {
+            enabled: true,
+          },
           workspace: {
             enabled: true,
           },
@@ -150,6 +169,40 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test',
       });
     });
 
+    it('create disallowed types within workspace', async () => {
+      const createDataSourceResult = await osdTestServer.request
+        .post(root, `/api/saved_objects/${dataSource.type}`)
+        .send({
+          attributes: dataSource.attributes,
+          workspaces: [createdFooWorkspace.id],
+        })
+        .expect(400);
+
+      expect(createDataSourceResult.body).toMatchInlineSnapshot(`
+        Object {
+          "error": "Bad Request",
+          "message": "Unsupported type in workspace: 'data-source' is not allowed to create in workspace.",
+          "statusCode": 400,
+        }
+      `);
+
+      const createConfigResult = await osdTestServer.request
+        .post(root, `/api/saved_objects/config`)
+        .send({
+          attributes: advancedSettings.attributes,
+          workspaces: [createdFooWorkspace.id],
+        })
+        .expect(400);
+
+      expect(createConfigResult.body).toMatchInlineSnapshot(`
+        Object {
+          "error": "Bad Request",
+          "message": "Unsupported type in workspace: 'config' is not allowed to create in workspace.",
+          "statusCode": 400,
+        }
+      `);
+    });
+
     it('bulk create', async () => {
       await clearFooAndBar();
       const createResultFoo = await osdTestServer.request
@@ -259,6 +312,79 @@ describe('saved_objects_wrapper_for_check_workspace_conflict integration test',
       );
     });
 
+    it('bulk create with disallowed types in workspace', async () => {
+      await clearFooAndBar();
+
+      // import advanced settings and data sources should throw error
+      const createResultFoo = await osdTestServer.request
+        .post(root, `/w/${createdFooWorkspace.id}/api/saved_objects/_bulk_create`)
+        .send([
+          {
+            ...dataSource,
+            id: 'foo',
+          },
+          {
+            ...advancedSettings,
+            id: packageInfo.version,
+          },
+        ])
+        .expect(200);
+      expect(createResultFoo.body.saved_objects[0].error).toEqual(
+        expect.objectContaining({
+          message:
+            "Unsupported type in workspace: 'data-source' is not allowed to import in workspace.",
+          statusCode: 400,
+        })
+      );
+      expect(createResultFoo.body.saved_objects[1].error).toEqual(
+        expect.objectContaining({
+          message: "Unsupported type in workspace: 'config' is not allowed to import in workspace.",
+          statusCode: 400,
+        })
+      );
+
+      // Data source should not be created
+      await osdTestServer.request
+        .get(
+          root,
+          `/w/${createdFooWorkspace.id}/api/saved_objects/${DATA_SOURCE_SAVED_OBJECT_TYPE}/foo`
+        )
+        .expect(404);
+
+      // Advanced settings should not be created within workspace
+      const findAdvancedSettings = await osdTestServer.request
+        .get(root, `/w/${createdFooWorkspace.id}/api/saved_objects/_find?type=config`)
+        .expect(200);
+      expect(findAdvancedSettings.body.total).toEqual(0);
+    });
+
+    it('bulk create with disallowed types out of workspace', async () => {
+      await clearFooAndBar();
+
+      // import advanced settings and data sources should throw error
+      const createResultFoo = await osdTestServer.request
+        .post(root, `/api/saved_objects/_bulk_create`)
+        .send([
+          {
+            ...advancedSettings,
+            id: packageInfo.version,
+          },
+        ])
+        .expect(200);
+      expect(createResultFoo.body).toEqual({
+        saved_objects: [
+          expect.objectContaining({
+            type: advancedSettings.type,
+          }),
+        ],
+      });
+
+      const findAdvancedSettings = await osdTestServer.request
+        .get(root, `/api/saved_objects/_find?type=${advancedSettings.type}`)
+        .expect(200);
+      expect(findAdvancedSettings.body.total).toEqual(1);
+    });
+
     it('checkConflicts when importing ndjson', async () => {
       await clearFooAndBar();
       const createResultFoo = await osdTestServer.request

diff --git a/...workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.test.ts b/...workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.test.ts
@@ -7,6 +7,7 @@ import { SavedObject } from '../../../../core/public';
 import { httpServerMock, savedObjectsClientMock, coreMock } from '../../../../core/server/mocks';
 import { WorkspaceConflictSavedObjectsClientWrapper } from './saved_objects_wrapper_for_check_workspace_conflict';
 import { SavedObjectsSerializer } from '../../../../core/server';
+import { DATA_SOURCE_SAVED_OBJECT_TYPE } from '../../../../plugins/data_source/common';
 
 describe('WorkspaceConflictSavedObjectsClientWrapper', () => {
   const requestHandlerContext = coreMock.createRequestHandlerContext();
@@ -115,6 +116,38 @@ describe('WorkspaceConflictSavedObjectsClientWrapper', () => {
         })
       );
     });
+
+    it(`Should throw error when trying to create disallowed types in workspace`, async () => {
+      await expect(
+        wrapperClient.create(
+          DATA_SOURCE_SAVED_OBJECT_TYPE,
+          {
+            name: 'foo',
+          },
+
+          {
+            workspaces: ['foo'],
+          }
+        )
+      ).rejects.toMatchInlineSnapshot(
+        `[Error: Unsupported type in workspace: 'data-source' is not allowed to create in workspace.]`
+      );
+
+      await expect(
+        wrapperClient.create(
+          'config',
+          {
+            name: 'foo',
+          },
+
+          {
+            workspaces: ['foo'],
+          }
+        )
+      ).rejects.toMatchInlineSnapshot(
+        `[Error: Unsupported type in workspace: 'config' is not allowed to create in workspace.]`
+      );
+    });
   });
 
   describe('bulkCreateWithWorkspaceConflictCheck', () => {
@@ -291,6 +324,41 @@ describe('WorkspaceConflictSavedObjectsClientWrapper', () => {
         }
       `);
     });
+    it(`Should return error when trying to create disallowed types within a workspace`, async () => {
+      mockedClient.bulkCreate.mockResolvedValueOnce({ saved_objects: [] });
+      const result = await wrapperClient.bulkCreate(
+        [
+          getSavedObject({
+            type: 'config',
+            id: 'foo',
+          }),
+          getSavedObject({
+            type: DATA_SOURCE_SAVED_OBJECT_TYPE,
+            id: 'foo',
+          }),
+        ],
+        {
+          workspaces: ['foo'],
+        }
+      );
+
+      expect(mockedClient.bulkCreate).toBeCalledWith([], {
+        workspaces: ['foo'],
+      });
+      expect(result.saved_objects[0].error).toEqual(
+        expect.objectContaining({
+          message: "Unsupported type in workspace: 'config' is not allowed to import in workspace.",
+          statusCode: 400,
+        })
+      );
+      expect(result.saved_objects[1].error).toEqual(
+        expect.objectContaining({
+          message:
+            "Unsupported type in workspace: 'data-source' is not allowed to import in workspace.",
+          statusCode: 400,
+        })
+      );
+    });
   });
 
   describe('checkConflictWithWorkspaceConflictCheck', () => {
@@ -393,4 +461,21 @@ describe('WorkspaceConflictSavedObjectsClientWrapper', () => {
       `);
     });
   });
+
+  describe('find', () => {
+    beforeEach(() => {
+      mockedClient.find.mockClear();
+    });
+
+    it(`workspaces parameters should be removed when finding data sources`, async () => {
+      await wrapperClient.find({
+        type: DATA_SOURCE_SAVED_OBJECT_TYPE,
+        workspaces: ['foo'],
+      });
+      expect(mockedClient.find).toBeCalledWith({
+        type: DATA_SOURCE_SAVED_OBJECT_TYPE,
+        workspaces: null,
+      });
+    });
+  });
 });
diff --git a/...gins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.ts b/...gins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.ts
@@ -15,7 +15,11 @@ import {
   SavedObjectsSerializer,
   SavedObjectsCheckConflictsObject,
   SavedObjectsCheckConflictsResponse,
+  SavedObjectsFindOptions,
 } from '../../../../core/server';
+import { DATA_SOURCE_SAVED_OBJECT_TYPE } from '../../../data_source/common';
+
+const UI_SETTINGS_SAVED_OBJECTS_TYPE = 'config';
 
 const errorContent = (error: Boom.Boom) => error.output.payload;
 
@@ -36,6 +40,21 @@ export class WorkspaceConflictSavedObjectsClientWrapper {
     );
   }
 
+  private isDataSourceType(type: SavedObjectsFindOptions['type']): boolean {
+    if (Array.isArray(type)) {
+      return type.every((item) => item === DATA_SOURCE_SAVED_OBJECT_TYPE);
+    }
+
+    return type === DATA_SOURCE_SAVED_OBJECT_TYPE;
+  }
+  private isConfigType(type: SavedObject['type']): boolean {
+    return type === UI_SETTINGS_SAVED_OBJECTS_TYPE;
+  }
+  private formatFindParams(options: SavedObjectsFindOptions): SavedObjectsFindOptions {
+    const isListingDataSource = this.isDataSourceType(options.type);
+    return isListingDataSource ? { ...options, workspaces: null } : options;
+  }
+
   /**
    * Workspace is a concept to manage saved objects and the `workspaces` field of each object indicates workspaces the object belongs to.
    * When user tries to update an existing object's attribute, workspaces field should be preserved. Below are some cases that this conflict wrapper will take effect:
@@ -49,6 +68,16 @@ export class WorkspaceConflictSavedObjectsClientWrapper {
       options: SavedObjectsCreateOptions = {}
     ) => {
       const { workspaces, id, overwrite } = options;
+
+      if (workspaces?.length && (this.isDataSourceType(type) || this.isConfigType(type))) {
+        // For 2.14, data source can only be created without workspace info
+        // config can not be created inside a workspace
+        throw SavedObjectsErrorHelpers.decorateBadRequestError(
+          new Error(`'${type}' is not allowed to create in workspace.`),
+          'Unsupported type in workspace'
+        );
+      }
+
       let savedObjectWorkspaces = options?.workspaces;
 
       /**
@@ -89,12 +118,33 @@ export class WorkspaceConflictSavedObjectsClientWrapper {
       objects: Array<SavedObjectsBulkCreateObject<T>>,
       options: SavedObjectsCreateOptions = {}
     ): Promise<SavedObjectsBulkResponse<T>> => {
-      const { overwrite, namespace } = options;
+      const { overwrite, namespace, workspaces } = options;
+
+      const disallowedSavedObjects: Array<SavedObjectsBulkCreateObject<T>> = [];
+      const allowedSavedObjects: Array<SavedObjectsBulkCreateObject<T>> = [];
+      objects.forEach((item) => {
+        const isImportIntoWorkspace = workspaces?.length || item.workspaces?.length;
+        // config can not be created inside a workspace
+        if (this.isConfigType(item.type) && isImportIntoWorkspace) {
+          disallowedSavedObjects.push(item);
+          return;
+        }
+
+        // For 2.14, data source can only be created without workspace info
+        if (this.isDataSourceType(item.type) && isImportIntoWorkspace) {
+          disallowedSavedObjects.push(item);
+          return;
+        }
+
+        allowedSavedObjects.push(item);
+        return;
+      });
+
       /**
        * When overwrite, filter out all the objects that have ids
        */
       const bulkGetDocs = overwrite
-        ? objects
+        ? allowedSavedObjects
             .filter((object) => !!object.id)
             .map((object) => {
               /**
@@ -169,7 +219,7 @@ export class WorkspaceConflictSavedObjectsClientWrapper {
       /**
        * Get all the objects that do not conflict on workspaces
        */
-      const objectsNoWorkspaceConflictError = objects.filter(
+      const objectsNoWorkspaceConflictError = allowedSavedObjects.filter(
         (item) =>
           !objectsConflictWithWorkspace.find(
             (errorItems) =>
@@ -211,6 +261,18 @@ export class WorkspaceConflictSavedObjectsClientWrapper {
         ...realBulkCreateResult,
         saved_objects: [
           ...objectsConflictWithWorkspace,
+          ...disallowedSavedObjects.map((item) => ({
+            references: [],
+            id: '',
+            ...item,
+            error: {
+              ...SavedObjectsErrorHelpers.decorateBadRequestError(
+                new Error(`'${item.type}' is not allowed to import in workspace.`),
+                'Unsupported type in workspace'
+              ).output.payload,
+              metadata: { isNotOverwritable: true },
+            },
+          })),
           ...(realBulkCreateResult?.saved_objects || []),
         ],
       } as SavedObjectsBulkResponse<T>;
@@ -316,7 +378,10 @@ export class WorkspaceConflictSavedObjectsClientWrapper {
       bulkCreate: bulkCreateWithWorkspaceConflictCheck,
       checkConflicts: checkConflictWithWorkspaceConflictCheck,
       delete: wrapperOptions.client.delete,
-      find: wrapperOptions.client.find,
+      find: (options: SavedObjectsFindOptions) =>
+        // TODO: The `formatFindParams` is a workaround for 2.14 to always list global data sources,
+        //       should remove this workaround in the upcoming release once readonly share is available.
+        wrapperOptions.client.find(this.formatFindParams(options)),
       bulkGet: wrapperOptions.client.bulkGet,
       get: wrapperOptions.client.get,
       update: wrapperOptions.client.update,