Fix: deprecated helm chart filtering (#2158)

* Refactor of excludeDeprecated helm service method

Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>

* Pick first helm chart from the list on load

Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>

* Removing helm filtering in UI

Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>

* Cleaning up

Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>

* Cleaning up type definitions

Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>

* Adding sorting charts by version

Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>

* Adding tests for methods that manipute chart listing

Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>

* Cleaning up tests a bit

Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>

* Adding semver coercion before comparing versions

Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>

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

@@ -0,0 +1,108 @@
+import { HelmRepo, HelmRepoManager } from "../helm-repo-manager";
+
+export class HelmChartManager {
+  private cache: any = {};
+  private repo: HelmRepo;
+
+  constructor(repo: HelmRepo){
+    this.cache = HelmRepoManager.cache;
+    this.repo = repo;
+  }
+
+  public async charts(): Promise<any> {
+    switch (this.repo.name) {
+      case "stable":
+        return Promise.resolve({
+          "apm-server": [
+            {
+              apiVersion: "3.0.0",
+              name: "apm-server",
+              version: "2.1.7",
+              repo: "stable",
+              digest: "test"
+            },
+            {
+              apiVersion: "3.0.0",
+              name: "apm-server",
+              version: "2.1.6",
+              repo: "stable",
+              digest: "test"
+            }
+          ],
+          "redis": [
+            {
+              apiVersion: "3.0.0",
+              name: "apm-server",
+              version: "1.0.0",
+              repo: "stable",
+              digest: "test"
+            },
+            {
+              apiVersion: "3.0.0",
+              name: "apm-server",
+              version: "0.0.9",
+              repo: "stable",
+              digest: "test"
+            }
+          ]
+        });
+      case "experiment":
+        return Promise.resolve({
+          "fairwind": [
+            {
+              apiVersion: "3.0.0",
+              name: "fairwind",
+              version: "0.0.1",
+              repo: "experiment",
+              digest: "test"
+            },
+            {
+              apiVersion: "3.0.0",
+              name: "fairwind",
+              version: "0.0.2",
+              repo: "experiment",
+              digest: "test",
+              deprecated: true
+            }
+          ]
+        });
+      case "bitnami":
+        return Promise.resolve({
+          "hotdog": [
+            {
+              apiVersion: "3.0.0",
+              name: "hotdog",
+              version: "1.0.1",
+              repo: "bitnami",
+              digest: "test"
+            },
+            {
+              apiVersion: "3.0.0",
+              name: "hotdog",
+              version: "1.0.2",
+              repo: "bitnami",
+              digest: "test",
+            }
+          ],
+          "pretzel": [
+            {
+              apiVersion: "3.0.0",
+              name: "pretzel",
+              version: "1.0",
+              repo: "bitnami",
+              digest: "test",
+            },
+            {
+              apiVersion: "3.0.0",
+              name: "pretzel",
+              version: "1.0.1",
+              repo: "bitnami",
+              digest: "test"
+            }
+          ]
+        });
+      default:
+        return Promise.resolve({});
+    }
+  }
+}

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

@@ -0,0 +1,104 @@
+import { helmService } from "../helm-service";
+import { repoManager } from "../helm-repo-manager";
+
+jest.spyOn(repoManager, "init").mockImplementation();
+
+jest.mock("../helm-chart-manager");
+
+describe("Helm Service tests", () => {
+  test("list charts without deprecated ones", async () => {
+    jest.spyOn(repoManager, "repositories").mockImplementation(async () => {
+      return [
+        { name: "stable", url: "stableurl" },
+        { name: "experiment", url: "experimenturl" }
+      ];
+    });
+
+    const charts = await helmService.listCharts();
+
+    expect(charts).toEqual({
+      stable: {
+        "apm-server": [
+          {
+            apiVersion: "3.0.0",
+            name: "apm-server",
+            version: "2.1.7",
+            repo: "stable",
+            digest: "test"
+          },
+          {
+            apiVersion: "3.0.0",
+            name: "apm-server",
+            version: "2.1.6",
+            repo: "stable",
+            digest: "test"
+          }
+        ],
+        "redis": [
+          {
+            apiVersion: "3.0.0",
+            name: "apm-server",
+            version: "1.0.0",
+            repo: "stable",
+            digest: "test"
+          },
+          {
+            apiVersion: "3.0.0",
+            name: "apm-server",
+            version: "0.0.9",
+            repo: "stable",
+            digest: "test"
+          }
+        ]
+      },
+      experiment: {}
+    });
+  });
+
+  test("list charts sorted by version in descending order", async () => {
+    jest.spyOn(repoManager, "repositories").mockImplementation(async () => {
+      return [
+        { name: "bitnami", url: "bitnamiurl" }
+      ];
+    });
+
+    const charts = await helmService.listCharts();
+
+    expect(charts).toEqual({
+      bitnami: {
+        "hotdog": [
+          {
+            apiVersion: "3.0.0",
+            name: "hotdog",
+            version: "1.0.2",
+            repo: "bitnami",
+            digest: "test",
+          },
+          {
+            apiVersion: "3.0.0",
+            name: "hotdog",
+            version: "1.0.1",
+            repo: "bitnami",
+            digest: "test"
+          },
+        ],
+        "pretzel": [
+          {
+            apiVersion: "3.0.0",
+            name: "pretzel",
+            version: "1.0.1",
+            repo: "bitnami",
+            digest: "test",
+          },
+          {
+            apiVersion: "3.0.0",
+            name: "pretzel",
+            version: "1.0",
+            repo: "bitnami",
+            digest: "test"
+          }
+        ]
+      }
+    });
+  });
+});

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

@@ -4,9 +4,10 @@ import { HelmRepo, HelmRepoManager } from "./helm-repo-manager";
 import logger from "../logger";
 import { promiseExec } from "../promise-exec";
 import { helmCli } from "./helm-cli";
+import type { RepoHelmChartList } from "../../renderer/api/endpoints/helm-charts.api";
 
 type CachedYaml = {
-  entries: any; // todo: types
+  entries: RepoHelmChartList
 };
 
 export class HelmChartManager {
@@ -24,15 +25,15 @@ export class HelmChartManager {
     return charts[name];
   }
 
-  public async charts(): Promise<any> {
+  public async charts(): Promise<RepoHelmChartList> {
     try {
       const cachedYaml = await this.cachedYaml();
 
       return cachedYaml["entries"];
     } catch(error) {
-      logger.error(error);
+      logger.error("HELM-CHART-MANAGER]: failed to list charts", { error });
 
-      return [];
+      return {};
     }
   }
 

src/main/helm/helm-service.ts

@@ -1,8 +1,10 @@
+import semver from "semver";
 import { Cluster } from "../cluster";
 import logger from "../logger";
 import { repoManager } from "./helm-repo-manager";
 import { HelmChartManager } from "./helm-chart-manager";
 import { releaseManager } from "./helm-release-manager";
+import { HelmChartList, RepoHelmChartList } from "../../renderer/api/endpoints/helm-charts.api";
 
 class HelmService {
   public async installChart(cluster: Cluster, data: { chart: string; values: {}; name: string; namespace: string; version: string }) {
@@ -10,7 +12,7 @@ class HelmService {
   }
 
   public async listCharts() {
-    const charts: any = {};
+    const charts: HelmChartList = {};
 
     await repoManager.init();
     const repositories = await repoManager.repositories();
@@ -18,14 +20,10 @@ class HelmService {
     for (const repo of repositories) {
       charts[repo.name] = {};
       const manager = new HelmChartManager(repo);
-      let entries = await manager.charts();
+      const sortedCharts = this.sortChartsByVersion(await manager.charts());
+      const enabledCharts = this.excludeDeprecatedChartGroups(sortedCharts);
 
-      entries = this.excludeDeprecated(entries);
+      charts[repo.name] = enabledCharts;
-
-      for (const key in entries) {
-        entries[key] = entries[key][0];
-      }
-      charts[repo.name] = entries;
     }
 
     return charts;
@@ -96,20 +94,30 @@ class HelmService {
     return { message: output };
   }
 
-  protected excludeDeprecated(entries: any) {
+  private excludeDeprecatedChartGroups(chartGroups: RepoHelmChartList) {
-    for (const key in entries) {
+    const groups = new Map(Object.entries(chartGroups));
-      entries[key] = entries[key].filter((entry: any) => {
-        if (Array.isArray(entry)) {
-          return entry[0]["deprecated"] != true;
-        }
 
-        return entry["deprecated"] != true;
+    for (const [chartName, charts] of groups) {
+      if (charts[0].deprecated) {
+        groups.delete(chartName);
+      }
+    }
+
+    return Object.fromEntries(groups);
+  }
+
+  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 entries;
+    return chartGroups;
   }
-
 }
 
 export const helmService = new HelmService();

src/renderer/api/endpoints/helm-charts.api.ts

@@ -3,11 +3,8 @@ import { apiBase } from "../index";
 import { stringify } from "querystring";
 import { autobind } from "../../utils";
 
-interface IHelmChartList {
+export type RepoHelmChartList = Record<string, HelmChart[]>;
-  [repo: string]: {
+export type HelmChartList = Record<string, RepoHelmChartList>;
-    [name: string]: HelmChart;
-  };
-}
 
 export interface IHelmChartDetails {
   readme: string;
@@ -22,12 +19,12 @@ const endpoint = compile(`/v2/charts/:repo?/:name?`) as (params?: {
 export const helmChartsApi = {
   list() {
     return apiBase
-      .get<IHelmChartList>(endpoint())
+      .get<HelmChartList>(endpoint())
       .then(data => {
         return Object
           .values(data)
           .reduce((allCharts, repoCharts) => allCharts.concat(Object.values(repoCharts)), [])
-          .map(HelmChart.create);
+          .map(([chart]) => HelmChart.create(chart));
       });
   },
 

src/renderer/components/+apps-helm-charts/helm-charts.tsx

@@ -72,9 +72,6 @@ export class HelmCharts extends Component<Props> {
             (chart: HelmChart) => chart.getAppVersion(),
             (chart: HelmChart) => chart.getKeywords(),
           ]}
-          filterItems={[
-            (items: HelmChart[]) => items.filter(item => !item.deprecated)
-          ]}
           customizeHeader={() => (
             <SearchInputUrl placeholder="Search Helm Charts" />
           )}