ui: all xhr paths from db console are now relative

This commit removes all prefix `/` characters from request paths in the DB Console and Cluster UI codebases. This ensures that if the DB Console is proxied at a subpath the requests continue to work as expected and are relative to the correct base path. Resolves: cockroachdb/helm-charts#228 Resolves: #91429 Epic: None Release note (ops change, ui change): The DB Console now constructs client-side requests using relative URLs instead of absolute ones. This enables proxying of the DB Console at arbitrary subpaths.
cockroachdb · Oct 3, 2023 · 7540da6 · 7540da6
1 parent f6a5970
commit 7540da6
Show file tree

Hide file tree

Showing 19 changed files with 134 additions and 32 deletions.
diff --git a/pkg/ui/workspaces/cluster-ui/src/api/basePath.spec.ts b/pkg/ui/workspaces/cluster-ui/src/api/basePath.spec.ts
@@ -0,0 +1,70 @@
+// Copyright 2023 The Cockroach Authors.
+//
+// Use of this software is governed by the Business Source License
+// included in the file licenses/BSL.txt.
+//
+// As of the Change Date specified in that file, in accordance with
+// the Business Source License, use of this software will be governed
+// by the Apache License, Version 2.0, included in the file
+// licenses/APL.txt.
+
+import { setBasePath, withBasePath } from "./basePath";
+
+describe("withBasePath", () => {
+  afterAll(() => {
+    setBasePath("");
+  });
+  beforeAll(() => {
+    setBasePath("");
+  });
+
+  const testCases = [
+    {
+      basePath: "",
+      path: "",
+      expected: "",
+    },
+    {
+      basePath: "",
+      path: "ppp",
+      expected: "ppp",
+    },
+    {
+      basePath: "",
+      path: "/ppp",
+      expectedError: `Application paths must remain compatible with relative base. Remove prefix \`/\` character.`,
+    },
+    {
+      basePath: "dbconsole",
+      path: "",
+      expected: "dbconsole/",
+    },
+    {
+      basePath: "dbconsole",
+      path: "ppp",
+      expected: "dbconsole/ppp",
+    },
+    {
+      basePath: "dbconsole/",
+      path: "",
+      expected: "dbconsole/",
+    },
+    {
+      basePath: "dbconsole/",
+      path: "ppp",
+      expected: "dbconsole/ppp",
+    },
+  ];
+
+  test.each(testCases)(
+    "inputs %s and %s",
+    ({ path, basePath, expected, expectedError }) => {
+      setBasePath(basePath);
+      if (expectedError && expectedError !== "") {
+        expect(() => withBasePath(path)).toThrow(expectedError);
+      } else {
+        expect(withBasePath(path)).toEqual(expected);
+      }
+    },
+  );
+});
diff --git a/pkg/ui/workspaces/cluster-ui/src/api/basePath.ts b/pkg/ui/workspaces/cluster-ui/src/api/basePath.ts
@@ -12,4 +12,40 @@ let path = "";
 
 export const setBasePath = (basePath: string): string => (path = basePath);
 
-export const getBasePath = (): string => path;
+// Do not call this directly, use `withBasePath` instead because it
+// ensures that if the basePath is blank the paths remain relative.
+const getBasePath = (): string => path;
+
+export const withBasePath = (p: string): string =>
+  joinPathsEnsureRelative(getBasePath(), p);
+
+// joinPathsEnsureRelative is a utility to ensure that paths relative
+// to base don't have a prefix `/` to remain compatible with an empty
+// relative basePath. It ensures that we append it to a base correctly,
+// regardless of trailing slash on the base path.
+//
+// Examples:
+//  joinPathsEnsureRelative("",  "/_status") -> Error! Not a relative path.
+//  joinPathsEnsureRelative("/", "/_status") -> Error! Not a relative path.
+//  joinPathsEnsureRelative("",  "_status") -> "_status"
+//  joinPathsEnsureRelative("/", "_status") -> "/_status"
+//  joinPathsEnsureRelative("dbconsole", "_status") -> "dbconsole/_status"
+//
+export const joinPathsEnsureRelative = (
+  left: string,
+  right: string,
+): string => {
+  if (right.startsWith("/")) {
+    throw new Error(
+      "Application paths must remain compatible with relative base. Remove prefix `/` character.",
+    );
+  }
+  if (left === "") {
+    return right;
+  }
+  if (left.endsWith("/")) {
+    return left.concat(right);
+  } else {
+    return left.concat("/", right);
+  }
+};
diff --git a/pkg/ui/workspaces/cluster-ui/src/api/fetchData.ts b/pkg/ui/workspaces/cluster-ui/src/api/fetchData.ts
@@ -10,7 +10,7 @@
 
 import { cockroach } from "@cockroachlabs/crdb-protobuf-client";
 import { RequestError } from "../util";
-import { getBasePath } from "./basePath";
+import { withBasePath } from "./basePath";
 
 interface ProtoBuilder<
   P extends ConstructorType,
@@ -62,9 +62,8 @@ export const fetchData = <P extends ProtoBuilder<P>, T extends ProtoBuilder<T>>(
     params.method = "POST";
     params.body = toArrayBuffer(encodedRequest);
   }
-  const basePath = getBasePath();
 
-  return fetch(`${basePath}${path}`, params)
+  return fetch(withBasePath(path), params)
     .then(response => {
       if (!response.ok) {
         return response.arrayBuffer().then(buffer => {
@@ -113,9 +112,7 @@ export function fetchDataJSON<ResponseType, RequestType>(
     params.body = JSON.stringify(reqPayload);
   }
 
-  const basePath = getBasePath();
-
-  return fetch(`${basePath}${path}`, params).then(response => {
+  return fetch(withBasePath(path), params).then(response => {
     if (!response.ok) {
       throw new RequestError(
         response.statusText,

diff --git a/pkg/ui/workspaces/cluster-ui/src/api/indexDetailsApi.ts b/pkg/ui/workspaces/cluster-ui/src/api/indexDetailsApi.ts
@@ -45,7 +45,7 @@ export const getIndexStats = (
 ): Promise<TableIndexStatsResponse> => {
   return fetchData(
     cockroach.server.serverpb.TableIndexStatsResponse,
-    `/_status/databases/${req.database}/tables/${req.table}/indexstats`,
+    `_status/databases/${req.database}/tables/${req.table}/indexstats`,
     null,
     null,
     "30M",
@@ -58,7 +58,7 @@ export const resetIndexStats = (
 ): Promise<ResetIndexUsageStatsResponse> => {
   return fetchData(
     cockroach.server.serverpb.ResetIndexUsageStatsResponse,
-    "/_status/resetindexusagestats",
+    "_status/resetindexusagestats",
     cockroach.server.serverpb.ResetIndexUsageStatsRequest,
     req,
     "30M",

diff --git a/pkg/ui/workspaces/cluster-ui/src/api/jobsApi.ts b/pkg/ui/workspaces/cluster-ui/src/api/jobsApi.ts
@@ -12,7 +12,7 @@ import { cockroach } from "@cockroachlabs/crdb-protobuf-client";
 import { fetchData } from "./fetchData";
 import { propsToQueryString } from "../util";
 
-const JOBS_PATH = "/_admin/v1/jobs";
+const JOBS_PATH = "_admin/v1/jobs";
 
 export type JobsRequest = cockroach.server.serverpb.JobsRequest;
 export type JobsResponse = cockroach.server.serverpb.JobsResponse;

diff --git a/pkg/ui/workspaces/cluster-ui/src/api/livenessApi.ts b/pkg/ui/workspaces/cluster-ui/src/api/livenessApi.ts
@@ -11,7 +11,7 @@
 import { cockroach } from "@cockroachlabs/crdb-protobuf-client";
 import { fetchData } from "src/api";
 
-const LIVENESS_PATH = "/_admin/v1/liveness";
+const LIVENESS_PATH = "_admin/v1/liveness";
 
 export const getLiveness =
   (): Promise<cockroach.server.serverpb.LivenessResponse> => {

diff --git a/pkg/ui/workspaces/cluster-ui/src/api/nodesApi.ts b/pkg/ui/workspaces/cluster-ui/src/api/nodesApi.ts
@@ -11,7 +11,7 @@
 import { cockroach } from "@cockroachlabs/crdb-protobuf-client";
 import { fetchData } from "src/api";
 
-const NODES_PATH = "/_status/nodes";
+const NODES_PATH = "_status/nodes";
 
 export const getNodes =
   (): Promise<cockroach.server.serverpb.NodesResponse> => {

diff --git a/pkg/ui/workspaces/cluster-ui/src/api/sessionsApi.ts b/pkg/ui/workspaces/cluster-ui/src/api/sessionsApi.ts
@@ -11,7 +11,7 @@
 import { cockroach } from "@cockroachlabs/crdb-protobuf-client";
 import { fetchData } from "src/api";
 
-const SESSIONS_PATH = "/_status/sessions";
+const SESSIONS_PATH = "_status/sessions";
 
 export type SessionsRequestMessage =
   cockroach.server.serverpb.ListSessionsRequest;

diff --git a/pkg/ui/workspaces/cluster-ui/src/api/sqlApi.ts b/pkg/ui/workspaces/cluster-ui/src/api/sqlApi.ts
@@ -69,7 +69,7 @@ export type SqlApiResponse<ResultType> = {
 
 export type SqlApiQueryResponse<Result> = Result & { error?: Error };
 
-export const SQL_API_PATH = "/api/v2/sql/";
+export const SQL_API_PATH = "api/v2/sql/";
 
 /**
  * executeSql executes the provided SQL statements in a single transaction

diff --git a/pkg/ui/workspaces/cluster-ui/src/api/sqlStatsApi.ts b/pkg/ui/workspaces/cluster-ui/src/api/sqlStatsApi.ts
@@ -11,7 +11,7 @@
 import { cockroach } from "@cockroachlabs/crdb-protobuf-client";
 import { fetchData } from "src/api";
 
-const RESET_SQL_STATS_PATH = "/_status/resetsqlstats";
+const RESET_SQL_STATS_PATH = "_status/resetsqlstats";
 
 export const resetSQLStats =
   (): Promise<cockroach.server.serverpb.ResetSQLStatsResponse> => {

diff --git a/pkg/ui/workspaces/cluster-ui/src/api/statementsApi.ts b/pkg/ui/workspaces/cluster-ui/src/api/statementsApi.ts
@@ -21,8 +21,8 @@ import Long from "long";
 import moment from "moment-timezone";
 
 import { AggregateStatistics } from "../statementsTable";
-const STATEMENTS_PATH = "/_status/combinedstmts";
-const STATEMENT_DETAILS_PATH = "/_status/stmtdetails";
+const STATEMENTS_PATH = "_status/combinedstmts";
+const STATEMENT_DETAILS_PATH = "_status/stmtdetails";
 
 export type StatementsRequest =
   cockroach.server.serverpb.CombinedStatementsStatsRequest;

diff --git a/pkg/ui/workspaces/cluster-ui/src/api/terminateQueryApi.ts b/pkg/ui/workspaces/cluster-ui/src/api/terminateQueryApi.ts
@@ -11,7 +11,7 @@
 import { cockroach } from "@cockroachlabs/crdb-protobuf-client";
 import { fetchData } from "src/api";
 
-const STATUS_PREFIX = "/_status";
+const STATUS_PREFIX = "_status";
 
 export type CancelSessionRequestMessage =
   cockroach.server.serverpb.CancelSessionRequest;

diff --git a/pkg/ui/workspaces/cluster-ui/src/api/userApi.ts b/pkg/ui/workspaces/cluster-ui/src/api/userApi.ts
@@ -19,7 +19,7 @@ export type UserSQLRolesResponseMessage =
 export function getUserSQLRoles(): Promise<UserSQLRolesResponseMessage> {
   return fetchData(
     cockroach.server.serverpb.UserSQLRolesResponse,
-    `/_status/sqlroles`,
+    `_status/sqlroles`,
     null,
     null,
     "30M",

diff --git a/pkg/ui/workspaces/cluster-ui/src/sessions/sessionsPageConnected.spec.ts b/pkg/ui/workspaces/cluster-ui/src/sessions/sessionsPageConnected.spec.ts
@@ -50,7 +50,7 @@ describe("SessionsPage Connections", () => {
       await driver.cancelQuery({ node_id: "1" });
       assert.deepStrictEqual(
         fetchMock.mock.calls[0][0],
-        "/_status/cancel_query/1",
+        "_status/cancel_query/1",
       );
     });
   });
@@ -62,7 +62,7 @@ describe("SessionsPage Connections", () => {
       await driver.cancelSession({ node_id: "1" });
       assert.deepStrictEqual(
         fetchMock.mock.calls[0][0],
-        "/_status/cancel_session/1",
+        "_status/cancel_session/1",
       );
     });
   });

diff --git a/pkg/ui/workspaces/cluster-ui/src/statementDetails/diagnostics/diagnosticsView.tsx b/pkg/ui/workspaces/cluster-ui/src/statementDetails/diagnostics/diagnosticsView.tsx
@@ -22,7 +22,7 @@ import emptyListResultsImg from "src/assets/emptyState/empty-list-results.svg";
 import { filterByTimeScale, getDiagnosticsStatus } from "./diagnosticsUtils";
 import { EmptyTable } from "src/empty";
 import styles from "./diagnosticsView.module.scss";
-import { getBasePath, StatementDiagnosticsReport } from "../../api";
+import { StatementDiagnosticsReport, withBasePath } from "../../api";
 import {
   TimeScale,
   timeScale1hMinOptions,
@@ -209,9 +209,9 @@ export class DiagnosticsView extends React.Component<
                 as="a"
                 size="small"
                 intent="tertiary"
-                href={`${getBasePath()}/_admin/v1/stmtbundle/${
-                  diagnostic.statement_diagnostics_id
-                }`}
+                href={withBasePath(
+                  `_admin/v1/stmtbundle/${diagnostic.statement_diagnostics_id}`,
+                )}
                 onClick={() =>
                   this.props.onDownloadDiagnosticBundleClick &&
                   this.props.onDownloadDiagnosticBundleClick(

diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTableContent.tsx b/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTableContent.tsx
@@ -30,7 +30,7 @@ import {
 } from "src/util";
 import styles from "./statementsTableContent.module.scss";
 import { EllipsisVertical } from "@cockroachlabs/icons";
-import { getBasePath } from "src/api/basePath";
+import { withBasePath } from "src/api/basePath";
 import { StatementDiagnosticsReport } from "src/api/statementDiagnosticsApi";
 import moment from "moment-timezone";
 
@@ -135,9 +135,9 @@ export const StatementTableCell = {
                       name: (
                         <a
                           className={cx("diagnostic-report-dropdown-option")}
-                          href={`${getBasePath()}/_admin/v1/stmtbundle/${
-                            dr.statement_diagnostics_id
-                          }`}
+                          href={withBasePath(
+                            `_admin/v1/stmtbundle/${dr.statement_diagnostics_id}`,
+                          )}
                         >
                           {`Download ${moment(dr.requested_at).format(
                             "MMM DD, YYYY [at] H:mm [(UTC)] [diagnostic]",

diff --git a/pkg/ui/workspaces/db-console/src/redux/analytics.ts b/pkg/ui/workspaces/db-console/src/redux/analytics.ts
@@ -314,7 +314,6 @@ export function initializeAnalytics(store: Store<AdminUIState>) {
     analyticsOpts,
   );
   analytics = new AnalyticsSync(analyticsInstance, store, defaultRedactions);
-
   // Attach a listener to the history object which will track a 'page' event
   // whenever the user navigates to a new path.
   let lastPageLocation: Location;

diff --git a/pkg/ui/workspaces/db-console/src/util/dataFromServer.ts b/pkg/ui/workspaces/db-console/src/util/dataFromServer.ts
@@ -33,7 +33,7 @@ declare global {
 }
 
 export function fetchDataFromServer(): Promise<DataFromServer> {
-  return fetch("/uiconfig", {
+  return fetch("uiconfig", {
     method: "GET",
     headers: {
       Accept: "application/json",

diff --git a/pkg/ui/workspaces/db-console/src/views/jwt/jwtAuthToken.tsx b/pkg/ui/workspaces/db-console/src/views/jwt/jwtAuthToken.tsx
@@ -159,7 +159,7 @@ export const JwtAuthTokenPage = () => {
   useEffect(() => {
     const { State, Code } = JSON.parse(atob(oidc));
 
-    fetch(`/oidc/v1/jwt?state=${State}&code=${Code}`).then(
+    fetch(`oidc/v1/jwt?state=${State}&code=${Code}`).then(
       (response: Response) => {
         setLoading(false);
         if (response.ok) {