Fix crash when upgrading release (#6626)

* Fix crash when upgrading release

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Fix crash when upgrading helm releases

- Fixes not being able to upgrade helm releases as well.

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Fix tests

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Fix test failures

Signed-off-by: Sebastian Malton <sebastian@malton.name>

Signed-off-by: Sebastian Malton <sebastian@malton.name>

src/common/k8s-api/endpoints/helm-releases.api/request-update.injectable.ts

@@ -3,9 +3,9 @@
  * Licensed under MIT License. See LICENSE in root directory for more information.
  */
 import { getInjectable } from "@ogre-tools/injectable";
-import yaml from "js-yaml";
 import { apiBaseInjectionToken } from "../../api-base";
 import { urlBuilderFor } from "../../../utils/buildUrl";
+import type { AsyncResult } from "../../../utils/async-result";
 
 interface HelmReleaseUpdatePayload {
   repo: string;
@@ -18,7 +18,7 @@ export type RequestHelmReleaseUpdate = (
   name: string,
   namespace: string,
   payload: HelmReleaseUpdatePayload
-) => Promise<{ updateWasSuccessful: true } | { updateWasSuccessful: false; error: unknown }>;
+) => Promise<AsyncResult<void, unknown>>;
 
 const requestUpdateEndpoint = urlBuilderFor("/v2/releases/:namespace/:name");
 
@@ -28,20 +28,20 @@ const requestHelmReleaseUpdateInjectable = getInjectable({
   instantiate: (di): RequestHelmReleaseUpdate => {
     const apiBase = di.inject(apiBaseInjectionToken);
 
-    return async (name, namespace, { repo, chart, values, ...data }) => {
+    return async (name, namespace, { repo, chart, values, version }) => {
       try {
         await apiBase.put(requestUpdateEndpoint.compile({ name, namespace }), {
           data: {
             chart: `${repo}/${chart}`,
-            values: yaml.load(values),
-            ...data,
+            values,
+            version,
           },
         });
       } catch (e) {
-        return { updateWasSuccessful: false, error: e };
+        return { callWasSuccessful: false, error: e };
       }
 
-      return { updateWasSuccessful: true };
+      return { callWasSuccessful: true };
     };
   },
 });

src/features/helm-charts/add-custom-helm-repository-in-preferences.test.ts

@@ -169,7 +169,10 @@ describe("add custom helm repository in preferences", () => {
               expect(execFileMock).toHaveBeenCalledWith(
                 "some-helm-binary-path",
                 ["repo", "add", "some-custom-repository", "http://some.url"],
-                { "maxBuffer": 34359738368 },
+                {
+                  maxBuffer: 34359738368,
+                  env: {},
+                },
               );
             });
 
@@ -373,7 +376,10 @@ describe("add custom helm repository in preferences", () => {
                     "--cert-file",
                     "some-cert-file",
                   ],
-                  { "maxBuffer": 34359738368 },
+                  {
+                    maxBuffer: 34359738368,
+                    env: {},
+                  },
                 );
               });
             });

src/features/helm-charts/add-helm-repository-from-list-in-preferences.test.ts

@@ -118,7 +118,10 @@ describe("add helm repository from list in preferences", () => {
             expect(execFileMock).toHaveBeenCalledWith(
               "some-helm-binary-path",
               ["repo", "add", "Some to be added repository", "some-other-url"],
-              { "maxBuffer": 34359738368 },
+              {
+                maxBuffer: 34359738368,
+                env: {},
+              },
             );
           });
 
@@ -232,7 +235,10 @@ describe("add helm repository from list in preferences", () => {
                     expect(execFileMock).toHaveBeenCalledWith(
                       "some-helm-binary-path",
                       ["repo", "remove", "Some already active repository"],
-                      { "maxBuffer": 34359738368 },
+                      {
+                        maxBuffer: 34359738368,
+                        env: {},
+                      },
                     );
                   });
 

src/features/helm-charts/listing-active-helm-repositories-in-preferences.test.ts

@@ -69,7 +69,10 @@ describe("listing active helm repositories in preferences", () => {
       expect(execFileMock).toHaveBeenCalledWith(
         "some-helm-binary-path",
         ["env"],
-        { "maxBuffer": 34359738368 },
+        {
+          maxBuffer: 34359738368,
+          env: {},
+        },
       );
     });
 
@@ -77,7 +80,10 @@ describe("listing active helm repositories in preferences", () => {
       expect(execFileMock).not.toHaveBeenCalledWith(
         "some-helm-binary-path",
         ["repo", "update"],
-        { "maxBuffer": 34359738368 },
+        {
+          maxBuffer: 34359738368,
+          env: {},
+        },
       );
     });
 
@@ -222,7 +228,10 @@ describe("listing active helm repositories in preferences", () => {
         expect(execFileMock).toHaveBeenCalledWith(
           "some-helm-binary-path",
           ["repo", "update"],
-          { "maxBuffer": 34359738368 },
+          {
+            maxBuffer: 34359738368,
+            env: {},
+          },
         );
       });
 
@@ -289,7 +298,10 @@ describe("listing active helm repositories in preferences", () => {
           expect(execFileMock).toHaveBeenCalledWith(
             "some-helm-binary-path",
             ["repo", "add", "bitnami", "https://charts.bitnami.com/bitnami"],
-            { "maxBuffer": 34359738368 },
+            {
+              maxBuffer: 34359738368,
+              env: {},
+            },
           );
         });
 
@@ -434,7 +446,10 @@ describe("listing active helm repositories in preferences", () => {
             expect(execFileMock).not.toHaveBeenCalledWith(
               "some-helm-binary-path",
               ["repo", "add", "bitnami", "https://charts.bitnami.com/bitnami"],
-              { "maxBuffer": 34359738368 },
+              {
+                maxBuffer: 34359738368,
+                env: {},
+              },
             );
           });
 

src/features/helm-charts/remove-helm-repository-from-list-of-active-repository-in-preferences.test.ts

@@ -85,7 +85,10 @@ describe("remove helm repository from list of active repositories in preferences
           expect(execFileMock).toHaveBeenCalledWith(
             "some-helm-binary-path",
             ["repo", "remove", "some-active-repository"],
-            { "maxBuffer": 34359738368 },
+            {
+              maxBuffer: 34359738368,
+              env: {},
+            },
           );
         });
 

src/features/helm-releases/showing-details-for-helm-release.test.ts

@@ -552,7 +552,7 @@ describe("showing details for helm release", () => {
                           requestHelmReleaseConfigurationMock.mockClear();
 
                           await requestHelmReleaseUpdateMock.resolve({
-                            updateWasSuccessful: true,
+                            callWasSuccessful: true,
                           });
                         });
 
@@ -591,7 +591,7 @@ describe("showing details for helm release", () => {
                           requestHelmReleaseConfigurationMock.mockClear();
 
                           await requestHelmReleaseUpdateMock.resolve({
-                            updateWasSuccessful: false,
+                            callWasSuccessful: false,
                             error: "some-error",
                           });
                         });

src/main/helm/exec-helm/exec-env.global-override-for-injectable.ts

@@ -0,0 +1,10 @@
+/**
+ * Copyright (c) OpenLens Authors. All rights reserved.
+ * Licensed under MIT License. See LICENSE in root directory for more information.
+ */
+
+import { computed } from "mobx";
+import { getGlobalOverride } from "../../../common/test-utils/get-global-override";
+import execHelmEnvInjectable from "./exec-env.injectable";
+
+export default getGlobalOverride(execHelmEnvInjectable, () => computed(() => ({})));

src/main/helm/exec-helm/exec-env.injectable.ts

@@ -0,0 +1,26 @@
+/**
+ * Copyright (c) OpenLens Authors. All rights reserved.
+ * Licensed under MIT License. See LICENSE in root directory for more information.
+ */
+import { getInjectable } from "@ogre-tools/injectable";
+import { computed } from "mobx";
+import userStoreInjectable from "../../../common/user-store/user-store.injectable";
+
+const execHelmEnvInjectable = getInjectable({
+  id: "exec-helm-env",
+  instantiate: (di) => {
+    const userStore = di.inject(userStoreInjectable);
+
+    return computed(() => {
+      const {
+        HTTPS_PROXY = userStore.httpsProxy,
+        ...env
+      } = process.env;
+
+      return { HTTPS_PROXY, ...env } as Partial<Record<string, string>>;
+    });
+  },
+  causesSideEffects: true,
+});
+
+export default execHelmEnvInjectable;

src/main/helm/exec-helm/exec-helm.injectable.ts

@@ -7,6 +7,7 @@ import type { ExecFileException } from "child_process";
 import execFileInjectable from "../../../common/fs/exec-file.injectable";
 import type { AsyncResult } from "../../../common/utils/async-result";
 import helmBinaryPathInjectable from "../helm-binary-path.injectable";
+import execHelmEnvInjectable from "./exec-env.injectable";
 
 export type ExecHelm = (args: string[]) => Promise<AsyncResult<string, ExecFileException & { stderr: string }>>;
 
@@ -15,10 +16,12 @@ const execHelmInjectable = getInjectable({
 
   instantiate: (di): ExecHelm => {
     const execFile = di.inject(execFileInjectable);
+    const execHelmEnv = di.inject(execHelmEnvInjectable);
     const helmBinaryPath = di.inject(helmBinaryPathInjectable);
 
     return async (args) => execFile(helmBinaryPath, args, {
       maxBuffer: 32 * 1024 * 1024 * 1024, // 32 MiB
+      env: execHelmEnv.get(),
     });
   },
 });

src/main/helm/helm-service/update-helm-release.injectable.ts

@@ -5,16 +5,15 @@
 import { getInjectable } from "@ogre-tools/injectable";
 import type { Cluster } from "../../../common/cluster/cluster";
 import loggerInjectable from "../../../common/logger.injectable";
-import type { JsonObject } from "type-fest";
-import { execHelm } from "../exec";
 import tempy from "tempy";
-import fse from "fs-extra";
-import yaml from "js-yaml";
 import getHelmReleaseInjectable from "./get-helm-release.injectable";
+import writeFileInjectable from "../../../common/fs/write-file.injectable";
+import removePathInjectable from "../../../common/fs/remove-path.injectable";
+import execHelmInjectable from "../exec-helm/exec-helm.injectable";
 
 export interface UpdateChartArgs {
   chart: string;
-  values: JsonObject;
+  values: string;
   version: string;
 }
 
@@ -24,40 +23,42 @@ const updateHelmReleaseInjectable = getInjectable({
   instantiate: (di) => {
     const logger = di.inject(loggerInjectable);
     const getHelmRelease = di.inject(getHelmReleaseInjectable);
+    const writeFile = di.inject(writeFileInjectable);
+    const removePath = di.inject(removePathInjectable);
+    const execHelm = di.inject(execHelmInjectable);
 
     return async (cluster: Cluster, releaseName: string, namespace: string, data: UpdateChartArgs) => {
       const proxyKubeconfig = await cluster.getProxyKubeconfigPath();
-
-      logger.debug("Upgrade release");
-
       const valuesFilePath = tempy.file({ name: "values.yaml" });
 
-      await fse.writeFile(valuesFilePath, yaml.dump(data.values));
-
-      const args = [
-        "upgrade",
-        releaseName,
-        data.chart,
-        "--version", data.version,
-        "--values", valuesFilePath,
-        "--namespace", namespace,
-        "--kubeconfig", proxyKubeconfig,
-      ];
+      logger.debug(`[HELM]: upgrading "${releaseName}" in "${namespace}" to ${data.version}`);
 
       try {
-        const output = await execHelm(args);
+        await writeFile(valuesFilePath, data.values);
+
+        const result = await execHelm([
+          "upgrade",
+          releaseName,
+          data.chart,
+          "--version", data.version,
+          "--values", valuesFilePath,
+          "--namespace", namespace,
+          "--kubeconfig", proxyKubeconfig,
+        ]);
+
+        if (result.callWasSuccessful === false) {
+          throw result.error; // keep the same interface
+        }
 
         return {
-          log: output,
+          log: result.response,
           release: await getHelmRelease(cluster, releaseName, namespace),
         };
       } finally {
-        await fse.unlink(valuesFilePath);
+        await removePath(valuesFilePath);
       }
     };
   },
-
-  causesSideEffects: true,
 });
 
 export default updateHelmReleaseInjectable;

src/main/routes/helm/releases/update-release-route.injectable.ts

@@ -17,8 +17,8 @@ const updateChartArgsValidator = Joi.object<UpdateChartArgs, true, UpdateChartAr
     .string()
     .required(),
   values: Joi
-    .object()
-    .unknown(true),
+    .string()
+    .required(),
 });
 
 const updateReleaseRouteInjectable = getRouteInjectable({

src/renderer/components/+helm-charts/helm-charts/versions.injectable.ts

@@ -14,10 +14,7 @@ const helmChartVersionsInjectable = getInjectable({
 
   instantiate: (di, release) => {
     const helmCharts = di.inject(helmChartsInjectable);
-
-    const requestVersionsOfHelmChart = di.inject(
-      requestVersionsOfHelmChartInjectable,
-    );
+    const requestVersionsOfHelmChart = di.inject(requestVersionsOfHelmChartInjectable);
 
     return asyncComputed({
       getValueFromObservedPromise: async () => {
@@ -25,8 +22,6 @@ const helmChartVersionsInjectable = getInjectable({
 
         return requestVersionsOfHelmChart(release, helmCharts.value.get());
       },
-
-      valueWhenPending: [],
     });
   },
 

src/renderer/components/+helm-releases/release-details/release-details-model/release-details-model.injectable.tsx

@@ -128,7 +128,7 @@ export class ReleaseDetailsModel {
         this.configuration.isSaving.set(false);
       });
 
-      if (!result.updateWasSuccessful) {
+      if (!result.callWasSuccessful) {
         this.dependencies.showCheckedErrorNotification(
           result.error,
           "Unknown error occured while updating release",

src/renderer/components/+helm-releases/update-release/update-release.injectable.ts

@@ -12,14 +12,14 @@ const updateReleaseInjectable = getInjectable({
 
   instantiate: (di): RequestHelmReleaseUpdate => {
     const releases = di.inject(releasesInjectable);
-    const callForHelmReleaseUpdate = di.inject(requestHelmReleaseUpdateInjectable);
+    const requestHelmReleaseUpdate = di.inject(requestHelmReleaseUpdateInjectable);
 
     return async (
       name,
       namespace,
       payload,
     ) => {
-      const result = await callForHelmReleaseUpdate(name, namespace, payload);
+      const result = await requestHelmReleaseUpdate(name, namespace, payload);
 
       releases.invalidate();
 

src/renderer/components/dock/upgrade-chart/upgrade-chart-model.injectable.ts

@@ -13,6 +13,7 @@ import releasesInjectable from "../../+helm-releases/releases.injectable";
 import updateReleaseInjectable from "../../+helm-releases/update-release/update-release.injectable";
 import type { HelmRelease } from "../../../../common/k8s-api/endpoints/helm-releases.api";
 import requestHelmReleaseConfigurationInjectable from "../../../../common/k8s-api/endpoints/helm-releases.api/request-configuration.injectable";
+import type { AsyncResult } from "../../../../common/utils/async-result";
 import { waitUntilDefined } from "../../../utils";
 import type { SelectOption } from "../../select";
 import type { DockTab } from "../dock/store";
@@ -31,11 +32,7 @@ export interface UpgradeChartModel {
     readonly value: IComputedValue<HelmChartVersion | undefined>;
     set: (value: SingleValue<SelectOption<HelmChartVersion>>) => void;
   };
-  submit: () => Promise<UpgradeChartSubmitResult>;
-}
-
-export interface UpgradeChartSubmitResult {
-  completedSuccessfully: boolean;
+  submit: () => Promise<AsyncResult<void, string>>;
 }
 
 const upgradeChartModelInjectable = getInjectable({
@@ -105,13 +102,23 @@ const upgradeChartModelInjectable = getInjectable({
       submit: async () => {
         const selectedVersion = version.value.get();
 
-        if (!selectedVersion || configrationEditError.get()) {
+        if (!selectedVersion) {
           return {
-            completedSuccessfully: false,
+            callWasSuccessful: false,
+            error: "No selected version",
           };
         }
 
-        await updateRelease(
+        const editError = configrationEditError.get();
+
+        if (editError) {
+          return {
+            callWasSuccessful: false,
+            error: editError,
+          };
+        }
+
+        const result = await updateRelease(
           release.getName(),
           release.getNs(),
           {
@@ -120,10 +127,16 @@ const upgradeChartModelInjectable = getInjectable({
             ...selectedVersion,
           },
         );
-        storedConfiguration.invalidate();
+
+        if (result.callWasSuccessful === true) {
+          storedConfiguration.invalidate();
+
+          return { callWasSuccessful: true };
+        }
 
         return {
-          completedSuccessfully: true,
+          callWasSuccessful: false,
+          error: String(result.error),
         };
       },
     };

src/renderer/components/dock/upgrade-chart/view.tsx

@@ -33,20 +33,20 @@ interface Dependencies {
 export class NonInjectedUpgradeChart extends React.Component<UpgradeChartProps & Dependencies> {
   upgrade = async () => {
     const { model } = this.props;
-    const { completedSuccessfully } = await model.submit();
+    const result = await model.submit();
 
-    if (completedSuccessfully) {
+    if (result.callWasSuccessful) {
       return (
         <p>
           {"Release "}
           <b>{model.release.getName()}</b>
           {" successfully upgraded to version "}
-          <b>{model.version.value.get()}</b>
+          <b>{model.version.value.get()?.version}</b>
         </p>
       );
     }
 
-    return null;
+    throw result.error;
   };
 
   render() {