1
0
mirror of https://github.com/lensapp/lens.git synced 2025-05-20 05:10:56 +00:00

Fix namespaces not respecting accessible namespace config (#7279)

* Fix namespaces not respecting accessible namespace config

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Fix warning hover text formatting

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Log state after refresh accessibility

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Fix tests

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Fix NamespaceStore.allowedNamespaces

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Remove unnecessary '?'

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Add deprecation messages

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Move selected_namespaces storage to injectable

- And its initialization

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Implement contextNamespaces logic in forNamespacedResources

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Update snapshot

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Fix formatting

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Fix formatting

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Fix test

- This was a side effect of the previous bug fixes where
  the clusterContext.hasAllSelected was previously erroneously 'false'

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Change log level

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Fix another test suite

Signed-off-by: Sebastian Malton <sebastian@malton.name>

---------

Signed-off-by: Sebastian Malton <sebastian@malton.name>
This commit is contained in:
Sebastian Malton 2023-03-06 14:28:08 -08:00 committed by GitHub
parent aad19677db
commit a286ae4c44
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
17 changed files with 112 additions and 65 deletions

View File

@ -498,6 +498,7 @@ export class Cluster implements ClusterModel {
this.allowedResources.replace(await this.getAllowedResources(requestNamespaceListPermissions));
this.ready = this.knownResources.length > 0;
this.dependencies.logger.debug(`[CLUSTER]: refreshed accessibility data`, this.getState());
}
/**

View File

@ -4,7 +4,7 @@
*/
import { getInjectable } from "@ogre-tools/injectable";
import { computed } from "mobx";
import namespaceStoreInjectable from "../../renderer/components/+namespaces/store.injectable";
import clusterFrameContextForNamespacedResourcesInjectable from "../../renderer/cluster-frame-context/for-namespaced-resources.injectable";
import { storesAndApisCanBeCreatedInjectionToken } from "./stores-apis-can-be-created.token";
const selectedFilterNamespacesInjectable = getInjectable({
@ -15,9 +15,9 @@ const selectedFilterNamespacesInjectable = getInjectable({
return computed(() => []);
}
const store = di.inject(namespaceStoreInjectable);
const context = di.inject(clusterFrameContextForNamespacedResourcesInjectable);
return computed(() => [...store.contextNamespaces]);
return computed(() => [...context.contextNamespaces]);
},
});

View File

@ -28,6 +28,9 @@ import requestHelmChartReadmeInjectable from "../../../common/k8s-api/endpoints/
import requestHelmChartValuesInjectable from "../../../common/k8s-api/endpoints/helm-charts.api/request-values.injectable";
import type { RequestDetailedHelmRelease } from "../../../renderer/components/+helm-releases/release-details/release-details-model/request-detailed-helm-release.injectable";
import requestDetailedHelmReleaseInjectable from "../../../renderer/components/+helm-releases/release-details/release-details-model/request-detailed-helm-release.injectable";
import type { RequestHelmReleases } from "../../../common/k8s-api/endpoints/helm-releases.api/request-releases.injectable";
import requestHelmReleasesInjectable from "../../../common/k8s-api/endpoints/helm-releases.api/request-releases.injectable";
import { flushPromises } from "../../../common/test-utils/flush-promises";
describe("installing helm chart from new tab", () => {
let builder: ApplicationBuilder;
@ -37,6 +40,7 @@ describe("installing helm chart from new tab", () => {
let requestHelmChartReadmeMock: AsyncFnMock<RequestHelmChartReadme>;
let requestHelmChartValuesMock: AsyncFnMock<RequestHelmChartValues>;
let requestCreateHelmReleaseMock: AsyncFnMock<RequestCreateHelmRelease>;
let requestHelmReleasesMock: AsyncFnMock<RequestHelmReleases>;
beforeEach(() => {
builder = getApplicationBuilder();
@ -49,6 +53,7 @@ describe("installing helm chart from new tab", () => {
requestHelmChartReadmeMock = asyncFn();
requestHelmChartValuesMock = asyncFn();
requestCreateHelmReleaseMock = asyncFn();
requestHelmReleasesMock = asyncFn();
builder.beforeWindowStart((windowDi) => {
windowDi.override(directoryForLensLocalStorageInjectable, () => "/some-directory-for-lens-local-storage");
@ -58,6 +63,7 @@ describe("installing helm chart from new tab", () => {
windowDi.override(requestHelmChartReadmeInjectable, () => requestHelmChartReadmeMock);
windowDi.override(requestHelmChartValuesInjectable, () => requestHelmChartValuesMock);
windowDi.override(requestCreateHelmReleaseInjectable, () => requestCreateHelmReleaseMock);
windowDi.override(requestHelmReleasesInjectable, () => requestHelmReleasesMock);
windowDi.override(getRandomInstallChartTabIdInjectable, () =>
jest
@ -386,12 +392,15 @@ describe("installing helm chart from new tab", () => {
});
describe("when selected to see the installed release", () => {
beforeEach(() => {
beforeEach(async () => {
const releaseButton = rendered.getByTestId(
"show-release-some-release-for-some-first-tab-id",
);
fireEvent.click(releaseButton);
await flushPromises();
await requestHelmReleasesMock.resolve([]);
});
it("renders", () => {

View File

@ -78,9 +78,12 @@ describe("showing details for helm release", () => {
});
builder.namespaces.add("some-namespace");
builder.namespaces.select("some-namespace");
builder.namespaces.add("some-namespace");
builder.namespaces.select("some-other-namespace");
builder.afterWindowStart(() => {
builder.namespaces.select("some-namespace");
builder.namespaces.select("some-other-namespace");
});
});
describe("given application is started", () => {
@ -106,10 +109,9 @@ describe("showing details for helm release", () => {
});
it("calls for releases for each selected namespace", () => {
expect(requestHelmReleasesMock.mock.calls).toEqual([
["some-namespace"],
["some-other-namespace"],
]);
expect(requestHelmReleasesMock).toBeCalledTimes(2);
expect(requestHelmReleasesMock).toBeCalledWith("some-namespace");
expect(requestHelmReleasesMock).toBeCalledWith("some-other-namespace");
});
it("shows spinner", () => {

View File

@ -0,0 +1,26 @@
/**
* Copyright (c) OpenLens Authors. All rights reserved.
* Licensed under MIT License. See LICENSE in root directory for more information.
*/
import { getInjectable } from "@ogre-tools/injectable";
import assert from "assert";
import hostedClusterInjectable from "../../../renderer/cluster-frame-context/hosted-cluster.injectable";
import createStorageInjectable from "../../../renderer/utils/create-storage/create-storage.injectable";
const selectedNamespacesStorageInjectable = getInjectable({
id: "selected-namespaces-storage",
instantiate: (di) => {
const createStorage = di.inject(createStorageInjectable);
const cluster = di.inject(hostedClusterInjectable);
assert(cluster, "selectedNamespacesStorage is only available in certain environments");
const defaultSelectedNamespaces = cluster.allowedNamespaces.includes("default")
? ["default"]
: cluster.allowedNamespaces.slice(0, 1);
return createStorage("selected_namespaces", defaultSelectedNamespaces);
},
});
export default selectedNamespacesStorageInjectable;

View File

@ -8,6 +8,7 @@ import namespaceStoreInjectable from "../components/+namespaces/store.injectable
import hostedClusterInjectable from "./hosted-cluster.injectable";
import assert from "assert";
import { computed } from "mobx";
import selectedNamespacesStorageInjectable from "../../features/namespace-filtering/renderer/storage.injectable";
const clusterFrameContextForNamespacedResourcesInjectable = getInjectable({
id: "cluster-frame-context-for-namespaced-resources",
@ -15,6 +16,7 @@ const clusterFrameContextForNamespacedResourcesInjectable = getInjectable({
instantiate: (di): ClusterContext => {
const cluster = di.inject(hostedClusterInjectable);
const namespaceStore = di.inject(namespaceStoreInjectable);
const selectedNamespacesStorage = di.inject(selectedNamespacesStorageInjectable);
assert(cluster, "This can only be injected within a cluster frame");
@ -32,13 +34,19 @@ const clusterFrameContextForNamespacedResourcesInjectable = getInjectable({
// fallback to cluster resolved namespaces because we could not load list
return cluster.allowedNamespaces.slice();
});
const contextNamespaces = computed(() => namespaceStore.contextNamespaces);
const contextNamespaces = computed(() => {
const selectedNamespaces = selectedNamespacesStorage.get();
return selectedNamespaces.length > 0
? selectedNamespaces
: allNamespaces.get();
});
const hasSelectedAll = computed(() => {
const namespaces = new Set(contextNamespaces.get());
return allNamespaces.get().length > 1
&& cluster.accessibleNamespaces.length === 0
&& allNamespaces.get().every(ns => namespaces.has(ns));
&& cluster.accessibleNamespaces.length === 0
&& allNamespaces.get().every(ns => namespaces.has(ns));
});
return {

View File

@ -22,13 +22,11 @@ const releasesInjectable = getInjectable({
getValueFromObservedPromise: async () => {
void releaseSecrets.get();
const releaseArrays = await (clusterContext.hasSelectedAll
? requestHelmReleases()
: Promise.all(
clusterContext.contextNamespaces.map((namespace) =>
requestHelmReleases(namespace),
),
));
const releaseArrays = await (
clusterContext.hasSelectedAll
? requestHelmReleases()
: Promise.all(clusterContext.contextNamespaces.map((namespace) => requestHelmReleases(namespace)))
);
return releaseArrays.flat().map(toHelmRelease);
},

View File

@ -1412,7 +1412,7 @@ exports[`<NamespaceSelectFilter /> once the subscribe resolves when clicked when
</span>
</i>
<span>
test-10
test-2
</span>
</div>
</div>
@ -1436,7 +1436,7 @@ exports[`<NamespaceSelectFilter /> once the subscribe resolves when clicked when
</span>
</i>
<span>
test-11
test-10
</span>
</div>
</div>
@ -1460,7 +1460,7 @@ exports[`<NamespaceSelectFilter /> once the subscribe resolves when clicked when
</span>
</i>
<span>
test-12
test-11
</span>
</div>
</div>
@ -1484,7 +1484,7 @@ exports[`<NamespaceSelectFilter /> once the subscribe resolves when clicked when
</span>
</i>
<span>
test-13
test-12
</span>
</div>
</div>
@ -1508,7 +1508,7 @@ exports[`<NamespaceSelectFilter /> once the subscribe resolves when clicked when
</span>
</i>
<span>
test-2
test-13
</span>
</div>
</div>

View File

@ -6,6 +6,7 @@ import { namespaceSelectFilterModelFor } from "./namespace-select-filter-model";
import { getInjectable } from "@ogre-tools/injectable";
import namespaceStoreInjectable from "../store.injectable";
import isMultiSelectionKeyInjectable from "./is-selection-key.injectable";
import clusterFrameContextForNamespacedResourcesInjectable from "../../../cluster-frame-context/for-namespaced-resources.injectable";
const namespaceSelectFilterModelInjectable = getInjectable({
id: "namespace-select-filter-model",
@ -13,6 +14,7 @@ const namespaceSelectFilterModelInjectable = getInjectable({
instantiate: (di) => namespaceSelectFilterModelFor({
namespaceStore: di.inject(namespaceStoreInjectable),
isMultiSelectionKey: di.inject(isMultiSelectionKeyInjectable),
context: di.inject(clusterFrameContextForNamespacedResourcesInjectable),
}),
});

View File

@ -11,8 +11,10 @@ import { Icon } from "../../icon";
import type { SelectOption } from "../../select";
import { observableCrate } from "../../../utils";
import type { IsMultiSelectionKey } from "./is-selection-key.injectable";
import type { ClusterContext } from "../../../cluster-frame-context/cluster-frame-context";
interface Dependencies {
context: ClusterContext;
namespaceStore: NamespaceStore;
isMultiSelectionKey: IsMultiSelectionKey;
}
@ -44,7 +46,7 @@ enum SelectMenuState {
}
export function namespaceSelectFilterModelFor(dependencies: Dependencies): NamespaceSelectFilterModel {
const { isMultiSelectionKey, namespaceStore } = dependencies;
const { isMultiSelectionKey, namespaceStore, context } = dependencies;
let didToggle = false;
let isMultiSelection = false;
@ -56,7 +58,7 @@ export function namespaceSelectFilterModelFor(dependencies: Dependencies): Names
didToggle = false;
},
}]);
const selectedNames = computed(() => new Set(namespaceStore.contextNamespaces), {
const selectedNames = computed(() => new Set(context.contextNamespaces), {
equals: comparer.structural,
});
const optionsSortingSelected = observable.set(selectedNames.get());
@ -78,9 +80,8 @@ export function namespaceSelectFilterModelFor(dependencies: Dependencies): Names
label: "All Namespaces",
id: "all-namespaces",
},
...namespaceStore
.items
.map(ns => ns.getName())
...context
.allNamespaces
.sort(sortNamespacesByIfTheyHaveBeenSelected)
.map(namespace => ({
value: namespace,

View File

@ -12,9 +12,9 @@ import type { SelectProps } from "../select";
import { Select } from "../select";
import { cssNames } from "../../utils";
import { Icon } from "../icon";
import type { NamespaceStore } from "./store";
import { withInjectables } from "@ogre-tools/injectable-react";
import namespaceStoreInjectable from "./store.injectable";
import clusterFrameContextForNamespacedResourcesInjectable from "../../cluster-frame-context/for-namespaced-resources.injectable";
import type { ClusterContext } from "../../cluster-frame-context/cluster-frame-context";
export type NamespaceSelectSort = (left: string, right: string) => number;
@ -25,12 +25,12 @@ export interface NamespaceSelectProps<IsMulti extends boolean> extends Omit<Sele
}
interface Dependencies {
namespaceStore: NamespaceStore;
context: ClusterContext;
}
function getOptions(namespaceStore: NamespaceStore, sort: NamespaceSelectSort | undefined) {
function getOptions(context: ClusterContext, sort: NamespaceSelectSort | undefined) {
return computed(() => {
const baseOptions = namespaceStore.items.map(ns => ns.getName());
const baseOptions = context.allNamespaces;
if (sort) {
baseOptions.sort(sort);
@ -44,16 +44,16 @@ function getOptions(namespaceStore: NamespaceStore, sort: NamespaceSelectSort |
}
const NonInjectedNamespaceSelect = observer(({
namespaceStore,
context,
showIcons,
formatOptionLabel,
sort,
className,
...selectProps
}: Dependencies & NamespaceSelectProps<boolean>) => {
const [baseOptions, setBaseOptions] = useState(getOptions(namespaceStore, sort));
const [baseOptions, setBaseOptions] = useState(getOptions(context, sort));
useEffect(() => setBaseOptions(getOptions(namespaceStore, sort)), [sort]);
useEffect(() => setBaseOptions(getOptions(context, sort)), [sort]);
return (
<Select
@ -77,7 +77,7 @@ const NonInjectedNamespaceSelect = observer(({
const InjectedNamespaceSelect = withInjectables<Dependencies, NamespaceSelectProps<boolean>>(NonInjectedNamespaceSelect, {
getProps: (di, props) => ({
...props,
namespaceStore: di.inject(namespaceStoreInjectable),
context: di.inject(clusterFrameContextForNamespacedResourcesInjectable),
}),
});

View File

@ -5,13 +5,13 @@
import { getInjectable } from "@ogre-tools/injectable";
import { NamespaceStore } from "./store";
import { kubeObjectStoreInjectionToken } from "../../../common/k8s-api/api-manager/kube-object-store-token";
import createStorageInjectable from "../../utils/create-storage/create-storage.injectable";
import namespaceApiInjectable from "../../../common/k8s-api/endpoints/namespace.api.injectable";
import assert from "assert";
import storesAndApisCanBeCreatedInjectable from "../../stores-apis-can-be-created.injectable";
import clusterFrameContextForClusterScopedResourcesInjectable from "../../cluster-frame-context/for-cluster-scoped-resources.injectable";
import clusterConfiguredAccessibleNamespacesInjectable from "../../cluster/accessible-namespaces.injectable";
import loggerInjectable from "../../../common/logger.injectable";
import selectedNamespacesStorageInjectable from "../../../features/namespace-filtering/renderer/storage.injectable";
const namespaceStoreInjectable = getInjectable({
id: "namespace-store",
@ -19,12 +19,11 @@ const namespaceStoreInjectable = getInjectable({
instantiate: (di) => {
assert(di.inject(storesAndApisCanBeCreatedInjectable), "namespaceStore is only available in certain environments");
const createStorage = di.inject(createStorageInjectable);
const api = di.inject(namespaceApiInjectable);
return new NamespaceStore({
context: di.inject(clusterFrameContextForClusterScopedResourcesInjectable),
storage: createStorage<string[] | undefined>("selected_namespaces", undefined),
storage: di.inject(selectedNamespacesStorageInjectable),
clusterConfiguredAccessibleNamespaces: di.inject(clusterConfiguredAccessibleNamespacesInjectable),
logger: di.inject(loggerInjectable),
}, api);

View File

@ -19,7 +19,7 @@ export interface NamespaceTree {
}
interface Dependencies extends KubeObjectStoreDependencies {
readonly storage: StorageLayer<string[] | undefined>;
readonly storage: StorageLayer<string[]>;
readonly clusterConfiguredAccessibleNamespaces: IComputedValue<string[]>;
}
@ -28,21 +28,6 @@ export class NamespaceStore extends KubeObjectStore<Namespace, NamespaceApi> {
super(dependencies, api);
makeObservable(this);
autoBind(this);
// initialize allowed namespaces
const { allowedNamespaces } = this;
const selectedNamespaces = this.dependencies.storage.get(); // raw namespaces, undefined on first load
// return previously saved namespaces from local-storage (if any)
if (Array.isArray(selectedNamespaces)) {
this.selectNamespaces(selectedNamespaces.filter(namespace => allowedNamespaces.includes(namespace)));
} else if (allowedNamespaces.includes("default")) {
this.selectNamespaces(["default"]);
} else if (allowedNamespaces.length) {
this.selectNamespaces([allowedNamespaces[0]]);
} else {
this.selectNamespaces([]);
}
}
public onContextChange(callback: (namespaces: string[]) => void, opts: { fireImmediately?: boolean } = {}): IReactionDisposer {
@ -60,12 +45,16 @@ export class NamespaceStore extends KubeObjectStore<Namespace, NamespaceApi> {
return this.dependencies.storage.get() ?? [];
}
/**
* @deprecated This doesn't contain the namespaces from cluster settings or from cluster context
*/
@computed get allowedNamespaces(): string[] {
return this.items.map(item => item.getName());
}
/**
* The list of selected namespace names (for filtering)
* @deprecated This doesn't contain the namespaces from cluster settings or from cluster context
*/
@computed get contextNamespaces() {
if (!this.selectedNamespaces.length) {
@ -77,6 +66,7 @@ export class NamespaceStore extends KubeObjectStore<Namespace, NamespaceApi> {
/**
* The set of select namespace names (for filtering)
* @deprecated This doesn't contain the namespaces from cluster settings or from cluster context
*/
@computed get selectedNames(): Set<string> {
return new Set(this.contextNamespaces);

View File

@ -4,7 +4,7 @@
*/
import { getInjectable } from "@ogre-tools/injectable";
import namespaceStoreInjectable from "../+namespaces/store.injectable";
import clusterFrameContextForNamespacedResourcesInjectable from "../../cluster-frame-context/for-namespaced-resources.injectable";
import showErrorNotificationInjectable from "../notifications/show-error-notification.injectable";
import podStoreInjectable from "./store.injectable";
@ -12,12 +12,12 @@ const loadPodsFromAllNamespacesInjectable = getInjectable({
id: "load-pods-from-all-namespaces",
instantiate: (di) => {
const podStore = di.inject(podStoreInjectable);
const namespaceStore = di.inject(namespaceStoreInjectable);
const context = di.inject(clusterFrameContextForNamespacedResourcesInjectable);
const showErrorNotification = di.inject(showErrorNotificationInjectable);
return () => {
podStore.loadAll({
namespaces: [...namespaceStore.getItems().map(ns => ns.getName())],
namespaces: context.allNamespaces,
onLoadFailure: error =>
showErrorNotification(`Can not load Pods. ${String(error)}`),
});

View File

@ -9,7 +9,7 @@ import React from "react";
import { computed, observable, reaction } from "mobx";
import { disposeOnUnmount, observer } from "mobx-react";
import type { Disposer } from "../../utils";
import { cssNames, isDefined } from "../../utils";
import { hasTypedProperty, isObject, isString, cssNames, isDefined } from "../../utils";
import type { KubeJsonApiDataFor, KubeObject } from "../../../common/k8s-api/kube-object";
import type { ItemListLayoutProps } from "../item-object-list/list-layout";
import { ItemListLayout } from "../item-object-list/list-layout";
@ -64,6 +64,10 @@ const getLoadErrorMessage = (error: unknown): string => {
return error.message;
}
if (isObject(error) && hasTypedProperty(error, "message", isString)) {
return error.message;
}
return `${error}`;
};

View File

@ -69,6 +69,7 @@ import fsInjectable from "../../../common/fs/fs.injectable";
import joinPathsInjectable from "../../../common/path/join-paths.injectable";
import homeDirectoryPathInjectable from "../../../common/os/home-directory-path.injectable";
import { testUsingFakeTime } from "../../../common/test-utils/use-fake-time";
import selectedNamespacesStorageInjectable from "../../../features/namespace-filtering/renderer/storage.injectable";
type Callback = (di: DiContainer) => void | Promise<void>;
@ -369,7 +370,12 @@ export const getApplicationBuilder = () => {
namespaces.add(namespace);
namespaceItems.replace(createNamespacesFor(namespaces));
}),
select: action((namespace) => selectedNamespaces.add(namespace)),
select: action((namespace) => {
const selectedNamespacesStorage = builder.applicationWindow.only.di.inject(selectedNamespacesStorageInjectable);
selectedNamespaces.add(namespace);
selectedNamespacesStorage.set([...selectedNamespaces]);
}),
},
applicationMenu: {
get items() {
@ -504,6 +510,7 @@ export const getApplicationBuilder = () => {
const clusterStub = {
id: "some-cluster-id",
accessibleNamespaces: observable.array(),
allowedNamespaces: observable.array(),
shouldShowResource: (kind) => allowedResourcesState.has(formatKubeApiResource(kind)),
} as Partial<Cluster> as Cluster;

View File

@ -103,7 +103,7 @@ export class KubeWatchApi {
return () => this.#watch.dec(store);
}
namespaces ??= this.dependencies.clusterContext?.contextNamespaces ?? [];
namespaces ??= this.dependencies.clusterContext.contextNamespaces ?? [];
let childController = new WrappedAbortController(parent);
const unsubscribe = disposer();