Merge remote-tracking branch 'upstream/main' into acl

.github/workflows/build_and_test_workflow.yml

@@ -6,7 +6,7 @@ name: Build and test
 # trigger on every commit push and PR for all branches except pushes for backport branches
 on:
   push:
-    branches: ['**', '!backport/**']
+    branches: ['main', '[0-9].x', '[0-9].[0=9]+'] # Run the functional test on push for only release branches
     paths-ignore:
       - '**/*.md'
       - 'docs/**'

CHANGELOG.md

@@ -22,6 +22,8 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 
 ### 📝 Documentation
 
+- Fix link to documentation for geoHash precision ([#5967](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5967))
+
 ### 🛠 Maintenance
 
 ### 🪛 Refactoring
@@ -70,6 +72,8 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - [Multiple Datasource] Add api registry and allow it to be added into client config in data source plugin ([#5895](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5895))
 - [Multiple Datasource] Concatenate data source name with index pattern name and change delimiter to double colon ([#5907](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5907))
 - [Multiple Datasource] Refactor client and legacy client to use authentication registry ([#5881](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5881))
+- [Multiple Datasource] Improved error handling for the search API when a null value is passed for the dataSourceId ([#5882](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5882))
+- [Multiple Datasource] Hide/Show authentication method in multi data source plugin based on configuration ([#5916](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5916))
 
 ### 🐛 Bug Fixes
 
@@ -97,9 +101,10 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - [BUG] Remove duplicate sample data as id 90943e30-9a47-11e8-b64d-95841ca0b247 ([5668](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5668))
 - [BUG][Multiple Datasource] Fix datasource testing connection unexpectedly passed with wrong endpoint [#5663](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5663)
 - [Table Visualization] Fix filter action buttons for split table aggregations ([#5619](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5619))
+- [osd/std] Add additional recovery from false-positives in handling of long numerals ([#5956](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5956))
+- [BUG][Discover] Allow saved sort from search embeddable to load in Dashboard ([#5934](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5934))
 - [BUG][Multiple Datasource] Fix missing customApiRegistryPromise param for test connection ([#5944](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5944))
 
-
 ### 🚞 Infrastructure
 
 - Re-enable CI workflows for feature branches ([#2908](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2908))

config/opensearch_dashboards.yml

@@ -270,6 +270,17 @@
 #    'ff00::/8',
 # ]
 
+# Set enabled false to hide authentication method in OpenSearch Dashboards.
+# If this setting is commented then all 3 options will be available in OpenSearch Dashboards.
+# Default value will be considered to True.
+#data_source.authTypes:
+#   NoAuthentication:
+#     enabled: true
+#   UsernamePassword:
+#     enabled: true
+#   AWSSigV4:
+#     enabled: true
+
 # Set the value of this setting to false to hide the help menu link to the OpenSearch Dashboards user survey
 # opensearchDashboards.survey.url: "https://survey.opensearch.org"
 

packages/osd-std/src/json.ts

@@ -150,16 +150,38 @@ const parseStringWithLongNumerals = (
     } catch (e) {
       hadException = true;
       /* There are two types of exception objects that can be raised:
-       *  1) a proper object with lineNumber and columnNumber which we can use
-       *  2) a textual message with the position that we need to parse
+       *  1) a textual message with the position that we need to parse
+       *     i. Unexpected [token|string ...] at position ...
+       *    ii. expected ',' or '}' after property value in object at line ... column ...
+       *  2) a proper object with lineNumber and columnNumber which we can use
+       *    Note: this might refer to the part of the code that threw the exception but
+       *          we will try it anyway; the regex is specific enough to not produce
+       *          false-positives.
        */
       let { lineNumber, columnNumber } = e;
-      if (!lineNumber || !columnNumber) {
-        const match = e?.message?.match?.(/^Unexpected token.*at position (\d+)$/);
+
+      if (typeof e?.message === 'string') {
+        /* Check for 1-i (seen in Node)
+         * Finding "..."෴1111"..." inside a string value, the extra quotes throw a syntax error
+         * and the position points to " that is assumed to be the begining of a value.
+         */
+        let match = e.message.match(/^Unexpected .*at position (\d+)(\s|$)/);
         if (match) {
           lineNumber = 1;
-          // The position is zero-indexed; adding 1 to normalize it for the -2 that comes later
+          // Add 1 to reach the marker
           columnNumber = parseInt(match[1], 10) + 1;
+        } else {
+          /* Check for 1-ii (seen in browsers)
+           * Finding "...,"෴1111"..." inside a string value, the extra quotes throw a syntax error
+           * and the column number points to the marker after the " that is assumed to be terminating the
+           * value.
+           */
+          // ToDo: Add functional tests for this path
+          match = e.message.match(/expected .*at line (\d+) column (\d+)(\s|$)/);
+          if (match) {
+            lineNumber = parseInt(match[1], 10);
+            columnNumber = parseInt(match[2], 10);
+          }
         }
       }
 

src/plugins/data/server/search/opensearch_search/decide_client.ts

@@ -11,12 +11,11 @@ export const decideClient = async (
   request: IOpenSearchSearchRequest,
   withLongNumeralsSupport: boolean = false
 ): Promise<OpenSearchClient> => {
-  // if data source feature is disabled, return default opensearch client of current user
-  const client =
-    request.dataSourceId && context.dataSource
-      ? await context.dataSource.opensearch.getClient(request.dataSourceId)
-      : withLongNumeralsSupport
-      ? context.core.opensearch.client.asCurrentUserWithLongNumeralsSupport
-      : context.core.opensearch.client.asCurrentUser;
-  return client;
+  const defaultOpenSearchClient = withLongNumeralsSupport
+    ? context.core.opensearch.client.asCurrentUserWithLongNumeralsSupport
+    : context.core.opensearch.client.asCurrentUser;
+
+  return request.dataSourceId && context.dataSource
+    ? await context.dataSource.opensearch.getClient(request.dataSourceId)
+    : defaultOpenSearchClient;
 };

src/plugins/data/server/search/opensearch_search/opensearch_search_strategy.test.ts

@@ -29,15 +29,47 @@
  */
 
 import { RequestHandlerContext } from '../../../../../core/server';
-import { pluginInitializerContextConfigMock } from '../../../../../core/server/mocks';
+import {
+  opensearchServiceMock,
+  pluginInitializerContextConfigMock,
+} from '../../../../../core/server/mocks';
 import { opensearchSearchStrategyProvider } from './opensearch_search_strategy';
 import { DataSourceError } from '../../../../data_source/server/lib/error';
 import { DataSourcePluginSetup } from '../../../../data_source/server';
+import { SearchUsage } from '../collectors';
 
 describe('OpenSearch search strategy', () => {
   const mockLogger: any = {
     debug: () => {},
   };
+  const mockSearchUsage: SearchUsage = {
+    trackError(): Promise<void> {
+      return Promise.resolve(undefined);
+    },
+    trackSuccess(duration: number): Promise<void> {
+      return Promise.resolve(undefined);
+    },
+  };
+  const mockDataSourcePluginSetupWithDataSourceEnabled: DataSourcePluginSetup = {
+    createDataSourceError(err: any): DataSourceError {
+      return new DataSourceError({});
+    },
+    dataSourceEnabled: jest.fn(() => true),
+    registerCredentialProvider: jest.fn(),
+    registerCustomApiSchema(schema: any): void {
+      throw new Error('Function not implemented.');
+    },
+  };
+  const mockDataSourcePluginSetupWithDataSourceDisabled: DataSourcePluginSetup = {
+    createDataSourceError(err: any): DataSourceError {
+      return new DataSourceError({});
+    },
+    dataSourceEnabled: jest.fn(() => false),
+    registerCredentialProvider: jest.fn(),
+    registerCustomApiSchema(schema: any): void {
+      throw new Error('Function not implemented.');
+    },
+  };
   const body = {
     body: {
       _shards: {
@@ -50,6 +82,7 @@ describe('OpenSearch search strategy', () => {
   };
   const mockOpenSearchApiCaller = jest.fn().mockResolvedValue(body);
   const mockDataSourceApiCaller = jest.fn().mockResolvedValue(body);
+  const mockOpenSearchApiCallerWithLongNumeralsSupport = jest.fn().mockResolvedValue(body);
   const dataSourceId = 'test-data-source-id';
   const mockDataSourceContext = {
     dataSource: {
@@ -67,7 +100,14 @@ describe('OpenSearch search strategy', () => {
           get: () => {},
         },
       },
-      opensearch: { client: { asCurrentUser: { search: mockOpenSearchApiCaller } } },
+      opensearch: {
+        client: {
+          asCurrentUser: { search: mockOpenSearchApiCaller },
+          asCurrentUserWithLongNumeralsSupport: {
+            search: mockOpenSearchApiCallerWithLongNumeralsSupport,
+          },
+        },
+      },
     },
   };
   const mockDataSourceEnabledContext = {
@@ -131,20 +171,110 @@ describe('OpenSearch search strategy', () => {
     expect(response).toHaveProperty('rawResponse');
   });
 
-  it('dataSource enabled, send request with dataSourceId get data source client', async () => {
-    const opensearchSearch = await opensearchSearchStrategyProvider(mockConfig$, mockLogger);
+  it('dataSource enabled, config host exist, send request with dataSourceId should get data source client', async () => {
+    const mockOpenSearchServiceSetup = opensearchServiceMock.createSetup();
+    mockOpenSearchServiceSetup.legacy.client = {
+      callAsInternalUser: jest.fn(),
+      asScoped: jest.fn(),
+      config: {
+        hosts: ['some host'],
+      },
+    };
+
+    const opensearchSearch = opensearchSearchStrategyProvider(
+      mockConfig$,
+      mockLogger,
+      mockSearchUsage,
+      mockDataSourcePluginSetupWithDataSourceEnabled,
+      mockOpenSearchServiceSetup
+    );
 
     await opensearchSearch.search(
       (mockDataSourceEnabledContext as unknown) as RequestHandlerContext,
       {
         dataSourceId,
       }
     );
+
     expect(mockDataSourceApiCaller).toBeCalled();
     expect(mockOpenSearchApiCaller).not.toBeCalled();
   });
 
-  it('dataSource disabled, send request with dataSourceId get default client', async () => {
+  it('dataSource enabled, config host exist, send request without dataSourceId should get default client', async () => {
+    const mockOpenSearchServiceSetup = opensearchServiceMock.createSetup();
+    mockOpenSearchServiceSetup.legacy.client = {
+      callAsInternalUser: jest.fn(),
+      asScoped: jest.fn(),
+      config: {
+        hosts: ['some host'],
+      },
+    };
+
+    const opensearchSearch = opensearchSearchStrategyProvider(
+      mockConfig$,
+      mockLogger,
+      mockSearchUsage,
+      mockDataSourcePluginSetupWithDataSourceEnabled,
+      mockOpenSearchServiceSetup
+    );
+
+    const dataSourceIdToBeTested = [undefined, ''];
+
+    dataSourceIdToBeTested.forEach(async (id) => {
+      const testRequest = id === undefined ? {} : { dataSourceId: id };
+
+      await opensearchSearch.search(
+        (mockDataSourceEnabledContext as unknown) as RequestHandlerContext,
+        testRequest
+      );
+      expect(mockOpenSearchApiCaller).toBeCalled();
+      expect(mockDataSourceApiCaller).not.toBeCalled();
+    });
+  });
+
+  it('dataSource enabled, config host is empty / undefined, send request with / without dataSourceId should both throw DataSourceError exception', async () => {
+    const hostsTobeTested = [undefined, []];
+    const dataSourceIdToBeTested = [undefined, '', dataSourceId];
+
+    hostsTobeTested.forEach((host) => {
+      const mockOpenSearchServiceSetup = opensearchServiceMock.createSetup();
+
+      if (host !== undefined) {
+        mockOpenSearchServiceSetup.legacy.client = {
+          callAsInternalUser: jest.fn(),
+          asScoped: jest.fn(),
+          config: {
+            hosts: [],
+          },
+        };
+      }
+
+      dataSourceIdToBeTested.forEach(async (id) => {
+        const testRequest = id === undefined ? {} : { dataSourceId: id };
+
+        try {
+          const opensearchSearch = opensearchSearchStrategyProvider(
+            mockConfig$,
+            mockLogger,
+            mockSearchUsage,
+            mockDataSourcePluginSetupWithDataSourceEnabled,
+            mockOpenSearchServiceSetup
+          );
+
+          await opensearchSearch.search(
+            (mockDataSourceEnabledContext as unknown) as RequestHandlerContext,
+            testRequest
+          );
+        } catch (e) {
+          expect(e).toBeTruthy();
+          expect(e).toBeInstanceOf(DataSourceError);
+          expect(e.statusCode).toEqual(400);
+        }
+      });
+    });
+  });
+
+  it('dataSource disabled, send request with dataSourceId should get default client', async () => {
     const opensearchSearch = await opensearchSearchStrategyProvider(mockConfig$, mockLogger);
 
     await opensearchSearch.search((mockContext as unknown) as RequestHandlerContext, {
@@ -154,11 +284,40 @@ describe('OpenSearch search strategy', () => {
     expect(mockDataSourceApiCaller).not.toBeCalled();
   });
 
-  it('dataSource enabled, send request without dataSourceId get default client', async () => {
+  it('dataSource disabled, send request without dataSourceId should get default client', async () => {
     const opensearchSearch = await opensearchSearchStrategyProvider(mockConfig$, mockLogger);
 
-    await opensearchSearch.search((mockContext as unknown) as RequestHandlerContext, {});
-    expect(mockOpenSearchApiCaller).toBeCalled();
-    expect(mockDataSourceApiCaller).not.toBeCalled();
+    const dataSourceIdToBeTested = [undefined, ''];
+
+    for (const testDataSourceId of dataSourceIdToBeTested) {
+      await opensearchSearch.search((mockContext as unknown) as RequestHandlerContext, {
+        dataSourceId: testDataSourceId,
+      });
+      expect(mockOpenSearchApiCaller).toBeCalled();
+      expect(mockDataSourceApiCaller).not.toBeCalled();
+    }
+  });
+
+  it('dataSource disabled and longNumeralsSupported, send request without dataSourceId should get longNumeralsSupport client', async () => {
+    const mockOpenSearchServiceSetup = opensearchServiceMock.createSetup();
+    const opensearchSearch = await opensearchSearchStrategyProvider(
+      mockConfig$,
+      mockLogger,
+      mockSearchUsage,
+      mockDataSourcePluginSetupWithDataSourceDisabled,
+      mockOpenSearchServiceSetup,
+      true
+    );
+
+    const dataSourceIdToBeTested = [undefined, ''];
+
+    for (const testDataSourceId of dataSourceIdToBeTested) {
+      await opensearchSearch.search((mockContext as unknown) as RequestHandlerContext, {
+        dataSourceId: testDataSourceId,
+      });
+      expect(mockOpenSearchApiCallerWithLongNumeralsSupport).toBeCalled();
+      expect(mockOpenSearchApiCaller).not.toBeCalled();
+      expect(mockDataSourceApiCaller).not.toBeCalled();
+    }
   });
 });

src/plugins/data/server/search/opensearch_search/opensearch_search_strategy.ts

@@ -29,7 +29,7 @@
  */
 
 import { first } from 'rxjs/operators';
-import { SharedGlobalConfig, Logger } from 'opensearch-dashboards/server';
+import { SharedGlobalConfig, Logger, OpenSearchServiceSetup } from 'opensearch-dashboards/server';
 import { SearchResponse } from 'elasticsearch';
 import { Observable } from 'rxjs';
 import { ApiResponse } from '@opensearch-project/opensearch';
@@ -50,6 +50,7 @@ export const opensearchSearchStrategyProvider = (
   logger: Logger,
   usage?: SearchUsage,
   dataSource?: DataSourcePluginSetup,
+  openSearchServiceSetup?: OpenSearchServiceSetup,
   withLongNumeralsSupport?: boolean
 ): ISearchStrategy => {
   return {
@@ -73,6 +74,13 @@ export const opensearchSearchStrategyProvider = (
       });
 
       try {
+        const isOpenSearchHostsEmpty =
+          openSearchServiceSetup?.legacy?.client?.config?.hosts?.length === 0;
+
+        if (dataSource?.dataSourceEnabled() && isOpenSearchHostsEmpty && !request.dataSourceId) {
+          throw new Error(`Data source id is required when no openseach hosts config provided`);
+        }
+
         const client = await decideClient(context, request, withLongNumeralsSupport);
         const promise = shimAbortSignal(client.search(params), options?.abortSignal);
 
@@ -92,7 +100,7 @@ export const opensearchSearchStrategyProvider = (
       } catch (e) {
         if (usage) usage.trackError();
 
-        if (dataSource && request.dataSourceId) {
+        if (dataSource?.dataSourceEnabled()) {
           throw dataSource.createDataSourceError(e);
         }
         throw e;