Unverified Commit 0745f9c0 authored by Sebastian Malton's avatar Sebastian Malton Committed by GitHub
Browse files

Fix Cluster.refresh sometimes taking upwards of 20s (#4826)


- Only read and parse the proxy config once

- Reuse the AuthorizationV1Api instance for the entire refresh instead
  of recreating it between 32 and 302 times, this should allow for
  reusing sockets
Signed-off-by: default avatarSebastian Malton <sebastian@malton.name>
Showing with 119 additions and 74 deletions
+119 -74
/**
* Copyright (c) OpenLens Authors. All rights reserved.
* Licensed under MIT License. See LICENSE in root directory for more information.
*/
import { AuthorizationV1Api, KubeConfig, V1ResourceAttributes } from "@kubernetes/client-node";
import logger from "../logger";
import { getInjectable, lifecycleEnum } from "@ogre-tools/injectable";
export type CanI = (resourceAttributes: V1ResourceAttributes) => Promise<boolean>;
/**
* @param proxyConfig This config's `currentContext` field must be set, and will be used as the target cluster
*/
export function authorizationReview(proxyConfig: KubeConfig): CanI {
const api = proxyConfig.makeApiClient(AuthorizationV1Api);
/**
* Requests the permissions for actions on the kube cluster
* @param resourceAttributes The descriptor of the action that is desired to be known if it is allowed
* @returns `true` if the actions described are allowed
*/
return async (resourceAttributes: V1ResourceAttributes): Promise<boolean> => {
try {
const { body } = await api.createSelfSubjectAccessReview({
apiVersion: "authorization.k8s.io/v1",
kind: "SelfSubjectAccessReview",
spec: { resourceAttributes },
});
return body.status?.allowed ?? false;
} catch (error) {
logger.error(`[AUTHORIZATION-REVIEW]: failed to create access review: ${error}`, { resourceAttributes });
return false;
}
};
}
const authorizationReviewInjectable = getInjectable({
instantiate: () => authorizationReview,
lifecycle: lifecycleEnum.singleton,
});
export default authorizationReviewInjectable;
......@@ -7,7 +7,7 @@ import { ipcMain } from "electron";
import { action, comparer, computed, makeObservable, observable, reaction, when } from "mobx";
import { broadcastMessage } from "../ipc";
import type { ContextHandler } from "../../main/context-handler/context-handler";
import { AuthorizationV1Api, CoreV1Api, HttpError, KubeConfig, V1ResourceAttributes } from "@kubernetes/client-node";
import { HttpError, KubeConfig } from "@kubernetes/client-node";
import type { Kubectl } from "../../main/kubectl/kubectl";
import type { KubeconfigManager } from "../../main/kubeconfig-manager/kubeconfig-manager";
import { loadConfigFromFile, loadConfigFromFileSync, validateKubeConfig } from "../kube-helpers";
......@@ -21,12 +21,16 @@ import { ClusterMetadataKey, initialNodeShellImage, ClusterStatus } from "../clu
import { disposer, toJS } from "../utils";
import type { Response } from "request";
import { clusterListNamespaceForbiddenChannel } from "../ipc/cluster";
interface Dependencies {
directoryForKubeConfigs: string,
createKubeconfigManager: (cluster: Cluster) => KubeconfigManager,
createContextHandler: (cluster: Cluster) => ContextHandler,
createKubectl: (clusterVersion: string) => Kubectl
import type { CanI } from "./authorization-review.injectable";
import type { ListNamespaces } from "./list-namespaces.injectable";
export interface ClusterDependencies {
readonly directoryForKubeConfigs: string;
createKubeconfigManager: (cluster: Cluster) => KubeconfigManager;
createContextHandler: (cluster: Cluster) => ContextHandler;
createKubectl: (clusterVersion: string) => Kubectl;
createAuthorizationReview: (config: KubeConfig) => CanI;
createListNamespaces: (config: KubeConfig) => ListNamespaces;
}
/**
......@@ -213,7 +217,7 @@ export class Cluster implements ClusterModel, ClusterState {
return this.preferences.defaultNamespace;
}
constructor(private dependencies: Dependencies, model: ClusterModel) {
constructor(private readonly dependencies: ClusterDependencies, model: ClusterModel) {
makeObservable(this);
this.id = model.id;
this.updateModel(model);
......@@ -425,10 +429,20 @@ export class Cluster implements ClusterModel, ClusterState {
* @internal
*/
private async refreshAccessibility(): Promise<void> {
this.isAdmin = await this.isClusterAdmin();
this.isGlobalWatchEnabled = await this.canUseWatchApi({ resource: "*" });
this.allowedNamespaces = await this.getAllowedNamespaces();
this.allowedResources = await this.getAllowedResources();
const proxyConfig = await this.getProxyKubeconfig();
const canI = this.dependencies.createAuthorizationReview(proxyConfig);
this.isAdmin = await canI({
namespace: "kube-system",
resource: "*",
verb: "create",
});
this.isGlobalWatchEnabled = await canI({
verb: "watch",
resource: "*",
});
this.allowedNamespaces = await this.getAllowedNamespaces(proxyConfig);
this.allowedResources = await this.getAllowedResources(canI);
this.ready = true;
}
......@@ -507,50 +521,6 @@ export class Cluster implements ClusterModel, ClusterState {
}
}
/**
* @internal
* @param resourceAttributes resource attributes
*/
async canI(resourceAttributes: V1ResourceAttributes): Promise<boolean> {
const authApi = (await this.getProxyKubeconfig()).makeApiClient(AuthorizationV1Api);
try {
const accessReview = await authApi.createSelfSubjectAccessReview({
apiVersion: "authorization.k8s.io/v1",
kind: "SelfSubjectAccessReview",
spec: { resourceAttributes },
});
return accessReview.body.status.allowed;
} catch (error) {
logger.error(`failed to request selfSubjectAccessReview: ${error}`);
return false;
}
}
/**
* @internal
*/
async isClusterAdmin(): Promise<boolean> {
return this.canI({
namespace: "kube-system",
resource: "*",
verb: "create",
});
}
/**
* @internal
*/
async canUseWatchApi(customizeResource: V1ResourceAttributes = {}): Promise<boolean> {
return this.canI({
verb: "watch",
resource: "*",
...customizeResource,
});
}
toJSON(): ClusterModel {
return toJS({
id: this.id,
......@@ -622,20 +592,17 @@ export class Cluster implements ClusterModel, ClusterState {
broadcastMessage(`cluster:${this.id}:connection-update`, update);
}
protected async getAllowedNamespaces() {
protected async getAllowedNamespaces(proxyConfig: KubeConfig) {
if (this.accessibleNamespaces.length) {
return this.accessibleNamespaces;
}
const api = (await this.getProxyKubeconfig()).makeApiClient(CoreV1Api);
try {
const { body: { items }} = await api.listNamespace();
const namespaces = items.map(ns => ns.metadata.name);
const listNamespaces = this.dependencies.createListNamespaces(proxyConfig);
return namespaces;
return await listNamespaces();
} catch (error) {
const ctx = (await this.getProxyKubeconfig()).getContextObject(this.contextName);
const ctx = proxyConfig.getContextObject(this.contextName);
const namespaceList = [ctx.namespace].filter(Boolean);
if (namespaceList.length === 0 && error instanceof HttpError && error.statusCode === 403) {
......@@ -649,7 +616,7 @@ export class Cluster implements ClusterModel, ClusterState {
}
}
protected async getAllowedResources() {
protected async getAllowedResources(canI: CanI) {
try {
if (!this.allowedNamespaces.length) {
return [];
......@@ -662,7 +629,7 @@ export class Cluster implements ClusterModel, ClusterState {
requests.push(apiLimit(async () => {
for (const namespace of this.allowedNamespaces.slice(0, 10)) {
if (!this.resourceAccessStatuses.get(apiResource)) {
const result = await this.canI({
const result = await canI({
resource: apiResource.apiName,
group: apiResource.group,
verb: "list",
......
/**
* Copyright (c) OpenLens Authors. All rights reserved.
* Licensed under MIT License. See LICENSE in root directory for more information.
*/
import { CoreV1Api, KubeConfig } from "@kubernetes/client-node";
import { getInjectable, lifecycleEnum } from "@ogre-tools/injectable";
export type ListNamespaces = () => Promise<string[]>;
export function listNamespaces(config: KubeConfig): ListNamespaces {
const coreApi = config.makeApiClient(CoreV1Api);
return async () => {
const { body: { items }} = await coreApi.listNamespace();
return items.map(ns => ns.metadata.name);
};
}
const listNamespacesInjectable = getInjectable({
instantiate: () => listNamespaces,
lifecycle: lifecycleEnum.singleton,
});
export default listNamespacesInjectable;
......@@ -39,6 +39,8 @@ import { Kubectl } from "../kubectl/kubectl";
import { getDiForUnitTesting } from "../getDiForUnitTesting";
import type { ClusterModel } from "../../common/cluster-types";
import { createClusterInjectionToken } from "../../common/cluster/create-cluster-injection-token";
import authorizationReviewInjectable from "../../common/cluster/authorization-review.injectable";
import listNamespacesInjectable from "../../common/cluster/list-namespaces.injectable";
console = new Console(process.stdout, process.stderr); // fix mockFS
......@@ -77,6 +79,9 @@ describe("create clusters", () => {
await di.runSetups();
di.override(authorizationReviewInjectable, () => () => () => Promise.resolve(true));
di.override(listNamespacesInjectable, () => () => () => Promise.resolve([ "default" ]));
createCluster = di.inject(createClusterInjectionToken);
jest.spyOn(Kubectl.prototype, "ensureKubectl").mockReturnValue(Promise.resolve(true));
......@@ -117,7 +122,6 @@ describe("create clusters", () => {
} as any;
jest.spyOn(cluster, "reconnect");
jest.spyOn(cluster, "canI");
jest.spyOn(cluster, "refreshConnectionStatus");
await cluster.activate();
......
......@@ -3,24 +3,27 @@
* Licensed under MIT License. See LICENSE in root directory for more information.
*/
import { getInjectable, lifecycleEnum } from "@ogre-tools/injectable";
import type { ClusterModel } from "../../common/cluster-types";
import { Cluster } from "../../common/cluster/cluster";
import { Cluster, ClusterDependencies } from "../../common/cluster/cluster";
import directoryForKubeConfigsInjectable from "../../common/app-paths/directory-for-kube-configs/directory-for-kube-configs.injectable";
import createKubeconfigManagerInjectable from "../kubeconfig-manager/create-kubeconfig-manager.injectable";
import createKubectlInjectable from "../kubectl/create-kubectl.injectable";
import createContextHandlerInjectable from "../context-handler/create-context-handler.injectable";
import { createClusterInjectionToken } from "../../common/cluster/create-cluster-injection-token";
import authorizationReviewInjectable from "../../common/cluster/authorization-review.injectable";
import listNamespacesInjectable from "../../common/cluster/list-namespaces.injectable";
const createClusterInjectable = getInjectable({
instantiate: (di) => {
const dependencies = {
const dependencies: ClusterDependencies = {
directoryForKubeConfigs: di.inject(directoryForKubeConfigsInjectable),
createKubeconfigManager: di.inject(createKubeconfigManagerInjectable),
createKubectl: di.inject(createKubectlInjectable),
createContextHandler: di.inject(createContextHandlerInjectable),
createAuthorizationReview: di.inject(authorizationReviewInjectable),
createListNamespaces: di.inject(listNamespacesInjectable),
};
return (model: ClusterModel) => new Cluster(dependencies, model);
return (model) => new Cluster(dependencies, model);
},
injectionToken: createClusterInjectionToken,
......
......@@ -3,21 +3,22 @@
* Licensed under MIT License. See LICENSE in root directory for more information.
*/
import { getInjectable, lifecycleEnum } from "@ogre-tools/injectable";
import type { ClusterModel } from "../../common/cluster-types";
import { Cluster } from "../../common/cluster/cluster";
import { Cluster, ClusterDependencies } from "../../common/cluster/cluster";
import directoryForKubeConfigsInjectable from "../../common/app-paths/directory-for-kube-configs/directory-for-kube-configs.injectable";
import { createClusterInjectionToken } from "../../common/cluster/create-cluster-injection-token";
const createClusterInjectable = getInjectable({
instantiate: (di) => {
const dependencies = {
const dependencies: ClusterDependencies = {
directoryForKubeConfigs: di.inject(directoryForKubeConfigsInjectable),
createKubeconfigManager: () => { throw new Error("Tried to access back-end feature in front-end."); },
createKubectl: () => { throw new Error("Tried to access back-end feature in front-end.");},
createContextHandler: () => { throw new Error("Tried to access back-end feature in front-end."); },
createAuthorizationReview: () => { throw new Error("Tried to access back-end feature in front-end."); },
createListNamespaces: () => { throw new Error("Tried to access back-end feature in front-end."); },
};
return (model: ClusterModel) => new Cluster(dependencies, model);
return (model) => new Cluster(dependencies, model);
},
injectionToken: createClusterInjectionToken,
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment