Use the served and storage version of a CRD instead of the first (#3999)

Signed-off-by: Jim Ehrismann <jehrismann@mirantis.com>

src/common/k8s-api/__tests__/crd.test.ts

@@ -20,92 +20,108 @@
  */
 
 import { CustomResourceDefinition } from "../endpoints";
-import type { KubeObjectMetadata } from "../kube-object";
 
 describe("Crds", () => {
   describe("getVersion", () => {
-    it("should get the first version name from the list of versions", () => {
+    it("should throw if none of the versions are served", () => {
       const crd = new CustomResourceDefinition({
-        apiVersion: "foo",
+        apiVersion: "apiextensions.k8s.io/v1",
         kind: "CustomResourceDefinition",
-        metadata: {} as KubeObjectMetadata,
+        metadata: {
+          name: "foo",
+          resourceVersion: "12345",
+          uid: "12345",
+        },
+        spec: {
+          versions: [
+            {
+              name: "123",
+              served: false,
+              storage: false,
+            },
+            {
+              name: "1234",
+              served: false,
+              storage: false,
+            },
+          ],
+        },
       });
 
-      crd.spec = {
-        versions: [
-          {
-            name: "123",
-            served: false,
-            storage: false,
-          }
-        ]
-      } as any;
+      expect(() => crd.getVersion()).toThrowError("Failed to find a version for CustomResourceDefinition foo");
+    });
+
+    it("should should get the version that is both served and stored", () => {
+      const crd = new CustomResourceDefinition({
+        apiVersion: "apiextensions.k8s.io/v1",
+        kind: "CustomResourceDefinition",
+        metadata: {
+          name: "foo",
+          resourceVersion: "12345",
+          uid: "12345",
+        },
+        spec: {
+          versions: [
+            {
+              name: "123",
+              served: true,
+              storage: true,
+            },
+            {
+              name: "1234",
+              served: false,
+              storage: false,
+            },
+          ],
+        },
+      });
 
       expect(crd.getVersion()).toBe("123");
     });
 
-    it("should get the first version name from the list of versions (length 2)", () => {
+    it("should should get the version that is both served and stored even with version field", () => {
       const crd = new CustomResourceDefinition({
-        apiVersion: "foo",
+        apiVersion: "apiextensions.k8s.io/v1",
         kind: "CustomResourceDefinition",
-        metadata: {} as KubeObjectMetadata,
+        metadata: {
+          name: "foo",
+          resourceVersion: "12345",
+          uid: "12345",
+        },
+        spec: {
+          version: "abc",
+          versions: [
+            {
+              name: "123",
+              served: true,
+              storage: true,
+            },
+            {
+              name: "1234",
+              served: false,
+              storage: false,
+            },
+          ],
+        },
       });
 
-      crd.spec = {
-        versions: [
-          {
-            name: "123",
-            served: false,
-            storage: false,
-          },
-          {
-            name: "1234",
-            served: false,
-            storage: false,
-          }
-        ]
-      } as any;
-
       expect(crd.getVersion()).toBe("123");
     });
 
-    it("should get the first version name from the list of versions (length 2) even with version field", () => {
+    it("should get the version name from the version field", () => {
       const crd = new CustomResourceDefinition({
-        apiVersion: "foo",
+        apiVersion: "apiextensions.k8s.io/v1beta1",
         kind: "CustomResourceDefinition",
-        metadata: {} as KubeObjectMetadata,
+        metadata: {
+          name: "foo",
+          resourceVersion: "12345",
+          uid: "12345",
+        },
+        spec: {
+          version: "abc",
+        }
       });
 
-      crd.spec = {
-        version: "abc",
-        versions: [
-          {
-            name: "123",
-            served: false,
-            storage: false,
-          },
-          {
-            name: "1234",
-            served: false,
-            storage: false,
-          }
-        ]
-      } as any;
-
-      expect(crd.getVersion()).toBe("123");
-    });
-
-    it("should get the first version name from the version field", () => {
-      const crd = new CustomResourceDefinition({
-        apiVersion: "foo",
-        kind: "CustomResourceDefinition",
-        metadata: {} as KubeObjectMetadata,
-      });
-
-      crd.spec = {
-        version: "abc"
-      } as any;
-
       expect(crd.getVersion()).toBe("abc");
     });
   });

src/common/k8s-api/endpoints/crd.api.ts

@@ -19,10 +19,11 @@
  * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
  */
 
-import { KubeObject } from "../kube-object";
+import { KubeCreationError, KubeObject } from "../kube-object";
 import { KubeApi } from "../kube-api";
 import { crdResourcesURL } from "../../routes";
 import { isClusterPageContext } from "../../utils/cluster-id-url-parsing";
+import type { KubeJsonApiData } from "../kube-json-api";
 
 type AdditionalPrinterColumnsCommon = {
   name: string;
@@ -39,10 +40,21 @@ type AdditionalPrinterColumnsV1Beta = AdditionalPrinterColumnsCommon & {
   JSONPath: string;
 };
 
+export interface CRDVersion {
+  name: string;
+  served: boolean;
+  storage: boolean;
+  schema?: object; // required in v1 but not present in v1beta
+  additionalPrinterColumns?: AdditionalPrinterColumnsV1[];
+}
+
 export interface CustomResourceDefinition {
   spec: {
     group: string;
-    version?: string; // deprecated in v1 api
+    /**
+     * @deprecated for apiextensions.k8s.io/v1 but used previously
+     */
+    version?: string;
     names: {
       plural: string;
       singular: string;
@@ -50,19 +62,19 @@ export interface CustomResourceDefinition {
       listKind: string;
     };
     scope: "Namespaced" | "Cluster" | string;
-    validation?: any;
-    versions?: {
-      name: string;
-      served: boolean;
-      storage: boolean;
-      schema?: unknown; // required in v1 but not present in v1beta
-      additionalPrinterColumns?: AdditionalPrinterColumnsV1[]
-    }[];
+    /**
+     * @deprecated for apiextensions.k8s.io/v1 but used previously
+     */
+    validation?: object;
+    versions?: CRDVersion[];
     conversion: {
       strategy?: string;
       webhook?: any;
     };
-    additionalPrinterColumns?: AdditionalPrinterColumnsV1Beta[]; // removed in v1
+    /**
+     * @deprecated for apiextensions.k8s.io/v1 but used previously
+     */
+    additionalPrinterColumns?: AdditionalPrinterColumnsV1Beta[];
   };
   status: {
     conditions: {
@@ -83,11 +95,23 @@ export interface CustomResourceDefinition {
   };
 }
 
+export interface CRDApiData extends KubeJsonApiData {
+  spec: object; // TODO: make better
+}
+
 export class CustomResourceDefinition extends KubeObject {
   static kind = "CustomResourceDefinition";
   static namespaced = false;
   static apiBase = "/apis/apiextensions.k8s.io/v1/customresourcedefinitions";
 
+  constructor(data: CRDApiData) {
+    super(data);
+
+    if (!data.spec || typeof data.spec !== "object") {
+      throw new KubeCreationError("Cannot create a CustomResourceDefinition from an object without spec", data);
+    }
+  }
+
   getResourceUrl() {
     return crdResourcesURL({
       params: {
@@ -125,9 +149,36 @@ export class CustomResourceDefinition extends KubeObject {
     return this.spec.scope;
   }
 
+  getPreferedVersion(): CRDVersion {
+    // Prefer the modern `versions` over the legacy `version`
+    if (this.spec.versions) {
+      for (const version of this.spec.versions) {
+        /**
+         * If the version is not served then 404 errors will occur
+         * We should also prefer the storage version
+         */
+        if (version.served && version.storage) {
+          return version;
+        }
+      }
+    } else if (this.spec.version) {
+      const { additionalPrinterColumns: apc } = this.spec;
+      const additionalPrinterColumns = apc?.map(({ JSONPath, ...apc}) => ({ ...apc, jsonPath: JSONPath }));
+
+      return {
+        name: this.spec.version,
+        served: true,
+        storage: true,
+        schema: this.spec.validation,
+        additionalPrinterColumns,
+      };
+    }
+
+    throw new Error(`Failed to find a version for CustomResourceDefinition ${this.metadata.name}`);
+  }
+
   getVersion() {
-    // v1 has removed the spec.version property, if it is present it must match the first version
-    return this.spec.versions?.[0]?.name ?? this.spec.version;
+    return this.getPreferedVersion().name;
   }
 
   isNamespaced() {
@@ -147,17 +198,14 @@ export class CustomResourceDefinition extends KubeObject {
   }
 
   getPrinterColumns(ignorePriority = true): AdditionalPrinterColumnsV1[] {
-    const columns = this.spec.versions?.find(a => this.getVersion() == a.name)?.additionalPrinterColumns
-      ?? this.spec.additionalPrinterColumns?.map(({ JSONPath, ...rest }) => ({ ...rest, jsonPath: JSONPath })) // map to V1 shape
-      ?? [];
+    const columns = this.getPreferedVersion().additionalPrinterColumns ?? [];
 
     return columns
-      .filter(column => column.name != "Age")
-      .filter(column => ignorePriority ? true : !column.priority);
+      .filter(column => column.name != "Age" && (ignorePriority || !column.priority));
   }
 
   getValidation() {
-    return JSON.stringify(this.spec.validation ?? this.spec.versions?.[0]?.schema, null, 2);
+    return JSON.stringify(this.getPreferedVersion().schema, null, 2);
   }
 
   getConditions() {

src/common/k8s-api/json-api.ts

@@ -27,8 +27,7 @@ import { stringify } from "querystring";
 import { EventEmitter } from "../../common/event-emitter";
 import logger from "../../common/logger";
 
-export interface JsonApiData {
-}
+export interface JsonApiData {}
 
 export interface JsonApiError {
   code?: number;

src/renderer/utils/createStorage.ts

@@ -28,6 +28,7 @@ import { StorageHelper } from "./storageHelper";
 import { ClusterStore } from "../../common/cluster-store";
 import logger from "../../main/logger";
 import { getHostedClusterId, getPath } from "../../common/utils";
+import { isTestEnv } from "../../common/vars";
 
 const storage = observable({
   initialized: false,
@@ -62,7 +63,9 @@ export function createAppStorage<T>(key: string, defaultValue: T, clusterId?: st
       .then(data => storage.data = data)
       .catch(() => null) // ignore empty / non-existing / invalid json files
       .finally(() => {
-        logger.info(`${logPrefix} loading finished for ${filePath}`);
+        if (!isTestEnv) {
+          logger.info(`${logPrefix} loading finished for ${filePath}`);
+        }
         storage.loaded = true;
       });