Fix filtering deprecated charts (#3635)

src/common/k8s-api/endpoints/helm-charts.api.ts

@@ -26,8 +26,7 @@ import type { RequestInit } from "node-fetch";
 import { autoBind, bifurcateArray } from "../../utils";
 import Joi from "joi";
 
-export type RepoHelmChartList = Record<string, HelmChart[]>;
+export type RepoHelmChartList = Record<string, RawHelmChart[]>;
-export type HelmChartList = Record<string, RepoHelmChartList>;
 
 export interface IHelmChartDetails {
   readme: string;
@@ -43,7 +42,7 @@ const endpoint = compile(`/v2/charts/:repo?/:name?`) as (params?: {
  * Get a list of all helm charts from all saved helm repos
  */
 export async function listCharts(): Promise<HelmChart[]> {
-  const data = await apiBase.get<HelmChartList>(endpoint());
+  const data = await apiBase.get<Record<string, RepoHelmChartList>>(endpoint());
 
   return Object
     .values(data)
@@ -311,11 +310,9 @@ export class HelmChart {
   }
 
   static create(data: RawHelmChart, { onError = "throw" }: HelmChartCreateOpts = {}): HelmChart | undefined {
-    const result = helmChartValidator.validate(data, {
+    const { value, error } = helmChartValidator.validate(data, {
       abortEarly: false,
     });
-    let { error } = result;
-    const { value } = result;
 
     if (!error) {
       return new HelmChart(value);
@@ -331,13 +328,13 @@ export class HelmChart {
       return new HelmChart(value);
     }
 
-    error = new Joi.ValidationError(actualErrors.map(er => er.message).join(". "), actualErrors, error._original);
+    const validationError = new Joi.ValidationError(actualErrors.map(er => er.message).join(". "), actualErrors, error._original);
 
     if (onError === "throw") {
-      throw error;
+      throw validationError;
     }
 
-    console.warn("[HELM-CHART]: failed to validate data", data, error);
+    console.warn("[HELM-CHART]: failed to validate data", data, validationError);
 
     return undefined;
   }

src/common/logger.ts

@@ -31,6 +31,8 @@ const logLevel = process.env.LOG_LEVEL
   ? process.env.LOG_LEVEL
   : isDebugging
     ? "debug"
+    : isTestEnv
+      ? "error"
       : "info";
 
 const transports: Transport[] = [

src/common/utils/sort-compare.ts

@@ -19,7 +19,10 @@
  * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
  */
 
-import semver, { SemVer } from "semver";
+import semver, { coerce, SemVer } from "semver";
+import * as iter from "./iter";
+import type { RawHelmChart } from "../k8s-api/endpoints/helm-charts.api";
+import logger from "../logger";
 
 export function sortCompare<T>(left: T, right: T): -1 | 0 | 1 {
   if (left < right) {
@@ -53,3 +56,32 @@ export function sortCompareChartVersions(left: ChartVersion, right: ChartVersion
 
   return sortCompare(left.version, right.version);
 }
+
+
+
+export function sortCharts(charts: RawHelmChart[]) {
+  interface ExtendedHelmChart extends RawHelmChart {
+    __version: SemVer;
+  }
+
+  const chartsWithVersion = Array.from(
+    iter.map(
+      charts,
+      (chart => {
+        const __version = coerce(chart.version, { includePrerelease: true, loose: true });
+
+        if (!__version) {
+          logger.warn(`[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));
+}

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

@@ -19,93 +19,91 @@
  * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
  */
 
-import { HelmRepo, HelmRepoManager } from "../helm-repo-manager";
+import { sortCharts } from "../../../common/utils";
+import type { HelmRepo } from "../helm-repo-manager";
 
-export class HelmChartManager {
+const charts = new Map([
-  cache: any = {};
+  ["stable", {
-  private repo: HelmRepo;
+    "invalid-semver": sortCharts([
-
-  constructor(repo: HelmRepo){
-    this.cache = HelmRepoManager.cache;
-    this.repo = repo;
-  }
-
-  public async charts(): Promise<any> {
-    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"
+        digest: "test",
+        created: "now",
       },
       {
         apiVersion: "3.0.0",
         name: "weird-versioning",
         version: "v4.3.0",
         repo: "stable",
-              digest: "test"
+        digest: "test",
+        created: "now",
       },
       {
         apiVersion: "3.0.0",
         name: "weird-versioning",
         version: "I am not semver but more",
         repo: "stable",
-              digest: "test"
+        digest: "test",
+        created: "now",
       },
       {
         apiVersion: "3.0.0",
         name: "weird-versioning",
         version: "v4.4.0",
         repo: "stable",
-              digest: "test"
+        digest: "test",
+        created: "now",
       },
-          ],
+    ]),
-          "apm-server": [
+    "apm-server": sortCharts([
       {
         apiVersion: "3.0.0",
         name: "apm-server",
         version: "2.1.7",
         repo: "stable",
-              digest: "test"
+        digest: "test",
+        created: "now",
       },
       {
         apiVersion: "3.0.0",
         name: "apm-server",
         version: "2.1.6",
         repo: "stable",
-              digest: "test"
+        digest: "test",
+        created: "now",
       }
-          ],
+    ]),
-          "redis": [
+    "redis": sortCharts([
       {
         apiVersion: "3.0.0",
         name: "apm-server",
         version: "1.0.0",
         repo: "stable",
-              digest: "test"
+        digest: "test",
+        created: "now",
       },
       {
         apiVersion: "3.0.0",
         name: "apm-server",
         version: "0.0.9",
         repo: "stable",
-              digest: "test"
+        digest: "test",
+        created: "now",
       }
-          ]
+    ]),
-        });
+  }],
-      case "experiment":
+  ["experiment", {
-        return Promise.resolve({
+    "fairwind": sortCharts([
-          "fairwind": [
       {
         apiVersion: "3.0.0",
         name: "fairwind",
         version: "0.0.1",
         repo: "experiment",
-              digest: "test"
+        digest: "test",
+        created: "now",
       },
       {
         apiVersion: "3.0.0",
@@ -113,19 +111,20 @@ export class HelmChartManager {
         version: "0.0.2",
         repo: "experiment",
         digest: "test",
-              deprecated: true
+        deprecated: true,
+        created: "now",
       }
-          ]
+    ]),
-        });
+  }],
-      case "bitnami":
+  ["bitnami", {
-        return Promise.resolve({
+    "hotdog": sortCharts([
-          "hotdog": [
       {
         apiVersion: "3.0.0",
         name: "hotdog",
         version: "1.0.1",
         repo: "bitnami",
-              digest: "test"
+        digest: "test",
+        created: "now",
       },
       {
         apiVersion: "3.0.0",
@@ -133,27 +132,38 @@ export class HelmChartManager {
         version: "1.0.2",
         repo: "bitnami",
         digest: "test",
+        created: "now",
       }
-          ],
+    ]),
-          "pretzel": [
+    "pretzel": sortCharts([
       {
         apiVersion: "3.0.0",
         name: "pretzel",
         version: "1.0",
         repo: "bitnami",
         digest: "test",
+        created: "now",
       },
       {
         apiVersion: "3.0.0",
         name: "pretzel",
         version: "1.0.1",
         repo: "bitnami",
-              digest: "test"
+        digest: "test",
-            }
+        created: "now",
-          ]
-        });
-      default:
-        return Promise.resolve({});
       }
+    ]),
+  }],
+]);
+
+export class HelmChartManager {
+  constructor(private repo: HelmRepo){ }
+
+  static forRepo(repo: HelmRepo) {
+    return new this(repo);
+  }
+
+  public async charts(): Promise<any> {
+    return charts.get(this.repo.name) ?? {};
   }
 }

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

@@ -31,7 +31,7 @@ describe("Helm Service tests", () => {
     jest.resetAllMocks();
   });
 
-  it("list charts without deprecated ones", async () => {
+  it("list charts with deprecated entries", async () => {
     mockHelmRepoManager.mockReturnValue({
       init: jest.fn(),
       repositories: jest.fn().mockImplementation(async () => {
@@ -52,14 +52,16 @@ describe("Helm Service tests", () => {
             name: "apm-server",
             version: "2.1.7",
             repo: "stable",
-            digest: "test"
+            digest: "test",
+            created: "now",
           },
           {
             apiVersion: "3.0.0",
             name: "apm-server",
             version: "2.1.6",
             repo: "stable",
-            digest: "test"
+            digest: "test",
+            created: "now",
           }
         ],
         "invalid-semver": [
@@ -68,28 +70,32 @@ describe("Helm Service tests", () => {
             name: "weird-versioning",
             version: "v4.4.0",
             repo: "stable",
-            digest: "test"
+            digest: "test",
+            created: "now",
           },
           {
             apiVersion: "3.0.0",
             name: "weird-versioning",
             version: "v4.3.0",
             repo: "stable",
-            digest: "test"
+            digest: "test",
+            created: "now",
           },
           {
             apiVersion: "3.0.0",
             name: "weird-versioning",
             version: "I am not semver",
             repo: "stable",
-            digest: "test"
+            digest: "test",
+            created: "now",
           },
           {
             apiVersion: "3.0.0",
             name: "weird-versioning",
             version: "I am not semver but more",
             repo: "stable",
-            digest: "test"
+            digest: "test",
+            created: "now",
           },
         ],
         "redis": [
@@ -98,18 +104,40 @@ describe("Helm Service tests", () => {
             name: "apm-server",
             version: "1.0.0",
             repo: "stable",
-            digest: "test"
+            digest: "test",
+            created: "now",
           },
           {
             apiVersion: "3.0.0",
             name: "apm-server",
             version: "0.0.9",
             repo: "stable",
-            digest: "test"
+            digest: "test",
+            created: "now",
           }
         ]
       },
-      experiment: {}
+      experiment: {
+        "fairwind": [
+          {
+            apiVersion: "3.0.0",
+            name: "fairwind",
+            version: "0.0.2",
+            repo: "experiment",
+            digest: "test",
+            deprecated: true,
+            created: "now",
+          },
+          {
+            apiVersion: "3.0.0",
+            name: "fairwind",
+            version: "0.0.1",
+            repo: "experiment",
+            digest: "test",
+            created: "now",
+          },
+        ]
+      }
     });
   });
 
@@ -134,13 +162,15 @@ describe("Helm Service tests", () => {
             version: "1.0.2",
             repo: "bitnami",
             digest: "test",
+            created: "now",
           },
           {
             apiVersion: "3.0.0",
             name: "hotdog",
             version: "1.0.1",
             repo: "bitnami",
-            digest: "test"
+            digest: "test",
+            created: "now",
           },
         ],
         "pretzel": [
@@ -150,13 +180,15 @@ describe("Helm Service tests", () => {
             version: "1.0.1",
             repo: "bitnami",
             digest: "test",
+            created: "now",
           },
           {
             apiVersion: "3.0.0",
             name: "pretzel",
             version: "1.0",
             repo: "bitnami",
-            digest: "test"
+            digest: "test",
+            created: "now",
           }
         ]
       }

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

@@ -20,27 +20,25 @@
  */
 
 import fs from "fs";
+import v8 from "v8";
 import * as yaml from "js-yaml";
-import { HelmRepo, HelmRepoManager } from "./helm-repo-manager";
+import type { HelmRepo } from "./helm-repo-manager";
 import logger from "../logger";
 import { promiseExec } from "../promise-exec";
 import { helmCli } from "./helm-cli";
 import type { RepoHelmChartList } from "../../common/k8s-api/endpoints/helm-charts.api";
-
+import { sortCharts } from "../../common/utils";
-type CachedYaml = {
-  entries: RepoHelmChartList
-};
 
 export class HelmChartManager {
-  protected cache: any = {};
+  static #cache = new Map<string, Buffer>();
-  protected repo: HelmRepo;
 
-  constructor(repo: HelmRepo){
+  private constructor(protected repo: HelmRepo) {}
-    this.cache = HelmRepoManager.cache;
+
-    this.repo = repo;
+  static forRepo(repo: HelmRepo) {
+    return new this(repo);
   }
 
-  public async chart(name: string) {
+  public async chartVersions(name: string) {
     const charts = await this.charts();
 
     return charts[name];
@@ -48,9 +46,7 @@ export class HelmChartManager {
 
   public async charts(): Promise<RepoHelmChartList> {
     try {
-      const cachedYaml = await this.cachedYaml();
+      return await this.cachedYaml();
-
-      return cachedYaml["entries"];
     } catch(error) {
       logger.error("HELM-CHART-MANAGER]: failed to list charts", { error });
 
@@ -58,48 +54,61 @@ export class HelmChartManager {
     }
   }
 
-  public async getReadme(name: string, version = "") {
+  private async executeCommand(action: string, name: string, version?: string) {
     const helm = await helmCli.binaryPath();
+    const cmd = [`"${helm}" ${action} ${this.repo.name}/${name}`];
 
-    if(version && version != "") {
+    if (version) {
-      const { stdout } = await promiseExec(`"${helm}" show readme ${this.repo.name}/${name} --version ${version}`).catch((error) => { throw(error.stderr);});
+      cmd.push("--version", version);
-
+    }
-      return stdout;
+
-    } else {
+    try {
-      const { stdout } = await promiseExec(`"${helm}" show readme ${this.repo.name}/${name}`).catch((error) => { throw(error.stderr);});
+      const { stdout } = await promiseExec(cmd.join(" "));
 
       return stdout;
+    } catch (error) {
+      throw error.stderr || error;
     }
   }
 
-  public async getValues(name: string, version = "") {
+  public async getReadme(name: string, version?: string) {
-    const helm = await helmCli.binaryPath();
+    return this.executeCommand("show readme", name, version);
-
-    if(version && version != "") {
-      const { stdout } = await promiseExec(`"${helm}" show values ${this.repo.name}/${name} --version ${version}`).catch((error) => { throw(error.stderr);});
-
-      return stdout;
-    } else {
-      const { stdout } = await promiseExec(`"${helm}" show values ${this.repo.name}/${name}`).catch((error) => { throw(error.stderr);});
-
-      return stdout;
-    }
   }
 
-  protected async cachedYaml(): Promise<CachedYaml> {
+  public async getValues(name: string, version?: string) {
-    if (!(this.repo.name in this.cache)) {
+    return this.executeCommand("show values", name, version);
+  }
+
+  protected async cachedYaml(): Promise<RepoHelmChartList> {
+    if (!HelmChartManager.#cache.has(this.repo.name)) {
       const cacheFile = await fs.promises.readFile(this.repo.cacheFilePath, "utf-8");
-      const data = yaml.safeLoad(cacheFile);
+      const { entries } = yaml.safeLoad(cacheFile) as { entries: RepoHelmChartList };
 
-      for(const key in data["entries"]) {
+      /**
-        data["entries"][key].forEach((version: any) => {
+       * Do some initial preprocessing on the data, so as to avoid needing to do it later
-          version["repo"] = this.repo.name;
+       * 1. Set the repo name
-          version["created"] = Date.parse(version.created).toString();
+       * 2. Normalize the created date
-        });
+       * 3. Filter out deprecated items
-      }
+       */
-      this.cache[this.repo.name] = Buffer.from(JSON.stringify(data));
+
+      const normalized = Object.fromEntries(
+        Object.entries(entries)
+          .map(([name, charts]) => [
+            name,
+            sortCharts(
+              charts.map(chart => ({
+                ...chart,
+                created: Date.parse(chart.created).toString(),
+                repo: this.repo.name,
+              })),
+            ),
+          ] as const)
+          .filter(([, charts]) => !charts.every(chart => chart.deprecated))
+      );
+
+      HelmChartManager.#cache.set(this.repo.name, v8.serialize(normalized));
     }
 
-    return JSON.parse(this.cache[this.repo.name].toString());
+    return v8.deserialize(HelmChartManager.#cache.get(this.repo.name));
   }
 }

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

@@ -50,8 +50,6 @@ export interface HelmRepo {
 }
 
 export class HelmRepoManager extends Singleton {
-  static cache = {}; // todo: remove implicit updates in helm-chart-manager.ts
-
   protected repos: HelmRepo[];
   protected helmEnv: HelmEnv;
   protected initialized: boolean;
@@ -97,6 +95,12 @@ export class HelmRepoManager extends Singleton {
     return env;
   }
 
+  public async repo(name: string): Promise<HelmRepo> {
+    const repos = await this.repositories();
+
+    return repos.find(repo => repo.name === name);
+  }
+
   public async repositories(): Promise<HelmRepo[]> {
     try {
       if (!this.initialized) {

src/main/helm/helm-service.ts

@@ -19,14 +19,11 @@
  * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
  */
 
-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 { HelmChart, HelmChartList, RepoHelmChartList } from "../../common/k8s-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;
@@ -42,43 +39,27 @@ class HelmService {
   }
 
   public async listCharts() {
-    const charts: HelmChartList = {};
     const repositories = await HelmRepoManager.getInstance().repositories();
 
-    for (const repo of repositories) {
+    return Object.fromEntries(
-      charts[repo.name] = {};
+      await Promise.all(repositories.map(async repo => [repo.name, await HelmChartManager.forRepo(repo).charts()]))
-      const manager = new HelmChartManager(repo);
+    );
-      const sortedCharts = this.sortChartsByVersion(await manager.charts());
-      const enabledCharts = this.excludeDeprecatedChartGroups(sortedCharts);
-
-      charts[repo.name] = enabledCharts;
-    }
-
-    return charts;
   }
 
   public async getChart(repoName: string, chartName: string, version = "") {
-    const result = {
+    const repo = await HelmRepoManager.getInstance().repo(repoName);
-      readme: "",
+    const chartManager = HelmChartManager.forRepo(repo);
-      versions: {}
+
+    return {
+      readme: await chartManager.getReadme(chartName, version),
+      versions: await chartManager.chartVersions(chartName),
     };
-    const repos = await HelmRepoManager.getInstance().repositories();
-    const repo = repos.find(repo => repo.name === repoName);
-    const chartManager = new HelmChartManager(repo);
-    const chart = await chartManager.chart(chartName);
-
-    result.readme = await chartManager.getReadme(chartName, version);
-    result.versions = chart;
-
-    return result;
   }
 
   public async getChartValues(repoName: string, chartName: string, version = "") {
-    const repos = await HelmRepoManager.getInstance().repositories();
+    const repo = await HelmRepoManager.getInstance().repo(repoName);
-    const repo = repos.find(repo => repo.name === repoName);
-    const chartManager = new HelmChartManager(repo);
 
-    return chartManager.getValues(chartName, version);
+    return HelmChartManager.forRepo(repo).getValues(chartName, version);
   }
 
   public async listReleases(cluster: Cluster, namespace: string = null) {
@@ -131,58 +112,6 @@ class HelmService {
 
     return { message: output };
   }
-
-  private excludeDeprecatedChartGroups(chartGroups: RepoHelmChartList) {
-    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;
-            }
-          }
-
-          return [name, charts];
-        }
-      )
-    );
-  }
-
-  private sortCharts(charts: HelmChart[]) {
-    interface ExtendedHelmChart extends HelmChart {
-      __version: SemVer;
-    }
-
-    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) {
-    return Object.fromEntries(
-      Object.entries(chartGroups)
-        .map(([name, charts]) => [name, this.sortCharts(charts)])
-    );
-  }
 }
 
 export const helmService = new HelmService();

src/renderer/components/+apps-helm-charts/helm-chart-details.scss

@@ -30,6 +30,12 @@
     box-sizing: content-box;
   }
 
+  .Select__option {
+    span.deprecated {
+      text-decoration: line-through;
+    }
+  }
+
   .intro-contents {
     .description {
       font-weight: bold;

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

@@ -33,12 +33,19 @@ import { Button } from "../button";
 import { Select, SelectOption } from "../select";
 import { createInstallChartTab } from "../dock/install-chart.store";
 import { Badge } from "../badge";
+import { Tooltip, withStyles } from "@material-ui/core";
 
 interface Props {
   chart: HelmChart;
   hideDetails(): void;
 }
 
+const LargeTooltip = withStyles({
+  tooltip: {
+    fontSize: "var(--font-size-small)",
+  }
+})(Tooltip);
+
 @observer
 export class HelmChartDetails extends Component<Props> {
   @observable chartVersions: HelmChart[];
@@ -73,15 +80,15 @@ export class HelmChartDetails extends Component<Props> {
   });
 
   @boundMethod
-  async onVersionChange({ value: version }: SelectOption<string>) {
+  async onVersionChange({ value: chart }: SelectOption<HelmChart>) {
-    this.selectedChart = this.chartVersions.find(chart => chart.version === version);
+    this.selectedChart = chart;
     this.readme = null;
 
     try {
       this.abortController?.abort();
       this.abortController = new AbortController();
       const { chart: { name, repo } } = this.props;
-      const { readme } = await getChartDetails(repo, name, { version, reqInit: { signal: this.abortController.signal }});
+      const { readme } = await getChartDetails(repo, name, { version: chart.version, reqInit: { signal: this.abortController.signal }});
 
       this.readme = readme;
     } catch (error) {
@@ -115,7 +122,19 @@ export class HelmChartDetails extends Component<Props> {
             <Select
               themeName="outlined"
               menuPortalTarget={null}
-              options={chartVersions.map(chart => chart.version)}
+              options={chartVersions.map(chart => ({
+                label: (
+                  chart.deprecated
+                    ? (
+                      <LargeTooltip title="Deprecated" placement="left">
+                        <span className="deprecated">{chart.version}</span>
+                      </LargeTooltip>
+                    )
+                    : chart.version
+                ),
+                value: chart,
+              }))}
+              isOptionDisabled={({ value: chart }) => chart.deprecated}
               value={selectedChart.getVersion()}
               onChange={onVersionChange}
             />