From df7f72e84ce6192778699b5bf5732213968efc42 Mon Sep 17 00:00:00 2001 From: Sebastian Malton Date: Fri, 19 Feb 2021 10:02:53 -0500 Subject: [PATCH] Fix NamespaceSelectFilter to correctly track accessibleNamespaces - Consolidate the ClusterContext functions into a class - Add method for the correct determiation of whether all possible namespaces are selected to ClusterContext - Use said method in the two API related use cases as well in the above component Fix only able to select one namespace - Extends isAllPossibleNamespaces with isFilterSelect option to correctly check the empty namespaces imply selecting all case - renamed contextNamespaces to selectedNamespaces - renamed contextNs to rawSelectedNamespaces fix not restarting subscribes when adding new accessibleNamespaces Signed-off-by: Sebastian Malton --- src/common/utils/as-tuple.ts | 8 ++++ src/common/utils/index.ts | 2 + src/renderer/api/kube-watch-api.ts | 12 ++++- .../+apps-releases/release.store.ts | 18 ++++--- .../+namespaces/namespace-select-filter.tsx | 7 ++- .../+namespaces/namespace-select.tsx | 44 +++++++++++------ .../components/+namespaces/namespace.store.ts | 48 ++++++++++++------- .../+workloads-overview/overview-statuses.tsx | 2 +- .../+workloads-overview/overview.tsx | 7 ++- src/renderer/components/app.tsx | 11 +++-- src/renderer/components/context.ts | 38 +++++++++------ .../item-object-list/item-list-layout.tsx | 2 +- .../kube-object/kube-object-list-layout.tsx | 3 +- src/renderer/kube-object.store.ts | 8 +--- 14 files changed, 141 insertions(+), 69 deletions(-) create mode 100644 src/common/utils/as-tuple.ts diff --git a/src/common/utils/as-tuple.ts b/src/common/utils/as-tuple.ts new file mode 100644 index 0000000000..4bb0d5ca9c --- /dev/null +++ b/src/common/utils/as-tuple.ts @@ -0,0 +1,8 @@ +/** + * This function changes the TS type from an array literal over the union of + * element types to a strict tuple. + * @param arg The array literal to be made into a tuple + */ +export function asTuple(arg: T): T { + return arg; +} diff --git a/src/common/utils/index.ts b/src/common/utils/index.ts index edb2b44d96..2b412392c8 100644 --- a/src/common/utils/index.ts +++ b/src/common/utils/index.ts @@ -3,6 +3,7 @@ export const noop: any = () => { /* empty */ }; export * from "./app-version"; +export * from "./as-tuple"; export * from "./autobind"; export * from "./base64"; export * from "./camelCase"; @@ -11,6 +12,7 @@ export * from "./debouncePromise"; export * from "./defineGlobal"; export * from "./delay"; export * from "./disposer"; +export * from "./disposer"; export * from "./downloadFile"; export * from "./escapeRegExp"; export * from "./getRandId"; diff --git a/src/renderer/api/kube-watch-api.ts b/src/renderer/api/kube-watch-api.ts index f2f50193db..c23ce9c85e 100644 --- a/src/renderer/api/kube-watch-api.ts +++ b/src/renderer/api/kube-watch-api.ts @@ -6,7 +6,7 @@ import type { ClusterContext } from "../components/context"; import plimit from "p-limit"; import { comparer, IReactionDisposer, observable, reaction, when } from "mobx"; -import { autobind, noop } from "../utils"; +import { asTuple, autobind, noop } from "../utils"; import { KubeApi } from "./kube-api"; import { KubeJsonApiData } from "./kube-json-api"; import { isDebugging, isProduction } from "../../common/vars"; @@ -88,7 +88,15 @@ export class KubeWatchApi { } // reload stores only for context namespaces change - cancelReloading = reaction(() => this.context?.contextNamespaces, namespaces => { + cancelReloading = reaction(() => { + const namespaces = this.context?.selectedNamespaces; + + /** + * react to the changing of "allPossibleNamespaces" so that adding + * accessibleNamespaces means that this is restarted + */ + return asTuple([this.context?.isAllPossibleNamespaces(namespaces), namespaces]); + }, ([, namespaces]) => { preloading?.cancelLoading(); unsubscribeList.forEach(unsubscribe => unsubscribe()); unsubscribeList.length = 0; diff --git a/src/renderer/components/+apps-releases/release.store.ts b/src/renderer/components/+apps-releases/release.store.ts index faeb509b71..0672c18172 100644 --- a/src/renderer/components/+apps-releases/release.store.ts +++ b/src/renderer/components/+apps-releases/release.store.ts @@ -7,11 +7,19 @@ import { Secret } from "../../api/endpoints"; import { secretsStore } from "../+config-secrets/secrets.store"; import { namespaceStore } from "../+namespaces/namespace.store"; import { Notifications } from "../notifications"; +import { ClusterContext } from "../context"; @autobind() export class ReleaseStore extends ItemStore { + @observable static defaultContext: ClusterContext; // TODO: support multiple cluster contexts releaseSecrets = observable.map(); + contextReady = when(() => Boolean(this.context)); + + get context(): ClusterContext { + return ReleaseStore.defaultContext; + } + constructor() { super(); when(() => secretsStore.isLoaded, () => { @@ -36,7 +44,7 @@ export class ReleaseStore extends ItemStore { } watchSelecteNamespaces(): (() => void) { - return reaction(() => namespaceStore.context.contextNamespaces, namespaces => { + return reaction(() => namespaceStore.selectedNamespaces, namespaces => { this.loadAll(namespaces); }); } @@ -79,15 +87,13 @@ export class ReleaseStore extends ItemStore { } async loadFromContextNamespaces(): Promise { - return this.loadAll(namespaceStore.context.contextNamespaces); + return this.loadAll(namespaceStore.selectedNamespaces); } async loadItems(namespaces: string[]) { - const isLoadingAll = namespaceStore.context.allNamespaces?.length > 1 - && namespaceStore.context.cluster.accessibleNamespaces.length === 0 - && namespaceStore.context.allNamespaces.every(ns => namespaces.includes(ns)); + await this.contextReady; - if (isLoadingAll) { + if (this.context.isAllPossibleNamespaces(namespaces)) { return listReleases(); } diff --git a/src/renderer/components/+namespaces/namespace-select-filter.tsx b/src/renderer/components/+namespaces/namespace-select-filter.tsx index 85a6299b17..66adc71a24 100644 --- a/src/renderer/components/+namespaces/namespace-select-filter.tsx +++ b/src/renderer/components/+namespaces/namespace-select-filter.tsx @@ -13,11 +13,14 @@ import { namespaceStore } from "./namespace.store"; const Placeholder = observer((props: PlaceholderProps) => { const getPlaceholder = (): React.ReactNode => { - const namespaces = namespaceStore.contextNamespaces; + const namespaces = namespaceStore.selectedNamespaces; + + if (namespaceStore.selectedAll) { + return <>All namespaces; + } switch (namespaces.length) { case 0: - case namespaceStore.allowedNamespaces.length: return <>All namespaces; case 1: return <>Namespace: {namespaces[0]}; diff --git a/src/renderer/components/+namespaces/namespace-select.tsx b/src/renderer/components/+namespaces/namespace-select.tsx index d398f91f0a..eab5a52737 100644 --- a/src/renderer/components/+namespaces/namespace-select.tsx +++ b/src/renderer/components/+namespaces/namespace-select.tsx @@ -11,17 +11,32 @@ import { kubeWatchApi } from "../../api/kube-watch-api"; import { components, ValueContainerProps } from "react-select"; interface Props extends SelectProps { + /** + * Show icons preceeding the entry names + * @default true + */ showIcons?: boolean; - showClusterOption?: boolean; // show "Cluster" option on the top (default: false) - showAllNamespacesOption?: boolean; // show "All namespaces" option on the top (default: false) + + /** + * show a "Cluster" option above all namespaces + * @default false + */ + showClusterOption?: boolean; + + /** + * show "All namespaces" option on the top (has precedence over `showClusterOption`) + * @default false + */ + showAllNamespacesOption?: boolean; + + /** + * A function to change the options for the select + * @param options the current options to display + * @default passthrough + */ customizeOptions?(options: SelectOption[]): SelectOption[]; } -const defaultProps: Partial = { - showIcons: true, - showClusterOption: false, -}; - function GradientValueContainer({children, ...rest}: ValueContainerProps) { return ( @@ -34,7 +49,12 @@ function GradientValueContainer({children, ...rest}: ValueContainerProps) @observer export class NamespaceSelect extends React.Component { - static defaultProps = defaultProps as object; + static defaultProps: Props = { + showIcons: true, + showClusterOption: false, + showAllNamespacesOption: false, + customizeOptions: (opts) => opts, + }; componentDidMount() { disposeOnUnmount(this, [ @@ -47,7 +67,7 @@ export class NamespaceSelect extends React.Component { @computed.struct get options(): SelectOption[] { const { customizeOptions, showClusterOption, showAllNamespacesOption } = this.props; - let options: SelectOption[] = namespaceStore.items.map(ns => ({ value: ns.getName() })); + const options: SelectOption[] = namespaceStore.allowedNamespaces.map(ns => ({ value: ns })); if (showAllNamespacesOption) { options.unshift({ label: "All Namespaces", value: "" }); @@ -55,11 +75,7 @@ export class NamespaceSelect extends React.Component { options.unshift({ label: "Cluster", value: "" }); } - if (customizeOptions) { - options = customizeOptions(options); - } - - return options; + return customizeOptions(options); } formatOptionLabel = (option: SelectOption) => { diff --git a/src/renderer/components/+namespaces/namespace.store.ts b/src/renderer/components/+namespaces/namespace.store.ts index 4415f416b9..bbec9ba1ca 100644 --- a/src/renderer/components/+namespaces/namespace.store.ts +++ b/src/renderer/components/+namespaces/namespace.store.ts @@ -31,7 +31,14 @@ export function getDummyNamespace(name: string) { export class NamespaceStore extends KubeObjectStore { api = namespacesApi; - @observable private contextNs = observable.set(); + @observable private rawSelectedNamespaces = observable.set(); + + /** + * @depreated use `NamespaceStore.rawSelectedNamespaces` instead + */ + get contextNs() { + return this.rawSelectedNamespaces; + } constructor() { super(); @@ -48,7 +55,7 @@ export class NamespaceStore extends KubeObjectStore { } public onContextChange(callback: (contextNamespaces: string[]) => void, opts: IReactionOptions = {}): IReactionDisposer { - return reaction(() => Array.from(this.contextNs), callback, { + return reaction(() => Array.from(this.rawSelectedNamespaces), callback, { equals: comparer.shallow, ...opts, }); @@ -89,6 +96,16 @@ export class NamespaceStore extends KubeObjectStore { return []; } + /** + * @deprecated use `NamespaceStore.allowedNamespaces` instead + */ + get contextNamespaces() { + return this.allowedNamespaces; + } + + /** + * The array of namespace names that this store knows about + */ @computed get allowedNamespaces(): string[] { return Array.from(new Set([ ...(this.context?.allNamespaces ?? []), // allowed namespaces from cluster (main), updating every 30s @@ -96,8 +113,8 @@ export class NamespaceStore extends KubeObjectStore { ].flat())); } - @computed get contextNamespaces(): string[] { - const namespaces = Array.from(this.contextNs); + @computed get selectedNamespaces(): string[] { + const namespaces = Array.from(this.rawSelectedNamespaces); if (!namespaces.length) { return this.allowedNamespaces; // show all namespaces when nothing selected @@ -133,28 +150,27 @@ export class NamespaceStore extends KubeObjectStore { setContext(namespace: string | string[]) { const namespaces = [namespace].flat(); - this.contextNs.replace(namespaces); - } - - @action - resetContext() { - this.contextNs.clear(); + this.rawSelectedNamespaces.replace(namespaces); } hasContext(namespaces: string | string[]) { - return [namespaces].flat().every(namespace => this.contextNs.has(namespace)); + return [namespaces].flat().every(namespace => this.rawSelectedNamespaces.has(namespace)); + } + + @computed get selectedAll(): boolean { + return this.context?.isAllPossibleNamespaces(Array.from(this.rawSelectedNamespaces), true) ?? false; } @computed get hasAllContexts(): boolean { - return this.contextNs.size === this.allowedNamespaces.length; + return this.rawSelectedNamespaces.size === this.allowedNamespaces.length; } @action toggleContext(namespace: string) { if (this.hasContext(namespace)) { - this.contextNs.delete(namespace); + this.rawSelectedNamespaces.delete(namespace); } else { - this.contextNs.add(namespace); + this.rawSelectedNamespaces.add(namespace); } } @@ -164,7 +180,7 @@ export class NamespaceStore extends KubeObjectStore { if (showAll) { this.setContext(this.allowedNamespaces); } else { - this.resetContext(); // empty context considered as "All namespaces" + this.rawSelectedNamespaces.clear(); } } else { this.toggleAll(!this.hasAllContexts); @@ -174,7 +190,7 @@ export class NamespaceStore extends KubeObjectStore { @action async remove(item: Namespace) { await super.remove(item); - this.contextNs.delete(item.getName()); + this.rawSelectedNamespaces.delete(item.getName()); } } diff --git a/src/renderer/components/+workloads-overview/overview-statuses.tsx b/src/renderer/components/+workloads-overview/overview-statuses.tsx index bc0484dadc..567ca3b692 100644 --- a/src/renderer/components/+workloads-overview/overview-statuses.tsx +++ b/src/renderer/components/+workloads-overview/overview-statuses.tsx @@ -26,7 +26,7 @@ export class OverviewStatuses extends React.Component { @autobind() renderWorkload(resource: KubeResource): React.ReactElement { const store = workloadStores[resource]; - const items = store.getAllByNs(namespaceStore.contextNamespaces); + const items = store.getAllByNs(namespaceStore.selectedNamespaces); return (
diff --git a/src/renderer/components/+workloads-overview/overview.tsx b/src/renderer/components/+workloads-overview/overview.tsx index 92bc569307..4dac7a17e0 100644 --- a/src/renderer/components/+workloads-overview/overview.tsx +++ b/src/renderer/components/+workloads-overview/overview.tsx @@ -16,13 +16,16 @@ import { cronJobStore } from "../+workloads-cronjobs/cronjob.store"; import { Events } from "../+events"; import { isAllowedResource } from "../../../common/rbac"; import { kubeWatchApi } from "../../api/kube-watch-api"; -import { clusterContext } from "../context"; +import { observable } from "mobx"; +import { ClusterContext } from "../context"; interface Props extends RouteComponentProps { } @observer export class WorkloadsOverview extends React.Component { + @observable static defaultContext: ClusterContext; // TODO: support multiple cluster contexts + componentDidMount() { disposeOnUnmount(this, [ kubeWatchApi.subscribeStores([ @@ -30,7 +33,7 @@ export class WorkloadsOverview extends React.Component { jobStore, cronJobStore, eventStore, ], { preload: true, - namespaces: clusterContext.contextNamespaces, + namespaces: WorkloadsOverview.defaultContext.selectedNamespaces, }), ]); } diff --git a/src/renderer/components/app.tsx b/src/renderer/components/app.tsx index 41fc63cf4d..429a28c3c6 100755 --- a/src/renderer/components/app.tsx +++ b/src/renderer/components/app.tsx @@ -49,7 +49,9 @@ import { kubeWatchApi } from "../api/kube-watch-api"; import { ReplicaSetScaleDialog } from "./+workloads-replicasets/replicaset-scale-dialog"; import { CommandContainer } from "./command-palette/command-container"; import { KubeObjectStore } from "../kube-object.store"; -import { clusterContext } from "./context"; +import { ReleaseStore } from "./+apps-releases/release.store"; +import { ClusterContext } from "./context"; +import { WorkloadsOverview } from "./+workloads-overview/overview"; @observer export class App extends React.Component { @@ -78,8 +80,11 @@ export class App extends React.Component { whatInput.ask(); // Start to monitor user input device // Setup hosted cluster context - KubeObjectStore.defaultContext = clusterContext; - kubeWatchApi.context = clusterContext; + KubeObjectStore.defaultContext + = ReleaseStore.defaultContext + = WorkloadsOverview.defaultContext + = kubeWatchApi.context + = new ClusterContext(); } componentDidMount() { diff --git a/src/renderer/components/context.ts b/src/renderer/components/context.ts index 5233faba83..cb48152825 100755 --- a/src/renderer/components/context.ts +++ b/src/renderer/components/context.ts @@ -2,16 +2,10 @@ import type { Cluster } from "../../main/cluster"; import { getHostedCluster } from "../../common/cluster-store"; import { namespaceStore } from "./+namespaces/namespace.store"; -export interface ClusterContext { - cluster?: Cluster; - allNamespaces: string[]; // available / allowed namespaces from cluster.ts - contextNamespaces: string[]; // selected by user (see: namespace-select.tsx) -} - -export const clusterContext: ClusterContext = { +export class ClusterContext { get cluster(): Cluster | null { - return getHostedCluster(); - }, + return getHostedCluster() ?? null; + } get allNamespaces(): string[] { if (!this.cluster) { @@ -30,9 +24,25 @@ export const clusterContext: ClusterContext = { // fallback to cluster resolved namespaces because we could not load list return this.cluster.allowedNamespaces || []; } - }, + } - get contextNamespaces(): string[] { - return namespaceStore.contextNamespaces ?? []; - }, -}; + /** + * This function returns true if the list of namespaces provided is the + * same as all the namespaces that exist (for certain) on the cluster + * @param namespaces The list of namespaces to check + */ + public isAllPossibleNamespaces(namespaceList: string[], isFilterSelect = false): boolean { + const namespaces = new Set(namespaceList); + + return this.allNamespaces.length > 1 + && this.cluster.accessibleNamespaces.length === 0 + && ( + (isFilterSelect && namespaces.size === 0) + || this.allNamespaces.every(ns => namespaces.has(ns)) + ); + } + + get selectedNamespaces(): string[] { + return namespaceStore.selectedNamespaces ?? []; + } +} diff --git a/src/renderer/components/item-object-list/item-list-layout.tsx b/src/renderer/components/item-object-list/item-list-layout.tsx index d3424f2c0a..2389409cb6 100644 --- a/src/renderer/components/item-object-list/item-list-layout.tsx +++ b/src/renderer/components/item-object-list/item-list-layout.tsx @@ -137,7 +137,7 @@ export class ItemListLayout extends React.Component { const stores = Array.from(new Set([store, ...dependentStores])); // load context namespaces by default (see also: ``) - stores.forEach(store => store.loadAll(namespaceStore.contextNamespaces)); + stores.forEach(store => store.loadAll(namespaceStore.selectedNamespaces)); } private filterCallbacks: { [type: string]: ItemsFilter } = { diff --git a/src/renderer/components/kube-object/kube-object-list-layout.tsx b/src/renderer/components/kube-object/kube-object-list-layout.tsx index ddae968dc9..f0049ca366 100644 --- a/src/renderer/components/kube-object/kube-object-list-layout.tsx +++ b/src/renderer/components/kube-object/kube-object-list-layout.tsx @@ -8,7 +8,6 @@ import { KubeObjectStore } from "../../kube-object.store"; import { KubeObjectMenu } from "./kube-object-menu"; import { kubeSelectedUrlParam, showDetails } from "./kube-object-details"; import { kubeWatchApi } from "../../api/kube-watch-api"; -import { clusterContext } from "../context"; export interface KubeObjectListLayoutProps extends ItemListLayoutProps { store: KubeObjectStore; @@ -34,7 +33,7 @@ export class KubeObjectListLayout extends React.Component extends ItemSt } @computed get contextItems(): T[] { - const namespaces = this.context?.contextNamespaces ?? []; + const namespaces = this.context?.selectedNamespaces ?? []; return this.items.filter(item => { const itemNamespace = item.getNs(); @@ -109,11 +109,7 @@ export abstract class KubeObjectStore extends ItemSt return api.list({}, this.query); } - const isLoadingAll = this.context.allNamespaces?.length > 1 - && this.context.cluster.accessibleNamespaces.length === 0 - && this.context.allNamespaces.every(ns => namespaces.includes(ns)); - - if (isLoadingAll) { + if (this.context.isAllPossibleNamespaces(namespaces)) { this.loadedNamespaces = []; return api.list({}, this.query);