Skip to content

Commit

Permalink
chore(plugin-server): add deprecated imports list, log if they are us…
Browse files Browse the repository at this point in the history
…ed, remove 'google' from global plugin scope
  • Loading branch information
bretthoerner committed Nov 17, 2023
1 parent 4164eee commit 93891e3
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 44 deletions.
6 changes: 5 additions & 1 deletion plugin-server/src/worker/plugins/setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { loadSchedule } from './loadSchedule'
import { teardownPlugins } from './teardown'

export async function setupPlugins(hub: Hub): Promise<void> {
const startTime = Date.now()
status.info('🔁', `Loading plugin configs...`)
const { plugins, pluginConfigs, pluginConfigsPerTeam } = await loadPluginsFromDB(hub)
const pluginVMLoadPromises: Array<Promise<any>> = []
Expand Down Expand Up @@ -59,5 +60,8 @@ export async function setupPlugins(hub: Hub): Promise<void> {
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`
)
}
20 changes: 0 additions & 20 deletions plugin-server/src/worker/vm/extensions/google.ts

This file was deleted.

9 changes: 7 additions & 2 deletions plugin-server/src/worker/vm/transforms/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,12 @@ import { replaceImports } from './replace-imports'

const memoize: Record<string, string> = {}

export function transformCode(rawCode: string, server: Hub, imports?: Record<string, any>): string {
export function transformCode(
rawCode: string,
server: Hub,
imports: Record<string, any>,
usedImports: Set<string>
): 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]
Expand All @@ -20,7 +25,7 @@ export function transformCode(rawCode: string, server: Hub, imports?: Record<str
configFile: false,
filename: 'index.ts',
presets: ['typescript', ['env', { targets: { node: process.versions.node } }]],
plugins: [replaceImports(server, imports), loopTimeout(server), promiseTimeout(server)],
plugins: [replaceImports(server, imports, usedImports), loopTimeout(server), promiseTimeout(server)],
})
if (!code) {
throw new Error(`Babel transform gone wrong! Could not process the following code:\n${rawCode}`)
Expand Down
6 changes: 5 additions & 1 deletion plugin-server/src/worker/vm/transforms/replace-imports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Hub } from '../../../types'
import { PluginGen } from './common'

export const replaceImports: PluginGen =
(server: Hub, imports: Record<string, any> = {}) =>
(_: Hub, imports: Record<string, any> = {}, usedImports: Set<string>) =>
({ types: t }) => ({
visitor: {
ImportDeclaration: {
Expand All @@ -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)) {
Expand Down Expand Up @@ -63,6 +65,8 @@ export const replaceImports: PluginGen =
)
}

usedImports.add(importSource)

path.replaceWith(
t.memberExpression(t.identifier('__pluginHostImports'), t.stringLiteral(importSource), true)
)
Expand Down
26 changes: 22 additions & 4 deletions plugin-server/src/worker/vm/vm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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
Expand All @@ -44,7 +55,16 @@ export function createPluginConfigVM(
})
}

const transformedCode = transformCode(indexJs, hub, AVAILABLE_IMPORTS)
const usedImports: Set<string> = 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({
Expand All @@ -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') {
Expand Down
42 changes: 26 additions & 16 deletions plugin-server/tests/worker/transforms.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<void>
Expand All @@ -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";
Expand All @@ -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";
Expand All @@ -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";
Expand All @@ -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";
Expand All @@ -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";
Expand All @@ -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";
Expand All @@ -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";
Expand Down Expand Up @@ -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";
Expand All @@ -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";
Expand All @@ -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.")
})

Expand All @@ -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<string>()
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";
Expand All @@ -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.")
})
})
Expand Down

0 comments on commit 93891e3

Please sign in to comment.