From 24240655c01f615fa6b4baea8b6496427536614b Mon Sep 17 00:00:00 2001 From: Sebastian Malton Date: Tue, 20 Dec 2022 08:00:15 -0800 Subject: [PATCH] Split out use of httpsProxy preference (#6771) * Split out use of httpsProxy preference Signed-off-by: Sebastian Malton * Rename file Signed-off-by: Sebastian Malton Signed-off-by: Sebastian Malton --- src/common/fetch/download-json.injectable.ts | 55 ------------------- src/common/fetch/download-json/impl.ts | 46 ++++++++++++++++ .../fetch/download-json/normal.injectable.ts | 14 +++++ .../fetch/download-json/proxy.injectable.ts | 14 +++++ src/common/fetch/fetch-module.injectable.ts | 19 +++++++ src/common/fetch/fetch.injectable.ts | 23 ++------ src/common/fetch/proxy-fetch.injectable.ts | 30 ++++++++++ .../navigation-using-application-menu.test.ts | 8 --- ...est-public-helm-repositories.injectable.ts | 4 +- .../+extensions/__tests__/extensions.test.tsx | 6 -- .../attempt-install-by-info.injectable.tsx | 2 +- 11 files changed, 130 insertions(+), 91 deletions(-) delete mode 100644 src/common/fetch/download-json.injectable.ts create mode 100644 src/common/fetch/download-json/impl.ts create mode 100644 src/common/fetch/download-json/normal.injectable.ts create mode 100644 src/common/fetch/download-json/proxy.injectable.ts create mode 100644 src/common/fetch/fetch-module.injectable.ts create mode 100644 src/common/fetch/proxy-fetch.injectable.ts diff --git a/src/common/fetch/download-json.injectable.ts b/src/common/fetch/download-json.injectable.ts deleted file mode 100644 index 78a7d030d7..0000000000 --- a/src/common/fetch/download-json.injectable.ts +++ /dev/null @@ -1,55 +0,0 @@ -/** - * 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 type { RequestInit, Response } from "node-fetch"; -import type { AsyncResult } from "../utils/async-result"; -import fetchInjectable from "./fetch.injectable"; - -export interface DownloadJsonOptions { - signal?: AbortSignal | null | undefined; -} - -export type DownloadJson = (url: string, opts?: DownloadJsonOptions) => Promise>; - -const downloadJsonInjectable = getInjectable({ - id: "download-json", - instantiate: (di): DownloadJson => { - const fetch = di.inject(fetchInjectable); - - return async (url, opts) => { - let result: Response; - - try { - result = await fetch(url, opts as RequestInit); - } catch (error) { - return { - callWasSuccessful: false, - error: String(error), - }; - } - - if (result.status < 200 || 300 <= result.status) { - return { - callWasSuccessful: false, - error: result.statusText, - }; - } - - try { - return { - callWasSuccessful: true, - response: await result.json(), - }; - } catch (error) { - return { - callWasSuccessful: false, - error: String(error), - }; - } - }; - }, -}); - -export default downloadJsonInjectable; diff --git a/src/common/fetch/download-json/impl.ts b/src/common/fetch/download-json/impl.ts new file mode 100644 index 0000000000..9faf9af124 --- /dev/null +++ b/src/common/fetch/download-json/impl.ts @@ -0,0 +1,46 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ +import type { AsyncResult } from "../../utils/async-result"; +import type { Fetch } from "../fetch.injectable"; +import type { RequestInit, Response } from "node-fetch"; + +export interface DownloadJsonOptions { + signal?: AbortSignal | null | undefined; +} + +export type DownloadJson = (url: string, opts?: DownloadJsonOptions) => Promise>; + +export const downloadJsonWith = (fetch: Fetch): DownloadJson => async (url, opts) => { + let result: Response; + + try { + result = await fetch(url, opts as RequestInit); + } catch (error) { + return { + callWasSuccessful: false, + error: String(error), + }; + } + + if (result.status < 200 || 300 <= result.status) { + return { + callWasSuccessful: false, + error: result.statusText, + }; + } + + try { + return { + callWasSuccessful: true, + response: await result.json(), + }; + } catch (error) { + return { + callWasSuccessful: false, + error: String(error), + }; + } +}; + diff --git a/src/common/fetch/download-json/normal.injectable.ts b/src/common/fetch/download-json/normal.injectable.ts new file mode 100644 index 0000000000..adb5e35d82 --- /dev/null +++ b/src/common/fetch/download-json/normal.injectable.ts @@ -0,0 +1,14 @@ +/** + * 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 fetchInjectable from "../fetch.injectable"; +import { downloadJsonWith } from "./impl"; + +const downloadJsonInjectable = getInjectable({ + id: "download-json", + instantiate: (di) => downloadJsonWith(di.inject(fetchInjectable)), +}); + +export default downloadJsonInjectable; diff --git a/src/common/fetch/download-json/proxy.injectable.ts b/src/common/fetch/download-json/proxy.injectable.ts new file mode 100644 index 0000000000..46268d4ddb --- /dev/null +++ b/src/common/fetch/download-json/proxy.injectable.ts @@ -0,0 +1,14 @@ +/** + * 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 proxyFetchInjectable from "../proxy-fetch.injectable"; +import { downloadJsonWith } from "./impl"; + +const proxyDownloadJsonInjectable = getInjectable({ + id: "proxy-download-json", + instantiate: (di) => downloadJsonWith(di.inject(proxyFetchInjectable)), +}); + +export default proxyDownloadJsonInjectable; diff --git a/src/common/fetch/fetch-module.injectable.ts b/src/common/fetch/fetch-module.injectable.ts new file mode 100644 index 0000000000..444333f196 --- /dev/null +++ b/src/common/fetch/fetch-module.injectable.ts @@ -0,0 +1,19 @@ +/** + * 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 type * as FetchModule from "node-fetch"; + +const { NodeFetch } = require("../../../build/webpack/node-fetch.bundle") as { NodeFetch: typeof FetchModule }; + +/** + * NOTE: while using this module can cause side effects, this specific injectable is not marked as + * such since sometimes the request can be wholely within the perview of unit test + */ +const nodeFetchModuleInjectable = getInjectable({ + id: "node-fetch-module", + instantiate: () => NodeFetch, +}); + +export default nodeFetchModuleInjectable; diff --git a/src/common/fetch/fetch.injectable.ts b/src/common/fetch/fetch.injectable.ts index bd1ba89db7..d4f51efe0d 100644 --- a/src/common/fetch/fetch.injectable.ts +++ b/src/common/fetch/fetch.injectable.ts @@ -3,32 +3,17 @@ * Licensed under MIT License. See LICENSE in root directory for more information. */ import { getInjectable } from "@ogre-tools/injectable"; -import { HttpsProxyAgent } from "hpagent"; -import type * as FetchModule from "node-fetch"; -import userStoreInjectable from "../user-store/user-store.injectable"; - -const { NodeFetch: { default: fetch }} = require("../../../build/webpack/node-fetch.bundle") as { NodeFetch: typeof FetchModule }; - -type Response = FetchModule.Response; -type RequestInit = FetchModule.RequestInit; +import type { RequestInit, Response } from "node-fetch"; +import nodeFetchModuleInjectable from "./fetch-module.injectable"; export type Fetch = (url: string, init?: RequestInit) => Promise; const fetchInjectable = getInjectable({ id: "fetch", instantiate: (di): Fetch => { - const { httpsProxy, allowUntrustedCAs } = di.inject(userStoreInjectable); - const agent = httpsProxy - ? new HttpsProxyAgent({ - proxy: httpsProxy, - rejectUnauthorized: !allowUntrustedCAs, - }) - : undefined; + const { default: fetch } = di.inject(nodeFetchModuleInjectable); - return (url, init = {}) => fetch(url, { - agent, - ...init, - }); + return (url, init) => fetch(url, init); }, causesSideEffects: true, }); diff --git a/src/common/fetch/proxy-fetch.injectable.ts b/src/common/fetch/proxy-fetch.injectable.ts new file mode 100644 index 0000000000..f13842c410 --- /dev/null +++ b/src/common/fetch/proxy-fetch.injectable.ts @@ -0,0 +1,30 @@ +/** + * 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 { HttpsProxyAgent } from "hpagent"; +import userStoreInjectable from "../user-store/user-store.injectable"; +import type { Fetch } from "./fetch.injectable"; +import fetchInjectable from "./fetch.injectable"; + +const proxyFetchInjectable = getInjectable({ + id: "proxy-fetch", + instantiate: (di): Fetch => { + const fetch = di.inject(fetchInjectable); + const { httpsProxy, allowUntrustedCAs } = di.inject(userStoreInjectable); + const agent = httpsProxy + ? new HttpsProxyAgent({ + proxy: httpsProxy, + rejectUnauthorized: !allowUntrustedCAs, + }) + : undefined; + + return (url, init = {}) => fetch(url, { + agent, + ...init, + }); + }, +}); + +export default proxyFetchInjectable; diff --git a/src/features/extensions/navigation-using-application-menu.test.ts b/src/features/extensions/navigation-using-application-menu.test.ts index c17653d7a3..237e0a93a1 100644 --- a/src/features/extensions/navigation-using-application-menu.test.ts +++ b/src/features/extensions/navigation-using-application-menu.test.ts @@ -6,8 +6,6 @@ import type { RenderResult } from "@testing-library/react"; import type { ApplicationBuilder } from "../../renderer/components/test-utils/get-application-builder"; import { getApplicationBuilder } from "../../renderer/components/test-utils/get-application-builder"; -import downloadBinaryInjectable, { type DownloadBinary } from "../../common/fetch/download-binary.injectable"; -import downloadJsonInjectable, { type DownloadJson } from "../../common/fetch/download-json.injectable"; import focusWindowInjectable from "../../renderer/navigation/focus-window.injectable"; // TODO: Make components free of side effects by making them deterministic @@ -17,20 +15,14 @@ describe("extensions - navigation using application menu", () => { let builder: ApplicationBuilder; let rendered: RenderResult; let focusWindowMock: jest.Mock; - let downloadJson: jest.MockedFunction; - let downloadBinary: jest.MockedFunction; beforeEach(async () => { builder = getApplicationBuilder(); builder.beforeWindowStart((windowDi) => { focusWindowMock = jest.fn(); - downloadJson = jest.fn().mockImplementation((url) => { throw new Error(`Unexpected call to downloadJson for url=${url}`); }); - downloadBinary = jest.fn().mockImplementation((url) => { throw new Error(`Unexpected call to downloadJson for url=${url}`); }); windowDi.override(focusWindowInjectable, () => focusWindowMock); - windowDi.override(downloadJsonInjectable, () => downloadJson); - windowDi.override(downloadBinaryInjectable, () => downloadBinary); }); rendered = await builder.render(); diff --git a/src/features/helm-charts/child-features/preferences/renderer/adding-of-public-helm-repository/public-helm-repositories/request-public-helm-repositories.injectable.ts b/src/features/helm-charts/child-features/preferences/renderer/adding-of-public-helm-repository/public-helm-repositories/request-public-helm-repositories.injectable.ts index 64e130aa09..325f3e305d 100644 --- a/src/features/helm-charts/child-features/preferences/renderer/adding-of-public-helm-repository/public-helm-repositories/request-public-helm-repositories.injectable.ts +++ b/src/features/helm-charts/child-features/preferences/renderer/adding-of-public-helm-repository/public-helm-repositories/request-public-helm-repositories.injectable.ts @@ -4,7 +4,7 @@ */ import { getInjectable } from "@ogre-tools/injectable"; import { sortBy } from "lodash/fp"; -import downloadJsonInjectable from "../../../../../../../common/fetch/download-json.injectable"; +import proxyDownloadJsonInjectable from "../../../../../../../common/fetch/download-json/proxy.injectable"; import { withTimeout } from "../../../../../../../common/fetch/timeout-controller"; import type { HelmRepo } from "../../../../../../../common/helm/helm-repo"; import loggerInjectable from "../../../../../../../common/logger.injectable"; @@ -15,7 +15,7 @@ const requestPublicHelmRepositoriesInjectable = getInjectable({ id: "request-public-helm-repositories", instantiate: (di) => { - const downloadJson = di.inject(downloadJsonInjectable); + const downloadJson = di.inject(proxyDownloadJsonInjectable); const logger = di.inject(loggerInjectable); return async (): Promise => { diff --git a/src/renderer/components/+extensions/__tests__/extensions.test.tsx b/src/renderer/components/+extensions/__tests__/extensions.test.tsx index b17b6bc782..751c1a38cc 100644 --- a/src/renderer/components/+extensions/__tests__/extensions.test.tsx +++ b/src/renderer/components/+extensions/__tests__/extensions.test.tsx @@ -25,9 +25,7 @@ import extensionInstallationStateStoreInjectable from "../../../../extensions/ex import { observable, when } from "mobx"; import type { RemovePath } from "../../../../common/fs/remove.injectable"; import removePathInjectable from "../../../../common/fs/remove.injectable"; -import type { DownloadJson } from "../../../../common/fetch/download-json.injectable"; import type { DownloadBinary } from "../../../../common/fetch/download-binary.injectable"; -import downloadJsonInjectable from "../../../../common/fetch/download-json.injectable"; import downloadBinaryInjectable from "../../../../common/fetch/download-binary.injectable"; import currentlyInClusterFrameInjectable from "../../../routes/currently-in-cluster-frame.injectable"; @@ -38,7 +36,6 @@ describe("Extensions", () => { let extensionInstallationStateStore: ExtensionInstallationStateStore; let render: DiRender; let deleteFileMock: jest.MockedFunction; - let downloadJson: jest.MockedFunction; let downloadBinary: jest.MockedFunction; beforeEach(() => { @@ -56,9 +53,6 @@ describe("Extensions", () => { deleteFileMock = jest.fn(); di.override(removePathInjectable, () => deleteFileMock); - downloadJson = jest.fn().mockImplementation((url) => { throw new Error(`Unexpected call to downloadJson for url=${url}`); }); - di.override(downloadJsonInjectable, () => downloadJson); - downloadBinary = jest.fn().mockImplementation((url) => { throw new Error(`Unexpected call to downloadJson for url=${url}`); }); di.override(downloadBinaryInjectable, () => downloadBinary); diff --git a/src/renderer/components/+extensions/attempt-install-by-info.injectable.tsx b/src/renderer/components/+extensions/attempt-install-by-info.injectable.tsx index 317162cc9f..1f3d65c802 100644 --- a/src/renderer/components/+extensions/attempt-install-by-info.injectable.tsx +++ b/src/renderer/components/+extensions/attempt-install-by-info.injectable.tsx @@ -15,7 +15,7 @@ import { reduce } from "lodash"; import getBasenameOfPathInjectable from "../../../common/path/get-basename.injectable"; import { withTimeout } from "../../../common/fetch/timeout-controller"; import downloadBinaryInjectable from "../../../common/fetch/download-binary.injectable"; -import downloadJsonInjectable from "../../../common/fetch/download-json.injectable"; +import downloadJsonInjectable from "../../../common/fetch/download-json/normal.injectable"; import type { PackageJson } from "type-fest"; import showErrorNotificationInjectable from "../notifications/show-error-notification.injectable"; import loggerInjectable from "../../../common/logger.injectable";