[MD]Remove endpoint validation for create data source saved object API (

#6899) * Remove endpoint validation for create data source saved object API Signed-off-by: Zhongnan Su <szhongna@amazon.com> * Changeset file for PR #6899 created/updated --------- Signed-off-by: Zhongnan Su <szhongna@amazon.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> (cherry picked from commit 47bd391) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
opensearch-project · Jun 5, 2024 · 2b3c832 · 2b3c832
1 parent eeeef3a
commit 2b3c832
Show file tree

Hide file tree

Showing 5 changed files with 29 additions and 133 deletions.
diff --git a/changelogs/fragments/6899.yml b/changelogs/fragments/6899.yml
@@ -0,0 +1,2 @@
+feat:
+- Remove endpoint validation for create data source saved object API ([#6899](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6899))
diff --git a/src/plugins/data_source/server/data_source_service.mock.ts b/src/plugins/data_source/server/data_source_service.mock.ts
diff --git a/src/plugins/data_source/server/plugin.ts b/src/plugins/data_source/server/plugin.ts
@@ -61,26 +61,16 @@ export class DataSourcePlugin implements Plugin<DataSourcePluginSetup, DataSourc
     const cryptographyServiceSetup: CryptographyServiceSetup = this.cryptographyService.setup(
       config
     );
-    const dataSourceServiceSetup: DataSourceServiceSetup = await this.dataSourceService.setup(
-      config
-    );
 
     const authRegistryPromise = core.getStartServices().then(([, , selfStart]) => {
       const dataSourcePluginStart = selfStart as DataSourcePluginStart;
       return dataSourcePluginStart.getAuthenticationMethodRegistry();
     });
-    const auditTrailPromise = core.getStartServices().then(([coreStart]) => coreStart.auditTrail);
-    const customApiSchemaRegistryPromise = core.getStartServices().then(([, , selfStart]) => {
-      const dataSourcePluginStart = selfStart as DataSourcePluginStart;
-      return dataSourcePluginStart.getCustomApiSchemaRegistry();
-    });
 
     const dataSourceSavedObjectsClientWrapper = new DataSourceSavedObjectsClientWrapper(
-      dataSourceServiceSetup,
       cryptographyServiceSetup,
       this.logger.get('data-source-saved-objects-client-wrapper-factory'),
       authRegistryPromise,
-      customApiSchemaRegistryPromise,
       config.endpointDeniedIPs
     );
 
@@ -114,12 +104,20 @@ export class DataSourcePlugin implements Plugin<DataSourcePluginSetup, DataSourc
       },
     };
     core.auditTrail.register(auditorFactory);
+    const auditTrailPromise = core.getStartServices().then(([coreStart]) => coreStart.auditTrail);
+
+    const dataSourceService: DataSourceServiceSetup = await this.dataSourceService.setup(config);
+
+    const customApiSchemaRegistryPromise = core.getStartServices().then(([, , selfStart]) => {
+      const dataSourcePluginStart = selfStart as DataSourcePluginStart;
+      return dataSourcePluginStart.getCustomApiSchemaRegistry();
+    });
 
     // Register data source plugin context to route handler context
     core.http.registerRouteHandlerContext(
       'dataSource',
       this.createDataSourceRouteHandlerContext(
-        dataSourceServiceSetup,
+        dataSourceService,
         cryptographyServiceSetup,
         this.logger,
         auditTrailPromise,
@@ -131,14 +129,14 @@ export class DataSourcePlugin implements Plugin<DataSourcePluginSetup, DataSourc
     const router = core.http.createRouter();
     registerTestConnectionRoute(
       router,
-      dataSourceServiceSetup,
+      dataSourceService,
       cryptographyServiceSetup,
       authRegistryPromise,
       customApiSchemaRegistryPromise
     );
     registerFetchDataSourceMetaDataRoute(
       router,
-      dataSourceServiceSetup,
+      dataSourceService,
       cryptographyServiceSetup,
       authRegistryPromise,
       customApiSchemaRegistryPromise

diff --git a/...plugins/data_source/server/saved_objects/data_source_saved_objects_client_wrapper.test.ts b/...plugins/data_source/server/saved_objects/data_source_saved_objects_client_wrapper.test.ts
@@ -14,9 +14,6 @@ import { AuthType } from '../../common/data_sources';
 import { cryptographyServiceSetupMock } from '../cryptography_service.mocks';
 import { DataSourceSavedObjectsClientWrapper } from './data_source_saved_objects_client_wrapper';
 import { SavedObject } from 'opensearch-dashboards/public';
-import { dataSourceServiceSetupMock } from '../data_source_service.mock';
-import { CustomApiSchemaRegistry } from '../schema_registry';
-import { DataSourceConnectionValidator } from '../routes/data_source_connection_validator';
 import { DATA_SOURCE_TITLE_LENGTH_LIMIT } from '../util/constants';
 
 describe('DataSourceSavedObjectsClientWrapper', () => {
@@ -37,23 +34,16 @@ describe('DataSourceSavedObjectsClientWrapper', () => {
   const cryptographyMock = cryptographyServiceSetupMock.create();
   const logger = loggingSystemMock.createLogger();
   const authRegistryPromise = Promise.resolve(authRegistry);
-  const customApiSchemaRegistry = new CustomApiSchemaRegistry();
-  const customApiSchemaRegistryPromise = Promise.resolve(customApiSchemaRegistry);
-  const dataSourceServiceSetup = dataSourceServiceSetupMock.create();
-  const requestMock = httpServerMock.createOpenSearchDashboardsRequest();
   const wrapperInstance = new DataSourceSavedObjectsClientWrapper(
-    dataSourceServiceSetup,
     cryptographyMock,
     logger,
-    authRegistryPromise,
-    customApiSchemaRegistryPromise
+    authRegistryPromise
   );
-
   const mockedClient = savedObjectsClientMock.create();
   const wrapperClient = wrapperInstance.wrapperFactory({
     client: mockedClient,
     typeRegistry: requestHandlerContext.savedObjects.typeRegistry,
-    request: requestMock,
+    request: httpServerMock.createOpenSearchDashboardsRequest(),
   });
 
   const getSavedObject = (savedObject: Partial<SavedObject>) => {
@@ -80,17 +70,13 @@ describe('DataSourceSavedObjectsClientWrapper', () => {
   describe('createWithCredentialsEncryption', () => {
     beforeEach(() => {
       mockedClient.create.mockClear();
-      jest
-        .spyOn(DataSourceConnectionValidator.prototype, 'validate')
-        .mockResolvedValue(Promise.resolve() as Promise<any>);
     });
     it('should create data source when auth type is NO_AUTH', async () => {
       const mockDataSourceAttributesWithNoAuth = attributes({
         auth: {
           type: AuthType.NoAuth,
         },
       });
-
       await wrapperClient.create(
         DATA_SOURCE_SAVED_OBJECT_TYPE,
         mockDataSourceAttributesWithNoAuth,
@@ -205,23 +191,6 @@ describe('DataSourceSavedObjectsClientWrapper', () => {
       ).rejects.toThrowError(`Invalid auth type: 'not_in_registry': Bad Request`);
     });
 
-    it('endpoint validator datasource client should be created with request as param', async () => {
-      const mockDataSourceAttributesWithNoAuth = attributes({
-        auth: {
-          type: AuthType.NoAuth,
-        },
-      });
-
-      await wrapperClient.create(
-        DATA_SOURCE_SAVED_OBJECT_TYPE,
-        mockDataSourceAttributesWithNoAuth,
-        {}
-      );
-      expect(dataSourceServiceSetup.getDataSourceClient).toBeCalledWith(
-        expect.objectContaining({ request: requestMock })
-      );
-    });
-
     describe('createWithCredentialsEncryption: Error handling', () => {
       it('should throw error when title is empty', async () => {
         const mockDataSourceAttributes = attributes({
@@ -252,23 +221,6 @@ describe('DataSourceSavedObjectsClientWrapper', () => {
         ).rejects.toThrowError(`"endpoint" attribute is not valid or allowed`);
       });
 
-      it('should throw error when endpoint is not valid OpenSearch endpoint', async () => {
-        const mockDataSourceAttributes = attributes({
-          auth: {
-            type: AuthType.NoAuth,
-          },
-        });
-        jest
-          .spyOn(DataSourceConnectionValidator.prototype, 'validate')
-          .mockImplementationOnce(() => {
-            throw new Error();
-          });
-
-        await expect(
-          wrapperClient.create(DATA_SOURCE_SAVED_OBJECT_TYPE, mockDataSourceAttributes, {})
-        ).rejects.toThrowError(`endpoint is not valid OpenSearch endpoint: Bad Request`);
-      });
-
       it('should throw error when auth is not present', async () => {
         await expect(
           wrapperClient.create(DATA_SOURCE_SAVED_OBJECT_TYPE, attributes(), {})

diff --git a/src/plugins/data_source/server/saved_objects/data_source_saved_objects_client_wrapper.ts b/src/plugins/data_source/server/saved_objects/data_source_saved_objects_client_wrapper.ts
@@ -4,8 +4,6 @@
  */
 
 import {
-  OpenSearchClient,
-  OpenSearchDashboardsRequest,
   SavedObjectsBulkCreateObject,
   SavedObjectsBulkResponse,
   SavedObjectsBulkUpdateObject,
@@ -28,9 +26,6 @@ import {
 import { EncryptionContext, CryptographyServiceSetup } from '../cryptography_service';
 import { isValidURL } from '../util/endpoint_validator';
 import { IAuthenticationMethodRegistry } from '../auth_registry';
-import { DataSourceServiceSetup } from '../data_source_service';
-import { CustomApiSchemaRegistry } from '../schema_registry';
-import { DataSourceConnectionValidator } from '../routes/data_source_connection_validator';
 import { DATA_SOURCE_TITLE_LENGTH_LIMIT } from '../util/constants';
 
 /**
@@ -52,10 +47,7 @@ export class DataSourceSavedObjectsClientWrapper {
         return await wrapperOptions.client.create(type, attributes, options);
       }
 
-      const encryptedAttributes = await this.validateAndEncryptAttributes(
-        attributes,
-        wrapperOptions.request
-      );
+      const encryptedAttributes = await this.validateAndEncryptAttributes(attributes);
 
       return await wrapperOptions.client.create(type, encryptedAttributes, options);
     };
@@ -74,7 +66,7 @@ export class DataSourceSavedObjectsClientWrapper {
 
           return {
             ...object,
-            attributes: await this.validateAndEncryptAttributes(attributes, wrapperOptions.request),
+            attributes: await this.validateAndEncryptAttributes(attributes),
           };
         })
       );
@@ -148,19 +140,14 @@ export class DataSourceSavedObjectsClientWrapper {
   };
 
   constructor(
-    private dataSourcesService: DataSourceServiceSetup,
     private cryptography: CryptographyServiceSetup,
     private logger: Logger,
     private authRegistryPromise: Promise<IAuthenticationMethodRegistry>,
-    private customApiSchemaRegistryPromise: Promise<CustomApiSchemaRegistry>,
     private endpointBlockedIps?: string[]
   ) {}
 
-  private async validateAndEncryptAttributes<T = unknown>(
-    attributes: T,
-    request?: OpenSearchDashboardsRequest
-  ) {
-    await this.validateAttributes(attributes, request);
+  private async validateAndEncryptAttributes<T = unknown>(attributes: T) {
+    await this.validateAttributes(attributes);
 
     const { endpoint, auth } = attributes;
 
@@ -264,58 +251,31 @@ export class DataSourceSavedObjectsClientWrapper {
     }
   }
 
-  private async validateAttributes<T = unknown>(
-    attributes: T,
-    request?: OpenSearchDashboardsRequest
-  ) {
+  private async validateAttributes<T = unknown>(attributes: T) {
     const { title, endpoint, auth } = attributes;
-
     this.validateTitle(title);
-    await this.validateEndpoint(endpoint, attributes as DataSourceAttributes, request);
+    this.validateEndpoint(endpoint);
     await this.validateAuth(auth);
   }
 
-  private validateTitle(title: string) {
-    if (!title.trim().length) {
-      throw SavedObjectsErrorHelpers.createBadRequestError(
-        '"title" attribute must be a non-empty string'
-      );
-    }
-
-    if (title.length > DATA_SOURCE_TITLE_LENGTH_LIMIT) {
+  private validateEndpoint(endpoint: string) {
+    if (!isValidURL(endpoint, this.endpointBlockedIps)) {
       throw SavedObjectsErrorHelpers.createBadRequestError(
-        `"title" attribute is limited to ${DATA_SOURCE_TITLE_LENGTH_LIMIT} characters`
+        '"endpoint" attribute is not valid or allowed'
       );
     }
   }
 
-  private async validateEndpoint(
-    endpoint: string,
-    attributes: DataSourceAttributes,
-    request?: OpenSearchDashboardsRequest
-  ) {
-    if (!isValidURL(endpoint, this.endpointBlockedIps)) {
+  private validateTitle(title: string) {
+    if (!title.trim().length) {
       throw SavedObjectsErrorHelpers.createBadRequestError(
-        '"endpoint" attribute is not valid or allowed'
+        '"title" attribute must be a non-empty string'
       );
     }
-    try {
-      const dataSourceClient: OpenSearchClient = await this.dataSourcesService.getDataSourceClient({
-        savedObjects: {} as any,
-        cryptography: this.cryptography,
-        testClientDataSourceAttr: attributes as DataSourceAttributes,
-        request,
-        authRegistry: await this.authRegistryPromise,
-        customApiSchemaRegistryPromise: this.customApiSchemaRegistryPromise,
-      });
-
-      const dataSourceValidator = new DataSourceConnectionValidator(dataSourceClient, attributes);
 
-      await dataSourceValidator.validate();
-    } catch (err: any) {
-      this.logger.error(err);
+    if (title.length > DATA_SOURCE_TITLE_LENGTH_LIMIT) {
       throw SavedObjectsErrorHelpers.createBadRequestError(
-        `endpoint is not valid OpenSearch endpoint`
+        `"title" attribute is limited to ${DATA_SOURCE_TITLE_LENGTH_LIMIT} characters`
       );
     }
   }