Skip to content

Commit

Permalink
[MD] Improve test connection
Browse files Browse the repository at this point in the history
Signed-off-by: Su <szhongna@amazon.com>
  • Loading branch information
zhongnansu committed Dec 20, 2022
1 parent 9299784 commit 777532e
Show file tree
Hide file tree
Showing 12 changed files with 64 additions and 60 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- [I18n] Register ru, ru-RU locale ([#2817](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2817))
- Add yarn opensearch arg to setup plugin dependencies ([#2544](https://github.com/opensearch-project/OpenSearch-Dashboards/issues/2544))
- [Multi DataSource] Test the connection to an external data source when creating or updating ([#2973](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2973))
- [Multi DataSource] Improve test connection ([#3110](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3110))

### 🐛 Bug Fixes

Expand Down
1 change: 0 additions & 1 deletion src/plugins/data_source/common/data_sources/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import { SavedObjectAttributes } from 'src/core/types';

export interface DataSourceAttributes extends SavedObjectAttributes {
id?: string;
title: string;
description?: string;
endpoint: string;
Expand Down
19 changes: 6 additions & 13 deletions src/plugins/data_source/server/client/configure_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export const configureClient = async (
logger: Logger
): Promise<Client> => {
try {
const { attributes: dataSource } = await getDataSource(dataSourceId, savedObjects);
const { attributes: dataSource } = await getDataSource(dataSourceId!, savedObjects);
const rootClient = getRootClient(dataSource, config, openSearchClientPoolSetup);

return await getQueryClient(rootClient, dataSource, cryptography);
Expand All @@ -38,36 +38,29 @@ export const configureClient = async (
};

export const configureTestClient = async (
{ savedObjects, cryptography }: DataSourceClientParams,
{ savedObjects, cryptography, dataSourceId }: DataSourceClientParams,
dataSource: DataSourceAttributes,
openSearchClientPoolSetup: OpenSearchClientPoolSetup,
config: DataSourcePluginConfigType,
logger: Logger
): Promise<Client> => {
try {
const {
id,
auth: { type, credentials },
} = dataSource;
let requireDecryption = false;

const rootClient = getRootClient(dataSource, config, openSearchClientPoolSetup);

if (type === AuthType.UsernamePasswordType && !credentials?.password && id) {
const { attributes: fetchedDataSource } = await getDataSource(id || '', savedObjects);
dataSource.auth = {
type,
credentials: {
username: credentials?.username || '',
password: fetchedDataSource.auth.credentials?.password || '',
},
};
if (type === AuthType.UsernamePasswordType && !credentials?.password && dataSourceId) {
const dataSourceSavedObject = await getDataSource(dataSourceId, savedObjects);
dataSource = dataSourceSavedObject.attributes;
requireDecryption = true;
}

return getQueryClient(rootClient, dataSource, cryptography, requireDecryption);
} catch (error: any) {
logger.error(`Failed to get data source client for dataSource: ${dataSource}`);
logger.error(`Failed to get test client for dataSource: ${dataSource}`);
logger.error(error);
// Re-throw as DataSourceError
throw createDataSourceError(error);
Expand Down
4 changes: 2 additions & 2 deletions src/plugins/data_source/server/data_source_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ export class DataSourceService {
}

async setup(config: DataSourcePluginConfigType): Promise<DataSourceServiceSetup> {
const opensearchClientPoolSetup = await this.openSearchClientPool.setup(config);
const legacyClientPoolSetup = await this.legacyClientPool.setup(config);
const opensearchClientPoolSetup = this.openSearchClientPool.setup(config);
const legacyClientPoolSetup = this.legacyClientPool.setup(config);

const getDataSourceClient = async (
params: DataSourceClientParams
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { configureLegacyClient } from './configure_legacy_client';
const DATA_SOURCE_ID = 'a54b76ec86771ee865a0f74a305dfff8';

// TODO: improve UT
describe.skip('configureLegacyClient', () => {
describe('configureLegacyClient', () => {
let logger: ReturnType<typeof loggingSystemMock.createLogger>;
let config: DataSourcePluginConfigType;
let savedObjectsMock: jest.Mocked<SavedObjectsClientContract>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ export const configureLegacyClient = async (
logger: Logger
) => {
try {
const dataSource = await getDataSource(dataSourceId, savedObjects);
const dataSource = await getDataSource(dataSourceId!, savedObjects);
const rootClient = getRootClient(dataSource.attributes, config, openSearchClientPoolSetup);

return await getQueryClient(rootClient, dataSource, cryptography, callApiParams);
return await getQueryClient(rootClient, dataSource.attributes, cryptography, callApiParams);
} catch (error: any) {
logger.error(`Failed to get data source client for dataSourceId: [${dataSourceId}]`);
logger.error(error);
Expand All @@ -55,11 +55,11 @@ export const configureLegacyClient = async (
*/
const getQueryClient = async (
rootClient: Client,
dataSource: SavedObject<DataSourceAttributes>,
dataSource: DataSourceAttributes,
cryptography: CryptographyServiceSetup,
{ endpoint, clientParams, options }: LegacyClientCallAPIParams
) => {
const authType = dataSource.attributes.auth.type;
const authType = dataSource.auth.type;

switch (authType) {
case AuthType.NoAuth:
Expand Down
13 changes: 11 additions & 2 deletions src/plugins/data_source/server/lib/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,18 @@ import { OsdError } from '../../../opensearch_dashboards_utils/common';
export class DataSourceError extends OsdError {
// must have statusCode to avoid route handler in search.ts to return 500
statusCode: number;
constructor(error: any, message?: string, statusCode?: number) {
message = message ? message : error.message;
constructor(error: any, context?: string, statusCode?: number) {
let message: string;
if (context) {
message = context;
} else if (isResponseError(error)) {
message = JSON.stringify(error.meta.body);
} else {
message = error.message;
}

super('Data Source Error: ' + message);

if (statusCode) {
this.statusCode = statusCode;
} else if (error.statusCode) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,7 @@ export class DataSourceConnectionValidator {
try {
return await this.callDataCluster.info<OpenSearchClient>();
} catch (e) {
if (e.statusCode === 403) {
return true;
} else {
throw createDataSourceError(e);
}
throw createDataSourceError(e);
}
}
}
52 changes: 28 additions & 24 deletions src/plugins/data_source/server/routes/test_connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,36 +20,40 @@ export const registerTestConnectionRoute = (
path: '/internal/data-source-management/validate',
validate: {
body: schema.object({
id: schema.string(),
endpoint: schema.string(),
auth: schema.maybe(
schema.object({
type: schema.oneOf([schema.literal('username_password'), schema.literal('no_auth')]),
credentials: schema.oneOf([
schema.object({
username: schema.string(),
password: schema.string(),
}),
schema.literal(null),
]),
})
),
id: schema.maybe(schema.string()),
dataSourceAttr: schema.object({
endpoint: schema.string(),
auth: schema.maybe(
schema.object({
type: schema.oneOf([
schema.literal('username_password'),
schema.literal('no_auth'),
]),
credentials: schema.oneOf([
schema.object({
username: schema.string(),
password: schema.string(),
}),
schema.literal(null),
]),
})
),
}),
}),
},
},
async (context, request, response) => {
const dataSource: DataSourceAttributes = request.body as DataSourceAttributes;

const dataSourceClient: OpenSearchClient = await dataSourceServiceSetup.getTestingClient(
{
dataSourceId: dataSource.id || '',
savedObjects: context.core.savedObjects.client,
cryptography,
},
dataSource
);
const { dataSourceAttr, id: dataSourceId } = request.body;

try {
const dataSourceClient: OpenSearchClient = await dataSourceServiceSetup.getTestingClient(
{
dataSourceId,
savedObjects: context.core.savedObjects.client,
cryptography,
},
dataSourceAttr as DataSourceAttributes
);
const dsValidator = new DataSourceConnectionValidator(dataSourceClient);

await dsValidator.validate();
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/data_source/server/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export interface LegacyClientCallAPIParams {
}

export interface DataSourceClientParams {
dataSourceId: string;
dataSourceId?: string;
// this saved objects client is used to fetch data source on behalf of users, caller should pass scoped saved objects client
savedObjects: SavedObjectsClientContract;
cryptography: CryptographyServiceSetup;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ describe('DataSourceManagement: Utils.ts', () => {
Array [
"/internal/data-source-management/validate",
Object {
"body": "{\\"id\\":\\"\\",\\"endpoint\\":\\"https://test.com\\",\\"auth\\":{\\"type\\":\\"no_auth\\",\\"credentials\\":null}}",
"body": "{\\"dataSourceAttr\\":{\\"endpoint\\":\\"https://test.com\\",\\"auth\\":{\\"type\\":\\"no_auth\\",\\"credentials\\":null}}}",
},
],
]
Expand All @@ -170,7 +170,7 @@ describe('DataSourceManagement: Utils.ts', () => {
Array [
"/internal/data-source-management/validate",
Object {
"body": "{\\"id\\":\\"test1234\\",\\"endpoint\\":\\"https://test.com\\",\\"auth\\":{\\"type\\":\\"no_auth\\",\\"credentials\\":null}}",
"body": "{\\"id\\":\\"test1234\\",\\"dataSourceAttr\\":{\\"endpoint\\":\\"https://test.com\\",\\"auth\\":{\\"type\\":\\"no_auth\\",\\"credentials\\":null}}}",
},
],
]
Expand Down
12 changes: 7 additions & 5 deletions src/plugins/data_source_management/public/components/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,13 @@ export async function testConnection(
dataSourceID?: string
) {
const query: any = {
id: dataSourceID || '',
endpoint,
auth: {
type,
credentials: type === AuthType.NoAuth ? null : { ...credentials },
id: dataSourceID,
dataSourceAttr: {
endpoint,
auth: {
type,
credentials: type === AuthType.NoAuth ? null : { ...credentials },
},
},
};

Expand Down

0 comments on commit 777532e

Please sign in to comment.