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

fix stack overflow and cycles in DI

Signed-off-by: Sebastian Malton <sebastian@malton.name>
This commit is contained in:
Sebastian Malton 2022-04-25 10:29:54 -04:00
parent 375018c330
commit 26062aa20b
13 changed files with 156 additions and 80 deletions

View File

@ -9,22 +9,27 @@ import { KubeObject } from "../kube-object";
import AbortController from "abort-controller";
import { delay } from "../../utils/delay";
import { PassThrough } from "stream";
import type { ApiManager } from "../api-manager";
import { apiManager } from "../api-manager";
import { ApiManager } from "../api-manager";
import type { FetchMock } from "jest-fetch-mock/types";
import { DeploymentApi, Ingress, IngressApi, Pod, PodApi } from "../endpoints";
import { getDiForUnitTesting } from "../../../main/getDiForUnitTesting";
import apiManagerInjectable from "../api-manager/manager.injectable";
jest.mock("../api-manager");
const mockApiManager = apiManager as jest.Mocked<ApiManager>;
const mockFetch = fetch as FetchMock;
describe("forRemoteCluster", () => {
let apiManager: jest.Mocked<ApiManager>;
beforeEach(async () => {
const di = getDiForUnitTesting({ doGeneralOverrides: true });
await di.runSetups();
apiManager = new ApiManager() as jest.Mocked<ApiManager>;
di.override(apiManagerInjectable, () => apiManager);
});
it("builds api client for KubeObject", async () => {
@ -79,6 +84,7 @@ describe("forRemoteCluster", () => {
describe("KubeApi", () => {
let request: KubeJsonApi;
let apiManager: jest.Mocked<ApiManager>;
beforeEach(async () => {
const di = getDiForUnitTesting({ doGeneralOverrides: true });
@ -89,6 +95,9 @@ describe("KubeApi", () => {
serverAddress: `http://127.0.0.1:9999`,
apiBase: "/api-kube",
});
apiManager = new ApiManager() as jest.Mocked<ApiManager>;
di.override(apiManagerInjectable, () => apiManager);
});
it("uses url from apiBase if apiBase contains the resource", async () => {
@ -218,7 +227,7 @@ describe("KubeApi", () => {
await (api as any).checkPreferredVersion();
expect(api.apiVersionPreferred).toBe("v1beta1");
expect(mockApiManager.registerApi).toBeCalledWith("/apis/extensions/v1beta1/ingresses", expect.anything());
expect(apiManager.registerApi).toBeCalledWith("/apis/extensions/v1beta1/ingresses", expect.anything());
});
it("registers with apiManager if checkPreferredVersion changes apiVersionPreferred with non-grouped apis", async () => {
@ -258,7 +267,7 @@ describe("KubeApi", () => {
await (api as any).checkPreferredVersion();
expect(api.apiVersionPreferred).toBe("v1beta1");
expect(mockApiManager.registerApi).toBeCalledWith("/api/v1beta1/pods", expect.anything());
expect(apiManager.registerApi).toBeCalledWith("/api/v1beta1/pods", expect.anything());
});
});

View File

@ -42,17 +42,28 @@ export class ApiManager {
return iter.find(this.apis.values(), api => api.kind === kind && api.apiVersionWithGroup === apiVersion);
}
registerApi<Api>(apiBase: string, api: RegisterableApi<Api>) {
if (!api.apiBase) return;
registerApi<Api>(api: RegisterableApi<Api>): void;
/**
* @deprecated Just register the `api` by itself
*/
registerApi<Api>(apiBase: string, api: RegisterableApi<Api>): void;
registerApi<Api>(apiBaseRaw: string | RegisterableApi<Api>, apiRaw?: RegisterableApi<Api>) {
const api = typeof apiBaseRaw === "string"
? apiRaw
: apiBaseRaw;
if (!this.apis.has(apiBase)) {
if (!api?.apiBase) {
return;
}
if (!this.apis.has(api.apiBase)) {
this.stores.forEach((store) => {
if (store.api as never === api) {
this.stores.set(apiBase, store);
if (store.api === api) {
this.stores.set(api.apiBase, store);
}
});
this.apis.set(apiBase, api as never);
this.apis.set(api.apiBase, api);
}
}

View File

@ -0,0 +1,27 @@
/**
* 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 EventEmitter from "events";
import type TypedEventEmitter from "typed-emitter";
import type { CustomResourceDefinition } from "../endpoints";
import type { KubeApi } from "../kube-api";
export interface LegacyAutoRegistration {
customResourceDefinition: (crd: CustomResourceDefinition) => void;
kubeApi: (api: KubeApi) => void;
}
/**
* This is used to remove dependency cycles from auto registering of instances
*
* - Custom Resource Definitions get their own registered store (will need in the future)
* - All KubeApi's get auto registered (this should be changed in the future)
*/
const autoRegistrationEmitterInjectable = getInjectable({
id: "auto-registration-emitter",
instantiate: (): TypedEventEmitter<LegacyAutoRegistration> => new EventEmitter(),
});
export default autoRegistrationEmitterInjectable;

View File

@ -0,0 +1,48 @@
/**
* 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 { KubeApi } from "../kube-api";
import { KubeObject } from "../kube-object";
import autoRegistrationEmitterInjectable from "./auto-registration-emitter.injectable";
import apiManagerInjectable from "./manager.injectable";
import { CustomResourceStore } from "./resource.store";
const autoRegistrationInjectable = getInjectable({
id: "api-manager-auto-registration",
instantiate: (di) => {
const apiManager = di.inject(apiManagerInjectable);
const autoRegistrationEmitter = di.inject(autoRegistrationEmitterInjectable);
autoRegistrationEmitter
.on("customResourceDefinition", (crd) => {
const objectConstructor = class extends KubeObject {
static readonly kind = crd.getResourceKind();
static readonly namespaced = crd.isNamespaced();
static readonly apiBase = crd.getResourceApiBase();
};
const api = (() => {
const rawApi = apiManager.getApi(objectConstructor.apiBase);
if (rawApi) {
return rawApi;
}
const api = new KubeApi({ objectConstructor });
apiManager.registerApi(api);
return api;
})();
if (!apiManager.getStore(api)) {
apiManager.registerStore(new CustomResourceStore(api));
}
})
.on("kubeApi", (api) => apiManager.registerApi(api));
},
});
export default autoRegistrationInjectable;

View File

@ -3,9 +3,9 @@
* Licensed under MIT License. See LICENSE in root directory for more information.
*/
import type { KubeApi } from "../../../common/k8s-api/kube-api";
import { KubeObjectStore } from "../../../common/k8s-api/kube-object.store";
import type { KubeObject } from "../../../common/k8s-api/kube-object";
import type { KubeApi } from "../kube-api";
import { KubeObjectStore } from "../kube-object.store";
import type { KubeObject } from "../kube-object";
export class CustomResourceStore<K extends KubeObject> extends KubeObjectStore<K, KubeApi<K>> {
constructor(api: KubeApi<K>) {

View File

@ -27,7 +27,7 @@ import assert from "assert";
import type { PartialDeep } from "type-fest";
import logger from "../logger";
import { Environments, getEnvironmentSpecificLegacyGlobalDiForExtensionApi } from "../../extensions/as-legacy-globals-for-extension-api/legacy-global-di-for-extension-api";
import apiManagerInjectable from "./api-manager/manager.injectable";
import autoRegistrationEmitterInjectable from "./api-manager/auto-registration-emitter.injectable";
/**
* The options used for creating a `KubeApi`
@ -311,15 +311,15 @@ export interface DeleteResourceDescriptor extends ResourceDescriptor {
/**
* @deprecated In the new extension API, don't expose `KubeApi`'s constructor
*/
function legacyRegisterApi(apiBase: string, api: KubeApi): void {
function legacyRegisterApi(api: KubeApi): void {
// Try both just in case, because we might be in a testing environment
for (const env of [Environments.main, Environments.renderer]) {
const di = getEnvironmentSpecificLegacyGlobalDiForExtensionApi(env);
if (di) {
const apiManager = di.inject(apiManagerInjectable);
const autoRegistrationEmitter = di.inject(autoRegistrationEmitterInjectable);
apiManager.registerApi(apiBase, api);
autoRegistrationEmitter.emit("kubeApi", api);
}
}
}
@ -346,15 +346,17 @@ export class KubeApi<
protected readonly fullApiPathname: string;
protected readonly fallbackApiBases: string[] | undefined;
constructor({
objectConstructor,
request = apiKube,
kind = objectConstructor.kind,
isNamespaced,
apiBase: fullApiPathname = objectConstructor.apiBase,
checkPreferredVersion: doCheckPreferredVersion = false,
fallbackApiBases,
}: KubeApiOptions<Object, Data>) {
constructor(opts: KubeApiOptions<Object, Data>) {
const {
objectConstructor,
request = apiKube,
kind = objectConstructor.kind,
isNamespaced,
apiBase: fullApiPathname = objectConstructor.apiBase,
checkPreferredVersion: doCheckPreferredVersion = false,
fallbackApiBases,
} = opts;
assert(fullApiPathname, "apiBase MUST be provied either via KubeApiOptions.apiBase or KubeApiOptions.objectConstructor.apiBase");
assert(request, "request MUST be provided if not in a cluster page frame context");
@ -375,8 +377,7 @@ export class KubeApi<
this.apiResource = resource;
this.request = request;
this.objectConstructor = objectConstructor;
this.parseResponse = this.parseResponse.bind(this);
legacyRegisterApi(apiBase, this as unknown as KubeApi);
legacyRegisterApi(this as unknown as KubeApi);
}
get apiVersionWithGroup() {
@ -457,7 +458,7 @@ export class KubeApi<
if (this.apiVersionPreferred) {
this.apiBase = this.computeApiBase();
legacyRegisterApi(this.apiBase, this as unknown as KubeApi);
legacyRegisterApi(this as unknown as KubeApi);
}
}
}

View File

@ -114,4 +114,4 @@ export type { ServiceAccountStore as ServiceAccountsStore } from "../../renderer
export type { RoleStore as RolesStore } from "../../renderer/components/+user-management/+roles/store";
export type { RoleBindingStore as RoleBindingsStore } from "../../renderer/components/+user-management/+role-bindings/store";
export type { CustomResourceDefinitionStore as CRDStore } from "../../renderer/components/+custom-resources/definition.store";
export type { CustomResourceStore as CRDResourceStore } from "../../renderer/components/+custom-resources/resource.store";
export type { CustomResourceStore as CRDResourceStore } from "../../common/k8s-api/api-manager/resource.store";

View File

@ -4,11 +4,11 @@
*/
import { getInjectable } from "@ogre-tools/injectable";
import assert from "assert";
import autoRegistrationEmitterInjectable from "../../../common/k8s-api/api-manager/auto-registration-emitter.injectable";
import { kubeObjectStoreInjectionToken } from "../../../common/k8s-api/api-manager/manager.injectable";
import customResourceDefinitionApiInjectable from "../../../common/k8s-api/endpoints/custom-resource-definition.api.injectable";
import createStoresAndApisInjectable from "../../create-stores-apis.injectable";
import { CustomResourceDefinitionStore } from "./definition.store";
import initCustomResourceStoreInjectable from "./init-resource-store.injectable";
const customResourceDefinitionStoreInjectable = getInjectable({
id: "custom-resource-definition-store",
@ -18,7 +18,7 @@ const customResourceDefinitionStoreInjectable = getInjectable({
const api = di.inject(customResourceDefinitionApiInjectable);
return new CustomResourceDefinitionStore({
initCustomResourceStore: di.inject(initCustomResourceStoreInjectable),
autoRegistration: di.inject(autoRegistrationEmitterInjectable),
}, api);
},
injectionToken: kubeObjectStoreInjectionToken,

View File

@ -9,10 +9,11 @@ import { KubeObjectStore } from "../../../common/k8s-api/kube-object.store";
import { autoBind } from "../../utils";
import type { CustomResourceDefinition, CustomResourceDefinitionApi } from "../../../common/k8s-api/endpoints/custom-resource-definition.api";
import type { KubeObject } from "../../../common/k8s-api/kube-object";
import type { InitCustomResourceStore } from "./init-resource-store.injectable";
import type TypedEventEmitter from "typed-emitter";
import type { LegacyAutoRegistration } from "../../../common/k8s-api/api-manager/auto-registration-emitter.injectable";
export interface CustomResourceDefinitionStoreDependencies {
initCustomResourceStore: InitCustomResourceStore;
readonly autoRegistration: TypedEventEmitter<LegacyAutoRegistration>;
}
export class CustomResourceDefinitionStore extends KubeObjectStore<CustomResourceDefinition, CustomResourceDefinitionApi> {
@ -25,8 +26,14 @@ export class CustomResourceDefinitionStore extends KubeObjectStore<CustomResourc
makeObservable(this);
autoBind(this);
// auto-init stores for crd-s
reaction(() => this.getItems(), items => items.forEach(this.dependencies.initCustomResourceStore));
reaction(
() => this.getItems(),
crds => {
for (const crd of crds) {
this.dependencies.autoRegistration.emit("customResourceDefinition", crd);
}
},
);
}
protected sortItems(items: CustomResourceDefinition[]) {

View File

@ -1,36 +0,0 @@
/**
* 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 apiManagerInjectable from "../../../common/k8s-api/api-manager/manager.injectable";
import type { CustomResourceDefinition } from "../../../common/k8s-api/endpoints";
import { KubeApi } from "../../../common/k8s-api/kube-api";
import { KubeObject } from "../../../common/k8s-api/kube-object";
import { CustomResourceStore } from "./resource.store";
export type InitCustomResourceStore = (crd: CustomResourceDefinition) => void;
const initCustomResourceStoreInjectable = getInjectable({
id: "init-custom-resource-store",
instantiate: (di): InitCustomResourceStore => {
const apiManager = di.inject(apiManagerInjectable);
return (crd) => {
const objectConstructor = class extends KubeObject {
static readonly kind = crd.getResourceKind();
static readonly namespaced = crd.isNamespaced();
static readonly apiBase = crd.getResourceApiBase();
};
const api = apiManager.getApi(objectConstructor.apiBase)
?? new KubeApi({ objectConstructor });
if (!apiManager.getStore(api)) {
apiManager.registerStore(new CustomResourceStore(api));
}
};
},
});
export default initCustomResourceStoreInjectable;

View File

@ -4,7 +4,7 @@
*/
import { getInjectable } from "@ogre-tools/injectable";
import assert from "assert";
import apiManagerInjectable from "../../../common/k8s-api/api-manager/manager.injectable";
import { kubeObjectStoreInjectionToken } from "../../../common/k8s-api/api-manager/manager.injectable";
import ingressApiInjectable from "../../../common/k8s-api/endpoints/ingress.api.injectable";
import createStoresAndApisInjectable from "../../create-stores-apis.injectable";
import { IngressStore } from "./store";
@ -15,13 +15,10 @@ const ingressStoreInjectable = getInjectable({
assert(di.inject(createStoresAndApisInjectable), "ingressStore is only available in certain environments");
const api = di.inject(ingressApiInjectable);
const apiManager = di.inject(apiManagerInjectable);
const store = new IngressStore(api);
apiManager.registerStore(store);
return store;
return new IngressStore(api);
},
injectionToken: kubeObjectStoreInjectionToken,
});
export default ingressStoreInjectable;

View File

@ -55,7 +55,10 @@ export const NonInjectedClusterFrame = observer(({
return (
<ErrorBoundary>
<MainLayout sidebar={<Sidebar />} footer={<Dock />}>
<MainLayout
sidebar={<Sidebar />}
footer={<Dock />}
>
{
Component
? <Component />

View File

@ -11,6 +11,7 @@ import hostedClusterInjectable from "../../../../common/cluster-store/hosted-clu
import appEventBusInjectable from "../../../../common/app-event-bus/app-event-bus.injectable";
import clusterFrameContextInjectable from "../../../cluster-frame-context/cluster-frame-context.injectable";
import assert from "assert";
import autoRegistrationInjectable from "../../../../common/k8s-api/api-manager/auto-registration.injectable";
const initClusterFrameInjectable = getInjectable({
id: "init-cluster-frame",
@ -20,6 +21,14 @@ const initClusterFrameInjectable = getInjectable({
assert(hostedCluster, "This can only be injected within a cluster frame");
/**
* This is injected here to initialize it for the side effect.
*
* The side effect CANNOT be within `apiManagerInjectable` itself since that causes circular
* dependencies with the current need for legacy di use
*/
di.inject(autoRegistrationInjectable);
return initClusterFrame({
hostedCluster,
loadExtensions: di.inject(extensionLoaderInjectable).loadOnClusterRenderer,