fix parseApi to not throw and to match apiVersion more specifically (#496)

* remove the explicit throw is new parseApi

* fix parseApi to be more consitent with the older version

- Add many more unit tests (these are exact copies of the old behaviour
  so that any changes do not change behaviour)

- Make explicit, with documentation, why certain actions are being taken
  with the parsing of the api URL parts

Signed-off-by: Sebastian Malton <smalton@mirantis.com>

Co-authored-by: Sebastian Malton <smalton@mirantis.com>

dashboard/client/api/__test__/parseAPI.test.ts

@@ -45,10 +45,62 @@ const tests: ParseAPITest[] = [
       namespace: undefined,
     },
   },
+  {
+    url: "/api/v1/namespaces",
+    expected: {
+      apiBase: "/api/v1/namespaces",
+      apiPrefix: "/api",
+      apiGroup: "",
+      apiVersion: "v1",
+      apiVersionWithGroup: "v1",
+      resource: "namespaces",
+      name: undefined,
+      namespace: undefined,
+    },
+  },
+  {
+    url: "/api/v1/secrets",
+    expected: {
+      apiBase: "/api/v1/secrets",
+      apiPrefix: "/api",
+      apiGroup: "",
+      apiVersion: "v1",
+      apiVersionWithGroup: "v1",
+      resource: "secrets",
+      name: undefined,
+      namespace: undefined,
+    },
+  },
+  {
+    url: "/api/v1/nodes/minikube",
+    expected: {
+      apiBase: "/api/v1/nodes",
+      apiPrefix: "/api",
+      apiGroup: "",
+      apiVersion: "v1",
+      apiVersionWithGroup: "v1",
+      resource: "nodes",
+      name: "minikube",
+      namespace: undefined,
+    },
+  },
+  {
+    url: "/api/foo-bar/nodes/minikube",
+    expected: {
+      apiBase: "/api/foo-bar/nodes",
+      apiPrefix: "/api",
+      apiGroup: "",
+      apiVersion: "foo-bar",
+      apiVersionWithGroup: "foo-bar",
+      resource: "nodes",
+      name: "minikube",
+      namespace: undefined,
+    },
+  },
 ];
 
 jest.mock('../kube-watch-api.ts', () => 'KubeWatchApi');
-describe("parseAPI unit tests", () => {
+describe("parseApi unit tests", () => {
   for (const i in tests) {
     const { url: tUrl, expected:tExpect} = tests[i];
     test(`test #${parseInt(i)+1}`, () => {

dashboard/client/api/api-manager.ts

@@ -18,24 +18,18 @@ export class ApiManager {
   private views = observable.map<KubeApi, ApiComponents>();
 
   getApi(pathOrCallback: string | ((api: KubeApi) => boolean)) {
-    const apis = this.apis;
     if (typeof pathOrCallback === "string") {
-      let api = apis.get(pathOrCallback);
+      return this.apis.get(pathOrCallback) || this.apis.get(KubeApi.parseApi(pathOrCallback).apiBase);
-      if (!api) {
-        const { apiBase } = KubeApi.parseApi(pathOrCallback);
-        api = apis.get(apiBase);
-      }
-      return api;
-    }
-    else {
-      return Array.from(apis.values()).find(pathOrCallback);
     }
+
+    return Array.from(this.apis.values()).find(pathOrCallback);
   }
 
   registerApi(apiBase: string, api: KubeApi) {
-    if (this.apis.has(apiBase)) return;
+    if (!this.apis.has(apiBase)) {
       this.apis.set(apiBase, api);
     }
+  }
 
   protected resolveApi(api: string | KubeApi): KubeApi {
     if (typeof api === "string") return this.getApi(api)

dashboard/client/api/kube-api.ts

@@ -8,6 +8,7 @@ import { apiKube } from "./index";
 import { kubeWatchApi } from "./kube-watch-api";
 import { apiManager } from "./api-manager";
 import { split } from "../utils/arrays";
+import isEqual from "lodash/isEqual";
 
 export interface IKubeApiOptions<T extends KubeObject> {
   kind: string; // resource type within api-group, e.g. "Namespace"
@@ -42,23 +43,65 @@ export interface IKubeApiLinkBase extends IKubeApiLinkRef {
 export class KubeApi<T extends KubeObject = any> {
   static parseApi(apiPath = ""): IKubeApiLinkBase {
     apiPath = new URL(apiPath, location.origin).pathname;
+
     const [, prefix, ...parts] = apiPath.split("/");
     const apiPrefix = `/${prefix}`;
 
-    const [left, right, found] = split(parts, "namespaces");
+    const [left, right, namespaced] = split(parts, "namespaces");
     let apiGroup, apiVersion, namespace, resource, name;
 
-    if (found) {
+    if (namespaced) {
-      if (left.length == 0) {
+      switch (right.length) {
-        throw new Error(`invalid apiPath: ${apiPath}`)
+      case 0:
+        resource = "namespaces"; // special case this due to `split` removing namespaces
+        break;
+      case 1:
+        resource = right[0];
+        break;
+      default:
+        [namespace, resource, name] = right;
+        break;
       }
 
       apiVersion = left.pop();
       apiGroup = left.join("/");
-      [namespace, resource, name] = right;
     } else {
-      [apiGroup, apiVersion, resource] = left;
+      switch (left.length) {
+      case 2:
+        resource = left.pop();
+      case 1:
+        apiVersion = left.pop();
+        apiGroup = "";
+        break;
+      default:
+        /**
+             * Given that 
+             *  - `apiVersion` is `GROUP/VERSION` and
+             *  - `VERSION` is `DNS_LABEL` which is /^[a-z0-9]((-[a-z0-9])|[a-z0-9])*$/i
+             *     where length <= 63
+             *  - `GROUP` is /^D(\.D)*$/ where D is `DNS_LABEL` and length <= 253
+             * 
+             * There is no well defined selection from an array of items that were
+             * seperated by '/'
+             * 
+             * Solution is to create a huristic. Namely:
+             * 1. if '.' in left[0] then apiGroup <- left[0]
+             * 2. if left[1] matches /^v[0-9]/ then apiGroup, apiVersion <- left[0], left[1]
+             * 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;
+      }
+    }
+
     const apiVersionWithGroup = [apiGroup, apiVersion].filter(v => v).join("/");
     const apiBase = [apiPrefix, apiGroup, apiVersion, resource].filter(v => v).join("/");