From 7540da6442fb81a52bacb35e6e579f20a1659096 Mon Sep 17 00:00:00 2001 From: David Hartunian Date: Wed, 9 Aug 2023 15:50:12 -0400 Subject: [PATCH] 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: https://github.com/cockroachdb/helm-charts/issues/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. --- .../cluster-ui/src/api/basePath.spec.ts | 70 +++++++++++++++++++ .../workspaces/cluster-ui/src/api/basePath.ts | 38 +++++++++- .../cluster-ui/src/api/fetchData.ts | 9 +-- .../cluster-ui/src/api/indexDetailsApi.ts | 4 +- .../workspaces/cluster-ui/src/api/jobsApi.ts | 2 +- .../cluster-ui/src/api/livenessApi.ts | 2 +- .../workspaces/cluster-ui/src/api/nodesApi.ts | 2 +- .../cluster-ui/src/api/sessionsApi.ts | 2 +- .../workspaces/cluster-ui/src/api/sqlApi.ts | 2 +- .../cluster-ui/src/api/sqlStatsApi.ts | 2 +- .../cluster-ui/src/api/statementsApi.ts | 4 +- .../cluster-ui/src/api/terminateQueryApi.ts | 2 +- .../workspaces/cluster-ui/src/api/userApi.ts | 2 +- .../sessions/sessionsPageConnected.spec.ts | 4 +- .../diagnostics/diagnosticsView.tsx | 8 +-- .../statementsTableContent.tsx | 8 +-- .../db-console/src/redux/analytics.ts | 1 - .../db-console/src/util/dataFromServer.ts | 2 +- .../db-console/src/views/jwt/jwtAuthToken.tsx | 2 +- 19 files changed, 134 insertions(+), 32 deletions(-) create mode 100644 pkg/ui/workspaces/cluster-ui/src/api/basePath.spec.ts diff --git a/pkg/ui/workspaces/cluster-ui/src/api/basePath.spec.ts b/pkg/ui/workspaces/cluster-ui/src/api/basePath.spec.ts new file mode 100644 index 000000000000..0835538c59ae --- /dev/null +++ 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 index 30d24ac57130..51e4fcdee608 100644 --- 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 index 653cb5fe009f..2efd6aebb91a 100644 --- 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 =

, T extends ProtoBuilder>( 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( 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 index c58841289a8e..7fef24b45d9c 100644 --- 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 => { 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 => { 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 index 0049eddbece1..03a860138b0a 100644 --- 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 index 279413e53416..6a0ba3e59750 100644 --- 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 => { diff --git a/pkg/ui/workspaces/cluster-ui/src/api/nodesApi.ts b/pkg/ui/workspaces/cluster-ui/src/api/nodesApi.ts index e4bf7bdcd23c..730c57316641 100644 --- 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 => { diff --git a/pkg/ui/workspaces/cluster-ui/src/api/sessionsApi.ts b/pkg/ui/workspaces/cluster-ui/src/api/sessionsApi.ts index 604c85b5374d..4bf39e56668e 100644 --- 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 index c11d60987f2a..d6c51d1f6caa 100644 --- 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 = { export type SqlApiQueryResponse = 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 index 8b24b57fb7ff..106bdc8d7d79 100644 --- 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 => { diff --git a/pkg/ui/workspaces/cluster-ui/src/api/statementsApi.ts b/pkg/ui/workspaces/cluster-ui/src/api/statementsApi.ts index 038ba9c6f94f..05f9c99ff401 100644 --- 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 index b0e65f00e7cf..708bc080d47f 100644 --- 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 index 29362b287943..8d26af67b486 100644 --- 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 { 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 index 0b83d61a3f6d..762d63ed678d 100644 --- 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 index e55be77e7e8d..00efa19cbba5 100644 --- 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 index fbd7412801fd..2445361b0063 100644 --- 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: ( {`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 index b7fdd7ab2086..a19cdde9f0e4 100644 --- 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) { 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 index 14135ab5c300..e1dac5af337b 100644 --- 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 { - 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 index 68d1a9a1204c..882f29d65095 100644 --- 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) {