diff --git a/packages/core/src/common/k8s-api/__tests__/kube-api-parse.test.ts b/packages/core/src/common/k8s-api/__tests__/kube-api-parse.test.ts index 547f78adef..6d05385a28 100644 --- a/packages/core/src/common/k8s-api/__tests__/kube-api-parse.test.ts +++ b/packages/core/src/common/k8s-api/__tests__/kube-api-parse.test.ts @@ -114,7 +114,7 @@ const tests: KubeApiParseTestData[] = [ }], ]; -const throwtests = [ +const invalidTests = [ undefined, "", "ajklsmh", @@ -125,7 +125,7 @@ describe("parseApi unit tests", () => { expect(parseKubeApi(url)).toStrictEqual(expected); }); - it.each(throwtests)("testing %j should throw", (url) => { - expect(() => parseKubeApi(url as never)).toThrowError("invalid apiPath"); + it.each(invalidTests)("testing %j should throw", (url) => { + expect(parseKubeApi(url as never)).toBe(undefined); }); }); 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 23574822ed..8e0eff8255 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 @@ -74,9 +74,13 @@ export class ApiManager { return iter.find(this.apis.values(), pathOrCallback); } - const { apiBase } = parseKubeApi(pathOrCallback); + const parsedApi = parseKubeApi(pathOrCallback); - return this.apis.get(apiBase); + if (!parsedApi) { + return undefined; + } + + return this.apis.get(parsedApi.apiBase); } getApiByKind(kind: string, apiVersion: string) { @@ -141,9 +145,10 @@ export class ApiManager { } const { apiBase } = typeof apiOrBase === "string" - ? parseKubeApi(apiOrBase) + ? parseKubeApi(apiOrBase) ?? {} : apiOrBase; - const api = this.getApi(apiBase); + + const api = apiBase && this.getApi(apiBase); if (!api) { return undefined; diff --git a/packages/core/src/common/k8s-api/kube-api-parse.ts b/packages/core/src/common/k8s-api/kube-api-parse.ts index 522e2812b8..e69d3ffa69 100644 --- a/packages/core/src/common/k8s-api/kube-api-parse.ts +++ b/packages/core/src/common/k8s-api/kube-api-parse.ts @@ -22,16 +22,16 @@ export interface IKubeApiParsed extends IKubeApiLinkRef { apiVersionWithGroup: string; } -export function parseKubeApi(path: string): IKubeApiParsed { +export function parseKubeApi(path: string): IKubeApiParsed | undefined { const apiPath = new URL(path, "https://localhost").pathname; const [, prefix, ...parts] = apiPath.split("/"); const apiPrefix = `/${prefix}`; const [left, right, namespaced] = array.split(parts, "namespaces"); - let apiGroup!: string; - let apiVersion!: string; - let namespace!: string; - let resource!: string; - let name!: string; + let apiGroup: string; + let apiVersion: string | undefined; + let namespace: string | undefined; + let resource: string; + let name: string | undefined; if (namespaced) { switch (right.length) { @@ -46,26 +46,21 @@ export function parseKubeApi(path: string): IKubeApiParsed { break; } - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - apiVersion = left.pop()!; - apiGroup = left.join("/"); + let rest: string[]; + + [apiVersion, ...rest] = left; + apiGroup = rest.join("/"); } else { - switch (left.length) { - case 0: - throw new Error(`invalid apiPath: ${apiPath}`); - case 4: - [apiGroup, apiVersion, resource, name] = left; - break; - case 2: - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - resource = left.pop()!; - // fallthrough - case 1: - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - apiVersion = left.pop()!; - apiGroup = ""; - break; - default: + if (left.length === 0) { + return undefined; + } + + if (left.length === 1 || left.length === 2) { + [apiVersion, resource] = left; + apiGroup = ""; + } else if (left.length === 4) { + [apiGroup, apiVersion, resource, name] = left; + } else { /** * Given that * - `apiVersion` is `GROUP/VERSION` and @@ -82,15 +77,14 @@ export function parseKubeApi(path: string): IKubeApiParsed { * 3. otherwise assume apiVersion <- left[0] * 4. always resource, name <- left[(0 or 1)+1..] */ - if (left[0].includes(".") || left[1].match(/^v[0-9]/)) { - [apiGroup, apiVersion] = left; - resource = left.slice(2).join("/"); - } else { - apiGroup = ""; - apiVersion = left[0]; - [resource, name] = left.slice(1); - } - break; + if (left[0].includes(".") || left[1].match(/^v[0-9]/)) { + [apiGroup, apiVersion] = left; + resource = left.slice(2).join("/"); + } else { + apiGroup = ""; + apiVersion = left[0]; + [resource, name] = left.slice(1); + } } } @@ -98,7 +92,7 @@ export function parseKubeApi(path: string): IKubeApiParsed { const apiBase = [apiPrefix, apiGroup, apiVersion, resource].filter(v => v).join("/"); if (!apiBase) { - throw new Error(`invalid apiPath: ${apiPath}`); + return undefined; } return { @@ -110,7 +104,7 @@ export function parseKubeApi(path: string): IKubeApiParsed { } function isIKubeApiParsed(refOrParsed: IKubeApiLinkRef | IKubeApiParsed): refOrParsed is IKubeApiParsed { - return "apiGroup" in refOrParsed; + return "apiGroup" in refOrParsed && !!refOrParsed.apiGroup; } export function createKubeApiURL(linkRef: IKubeApiLinkRef): string; diff --git a/packages/core/src/common/k8s-api/kube-api.ts b/packages/core/src/common/k8s-api/kube-api.ts index cf9d8f15a7..d10b5b3e51 100644 --- a/packages/core/src/common/k8s-api/kube-api.ts +++ b/packages/core/src/common/k8s-api/kube-api.ts @@ -264,12 +264,16 @@ export class KubeApi< allowedUsableVersions, } = opts; - assert(fullApiPathname, "apiBase MUST be provied either via KubeApiOptions.apiBase or KubeApiOptions.objectConstructor.apiBase"); + assert(fullApiPathname, "apiBase MUST be provided either via KubeApiOptions.apiBase or KubeApiOptions.objectConstructor.apiBase"); assert(request, "request MUST be provided if not in a cluster page frame context"); - const { apiBase, apiPrefix, apiGroup, apiVersion, resource } = parseKubeApi(fullApiPathname); + const parsedApi = parseKubeApi(fullApiPathname); - assert(kind, "kind MUST be provied either via KubeApiOptions.kind or KubeApiOptions.objectConstructor.kind"); + assert(parsedApi, "apiBase MUST be a valid kube api pathname"); + + const { apiBase, apiPrefix, apiGroup, apiVersion, resource } = parsedApi; + + assert(kind, "kind MUST be provided either via KubeApiOptions.kind or KubeApiOptions.objectConstructor.kind"); assert(apiPrefix, "apiBase MUST be parsable as a kubeApi selfLink style string"); this.doCheckPreferredVersion = doCheckPreferredVersion; @@ -308,24 +312,26 @@ export class KubeApi< const apiBases = new Set(rawApiBases); for (const apiUrl of apiBases) { - try { - const { apiPrefix, apiGroup, resource } = parseKubeApi(apiUrl); - const list = await this.request.get(`${apiPrefix}/${apiGroup}`) as KubeApiResourceVersionList; - const resourceVersions = getOrderedVersions(list, this.allowedUsableVersions?.[apiGroup]); + const parsedApi = parseKubeApi(apiUrl); - for (const resourceVersion of resourceVersions) { - const { resources } = await this.request.get(`${apiPrefix}/${resourceVersion.groupVersion}`) as KubeApiResourceList; + if (!parsedApi) { + continue; + } - if (resources.some(({ name }) => name === resource)) { - return { - apiPrefix, - apiGroup, - apiVersionPreferred: resourceVersion.version, - }; - } + const { apiPrefix, apiGroup, resource } = parsedApi; + const list = await this.request.get(`${apiPrefix}/${apiGroup}`) as KubeApiResourceVersionList; + const resourceVersions = getOrderedVersions(list, this.allowedUsableVersions?.[apiGroup]); + + for (const resourceVersion of resourceVersions) { + const { resources } = await this.request.get(`${apiPrefix}/${resourceVersion.groupVersion}`) as KubeApiResourceList; + + if (resources.some(({ name }) => name === resource)) { + return { + apiPrefix, + apiGroup, + apiVersionPreferred: resourceVersion.version, + }; } - } catch (error) { - // Exception is ignored as we can try the next url } } diff --git a/packages/core/src/common/k8s-api/kube-api/get-kube-api-from-path.injectable.ts b/packages/core/src/common/k8s-api/kube-api/get-kube-api-from-path.injectable.ts index 669185684f..61ac7b1e96 100644 --- a/packages/core/src/common/k8s-api/kube-api/get-kube-api-from-path.injectable.ts +++ b/packages/core/src/common/k8s-api/kube-api/get-kube-api-from-path.injectable.ts @@ -7,18 +7,18 @@ import { parseKubeApi } from "../kube-api-parse"; import { kubeApiInjectionToken } from "./kube-api-injection-token"; import type { KubeApi } from "../kube-api"; +export type GetKubeApiFromPath = (apiPath: string) => KubeApi | undefined; + const getKubeApiFromPathInjectable = getInjectable({ id: "get-kube-api-from-path", - instantiate: (di) => { + instantiate: (di): GetKubeApiFromPath => { const kubeApis = di.injectMany(kubeApiInjectionToken); return (apiPath: string) => { const parsed = parseKubeApi(apiPath); - const kubeApi = kubeApis.find((api) => api.apiBase === parsed.apiBase); - - return (kubeApi as KubeApi) || undefined; + return kubeApis.find((api) => api.apiBase === parsed?.apiBase); }; }, }); diff --git a/packages/core/src/common/k8s-api/kube-object.store.ts b/packages/core/src/common/k8s-api/kube-object.store.ts index 9effdc8f57..9efaca4dee 100644 --- a/packages/core/src/common/k8s-api/kube-object.store.ts +++ b/packages/core/src/common/k8s-api/kube-object.store.ts @@ -324,7 +324,11 @@ export class KubeObjectStore< @action async loadFromPath(resourcePath: string) { - const { namespace, name } = parseKubeApi(resourcePath); + const parsedApi = parseKubeApi(resourcePath); + + assert(parsedApi, "resourcePath must be a valid kube api"); + + const { namespace, name } = parsedApi; assert(name, "name must be part of resourcePath"); diff --git a/packages/core/src/features/telemetry/renderer/telemetry-decorator-for-show-details.injectable.ts b/packages/core/src/features/telemetry/renderer/telemetry-decorator-for-show-details.injectable.ts index 39a729bc4d..221bcd36fa 100644 --- a/packages/core/src/features/telemetry/renderer/telemetry-decorator-for-show-details.injectable.ts +++ b/packages/core/src/features/telemetry/renderer/telemetry-decorator-for-show-details.injectable.ts @@ -4,6 +4,7 @@ */ import { getInjectable, createInstantiationTargetDecorator, instantiationDecoratorToken } from "@ogre-tools/injectable"; import { pick } from "lodash"; +import { inspect } from "util"; import { parseKubeApi } from "../../../common/k8s-api/kube-api-parse"; import showDetailsInjectable from "../../../renderer/components/kube-detail-params/show-details.injectable"; import emitTelemetryInjectable from "./emit-telemetry.injectable"; @@ -21,13 +22,15 @@ const telemetryDecoratorForShowDetailsInjectable = getInjectable({ ? { action: "open", ...(() => { - try { - return { - resource: pick(parseKubeApi(args[0]), "apiPrefix", "apiVersion", "apiGroup", "namespace", "resource", "name"), - }; - } catch (error) { - return { error: `${error}` }; + const parsedApi = parseKubeApi(args[0]); + + if (!parsedApi) { + return { error: `invalid apiPath: ${inspect(args[0])}` }; } + + return { + resource: pick(parsedApi, "apiPrefix", "apiVersion", "apiGroup", "namespace", "resource", "name"), + }; })(), } : { diff --git a/packages/core/src/features/telemetry/show-details-calls.test.ts b/packages/core/src/features/telemetry/show-details-calls.test.ts index 736fe13530..ac90d99a87 100644 --- a/packages/core/src/features/telemetry/show-details-calls.test.ts +++ b/packages/core/src/features/telemetry/show-details-calls.test.ts @@ -21,7 +21,7 @@ describe("emit telemetry with params for calls to showDetails", () => { showDetails = di.inject(showDetailsInjectable); }); - it("when showDetails is called with no selflink (ie closing) should emit telemetry with param indicating closing the drawer", () => { + it("when showDetails is called with no selfLink (ie closing) should emit telemetry with param indicating closing the drawer", () => { showDetails(undefined); expect(emitAppEventMock).toBeCalledWith({ @@ -34,7 +34,7 @@ describe("emit telemetry with params for calls to showDetails", () => { }); }); - it("when showDetails is called with empty selflink (ie closing) should emit telemetry with param indicating closing the drawer", () => { + it("when showDetails is called with empty selfLink (ie closing) should emit telemetry with param indicating closing the drawer", () => { showDetails(""); expect(emitAppEventMock).toBeCalledWith({ @@ -47,7 +47,7 @@ describe("emit telemetry with params for calls to showDetails", () => { }); }); - it("when showDetails is called with valid selflink should emit telemetry with param indicating opening the drawer with that resource", () => { + it("when showDetails is called with valid selfLink should emit telemetry with param indicating opening the drawer with that resource", () => { showDetails("/api/v1/namespaces/default/pods/some-name"); expect(emitAppEventMock).toBeCalledWith({ @@ -68,7 +68,7 @@ describe("emit telemetry with params for calls to showDetails", () => { }); }); - it("when showDetails is called with invalid selflink should emit telemetry with param indicating opening the drawer but also show error", () => { + it("when showDetails is called with invalid selfLink should emit telemetry with param indicating opening the drawer but also show error", () => { showDetails("some-non-self-link-value"); expect(emitAppEventMock).toBeCalledWith({ @@ -77,7 +77,7 @@ describe("emit telemetry with params for calls to showDetails", () => { name: "show-details", params: { action: "open", - error: "Error: invalid apiPath: /some-non-self-link-value", + error: "invalid apiPath: 'some-non-self-link-value'", }, }); }); diff --git a/packages/core/src/renderer/components/dock/edit-resource/edit-resource-model/call-for-resource/call-for-resource.injectable.ts b/packages/core/src/renderer/components/dock/edit-resource/edit-resource-model/call-for-resource/call-for-resource.injectable.ts index 42ffa28946..8f66768c9c 100644 --- a/packages/core/src/renderer/components/dock/edit-resource/edit-resource-model/call-for-resource/call-for-resource.injectable.ts +++ b/packages/core/src/renderer/components/dock/edit-resource/edit-resource-model/call-for-resource/call-for-resource.injectable.ts @@ -20,7 +20,7 @@ const callForResourceInjectable = getInjectable({ return async (apiPath: string) => { const parsed = parseKubeApi(apiPath); - if (!parsed.name) { + if (!parsed?.name) { return { callWasSuccessful: false, error: "Invalid API path" }; } diff --git a/packages/core/src/renderer/components/dock/edit-resource/edit-resource-model/edit-resource-model.injectable.tsx b/packages/core/src/renderer/components/dock/edit-resource/edit-resource-model/edit-resource-model.injectable.tsx index e1f8eca988..5b5cb352e8 100644 --- a/packages/core/src/renderer/components/dock/edit-resource/edit-resource-model/edit-resource-model.injectable.tsx +++ b/packages/core/src/renderer/components/dock/edit-resource/edit-resource-model/edit-resource-model.injectable.tsx @@ -59,11 +59,17 @@ interface Dependencies { readonly tabId: string; } -function getEditSelfLinkFor(object: RawKubeObject): string { +function getEditSelfLinkFor(object: RawKubeObject): string | undefined { const lensVersionLabel = object.metadata.labels?.[EditResourceLabelName]; if (lensVersionLabel) { - const { apiVersionWithGroup, ...parsedApi } = parseKubeApi(object.metadata.selfLink); + const parsedKubeApi = parseKubeApi(object.metadata.selfLink); + + if (!parsedKubeApi) { + return undefined; + } + + const { apiVersionWithGroup, ...parsedApi } = parsedKubeApi; parsedApi.apiVersion = lensVersionLabel; @@ -139,6 +145,10 @@ export class EditResourceModel { if (result?.response?.metadata.labels?.[EditResourceLabelName]) { const parsed = parseKubeApi(this.selfLink); + if (!parsed) { + return void this.dependencies.showErrorNotification(`Object's selfLink is invalid: "${this.selfLink}"`); + } + parsed.apiVersion = result.response.metadata.labels[EditResourceLabelName]; result = await this.dependencies.callForResource(createKubeApiURL(parsed)); @@ -186,6 +196,17 @@ export class EditResourceModel { const patches = createPatch(firstVersion, currentVersion); const selfLink = getEditSelfLinkFor(currentVersion); + + if (!selfLink) { + this.dependencies.showErrorNotification(( +

+ {`Cannot save resource, unknown selfLink: "${currentVersion.metadata.selfLink}"`} +

+ )); + + return null; + } + const result = await this.dependencies.callForPatchResource(this.resource, patches); if (!result.callWasSuccessful) {