From 36e8888ecb5de9fc8551669596fb975916d451c1 Mon Sep 17 00:00:00 2001 From: Sebastian Malton Date: Mon, 31 May 2021 16:43:08 -0400 Subject: [PATCH] Improve loading animation when switching release details (#2367) Signed-off-by: Sebastian Malton --- src/common/utils/buildUrl.ts | 8 ++ src/main/helm/helm-release-manager.ts | 10 ++- src/main/helm/helm-service.ts | 12 ++- src/main/routes/helm-route.ts | 8 +- src/main/routes/utils/index.ts | 21 +++++ src/main/routes/utils/parse-query.ts | 34 ++++++++ .../api/endpoints/helm-releases.api.ts | 41 +++++---- .../+apps-releases/release-details.tsx | 84 ++++++++++--------- .../components/ace-editor/ace-editor.scss | 15 +++- .../components/ace-editor/ace-editor.tsx | 3 +- 10 files changed, 166 insertions(+), 70 deletions(-) create mode 100644 src/main/routes/utils/index.ts create mode 100644 src/main/routes/utils/parse-query.ts diff --git a/src/common/utils/buildUrl.ts b/src/common/utils/buildUrl.ts index d373aa634d..8ad88945df 100644 --- a/src/common/utils/buildUrl.ts +++ b/src/common/utils/buildUrl.ts @@ -41,3 +41,11 @@ export function buildURL

(path: str return parts.filter(Boolean).join(""); }; } + +export function buildURLPositional

(path: string | any) { + const builder = buildURL(path); + + return function(params?: P, query?: Q, fragment?: string): string { + return builder({ params, query, fragment }); + }; +} diff --git a/src/main/helm/helm-release-manager.ts b/src/main/helm/helm-release-manager.ts index 852c649ede..492474c151 100644 --- a/src/main/helm/helm-release-manager.ts +++ b/src/main/helm/helm-release-manager.ts @@ -127,10 +127,16 @@ export async function deleteRelease(name: string, namespace: string, pathToKubec } } -export async function getValues(name: string, namespace: string, all: boolean, pathToKubeconfig: string) { +interface GetValuesOptions { + namespace: string; + all?: boolean; + pathToKubeconfig: string; +} + +export async function getValues(name: string, { namespace, all = false, pathToKubeconfig }: GetValuesOptions) { try { const helm = await helmCli.binaryPath(); - const { stdout, } = await promiseExec(`"${helm}" get values ${name} ${all ? "--all": ""} --output yaml --namespace ${namespace} --kubeconfig ${pathToKubeconfig}`); + const { stdout } = await promiseExec(`"${helm}" get values ${name} ${all ? "--all" : ""} --output yaml --namespace ${namespace} --kubeconfig ${pathToKubeconfig}`); return stdout; } catch ({ stderr }) { diff --git a/src/main/helm/helm-service.ts b/src/main/helm/helm-service.ts index 73e93e5ac1..01262b6ef8 100644 --- a/src/main/helm/helm-service.ts +++ b/src/main/helm/helm-service.ts @@ -27,6 +27,12 @@ import { HelmChartManager } from "./helm-chart-manager"; import type { HelmChartList, RepoHelmChartList } from "../../renderer/api/endpoints/helm-charts.api"; import { deleteRelease, getHistory, getRelease, getValues, installChart, listReleases, rollback, upgradeRelease } from "./helm-release-manager"; +interface GetReleaseValuesArgs { + cluster: Cluster; + namespace: string; + all: boolean; +} + class HelmService { public async installChart(cluster: Cluster, data: { chart: string; values: {}; name: string; namespace: string; version: string }) { const proxyKubeconfig = await cluster.getProxyKubeconfigPath(); @@ -86,12 +92,12 @@ class HelmService { return getRelease(releaseName, namespace, cluster); } - public async getReleaseValues(cluster: Cluster, releaseName: string, namespace: string, all: boolean) { - const proxyKubeconfig = await cluster.getProxyKubeconfigPath(); + public async getReleaseValues(releaseName: string, { cluster, namespace, all }: GetReleaseValuesArgs) { + const pathToKubeconfig = await cluster.getProxyKubeconfigPath(); logger.debug("Fetch release values"); - return getValues(releaseName, namespace, all, proxyKubeconfig); + return getValues(releaseName, { namespace, all, pathToKubeconfig }); } public async getReleaseHistory(cluster: Cluster, releaseName: string, namespace: string) { diff --git a/src/main/routes/helm-route.ts b/src/main/routes/helm-route.ts index 7a4033d4f0..5e104f6a11 100644 --- a/src/main/routes/helm-route.ts +++ b/src/main/routes/helm-route.ts @@ -21,8 +21,9 @@ import type { LensApiRequest } from "../router"; import { helmService } from "../helm/helm-service"; -import { respondJson, respondText } from "../utils/http-responses"; import logger from "../logger"; +import { respondJson, respondText } from "../utils/http-responses"; +import { getBoolean } from "./utils/parse-query"; export class HelmApiRoute { static async listCharts(request: LensApiRequest) { @@ -122,10 +123,11 @@ export class HelmApiRoute { } static async getReleaseValues(request: LensApiRequest) { - const { cluster, params, response, query } = request; + const { cluster, params: { namespace, release }, response, query } = request; + const all = getBoolean(query, "all"); try { - const result = await helmService.getReleaseValues(cluster, params.release, params.namespace, query.has("all")); + const result = await helmService.getReleaseValues(release, { cluster, namespace, all }); respondText(response, result); } catch (error) { diff --git a/src/main/routes/utils/index.ts b/src/main/routes/utils/index.ts new file mode 100644 index 0000000000..4a7de80225 --- /dev/null +++ b/src/main/routes/utils/index.ts @@ -0,0 +1,21 @@ +/** + * 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. + */ +export * from "./parse-query"; diff --git a/src/main/routes/utils/parse-query.ts b/src/main/routes/utils/parse-query.ts new file mode 100644 index 0000000000..b426f40c28 --- /dev/null +++ b/src/main/routes/utils/parse-query.ts @@ -0,0 +1,34 @@ +/** + * 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. + */ +export function getBoolean(query: URLSearchParams, key: string): boolean { + const value = query.get(key); + + switch (value?.toLowerCase()) { + case "false": + case "f": + case "0": + case null: + case undefined: + return false; + default: + return true; + } +} diff --git a/src/renderer/api/endpoints/helm-releases.api.ts b/src/renderer/api/endpoints/helm-releases.api.ts index 0e62027dbe..1eb5356465 100644 --- a/src/renderer/api/endpoints/helm-releases.api.ts +++ b/src/renderer/api/endpoints/helm-releases.api.ts @@ -20,7 +20,6 @@ */ import jsYaml from "js-yaml"; -import { compile } from "path-to-regexp"; import { autoBind, formatDuration } from "../../utils"; import capitalize from "lodash/capitalize"; import { apiBase } from "../index"; @@ -28,6 +27,7 @@ import { helmChartStore } from "../../components/+apps-helm-charts/helm-chart.st import type { ItemObject } from "../../item.store"; import { KubeObject } from "../kube-object"; import type { JsonApiData } from "../json-api"; +import { buildURLPositional } from "../../../common/utils/buildUrl"; interface IReleasePayload { name: string; @@ -83,12 +83,16 @@ export interface IReleaseRevision { description: string; } -const endpoint = compile(`/v2/releases/:namespace?/:name?`) as ( - params?: { - namespace?: string; - name?: string; - } -) => string; +type EndpointParams = {} + | { namespace: string } + | { namespace: string, name: string } + | { namespace: string, name: string, route: string }; + +interface EndpointQuery { + all?: boolean; +} + +const endpoint = buildURLPositional("/v2/releases/:namespace?/:name?/:route?"); export async function listReleases(namespace?: string): Promise { const releases = await apiBase.get(endpoint({ namespace })); @@ -134,25 +138,25 @@ export async function deleteRelease(name: string, namespace: string): Promise { - const path = `${endpoint({ name, namespace })}/values${all? "?all": ""}`; + const route = "values"; + const path = endpoint({ name, namespace, route }, { all }); return apiBase.get(path); } export async function getReleaseHistory(name: string, namespace: string): Promise { - const path = `${endpoint({ name, namespace })}/history`; + const route = "history"; + const path = endpoint({ name, namespace, route }); return apiBase.get(path); } export async function rollbackRelease(name: string, namespace: string, revision: number): Promise { - const path = `${endpoint({ name, namespace })}/rollback`; + const route = "rollback"; + const path = endpoint({ name, namespace, route }); + const data = { revision }; - return apiBase.put(path, { - data: { - revision - } - }); + return apiBase.put(path, { data }); } export interface HelmRelease { @@ -210,12 +214,7 @@ export class HelmRelease implements ItemObject { getVersion() { const versions = this.chart.match(/(?<=-)(v?\d+)[^-].*$/); - if (versions) { - return versions[0]; - } - else { - return ""; - } + return versions?.[0] ?? ""; } getUpdated(humanize = true, compact = true) { diff --git a/src/renderer/components/+apps-releases/release-details.tsx b/src/renderer/components/+apps-releases/release-details.tsx index 2e7eca5e1a..d7b6b76279 100644 --- a/src/renderer/components/+apps-releases/release-details.tsx +++ b/src/renderer/components/+apps-releases/release-details.tsx @@ -58,32 +58,35 @@ export class ReleaseDetails extends Component { @observable details: IReleaseDetails; @observable values = ""; @observable valuesLoading = false; - @observable userSuppliedOnly = false; + @observable showOnlyUserSuppliedValues = false; @observable saving = false; @observable releaseSecret: Secret; - @disposeOnUnmount - releaseSelector = reaction(() => this.props.release, release => { - if (!release) return; - this.loadDetails(); - this.loadValues(); - this.releaseSecret = null; + componentDidMount() { + disposeOnUnmount(this, [ + reaction(() => this.props.release, release => { + if (!release) return; + this.loadDetails(); + this.loadValues(); + this.releaseSecret = null; + }), + reaction(() => secretsStore.getItems(), () => { + if (!this.props.release) return; + const { getReleaseSecret } = releaseStore; + const { release } = this.props; + const secret = getReleaseSecret(release); + + if (this.releaseSecret) { + if (isEqual(this.releaseSecret.getLabels(), secret.getLabels())) return; + this.loadDetails(); + } + this.releaseSecret = secret; + }), + reaction(() => this.showOnlyUserSuppliedValues, () => { + this.loadValues(); + }), + ]); } - ); - - @disposeOnUnmount - secretWatcher = reaction(() => secretsStore.getItems(), () => { - if (!this.props.release) return; - const { getReleaseSecret } = releaseStore; - const { release } = this.props; - const secret = getReleaseSecret(release); - - if (this.releaseSecret) { - if (isEqual(this.releaseSecret.getLabels(), secret.getLabels())) return; - this.loadDetails(); - } - this.releaseSecret = secret; - }); constructor(props: Props) { super(props); @@ -100,10 +103,15 @@ export class ReleaseDetails extends Component { async loadValues() { const { release } = this.props; - this.values = ""; - this.valuesLoading = true; - this.values = (await getReleaseValues(release.getName(), release.getNs(), !this.userSuppliedOnly)) ?? ""; - this.valuesLoading = false; + try { + this.valuesLoading = true; + this.values = (await getReleaseValues(release.getName(), release.getNs(), !this.showOnlyUserSuppliedValues)) ?? ""; + } catch (error) { + Notifications.error(`Failed to load values for ${release.getName()}: ${error}`); + this.values = ""; + } finally { + this.valuesLoading = false; + } } updateValues = async () => { @@ -146,21 +154,19 @@ export class ReleaseDetails extends Component {

{ - this.userSuppliedOnly = values; - this.loadValues(); - }} + value={this.showOnlyUserSuppliedValues} + onChange={value => this.showOnlyUserSuppliedValues = value} disabled={valuesLoading} /> - {valuesLoading - ? - : this.values = values} - /> - } + this.values = text} + className={cssNames({ loading: valuesLoading })} + readOnly={valuesLoading || this.showOnlyUserSuppliedValues} + > + {valuesLoading && } +