Skip to content

Commit

Permalink
- Refactor dev tool console to use opensearch-js client to interact w…
Browse files Browse the repository at this point in the history 
…ith OpenSearch

- Update tests

Signed-off-by: Su <[email protected]>
  • Loading branch information
@zhongnansu
zhongnansu committed Mar 8, 2023
1 parent 844059c commit 13c5c57
Show file tree
Hide file tree
Showing 7 changed files with 109 additions and 268 deletions.
7 changes: 4 additions & 3 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,11 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- [Optimizer] Increase timeout waiting for the exiting of an optimizer worker ([#3193](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3193))
- [Data] Update `createAggConfig` so that newly created configs can be added to beginning of `aggConfig` array ([#3160](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3160))
- Add disablePrototypePoisoningProtection configuration to prevent JS client from erroring when cluster utilizes JS reserved words ([#2992](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2992))
- [Multiple DataSource] Add support for SigV4 authentication ([#3058](https://github.com/opensearch-project/OpenSearch-Dashboards/issues/3058))
- Make build scripts find and use the latest version of Node.js that satisfies `engines.node` ([#3467](https://github.com/opensearch-project/OpenSearch-Dashboards/issues/3467))
- [Multiple DataSource] Refactor test connection to support SigV4 auth type ([#3456](https://github.com/opensearch-project/OpenSearch-Dashboards/issues/3456))
- [Multiple DataSource] Add support for SigV4 authentication ([#3058](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3058))
- Make build scripts find and use the latest version of Node.js that satisfies `engines.node` ([#3467](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3467))
- [Multiple DataSource] Refactor test connection to support SigV4 auth type ([#3456](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3456))
- [Darwin] Add support for Darwin for running OpenSearch snapshots with `yarn opensearch snapshot` ([#3537](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3537))
- [Multiple DataSource] Refactor dev tool console to use opensearch-js client to send requests ([#3544](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3544))

### 🐛 Bug Fixes

Expand Down
161 changes: 37 additions & 124 deletions src/plugins/console/server/routes/api/console/proxy/create_handler.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,77 +28,18 @@
* under the License.
*/

import { Agent, IncomingMessage } from 'http';
import * as url from 'url';
import { pick, trimStart, trimEnd } from 'lodash';

import { OpenSearchDashboardsRequest, RequestHandler } from 'opensearch-dashboards/server';

import { OpenSearchConfigForProxy } from '../../../../types';
import {
getOpenSearchProxyConfig,
ProxyConfigCollection,
proxyRequest,
setHeaders,
} from '../../../../lib';
import { ResponseError } from '@opensearch-project/opensearch/lib/errors';
import { ApiResponse } from '@opensearch-project/opensearch/';

// TODO: find a better way to get information from the request like remoteAddress and remotePort
// for forwarding.
// eslint-disable-next-line @osd/eslint/no-restricted-paths
import { ensureRawRequest } from '../../../../../../../core/server/http/router';

import { RouteDependencies } from '../../../';

import { Body, Query } from './validation_config';

function toURL(base: string, path: string) {
const urlResult = new url.URL(`${trimEnd(base, '/')}/${trimStart(path, '/')}`);
// Appending pretty here to have OpenSearch do the JSON formatting, as doing
// in JS can lead to data loss (7.0 will get munged into 7, thus losing indication of
// measurement precision)
if (!urlResult.searchParams.get('pretty')) {
urlResult.searchParams.append('pretty', 'true');
}
return urlResult;
}

function filterHeaders(originalHeaders: object, headersToKeep: string[]): object {
const normalizeHeader = function (header: any) {
if (!header) {
return '';
}
header = header.toString();
return header.trim().toLowerCase();
};

// Normalize list of headers we want to allow in upstream request
const headersToKeepNormalized = headersToKeep.map(normalizeHeader);

return pick(originalHeaders, headersToKeepNormalized);
}

function getRequestConfig(
headers: object,
opensearchConfig: OpenSearchConfigForProxy,
proxyConfigCollection: ProxyConfigCollection,
uri: string
): { agent: Agent; timeout: number; headers: object; rejectUnauthorized?: boolean } {
const filteredHeaders = filterHeaders(headers, opensearchConfig.requestHeadersWhitelist);
const newHeaders = setHeaders(filteredHeaders, opensearchConfig.customHeaders);

if (proxyConfigCollection.hasConfig()) {
return {
...proxyConfigCollection.configForUri(uri),
headers: newHeaders,
} as any;
}

return {
...getOpenSearchProxyConfig(opensearchConfig),
headers: newHeaders,
};
}

function getProxyHeaders(req: OpenSearchDashboardsRequest) {
const headers = Object.create(null);

Expand Down Expand Up @@ -131,6 +72,10 @@ export const createHandler = ({
const { body, query } = request;
const { path, method } = query;

let opensearchResponse: ApiResponse;

const client = ctx.core.opensearch.client.asCurrentUser;

if (!pathFilters.some((re) => re.test(path))) {
return response.forbidden({
body: `Error connecting to '${path}':\n\nUnable to send requests to that path.`,
Expand All @@ -140,77 +85,45 @@ export const createHandler = ({
});
}

const legacyConfig = await readLegacyOpenSearchConfig();
const { hosts } = legacyConfig;
let opensearchIncomingMessage: IncomingMessage;

for (let idx = 0; idx < hosts.length; ++idx) {
const host = hosts[idx];
try {
const uri = toURL(host, path);

// Because this can technically be provided by a settings-defined proxy config, we need to
// preserve these property names to maintain BWC.
const { timeout, agent, headers, rejectUnauthorized } = getRequestConfig(
request.headers,
legacyConfig,
proxyConfigCollection,
uri.toString()
);

const requestHeaders = {
...headers,
...getProxyHeaders(request),
};

opensearchIncomingMessage = await proxyRequest({
method: method.toLowerCase() as any,
headers: requestHeaders,
uri,
timeout,
payload: body,
rejectUnauthorized,
agent,
try {
const requestHeaders = {
...getProxyHeaders(request),
};

opensearchResponse = await client.transport.request(
{ path: path + '?pretty=true', method, body },
{ headers: requestHeaders }
);

const { statusCode, body: responseContent, warnings } = opensearchResponse;

if (method.toUpperCase() !== 'HEAD') {
return response.custom({
statusCode: statusCode!,
body: responseContent,
headers: {
warning: warnings || '',
},
});

break;
} catch (e) {
// If we reached here it means we hit a lower level network issue than just, for e.g., a 500.
// We try contacting another node in that case.
log.error(e);
if (idx === hosts.length - 1) {
log.warn(`Could not connect to any configured OpenSearch node [${hosts.join(', ')}]`);
return response.customError({
statusCode: 502,
body: e,
});
}
// Otherwise, try the next host...
}
}

const {
statusCode,
statusMessage,
headers: { warning },
} = opensearchIncomingMessage!;

if (method.toUpperCase() !== 'HEAD') {
return response.custom({
statusCode: statusCode!,
body: opensearchIncomingMessage!,
body: `${statusCode} - ${responseContent}`,
headers: {
warning: warning || '',
warning: warnings || '',
'Content-Type': 'text/plain',
},
});
} catch (e: any) {
log.error(e);
return response.customError({
statusCode: isResponseError(e) ? e.statusCode : 502,
body: isResponseError(e) ? JSON.stringify(e.meta.body) : e,
});
}
};

return response.custom({
statusCode: statusCode!,
body: `${statusCode} - ${statusMessage}`,
headers: {
warning: warning || '',
'Content-Type': 'text/plain',
},
});
const isResponseError = (error: any): error is ResponseError => {
return Boolean(error.body && error.statusCode && error.headers);
};
41 changes: 20 additions & 21 deletions src/plugins/console/server/routes/api/console/proxy/tests/body.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,23 +31,31 @@
import { getProxyRouteHandlerDeps } from './mocks';

import expect from '@osd/expect';
import { Readable } from 'stream';

import { opensearchDashboardsResponseFactory } from '../../../../../../../../core/server';
import {
IScopedClusterClient,
opensearchDashboardsResponseFactory,
} from '../../../../../../../../core/server';
import { createHandler } from '../create_handler';
import * as requestModule from '../../../../../lib/proxy_request';
import { createResponseStub } from './stubs';

import { coreMock, opensearchServiceMock } from '../../../../../../../../core/server/mocks';

describe('Console Proxy Route', () => {
let request: any;
let opensearchClient: DeeplyMockedKeys<IScopedClusterClient>;

beforeEach(() => {
request = (method: string, path: string, response: string) => {
(requestModule.proxyRequest as jest.Mock).mockResolvedValue(createResponseStub(response));
const mockResponse = opensearchServiceMock.createSuccessTransportRequestPromise(response);

const requestHandlerContextMock = coreMock.createRequestHandlerContext();
opensearchClient = requestHandlerContextMock.opensearch.client;

opensearchClient.asCurrentUser.transport.request.mockResolvedValueOnce(mockResponse);
const handler = createHandler(getProxyRouteHandlerDeps({}));

return handler(
{} as any,
{ core: requestHandlerContextMock, dataSource: {} as any },
{
headers: {},
query: { method, path },
Expand All @@ -57,15 +65,6 @@ describe('Console Proxy Route', () => {
};
});

const readStream = (s: Readable) =>
new Promise((resolve) => {
let v = '';
s.on('data', (data) => {
v += data;
});
s.on('end', () => resolve(v));
});

afterEach(async () => {
jest.resetAllMocks();
});
Expand All @@ -74,36 +73,36 @@ describe('Console Proxy Route', () => {
describe('GET request', () => {
it('returns the exact body', async () => {
const { payload } = await request('GET', '/', 'foobar');
expect(await readStream(payload)).to.be('foobar');
expect(payload).to.be('foobar');
});
});
describe('POST request', () => {
it('returns the exact body', async () => {
const { payload } = await request('POST', '/', 'foobar');
expect(await readStream(payload)).to.be('foobar');
expect(payload).to.be('foobar');
});
});
describe('PUT request', () => {
it('returns the exact body', async () => {
const { payload } = await request('PUT', '/', 'foobar');
expect(await readStream(payload)).to.be('foobar');
expect(payload).to.be('foobar');
});
});
describe('DELETE request', () => {
it('returns the exact body', async () => {
const { payload } = await request('DELETE', '/', 'foobar');
expect(await readStream(payload)).to.be('foobar');
expect(payload).to.be('foobar');
});
});
describe('HEAD request', () => {
it('returns the status code and text', async () => {
const { payload } = await request('HEAD', '/');
const { payload } = await request('HEAD', '/', 'OK');
expect(typeof payload).to.be('string');
expect(payload).to.be('200 - OK');
});
describe('mixed casing', () => {
it('returns the status code and text', async () => {
const { payload } = await request('HeAd', '/');
const { payload } = await request('HeAd', '/', 'OK');
expect(typeof payload).to.be('string');
expect(payload).to.be('200 - OK');
});
Expand Down
31 changes: 15 additions & 16 deletions src/plugins/console/server/routes/api/console/proxy/tests/headers.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,25 +32,21 @@ jest.mock('../../../../../../../../core/server/http/router/request', () => ({
ensureRawRequest: jest.fn(),
}));

import { opensearchDashboardsResponseFactory } from '../../../../../../../../core/server';

import {
IScopedClusterClient,
opensearchDashboardsResponseFactory,
} from '../../../../../../../../core/server';
// eslint-disable-next-line @osd/eslint/no-restricted-paths
import { ensureRawRequest } from '../../../../../../../../core/server/http/router/request';

import { getProxyRouteHandlerDeps } from './mocks';

import expect from '@osd/expect';
import * as requestModule from '../../../../../lib/proxy_request';

import { createHandler } from '../create_handler';

import { createResponseStub } from './stubs';
import { coreMock } from '../../../../../../../../core/server/mocks';

describe('Console Proxy Route', () => {
let handler: ReturnType<typeof createHandler>;

beforeEach(() => {
(requestModule.proxyRequest as jest.Mock).mockResolvedValue(createResponseStub(''));
handler = createHandler(getProxyRouteHandlerDeps({}));
});

Expand All @@ -59,7 +55,10 @@ describe('Console Proxy Route', () => {
});

describe('headers', () => {
let opensearchClient: DeeplyMockedKeys<IScopedClusterClient>;
it('forwards the remote header info', async () => {
const requestHandlerContextMock = coreMock.createRequestHandlerContext();
opensearchClient = requestHandlerContextMock.opensearch.client;
(ensureRawRequest as jest.Mock).mockReturnValue({
// This mocks the shape of the hapi request object, will probably change
info: {
Expand All @@ -75,7 +74,7 @@ describe('Console Proxy Route', () => {
});

await handler(
{} as any,
{ core: requestHandlerContextMock, dataSource: {} as any },
{
headers: {},
query: {
Expand All @@ -86,16 +85,16 @@ describe('Console Proxy Route', () => {
opensearchDashboardsResponseFactory
);

expect((requestModule.proxyRequest as jest.Mock).mock.calls.length).to.be(1);
const [[{ headers }]] = (requestModule.proxyRequest as jest.Mock).mock.calls;
const [[, opts]] = opensearchClient.asCurrentUser.transport.request.mock.calls;
const headers = opts?.headers;
expect(headers).to.have.property('x-forwarded-for');
expect(headers['x-forwarded-for']).to.be('0.0.0.0');
expect(headers!['x-forwarded-for']).to.be('0.0.0.0');
expect(headers).to.have.property('x-forwarded-port');
expect(headers['x-forwarded-port']).to.be('1234');
expect(headers!['x-forwarded-port']).to.be('1234');
expect(headers).to.have.property('x-forwarded-proto');
expect(headers['x-forwarded-proto']).to.be('http');
expect(headers!['x-forwarded-proto']).to.be('http');
expect(headers).to.have.property('x-forwarded-host');
expect(headers['x-forwarded-host']).to.be('test');
expect(headers!['x-forwarded-host']).to.be('test');
});
});
});
Loading

0 comments on commit 13c5c57

Please sign in to comment.