From f2a229cef6107c99b8b61b9bbc4e566ee21f8358 Mon Sep 17 00:00:00 2001 From: Sebastian Malton Date: Fri, 24 Feb 2023 07:44:19 -0800 Subject: [PATCH] Fix ApiManager not handling duplicate apiBases of KubeApis (#7235) * Add failing unit test Signed-off-by: Sebastian Malton * Fix failing unit test Signed-off-by: Sebastian Malton --------- Signed-off-by: Sebastian Malton --- .../k8s-api/__tests__/api-manager.test.ts | 38 +++++++++++++++++-- .../common/k8s-api/api-manager/api-manager.ts | 9 +++-- 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/packages/core/src/common/k8s-api/__tests__/api-manager.test.ts b/packages/core/src/common/k8s-api/__tests__/api-manager.test.ts index b83f52c3f0..2e7e545f76 100644 --- a/packages/core/src/common/k8s-api/__tests__/api-manager.test.ts +++ b/packages/core/src/common/k8s-api/__tests__/api-manager.test.ts @@ -19,6 +19,9 @@ import { KubeObject } from "../kube-object"; import { KubeObjectStore } from "../kube-object.store"; import maybeKubeApiInjectable from "../maybe-kube-api.injectable"; +// eslint-disable-next-line no-restricted-imports +import { KubeApi as ExternalKubeApi } from "../../../extensions/common-api/k8s-api"; + class TestApi extends KubeApi { protected async checkPreferredVersion() { return; @@ -54,7 +57,7 @@ describe("ApiManager", () => { }); describe("registerApi", () => { - it("re-register store if apiBase changed", async () => { + it("re-register store if apiBase changed", () => { const apiBase = "apis/v1/foo"; const fallbackApiBase = "/apis/extensions/v1beta1/foo"; const kubeApi = new TestApi({ @@ -72,21 +75,48 @@ describe("ApiManager", () => { logger: di.inject(loggerInjectable), }, kubeApi); - apiManager.registerApi(apiBase, kubeApi); + apiManager.registerApi(kubeApi); // Define to use test api for ingress store Object.defineProperty(kubeStore, "api", { value: kubeApi }); - apiManager.registerStore(kubeStore, [kubeApi]); + apiManager.registerStore(kubeStore); // Test that store is returned with original apiBase expect(apiManager.getStore(kubeApi)).toBe(kubeStore); // Change apiBase similar as checkPreferredVersion does Object.defineProperty(kubeApi, "apiBase", { value: fallbackApiBase }); - apiManager.registerApi(fallbackApiBase, kubeApi); + apiManager.registerApi(kubeApi); // Test that store is returned with new apiBase expect(apiManager.getStore(kubeApi)).toBe(kubeStore); }); }); + + describe("technical tests for autorun", () => { + it("given two extensions register apis with the same apibase, settle into using the second", () => { + const apiBase = "/apis/aquasecurity.github.io/v1alpha1/vulnerabilityreports"; + const firstApi = Object.assign(new ExternalKubeApi({ + objectConstructor: KubeObject, + apiBase, + kind: "VulnerabilityReport", + }), { + myField: 1, + }); + const secondApi = Object.assign(new ExternalKubeApi({ + objectConstructor: KubeObject, + apiBase, + kind: "VulnerabilityReport", + }), { + myField: 2, + }); + + void firstApi; + void secondApi; + + expect(apiManager.getApi(apiBase)).toMatchObject({ + myField: 2, + }); + }); + }); }); diff --git a/packages/core/src/common/k8s-api/api-manager/api-manager.ts b/packages/core/src/common/k8s-api/api-manager/api-manager.ts index 81b59ef9c2..0dad9a2a21 100644 --- a/packages/core/src/common/k8s-api/api-manager/api-manager.ts +++ b/packages/core/src/common/k8s-api/api-manager/api-manager.ts @@ -41,19 +41,22 @@ export class ApiManager { const apis = chain(this.dependencies.apis.get().values()) .concat(this.externalApis.values()); const removedApis = new Set(this.apis.values()); + const newState = new Map(this.apis); for (const api of apis) { removedApis.delete(api); - this.apis.set(api.apiBase, api); + newState.set(api.apiBase, api); } for (const api of removedApis) { - for (const [apiBase, storedApi] of this.apis) { + for (const [apiBase, storedApi] of newState) { if (storedApi === api) { - this.apis.delete(apiBase); + newState.delete(apiBase); } } } + + this.apis.replace(newState); }); }