Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/main' into feat-add-workspace-cr…
Browse files Browse the repository at this point in the history
…eate-page

Signed-off-by: Lin Wang <wonglam@amazon.com>
  • Loading branch information
wanglam committed Mar 26, 2024
2 parents e5f53f0 + 4f1884b commit 41e7201
Show file tree
Hide file tree
Showing 47 changed files with 884 additions and 536 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- [Multiple Datasource] Add Vega support to MDS by specifying a data source name in the Vega spec ([#5975](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5975))
- [Multiple Datasource] Test connection schema validation for registered auth types ([#6109](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6109))
- [Multiple DataSource] DataSource creation and edition page improvement to better support registered auth types ([#6122](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6122))
- [Multiple DataSource] Codebase maintenance involves updating typos and removing unused imported packages ([#6238](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6238))
- [Workspace] Consume workspace id in saved object client ([#6014](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6014))
- [Multiple Datasource] Export DataSourcePluginRequestContext at top level for plugins to use ([#6108](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6108))
- [Multiple Datasource] Expose filterfn in datasource menu component to allow filter data sources before rendering in navigation bar ([#6113](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6113))
Expand All @@ -63,6 +64,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- [Workspace] Register a workspace dropdown menu at the top of left nav bar ([#6150](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6150))
- [Multiple Datasource] Add icon in datasource table page to show the default datasource ([#6231](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6231))
- [Multiple Datasource] Add TLS configuration for multiple data sources ([#6171](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6171))
- [Multiple Datasource] Refactor data source menu and interface to allow cleaner selection of component and related configurations ([#6256](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6256))
- [Workspace] Add create workspace page ([#6179](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6179))

### 🐛 Bug Fixes
Expand All @@ -77,6 +79,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- [Discover] Fix lazy loading of the legacy table from getting stuck ([#6041](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6041))
- [BUG][Discover] Allow saved sort from search embeddable to load in Dashboard ([#5934](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5934))
- [osd/std] Add additional recovery from false-positives in handling of long numerals ([#5956](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5956), [#6245](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6245))
- [osd/std] Add fallback mechanism when recovery from false-positives in handling of long numerals fails ([#6253](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6253))
- [BUG][Multiple Datasource] Fix missing customApiRegistryPromise param for test connection ([#5944](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5944))
- [BUG][Multiple Datasource] Add a migration function for datasource to add migrationVersion field ([#6025](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6025))
- [BUG][MD]Expose picker using function in data source management plugin setup([#6030](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6030))
Expand Down
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ OpenSearch is a community project that is built and maintained by people just li

### Join the Discussion

See the [communication guide](COMMUNICATION.md)for information on how to join our slack workspace, forum, or developer office hours.
See the [communication guide](COMMUNICATIONS.md)for information on how to join our slack workspace, forum, or developer office hours.

### Bug Reports

Expand Down
129 changes: 67 additions & 62 deletions packages/osd-std/src/json.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,77 +143,82 @@ const parseStringWithLongNumerals = (
* To find those instances, we try to parse and watch for the location of any errors. If an error
* is caused by the marking, we remove that single marking and try again.
*/
do {
try {
hadException = false;
obj = parseMarkedText(markedJSON);
} catch (e) {
hadException = true;
/* There are two types of exception objects that can be raised:
* 1) a textual message with the position that we need to parse
* i. Unexpected [token|string ...] at position ...
* ii. Expected ',' or ... after ... in JSON at position ...
* iii. expected ',' or ... after ... 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 (typeof e?.message === 'string') {
/* Check for 1-i and 1-ii
* 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.
try {
do {
try {
hadException = false;
obj = parseMarkedText(markedJSON);
} catch (e) {
hadException = true;
/* There are two types of exception objects that can be raised:
* 1) a textual message with the position that we need to parse
* i. Unexpected [token|string ...] at position ...
* ii. Expected ',' or ... after ... in JSON at position ...
* iii. expected ',' or ... after ... 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 match = e.message.match(/^(?:Une|E)xpected .*at position (\d+)(\D|$)/i);
if (match) {
lineNumber = 1;
// Add 1 to reach the marker
columnNumber = parseInt(match[1], 10) + 1;
} else {
/* Check for 1-iii
* 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.
* PS: There are different versions of this error across browsers and platforms.
let { lineNumber, columnNumber } = e;

if (typeof e?.message === 'string') {
/* Check for 1-i and 1-ii
* 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.
*/
// ToDo: Add functional tests for this path
match = e.message.match(/expected .*line (\d+) column (\d+)(\D|$)/i);
let match = e.message.match(/^(?:Un)?expected .*at position (\d+)(\D|$)/i);
if (match) {
lineNumber = parseInt(match[1], 10);
columnNumber = parseInt(match[2], 10);
lineNumber = 1;
// Add 1 to reach the marker
columnNumber = parseInt(match[1], 10) + 1;
} else {
/* Check for 1-iii
* 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.
* PS: There are different versions of this error across browsers and platforms.
*/
// ToDo: Add functional tests for this path
match = e.message.match(/expected .*line (\d+) column (\d+)(\D|$)/i);
if (match) {
lineNumber = parseInt(match[1], 10);
columnNumber = parseInt(match[2], 10);
}
}
}
}

if (lineNumber < 1 || columnNumber < 2) {
/* The problem is not with this replacement.
* Note: This will never happen because the outer parse would have already thrown.
*/
// coverage:ignore-line
throw e;
}
if (lineNumber < 1 || columnNumber < 2) {
/* The problem is not with this replacement.
* Note: This will never happen because the outer parse would have already thrown.
*/
// coverage:ignore-line
throw e;
}

/* We need to skip e.lineNumber - 1 number of `\n` occurrences.
* Then, we need to go to e.columnNumber - 2 to look for `"<mark>\d+"`; we need to `-1` to
* account for the quote but an additional `-1` is needed because columnNumber starts from 1.
*/
const re = new RegExp(
`^((?:.*\\n){${lineNumber - 1}}[^\\n]{${columnNumber - 2}})"${marker}(-?\\d+)"`
);
if (!re.test(markedJSON)) {
/* The exception is not caused by adding the marker.
* Note: This will never happen because the outer parse would have already thrown.
/* We need to skip e.lineNumber - 1 number of `\n` occurrences.
* Then, we need to go to e.columnNumber - 2 to look for `"<mark>\d+"`; we need to `-1` to
* account for the quote but an additional `-1` is needed because columnNumber starts from 1.
*/
// coverage:ignore-line
throw e;
}
const re = new RegExp(
`^((?:.*\\n){${lineNumber - 1}}[^\\n]{${columnNumber - 2}})"${marker}(-?\\d+)"`
);
if (!re.test(markedJSON)) {
/* The exception is not caused by adding the marker.
* Note: This will never happen because the outer parse would have already thrown.
*/
// coverage:ignore-line
throw e;
}

// We have found a bad replacement; let's remove it.
markedJSON = markedJSON.replace(re, '$1$2');
}
} while (hadException);
// We have found a bad replacement; let's remove it.
markedJSON = markedJSON.replace(re, '$1$2');
}
} while (hadException);
} catch (ex) {
// If parsing of marked `text` fails, fallback to parsing the original `text`
obj = JSON.parse(text, reviver || undefined);
}

return obj;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { IAuthenticationMethodRegistery } from './authentication_methods_registry';
import { IAuthenticationMethodRegistry } from './authentication_methods_registry';

const create = () =>
(({
getAllAuthenticationMethods: jest.fn(),
getAuthenticationMethod: jest.fn(),
} as unknown) as jest.Mocked<IAuthenticationMethodRegistery>);
} as unknown) as jest.Mocked<IAuthenticationMethodRegistry>);

export const authenticationMethodRegisteryMock = { create };
export const authenticationMethodRegistryMock = { create };
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { AuthenticationMethodRegistery } from './authentication_methods_registry';
import { AuthenticationMethodRegistry } from './authentication_methods_registry';
import { AuthenticationMethod } from '../../server/types';

const createAuthenticationMethod = (
Expand All @@ -14,11 +14,11 @@ const createAuthenticationMethod = (
...authMethod,
});

describe('AuthenticationMethodRegistery', () => {
let registry: AuthenticationMethodRegistery;
describe('AuthenticationMethodRegistry', () => {
let registry: AuthenticationMethodRegistry;

beforeEach(() => {
registry = new AuthenticationMethodRegistery();
registry = new AuthenticationMethodRegistry();
});

it('allows to register authentication method', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@ import { deepFreeze } from '@osd/std';
import { AuthenticationMethod } from '../../server/types';
import { AuthType } from '../../common/data_sources';

export type IAuthenticationMethodRegistery = Omit<
AuthenticationMethodRegistery,
export type IAuthenticationMethodRegistry = Omit<
AuthenticationMethodRegistry,
'registerAuthenticationMethod'
>;

export class AuthenticationMethodRegistery {
export class AuthenticationMethodRegistry {
private readonly authMethods = new Map<string, AuthenticationMethod>();
/**
* Register a authMethods with function to return credentials inside the registry.
Expand Down
4 changes: 2 additions & 2 deletions src/plugins/data_source/server/auth_registry/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@
*/

export {
IAuthenticationMethodRegistery,
AuthenticationMethodRegistery,
IAuthenticationMethodRegistry,
AuthenticationMethodRegistry,
} from './authentication_methods_registry';
28 changes: 14 additions & 14 deletions src/plugins/data_source/server/client/configure_client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ import { cryptographyServiceSetupMock } from '../cryptography_service.mocks';
import { CryptographyServiceSetup } from '../cryptography_service';
import { DataSourceClientParams, AuthenticationMethod, ClientParameters } from '../types';
import { CustomApiSchemaRegistry } from '../schema_registry';
import { IAuthenticationMethodRegistery } from '../auth_registry';
import { authenticationMethodRegisteryMock } from '../auth_registry/authentication_methods_registry.mock';
import { IAuthenticationMethodRegistry } from '../auth_registry';
import { authenticationMethodRegistryMock } from '../auth_registry/authentication_methods_registry.mock';

const DATA_SOURCE_ID = 'a54b76ec86771ee865a0f74a305dfff8';

Expand All @@ -46,7 +46,7 @@ describe('configureClient', () => {
let usernamePasswordAuthContent: UsernamePasswordTypedContent;
let sigV4AuthContent: SigV4Content;
let customApiSchemaRegistry: CustomApiSchemaRegistry;
let authenticationMethodRegistery: jest.Mocked<IAuthenticationMethodRegistery>;
let authenticationMethodRegistry: jest.Mocked<IAuthenticationMethodRegistry>;
let clientParameters: ClientParameters;

const customAuthContent = {
Expand All @@ -70,7 +70,7 @@ describe('configureClient', () => {
savedObjectsMock = savedObjectsClientMock.create();
cryptographyMock = cryptographyServiceSetupMock.create();
customApiSchemaRegistry = new CustomApiSchemaRegistry();
authenticationMethodRegistery = authenticationMethodRegisteryMock.create();
authenticationMethodRegistry = authenticationMethodRegistryMock.create();

config = {
enabled: true,
Expand Down Expand Up @@ -128,7 +128,7 @@ describe('configureClient', () => {
};

ClientMock.mockImplementation(() => dsClient);
authenticationMethodRegistery.getAuthenticationMethod.mockImplementation(() => authMethod);
authenticationMethodRegistry.getAuthenticationMethod.mockImplementation(() => authMethod);
authRegistryCredentialProviderMock.mockReturnValue(clientParameters);
});

Expand Down Expand Up @@ -299,13 +299,13 @@ describe('configureClient', () => {
});

const client = await configureClient(
{ ...dataSourceClientParams, authRegistry: authenticationMethodRegistery },
{ ...dataSourceClientParams, authRegistry: authenticationMethodRegistry },
clientPoolSetup,
config,
logger
);
expect(authRegistryCredentialProviderMock).toHaveBeenCalled();
expect(authenticationMethodRegistery.getAuthenticationMethod).toHaveBeenCalledTimes(1);
expect(authenticationMethodRegistry.getAuthenticationMethod).toHaveBeenCalledTimes(1);
expect(ClientMock).toHaveBeenCalledTimes(1);
expect(savedObjectsMock.get).toHaveBeenCalledTimes(1);
expect(client).toBe(dsClient.child.mock.results[0].value);
Expand Down Expand Up @@ -343,7 +343,7 @@ describe('configureClient', () => {
});

const client = await configureClient(
{ ...dataSourceClientParams, authRegistry: authenticationMethodRegistery },
{ ...dataSourceClientParams, authRegistry: authenticationMethodRegistry },
clientPoolSetup,
config,
logger
Expand Down Expand Up @@ -379,7 +379,7 @@ describe('configureClient', () => {
});

const client = await configureClient(
{ ...dataSourceClientParams, authRegistry: authenticationMethodRegistery },
{ ...dataSourceClientParams, authRegistry: authenticationMethodRegistry },
clientPoolSetup,
config,
logger
Expand Down Expand Up @@ -555,7 +555,7 @@ describe('configureClient', () => {
name: 'clientPoolTest',
credentialProvider: jest.fn(),
};
authenticationMethodRegistery.getAuthenticationMethod
authenticationMethodRegistry.getAuthenticationMethod
.mockReset()
.mockImplementation(() => authMethodWithClientPool);
const mockDataSourceAttr = { ...dataSourceAttr, name: 'custom_auth' };
Expand All @@ -574,14 +574,14 @@ describe('configureClient', () => {
});
test('If endpoint is same for multiple requests client pool size should be 1', async () => {
await configureClient(
{ ...dataSourceClientParams, authRegistry: authenticationMethodRegistery },
{ ...dataSourceClientParams, authRegistry: authenticationMethodRegistry },
opensearchClientPoolSetup,
config,
logger
);

await configureClient(
{ ...dataSourceClientParams, authRegistry: authenticationMethodRegistery },
{ ...dataSourceClientParams, authRegistry: authenticationMethodRegistry },
opensearchClientPoolSetup,
config,
logger
Expand All @@ -592,7 +592,7 @@ describe('configureClient', () => {

test('If endpoint is different for two requests client pool size should be 2', async () => {
await configureClient(
{ ...dataSourceClientParams, authRegistry: authenticationMethodRegistery },
{ ...dataSourceClientParams, authRegistry: authenticationMethodRegistry },
opensearchClientPoolSetup,
config,
logger
Expand Down Expand Up @@ -622,7 +622,7 @@ describe('configureClient', () => {
});

await configureClient(
{ ...dataSourceClientParams, authRegistry: authenticationMethodRegistery },
{ ...dataSourceClientParams, authRegistry: authenticationMethodRegistry },
opensearchClientPoolSetup,
config,
logger
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,7 @@

import { Client } from '@opensearch-project/opensearch';
import { Client as LegacyClient } from 'elasticsearch';
import {
OpenSearchDashboardsRequest,
SavedObjectsClientContract,
} from '../../../../../src/core/server';
import { SavedObjectsClientContract } from '../../../../../src/core/server';
import { DATA_SOURCE_SAVED_OBJECT_TYPE } from '../../common';
import {
DataSourceAttributes,
Expand All @@ -18,7 +15,7 @@ import {
} from '../../common/data_sources';
import { CryptographyServiceSetup } from '../cryptography_service';
import { createDataSourceError } from '../lib/error';
import { IAuthenticationMethodRegistery } from '../auth_registry';
import { IAuthenticationMethodRegistry } from '../auth_registry';
import { AuthenticationMethod, ClientParameters } from '../types';

/**
Expand Down Expand Up @@ -142,7 +139,7 @@ export const generateCacheKey = (endpoint: string, cacheKeySuffix?: string) => {

export const getAuthenticationMethod = (
dataSourceAttr: DataSourceAttributes,
authRegistry?: IAuthenticationMethodRegistery
authRegistry?: IAuthenticationMethodRegistry
): AuthenticationMethod => {
const name = dataSourceAttr.name ?? dataSourceAttr.auth.type;
return authRegistry?.getAuthenticationMethod(name) as AuthenticationMethod;
Expand Down
Loading

0 comments on commit 41e7201

Please sign in to comment.