Fix listing charts failing due to bad versions (#3308)

src/common/utils/index.ts

@@ -45,6 +45,7 @@ export * from "./openExternal";
 export * from "./paths";
 export * from "./reject-promise";
 export * from "./singleton";
+export * from "./sort-compare";
 export * from "./splitArray";
 export * from "./tar";
 export * from "./toggle-set";

src/common/utils/sort-compare.ts

@@ -0,0 +1,55 @@
+/**
+ * Copyright (c) 2021 OpenLens Authors
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy of
+ * this software and associated documentation files (the "Software"), to deal in
+ * the Software without restriction, including without limitation the rights to
+ * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of
+ * the Software, and to permit persons to whom the Software is furnished to do so,
+ * subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all
+ * copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
+ * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
+ * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER
+ * IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+import semver, { SemVer } from "semver";
+
+export function sortCompare<T>(left: T, right: T): -1 | 0 | 1 {
+  if (left < right) {
+    return -1;
+  }
+
+  if (left === right) {
+    return 0;
+  }
+
+  return 1;
+}
+
+interface ChartVersion {
+  version: string;
+  __version?: SemVer;
+}
+
+export function sortCompareChartVersions(left: ChartVersion, right: ChartVersion): -1 | 0 | 1 {
+  if (left.__version && right.__version) {
+    return semver.compare(right.__version, left.__version);
+  }
+
+  if (!left.__version && right.__version) {
+    return 1;
+  }
+
+  if (left.__version && !right.__version) {
+    return -1;
+  }
+
+  return sortCompare(left.version, right.version);
+}

src/main/helm/__mocks__/helm-chart-manager.ts

@@ -34,6 +34,36 @@ export class HelmChartManager {
     switch (this.repo.name) {
       case "stable":
         return Promise.resolve({
+          "invalid-semver": [
+            {
+              apiVersion: "3.0.0",
+              name: "weird-versioning",
+              version: "I am not semver",
+              repo: "stable",
+              digest: "test"
+            },
+            {
+              apiVersion: "3.0.0",
+              name: "weird-versioning",
+              version: "v4.3.0",
+              repo: "stable",
+              digest: "test"
+            },
+            {
+              apiVersion: "3.0.0",
+              name: "weird-versioning",
+              version: "I am not semver but more",
+              repo: "stable",
+              digest: "test"
+            },
+            {
+              apiVersion: "3.0.0",
+              name: "weird-versioning",
+              version: "v4.4.0",
+              repo: "stable",
+              digest: "test"
+            },
+          ],
           "apm-server": [
             {
               apiVersion: "3.0.0",

src/main/helm/__tests__/helm-service.test.ts

@@ -62,6 +62,36 @@ describe("Helm Service tests", () => {
             digest: "test"
           }
         ],
+        "invalid-semver": [
+          {
+            apiVersion: "3.0.0",
+            name: "weird-versioning",
+            version: "v4.4.0",
+            repo: "stable",
+            digest: "test"
+          },
+          {
+            apiVersion: "3.0.0",
+            name: "weird-versioning",
+            version: "v4.3.0",
+            repo: "stable",
+            digest: "test"
+          },
+          {
+            apiVersion: "3.0.0",
+            name: "weird-versioning",
+            version: "I am not semver",
+            repo: "stable",
+            digest: "test"
+          },
+          {
+            apiVersion: "3.0.0",
+            name: "weird-versioning",
+            version: "I am not semver but more",
+            repo: "stable",
+            digest: "test"
+          },
+        ],
         "redis": [
           {
             apiVersion: "3.0.0",

src/main/helm/helm-service.ts

@@ -19,13 +19,14 @@
  * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
  */
 
-import semver from "semver";
+import semver, { SemVer } from "semver";
 import type { Cluster } from "../cluster";
 import logger from "../logger";
 import { HelmRepoManager } from "./helm-repo-manager";
 import { HelmChartManager } from "./helm-chart-manager";
-import type { HelmChartList, RepoHelmChartList } from "../../renderer/api/endpoints/helm-charts.api";
+import type { HelmChart, HelmChartList, RepoHelmChartList } from "../../renderer/api/endpoints/helm-charts.api";
 import { deleteRelease, getHistory, getRelease, getValues, installChart, listReleases, rollback, upgradeRelease } from "./helm-release-manager";
+import { iter, sortCompareChartVersions } from "../../common/utils";
 
 interface GetReleaseValuesArgs {
   cluster: Cluster;
@@ -132,28 +133,55 @@ class HelmService {
   }
 
   private excludeDeprecatedChartGroups(chartGroups: RepoHelmChartList) {
-    const groups = new Map(Object.entries(chartGroups));
+    return Object.fromEntries(
+      iter.filterMap(
+        Object.entries(chartGroups),
+        ([name, charts]) => {
+          for (const chart of charts) {
+            if (chart.deprecated) {
+              // ignore chart group if any chart is deprecated
+              return undefined;
+            }
+          }
 
-    for (const [chartName, charts] of groups) {
-      if (charts[0].deprecated) {
-        groups.delete(chartName);
-      }
+          return [name, charts];
+        }
+      )
+    );
+  }
+
+  private sortCharts(charts: HelmChart[]) {
+    interface ExtendedHelmChart extends HelmChart {
+      __version: SemVer;
     }
 
-    return Object.fromEntries(groups);
+    const chartsWithVersion = Array.from(
+      iter.map(
+        charts,
+        (chart => {
+          const __version = semver.coerce(chart.version, { includePrerelease: true, loose: true });
+
+          if (!__version) {
+            logger.error(`[HELM-SERVICE]: Version from helm chart is not loosely coercable to semver.`, { name: chart.name, version: chart.version, repo: chart.repo });
+          }
+
+          (chart as ExtendedHelmChart).__version = __version;
+
+          return chart as ExtendedHelmChart;
+        })
+      ),
+    );
+
+    return chartsWithVersion
+      .sort(sortCompareChartVersions)
+      .map(chart => (delete chart.__version, chart as HelmChart));
   }
 
   private sortChartsByVersion(chartGroups: RepoHelmChartList) {
-    for (const key in chartGroups) {
-      chartGroups[key] = chartGroups[key].sort((first, second) => {
-        const firstVersion = semver.coerce(first.version || 0);
-        const secondVersion = semver.coerce(second.version || 0);
-
-        return semver.compare(secondVersion, firstVersion);
-      });
-    }
-
-    return chartGroups;
+    return Object.fromEntries(
+      Object.entries(chartGroups)
+        .map(([name, charts]) => [name, this.sortCharts(charts)])
+    );
   }
 }
 

src/renderer/components/+apps-helm-charts/helm-chart.store.ts

@@ -21,7 +21,7 @@
 
 import semver from "semver";
 import { observable, makeObservable } from "mobx";
-import { autoBind } from "../../utils";
+import { autoBind, sortCompareChartVersions } from "../../utils";
 import { getChartDetails, HelmChart, listCharts } from "../../api/endpoints/helm-charts.api";
 import { ItemStore } from "../../item.store";
 import flatten from "lodash/flatten";
@@ -60,12 +60,10 @@ export class HelmChartStore extends ItemStore<HelmChart> {
   }
 
   protected sortVersions = (versions: IChartVersion[]) => {
-    return versions.sort((first, second) => {
-      const firstVersion = semver.coerce(first.version || 0);
-      const secondVersion = semver.coerce(second.version || 0);
-
-      return semver.compare(secondVersion, firstVersion);
-    });
+    return versions
+      .map(chartVersion => ({ ...chartVersion, __version: semver.coerce(chartVersion.version, { includePrerelease: true, loose: true }), }))
+      .sort(sortCompareChartVersions)
+      .map(({ __version, ...chartVersion }) => chartVersion);
   };
 
   async getVersions(chartName: string, force?: boolean): Promise<IChartVersion[]> {