Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Multiple Datasource] Support Amazon OpenSearch Serverless #3957

Merged
merged 5 commits into from
May 19, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- Add satisfaction survey link to help menu ([#3676] (https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3676))
- [Vis Builder] Add persistence to visualizations inner state ([#3751](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3751))
- [Table Visualization] Move format table, consolidate types and add unit tests ([#3397](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3397))

- [Multiple Datasource] Support Amazon OpenSearch Serverless ([#3957](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3957))
### 🐛 Bug Fixes

- [Vis Builder] Fixes auto bounds for timeseries bar chart visualization ([2401](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2401))
Expand Down
3 changes: 1 addition & 2 deletions config/opensearch_dashboards.yml
Original file line number Diff line number Diff line change
Expand Up @@ -229,8 +229,7 @@
# functionality in Visualization.
# vis_builder.enabled: false

# Set the value of this setting to true to enable the experimental multiple data source
# support feature. Use with caution.
# Set the value of this setting to true to enable multiple data source feature.
#data_source.enabled: false
# Set the value of these settings to customize crypto materials to encryption saved credentials
# in data sources.
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@
"@hapi/podium": "^4.1.3",
"@hapi/vision": "^6.1.0",
"@hapi/wreck": "^17.1.0",
"@opensearch-project/opensearch": "^2.1.0",
"@opensearch-project/opensearch": "^2.2.0",
"@osd/ace": "1.0.0",
"@osd/analytics": "1.0.0",
"@osd/apm-config-loader": "1.0.0",
Expand Down Expand Up @@ -169,7 +169,7 @@
"dns-sync": "^0.2.1",
"elastic-apm-node": "^3.7.0",
"elasticsearch": "^16.7.0",
"http-aws-es": "6.0.0",
"http-aws-es": "npm:@zhongnansu/http-aws-es@6.0.1",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we change the version here? are we allowed to do this?

Copy link
Member Author

@zhongnansu zhongnansu May 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a drop-in replacement as a short term solution to support serverless in legacy client, before #2220 is completed . See this comment for the reasoning and long term solution. #3623 (comment)

I've seen this kind of drop-in being done before as short term solution. Besides, the http-aws-es package is only used for data source feature

"vega-interpreter": "npm:@amoo-miki/vega-forced-csp-compliant-interpreter@1.0.6",

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

discussed with the team and reached an agreement to do this as a short term solution.

"execa": "^4.0.2",
"expiry-js": "0.1.7",
"fast-deep-equal": "^3.1.1",
Expand Down
2 changes: 1 addition & 1 deletion packages/osd-opensearch/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
"osd:watch": "../../scripts/use_node scripts/build --watch"
},
"dependencies": {
"@opensearch-project/opensearch": "^2.1.0",
"@opensearch-project/opensearch": "^2.2.0",
"@osd/dev-utils": "1.0.0",
"abort-controller": "^3.0.0",
"chalk": "^4.1.0",
Expand Down
6 changes: 6 additions & 0 deletions src/plugins/data_source/common/data_sources/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export interface SigV4Content extends SavedObjectAttributes {
accessKey: string;
secretKey: string;
region: string;
service?: SigV4ServiceName;
}

export interface UsernamePasswordTypedContent extends SavedObjectAttributes {
Expand All @@ -37,3 +38,8 @@ export enum AuthType {
UsernamePasswordType = 'username_password',
SigV4 = 'sigv4',
}

export enum SigV4ServiceName {
OpenSearch = 'es',
OpenSearchServerless = 'aoss',
}
3 changes: 2 additions & 1 deletion src/plugins/data_source/opensearch_dashboards.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,6 @@
"server": true,
"ui": true,
"requiredPlugins": [],
"optionalPlugins": []
"optionalPlugins": [],
"extraPublicDirs": ["common/data_sources"]
}
24 changes: 24 additions & 0 deletions src/plugins/data_source/server/client/configure_client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,30 @@ describe('configureClient', () => {
expect(decodeAndDecryptSpy).toHaveBeenCalledTimes(2);
});

test('configure client with auth.type == sigv4, service == aoss, should successfully call new Client()', async () => {
savedObjectsMock.get.mockReset().mockResolvedValueOnce({
id: DATA_SOURCE_ID,
type: DATA_SOURCE_SAVED_OBJECT_TYPE,
attributes: {
...dataSourceAttr,
auth: {
type: AuthType.SigV4,
credentials: { ...sigV4AuthContent, service: 'aoss' },
},
},
references: [],
});

jest.spyOn(cryptographyMock, 'decodeAndDecrypt').mockResolvedValue({
decryptedText: 'accessKey',
encryptionContext: { endpoint: 'http://localhost' },
});

await configureClient(dataSourceClientParams, clientPoolSetup, config, logger);

expect(ClientMock).toHaveBeenCalledTimes(1);
});

test('configure test client for non-exist datasource should not call saved object api, nor decode any credential', async () => {
const decodeAndDecryptSpy = jest.spyOn(cryptographyMock, 'decodeAndDecrypt').mockResolvedValue({
decryptedText: 'password',
Expand Down
3 changes: 2 additions & 1 deletion src/plugins/data_source/server/client/configure_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ const getBasicAuthClient = (
};

const getAWSClient = (credential: SigV4Content, clientOptions: ClientOptions): Client => {
const { accessKey, secretKey, region } = credential;
const { accessKey, secretKey, region, service } = credential;

const credentialProvider = (): Promise<Credentials> => {
return new Promise((resolve) => {
Expand All @@ -172,6 +172,7 @@ const getAWSClient = (credential: SigV4Content, clientOptions: ClientOptions): C
...AwsSigv4Signer({
region,
getCredentials: credentialProvider,
service,
}),
...clientOptions,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ export const getAWSCredential = async (
cryptography: CryptographyServiceSetup
): Promise<SigV4Content> => {
const { endpoint } = dataSource;
const { accessKey, secretKey, region } = dataSource.auth.credentials! as SigV4Content;
const { accessKey, secretKey, region, service } = dataSource.auth.credentials! as SigV4Content;

const {
decryptedText: accessKeyText,
Expand Down Expand Up @@ -122,6 +122,7 @@ export const getAWSCredential = async (
region,
accessKey: accessKeyText,
secretKey: secretKeyText,
service,
};

return credential;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import { SavedObjectsClientContract } from '../../../../core/server';
import { loggingSystemMock, savedObjectsClientMock } from '../../../../core/server/mocks';
import { DATA_SOURCE_SAVED_OBJECT_TYPE } from '../../common';
import { AuthType, DataSourceAttributes } from '../../common/data_sources';
import { AuthType, DataSourceAttributes, SigV4Content } from '../../common/data_sources';
import { DataSourcePluginConfigType } from '../../config';
import { cryptographyServiceSetupMock } from '../cryptography_service.mocks';
import { CryptographyServiceSetup } from '../cryptography_service';
Expand All @@ -27,6 +27,7 @@ describe('configureLegacyClient', () => {
let clientPoolSetup: OpenSearchClientPoolSetup;
let configOptions: ConfigOptions;
let dataSourceAttr: DataSourceAttributes;
let sigV4AuthContent: SigV4Content;

let mockOpenSearchClientInstance: {
close: jest.Mock;
Expand Down Expand Up @@ -71,6 +72,12 @@ describe('configureLegacyClient', () => {
},
} as DataSourceAttributes;

sigV4AuthContent = {
region: 'us-east-1',
accessKey: 'accessKey',
secretKey: 'secretKey',
};

clientPoolSetup = {
getClientFromPool: jest.fn(),
addClientToPool: jest.fn(),
Expand Down Expand Up @@ -157,6 +164,42 @@ describe('configureLegacyClient', () => {
expect(mockResult).toBeDefined();
});

test('configure client with auth.type == sigv4 and service param, should call new Client() with service param', async () => {
savedObjectsMock.get.mockReset().mockResolvedValueOnce({
id: DATA_SOURCE_ID,
type: DATA_SOURCE_SAVED_OBJECT_TYPE,
attributes: {
...dataSourceAttr,
auth: {
type: AuthType.SigV4,
credentials: { ...sigV4AuthContent, service: 'aoss' },
},
},
references: [],
});

parseClientOptionsMock.mockReturnValue(configOptions);

jest.spyOn(cryptographyMock, 'decodeAndDecrypt').mockResolvedValue({
decryptedText: 'accessKey',
encryptionContext: { endpoint: 'http://localhost' },
});

await configureLegacyClient(
dataSourceClientParams,
callApiParams,
clientPoolSetup,
config,
logger
);

expect(parseClientOptionsMock).toHaveBeenCalled();
expect(ClientMock).toHaveBeenCalledTimes(1);
expect(ClientMock).toHaveBeenCalledWith(expect.objectContaining({ service: 'aoss' }));

expect(savedObjectsMock.get).toHaveBeenCalledTimes(1);
});

test('configure client with auth.type == username_password and password contaminated', async () => {
const decodeAndDecryptSpy = jest
.spyOn(cryptographyMock, 'decodeAndDecrypt')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@

import { Client } from '@opensearch-project/opensearch';
import { Client as LegacyClient, ConfigOptions } from 'elasticsearch';
import { Credentials } from 'aws-sdk';
import { Credentials, Config } from 'aws-sdk';
import { get } from 'lodash';
import HttpAmazonESConnector from 'http-aws-es';
import { Config } from 'aws-sdk';
import {
Headers,
LegacyAPICaller,
Expand All @@ -27,7 +26,7 @@ import { CryptographyServiceSetup } from '../cryptography_service';
import { DataSourceClientParams, LegacyClientCallAPIParams } from '../types';
import { OpenSearchClientPoolSetup } from '../client';
import { parseClientOptions } from './client_config';
import { createDataSourceError, DataSourceError } from '../lib/error';
import { createDataSourceError } from '../lib/error';
import {
getRootClient,
getAWSCredential,
Expand Down Expand Up @@ -195,13 +194,14 @@ const getBasicAuthClient = async (
};

const getAWSClient = (credential: SigV4Content, clientOptions: ConfigOptions): LegacyClient => {
const { accessKey, secretKey, region } = credential;
const { accessKey, secretKey, region, service } = credential;
const client = new LegacyClient({
connectionClass: HttpAmazonESConnector,
awsConfig: new Config({
region,
credentials: new Credentials({ accessKeyId: accessKey, secretAccessKey: secretKey }),
}),
service,
...clientOptions,
});
return client;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,19 @@

import { OpenSearchClient } from 'opensearch-dashboards/server';
import { createDataSourceError } from '../lib/error';

import { SigV4ServiceName } from '../../common/data_sources';
export class DataSourceConnectionValidator {
constructor(private readonly callDataCluster: OpenSearchClient) {}
constructor(
private readonly callDataCluster: OpenSearchClient,
private readonly dataSourceAttr: any
) {}

async validate() {
try {
return await this.callDataCluster.info<OpenSearchClient>();
// Amazon OpenSearch Serverless does not support .info() API
if (this.dataSourceAttr.auth?.credentials?.service === SigV4ServiceName.OpenSearchServerless)
return await this.callDataCluster.cat.indices();
return await this.callDataCluster.info();
} catch (e) {
throw createDataSourceError(e);
}
Expand Down
14 changes: 11 additions & 3 deletions src/plugins/data_source/server/routes/test_connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import { schema } from '@osd/config-schema';
import { IRouter, OpenSearchClient } from 'opensearch-dashboards/server';
import { AuthType, DataSourceAttributes } from '../../common/data_sources';
import { AuthType, DataSourceAttributes, SigV4ServiceName } from '../../common/data_sources';
import { DataSourceConnectionValidator } from './data_source_connection_validator';
import { DataSourceServiceSetup } from '../data_source_service';
import { CryptographyServiceSetup } from '../cryptography_service';
Expand Down Expand Up @@ -40,6 +40,10 @@ export const registerTestConnectionRoute = (
region: schema.string(),
accessKey: schema.string(),
secretKey: schema.string(),
service: schema.oneOf([
schema.literal(SigV4ServiceName.OpenSearch),
schema.literal(SigV4ServiceName.OpenSearchServerless),
]),
}),
])
),
Expand All @@ -61,9 +65,13 @@ export const registerTestConnectionRoute = (
testClientDataSourceAttr: dataSourceAttr as DataSourceAttributes,
}
);
const dsValidator = new DataSourceConnectionValidator(dataSourceClient);

await dsValidator.validate();
const dataSourceValidator = new DataSourceConnectionValidator(
dataSourceClient,
dataSourceAttr
);

await dataSourceValidator.validate();

return response.ok({
body: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ export class DataSourceSavedObjectsClientWrapper {
);
}

const { accessKey, secretKey, region } = credentials as SigV4Content;
const { accessKey, secretKey, region, service } = credentials as SigV4Content;

if (!accessKey) {
throw SavedObjectsErrorHelpers.createBadRequestError(
Expand All @@ -320,6 +320,12 @@ export class DataSourceSavedObjectsClientWrapper {
'"auth.credentials.region" attribute is required'
);
}

if (!service) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is multiple datasources out of experimental?

If so, then it will likely need a migration for this saved object otherwise previous saved objects will fail.

Copy link
Member Author

@zhongnansu zhongnansu May 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, MD is out of experimental.

I don't think previous saved object will fail, because 1) we are using dynamic mapping for data source saved object. 2) service name is actually defined as optional field. Now it can still be undefined and be backwards compatible

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This condition is added to the create API of saved object wrapper, to align with UI dropdown list, that a service name must be selected when creating the corresponding data source

throw SavedObjectsErrorHelpers.createBadRequestError(
'"auth.credentials.service" attribute is required'
);
}
break;
default:
throw SavedObjectsErrorHelpers.createBadRequestError(`Invalid auth type: '${type}'`);
Expand Down Expand Up @@ -457,7 +463,7 @@ export class DataSourceSavedObjectsClientWrapper {

private async encryptSigV4Credential<T = unknown>(auth: T, encryptionContext: EncryptionContext) {
const {
credentials: { accessKey, secretKey, region },
credentials: { accessKey, secretKey, region, service },
} = auth;

return {
Expand All @@ -466,6 +472,7 @@ export class DataSourceSavedObjectsClientWrapper {
region,
accessKey: await this.cryptography.encryptAndEncode(accessKey, encryptionContext),
secretKey: await this.cryptography.encryptAndEncode(secretKey, encryptionContext),
service,
},
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ describe('Datasource Management: Create Datasource form', () => {
let component: ReactWrapper<any, Readonly<{}>, React.Component<{}, {}, any>>;
const mockSubmitHandler = jest.fn();
const mockTestConnectionHandler = jest.fn();
const mockCancelHandler = jest.fn();

const getFields = (comp: ReactWrapper<any, Readonly<{}>, React.Component<{}, {}, any>>) => {
return {
Expand Down Expand Up @@ -65,6 +66,7 @@ describe('Datasource Management: Create Datasource form', () => {
<CreateDataSourceForm
handleTestConnection={mockTestConnectionHandler}
handleSubmit={mockSubmitHandler}
handleCancel={mockCancelHandler}
existingDatasourceNamesList={['dup20']}
/>
),
Expand Down
Loading