From 9ed64a29df04619eb914b0171edd894587591165 Mon Sep 17 00:00:00 2001 From: Sebastian Malton Date: Tue, 15 Nov 2022 08:04:51 -0800 Subject: [PATCH] Fix auto finding logic of preferred versions (#6573) * Fix auto finding logic of preferred versions - The kube preferred version might not contain the resource requested in some kube versions. Whereas the resource does exist on some previous api version Signed-off-by: Sebastian Malton * Simplify getOrderedVersions Signed-off-by: Sebastian Malton * Split test file Signed-off-by: Sebastian Malton * Fix grammer Signed-off-by: Sebastian Malton Signed-off-by: Sebastian Malton --- .../kube-api-version-detection.test.ts | 719 ++++++++++++++++++ src/common/k8s-api/__tests__/kube-api.test.ts | 437 +---------- .../endpoints/cron-job.api.injectable.ts | 3 - .../k8s-api/endpoints/job.api.injectable.ts | 4 +- src/common/k8s-api/kube-api-parse.ts | 1 + src/common/k8s-api/kube-api.ts | 107 +-- src/common/k8s-api/kube-object.store.ts | 4 +- .../kube-object-list-layout.tsx | 18 +- 8 files changed, 800 insertions(+), 493 deletions(-) create mode 100644 src/common/k8s-api/__tests__/kube-api-version-detection.test.ts diff --git a/src/common/k8s-api/__tests__/kube-api-version-detection.test.ts b/src/common/k8s-api/__tests__/kube-api-version-detection.test.ts new file mode 100644 index 0000000000..eb13464716 --- /dev/null +++ b/src/common/k8s-api/__tests__/kube-api-version-detection.test.ts @@ -0,0 +1,719 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ +import type { KubeJsonApi } from "../kube-json-api"; +import { PassThrough } from "stream"; +import type { ApiManager } from "../api-manager"; +import { Ingress, IngressApi } from "../endpoints"; +import { getDiForUnitTesting } from "../../../renderer/getDiForUnitTesting"; +import apiManagerInjectable from "../api-manager/manager.injectable"; +import autoRegistrationInjectable from "../api-manager/auto-registration.injectable"; +import type { Fetch } from "../../fetch/fetch.injectable"; +import fetchInjectable from "../../fetch/fetch.injectable"; +import type { AsyncFnMock } from "@async-fn/jest"; +import asyncFn from "@async-fn/jest"; +import { flushPromises } from "../../test-utils/flush-promises"; +import createKubeJsonApiInjectable from "../create-kube-json-api.injectable"; +import type { Response, Headers as NodeFetchHeaders } from "node-fetch"; + +const createMockResponseFromString = (url: string, data: string, statusCode = 200) => { + const res: jest.Mocked = { + buffer: jest.fn(async () => { throw new Error("buffer() is not supported"); }), + clone: jest.fn(() => res), + arrayBuffer: jest.fn(async () => { throw new Error("arrayBuffer() is not supported"); }), + blob: jest.fn(async () => { throw new Error("blob() is not supported"); }), + body: new PassThrough(), + bodyUsed: false, + headers: new Headers() as NodeFetchHeaders, + json: jest.fn(async () => JSON.parse(await res.text())), + ok: 200 <= statusCode && statusCode < 300, + redirected: 300 <= statusCode && statusCode < 400, + size: data.length, + status: statusCode, + statusText: "some-text", + text: jest.fn(async () => data), + type: "basic", + url, + formData: jest.fn(async () => { throw new Error("formData() is not supported"); }), + }; + + return res; +}; + +describe("KubeApi", () => { + let request: KubeJsonApi; + let registerApiSpy: jest.SpiedFunction; + let fetchMock: AsyncFnMock; + + beforeEach(async () => { + const di = getDiForUnitTesting({ doGeneralOverrides: true }); + + fetchMock = asyncFn(); + di.override(fetchInjectable, () => fetchMock); + + const createKubeJsonApi = di.inject(createKubeJsonApiInjectable); + + request = createKubeJsonApi({ + serverAddress: `http://127.0.0.1:9999`, + apiBase: "/api-kube", + }); + registerApiSpy = jest.spyOn(di.inject(apiManagerInjectable), "registerApi"); + + di.inject(autoRegistrationInjectable); + }); + + describe("on first call to IngressApi.get()", () => { + let ingressApi: IngressApi; + let getCall: Promise; + + beforeEach(async () => { + ingressApi = new IngressApi({ + request, + objectConstructor: Ingress, + apiBase: "/apis/networking.k8s.io/v1/ingresses", + fallbackApiBases: ["/apis/extensions/v1beta1/ingresses"], + checkPreferredVersion: true, + }); + getCall = ingressApi.get({ + name: "foo", + namespace: "default", + }); + + // This is needed because of how JS promises work + await flushPromises(); + }); + + it("requests version list from the api group from the initial apiBase", () => { + expect(fetchMock.mock.lastCall).toMatchObject([ + "http://127.0.0.1:9999/api-kube/apis/networking.k8s.io", + { + headers: { + "content-type": "application/json", + }, + method: "get", + }, + ]); + }); + + describe("when the version list from the api group resolves", () => { + beforeEach(async () => { + await fetchMock.resolveSpecific( + ["http://127.0.0.1:9999/api-kube/apis/networking.k8s.io"], + createMockResponseFromString("http://127.0.0.1:9999/api-kube/apis/networking.k8s.io", JSON.stringify({ + apiVersion: "v1", + kind: "APIGroup", + name: "networking.k8s.io", + versions: [ + { + groupVersion: "networking.k8s.io/v1", + version: "v1", + }, + { + groupVersion: "networking.k8s.io/v1beta1", + version: "v1beta1", + }, + ], + preferredVersion: { + groupVersion: "networking.k8s.io/v1", + version: "v1", + }, + })), + ); + }); + + it("requests resources from the versioned api group from the initial apiBase", () => { + expect(fetchMock.mock.lastCall).toMatchObject([ + "http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1", + { + headers: { + "content-type": "application/json", + }, + method: "get", + }, + ]); + }); + + describe("when resource request fufills with a resource", () => { + beforeEach(async () => { + await fetchMock.resolveSpecific( + ["http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1"], + createMockResponseFromString("http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1", JSON.stringify({ + resources: [{ + name: "ingresses", + }], + })), + ); + }); + + it("makes the request to get the resource", () => { + expect(fetchMock.mock.lastCall).toMatchObject([ + "http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1/namespaces/default/ingresses/foo", + { + headers: { + "content-type": "application/json", + }, + method: "get", + }, + ]); + }); + + it("sets fields in the api instance", () => { + expect(ingressApi).toEqual(expect.objectContaining({ + apiVersionPreferred: "v1", + apiPrefix: "/apis", + apiGroup: "networking.k8s.io", + })); + }); + + it("registers the api with the changes info", () => { + expect(registerApiSpy).toBeCalledWith(ingressApi); + }); + + describe("when the request resolves with no data", () => { + let result: Ingress | null; + + beforeEach(async () => { + await fetchMock.resolveSpecific( + ["http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1/namespaces/default/ingresses/foo"], + createMockResponseFromString("http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1/namespaces/default/ingresses/foo", JSON.stringify({})), + ); + result = await getCall; + }); + + it("results in the get call resolving to null", () => { + expect(result).toBeNull(); + }); + + describe("on the second call to IngressApi.get()", () => { + let getCall: Promise; + + beforeEach(async () => { + getCall = ingressApi.get({ + name: "foo1", + namespace: "default", + }); + + // This is needed because of how JS promises work + await flushPromises(); + }); + + it("makes the request to get the resource", () => { + expect(fetchMock.mock.lastCall).toMatchObject([ + "http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1/namespaces/default/ingresses/foo1", + { + headers: { + "content-type": "application/json", + }, + method: "get", + }, + ]); + }); + + describe("when the request resolves with no data", () => { + let result: Ingress | null; + + beforeEach(async () => { + await fetchMock.resolveSpecific( + ["http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1/namespaces/default/ingresses/foo1"], + createMockResponseFromString("http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1/namespaces/default/ingresses/foo1", JSON.stringify({})), + ); + result = await getCall; + }); + + it("results in the get call resolving to null", () => { + expect(result).toBeNull(); + }); + }); + }); + }); + + describe("when the request resolves with data", () => { + let result: Ingress | null; + + beforeEach(async () => { + await fetchMock.resolveSpecific( + ["http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1/namespaces/default/ingresses/foo"], + createMockResponseFromString("http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1/namespaces/default/ingresses/foo", JSON.stringify({ + apiVersion: "v1", + kind: "Ingress", + metadata: { + name: "foo", + namespace: "default", + resourceVersion: "1", + uid: "12345", + }, + })), + ); + result = await getCall; + }); + + it("results in the get call resolving to an instance", () => { + expect(result).toBeInstanceOf(Ingress); + }); + + describe("on the second call to IngressApi.get()", () => { + let getCall: Promise; + + beforeEach(async () => { + getCall = ingressApi.get({ + name: "foo1", + namespace: "default", + }); + + // This is needed because of how JS promises work + await flushPromises(); + }); + + it("makes the request to get the resource", () => { + expect(fetchMock.mock.lastCall).toMatchObject([ + "http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1/namespaces/default/ingresses/foo1", + { + headers: { + "content-type": "application/json", + }, + method: "get", + }, + ]); + }); + + describe("when the request resolves with no data", () => { + let result: Ingress | null; + + beforeEach(async () => { + await fetchMock.resolveSpecific( + ["http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1/namespaces/default/ingresses/foo1"], + createMockResponseFromString("http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1/namespaces/default/ingresses/foo1", JSON.stringify({})), + ); + result = await getCall; + }); + + it("results in the get call resolving to null", () => { + expect(result).toBeNull(); + }); + }); + }); + }); + }); + + describe("when resource request fufills with no resource", () => { + beforeEach(async () => { + await fetchMock.resolveSpecific( + ["http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1"], + createMockResponseFromString("http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1", JSON.stringify({ + resources: [], + })), + ); + }); + + it("requests resources from the second versioned api group from the initial apiBase", () => { + expect(fetchMock.mock.lastCall).toMatchObject([ + "http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1beta1", + { + headers: { + "content-type": "application/json", + }, + method: "get", + }, + ]); + }); + + + + describe("when resource request fufills with a resource", () => { + beforeEach(async () => { + await fetchMock.resolveSpecific( + ["http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1beta1"], + createMockResponseFromString("http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1beta1", JSON.stringify({ + resources: [{ + name: "ingresses", + }], + })), + ); + }); + + it("makes the request to get the resource", () => { + expect(fetchMock.mock.lastCall).toMatchObject([ + "http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1beta1/namespaces/default/ingresses/foo", + { + headers: { + "content-type": "application/json", + }, + method: "get", + }, + ]); + }); + + it("sets fields in the api instance", () => { + expect(ingressApi).toEqual(expect.objectContaining({ + apiVersionPreferred: "v1beta1", + apiPrefix: "/apis", + apiGroup: "networking.k8s.io", + })); + }); + + it("registers the api with the changes info", () => { + expect(registerApiSpy).toBeCalledWith(ingressApi); + }); + + describe("when the request resolves with no data", () => { + let result: Ingress | null; + + beforeEach(async () => { + await fetchMock.resolveSpecific( + ["http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1beta1/namespaces/default/ingresses/foo"], + createMockResponseFromString("http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1beta1/namespaces/default/ingresses/foo", JSON.stringify({})), + ); + result = await getCall; + }); + + it("results in the get call resolving to null", () => { + expect(result).toBeNull(); + }); + + describe("on the second call to IngressApi.get()", () => { + let getCall: Promise; + + beforeEach(async () => { + getCall = ingressApi.get({ + name: "foo1", + namespace: "default", + }); + + // This is needed because of how JS promises work + await flushPromises(); + }); + + it("makes the request to get the resource", () => { + expect(fetchMock.mock.lastCall).toMatchObject([ + "http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1beta1/namespaces/default/ingresses/foo1", + { + headers: { + "content-type": "application/json", + }, + method: "get", + }, + ]); + }); + + describe("when the request resolves with no data", () => { + let result: Ingress | null; + + beforeEach(async () => { + await fetchMock.resolveSpecific( + ["http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1beta1/namespaces/default/ingresses/foo1"], + createMockResponseFromString("http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1beta1/namespaces/default/ingresses/foo1", JSON.stringify({})), + ); + result = await getCall; + }); + + it("results in the get call resolving to null", () => { + expect(result).toBeNull(); + }); + }); + }); + }); + + describe("when the request resolves with data", () => { + let result: Ingress | null; + + beforeEach(async () => { + await fetchMock.resolveSpecific( + ["http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1beta1/namespaces/default/ingresses/foo"], + createMockResponseFromString("http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1beta1/namespaces/default/ingresses/foo", JSON.stringify({ + apiVersion: "v1", + kind: "Ingress", + metadata: { + name: "foo", + namespace: "default", + resourceVersion: "1", + uid: "12345", + }, + })), + ); + result = await getCall; + }); + + it("results in the get call resolving to an instance", () => { + expect(result).toBeInstanceOf(Ingress); + }); + + describe("on the second call to IngressApi.get()", () => { + let getCall: Promise; + + beforeEach(async () => { + getCall = ingressApi.get({ + name: "foo1", + namespace: "default", + }); + + // This is needed because of how JS promises work + await flushPromises(); + }); + + it("makes the request to get the resource", () => { + expect(fetchMock.mock.lastCall).toMatchObject([ + "http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1beta1/namespaces/default/ingresses/foo1", + { + headers: { + "content-type": "application/json", + }, + method: "get", + }, + ]); + }); + + describe("when the request resolves with no data", () => { + let result: Ingress | null; + + beforeEach(async () => { + await fetchMock.resolveSpecific( + ["http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1beta1/namespaces/default/ingresses/foo1"], + createMockResponseFromString("http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1beta1/namespaces/default/ingresses/foo1", JSON.stringify({})), + ); + result = await getCall; + }); + + it("results in the get call resolving to null", () => { + expect(result).toBeNull(); + }); + }); + }); + }); + }); + }); + }); + + describe("when the version list from the api group resolves with no versions", () => { + beforeEach(async () => { + await fetchMock.resolveSpecific( + ["http://127.0.0.1:9999/api-kube/apis/networking.k8s.io"], + createMockResponseFromString("http://127.0.0.1:9999/api-kube/apis/networking.k8s.io", JSON.stringify({ + "metadata": {}, + "status": "Failure", + "message": "the server could not find the requested resource", + "reason": "NotFound", + "details": { + "causes": [ + { + "reason": "UnexpectedServerResponse", + "message": "404 page not found", + }, + ], + }, + "code": 404, + }), 404), + ); + }); + + it("requests the resources from the base api url from the fallback api", () => { + expect(fetchMock.mock.lastCall).toMatchObject([ + "http://127.0.0.1:9999/api-kube/apis/extensions", + { + headers: { + "content-type": "application/json", + }, + method: "get", + }, + ]); + }); + + describe("when resource request fufills with a resource", () => { + beforeEach(async () => { + await fetchMock.resolveSpecific( + ["http://127.0.0.1:9999/api-kube/apis/extensions"], + createMockResponseFromString("http://127.0.0.1:9999/api-kube/apis/extensions", JSON.stringify({ + apiVersion: "v1", + kind: "APIGroup", + name: "extensions", + versions: [ + { + groupVersion: "extensions/v1beta1", + version: "v1beta1", + }, + ], + preferredVersion: { + groupVersion: "extensions/v1beta1", + version: "v1beta1", + }, + })), + ); + }); + + it("requests resource versions from the versioned api group from the fallback apiBase", () => { + expect(fetchMock.mock.lastCall).toMatchObject([ + "http://127.0.0.1:9999/api-kube/apis/extensions/v1beta1", + { + headers: { + "content-type": "application/json", + }, + method: "get", + }, + ]); + }); + + describe("when the preferred version request resolves to v1beta1", () => { + beforeEach(async () => { + await fetchMock.resolveSpecific( + ["http://127.0.0.1:9999/api-kube/apis/extensions/v1beta1"], + createMockResponseFromString("http://127.0.0.1:9999/api-kube/apis/extensions", JSON.stringify({ + resources: [{ + name: "ingresses", + }], + })), + ); + }); + + it("makes the request to get the resource", () => { + expect(fetchMock.mock.lastCall).toMatchObject([ + "http://127.0.0.1:9999/api-kube/apis/extensions/v1beta1/namespaces/default/ingresses/foo", + { + headers: { + "content-type": "application/json", + }, + method: "get", + }, + ]); + }); + + it("sets fields in the api instance", () => { + expect(ingressApi).toEqual(expect.objectContaining({ + apiVersionPreferred: "v1beta1", + apiPrefix: "/apis", + apiGroup: "extensions", + })); + }); + + it("registers the api with the changes info", () => { + expect(registerApiSpy).toBeCalledWith(ingressApi); + }); + + describe("when the request resolves with no data", () => { + let result: Ingress | null; + + beforeEach(async () => { + await fetchMock.resolveSpecific( + ["http://127.0.0.1:9999/api-kube/apis/extensions/v1beta1/namespaces/default/ingresses/foo"], + createMockResponseFromString("http://127.0.0.1:9999/api-kube/apis/extensions/v1beta1/namespaces/default/ingresses/foo", JSON.stringify({})), + ); + result = await getCall; + }); + + it("results in the get call resolving to null", () => { + expect(result).toBeNull(); + }); + + describe("on the second call to IngressApi.get()", () => { + let getCall: Promise; + + beforeEach(async () => { + getCall = ingressApi.get({ + name: "foo1", + namespace: "default", + }); + + // This is needed because of how JS promises work + await flushPromises(); + }); + + it("makes the request to get the resource", () => { + expect(fetchMock.mock.lastCall).toMatchObject([ + "http://127.0.0.1:9999/api-kube/apis/extensions/v1beta1/namespaces/default/ingresses/foo1", + { + headers: { + "content-type": "application/json", + }, + method: "get", + }, + ]); + }); + + describe("when the request resolves with no data", () => { + let result: Ingress | null; + + beforeEach(async () => { + await fetchMock.resolveSpecific( + ["http://127.0.0.1:9999/api-kube/apis/extensions/v1beta1/namespaces/default/ingresses/foo1"], + createMockResponseFromString("http://127.0.0.1:9999/api-kube/apis/extensions/v1beta1/namespaces/default/ingresses/foo1", JSON.stringify({})), + ); + result = await getCall; + }); + + it("results in the get call resolving to null", () => { + expect(result).toBeNull(); + }); + }); + }); + }); + + describe("when the request resolves with data", () => { + let result: Ingress | null; + + beforeEach(async () => { + await fetchMock.resolveSpecific( + ["http://127.0.0.1:9999/api-kube/apis/extensions/v1beta1/namespaces/default/ingresses/foo"], + createMockResponseFromString("http://127.0.0.1:9999/api-kube/apis/extensions/v1beta1/namespaces/default/ingresses/foo", JSON.stringify({ + apiVersion: "v1beta1", + kind: "Ingress", + metadata: { + name: "foo", + namespace: "default", + resourceVersion: "1", + uid: "12345", + }, + })), + ); + result = await getCall; + }); + + it("results in the get call resolving to an instance", () => { + expect(result).toBeInstanceOf(Ingress); + }); + + describe("on the second call to IngressApi.get()", () => { + let getCall: Promise; + + beforeEach(async () => { + getCall = ingressApi.get({ + name: "foo1", + namespace: "default", + }); + + // This is needed because of how JS promises work + await flushPromises(); + }); + + it("makes the request to get the resource", () => { + expect(fetchMock.mock.lastCall).toMatchObject([ + "http://127.0.0.1:9999/api-kube/apis/extensions/v1beta1/namespaces/default/ingresses/foo1", + { + headers: { + "content-type": "application/json", + }, + method: "get", + }, + ]); + }); + + describe("when the request resolves with no data", () => { + let result: Ingress | null; + + beforeEach(async () => { + await fetchMock.resolveSpecific( + ["http://127.0.0.1:9999/api-kube/apis/extensions/v1beta1/namespaces/default/ingresses/foo1"], + createMockResponseFromString("http://127.0.0.1:9999/api-kube/apis/extensions/v1beta1/namespaces/default/ingresses/foo1", JSON.stringify({})), + ); + result = await getCall; + }); + + it("results in the get call resolving to null", () => { + expect(result).toBeNull(); + }); + }); + }); + }); + }); + }); + }); + }); +}); diff --git a/src/common/k8s-api/__tests__/kube-api.test.ts b/src/common/k8s-api/__tests__/kube-api.test.ts index 2e681d4ba9..9244d5e6ab 100644 --- a/src/common/k8s-api/__tests__/kube-api.test.ts +++ b/src/common/k8s-api/__tests__/kube-api.test.ts @@ -6,10 +6,8 @@ import type { KubeApiWatchCallback } from "../kube-api"; import { KubeApi } from "../kube-api"; import type { KubeJsonApi, KubeJsonApiData } from "../kube-json-api"; import { PassThrough } from "stream"; -import type { ApiManager } from "../api-manager"; -import { Deployment, DeploymentApi, Ingress, IngressApi, NamespaceApi, Pod, PodApi } from "../endpoints"; +import { Deployment, DeploymentApi, NamespaceApi, Pod, PodApi } from "../endpoints"; import { getDiForUnitTesting } from "../../../renderer/getDiForUnitTesting"; -import apiManagerInjectable from "../api-manager/manager.injectable"; import autoRegistrationInjectable from "../api-manager/auto-registration.injectable"; import type { Fetch } from "../../fetch/fetch.injectable"; import fetchInjectable from "../../fetch/fetch.injectable"; @@ -171,7 +169,6 @@ describe("createKubeApiForRemoteCluster", () => { describe("KubeApi", () => { let request: KubeJsonApi; - let registerApiSpy: jest.SpiedFunction; let fetchMock: AsyncFnMock; beforeEach(async () => { @@ -186,442 +183,10 @@ describe("KubeApi", () => { serverAddress: `http://127.0.0.1:9999`, apiBase: "/api-kube", }); - registerApiSpy = jest.spyOn(di.inject(apiManagerInjectable), "registerApi"); di.inject(autoRegistrationInjectable); }); - describe("on first call to IngressApi.get()", () => { - let ingressApi: IngressApi; - let getCall: Promise; - - beforeEach(async () => { - ingressApi = new IngressApi({ - request, - objectConstructor: Ingress, - apiBase: "/apis/networking.k8s.io/v1/ingresses", - fallbackApiBases: ["/apis/extensions/v1beta1/ingresses"], - checkPreferredVersion: true, - }); - getCall = ingressApi.get({ - name: "foo", - namespace: "default", - }); - - // This is needed because of how JS promises work - await flushPromises(); - }); - - it("requests resources from the versioned api group from the initial apiBase", () => { - expect(fetchMock.mock.lastCall).toMatchObject([ - "http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1", - { - headers: { - "content-type": "application/json", - }, - method: "get", - }, - ]); - }); - - describe("when resource request fufills with a resource", () => { - beforeEach(async () => { - await fetchMock.resolveSpecific( - ["http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1"], - createMockResponseFromString("http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1", JSON.stringify({ - resources: [{ - name: "ingresses", - }], - })), - ); - }); - - it("requests the perferred version of that api group", () => { - expect(fetchMock.mock.lastCall).toMatchObject([ - "http://127.0.0.1:9999/api-kube/apis/networking.k8s.io", - { - headers: { - "content-type": "application/json", - }, - method: "get", - }, - ]); - }); - - describe("when the preferred version resolves with v1", () => { - beforeEach(async () => { - await fetchMock.resolveSpecific( - ["http://127.0.0.1:9999/api-kube/apis/networking.k8s.io"], - createMockResponseFromString("http://127.0.0.1:9999/api-kube/apis/networking.k8s.io", JSON.stringify({ - preferredVersion: { - version: "v1", - }, - })), - ); - }); - - it("makes the request to get the resource", () => { - expect(fetchMock.mock.lastCall).toMatchObject([ - "http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1/namespaces/default/ingresses/foo", - { - headers: { - "content-type": "application/json", - }, - method: "get", - }, - ]); - }); - - it("sets fields in the api instance", () => { - expect(ingressApi).toEqual(expect.objectContaining({ - apiVersionPreferred: "v1", - apiPrefix: "/apis", - apiGroup: "networking.k8s.io", - })); - }); - - it("registers the api with the changes info", () => { - expect(registerApiSpy).toBeCalledWith(ingressApi); - }); - - describe("when the request resolves with no data", () => { - let result: Ingress | null; - - beforeEach(async () => { - await fetchMock.resolveSpecific( - ["http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1/namespaces/default/ingresses/foo"], - createMockResponseFromString("http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1/namespaces/default/ingresses/foo", JSON.stringify({})), - ); - result = await getCall; - }); - - it("results in the get call resolving to null", () => { - expect(result).toBeNull(); - }); - - describe("on the second call to IngressApi.get()", () => { - let getCall: Promise; - - beforeEach(async () => { - getCall = ingressApi.get({ - name: "foo1", - namespace: "default", - }); - - // This is needed because of how JS promises work - await flushPromises(); - }); - - it("makes the request to get the resource", () => { - expect(fetchMock.mock.lastCall).toMatchObject([ - "http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1/namespaces/default/ingresses/foo1", - { - headers: { - "content-type": "application/json", - }, - method: "get", - }, - ]); - }); - - describe("when the request resolves with no data", () => { - let result: Ingress | null; - - beforeEach(async () => { - await fetchMock.resolveSpecific( - ["http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1/namespaces/default/ingresses/foo1"], - createMockResponseFromString("http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1/namespaces/default/ingresses/foo1", JSON.stringify({})), - ); - result = await getCall; - }); - - it("results in the get call resolving to null", () => { - expect(result).toBeNull(); - }); - }); - }); - }); - - describe("when the request resolves with data", () => { - let result: Ingress | null; - - beforeEach(async () => { - await fetchMock.resolveSpecific( - ["http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1/namespaces/default/ingresses/foo"], - createMockResponseFromString("http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1/namespaces/default/ingresses/foo", JSON.stringify({ - apiVersion: "v1", - kind: "Ingress", - metadata: { - name: "foo", - namespace: "default", - resourceVersion: "1", - uid: "12345", - }, - })), - ); - result = await getCall; - }); - - it("results in the get call resolving to an instance", () => { - expect(result).toBeInstanceOf(Ingress); - }); - - describe("on the second call to IngressApi.get()", () => { - let getCall: Promise; - - beforeEach(async () => { - getCall = ingressApi.get({ - name: "foo1", - namespace: "default", - }); - - // This is needed because of how JS promises work - await flushPromises(); - }); - - it("makes the request to get the resource", () => { - expect(fetchMock.mock.lastCall).toMatchObject([ - "http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1/namespaces/default/ingresses/foo1", - { - headers: { - "content-type": "application/json", - }, - method: "get", - }, - ]); - }); - - describe("when the request resolves with no data", () => { - let result: Ingress | null; - - beforeEach(async () => { - await fetchMock.resolveSpecific( - ["http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1/namespaces/default/ingresses/foo1"], - createMockResponseFromString("http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1/namespaces/default/ingresses/foo1", JSON.stringify({})), - ); - result = await getCall; - }); - - it("results in the get call resolving to null", () => { - expect(result).toBeNull(); - }); - }); - }); - }); - }); - }); - - describe("when resource request fufills with no resource", () => { - beforeEach(async () => { - await fetchMock.resolveSpecific( - ["http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1"], - createMockResponseFromString("http://127.0.0.1:9999/api-kube/apis/networking.k8s.io/v1", JSON.stringify({ - resources: [], - })), - ); - }); - - it("requests the resources from the base api url from the fallback api", () => { - expect(fetchMock.mock.lastCall).toMatchObject([ - "http://127.0.0.1:9999/api-kube/apis/extensions/v1beta1", - { - headers: { - "content-type": "application/json", - }, - method: "get", - }, - ]); - }); - - describe("when resource request fufills with a resource", () => { - beforeEach(async () => { - await fetchMock.resolveSpecific( - ["http://127.0.0.1:9999/api-kube/apis/extensions/v1beta1"], - createMockResponseFromString("http://127.0.0.1:9999/api-kube/apis/extensions/v1beta1", JSON.stringify({ - resources: [{ - name: "ingresses", - }], - })), - ); - }); - - it("requests the preferred version for that api group", () => { - expect(fetchMock.mock.lastCall).toMatchObject([ - "http://127.0.0.1:9999/api-kube/apis/extensions", - { - headers: { - "content-type": "application/json", - }, - method: "get", - }, - ]); - }); - - describe("when the preferred version request resolves to v1beta1", () => { - beforeEach(async () => { - await fetchMock.resolveSpecific( - ["http://127.0.0.1:9999/api-kube/apis/extensions"], - createMockResponseFromString("http://127.0.0.1:9999/api-kube/apis/extensions", JSON.stringify({ - preferredVersion: { - version: "v1beta1", - }, - })), - ); - }); - - it("makes the request to get the resource", () => { - expect(fetchMock.mock.lastCall).toMatchObject([ - "http://127.0.0.1:9999/api-kube/apis/extensions/v1beta1/namespaces/default/ingresses/foo", - { - headers: { - "content-type": "application/json", - }, - method: "get", - }, - ]); - }); - - it("sets fields in the api instance", () => { - expect(ingressApi).toEqual(expect.objectContaining({ - apiVersionPreferred: "v1beta1", - apiPrefix: "/apis", - apiGroup: "extensions", - })); - }); - - it("registers the api with the changes info", () => { - expect(registerApiSpy).toBeCalledWith(ingressApi); - }); - - describe("when the request resolves with no data", () => { - let result: Ingress | null; - - beforeEach(async () => { - await fetchMock.resolveSpecific( - ["http://127.0.0.1:9999/api-kube/apis/extensions/v1beta1/namespaces/default/ingresses/foo"], - createMockResponseFromString("http://127.0.0.1:9999/api-kube/apis/extensions/v1beta1/namespaces/default/ingresses/foo", JSON.stringify({})), - ); - result = await getCall; - }); - - it("results in the get call resolving to null", () => { - expect(result).toBeNull(); - }); - - describe("on the second call to IngressApi.get()", () => { - let getCall: Promise; - - beforeEach(async () => { - getCall = ingressApi.get({ - name: "foo1", - namespace: "default", - }); - - // This is needed because of how JS promises work - await flushPromises(); - }); - - it("makes the request to get the resource", () => { - expect(fetchMock.mock.lastCall).toMatchObject([ - "http://127.0.0.1:9999/api-kube/apis/extensions/v1beta1/namespaces/default/ingresses/foo1", - { - headers: { - "content-type": "application/json", - }, - method: "get", - }, - ]); - }); - - describe("when the request resolves with no data", () => { - let result: Ingress | null; - - beforeEach(async () => { - await fetchMock.resolveSpecific( - ["http://127.0.0.1:9999/api-kube/apis/extensions/v1beta1/namespaces/default/ingresses/foo1"], - createMockResponseFromString("http://127.0.0.1:9999/api-kube/apis/extensions/v1beta1/namespaces/default/ingresses/foo1", JSON.stringify({})), - ); - result = await getCall; - }); - - it("results in the get call resolving to null", () => { - expect(result).toBeNull(); - }); - }); - }); - }); - - describe("when the request resolves with data", () => { - let result: Ingress | null; - - beforeEach(async () => { - await fetchMock.resolveSpecific( - ["http://127.0.0.1:9999/api-kube/apis/extensions/v1beta1/namespaces/default/ingresses/foo"], - createMockResponseFromString("http://127.0.0.1:9999/api-kube/apis/extensions/v1beta1/namespaces/default/ingresses/foo", JSON.stringify({ - apiVersion: "v1beta1", - kind: "Ingress", - metadata: { - name: "foo", - namespace: "default", - resourceVersion: "1", - uid: "12345", - }, - })), - ); - result = await getCall; - }); - - it("results in the get call resolving to an instance", () => { - expect(result).toBeInstanceOf(Ingress); - }); - - describe("on the second call to IngressApi.get()", () => { - let getCall: Promise; - - beforeEach(async () => { - getCall = ingressApi.get({ - name: "foo1", - namespace: "default", - }); - - // This is needed because of how JS promises work - await flushPromises(); - }); - - it("makes the request to get the resource", () => { - expect(fetchMock.mock.lastCall).toMatchObject([ - "http://127.0.0.1:9999/api-kube/apis/extensions/v1beta1/namespaces/default/ingresses/foo1", - { - headers: { - "content-type": "application/json", - }, - method: "get", - }, - ]); - }); - - describe("when the request resolves with no data", () => { - let result: Ingress | null; - - beforeEach(async () => { - await fetchMock.resolveSpecific( - ["http://127.0.0.1:9999/api-kube/apis/extensions/v1beta1/namespaces/default/ingresses/foo1"], - createMockResponseFromString("http://127.0.0.1:9999/api-kube/apis/extensions/v1beta1/namespaces/default/ingresses/foo1", JSON.stringify({})), - ); - result = await getCall; - }); - - it("results in the get call resolving to null", () => { - expect(result).toBeNull(); - }); - }); - }); - }); - }); - }); - }); - }); - describe("patching deployments", () => { let api: DeploymentApi; diff --git a/src/common/k8s-api/endpoints/cron-job.api.injectable.ts b/src/common/k8s-api/endpoints/cron-job.api.injectable.ts index 252dcd624e..e2230ee2db 100644 --- a/src/common/k8s-api/endpoints/cron-job.api.injectable.ts +++ b/src/common/k8s-api/endpoints/cron-job.api.injectable.ts @@ -14,9 +14,6 @@ const cronJobApiInjectable = getInjectable({ assert(di.inject(storesAndApisCanBeCreatedInjectionToken), "cronJobApi is only available in certain environments"); return new CronJobApi({ - fallbackApiBases: [ - "/apis/batch/v1beta1/cronjobs", - ], checkPreferredVersion: true, }); }, diff --git a/src/common/k8s-api/endpoints/job.api.injectable.ts b/src/common/k8s-api/endpoints/job.api.injectable.ts index a9c4252e59..fc25c8c61f 100644 --- a/src/common/k8s-api/endpoints/job.api.injectable.ts +++ b/src/common/k8s-api/endpoints/job.api.injectable.ts @@ -13,7 +13,9 @@ const jobApiInjectable = getInjectable({ instantiate: (di) => { assert(di.inject(storesAndApisCanBeCreatedInjectionToken), "jobApi is only available in certain environments"); - return new JobApi(); + return new JobApi({ + checkPreferredVersion: true, + }); }, injectionToken: kubeApiInjectionToken, diff --git a/src/common/k8s-api/kube-api-parse.ts b/src/common/k8s-api/kube-api-parse.ts index 509f1c28e9..dcb8b18636 100644 --- a/src/common/k8s-api/kube-api-parse.ts +++ b/src/common/k8s-api/kube-api-parse.ts @@ -17,6 +17,7 @@ export interface IKubeApiLinkRef { export interface IKubeApiParsed extends IKubeApiLinkRef { apiBase: string; + apiPrefix: string; apiGroup: string; apiVersionWithGroup: string; } diff --git a/src/common/k8s-api/kube-api.ts b/src/common/k8s-api/kube-api.ts index ae06341d1b..593a1de38c 100644 --- a/src/common/k8s-api/kube-api.ts +++ b/src/common/k8s-api/kube-api.ts @@ -19,12 +19,14 @@ import type { RequestInit, Response } from "node-fetch"; import type { Patch } from "rfc6902"; import assert from "assert"; import type { PartialDeep } from "type-fest"; -import logger from "../logger"; +import type { Logger } from "../logger"; import { Environments, getEnvironmentSpecificLegacyGlobalDiForExtensionApi } from "../../extensions/as-legacy-globals-for-extension-api/legacy-global-di-for-extension-api"; import autoRegistrationEmitterInjectable from "./api-manager/auto-registration-emitter.injectable"; import { asLegacyGlobalForExtensionApi } from "../../extensions/as-legacy-globals-for-extension-api/as-legacy-global-object-for-extension-api"; import { apiKubeInjectionToken } from "./api-kube"; import type AbortController from "abort-controller"; +import loggerInjectable from "../logger.injectable"; +import { matches } from "lodash/fp"; /** * The options used for creating a `KubeApi` @@ -142,6 +144,26 @@ export interface KubeApiResourceList { resources: KubeApiResource[]; } +export interface KubeApiResourceVersion { + groupVersion: string; + version: string; +} + +export interface KubeApiResourceVersionList { + apiVersion: string; + kind: string; + name: string; + preferredVersion: KubeApiResourceVersion; + versions: KubeApiResourceVersion[]; +} + +const not = (fn: (val: T) => boolean) => (val: T) => !(fn(val)); + +const getOrderedVersions = (list: KubeApiResourceVersionList): KubeApiResourceVersion[] => [ + list.preferredVersion, + ...list.versions.filter(not(matches(list.preferredVersion))), +]; + export type PropagationPolicy = undefined | "Orphan" | "Foreground" | "Background"; export type KubeApiWatchCallback = (data: IKubeWatchEvent | null, error: KubeStatus | Response | null | any) => void; @@ -233,6 +255,10 @@ function legacyRegisterApi(api: KubeApi): void { } } +export interface KubeApiDependencies { + readonly logger: Logger; +} + export class KubeApi< Object extends KubeObject = KubeObject, Data extends KubeJsonApiDataFor = KubeJsonApiDataFor, @@ -255,6 +281,8 @@ export class KubeApi< protected readonly fullApiPathname: string; protected readonly fallbackApiBases: string[] | undefined; + protected readonly dependencies: KubeApiDependencies; + constructor(opts: KubeApiOptions) { const { objectConstructor, @@ -287,6 +315,10 @@ export class KubeApi< this.request = request; this.objectConstructor = objectConstructor; legacyRegisterApi(this); + + this.dependencies = { + logger: asLegacyGlobalForExtensionApi(loggerInjectable), + }; } get apiVersionWithGroup() { @@ -310,15 +342,20 @@ export class KubeApi< for (const apiUrl of apiBases) { try { - // Split e.g. "/apis/extensions/v1beta1/ingresses" to parts - const { apiPrefix, apiGroup, apiVersionWithGroup, resource } = parseKubeApi(apiUrl); + const { apiPrefix, apiGroup, resource } = parseKubeApi(apiUrl); + const list = await this.request.get(`${apiPrefix}/${apiGroup}`) as KubeApiResourceVersionList; + const resourceVersions = getOrderedVersions(list); - // Request available resources - const { resources } = (await this.request.get(`${apiPrefix}/${apiVersionWithGroup}`)) as unknown as KubeApiResourceList; + for (const resourceVersion of resourceVersions) { + const { resources } = await this.request.get(`${apiPrefix}/${resourceVersion.groupVersion}`) as KubeApiResourceList; - // If the resource is found in the group, use this apiUrl - if (resources.find(({ name }) => name === resource)) { - return { apiPrefix, apiGroup }; + if (resources.some(({ name }) => name === resource)) { + return { + apiPrefix, + apiGroup, + apiVersionPreferred: resourceVersion.version, + }; + } } } catch (error) { // Exception is ignored as we can try the next url @@ -328,48 +365,19 @@ export class KubeApi< throw new Error(`Can't find working API for the Kubernetes resource ${this.apiResource}`); } - /** - * Get the apiPrefix and apiGroup to be used for fetching the preferred version. - */ - private async getPreferredVersionPrefixGroup() { - if (this.fallbackApiBases) { - try { - return await this.getLatestApiPrefixGroup(); - } catch (error) { - // If valid API wasn't found, log the error and return defaults below - logger.error(`[KUBE-API]: ${error}`); - } - } - - return { - apiPrefix: this.apiPrefix, - apiGroup: this.apiGroup, - }; - } - protected async checkPreferredVersion() { if (this.fallbackApiBases && !this.doCheckPreferredVersion) { throw new Error("checkPreferredVersion must be enabled if fallbackApiBases is set in KubeApi"); } if (this.doCheckPreferredVersion && this.apiVersionPreferred === undefined) { - const { apiPrefix, apiGroup } = await this.getPreferredVersionPrefixGroup(); + const { apiPrefix, apiGroup, apiVersionPreferred } = await this.getLatestApiPrefixGroup(); - assert(apiPrefix); - - // The apiPrefix and apiGroup might change due to fallbackApiBases, so we must override them this.apiPrefix = apiPrefix; this.apiGroup = apiGroup; - - const url = [apiPrefix, apiGroup].filter(Boolean).join("/"); - const res = await this.request.get(url) as IKubePreferredVersion; - - this.apiVersionPreferred = res?.preferredVersion?.version; - - if (this.apiVersionPreferred) { - this.apiBase = this.computeApiBase(); - legacyRegisterApi(this); - } + this.apiVersionPreferred = apiVersionPreferred; + this.apiBase = this.computeApiBase(); + legacyRegisterApi(this); } } @@ -639,7 +647,7 @@ export class KubeApi< const abortController = new WrappedAbortController(opts?.abortController); abortController.signal.addEventListener("abort", () => { - logger.info(`[KUBE-API] watch (${watchId}) aborted ${watchUrl}`); + this.dependencies.logger.info(`[KUBE-API] watch (${watchId}) aborted ${watchUrl}`); clearTimeout(timedRetry); }); @@ -651,7 +659,7 @@ export class KubeApi< signal: abortController.signal, }); - logger.info(`[KUBE-API] watch (${watchId}) ${retry === true ? "retried" : "started"} ${watchUrl}`); + this.dependencies.logger.info(`[KUBE-API] watch (${watchId}) ${retry === true ? "retried" : "started"} ${watchUrl}`); responsePromise .then(response => { @@ -659,7 +667,7 @@ export class KubeApi< let requestRetried = false; if (!response.ok) { - logger.warn(`[KUBE-API] watch (${watchId}) error response ${watchUrl}`, { status: response.status }); + this.dependencies.logger.warn(`[KUBE-API] watch (${watchId}) error response ${watchUrl}`, { status: response.status }); return callback(null, response); } @@ -676,7 +684,7 @@ export class KubeApi< // Close current request abortController.abort(); - logger.info(`[KUBE-API] Watch timeout set, but not retried, retrying now`); + this.dependencies.logger.info(`[KUBE-API] Watch timeout set, but not retried, retrying now`); requestRetried = true; @@ -688,7 +696,7 @@ export class KubeApi< } if (!response.body) { - logger.error(`[KUBE-API]: watch (${watchId}) did not return a body`); + this.dependencies.logger.error(`[KUBE-API]: watch (${watchId}) did not return a body`); requestRetried = true; clearTimeout(timedRetry); @@ -707,7 +715,7 @@ export class KubeApi< return; } - logger.info(`[KUBE-API] watch (${watchId}) ${eventName} ${watchUrl}`); + this.dependencies.logger.info(`[KUBE-API] watch (${watchId}) ${eventName} ${watchUrl}`); requestRetried = true; @@ -736,8 +744,9 @@ export class KubeApi< }); }) .catch(error => { - logger.error(`[KUBE-API] watch (${watchId}) throwed ${watchUrl}`, error); - + if (!abortController.signal.aborted) { + this.dependencies.logger.error(`[KUBE-API] watch (${watchId}) threw ${watchUrl}`, error); + } callback(null, error); }); diff --git a/src/common/k8s-api/kube-object.store.ts b/src/common/k8s-api/kube-object.store.ts index 12456aa20c..309c183e42 100644 --- a/src/common/k8s-api/kube-object.store.ts +++ b/src/common/k8s-api/kube-object.store.ts @@ -221,7 +221,7 @@ export abstract class KubeObjectStore< try { return await res ?? []; } catch (error) { - onLoadFailure(String(error)); + onLoadFailure(new Error(`Failed to load ${this.api.apiBase}`, { cause: error })); // reset the store because we are loading all, so that nothing is displayed this.items.clear(); @@ -249,7 +249,7 @@ export abstract class KubeObjectStore< case "rejected": if (onLoadFailure) { - onLoadFailure(result.reason.message || result.reason); + onLoadFailure(new Error(`Failed to load ${this.api.apiBase}`, { cause: result.reason })); } else { // if onLoadFailure is not provided then preserve old behaviour throw result.reason; diff --git a/src/renderer/components/kube-object-list-layout/kube-object-list-layout.tsx b/src/renderer/components/kube-object-list-layout/kube-object-list-layout.tsx index 03ffef3199..707351c231 100644 --- a/src/renderer/components/kube-object-list-layout/kube-object-list-layout.tsx +++ b/src/renderer/components/kube-object-list-layout/kube-object-list-layout.tsx @@ -49,6 +49,18 @@ interface Dependencies { toggleKubeDetailsPane: ToggleKubeDetailsPane; } +const getLoadErrorMessage = (error: unknown): string => { + if (error instanceof Error) { + if (error.cause) { + return `${error.message}: ${getLoadErrorMessage(error.cause)}`; + } + + return error.message; + } + + return `${error}`; +}; + @observer class NonInjectedKubeObjectListLayout< K extends KubeObject, @@ -59,7 +71,7 @@ class NonInjectedKubeObjectListLayout< subscribeStores: true, }; - private loadErrors = observable.array(); + private readonly loadErrors = observable.array(); @computed get selectedItem() { return this.props.store.getByPath(this.props.kubeSelectedUrlParam.get()); @@ -78,7 +90,9 @@ class NonInjectedKubeObjectListLayout< if (subscribeStores) { reactions.push( this.props.subscribeToStores(stores, { - onLoadFailure: error => this.loadErrors.push(String(error)), + onLoadFailure: error => { + this.loadErrors.push(getLoadErrorMessage(error)); + }, }), ); }