Refactor dev tool console to use opensearch-js client to send request (…

…opensearch-project#3544) Refactor dev tool console to use opensearch-js client to interact with OpenSearch Signed-off-by: Su <szhongna@amazon.com>
kappassov · Mar 21, 2023 · 1d94df4 · 1d94df4
1 parent 95a0b24
commit 1d94df4
Show file tree

Hide file tree

Showing 7 changed files with 123 additions and 270 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -62,12 +62,13 @@ 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))
 - [Vis Builder] Add metric to metric, bucket to bucket aggregation persistence ([#3495](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3495))
 - Use mirrors to download Node.js binaries to escape sporadic 404 errors ([#3619](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3619))
+- [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
 

diff --git a/src/plugins/console/server/routes/api/console/proxy/create_handler.ts b/src/plugins/console/server/routes/api/console/proxy/create_handler.ts
@@ -28,77 +28,19 @@
  * 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 { trimStart } from 'lodash';
 
-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);
 
@@ -124,12 +66,26 @@ function getProxyHeaders(req: OpenSearchDashboardsRequest) {
   return headers;
 }
 
+function toUrlPath(path: string) {
+  let urlPath = `/${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 (!urlPath.includes('?pretty')) {
+    urlPath += '?pretty=true';
+  }
+  return urlPath;
+}
+
 export const createHandler = ({
   log,
   proxy: { readLegacyOpenSearchConfig, pathFilters, proxyConfigCollection },
 }: RouteDependencies): RequestHandler<unknown, Query, Body> => async (ctx, request, response) => {
   const { body, query } = request;
   const { path, method } = query;
+  const client = ctx.core.opensearch.client.asCurrentUser;
+
+  let opensearchResponse: ApiResponse;
 
   if (!pathFilters.some((re) => re.test(path))) {
     return response.forbidden({
@@ -140,77 +96,46 @@ 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: toUrlPath(path), 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);
+    const isResponseErrorFlag = isResponseError(e);
+    return response.customError({
+      statusCode: isResponseErrorFlag ? e.statusCode : 502,
+      body: isResponseErrorFlag ? JSON.stringify(e.meta.body) : `502.${e.statusCode || 0}`,
+    });
   }
+};
 
-  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 && error.body && error.statusCode && error.header);
 };
diff --git a/src/plugins/console/server/routes/api/console/proxy/tests/body.test.ts b/src/plugins/console/server/routes/api/console/proxy/tests/body.test.ts
@@ -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 },
@@ -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();
   });
@@ -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');
         });