feat: add fullURL to urlPartsSchema and use fullURL generate url string

Signed-off-by: Lin Wang <wonglam@amazon.com>
opensearch-project · Feb 9, 2023 · 32c4581 · 32c4581
1 parent cbda0f2
commit 32c4581
Show file tree

Hide file tree

Showing 21 changed files with 112 additions and 120 deletions.
diff --git a/packages/osd-opensearch-archiver/src/cli.ts b/packages/osd-opensearch-archiver/src/cli.ts
@@ -35,7 +35,6 @@
  *************************************************************/
 
 import Path from 'path';
-import Url from 'url';
 import readline from 'readline';
 
 import { RunWithCommands, createFlagError } from '@osd/dev-utils';
@@ -72,7 +71,7 @@ export function runCli() {
         throw createFlagError('--opensearch-url must be a string');
       }
       if (!opensearchUrl && config) {
-        opensearchUrl = Url.format(config.get('servers.opensearch'));
+        opensearchUrl = config.get('servers.opensearch').fullURL.toString();
       }
       if (!opensearchUrl) {
         throw createFlagError('--opensearch-url or --config must be defined');
@@ -83,7 +82,9 @@ export function runCli() {
         throw createFlagError('--opensearch-dashboards-url must be a string');
       }
       if (!opensearchDashboardsUrl && config) {
-        opensearchDashboardsUrl = Url.format(config.get('servers.opensearchDashboards'));
+        opensearchDashboardsUrl = config
+          .get('servers.opensearchDashboards')
+          .fullURL.toString() as string;
       }
       if (!opensearchDashboardsUrl) {
         throw createFlagError('---url or --config must be defined');

diff --git a/packages/osd-test/src/functional_test_runner/lib/config/config.ts b/packages/osd-test/src/functional_test_runner/lib/config/config.ts
@@ -126,6 +126,10 @@ export class Config {
       if (typeof v === 'function') {
         return v;
       }
+
+      if (v instanceof URL) {
+        return new URL(v.toString());
+      }
     });
   }
 
@@ -134,6 +138,10 @@ export class Config {
       if (typeof v === 'function') {
         return v;
       }
+
+      if (v instanceof URL) {
+        return new URL(v.toString());
+      }
     });
   }
 }
diff --git a/packages/osd-test/src/functional_test_runner/lib/config/schema.ts b/packages/osd-test/src/functional_test_runner/lib/config/schema.ts
@@ -50,6 +50,7 @@ const urlPartsSchema = () =>
       pathname: Joi.string().regex(/^\//, 'start with a /'),
       hash: Joi.string().regex(/^\//, 'start with a /'),
       certificateAuthorities: Joi.array().items(Joi.binary()).optional(),
+      fullURL: Joi.object().type(URL),
     })
     .default();
 

diff --git a/packages/osd-test/src/legacy_opensearch/legacy_opensearch_test_cluster.js b/packages/osd-test/src/legacy_opensearch/legacy_opensearch_test_cluster.js
@@ -29,7 +29,6 @@
  */
 
 import { resolve } from 'path';
-import { format } from 'url';
 import { get, toPath } from 'lodash';
 import { Cluster } from '@osd/opensearch';
 import { CI_PARALLEL_PROCESS_PREFIX } from '../ci_parallel_process_prefix';
@@ -135,10 +134,10 @@ export function createLegacyOpenSearchTestCluster(options = {}) {
     }
 
     getUrl() {
-      const parts = opensearchTestConfig.getUrlParts();
-      parts.port = port;
+      const url = new URL(opensearchTestConfig.getUrlParts().fullURL);
+      url.port = port;
 
-      return format(parts);
+      return url.toString();
     }
   })();
 }

diff --git a/packages/osd-test/src/legacy_opensearch/opensearch_test_config.js b/packages/osd-test/src/legacy_opensearch/opensearch_test_config.js
@@ -28,7 +28,6 @@
  * under the License.
  */
 
-import url, { format as formatUrl } from 'url';
 import pkg from '../../../../package.json';
 import { adminTestUser } from '../osd';
 
@@ -42,7 +41,7 @@ export const opensearchTestConfig = new (class OpenSearchTestConfig {
   }
 
   getUrl() {
-    return formatUrl(this.getUrlParts());
+    return this.getUrlParts().fullURL.toString();
   }
 
   getBuildFrom() {
@@ -56,30 +55,36 @@ export const opensearchTestConfig = new (class OpenSearchTestConfig {
   getUrlParts() {
     // Allow setting one complete TEST_OPENSEARCH_URL for opensearch like https://opensearch:changeme@example.com:9200
     if (process.env.TEST_OPENSEARCH_URL) {
-      const testOpenSearchUrl = url.parse(process.env.TEST_OPENSEARCH_URL);
+      const testOpenSearchUrl = new URL('', process.env.TEST_OPENSEARCH_URL);
       return {
         // have to remove the ":" off protocol
         protocol: testOpenSearchUrl.protocol.slice(0, -1),
         hostname: testOpenSearchUrl.hostname,
         port: parseInt(testOpenSearchUrl.port, 10),
-        username: testOpenSearchUrl.auth.split(':')[0],
-        password: testOpenSearchUrl.auth.split(':')[1],
-        auth: testOpenSearchUrl.auth,
+        username: testOpenSearchUrl.username,
+        password: testOpenSearchUrl.password,
+        auth: `${testOpenSearchUrl.username}:${testOpenSearchUrl.password}`,
+        fullURL: testOpenSearchUrl,
       };
     }
 
     const username = process.env.TEST_OPENSEARCH_USERNAME || adminTestUser.username;
     const password = process.env.TEST_OPENSEARCH_PASSWORD || adminTestUser.password;
+    const protocol = process.env.TEST_OPENSEARCH_PROTOCOL || 'http';
+    const hostname = process.env.TEST_OPENSEARCH_HOSTNAME || 'localhost';
+    const port = parseInt(process.env.TEST_OPENSEARCH_PORT, 10) || 9220;
+    const fullURL = new URL('', `${protocol}://${username}:${password}@${hostname}:${port}`);
 
     return {
       // Allow setting any individual component(s) of the URL,
       // or use default values (username and password from ../osd/users.js)
-      protocol: process.env.TEST_OPENSEARCH_PROTOCOL || 'http',
-      hostname: process.env.TEST_OPENSEARCH_HOSTNAME || 'localhost',
-      port: parseInt(process.env.TEST_OPENSEARCH_PORT, 10) || 9220,
+      protocol,
+      hostname,
+      port,
       auth: `${username}:${password}`,
       username: username,
       password: password,
+      fullURL,
     };
   }
 })();
diff --git a/packages/osd-test/src/osd/osd_test_config.ts b/packages/osd-test/src/osd/osd_test_config.ts
@@ -28,7 +28,6 @@
  * under the License.
  */
 
-import url from 'url';
 import { opensearchDashboardsTestUser } from './users';
 
 interface UrlParts {
@@ -38,6 +37,7 @@ interface UrlParts {
   auth?: string;
   username?: string;
   password?: string;
+  fullURL: URL;
 }
 
 export const osdTestConfig = new (class OsdTestConfig {
@@ -48,32 +48,42 @@ export const osdTestConfig = new (class OsdTestConfig {
   getUrlParts(): UrlParts {
     // allow setting one complete TEST_OPENSEARCH_DASHBOARDS_URL for opensearch like https://opensearch:changeme@example.com:9200
     if (process.env.TEST_OPENSEARCH_DASHBOARDS_URL) {
-      const testOpenSearchDashboardsUrl = url.parse(process.env.TEST_OPENSEARCH_DASHBOARDS_URL);
+      const testOpenSearchDashboardsUrl = new URL('', process.env.TEST_OPENSEARCH_DASHBOARDS_URL);
       return {
         protocol: testOpenSearchDashboardsUrl.protocol?.slice(0, -1),
         hostname: testOpenSearchDashboardsUrl.hostname ?? undefined,
         port: testOpenSearchDashboardsUrl.port
           ? parseInt(testOpenSearchDashboardsUrl.port, 10)
           : undefined,
-        auth: testOpenSearchDashboardsUrl.auth ?? undefined,
-        username: testOpenSearchDashboardsUrl.auth?.split(':')[0],
-        password: testOpenSearchDashboardsUrl.auth?.split(':')[1],
+        auth:
+          testOpenSearchDashboardsUrl.username && testOpenSearchDashboardsUrl.password
+            ? `${testOpenSearchDashboardsUrl.username}:${testOpenSearchDashboardsUrl.password}`
+            : undefined,
+        username: testOpenSearchDashboardsUrl.username ?? undefined,
+        password: testOpenSearchDashboardsUrl.password ?? undefined,
+        fullURL: testOpenSearchDashboardsUrl,
       };
     }
 
     const username =
       process.env.TEST_OPENSEARCH_DASHBOARDS_USERNAME || opensearchDashboardsTestUser.username;
     const password =
       process.env.TEST_OPENSEARCH_DASHBOARDS_PASSWORD || opensearchDashboardsTestUser.password;
+    const protocol = process.env.TEST_OPENSEARCH_DASHBOARDS_PROTOCOL || 'http';
+    const hostname = process.env.TEST_OPENSEARCH_DASHBOARDS_HOSTNAME || 'localhost';
+    const port = process.env.TEST_OPENSEARCH_DASHBOARDS_PORT
+      ? parseInt(process.env.TEST_OPENSEARCH_DASHBOARDS_PORT, 10)
+      : 5620;
+    const fullURL = new URL(`${protocol}://${username}:${password}@${hostname}:${port}`);
+
     return {
-      protocol: process.env.TEST_OPENSEARCH_DASHBOARDS_PROTOCOL || 'http',
-      hostname: process.env.TEST_OPENSEARCH_DASHBOARDS_HOSTNAME || 'localhost',
-      port: process.env.TEST_OPENSEARCH_DASHBOARDS_PORT
-        ? parseInt(process.env.TEST_OPENSEARCH_DASHBOARDS_PORT, 10)
-        : 5620,
+      protocol,
+      hostname,
+      port,
       auth: `${username}:${password}`,
       username,
       password,
+      fullURL,
     };
   }
 })();
diff --git a/src/test_utils/get_url.js b/src/test_utils/get_url.js
@@ -28,9 +28,6 @@
  * under the License.
  */
 
-import _ from 'lodash';
-import url from 'url';
-
 /**
  * Converts a config and a pathname to a url
  * @param {object} config A url config
@@ -50,17 +47,24 @@ import url from 'url';
  * @return {string}
  */
 
-export default function getUrl(config, app) {
-  return url.format(_.assign({}, config, app));
+export default function getUrl(url, app) {
+  url = new URL(url);
+  if (app.pathname) {
+    url.pathname = app.pathname;
+  }
+  if (app.hash) {
+    url.hash = app.hash;
+  }
+  return url.toString();
 }
 
-getUrl.noAuth = function getUrlNoAuth(config, app) {
-  config = _.pickBy(config, function (val, param) {
-    return param !== 'auth';
-  });
-  return getUrl(config, app);
+getUrl.noAuth = function getUrlNoAuth(url, app) {
+  url = new URL(url);
+  url.username = '';
+  url.password = '';
+  return getUrl(url, app);
 };
 
-getUrl.baseUrl = function getBaseUrl(config) {
-  return url.format(_.pick(config, 'protocol', 'hostname', 'port'));
+getUrl.baseUrl = function getBaseUrl(url) {
+  return url.origin;
 };
diff --git a/src/test_utils/get_url.test.ts b/src/test_utils/get_url.test.ts
@@ -33,46 +33,27 @@ import getUrl from './get_url';
 
 describe('getUrl', function () {
   it('should convert to a url', function () {
-    const url = getUrl(
-      {
-        protocol: 'http',
-        hostname: 'localhost',
-      },
-      {
-        pathname: 'foo',
-      }
-    );
+    const url = getUrl(new URL('http://localhost'), {
+      pathname: 'foo',
+    });
 
     expect(url).to.be('http://localhost/foo');
   });
 
   it('should convert to a url with port', function () {
-    const url = getUrl(
-      {
-        protocol: 'http',
-        hostname: 'localhost',
-        port: 9220,
-      },
-      {
-        pathname: 'foo',
-      }
-    );
+    const url = getUrl(new URL('http://localhost:9220'), {
+      pathname: 'foo',
+    });
 
     expect(url).to.be('http://localhost:9220/foo');
   });
 
   it('should convert to a secure hashed url', function () {
     expect(
-      getUrl(
-        {
-          protocol: 'https',
-          hostname: 'localhost',
-        },
-        {
-          pathname: 'foo',
-          hash: 'bar',
-        }
-      )
+      getUrl(new URL('https://localhost'), {
+        pathname: 'foo',
+        hash: 'bar',
+      })
     ).to.be('https://localhost/foo#bar');
   });
 });
diff --git a/test/api_integration/services/supertest.ts b/test/api_integration/services/supertest.ts
@@ -29,18 +29,19 @@
  */
 
 import { FtrProviderContext } from 'test/functional/ftr_provider_context';
-import { format as formatUrl } from 'url';
 
 import supertestAsPromised from 'supertest-as-promised';
 
 export function OpenSearchDashboardsSupertestProvider({ getService }: FtrProviderContext) {
   const config = getService('config');
-  const opensearchDashboardsServerUrl = formatUrl(config.get('servers.opensearchDashboards'));
+  const opensearchDashboardsServerUrl = config
+    .get('servers.opensearchDashboards')
+    .fullURL.toString();
   return supertestAsPromised(opensearchDashboardsServerUrl);
 }
 
 export function OpenSearchSupertestProvider({ getService }: FtrProviderContext) {
   const config = getService('config');
-  const elasticSearchServerUrl = formatUrl(config.get('servers.opensearch'));
+  const elasticSearchServerUrl = config.get('servers.opensearch').fullURL.toString();
   return supertestAsPromised(elasticSearchServerUrl);
 }
diff --git a/test/common/config.js b/test/common/config.js
@@ -29,7 +29,6 @@
  */
 
 //import path from 'path';
-import { format as formatUrl } from 'url';
 import { opensearchTestConfig, osdTestConfig, opensearchDashboardsServerTestUser } from '@osd/test';
 import { services } from './services';
 
@@ -55,7 +54,7 @@ export default function () {
         '--logging.json=false',
         `--server.port=${osdTestConfig.getPort()}`,
         '--status.allowAnonymous=true',
-        `--opensearch.hosts=${formatUrl(servers.opensearch)}`,
+        `--opensearch.hosts=${servers.opensearch.fullURL.toString()}`,
         `--opensearch.username=${opensearchDashboardsServerTestUser.username}`,
         `--opensearch.password=${opensearchDashboardsServerTestUser.password}`,
         `--home.disableWelcomeScreen=false`,

diff --git a/test/common/services/deployment.ts b/test/common/services/deployment.ts
@@ -44,14 +44,14 @@ export function DeploymentProvider({ getService }: FtrProviderContext) {
      * Returns OpenSearch Dashboards host URL
      */
     getHostPort() {
-      return getUrl.baseUrl(config.get('servers.opensearchDashboards'));
+      return getUrl.baseUrl(config.get('servers.opensearchDashboards.fullURL'));
     },
 
     /**
      * Returns OpenSearch host URL
      */
     getOpenSearchHostPort() {
-      return getUrl.baseUrl(config.get('servers.opensearch'));
+      return getUrl.baseUrl(config.get('servers.opensearch.fullURL'));
     },
 
     /**

diff --git a/test/common/services/legacy_opensearch.ts b/test/common/services/legacy_opensearch.ts
@@ -28,8 +28,6 @@
  * under the License.
  */
 
-import { format as formatUrl } from 'url';
-
 import * as legacyOpenSearch from 'elasticsearch';
 
 import { DEFAULT_API_VERSION } from '../../../src/core/server/opensearch/opensearch_config';
@@ -42,7 +40,7 @@ export function LegacyOpenSearchProvider({
 
   return new legacyOpenSearch.Client({
     apiVersion: DEFAULT_API_VERSION,
-    host: formatUrl(config.get('servers.opensearch')),
+    host: config.get('servers.opensearch.fullURL').toString(),
     requestTimeout: config.get('timeouts.opensearchRequestTimeout'),
   });
 }