Cleanup HelmRepoManager and <HelmCharts> (#4564)

integration/__tests__/app-preferences.tests.ts

@@ -60,9 +60,7 @@ describe("preferences page tests", () => {
   // Skipping, but will turn it on again in the follow up PR
   it.skip("ensures helm repos", async () => {
     await window.click("[data-testid=kubernetes-tab]");
-    await window.waitForSelector("[data-testid=repository-name]", {
+    await window.waitForSelector("[data-testid=repository-name]");
-      timeout: 140_000,
-    });
     await window.click("#HelmRepoSelect");
     await window.waitForSelector("div.Select__option");
   }, 10*60*1000);

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

@@ -104,10 +104,8 @@ export async function upgradeRelease(name: string, chart: string, values: any, n
   ];
 
   try {
-    const output = await execHelm(args);
-
     return {
-      log: output,
+      log: await execHelm(args),
       release: getRelease(name, namespace, kubeconfigPath, kubectlPath),
     };
   } finally {

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

@@ -49,9 +49,9 @@ async function execHelm(args: string[], options?: BaseEncodingOptions & ExecFile
 export class HelmRepoManager extends Singleton {
   protected repos: HelmRepo[];
   protected helmEnv: HelmEnv;
-  protected initialized: boolean;
+  protected didUpdateOnce: boolean;
 
-  public static async loadAvailableRepos(): Promise<HelmRepo[]> {
+  public async loadAvailableRepos(): Promise<HelmRepo[]> {
     const res = await customRequestPromise({
       uri: "https://github.com/lensapp/artifact-hub-repositories/releases/download/latest/repositories.json",
       json: true,
@@ -59,21 +59,31 @@ export class HelmRepoManager extends Singleton {
       timeout: 10000,
     });
 
-    return orderBy<HelmRepo>(res.body, repo => repo.name);
+    return orderBy(res.body as HelmRepo[], repo => repo.name);
   }
 
-  private async init() {
+  private async ensureInitialized() {
     helmCli.setLogger(logger);
     await helmCli.ensureBinary();
 
-    if (!this.initialized) {
+    this.helmEnv ??= await this.parseHelmEnv();
-      this.helmEnv = await HelmRepoManager.parseHelmEnv();
+
-      await HelmRepoManager.update();
+    const repos = await this.list();
-      this.initialized = true;
+
+    if (repos.length === 0) {
+      await this.addRepo({
+        name: "bitnami",
+        url: "https://charts.bitnami.com/bitnami",
+      });
+    }
+
+    if (!this.didUpdateOnce) {
+      await this.update();
+      this.didUpdateOnce = true;
     }
   }
 
-  protected static async parseHelmEnv() {
+  protected async parseHelmEnv() {
     const output = await execHelm(["env"]);
     const lines = output.split(/\r?\n/); // split by new line feed
     const env: HelmEnv = {};
@@ -95,38 +105,28 @@ export class HelmRepoManager extends Singleton {
     return repos.find(repo => repo.name === name);
   }
 
-  private async readConfig(): Promise<HelmRepoConfig> {
+  private async list(): Promise<HelmRepo[]> {
     try {
       const rawConfig = await readFile(this.helmEnv.HELM_REPOSITORY_CONFIG, "utf8");
-      const parsedConfig = yaml.load(rawConfig);
+      const parsedConfig = yaml.load(rawConfig) as HelmRepoConfig;
 
       if (typeof parsedConfig === "object" && parsedConfig) {
-        return parsedConfig as HelmRepoConfig;
+        return parsedConfig.repositories;
       }
     } catch {
       // ignore error
     }
 
-    return {
+    return [];
-      repositories: [],
-    };
   }
 
   public async repositories(): Promise<HelmRepo[]> {
     try {
-      if (!this.initialized) {
+      await this.ensureInitialized();
-        await this.init();
-      }
 
-      const { repositories } = await this.readConfig();
+      const repos = await this.list();
 
-      if (!repositories.length) {
+      return repos.map(repo => ({
-        await HelmRepoManager.addRepo({ name: "bitnami", url: "https://charts.bitnami.com/bitnami" });
-
-        return await this.repositories();
-      }
-
-      return repositories.map(repo => ({
         ...repo,
         cacheFilePath: `${this.helmEnv.HELM_REPOSITORY_CACHE}/${repo.name}-index.yaml`,
       }));
@@ -137,25 +137,14 @@ export class HelmRepoManager extends Singleton {
     }
   }
 
-  public static async update() {
+  public async update() {
     return execHelm([
       "repo",
       "update",
     ]);
   }
 
-  public static async addRepo({ name, url }: HelmRepo) {
+  public async addRepo({ name, url, insecureSkipTlsVerify, username, password, caFile, keyFile, certFile }: HelmRepo) {
-    logger.info(`[HELM]: adding repo "${name}" from ${url}`);
-
-    return execHelm([
-      "repo",
-      "add",
-      name,
-      url,
-    ]);
-  }
-
-  public static async addCustomRepo({ name, url, insecureSkipTlsVerify, username, password, caFile, keyFile, certFile }: HelmRepo) {
     logger.info(`[HELM]: adding repo ${name} from ${url}`);
     const args = [
       "repo",
@@ -191,7 +180,7 @@ export class HelmRepoManager extends Singleton {
     return execHelm(args);
   }
 
-  public static async removeRepo({ name, url }: HelmRepo): Promise<string> {
+  public async removeRepo({ name, url }: HelmRepo): Promise<string> {
     logger.info(`[HELM]: removing repo ${name} (${url})`);
 
     return execHelm([

src/renderer/components/+preferences/add-helm-repo-dialog.tsx

@@ -7,7 +7,7 @@ import "./add-helm-repo-dialog.scss";
 
 import React from "react";
 import type { FileFilter } from "electron";
-import { observable, makeObservable } from "mobx";
+import { observable, makeObservable, action } from "mobx";
 import { observer } from "mobx-react";
 import { Dialog, DialogProps } from "../dialog";
 import { Wizard, WizardStep } from "../wizard";
@@ -35,10 +35,12 @@ const dialogState = observable.object({
   isOpen: false,
 });
 
+function getEmptyRepo(): HelmRepo {
+  return { name: "", url: "", username: "", password: "", insecureSkipTlsVerify: false, caFile: "", keyFile: "", certFile: "" };
+}
+
 @observer
 export class AddHelmRepoDialog extends React.Component<Props> {
-  private emptyRepo = { name: "", url: "", username: "", password: "", insecureSkipTlsVerify: false, caFile:"", keyFile: "", certFile: "" };
-
   private static keyExtensions = ["key", "keystore", "jks", "p12", "pfx", "pem"];
   private static certExtensions = ["crt", "cer", "ca-bundle", "p7b", "p7c", "p7s", "p12", "pfx", "pem"];
 
@@ -55,12 +57,13 @@ export class AddHelmRepoDialog extends React.Component<Props> {
     dialogState.isOpen = false;
   }
 
-  @observable helmRepo : HelmRepo = this.emptyRepo;
+  @observable helmRepo = getEmptyRepo();
   @observable showOptions = false;
 
+  @action
     close = () => {
       AddHelmRepoDialog.close();
-    this.helmRepo = this.emptyRepo;
+      this.helmRepo = getEmptyRepo();
       this.showOptions = false;
     };
 
@@ -91,8 +94,8 @@ export class AddHelmRepoDialog extends React.Component<Props> {
 
   async addCustomRepo() {
     try {
-      await HelmRepoManager.addCustomRepo(this.helmRepo);
+      await HelmRepoManager.getInstance().addRepo(this.helmRepo);
-      Notifications.ok(<>Helm repository <b>{this.helmRepo.name}</b> has added</>);
+      Notifications.ok(<>Helm repository <b>{this.helmRepo.name}</b> has been added</>);
       this.props.onAddRepo();
       this.close();
     } catch (err) {
@@ -127,9 +130,9 @@ export class AddHelmRepoDialog extends React.Component<Props> {
           value={this.helmRepo.insecureSkipTlsVerify}
           onChange={v => this.helmRepo.insecureSkipTlsVerify = v}
         />
-        {this.renderFileInput(`Key file`, FileType.KeyFile, AddHelmRepoDialog.keyExtensions)}
+        {this.renderFileInput("Key file", FileType.KeyFile, AddHelmRepoDialog.keyExtensions)}
-        {this.renderFileInput(`Ca file`, FileType.CaFile, AddHelmRepoDialog.certExtensions)}
+        {this.renderFileInput("Ca file", FileType.CaFile, AddHelmRepoDialog.certExtensions)}
-        {this.renderFileInput(`Certificate file`, FileType.CertFile, AddHelmRepoDialog.certExtensions)}
+        {this.renderFileInput("Certificate file", FileType.CertFile, AddHelmRepoDialog.certExtensions)}
         <SubTitle title="Chart Repository Credentials" />
         <Input
           placeholder="Username"

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

@@ -6,7 +6,7 @@
 import styles from "./helm-charts.module.scss";
 
 import React from "react";
-import { action, computed, observable, makeObservable } from "mobx";
+import { computed, observable, makeObservable } from "mobx";
 
 import { HelmRepo, HelmRepoManager } from "../../../main/helm/helm-repo-manager";
 import { Button } from "../button";
@@ -18,10 +18,12 @@ import { observer } from "mobx-react";
 import { RemovableItem } from "./removable-item";
 import { Notice } from "../+extensions/notice";
 import { Spinner } from "../spinner";
+import { noop } from "../../utils";
 
 @observer
 export class HelmCharts extends React.Component {
-  @observable loading = false;
+  @observable loadingRepos = false;
+  @observable loadingAvailableRepos = false;
   @observable repos: HelmRepo[] = [];
   @observable addedRepos = observable.map<string, HelmRepo>();
 
@@ -37,32 +39,42 @@ export class HelmCharts extends React.Component {
     }));
   }
 
-  async componentDidMount() {
+  componentDidMount() {
-    await this.loadRepos();
+    this.loadAvailableRepos().catch(noop);
+    this.loadRepos().catch(noop);
   }
 
-  @action
+  async loadAvailableRepos() {
-  async loadRepos() {
+    this.loadingAvailableRepos = true;
-    this.loading = true;
 
     try {
       if (!this.repos.length) {
-        this.repos = await HelmRepoManager.loadAvailableRepos();
+        this.repos = await HelmRepoManager.getInstance().loadAvailableRepos();
       }
-      const repos = await HelmRepoManager.getInstance().repositories(); // via helm-cli
-
-      this.addedRepos.clear();
-      repos.forEach(repo => this.addedRepos.set(repo.name, repo));
     } catch (err) {
       Notifications.error(String(err));
     }
 
-    this.loading = false;
+    this.loadingAvailableRepos = false;
+  }
+
+  async loadRepos() {
+    this.loadingRepos = true;
+
+    try {
+      const repos = await HelmRepoManager.getInstance().repositories(); // via helm-cli
+
+      this.addedRepos.replace(repos.map(repo => [repo.name, repo]));
+    } catch (err) {
+      Notifications.error(String(err));
+    }
+
+    this.loadingRepos = false;
   }
 
   async addRepo(repo: HelmRepo) {
     try {
-      await HelmRepoManager.addRepo(repo);
+      await HelmRepoManager.getInstance().addRepo(repo);
       this.addedRepos.set(repo.name, repo);
     } catch (err) {
       Notifications.error(<>Adding helm branch <b>{repo.name}</b> has failed: {String(err)}</>);
@@ -71,7 +83,7 @@ export class HelmCharts extends React.Component {
 
   async removeRepo(repo: HelmRepo) {
     try {
-      await HelmRepoManager.removeRepo(repo);
+      await HelmRepoManager.getInstance().removeRepo(repo);
       this.addedRepos.delete(repo.name);
     } catch (err) {
       Notifications.error(
@@ -80,17 +92,14 @@ export class HelmCharts extends React.Component {
     }
   }
 
-  onRepoSelect = async ({ value: repo }: SelectOption<HelmRepo>) => {
+  onRepoSelect = async ({ value: repo }: SelectOption<HelmRepo>): Promise<void> => {
     const isAdded = this.addedRepos.has(repo.name);
 
     if (isAdded) {
-      Notifications.ok(<>Helm branch <b>{repo.name}</b> already in use</>);
+      return void Notifications.ok(<>Helm branch <b>{repo.name}</b> already in use</>);
-
-      return;
     }
-    this.loading = true;
+
     await this.addRepo(repo);
-    this.loading = false;
   };
 
   formatOptionLabel = ({ value: repo }: SelectOption<HelmRepo>) => {
@@ -107,7 +116,7 @@ export class HelmCharts extends React.Component {
   renderRepositories() {
     const repos = Array.from(this.addedRepos);
 
-    if (this.loading) {
+    if (this.loadingRepos) {
       return <div className="pt-5 relative"><Spinner center/></div>;
     }
 
@@ -137,8 +146,8 @@ export class HelmCharts extends React.Component {
         <div className="flex gaps">
           <Select id="HelmRepoSelect"
             placeholder="Repositories"
-            isLoading={this.loading}
+            isLoading={this.loadingAvailableRepos}
-            isDisabled={this.loading}
+            isDisabled={this.loadingAvailableRepos}
             options={this.options}
             onChange={this.onRepoSelect}
             formatOptionLabel={this.formatOptionLabel}