From 005ed1c34e8d8c07af06f5a9bdfd614f86e59f24 Mon Sep 17 00:00:00 2001 From: Sebastian Malton Date: Tue, 29 Nov 2022 05:56:09 -0800 Subject: [PATCH] Remove uses of legacy global logger from injectables (#6660) - Clean up some of them to use more injectables Signed-off-by: Sebastian Malton Signed-off-by: Sebastian Malton --- .../metrics/add-metrics-route.injectable.ts | 110 +++++++------- .../start-port-forward-route.injectable.ts | 3 +- ...p-current-port-forward-route.injectable.ts | 57 ++++---- ...ate-temp-files-and-validate.injectable.tsx | 134 +++++++++--------- 4 files changed, 153 insertions(+), 151 deletions(-) diff --git a/src/main/routes/metrics/add-metrics-route.injectable.ts b/src/main/routes/metrics/add-metrics-route.injectable.ts index a2c6a67d50..eb59f2840b 100644 --- a/src/main/routes/metrics/add-metrics-route.injectable.ts +++ b/src/main/routes/metrics/add-metrics-route.injectable.ts @@ -7,13 +7,13 @@ import { apiPrefix } from "../../../common/vars"; import { getRouteInjectable } from "../../router/router.injectable"; import type { ClusterPrometheusMetadata } from "../../../common/cluster-types"; import { ClusterMetadataKey } from "../../../common/cluster-types"; -import logger from "../../logger"; import type { Cluster } from "../../../common/cluster/cluster"; import { clusterRoute } from "../../router/route"; import { isObject } from "lodash"; -import { isRequestError } from "../../../common/utils"; +import { isRequestError, object } from "../../../common/utils"; import type { GetMetrics } from "../../get-metrics.injectable"; import getMetricsInjectable from "../../get-metrics.injectable"; +import loggerInjectable from "../../../common/logger.injectable"; // This is used for backoff retry tracking. const ATTEMPTS = [false, false, false, false, true]; @@ -55,66 +55,70 @@ const loadMetricsFor = (getMetrics: GetMetrics) => async (promQueries: string[], const addMetricsRouteInjectable = getRouteInjectable({ id: "add-metrics-route", - instantiate: (di) => clusterRoute({ - method: "post", - path: `${apiPrefix}/metrics`, - })(async ({ cluster, payload, query }) => { + instantiate: (di) => { const getMetrics = di.inject(getMetricsInjectable); const loadMetrics = loadMetricsFor(getMetrics); + const logger = di.inject(loggerInjectable); - const queryParams: Partial> = Object.fromEntries(query.entries()); - const prometheusMetadata: ClusterPrometheusMetadata = {}; + return clusterRoute({ + method: "post", + path: `${apiPrefix}/metrics`, + })(async ({ cluster, payload, query }) => { + const queryParams: Partial> = Object.fromEntries(query.entries()); + const prometheusMetadata: ClusterPrometheusMetadata = {}; - try { - const { prometheusPath, provider } = await cluster.contextHandler.getPrometheusDetails(); + try { + const { prometheusPath, provider } = await cluster.contextHandler.getPrometheusDetails(); - prometheusMetadata.provider = provider?.kind; - prometheusMetadata.autoDetected = !cluster.preferences.prometheusProvider?.type; + prometheusMetadata.provider = provider?.kind; + prometheusMetadata.autoDetected = !cluster.preferences.prometheusProvider?.type; - if (!prometheusPath) { - prometheusMetadata.success = false; + if (!prometheusPath) { + prometheusMetadata.success = false; + + return { response: {}}; + } + + // return data in same structure as query + if (typeof payload === "string") { + const [data] = await loadMetrics([payload], cluster, prometheusPath, queryParams); + + return { response: data }; + } + + if (Array.isArray(payload)) { + const data = await loadMetrics(payload, cluster, prometheusPath, queryParams); + + return { response: data }; + } + + if (isObject(payload)) { + const data = payload as Record>; + const queries = object.entries(data) + .map(([queryName, queryOpts]) => ( + provider.getQuery(queryOpts, queryName) + )); + + const result = await loadMetrics(queries, cluster, prometheusPath, queryParams); + const response = object.fromEntries(object.keys(data).map((metricName, i) => [metricName, result[i]])); + + prometheusMetadata.success = true; + + return { response }; + } return { response: {}}; + } catch (error) { + prometheusMetadata.success = false; + + logger.warn(`[METRICS-ROUTE]: failed to get metrics for clusterId=${cluster.id}:`, error); + + return { response: {}}; + } finally { + cluster.metadata[ClusterMetadataKey.PROMETHEUS] = prometheusMetadata; } - - // return data in same structure as query - if (typeof payload === "string") { - const [data] = await loadMetrics([payload], cluster, prometheusPath, queryParams); - - return { response: data }; - } - - if (Array.isArray(payload)) { - const data = await loadMetrics(payload, cluster, prometheusPath, queryParams); - - return { response: data }; - } - - if (isObject(payload)) { - const queries = Object.entries(payload as Record>) - .map(([queryName, queryOpts]) => ( - provider.getQuery(queryOpts, queryName) - )); - - const result = await loadMetrics(queries, cluster, prometheusPath, queryParams); - const data = Object.fromEntries(Object.keys(payload).map((metricName, i) => [metricName, result[i]])); - - prometheusMetadata.success = true; - - return { response: data }; - } - - return { response: {}}; - } catch (error) { - prometheusMetadata.success = false; - - logger.warn(`[METRICS-ROUTE]: failed to get metrics for clusterId=${cluster.id}:`, error); - - return { response: {}}; - } finally { - cluster.metadata[ClusterMetadataKey.PROMETHEUS] = prometheusMetadata; - } - }), + }); + }, }); export default addMetricsRouteInjectable; diff --git a/src/main/routes/port-forward/start-port-forward-route.injectable.ts b/src/main/routes/port-forward/start-port-forward-route.injectable.ts index ba7a857255..0716b81a2f 100644 --- a/src/main/routes/port-forward/start-port-forward-route.injectable.ts +++ b/src/main/routes/port-forward/start-port-forward-route.injectable.ts @@ -5,15 +5,16 @@ import { getRouteInjectable } from "../../router/router.injectable"; import { apiPrefix } from "../../../common/vars"; import { PortForward } from "./functionality/port-forward"; -import logger from "../../logger"; import createPortForwardInjectable from "./functionality/create-port-forward.injectable"; import { clusterRoute } from "../../router/route"; +import loggerInjectable from "../../../common/logger.injectable"; const startPortForwardRouteInjectable = getRouteInjectable({ id: "start-current-port-forward-route", instantiate: (di) => { const createPortForward = di.inject(createPortForwardInjectable); + const logger = di.inject(loggerInjectable); return clusterRoute({ method: "post", diff --git a/src/main/routes/port-forward/stop-current-port-forward-route.injectable.ts b/src/main/routes/port-forward/stop-current-port-forward-route.injectable.ts index 6ae23af9c2..1fc76e9b72 100644 --- a/src/main/routes/port-forward/stop-current-port-forward-route.injectable.ts +++ b/src/main/routes/port-forward/stop-current-port-forward-route.injectable.ts @@ -5,39 +5,42 @@ import { getRouteInjectable } from "../../router/router.injectable"; import { apiPrefix } from "../../../common/vars"; import { PortForward } from "./functionality/port-forward"; -import logger from "../../logger"; import { clusterRoute } from "../../router/route"; +import loggerInjectable from "../../../common/logger.injectable"; const stopCurrentPortForwardRouteInjectable = getRouteInjectable({ id: "stop-current-port-forward-route", - instantiate: () => clusterRoute({ - method: "delete", - path: `${apiPrefix}/pods/port-forward/{namespace}/{resourceType}/{resourceName}`, - })(async ({ params, query, cluster }) => { - const { namespace, resourceType, resourceName } = params; - const port = Number(query.get("port")); - const forwardPort = Number(query.get("forwardPort")); - const portForward = PortForward.getPortforward({ - clusterId: cluster.id, kind: resourceType, name: resourceName, - namespace, port, forwardPort, + instantiate: (di) => { + const logger = di.inject(loggerInjectable); + + return clusterRoute({ + method: "delete", + path: `${apiPrefix}/pods/port-forward/{namespace}/{resourceType}/{resourceName}`, + })(async ({ params, query, cluster }) => { + const { namespace, resourceType, resourceName } = params; + const port = Number(query.get("port")); + const forwardPort = Number(query.get("forwardPort")); + const portForward = PortForward.getPortforward({ + clusterId: cluster.id, kind: resourceType, name: resourceName, + namespace, port, forwardPort, + }); + + try { + await portForward?.stop(); + + return { response: { status: true }}; + } catch (error) { + logger.error("[PORT-FORWARD-ROUTE]: error stopping a port-forward", { namespace, port, forwardPort, resourceType, resourceName }); + + return { + error: { + message: `error stopping a forward port ${port}`, + }, + }; + } }); - - try { - await portForward?.stop(); - - return { response: { status: true }}; - } catch (error) { - logger.error("[PORT-FORWARD-ROUTE]: error stopping a port-forward", { namespace, port, forwardPort, resourceType, resourceName }); - - return { - error: { - message: `error stopping a forward port ${port}`, - }, - }; - - } - }), + }, }); export default stopCurrentPortForwardRouteInjectable; diff --git a/src/renderer/components/+extensions/attempt-install/create-temp-files-and-validate.injectable.tsx b/src/renderer/components/+extensions/attempt-install/create-temp-files-and-validate.injectable.tsx index e9fea96e44..5c1674fc94 100644 --- a/src/renderer/components/+extensions/attempt-install/create-temp-files-and-validate.injectable.tsx +++ b/src/renderer/components/+extensions/attempt-install/create-temp-files-and-validate.injectable.tsx @@ -5,16 +5,15 @@ import { getInjectable } from "@ogre-tools/injectable"; import extensionDiscoveryInjectable from "../../../../extensions/extension-discovery/extension-discovery.injectable"; import { validatePackage } from "./validate-package"; -import type { ExtensionDiscovery } from "../../../../extensions/extension-discovery/extension-discovery"; import { getMessageFromError } from "../get-message-from-error/get-message-from-error"; -import logger from "../../../../main/logger"; -import { Notifications } from "../../notifications"; -import path from "path"; -import fse from "fs-extra"; import React from "react"; -import os from "os"; import type { LensExtensionId, LensExtensionManifest } from "../../../../extensions/lens-extension"; import type { InstallRequest } from "./attempt-install.injectable"; +import loggerInjectable from "../../../../common/logger.injectable"; +import writeFileInjectable from "../../../../common/fs/write-file.injectable"; +import joinPathsInjectable from "../../../../common/path/join-paths.injectable"; +import tempDirectoryPathInjectable from "../../../../common/os/temp-directory-path.injectable"; +import showErrorNotificationInjectable from "../../notifications/show-error-notification.injectable"; export interface InstallRequestValidated { fileName: string; @@ -24,74 +23,69 @@ export interface InstallRequestValidated { tempFile: string; // temp system path to packed extension for unpacking } -interface Dependencies { - extensionDiscovery: ExtensionDiscovery; -} - export type CreateTempFilesAndValidate = (request: InstallRequest) => Promise; -const createTempFilesAndValidate = ({ - extensionDiscovery, -}: Dependencies): CreateTempFilesAndValidate => ( - async ({ fileName, data }) => { - // copy files to temp - await fse.ensureDir(getExtensionPackageTemp()); - - // validate packages - const tempFile = getExtensionPackageTemp(fileName); - - try { - await fse.writeFile(tempFile, data); - const manifest = await validatePackage(tempFile); - const id = path.join( - extensionDiscovery.nodeModulesPath, - manifest.name, - "package.json", - ); - - return { - fileName, - data, - manifest, - tempFile, - id, - }; - } catch (error) { - const message = getMessageFromError(error); - - logger.info( - `[EXTENSION-INSTALLATION]: installing ${fileName} has failed: ${message}`, - { error }, - ); - Notifications.error(( -
-

- {"Installing "} - {fileName} - {" has failed, skipping."} -

-

- {"Reason: "} - {message} -

-
- )); - } - - return null; - } -); - - -function getExtensionPackageTemp(fileName = "") { - return path.join(os.tmpdir(), "lens-extensions", fileName); -} - const createTempFilesAndValidateInjectable = getInjectable({ id: "create-temp-files-and-validate", - instantiate: (di) => createTempFilesAndValidate({ - extensionDiscovery: di.inject(extensionDiscoveryInjectable), - }), + instantiate: (di): CreateTempFilesAndValidate => { + const extensionDiscovery = di.inject(extensionDiscoveryInjectable); + const logger = di.inject(loggerInjectable); + const writeFile = di.inject(writeFileInjectable); + const joinPaths = di.inject(joinPathsInjectable); + const showErrorNotification = di.inject(showErrorNotificationInjectable); + const tempDirectoryPath = di.inject(tempDirectoryPathInjectable); + + const getTempExtensionPackagePath = (fileName: string) => joinPaths( + tempDirectoryPath, + "lens-extensions", + fileName, + ); + + return async ({ fileName, data }) => { + // validate packages + const tempFile = getTempExtensionPackagePath(fileName); + + try { + await writeFile(tempFile, data); + const manifest = await validatePackage(tempFile); + const id = joinPaths( + extensionDiscovery.nodeModulesPath, + manifest.name, + "package.json", + ); + + return { + fileName, + data, + manifest, + tempFile, + id, + }; + } catch (error) { + const message = getMessageFromError(error); + + logger.info( + `[EXTENSION-INSTALLATION]: installing ${fileName} has failed: ${message}`, + { error }, + ); + showErrorNotification(( +
+

+ {"Installing "} + {fileName} + {" has failed, skipping."} +

+

+ {"Reason: "} + {message} +

+
+ )); + } + + return null; + }; + }, }); export default createTempFilesAndValidateInjectable;