From a7fa8554f5a4747fc2c3263457f4322952806f0a Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Tue, 10 Dec 2024 10:13:59 -0600 Subject: [PATCH 1/4] Refactored url management to accomodate envoy (163) --- src/controllers/ExtensionController.ts | 28 +++++++++++++++++++------- src/dh/dhe.ts | 13 ++++++++++-- src/services/DheService.ts | 4 ++-- src/services/ServerManager.ts | 11 ++++++---- src/types/commonTypes.d.ts | 9 +++++++-- src/types/serviceTypes.d.ts | 5 ++--- 6 files changed, 50 insertions(+), 20 deletions(-) diff --git a/src/controllers/ExtensionController.ts b/src/controllers/ExtensionController.ts index 5e94d21a..ae75dbfb 100644 --- a/src/controllers/ExtensionController.ts +++ b/src/controllers/ExtensionController.ts @@ -66,6 +66,7 @@ import type { CoreAuthenticatedClient, ICoreClientFactory, CoreUnauthenticatedClient, + WorkerURL, } from '../types'; import { ServerConnectionTreeDragAndDropController } from './ServerConnectionTreeDragAndDropController'; import { ConnectionController } from './ConnectionController'; @@ -342,14 +343,27 @@ export class ExtensionController implements Disposable { url: URL ): Promise => { assertDefined(this._coreJsApiCache, 'coreJsApiCache'); + + const workerInfo = await this._serverManager?.getWorkerInfo( + url as WorkerURL + ); + const dhc = await this._coreJsApiCache.get(url); - const client = new dhc.CoreClient(url.toString(), { - debug: true, - // TODO: This should be optional, but types aren't happy yet - headers: {}, - transportFactory: NodeHttp2gRPCTransport.factory, - }) as CoreUnauthenticatedClient; + const clientUrl = workerInfo == null ? url : workerInfo.grpcUrl; + const envoyPrefix = workerInfo?.envoyPrefix; + + const headers: { [key: string]: string } = + // eslint-disable-next-line @typescript-eslint/naming-convention + envoyPrefix == null ? {} : { 'envoy-prefix': envoyPrefix }; + + const client = new dhc.CoreClient( + clientUrl.toString().replace(/\/$/, ''), + { + headers, + transportFactory: NodeHttp2gRPCTransport.factory, + } + ) as CoreUnauthenticatedClient; // Attach a dispose method so that client caches can dispose of the client return Object.assign(client, { @@ -450,7 +464,7 @@ export class ExtensionController implements Disposable { */ initializeTempDirectory = (): void => { // recreate tmp dir that will be used to dowload JS Apis - getTempDir({ recreate: true }); + getTempDir({ recreate: false }); }; /** diff --git a/src/dh/dhe.ts b/src/dh/dhe.ts index 94af90e4..f0b39b93 100644 --- a/src/dh/dhe.ts +++ b/src/dh/dhe.ts @@ -10,7 +10,9 @@ import type { AuthenticatedClient as DheAuthenticatedClient } from '@deephaven-e import { hasStatusCode, loadModules } from '@deephaven/jsapi-nodejs'; import type { ConsoleType, + GrpcURL, IdeURL, + JsapiURL, QuerySerial, UniqueID, WorkerConfig, @@ -271,15 +273,22 @@ export async function getWorkerInfoFromQuery( return; } - const { grpcUrl, ideUrl, processInfoId, workerName } = queryInfo.designated; + const { envoyPrefix, grpcUrl, ideUrl, jsApiUrl, processInfoId, workerName } = + queryInfo.designated; + + const workerUrl = new URL(jsApiUrl) as WorkerURL; + workerUrl.pathname = workerUrl.pathname.replace(/jsapi\/dh-core.js$/, ''); return { tagId, serial: querySerial, - grpcUrl: new URL(grpcUrl) as WorkerURL, + envoyPrefix, + grpcUrl: new URL(grpcUrl) as GrpcURL, ideUrl: new URL(ideUrl) as IdeURL, + jsapiUrl: new URL(jsApiUrl) as JsapiURL, processInfoId, workerName, + workerUrl, }; } diff --git a/src/services/DheService.ts b/src/services/DheService.ts index dc5e805a..72b85295 100644 --- a/src/services/DheService.ts +++ b/src/services/DheService.ts @@ -2,7 +2,6 @@ import * as vscode from 'vscode'; import type { AuthenticatedClient as DheAuthenticatedClient } from '@deephaven-enterprise/auth-nodejs'; import type { EnterpriseDhType as DheType } from '@deephaven-enterprise/jsapi-types'; import { - WorkerURL, type ConsoleType, type IAsyncCacheService, type IConfigService, @@ -13,6 +12,7 @@ import { type UniqueID, type WorkerConfig, type WorkerInfo, + type WorkerURL, } from '../types'; import { URLMap } from './URLMap'; import { Logger } from '../util'; @@ -231,7 +231,7 @@ export class DheService implements IDheService { throw new Error('Failed to create worker.'); } - this._workerInfoMap.set(workerInfo.grpcUrl, workerInfo); + this._workerInfoMap.set(workerInfo.workerUrl, workerInfo); return workerInfo; }; diff --git a/src/services/ServerManager.ts b/src/services/ServerManager.ts index 18ea0417..ac5371e3 100644 --- a/src/services/ServerManager.ts +++ b/src/services/ServerManager.ts @@ -13,12 +13,12 @@ import type { WorkerInfo, IDheService, IAsyncCacheService, - WorkerURL, UniqueID, IToastService, CoreAuthenticatedClient, ISecretService, Psk, + WorkerURL, } from '../types'; import { getInitialServerStates, @@ -211,7 +211,7 @@ export class ServerManager implements IServerManager { // this indicates that the user cancelled the creation before it was ready. // In this case, dispose of the worker. if (!this._connectionMap.has(placeholderUrl)) { - dheService.deleteWorker(workerInfo.grpcUrl); + dheService.deleteWorker(workerInfo.workerUrl); this._onDidUpdate.fire(); return null; } @@ -228,11 +228,14 @@ export class ServerManager implements IServerManager { // Map the worker URL to the server URL to make things easier to dispose // later. - this._workerURLToServerURLMap.set(new URL(workerInfo.grpcUrl), serverUrl); + this._workerURLToServerURLMap.set( + new URL(workerInfo.workerUrl), + serverUrl + ); // Update the server URL to the worker url to be used below with core // connection creation. - serverUrl = new URL(workerInfo.grpcUrl); + serverUrl = new URL(workerInfo.workerUrl); } const connection = this._dhcServiceFactory.create(serverUrl, tagId); diff --git a/src/types/commonTypes.d.ts b/src/types/commonTypes.d.ts index 1b7c15cc..26a81065 100644 --- a/src/types/commonTypes.d.ts +++ b/src/types/commonTypes.d.ts @@ -87,17 +87,22 @@ export interface ConnectionState { readonly tagId?: UniqueID; } -export type WorkerURL = Brand<'GrpcUrl', URL>; +export type GrpcURL = Brand<'GrpcURL', URL>; export type IdeURL = Brand<'IdeUrl', URL>; +export type JsapiURL = Brand<'JsapiURL', URL>; export type QuerySerial = Brand<'QuerySerial', string>; +export type WorkerURL = Brand<'WorkerURL', URL>; export interface WorkerInfo { tagId: UniqueID; - grpcUrl: WorkerURL; + envoyPrefix: string | null; + grpcUrl: GrpcURL; ideUrl: IdeURL; + jsapiUrl: JsapiURL; processInfoId: string | null; serial: QuerySerial; workerName: string | null; + workerUrl: WorkerURL; } export interface Disposable { diff --git a/src/types/serviceTypes.d.ts b/src/types/serviceTypes.d.ts index cbfcc456..498bd33f 100644 --- a/src/types/serviceTypes.d.ts +++ b/src/types/serviceTypes.d.ts @@ -8,19 +8,18 @@ import type { EventListenerT, ConnectionState, ServerState, - UnsubscribeEventListener, VariableChanges, VariableDefintion, VariableID, WorkerInfo, - WorkerURL, UniqueID, UserKeyPairs, UserLoginPreferences, CoreUnauthenticatedClient, Psk, CoreAuthenticatedClient, -} from '../types/commonTypes'; + WorkerURL, +} from './commonTypes'; import type { AuthenticatedClient as DheAuthenticatedClient, UnauthenticatedClient as DheUnauthenticatedClient, From 77b1ba782f47a5bc8880ede212545410b78a407a Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Tue, 10 Dec 2024 10:16:00 -0600 Subject: [PATCH 2/4] Recreate tmp dir (163) --- src/controllers/ExtensionController.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/controllers/ExtensionController.ts b/src/controllers/ExtensionController.ts index ae75dbfb..07017ae0 100644 --- a/src/controllers/ExtensionController.ts +++ b/src/controllers/ExtensionController.ts @@ -464,7 +464,7 @@ export class ExtensionController implements Disposable { */ initializeTempDirectory = (): void => { // recreate tmp dir that will be used to dowload JS Apis - getTempDir({ recreate: false }); + getTempDir({ recreate: true }); }; /** From 84b727e805e3b8c6a3e33e29a85471a55d4f20c8 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Tue, 10 Dec 2024 11:43:00 -0600 Subject: [PATCH 3/4] Pass envoyPrefix to panel iframe url (163) --- src/controllers/PanelController.ts | 12 +++++++----- src/dh/dhc.ts | 9 ++++++++- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/controllers/PanelController.ts b/src/controllers/PanelController.ts index 3a2839c7..624ac42f 100644 --- a/src/controllers/PanelController.ts +++ b/src/controllers/PanelController.ts @@ -4,6 +4,7 @@ import type { IPanelService, IServerManager, VariableDefintion, + WorkerInfo, WorkerURL, } from '../types'; import { @@ -192,8 +193,8 @@ export class PanelController extends ControllerBase { const connection = this._serverManager.getConnection(serverUrl); assertDefined(connection, 'connection'); - const isWorkerUrl = Boolean( - await this._serverManager.getWorkerInfo(serverUrl as WorkerURL) + const workerInfo = await this._serverManager.getWorkerInfo( + serverUrl as WorkerURL ); for (const { id, title } of variables) { @@ -202,7 +203,7 @@ export class PanelController extends ControllerBase { const iframeUrl = await getEmbedWidgetUrlForConnection( connection, title, - isWorkerUrl + workerInfo ); panel.webview.html = getPanelHtml(iframeUrl, title); @@ -232,7 +233,7 @@ export class PanelController extends ControllerBase { export async function getEmbedWidgetUrlForConnection( connection: ConnectionState, title: string, - isWorkerUrl: boolean + workerInfo?: WorkerInfo ): Promise { return getEmbedWidgetUrl({ serverUrl: connection.serverUrl, @@ -241,7 +242,8 @@ export async function getEmbedWidgetUrlForConnection( // For Core+ workers in DHE, we use `postMessage` apis for auth where DH // iframe communicates with parent (the extension) to get login credentials // from the DHE client. See `getPanelHtml` util for more details. - authProvider: isWorkerUrl ? 'parent' : undefined, + authProvider: workerInfo == null ? undefined : 'parent', + envoyPrefix: workerInfo?.envoyPrefix, psk: connection instanceof DhcService ? await connection.getPsk() : undefined, }); diff --git a/src/dh/dhc.ts b/src/dh/dhc.ts index 743b362b..45852b4a 100644 --- a/src/dh/dhc.ts +++ b/src/dh/dhc.ts @@ -58,6 +58,7 @@ export async function getDhc( * @param title Widget title * @param themeKey Theme key * @param authProvider Optional auth provider + * @param envoyPrefix Optional envoy prefix for Core+ workers * @param psk Optional psk */ export function getEmbedWidgetUrl({ @@ -65,15 +66,17 @@ export function getEmbedWidgetUrl({ title, themeKey, authProvider, + envoyPrefix, psk, }: { serverUrl: URL; title: string; themeKey: string; authProvider?: 'parent'; + envoyPrefix?: string | null; psk?: string | null; }): URL { - const url = new URL('/iframe/widget', serverUrl); + const url = new URL('iframe/widget/', serverUrl); url.searchParams.set('name', title); url.searchParams.set('theme', themeKey); @@ -82,6 +85,10 @@ export function getEmbedWidgetUrl({ url.searchParams.set('authProvider', authProvider); } + if (envoyPrefix) { + url.searchParams.set('envoyPrefix', envoyPrefix); + } + if (psk) { url.searchParams.set('psk', psk); } From 6cdbbb8e0cf5d0c6297c2af7720f5bef7a489295 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Thu, 12 Dec 2024 09:17:43 -0600 Subject: [PATCH 4/4] Dispose gRPC transport (163) --- src/controllers/ExtensionController.ts | 2 ++ src/dh/NodeHttp2gRPCTransport.ts | 27 +++++++++++++++----------- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/controllers/ExtensionController.ts b/src/controllers/ExtensionController.ts index 07017ae0..ea6927b6 100644 --- a/src/controllers/ExtensionController.ts +++ b/src/controllers/ExtensionController.ts @@ -114,6 +114,8 @@ export class ExtensionController implements Disposable { this.initializeWebViews(); this.initializeServerUpdates(); + this._context.subscriptions.push(NodeHttp2gRPCTransport); + logger.info( 'Congratulations, your extension "vscode-deephaven" is now active!' ); diff --git a/src/dh/NodeHttp2gRPCTransport.ts b/src/dh/NodeHttp2gRPCTransport.ts index b117d615..5640bf24 100644 --- a/src/dh/NodeHttp2gRPCTransport.ts +++ b/src/dh/NodeHttp2gRPCTransport.ts @@ -9,6 +9,16 @@ import { assertDefined } from '../util'; export class NodeHttp2gRPCTransport implements GrpcTransport { static _sessionMap: Map = new Map(); + /** + * Cleanup. + */ + static dispose(): void { + for (const session of NodeHttp2gRPCTransport._sessionMap.values()) { + session.close(); + } + NodeHttp2gRPCTransport._sessionMap.clear(); + } + /** * TODO: Cleanup requests similar to https://github.com/deephaven/deephaven-core/blob/c05b35957e466fded4da61154ba106cfc3198bc5/web/client-api/src/main/java/io/deephaven/web/client/api/grpc/MultiplexedWebsocketTransport.java#L129 * Create a Transport instance. @@ -16,7 +26,12 @@ export class NodeHttp2gRPCTransport implements GrpcTransport { * @returns Transport instance. */ static readonly factory: GrpcTransportFactory = { - create: options => { + /** + * Create a new transport instance. + * @param options - options for creating the transport + * @return a transport instance to use for gRPC communication + */ + create: (options): GrpcTransport => { const { origin } = new URL(options.url); if (!NodeHttp2gRPCTransport._sessionMap.has(origin)) { @@ -126,14 +141,4 @@ export class NodeHttp2gRPCTransport implements GrpcTransport { assertDefined(this._request, '_request'); this._request.close(); } - - /** - * Cleanup. - */ - static dispose(): void { - for (const session of NodeHttp2gRPCTransport._sessionMap.values()) { - session.close(); - } - NodeHttp2gRPCTransport._sessionMap.clear(); - } }