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] Add installedPlugins list to data source saved … #6348

Merged
merged 2 commits into from
Apr 5, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- [Workspace] Add permission control logic ([#6052](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6052))
- [Multiple Datasource] Add default icon for selectable component and make sure the default datasource shows automatically ([#6327](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6327))
- [Multiple Datasource] Pass selected data sources to plugin consumers when the multi-select component initially loads ([#6333](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6333))
- [Multiple Datasource] Add installedPlugins list to data source saved object ([#6348](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6348))
- [Workspace] Add APIs to support plugin state in request ([#6303](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6303))

### 🐛 Bug Fixes
Expand Down
4 changes: 2 additions & 2 deletions src/plugins/data_source/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import { DATA_SOURCE_SAVED_OBJECT_TYPE } from '../common';
import { ensureRawRequest } from '../../../../src/core/server/http/router';
import { createDataSourceError } from './lib/error';
import { registerTestConnectionRoute } from './routes/test_connection';
import { registerFetchDataSourceVersionRoute } from './routes/fetch_data_source_version';
import { registerFetchDataSourceMetaDataRoute } from './routes/fetch_data_source_metadata';
import { AuthenticationMethodRegistry, IAuthenticationMethodRegistry } from './auth_registry';
import { CustomApiSchemaRegistry } from './schema_registry';

Expand Down Expand Up @@ -134,7 +134,7 @@ export class DataSourcePlugin implements Plugin<DataSourcePluginSetup, DataSourc
authRegistryPromise,
customApiSchemaRegistryPromise
);
registerFetchDataSourceVersionRoute(
registerFetchDataSourceMetaDataRoute(
router,
dataSourceService,
cryptographyServiceSetup,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,38 @@ describe('DataSourceManagement: data_source_connection_validator.ts', () => {
expect(fetchDataSourcesVersionResponse).toBe('2.11.0');
});

test('fetchInstalledPlugins - Success: opensearch client response code is 200 and response body have installed plugin list', async () => {
const opensearchClient = opensearchServiceMock.createOpenSearchClient();
opensearchClient.info.mockResolvedValue(
opensearchServiceMock.createApiResponse({
statusCode: 200,
body: [
{
name: 'b40f6833d895d3a95333e325e8bea79b',
component: ' analysis-icu',
version: '2.11.0',
},
{
name: 'b40f6833d895d3a95333e325e8bea79b',
component: 'analysis-ik',
version: '2.11.0',
},
{
name: 'b40f6833d895d3a95333e325e8bea79b',
component: 'analysis-seunjeon',
version: '2.11.0',
},
],
})
);
const dataSourceValidator = new DataSourceConnectionValidator(opensearchClient, {});
const fetchInstalledPluginsReponse = Array.from(
await dataSourceValidator.fetchInstalledPlugins()
);
const installedPlugins = ['analysis-icu', 'analysis-ik', 'analysis-seunjeon'];
fetchInstalledPluginsReponse.map((plugin) => expect(installedPlugins).toContain(plugin));
});

test('failure: opensearch client response code is 200 but response body not have cluster name', async () => {
try {
const opensearchClient = opensearchServiceMock.createOpenSearchClient();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,37 @@

return dataSourceVersion;
} catch (e) {
// return empty dataSoyrce version instead of throwing exception in case info() api call fails
// return empty dataSource version instead of throwing exception in case info() api call fails
return dataSourceVersion;
}
}

async fetchInstalledPlugins() {
const installedPlugins = new Set();
try {
// TODO : retrieve installed plugins from OpenSearch Serverless datasource
if (
this.dataSourceAttr.auth?.credentials?.service === SigV4ServiceName.OpenSearchServerless
) {
return installedPlugins;

Check warning on line 69 in src/plugins/data_source/server/routes/data_source_connection_validator.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/data_source/server/routes/data_source_connection_validator.ts#L69

Added line #L69 was not covered by tests
}

await this.callDataCluster.cat
.plugins({
format: 'JSON',
v: true,
})
.then((response) => response.body)
.then((body) => {
body.forEach((plugin) => {
installedPlugins.add(plugin.component);
});
});

return installedPlugins;
} catch (e) {
// return empty installedPlugins instead of throwing exception in case cat.plugins() api call fails
return installedPlugins;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,17 @@ import { authenticationMethodRegistryMock } from '../auth_registry/authenticatio
import { CustomApiSchemaRegistry } from '../schema_registry';
import { DataSourceServiceSetup } from '../../server/data_source_service';
import { CryptographyServiceSetup } from '../cryptography_service';
import { registerFetchDataSourceVersionRoute } from './fetch_data_source_version';
import { registerFetchDataSourceMetaDataRoute } from './fetch_data_source_metadata';
import { AuthType } from '../../common/data_sources';
// eslint-disable-next-line @osd/eslint/no-restricted-paths
import { opensearchClientMock } from '../../../../../src/core/server/opensearch/client/mocks';
import { index } from 'mathjs';

type SetupServerReturn = UnwrapPromise<ReturnType<typeof setupServer>>;

const URL = '/internal/data-source-management/fetchDataSourceVersion';
const URL = '/internal/data-source-management/fetchDataSourceMetaData';

describe(`Fetch DataSource Version ${URL}`, () => {
describe(`Fetch DataSource MetaData ${URL}`, () => {
let server: SetupServerReturn['server'];
let httpSetup: SetupServerReturn['httpSetup'];
let handlerContext: SetupServerReturn['handlerContext'];
Expand Down Expand Up @@ -167,7 +168,16 @@ describe(`Fetch DataSource Version ${URL}`, () => {
dataSourceClient.info.mockImplementationOnce(() =>
opensearchClientMock.createSuccessTransportRequestPromise({ version: { number: '2.11.0' } })
);
registerFetchDataSourceVersionRoute(

const installedPlugins = [
{ name: 'b40f6833d895d3a95333e325e8bea79b', component: 'opensearch-ml', version: '2.11.0' },
{ name: 'b40f6833d895d3a95333e325e8bea79b', component: 'opensearch-sql', version: '2.11.0' },
];
dataSourceClient.cat.plugins.mockImplementationOnce(() =>
opensearchClientMock.createSuccessTransportRequestPromise(installedPlugins)
);

registerFetchDataSourceMetaDataRoute(
router,
dataSourceServiceSetupMock,
cryptographyMock,
Expand All @@ -190,7 +200,10 @@ describe(`Fetch DataSource Version ${URL}`, () => {
dataSourceAttr,
})
.expect(200);
expect(result.body).toEqual({ dataSourceVersion: '2.11.0' });
expect(result.body).toEqual({
dataSourceVersion: '2.11.0',
installedPlugins: ['opensearch-ml', 'opensearch-sql'],
});
expect(dataSourceServiceSetupMock.getDataSourceClient).toHaveBeenCalledWith(
expect.objectContaining({
savedObjects: handlerContext.savedObjects.client,
Expand All @@ -210,7 +223,10 @@ describe(`Fetch DataSource Version ${URL}`, () => {
dataSourceAttr: dataSourceAttrMissingCredentialForNoAuth,
})
.expect(200);
expect(result.body).toEqual({ dataSourceVersion: '2.11.0' });
expect(result.body).toEqual({
dataSourceVersion: '2.11.0',
installedPlugins: ['opensearch-ml', 'opensearch-sql'],
});
});

it('no credential with basic auth should fail', async () => {
Expand Down Expand Up @@ -307,7 +323,10 @@ describe(`Fetch DataSource Version ${URL}`, () => {
dataSourceAttr: dataSourceAttrForSigV4Auth,
})
.expect(200);
expect(result.body).toEqual({ dataSourceVersion: '2.11.0' });
expect(result.body).toEqual({
dataSourceVersion: '2.11.0',
installedPlugins: ['opensearch-ml', 'opensearch-sql'],
});
});

it('credential with registered auth type should success', async () => {
Expand All @@ -318,7 +337,10 @@ describe(`Fetch DataSource Version ${URL}`, () => {
dataSourceAttr: dataSourceAttrForRegisteredAuthWithCredentials,
})
.expect(200);
expect(result.body).toEqual({ dataSourceVersion: '2.11.0' });
expect(result.body).toEqual({
dataSourceVersion: '2.11.0',
installedPlugins: ['opensearch-ml', 'opensearch-sql'],
});
});

it('empty credential with registered auth type should success', async () => {
Expand All @@ -329,7 +351,10 @@ describe(`Fetch DataSource Version ${URL}`, () => {
dataSourceAttr: dataSourceAttrForRegisteredAuthWithEmptyCredentials,
})
.expect(200);
expect(result.body).toEqual({ dataSourceVersion: '2.11.0' });
expect(result.body).toEqual({
dataSourceVersion: '2.11.0',
installedPlugins: ['opensearch-ml', 'opensearch-sql'],
});
});

it('no credential with registered auth type should success', async () => {
Expand All @@ -340,6 +365,9 @@ describe(`Fetch DataSource Version ${URL}`, () => {
dataSourceAttr: dataSourceAttrForRegisteredAuthWithoutCredentials,
})
.expect(200);
expect(result.body).toEqual({ dataSourceVersion: '2.11.0' });
expect(result.body).toEqual({
dataSourceVersion: '2.11.0',
installedPlugins: ['opensearch-ml', 'opensearch-sql'],
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { CryptographyServiceSetup } from '../cryptography_service';
import { IAuthenticationMethodRegistry } from '../auth_registry';
import { CustomApiSchemaRegistry } from '../schema_registry/custom_api_schema_registry';

export const registerFetchDataSourceVersionRoute = async (
export const registerFetchDataSourceMetaDataRoute = async (
router: IRouter,
dataSourceServiceSetup: DataSourceServiceSetup,
cryptography: CryptographyServiceSetup,
Expand All @@ -22,7 +22,7 @@ export const registerFetchDataSourceVersionRoute = async (
const authRegistry = await authRegistryPromise;
router.post(
{
path: '/internal/data-source-management/fetchDataSourceVersion',
path: '/internal/data-source-management/fetchDataSourceMetaData',
validate: {
body: schema.object({
id: schema.maybe(schema.string()),
Expand Down Expand Up @@ -75,7 +75,6 @@ export const registerFetchDataSourceVersionRoute = async (
},
async (context, request, response) => {
const { dataSourceAttr, id: dataSourceId } = request.body;
let dataSourceVersion = '';

try {
const dataSourceClient: OpenSearchClient = await dataSourceServiceSetup.getDataSourceClient(
Expand All @@ -95,11 +94,13 @@ export const registerFetchDataSourceVersionRoute = async (
dataSourceAttr
);

dataSourceVersion = await dataSourceValidator.fetchDataSourceVersion();
const dataSourceVersion = await dataSourceValidator.fetchDataSourceVersion();
const installedPlugins = Array.from(await dataSourceValidator.fetchInstalledPlugins());

return response.ok({
body: {
dataSourceVersion,
installedPlugins,
},
});
} catch (err) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import React from 'react';
import {
fetchDataSourceVersion,
fetchDataSourceMetaData,
getMappedDataSources,
mockDataSourceAttributesWithAuth,
mockManagementPlugin,
Expand All @@ -28,8 +28,8 @@ describe('Datasource Management: Create Datasource Wizard', () => {
describe('case1: should load resources successfully', () => {
beforeEach(async () => {
spyOn(utils, 'getDataSources').and.returnValue(Promise.resolve(getMappedDataSources));
spyOn(utils, 'fetchDataSourceVersion').and.returnValue(
Promise.resolve(fetchDataSourceVersion)
spyOn(utils, 'fetchDataSourceMetaData').and.returnValue(
Promise.resolve(fetchDataSourceMetaData)
);
await act(async () => {
component = mount(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {
createSingleDataSource,
getDataSources,
testConnection,
fetchDataSourceVersion,
fetchDataSourceMetaData,
handleSetDefaultDatasource,
} from '../utils';
import { LoadingMask } from '../loading_mask';
Expand Down Expand Up @@ -75,8 +75,10 @@ export const CreateDataSourceWizard: React.FunctionComponent<CreateDataSourceWiz
const handleSubmit = async (attributes: DataSourceAttributes) => {
setIsLoading(true);
try {
const version = await fetchDataSourceVersion(http, attributes);
attributes.dataSourceVersion = version.dataSourceVersion;
// Fetch data source metadata from added OS/ES domain/cluster
const metadata = await fetchDataSourceMetaData(http, attributes);
attributes.dataSourceVersion = metadata.dataSourceVersion;
attributes.installedPlugins = metadata.installedPlugins;
await createSingleDataSource(savedObjects.client, attributes);
// Set the first create data source as default data source.
await handleSetDefaultDatasource(savedObjects.client, uiSettings);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@
});
}

export async function fetchDataSourceVersion(
export async function fetchDataSourceMetaData(
http: HttpStart,
{ endpoint, auth: { type, credentials } }: DataSourceAttributes,
dataSourceID?: string
Expand All @@ -219,7 +219,7 @@
},
};

return await http.post(`/internal/data-source-management/fetchDataSourceVersion`, {
return await http.post(`/internal/data-source-management/fetchDataSourceMetaData`, {

Check warning on line 222 in src/plugins/data_source_management/public/components/utils.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/data_source_management/public/components/utils.ts#L222

Added line #L222 was not covered by tests
body: JSON.stringify(query),
});
}
Expand Down
3 changes: 2 additions & 1 deletion src/plugins/data_source_management/public/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -232,8 +232,9 @@ export const getMappedDataSources = [
},
];

export const fetchDataSourceVersion = {
export const fetchDataSourceMetaData = {
dataSourceVersion: '2.11.0',
installedPlugins: ['opensearch-ml', 'opensearch-sql'],
};

export const mockDataSourceAttributesWithAuth = {
Expand Down
1 change: 1 addition & 0 deletions src/plugins/data_source_management/public/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ export interface DataSourceAttributes extends SavedObjectAttributes {
description?: string;
endpoint?: string;
dataSourceVersion?: string;
installedPlugins?: string[];
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we need to expose getters for the installed plugins and version fields for plugins to use so that they don't need to know implementation details

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yep, will wrap the field in getters in next one, this one will be focusing on adding the installedPlugins first ~

auth: {
type: AuthType | string;
credentials:
Expand Down
Loading