From 93891e350840af8d2b6ae85ebf38e7b0d503ad1e Mon Sep 17 00:00:00 2001 From: Brett Hoerner Date: Fri, 17 Nov 2023 12:02:36 -0700 Subject: [PATCH] chore(plugin-server): add deprecated imports list, log if they are used, remove 'google' from global plugin scope --- plugin-server/src/worker/plugins/setup.ts | 6 ++- .../src/worker/vm/extensions/google.ts | 20 --------- .../src/worker/vm/transforms/index.ts | 9 +++- .../worker/vm/transforms/replace-imports.ts | 6 ++- plugin-server/src/worker/vm/vm.ts | 26 ++++++++++-- plugin-server/tests/worker/transforms.test.ts | 42 ++++++++++++------- 6 files changed, 65 insertions(+), 44 deletions(-) delete mode 100644 plugin-server/src/worker/vm/extensions/google.ts diff --git a/plugin-server/src/worker/plugins/setup.ts b/plugin-server/src/worker/plugins/setup.ts index b54e1e2b355a7..58fdf73fd6b03 100644 --- a/plugin-server/src/worker/plugins/setup.ts +++ b/plugin-server/src/worker/plugins/setup.ts @@ -7,6 +7,7 @@ import { loadSchedule } from './loadSchedule' import { teardownPlugins } from './teardown' export async function setupPlugins(hub: Hub): Promise { + const startTime = Date.now() status.info('🔁', `Loading plugin configs...`) const { plugins, pluginConfigs, pluginConfigsPerTeam } = await loadPluginsFromDB(hub) const pluginVMLoadPromises: Array> = [] @@ -59,5 +60,8 @@ export async function setupPlugins(hub: Hub): Promise { await loadSchedule(hub) } - status.info('✅', `Loaded ${pluginConfigs.size} configs for ${plugins.size} plugins`) + status.info( + '✅', + `Loaded ${pluginConfigs.size} configs for ${plugins.size} plugins, took ${Date.now() - startTime}ms` + ) } diff --git a/plugin-server/src/worker/vm/extensions/google.ts b/plugin-server/src/worker/vm/extensions/google.ts deleted file mode 100644 index 51facdc6ea8f1..0000000000000 --- a/plugin-server/src/worker/vm/extensions/google.ts +++ /dev/null @@ -1,20 +0,0 @@ -import * as BigQuery from '@google-cloud/bigquery' -import * as PubSub from '@google-cloud/pubsub' - -type DummyCloud = { - bigquery: typeof BigQuery - pubsub: typeof PubSub -} - -type DummyGoogle = { - cloud: DummyCloud -} - -export function createGoogle(): DummyGoogle { - return { - cloud: { - bigquery: BigQuery, - pubsub: PubSub, - }, - } -} diff --git a/plugin-server/src/worker/vm/transforms/index.ts b/plugin-server/src/worker/vm/transforms/index.ts index d0cd6d38219d0..7bc4b1621f783 100644 --- a/plugin-server/src/worker/vm/transforms/index.ts +++ b/plugin-server/src/worker/vm/transforms/index.ts @@ -7,7 +7,12 @@ import { replaceImports } from './replace-imports' const memoize: Record = {} -export function transformCode(rawCode: string, server: Hub, imports?: Record): string { +export function transformCode( + rawCode: string, + server: Hub, + imports: Record, + usedImports: Set +): string { if (process.env.NODE_ENV === 'test' && memoize[rawCode]) { // Memoizing in tests for speed, not in production though due to reliability concerns return memoize[rawCode] @@ -20,7 +25,7 @@ export function transformCode(rawCode: string, server: Hub, imports?: Record = {}) => + (_: Hub, imports: Record = {}, usedImports: Set) => ({ types: t }) => ({ visitor: { ImportDeclaration: { @@ -17,6 +17,8 @@ export const replaceImports: PluginGen = ) } + usedImports.add(importSource) + for (const specifier of node.specifiers) { if (t.isImportSpecifier(specifier)) { if (t.isStringLiteral(specifier.imported)) { @@ -63,6 +65,8 @@ export const replaceImports: PluginGen = ) } + usedImports.add(importSource) + path.replaceWith( t.memberExpression(t.identifier('__pluginHostImports'), t.stringLiteral(importSource), true) ) diff --git a/plugin-server/src/worker/vm/vm.ts b/plugin-server/src/worker/vm/vm.ts index e12d114ec0b22..ffaf928fda601 100644 --- a/plugin-server/src/worker/vm/vm.ts +++ b/plugin-server/src/worker/vm/vm.ts @@ -3,10 +3,10 @@ import { randomBytes } from 'crypto' import { VM } from 'vm2' import { Hub, PluginConfig, PluginConfigVMResponse } from '../../types' +import { status } from '../../utils/status' import { createCache } from './extensions/cache' import { createConsole } from './extensions/console' import { createGeoIp } from './extensions/geoip' -import { createGoogle } from './extensions/google' import { createJobs } from './extensions/jobs' import { createPosthog } from './extensions/posthog' import { createStorage } from './extensions/storage' @@ -29,6 +29,17 @@ export class TimeoutError extends RetryError { } } +const DEPRECATED_IMPORTS = new Set([ + '@google-cloud/bigquery', + 'ethers', + 'faker', + 'generic-pool', + 'jsonwebtoken', + 'pg', + 'snowflake-sdk', + 'zlib', +]) + export function createPluginConfigVM( hub: Hub, pluginConfig: PluginConfig, // NB! might have team_id = 0 @@ -44,7 +55,16 @@ export function createPluginConfigVM( }) } - const transformedCode = transformCode(indexJs, hub, AVAILABLE_IMPORTS) + const usedImports: Set = new Set() + const transformedCode = transformCode(indexJs, hub, AVAILABLE_IMPORTS, usedImports) + + const usedDeprecatedImports = [...usedImports].filter((i) => DEPRECATED_IMPORTS.has(i)) + if (usedDeprecatedImports.length > 0) { + status.warn('⚠️', 'Plugin used deprecated import(s)', { + imports: usedDeprecatedImports, + pluginConfig: pluginConfig.id, + }) + } // Create virtual machine const vm = new VM({ @@ -58,8 +78,6 @@ export function createPluginConfigVM( // Add non-PostHog utilities to virtual machine vm.freeze(AVAILABLE_IMPORTS['node-fetch'], 'fetch') - vm.freeze(createGoogle(), 'google') - vm.freeze(AVAILABLE_IMPORTS, '__pluginHostImports') if (process.env.NODE_ENV === 'test') { diff --git a/plugin-server/tests/worker/transforms.test.ts b/plugin-server/tests/worker/transforms.test.ts index 42b77e5388fc8..51eae0cc2584a 100644 --- a/plugin-server/tests/worker/transforms.test.ts +++ b/plugin-server/tests/worker/transforms.test.ts @@ -6,6 +6,8 @@ import { resetTestDatabase } from '../helpers/sql' jest.mock('../../src/utils/status') +const EMPTY_IMPORTS = {} + describe('transforms', () => { let hub: Hub let closeHub: () => Promise @@ -27,7 +29,7 @@ describe('transforms', () => { } ` - const transformedCode = transformCode(rawCode, hub) + const transformedCode = transformCode(rawCode, hub, EMPTY_IMPORTS, new Set()) expect(transformedCode).toStrictEqual(code` "use strict"; @@ -45,7 +47,7 @@ describe('transforms', () => { } ` - const transformedCode = transformCode(rawCode, hub) + const transformedCode = transformCode(rawCode, hub, EMPTY_IMPORTS, new Set()) expect(transformedCode).toStrictEqual(code` "use strict"; @@ -63,7 +65,7 @@ describe('transforms', () => { } ` - const transformedCode = transformCode(rawCode, hub) + const transformedCode = transformCode(rawCode, hub, EMPTY_IMPORTS, new Set()) expect(transformedCode).toStrictEqual(code` "use strict"; @@ -82,7 +84,7 @@ describe('transforms', () => { x.then(() => null) ` - const transformedCode = transformCode(rawCode, hub) + const transformedCode = transformCode(rawCode, hub, EMPTY_IMPORTS, new Set()) expect(transformedCode).toStrictEqual(code` "use strict"; @@ -99,7 +101,7 @@ describe('transforms', () => { } ` - const transformedCode = transformCode(rawCode, hub) + const transformedCode = transformCode(rawCode, hub, EMPTY_IMPORTS, new Set()) expect(transformedCode).toStrictEqual(code` "use strict"; @@ -117,7 +119,7 @@ describe('transforms', () => { for (let i = 0; i < i + 1; i++) console.log(i) ` - const transformedCode = transformCode(rawCode, hub) + const transformedCode = transformCode(rawCode, hub, EMPTY_IMPORTS, new Set()) expect(transformedCode).toStrictEqual(code` "use strict"; @@ -139,7 +141,7 @@ describe('transforms', () => { } ` - const transformedCode = transformCode(rawCode, hub) + const transformedCode = transformCode(rawCode, hub, EMPTY_IMPORTS, new Set()) expect(transformedCode).toStrictEqual(code` "use strict"; @@ -168,7 +170,7 @@ describe('transforms', () => { console.log(k({ a, b: 'tomato' })) ` - const transformedCode = transformCode(rawCode, hub) + const transformedCode = transformCode(rawCode, hub, EMPTY_IMPORTS, new Set()) expect(transformedCode).toStrictEqual(code` "use strict"; @@ -194,8 +196,9 @@ describe('transforms', () => { import * as fetch2 from 'node-fetch' console.log(bla, bla2, bla4, fetch1, fetch2); ` - - const transformedCode = transformCode(rawCode, hub, { 'node-fetch': { bla: () => true } }) + const usedImports = new Set() + const transformedCode = transformCode(rawCode, hub, { 'node-fetch': { bla: () => true } }, usedImports) + expect(usedImports).toEqual(new Set(['node-fetch'])) expect(transformedCode).toStrictEqual(code` "use strict"; @@ -216,7 +219,7 @@ describe('transforms', () => { ` expect(() => { - transformCode(rawCode, hub, { 'node-fetch': { default: () => true } }) + transformCode(rawCode, hub, { 'node-fetch': { default: () => true } }, new Set()) }).toThrow("/index.ts: Cannot import 'kea'! This package is not provided by PostHog in plugins.") }) @@ -227,10 +230,17 @@ describe('transforms', () => { console.log(fetch, BigQuery); ` - const transformedCode = transformCode(rawCode, hub, { - 'node-fetch': { bla: () => true }, - '@google-cloud/bigquery': { BigQuery: () => true }, - }) + const usedImports = new Set() + const transformedCode = transformCode( + rawCode, + hub, + { + 'node-fetch': { bla: () => true }, + '@google-cloud/bigquery': { BigQuery: () => true }, + }, + usedImports + ) + expect(usedImports).toEqual(new Set(['node-fetch', '@google-cloud/bigquery'])) expect(transformedCode).toStrictEqual(code` "use strict"; @@ -250,7 +260,7 @@ describe('transforms', () => { ` expect(() => { - transformCode(rawCode, hub, { 'node-fetch': { default: () => true } }) + transformCode(rawCode, hub, { 'node-fetch': { default: () => true } }, new Set()) }).toThrow("/index.ts: Cannot import 'kea'! This package is not provided by PostHog in plugins.") }) })