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

Fix Datasource testing connection don't validate endpoints with path #5663

Merged
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- [BUG][Discover] Fix what is displayed in `selected fields` when removing columns from canvas [#5537](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5537)
- [BUG][Discover] Fix advanced setting `discover:modifyColumnsOnSwitch` ([#5508](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5508))
- [Discover] Fix missing index pattern field from breaking Discover [#5626](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5626)
- [BUG][Multiple Datasource] fix Datasource testing connection don't validate endpoints with path [#5663](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5663)

### 🚞 Infrastructure

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,9 @@ describe('DataSourceManagement: Utils.ts', () => {
expect(isValidUrl('')).toBeFalsy();
expect(isValidUrl('test')).toBeFalsy();

/* False cases: path name scenario*/
expect(isValidUrl('https://test.com/_somepath')).toBeFalsy();

/* True cases */
expect(isValidUrl('https://test.com')).toBeTruthy();
expect(isValidUrl('http://test.com')).toBeTruthy();
Expand Down
xinruiba marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,11 @@ export async function testConnection(
export const isValidUrl = (endpoint: string) => {
try {
const url = new URL(endpoint);
return Boolean(url) && (url.protocol === 'http:' || url.protocol === 'https:');
return (
Boolean(url) &&
(url.protocol === 'http:' || url.protocol === 'https:') &&
(!url.pathname || url.pathname.length === 0 || url.pathname === '/')
);
} catch (e) {
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@ import { AuthType } from '../../types';
import { CreateDataSourceState } from '../create_data_source_wizard/components/create_form/create_data_source_form';
import { EditDataSourceState } from '../edit_data_source/components/edit_form/edit_data_source_form';
import { defaultValidation, performDataSourceFormValidation } from './datasource_form_validation';
import { mockDataSourceAttributesWithAuth } from '../../mocks';
import {
mockDataSourceAttributesWithAuth,
mockDataSourceAttributesWithValidEndpoint,
mockDataSourceAttributesWithInvalidEndpoint,
} from '../../mocks';

describe('DataSourceManagement: Form Validation', () => {
describe('validate create/edit datasource', () => {
Expand Down Expand Up @@ -49,6 +53,18 @@ describe('DataSourceManagement: Form Validation', () => {
const result = performDataSourceFormValidation(form, [], '');
expect(result).toBe(false);
});
test('should fail validation when endpoint path is not empty', () => {
form.endpoint = mockDataSourceAttributesWithInvalidEndpoint.endpoint;
const result = performDataSourceFormValidation(form, [], '');
expect(result).toBe(false);
});
test('should NOT fail validation when endpoint path is empty', () => {
form.auth.type = AuthType.NoAuth;
form.title = 'test';
form.endpoint = mockDataSourceAttributesWithValidEndpoint.endpoint;
xinruiba marked this conversation as resolved.
Show resolved Hide resolved
const result = performDataSourceFormValidation(form, [], '');
expect(result).toBe(true);
});
test('should NOT fail validation on empty username/password when No Auth is selected', () => {
form.auth.type = AuthType.NoAuth;
form.title = 'test';
Expand Down
28 changes: 28 additions & 0 deletions src/plugins/data_source_management/public/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,34 @@ export const getMappedDataSources = [
},
];

export const mockDataSourceAttributesWithValidEndpoint = {
id: 'test',
title: 'create-test-ds',
description: 'jest testing',
endpoint: 'https://test.com',
auth: {
type: AuthType.UsernamePasswordType,
credentials: {
username: 'test123',
password: 'test123',
},
},
};

export const mockDataSourceAttributesWithInvalidEndpoint = {
id: 'test',
title: 'create-test-ds',
description: 'jest testing',
endpoint: 'https://test.com/_somepath',
auth: {
type: AuthType.UsernamePasswordType,
credentials: {
username: 'test123',
password: 'test123',
},
},
};

export const mockDataSourceAttributesWithAuth = {
id: 'test',
title: 'create-test-ds',
Expand Down
Loading