From 7c2d43a2dce29678d815a1b88a9de88faafc1226 Mon Sep 17 00:00:00 2001 From: Raphael 'kena' Poss Date: Sun, 29 Jan 2023 10:40:54 +0100 Subject: [PATCH 1/2] rpc: prevent cross-tenant RPCs Prior to this patch, it was possible for a server for tenant 123 to perform RPCs to a server for enant 456. This patch disables that. Release note: None --- pkg/rpc/auth.go | 16 +++++++++++++-- pkg/rpc/auth_test.go | 45 ++++++++++++++++++++++++----------------- pkg/rpc/helpers_test.go | 6 ++++-- 3 files changed, 45 insertions(+), 22 deletions(-) diff --git a/pkg/rpc/auth.go b/pkg/rpc/auth.go index fef89de06078..6e7ce1c1113d 100644 --- a/pkg/rpc/auth.go +++ b/pkg/rpc/auth.go @@ -143,8 +143,20 @@ func (a kvAuth) authenticate(ctx context.Context) (roachpb.TenantID, error) { // // Is this a connection from another SQL tenant server? if security.IsTenantCertificate(clientCert) { - // Incoming connection originating from a tenant SQL server. Let through. - // The other server is able to use any of this server's RPCs. + // Incoming connection originating from a tenant SQL server. + tid, err := tenantFromCommonName(clientCert.Subject.CommonName) + if err != nil { + return roachpb.TenantID{}, err + } + // Verify that our peer is a service for the same tenant + // as ourselves (we don't want to allow tenant 123 to + // serve requests for a client coming from tenant 456). + if tid != a.tenant.tenantID { + return roachpb.TenantID{}, authErrorf("this tenant (%v) cannot serve requests from a server for tenant %v", a.tenant.tenantID, tid) + } + + // We return an unset tenant ID, to bypass authorization checks: + // the other server is able to use any of this server's RPCs. return roachpb.TenantID{}, nil } } diff --git a/pkg/rpc/auth_test.go b/pkg/rpc/auth_test.go index 689c0abefac1..6e832d395949 100644 --- a/pkg/rpc/auth_test.go +++ b/pkg/rpc/auth_test.go @@ -15,6 +15,7 @@ import ( "crypto/tls" "crypto/x509" "crypto/x509/pkix" + "fmt" "io" "testing" @@ -90,32 +91,38 @@ func TestWrappedServerStream(t *testing.T) { require.Equal(t, 3, recv) } -func TestTenantFromCert(t *testing.T) { +func TestAuthenticateTenant(t *testing.T) { defer leaktest.AfterTest(t)() correctOU := []string{security.TenantsOU} + stid := roachpb.SystemTenantID + tenTen := roachpb.MustMakeTenantID(10) for _, tc := range []struct { + systemID roachpb.TenantID ous []string commonName string expTenID roachpb.TenantID expErr string tenantScope uint64 }{ - {ous: correctOU, commonName: "10", expTenID: roachpb.MustMakeTenantID(10)}, - {ous: correctOU, commonName: roachpb.MinTenantID.String(), expTenID: roachpb.MinTenantID}, - {ous: correctOU, commonName: roachpb.MaxTenantID.String(), expTenID: roachpb.MaxTenantID}, - {ous: correctOU, commonName: roachpb.SystemTenantID.String() /* "system" */, expErr: `could not parse tenant ID from Common Name \(CN\)`}, - {ous: correctOU, commonName: "-1", expErr: `could not parse tenant ID from Common Name \(CN\)`}, - {ous: correctOU, commonName: "0", expErr: `invalid tenant ID 0 in Common Name \(CN\)`}, - {ous: correctOU, commonName: "1", expErr: `invalid tenant ID 1 in Common Name \(CN\)`}, - {ous: correctOU, commonName: "root", expErr: `could not parse tenant ID from Common Name \(CN\)`}, - {ous: correctOU, commonName: "other", expErr: `could not parse tenant ID from Common Name \(CN\)`}, - {ous: []string{"foo"}, commonName: "other", expErr: `client certificate CN=other,OU=foo cannot be used to perform RPC on tenant {1}`}, - {ous: nil, commonName: "other", expErr: `client certificate CN=other cannot be used to perform RPC on tenant {1}`}, - {ous: append([]string{"foo"}, correctOU...), commonName: "other", expErr: `could not parse tenant ID from Common Name`}, - {ous: nil, commonName: "root"}, - {ous: nil, commonName: "root", tenantScope: 10, expErr: "client certificate CN=root cannot be used to perform RPC on tenant {1}"}, + {systemID: stid, ous: correctOU, commonName: "10", expTenID: tenTen}, + {systemID: stid, ous: correctOU, commonName: roachpb.MinTenantID.String(), expTenID: roachpb.MinTenantID}, + {systemID: stid, ous: correctOU, commonName: roachpb.MaxTenantID.String(), expTenID: roachpb.MaxTenantID}, + {systemID: stid, ous: correctOU, commonName: roachpb.SystemTenantID.String() /* "system" */, expErr: `could not parse tenant ID from Common Name \(CN\)`}, + {systemID: stid, ous: correctOU, commonName: "-1", expErr: `could not parse tenant ID from Common Name \(CN\)`}, + {systemID: stid, ous: correctOU, commonName: "0", expErr: `invalid tenant ID 0 in Common Name \(CN\)`}, + {systemID: stid, ous: correctOU, commonName: "1", expErr: `invalid tenant ID 1 in Common Name \(CN\)`}, + {systemID: stid, ous: correctOU, commonName: "root", expErr: `could not parse tenant ID from Common Name \(CN\)`}, + {systemID: stid, ous: correctOU, commonName: "other", expErr: `could not parse tenant ID from Common Name \(CN\)`}, + {systemID: stid, ous: []string{"foo"}, commonName: "other", expErr: `client certificate CN=other,OU=foo cannot be used to perform RPC on tenant {1}`}, + {systemID: stid, ous: nil, commonName: "other", expErr: `client certificate CN=other cannot be used to perform RPC on tenant {1}`}, + {systemID: stid, ous: append([]string{"foo"}, correctOU...), commonName: "other", expErr: `could not parse tenant ID from Common Name`}, + {systemID: stid, ous: nil, commonName: "root"}, + {systemID: stid, ous: nil, commonName: "root", tenantScope: 10, expErr: "client certificate CN=root cannot be used to perform RPC on tenant {1}"}, + {systemID: tenTen, ous: correctOU, commonName: "10", expTenID: roachpb.TenantID{}}, + {systemID: tenTen, ous: correctOU, commonName: "123", expErr: `this tenant \(10\) cannot serve requests from a server for tenant 123`}, + {systemID: tenTen, ous: correctOU, commonName: "1", expErr: `invalid tenant ID 1 in Common Name \(CN\)`}, } { - t.Run(tc.commonName, func(t *testing.T) { + t.Run(fmt.Sprintf("from %v to %v", tc.commonName, tc.systemID), func(t *testing.T) { cert := &x509.Certificate{ Subject: pkix.Name{ CommonName: tc.commonName, @@ -123,7 +130,9 @@ func TestTenantFromCert(t *testing.T) { }, } if tc.tenantScope > 0 { - tenantSANs, err := security.MakeTenantURISANs(username.MakeSQLUsernameFromPreNormalizedString(tc.commonName), []roachpb.TenantID{roachpb.MustMakeTenantID(tc.tenantScope)}) + tenantSANs, err := security.MakeTenantURISANs( + username.MakeSQLUsernameFromPreNormalizedString(tc.commonName), + []roachpb.TenantID{roachpb.MustMakeTenantID(tc.tenantScope)}) require.NoError(t, err) cert.URIs = append(cert.URIs, tenantSANs...) } @@ -135,7 +144,7 @@ func TestTenantFromCert(t *testing.T) { p := peer.Peer{AuthInfo: tlsInfo} ctx := peer.NewContext(context.Background(), &p) - tenID, err := rpc.TestingAuthenticateTenant(ctx) + tenID, err := rpc.TestingAuthenticateTenant(ctx, tc.systemID) if tc.expErr == "" { require.Equal(t, tc.expTenID, tenID) diff --git a/pkg/rpc/helpers_test.go b/pkg/rpc/helpers_test.go index 353f9886c095..39bb2dfb2753 100644 --- a/pkg/rpc/helpers_test.go +++ b/pkg/rpc/helpers_test.go @@ -36,8 +36,10 @@ func TestingNewWrappedServerStream( // TestingAuthenticateTenant performs authentication of a tenant from a context // for testing. -func TestingAuthenticateTenant(ctx context.Context) (roachpb.TenantID, error) { - return kvAuth{tenant: tenantAuthorizer{tenantID: roachpb.SystemTenantID}}.authenticate(ctx) +func TestingAuthenticateTenant( + ctx context.Context, serverTenantID roachpb.TenantID, +) (roachpb.TenantID, error) { + return kvAuth{tenant: tenantAuthorizer{tenantID: serverTenantID}}.authenticate(ctx) } // TestingAuthorizeTenantRequest performs authorization of a tenant request From d5770bb7b649713a8686ec7500cb542b315b04e7 Mon Sep 17 00:00:00 2001 From: Eric Harmeling Date: Wed, 25 Jan 2023 17:13:45 -0500 Subject: [PATCH 2/2] ui: move jobs details to keyed reducer, simplify polling This commit moves the jobs details to a keyed reducer and removes the invalidating receivedJobSaga, in favor of the component-level refresh interval. Release note: None --- .../workspaces/cluster-ui/src/api/jobsApi.ts | 8 ++ .../src/jobs/jobDetailsPage/jobDetails.tsx | 6 +- .../jobDetailsPage/jobDetailsConnected.tsx | 18 ++-- .../src/store/jobDetails/job.reducer.ts | 56 ++++++++---- .../src/store/jobDetails/job.sagas.spec.ts | 90 ++++++++++--------- .../src/store/jobDetails/job.sagas.ts | 38 ++++---- .../src/store/jobDetails/job.selectors.ts | 52 ++++++++++- .../cluster-ui/src/store/reducers.ts | 4 +- .../db-console/src/views/jobs/jobDetails.tsx | 57 ++++++++++-- 9 files changed, 224 insertions(+), 105 deletions(-) diff --git a/pkg/ui/workspaces/cluster-ui/src/api/jobsApi.ts b/pkg/ui/workspaces/cluster-ui/src/api/jobsApi.ts index ac4c820642de..0049eddbece1 100644 --- a/pkg/ui/workspaces/cluster-ui/src/api/jobsApi.ts +++ b/pkg/ui/workspaces/cluster-ui/src/api/jobsApi.ts @@ -19,6 +19,14 @@ export type JobsResponse = cockroach.server.serverpb.JobsResponse; export type JobRequest = cockroach.server.serverpb.JobRequest; export type JobResponse = cockroach.server.serverpb.JobResponse; +export type JobResponseWithKey = { + jobResponse: JobResponse; + key: string; +}; +export type ErrorWithKey = { + err: Error; + key: string; +}; export const getJobs = ( req: JobsRequest, diff --git a/pkg/ui/workspaces/cluster-ui/src/jobs/jobDetailsPage/jobDetails.tsx b/pkg/ui/workspaces/cluster-ui/src/jobs/jobDetailsPage/jobDetails.tsx index 636c2bf29b74..9d827502dabc 100644 --- a/pkg/ui/workspaces/cluster-ui/src/jobs/jobDetailsPage/jobDetails.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/jobs/jobDetailsPage/jobDetails.tsx @@ -76,7 +76,9 @@ export class JobDetails extends React.Component { } componentDidMount(): void { - this.refresh(); + if (!this.props.job) { + this.refresh(); + } // Refresh every 10s. this.refreshDataInterval = setInterval(() => this.refresh(), 10 * 1000); } @@ -154,7 +156,7 @@ export class JobDetails extends React.Component { render(): React.ReactElement { const isLoading = !this.props.job || this.props.jobLoading; - const error = this.props.job && this.props.jobError; + const error = this.props.jobError; return (
diff --git a/pkg/ui/workspaces/cluster-ui/src/jobs/jobDetailsPage/jobDetailsConnected.tsx b/pkg/ui/workspaces/cluster-ui/src/jobs/jobDetailsPage/jobDetailsConnected.tsx index 05d540cc0fd2..8797d10f880c 100644 --- a/pkg/ui/workspaces/cluster-ui/src/jobs/jobDetailsPage/jobDetailsConnected.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/jobs/jobDetailsPage/jobDetailsConnected.tsx @@ -12,7 +12,11 @@ import { connect } from "react-redux"; import { RouteComponentProps, withRouter } from "react-router-dom"; import { AppState } from "src/store"; -import { selectJobState } from "../../store/jobDetails/job.selectors"; +import { + selectJob, + selectJobLoading, + selectJobError, +} from "../../store/jobDetails/job.selectors"; import { JobDetailsStateProps, JobDetailsDispatchProps, @@ -21,11 +25,13 @@ import { import { JobRequest } from "src/api/jobsApi"; import { actions as jobActions } from "src/store/jobDetails"; -const mapStateToProps = (state: AppState): JobDetailsStateProps => { - const jobState = selectJobState(state); - const job = jobState ? jobState.data : null; - const jobLoading = jobState ? jobState.inFlight : false; - const jobError = jobState ? jobState.lastError : null; +const mapStateToProps = ( + state: AppState, + props: RouteComponentProps, +): JobDetailsStateProps => { + const job = selectJob(state, props); + const jobLoading = selectJobLoading(state, props); + const jobError = selectJobError(state, props); return { job, jobLoading, diff --git a/pkg/ui/workspaces/cluster-ui/src/store/jobDetails/job.reducer.ts b/pkg/ui/workspaces/cluster-ui/src/store/jobDetails/job.reducer.ts index 1269021c7896..9ba12b895d9b 100644 --- a/pkg/ui/workspaces/cluster-ui/src/store/jobDetails/job.reducer.ts +++ b/pkg/ui/workspaces/cluster-ui/src/store/jobDetails/job.reducer.ts @@ -9,7 +9,12 @@ // licenses/APL.txt. import { createSlice, PayloadAction } from "@reduxjs/toolkit"; -import { JobRequest, JobResponse } from "src/api/jobsApi"; +import { + ErrorWithKey, + JobRequest, + JobResponse, + JobResponseWithKey, +} from "src/api/jobsApi"; import { DOMAIN_NAME } from "../utils"; export type JobState = { @@ -19,33 +24,46 @@ export type JobState = { inFlight: boolean; }; -const initialState: JobState = { - data: null, - lastError: null, - valid: true, - inFlight: false, +export type JobDetailsReducerState = { + cachedData: { + [id: string]: JobState; + }; +}; + +const initialState: JobDetailsReducerState = { + cachedData: {}, }; const JobSlice = createSlice({ name: `${DOMAIN_NAME}/job`, initialState, reducers: { - received: (state, action: PayloadAction) => { - state.data = action.payload; - state.valid = true; - state.lastError = null; - state.inFlight = false; + received: (state, action: PayloadAction) => { + state.cachedData[action.payload.key] = { + data: action.payload.jobResponse, + valid: true, + lastError: null, + inFlight: false, + }; }, - failed: (state, action: PayloadAction) => { - state.valid = false; - state.lastError = action.payload; + failed: (state, action: PayloadAction) => { + state.cachedData[action.payload.key] = { + data: null, + valid: false, + lastError: action.payload.err, + inFlight: false, + }; }, - invalidated: state => { - state.valid = false; + invalidated: (state, action: PayloadAction<{ key: string }>) => { + state.cachedData[action.payload.key] = { + data: null, + valid: false, + lastError: null, + inFlight: false, + }; }, - refresh: (_, action: PayloadAction) => {}, - request: (_, action: PayloadAction) => {}, - reset: (_, action: PayloadAction) => {}, + refresh: (_, _action: PayloadAction) => {}, + request: (_, _action: PayloadAction) => {}, }, }); diff --git a/pkg/ui/workspaces/cluster-ui/src/store/jobDetails/job.sagas.spec.ts b/pkg/ui/workspaces/cluster-ui/src/store/jobDetails/job.sagas.spec.ts index 2f25742347d6..e058c9984245 100644 --- a/pkg/ui/workspaces/cluster-ui/src/store/jobDetails/job.sagas.spec.ts +++ b/pkg/ui/workspaces/cluster-ui/src/store/jobDetails/job.sagas.spec.ts @@ -16,28 +16,46 @@ import { } from "redux-saga-test-plan/providers"; import * as matchers from "redux-saga-test-plan/matchers"; import { cockroach } from "@cockroachlabs/crdb-protobuf-client"; - -import { getJob } from "src/api/jobsApi"; -import { refreshJobSaga, requestJobSaga, receivedJobSaga } from "./job.sagas"; -import { actions, reducer, JobState } from "./job.reducer"; +import { + ErrorWithKey, + getJob, + JobRequest, + JobResponseWithKey, +} from "src/api/jobsApi"; +import { refreshJobSaga, requestJobSaga } from "./job.sagas"; +import { actions, reducer, JobDetailsReducerState } from "./job.reducer"; import { succeededJobFixture } from "../../jobs/jobsPage/jobsPage.fixture"; import Long from "long"; +import { PayloadAction } from "@reduxjs/toolkit"; describe("job sagas", () => { const payload = new cockroach.server.serverpb.JobRequest({ job_id: new Long(8136728577, 70289336), }); + + const jobID = payload.job_id.toString(); + const jobResponse = new cockroach.server.serverpb.JobResponse( succeededJobFixture, ); + const jobResponseWithKey: JobResponseWithKey = { + key: jobID, + jobResponse: jobResponse, + }; + const jobAPIProvider: (EffectProviders | StaticProvider)[] = [ [matchers.call.fn(getJob), jobResponse], ]; + const action: PayloadAction = { + payload: payload, + type: "request", + }; + describe("refreshJobSaga", () => { it("dispatches refresh job action", () => { - return expectSaga(refreshJobSaga, actions.request(payload)) + return expectSaga(refreshJobSaga, action) .provide(jobAPIProvider) .put(actions.request(payload)) .run(); @@ -46,54 +64,44 @@ describe("job sagas", () => { describe("requestJobSaga", () => { it("successfully requests job", () => { - return expectSaga(requestJobSaga, actions.request(payload)) + return expectSaga(requestJobSaga, action) .provide(jobAPIProvider) - .put(actions.received(jobResponse)) + .put(actions.received(jobResponseWithKey)) .withReducer(reducer) - .hasFinalState({ - data: jobResponse, - lastError: null, - valid: true, - inFlight: false, + .hasFinalState({ + cachedData: { + [jobID]: { + data: jobResponseWithKey.jobResponse, + lastError: null, + valid: true, + inFlight: false, + }, + }, }) .run(); }); it("returns error on failed request", () => { const error = new Error("Failed request"); - return expectSaga(requestJobSaga, actions.request(payload)) + const errorWithKey: ErrorWithKey = { + key: jobID, + err: error, + }; + return expectSaga(requestJobSaga, action) .provide([[matchers.call.fn(getJob), throwError(error)]]) - .put(actions.failed(error)) + .put(actions.failed(errorWithKey)) .withReducer(reducer) - .hasFinalState({ - data: null, - lastError: error, - valid: false, - inFlight: false, + .hasFinalState({ + cachedData: { + [jobID]: { + data: null, + lastError: error, + valid: false, + inFlight: false, + }, + }, }) .run(); }); }); - - describe("receivedJobSaga", () => { - it("sets valid status to false after specified period of time", () => { - const timeout = 500; - return expectSaga(receivedJobSaga, timeout) - .delay(timeout) - .put(actions.invalidated()) - .withReducer(reducer, { - data: jobResponse, - lastError: null, - valid: true, - inFlight: false, - }) - .hasFinalState({ - data: jobResponse, - lastError: null, - valid: false, - inFlight: false, - }) - .run(1000); - }); - }); }); diff --git a/pkg/ui/workspaces/cluster-ui/src/store/jobDetails/job.sagas.ts b/pkg/ui/workspaces/cluster-ui/src/store/jobDetails/job.sagas.ts index e88890544899..edac1fead7a1 100644 --- a/pkg/ui/workspaces/cluster-ui/src/store/jobDetails/job.sagas.ts +++ b/pkg/ui/workspaces/cluster-ui/src/store/jobDetails/job.sagas.ts @@ -8,43 +8,37 @@ // by the Apache License, Version 2.0, included in the file // licenses/APL.txt. -import { all, call, delay, put, takeLatest } from "redux-saga/effects"; +import { all, call, put, takeLatest } from "redux-saga/effects"; import { actions } from "./job.reducer"; -import { getJob, JobRequest } from "src/api/jobsApi"; -import { CACHE_INVALIDATION_PERIOD, throttleWithReset } from "../utils"; -import { rootActions } from "../reducers"; +import { getJob, JobRequest, JobResponseWithKey } from "src/api/jobsApi"; import { PayloadAction } from "@reduxjs/toolkit"; +import { ErrorWithKey } from "../../api"; export function* refreshJobSaga(action: PayloadAction) { yield put(actions.request(action.payload)); } export function* requestJobSaga(action: PayloadAction): any { + const key = action.payload.job_id.toString(); try { const result = yield call(getJob, action.payload); - yield put(actions.received(result)); + const resultWithKey: JobResponseWithKey = { + key: key, + jobResponse: result, + }; + yield put(actions.received(resultWithKey)); } catch (e) { - yield put(actions.failed(e)); + const err: ErrorWithKey = { + err: e, + key, + }; + yield put(actions.failed(err)); } } - -export function* receivedJobSaga(delayMs: number) { - yield delay(delayMs); - yield put(actions.invalidated()); -} - -export function* jobSaga( - cacheInvalidationPeriod: number = CACHE_INVALIDATION_PERIOD, -) { +export function* jobSaga() { yield all([ - throttleWithReset( - cacheInvalidationPeriod, - actions.refresh, - [actions.invalidated, rootActions.resetState], - refreshJobSaga, - ), + takeLatest(actions.refresh, refreshJobSaga), takeLatest(actions.request, requestJobSaga), - takeLatest(actions.received, receivedJobSaga, cacheInvalidationPeriod), ]); } diff --git a/pkg/ui/workspaces/cluster-ui/src/store/jobDetails/job.selectors.ts b/pkg/ui/workspaces/cluster-ui/src/store/jobDetails/job.selectors.ts index f4f92b61ae50..ee1451f25167 100644 --- a/pkg/ui/workspaces/cluster-ui/src/store/jobDetails/job.selectors.ts +++ b/pkg/ui/workspaces/cluster-ui/src/store/jobDetails/job.selectors.ts @@ -9,9 +9,53 @@ // licenses/APL.txt. import { createSelector } from "reselect"; -import { adminUISelector } from "../utils/selectors"; +import { adminUISelector } from "src/store/utils/selectors"; +import { selectID } from "src/selectors"; +import { JobResponse } from "src/api/jobsApi"; -export const selectJobState = createSelector( - adminUISelector, - adminUiState => adminUiState.job, +const selectJobState = createSelector(adminUISelector, state => { + const jobState = state?.job?.cachedData; + const emptyJobCache = !jobState || Object.keys(jobState).length === 0; + if (emptyJobCache) { + return null; + } + return jobState; +}); + +export const selectJob = createSelector( + [adminUISelector, selectJobState, selectID], + (adminUIState, jobState, jobID) => { + const jobsCache = adminUIState?.jobs?.data; + let job: JobResponse; + if (!jobID || (!jobsCache && !jobState)) { + return null; + } else if (jobsCache) { + job = Object(jobsCache.jobs.find(job => job.id.toString() === jobID)); + } else if (jobState) { + job = jobState[jobID]?.data; + } + return job; + }, +); + +export const selectJobError = createSelector( + selectJobState, + selectID, + (state, jobID) => { + if (!state || !jobID) { + return null; + } + return state[jobID]?.lastError; + }, +); + +export const selectJobLoading = createSelector( + selectJobState, + selectID, + (state, jobID) => { + if (!state || !jobID) { + return null; + } + return state[jobID]?.inFlight; + }, ); diff --git a/pkg/ui/workspaces/cluster-ui/src/store/reducers.ts b/pkg/ui/workspaces/cluster-ui/src/store/reducers.ts index 743a152387cb..72cee45364c3 100644 --- a/pkg/ui/workspaces/cluster-ui/src/store/reducers.ts +++ b/pkg/ui/workspaces/cluster-ui/src/store/reducers.ts @@ -34,7 +34,7 @@ import { TxnInsightsState, reducer as txnInsights, } from "./insights/transactionInsights"; -import { JobState, reducer as job } from "./jobDetails"; +import { JobDetailsReducerState, reducer as job } from "./jobDetails"; import { JobsState, reducer as jobs } from "./jobs"; import { LivenessState, reducer as liveness } from "./liveness"; import { LocalStorageState, reducer as localStorage } from "./localStorage"; @@ -72,7 +72,7 @@ export type AdminUiState = { sqlDetailsStats: SQLDetailsStatsReducerState; indexStats: IndexStatsReducerState; jobs: JobsState; - job: JobState; + job: JobDetailsReducerState; clusterLocks: ClusterLocksReqState; databasesList: DatabasesListState; stmtInsights: StmtInsightsState; diff --git a/pkg/ui/workspaces/db-console/src/views/jobs/jobDetails.tsx b/pkg/ui/workspaces/db-console/src/views/jobs/jobDetails.tsx index f7e5e287d7ce..8600db857311 100644 --- a/pkg/ui/workspaces/db-console/src/views/jobs/jobDetails.tsx +++ b/pkg/ui/workspaces/db-console/src/views/jobs/jobDetails.tsx @@ -15,17 +15,57 @@ import { import { connect } from "react-redux"; import { RouteComponentProps, withRouter } from "react-router-dom"; import { createSelector } from "reselect"; -import { CachedDataReducerState, refreshJob } from "src/redux/apiReducers"; +import { KeyedCachedDataReducerState, refreshJob } from "src/redux/apiReducers"; import { AdminUIState } from "src/redux/state"; import { JobResponseMessage } from "src/util/api"; const selectJobState = createSelector( - [(state: AdminUIState) => state.cachedData.job, selectID], - (job, jobID): CachedDataReducerState => { - if (!job) { + [(state: AdminUIState) => state.cachedData?.job], + (jobState): KeyedCachedDataReducerState => { + if (!jobState) { return null; } - return job[jobID]; + return jobState; + }, +); + +export const selectJob = createSelector( + [(state: AdminUIState) => state.cachedData?.jobs, selectJobState, selectID], + (jobsState, jobState, jobID) => { + const jobsCache = jobsState.data; + let job: JobResponseMessage; + if (!jobID || (!jobsCache && !jobState)) { + return null; + } else if (jobsCache) { + job = Object( + jobsCache.data.jobs.find(job => job.id.toString() === jobID), + ); + } else if (jobState) { + job = jobState[jobID]?.data; + } + return job; + }, +); + +export const selectJobError = createSelector( + selectJobState, + selectID, + (state: KeyedCachedDataReducerState, jobID) => { + if (!state || !jobID) { + return null; + } + return state[jobID]?.lastError; + }, +); + +export const selectJobLoading = createSelector( + selectJobState, + selectID, + (state: KeyedCachedDataReducerState, jobID) => { + if (!state || !jobID) { + return null; + } + return state[jobID]?.inFlight; }, ); @@ -33,10 +73,9 @@ const mapStateToProps = ( state: AdminUIState, props: RouteComponentProps, ): JobDetailsStateProps => { - const jobState = selectJobState(state, props); - const job = jobState ? jobState.data : null; - const jobLoading = jobState ? jobState.inFlight : false; - const jobError = jobState ? jobState.lastError : null; + const job = selectJob(state, props); + const jobLoading = selectJobLoading(state, props); + const jobError = selectJobError(state, props); return { job, jobLoading,