From e966fa75406b0315ecd10f85a04008b750a55644 Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Thu, 2 Feb 2023 18:25:50 +0800 Subject: [PATCH] meta: use load balancer for companion in e2e tests (#4228) --- .eslintrc.js | 4 +- .github/workflows/e2e.yml | 19 ++- e2e/cypress.config.mjs | 5 +- e2e/generate-test.mjs | 1 - e2e/package.json | 1 + e2e/start-companion-with-load-balancer.mjs | 79 +++++++++++++ package.json | 5 +- packages/@uppy/companion/src/companion.js | 8 ++ packages/@uppy/companion/src/server/logger.js | 34 +++--- .../@uppy/companion/src/standalone/helper.js | 110 ++++++++++-------- .../@uppy/companion/src/standalone/index.js | 22 ++-- .../companion/src/standalone/start-server.js | 11 +- website/src/docs/companion.md | 2 + yarn.lock | 1 + 14 files changed, 205 insertions(+), 97 deletions(-) create mode 100755 e2e/start-companion-with-load-balancer.mjs diff --git a/.eslintrc.js b/.eslintrc.js index 8b89e889e1..383849adfd 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -526,8 +526,8 @@ module.exports = { extends: ['plugin:cypress/recommended'], }, { - files: ['e2e/**/*.ts', 'e2e/**/*.js', 'e2e/**/*.jsx'], - rules: { 'import/no-extraneous-dependencies': 'off', 'no-unused-expressions': 'off' }, + files: ['e2e/**/*.ts', 'e2e/**/*.js', 'e2e/**/*.jsx', 'e2e/**/*.mjs'], + rules: { 'import/no-extraneous-dependencies': 'off', 'no-unused-expressions': 'off', 'no-console': 'off' }, }, ], } diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index 20b9c62ea7..d7103de1f8 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -62,6 +62,12 @@ jobs: uses: actions/setup-node@v3 with: node-version: lts/* + + - name: Start Redis + uses: supercharge/redis-github-action@1.4.0 + with: + redis-version: 7 + - name: Install dependencies run: corepack yarn install --immutable env: @@ -72,18 +78,21 @@ jobs: - name: Run end-to-end browser tests run: corepack yarn run e2e:ci env: + COMPANION_DATADIR: ./output + COMPANION_DOMAIN: localhost:3020 + COMPANION_PROTOCOL: http + COMPANION_REDIS_URL: redis://localhost:6379 COMPANION_UNSPLASH_KEY: ${{secrets.COMPANION_UNSPLASH_KEY}} COMPANION_UNSPLASH_SECRET: ${{secrets.COMPANION_UNSPLASH_SECRET}} + COMPANION_AWS_KEY: ${{secrets.COMPANION_AWS_KEY}} + COMPANION_AWS_SECRET: ${{secrets.COMPANION_AWS_SECRET}} + COMPANION_AWS_BUCKET: ${{secrets.COMPANION_AWS_BUCKET}} + COMPANION_AWS_REGION: ${{secrets.COMPANION_AWS_REGION}} VITE_COMPANION_URL: http://localhost:3020 VITE_TRANSLOADIT_KEY: ${{secrets.TRANSLOADIT_KEY}} VITE_TRANSLOADIT_SECRET: ${{secrets.TRANSLOADIT_SECRET}} VITE_TRANSLOADIT_TEMPLATE: ${{secrets.TRANSLOADIT_TEMPLATE}} VITE_TRANSLOADIT_SERVICE_URL: ${{secrets.TRANSLOADIT_SERVICE_URL}} - COMPANION_AWS_KEY: ${{secrets.COMPANION_AWS_KEY}} - COMPANION_AWS_SECRET: ${{secrets.COMPANION_AWS_SECRET}} - COMPANION_AWS_BUCKET: ${{secrets.COMPANION_AWS_BUCKET}} - COMPANION_AWS_REGION: ${{secrets.COMPANION_AWS_REGION}} - COMPANION_AWS_DISABLE_ACL: 'true' # https://docs.cypress.io/guides/references/advanced-installation#Binary-cache CYPRESS_CACHE_FOLDER: ${{ steps.cypress-cache-dir-path.outputs.dir }} - name: Upload videos in case of failure diff --git a/e2e/cypress.config.mjs b/e2e/cypress.config.mjs index 4873b7b451..9932263602 100644 --- a/e2e/cypress.config.mjs +++ b/e2e/cypress.config.mjs @@ -1,6 +1,4 @@ -// eslint-disable-next-line import/no-extraneous-dependencies import { defineConfig } from 'cypress' -// eslint-disable-next-line import/no-extraneous-dependencies import installLogsPrinter from 'cypress-terminal-report/src/installLogsPrinter.js' export default defineConfig({ @@ -10,8 +8,7 @@ export default defineConfig({ baseUrl: 'http://localhost:1234', specPattern: 'cypress/integration/*.spec.ts', - // eslint-disable-next-line no-unused-vars - setupNodeEvents (on, config) { + setupNodeEvents (on) { // implement node event listeners here installLogsPrinter(on) }, diff --git a/e2e/generate-test.mjs b/e2e/generate-test.mjs index 739a3936c5..c4e9cc8eb5 100755 --- a/e2e/generate-test.mjs +++ b/e2e/generate-test.mjs @@ -1,5 +1,4 @@ #!/usr/bin/env node -/* eslint-disable no-console, import/no-extraneous-dependencies */ import prompts from 'prompts' import fs from 'node:fs/promises' diff --git a/e2e/package.json b/e2e/package.json index 92df911f03..2fe1ef2582 100644 --- a/e2e/package.json +++ b/e2e/package.json @@ -48,6 +48,7 @@ "cypress": "^10.0.0", "cypress-terminal-report": "^4.1.2", "deep-freeze": "^0.0.1", + "execa": "^6.1.0", "parcel": "^2.0.1", "prompts": "^2.4.2", "react": "^18.1.0", diff --git a/e2e/start-companion-with-load-balancer.mjs b/e2e/start-companion-with-load-balancer.mjs new file mode 100755 index 0000000000..28de8fcc71 --- /dev/null +++ b/e2e/start-companion-with-load-balancer.mjs @@ -0,0 +1,79 @@ +#!/usr/bin/env node + +import { execa } from 'execa' +import http from 'node:http' +import httpProxy from 'http-proxy' + +const numInstances = 3 +const lbPort = 3020 +const companionStartPort = 3021 + +// simple load balancer that will direct requests round robin between companion instances +function createLoadBalancer (baseUrls) { + const proxy = httpProxy.createProxyServer({ ws: true }) + + let i = 0 + + function getTarget () { + return baseUrls[i % baseUrls.length] + } + + const server = http.createServer((req, res) => { + const target = getTarget() + // console.log('req', req.method, target, req.url) + proxy.web(req, res, { target }, (err) => { + console.error('Load balancer failed to proxy request', err.message) + res.statusCode = 500 + res.end() + }) + i++ + }) + + server.on('upgrade', (req, socket, head) => { + const target = getTarget() + // console.log('upgrade', target, req.url) + proxy.ws(req, socket, head, { target }, (err) => { + console.error('Load balancer failed to proxy websocket', err.message) + console.error(err) + socket.destroy() + }) + i++ + }) + + server.listen(lbPort) + console.log('Load balancer listening', lbPort) + return server +} + +const startCompanion = ({ name, port }) => execa('nodemon', [ + '--watch', 'packages/@uppy/companion/src', '--exec', 'node', '-r', 'dotenv/config', './packages/@uppy/companion/src/standalone/start-server.js', +], { + cwd: new URL('../', import.meta.url), + stdio: 'inherit', + env: { + // Note: these env variables will override anything set in .env + COMPANION_PORT: port, + COMPANION_SECRET: 'development', // multi instance will not work without secret set + COMPANION_PREAUTH_SECRET: 'development', // multi instance will not work without secret set + COMPANION_ALLOW_LOCAL_URLS: 'true', + COMPANION_LOGGER_PROCESS_NAME: name, + }, +}) + +const hosts = Array.from({ length: numInstances }, (_, index) => { + const port = companionStartPort + index + return { index, port } +}) + +console.log('Starting companion instances on ports', hosts.map(({ port }) => port)) + +const companions = hosts.map(({ index, port }) => startCompanion({ name: `companion${index}`, port })) + +let loadBalancer +try { + loadBalancer = createLoadBalancer(hosts.map(({ port }) => `http://localhost:${port}`)) + await Promise.all(companions) +} finally { + loadBalancer?.close() + companions.forEach((companion) => companion.kill()) +} diff --git a/package.json b/package.json index 339b4a45b5..45cc17323d 100644 --- a/package.json +++ b/package.json @@ -130,10 +130,11 @@ "release": "PACKAGES=$(yarn workspaces list --json) yarn workspace @uppy-dev/release interactive", "size": "echo 'JS Bundle mingz:' && cat ./packages/uppy/dist/uppy.min.js | gzip | wc -c && echo 'CSS Bundle mingz:' && cat ./packages/uppy/dist/uppy.min.css | gzip | wc -c", "start:companion": "bash bin/companion.sh", + "start:companion:with-loadbalancer": "e2e/start-companion-with-load-balancer.mjs", "start": "npm-run-all --parallel watch start:companion web:start", "e2e": "yarn build && yarn e2e:skip-build", - "e2e:skip-build": "npm-run-all --parallel watch:js:lib e2e:client start:companion e2e:cypress", - "e2e:ci": "start-server-and-test 'npm-run-all --parallel e2e:client start:companion' '1234|3020' e2e:headless", + "e2e:skip-build": "npm-run-all --parallel watch:js:lib e2e:client start:companion:with-loadbalancer e2e:cypress", + "e2e:ci": "start-server-and-test 'npm-run-all --parallel e2e:client start:companion:with-loadbalancer' '1234|3020' e2e:headless", "e2e:client": "yarn workspace e2e client:start", "e2e:cypress": "yarn workspace e2e cypress:open", "e2e:headless": "yarn workspace e2e cypress:headless", diff --git a/packages/@uppy/companion/src/companion.js b/packages/@uppy/companion/src/companion.js index c7d739b7b4..dae53724a1 100644 --- a/packages/@uppy/companion/src/companion.js +++ b/packages/@uppy/companion/src/companion.js @@ -21,6 +21,10 @@ const { getCredentialsOverrideMiddleware } = require('./server/provider/credenti // @ts-ignore const { version } = require('../package.json') +function setLoggerProcessName ({ loggerProcessName }) { + if (loggerProcessName != null) logger.setProcessName(loggerProcessName) +} + // intercepts grantJS' default response error when something goes // wrong during oauth process. const interceptGrantErrorResponse = interceptor((req, res) => { @@ -51,6 +55,8 @@ const interceptGrantErrorResponse = interceptor((req, res) => { module.exports.errors = { ProviderApiError, ProviderAuthError } module.exports.socket = require('./server/socket') +module.exports.setLoggerProcessName = setLoggerProcessName + /** * Entry point into initializing the Companion app. * @@ -58,6 +64,8 @@ module.exports.socket = require('./server/socket') * @returns {{ app: import('express').Express, emitter: any }}} */ module.exports.app = (optionsArg = {}) => { + setLoggerProcessName(optionsArg) + validateConfig(optionsArg) const options = merge({}, defaultOptions, optionsArg) diff --git a/packages/@uppy/companion/src/server/logger.js b/packages/@uppy/companion/src/server/logger.js index c10b753894..112a35555b 100644 --- a/packages/@uppy/companion/src/server/logger.js +++ b/packages/@uppy/companion/src/server/logger.js @@ -33,18 +33,25 @@ function maskMessage (msg) { return out } +let processName = 'companion' + +exports.setProcessName = (newProcessName) => { + processName = newProcessName +} + /** * message log * - * @param {string | Error} arg the message or error to log - * @param {string} tag a unique tag to easily search for this message - * @param {string} level error | info | debug - * @param {string} [id] a unique id to easily trace logs tied to a request - * @param {Function} [color] function to display the log in appropriate color + * @param {object} params + * @param {string | Error} params.arg the message or error to log + * @param {string} params.tag a unique tag to easily search for this message + * @param {string} params.level error | info | debug + * @param {string} [params.traceId] a unique id to easily trace logs tied to a request + * @param {Function} [params.color] function to display the log in appropriate color */ -const log = (arg, tag = '', level, id = '', color = (message) => message) => { +const log = ({ arg, tag = '', level, traceId = '', color = (message) => message }) => { const time = new Date().toISOString() - const whitespace = tag && id ? ' ' : '' + const whitespace = tag && traceId ? ' ' : '' function msgToString () { // We don't need to log stack trace on special errors that we ourselves have produced @@ -59,7 +66,7 @@ const log = (arg, tag = '', level, id = '', color = (message) => message) => { const msgString = msgToString() const masked = maskMessage(msgString) // eslint-disable-next-line no-console - console.log(color(`companion: ${time} [${level}] ${id}${whitespace}${tag}`), color(masked)) + console.log(color(`${processName}: ${time} [${level}] ${traceId}${whitespace}${tag}`), color(masked)) } /** @@ -70,7 +77,7 @@ const log = (arg, tag = '', level, id = '', color = (message) => message) => { * @param {string} [traceId] a unique id to easily trace logs tied to a request */ exports.info = (msg, tag, traceId) => { - log(msg, tag, 'info', traceId) + log({ arg: msg, tag, level: 'info', traceId }) } /** @@ -81,8 +88,7 @@ exports.info = (msg, tag, traceId) => { * @param {string} [traceId] a unique id to easily trace logs tied to a request */ exports.warn = (msg, tag, traceId) => { - // @ts-ignore - log(msg, tag, 'warn', traceId, chalk.bold.yellow) + log({ arg: msg, tag, level: 'warn', traceId, color: chalk.bold.yellow }) } /** @@ -93,8 +99,7 @@ exports.warn = (msg, tag, traceId) => { * @param {string} [traceId] a unique id to easily trace logs tied to a request */ exports.error = (msg, tag, traceId) => { - // @ts-ignore - log(msg, tag, 'error', traceId, chalk.bold.red) + log({ arg: msg, tag, level: 'error', traceId, color: chalk.bold.red }) } /** @@ -106,7 +111,6 @@ exports.error = (msg, tag, traceId) => { */ exports.debug = (msg, tag, traceId) => { if (process.env.NODE_ENV !== 'production') { - // @ts-ignore - log(msg, tag, 'debug', traceId, chalk.bold.blue) + log({ arg: msg, tag, level: 'debug', traceId, color: chalk.bold.blue }) } } diff --git a/packages/@uppy/companion/src/standalone/helper.js b/packages/@uppy/companion/src/standalone/helper.js index 5d8df6b524..4b90eda6a0 100644 --- a/packages/@uppy/companion/src/standalone/helper.js +++ b/packages/@uppy/companion/src/standalone/helper.js @@ -9,13 +9,48 @@ const logger = require('../server/logger') const { version } = require('../../package.json') /** - * Reads all companion configuration set via environment variables - * and via the config file path + * Tries to read the secret from a file if the according environment variable is set. + * Otherwise it falls back to the standard secret environment variable. * - * @returns {object} + * @param {string} baseEnvVar + * + * @returns {string} */ -exports.getCompanionOptions = (options = {}) => { - return merge({}, getConfigFromEnv(), getConfigFromFile(), options) +const getSecret = (baseEnvVar) => { + const secretFile = process.env[`${baseEnvVar}_FILE`] + return secretFile + ? fs.readFileSync(secretFile).toString() + : process.env[baseEnvVar] +} + +/** + * Auto-generates server secret + * + * @returns {string} + */ +exports.generateSecret = () => { + logger.warn('auto-generating server secret because none was specified', 'startup.secret') + return crypto.randomBytes(64).toString('hex') +} + +/** + * + * @param {string} url + */ +const hasProtocol = (url) => { + return url.startsWith('https://') || url.startsWith('http://') +} + +function getCorsOrigins () { + if (process.env.COMPANION_CLIENT_ORIGINS) { + return process.env.COMPANION_CLIENT_ORIGINS + .split(',') + .map((url) => (hasProtocol(url) ? url : `${process.env.COMPANION_PROTOCOL || 'http'}://${url}`)) + } + if (process.env.COMPANION_CLIENT_ORIGINS_REGEX) { + return new RegExp(process.env.COMPANION_CLIENT_ORIGINS_REGEX) + } + return undefined } /** @@ -104,8 +139,8 @@ const getConfigFromEnv = () => { redisOptions: {}, sendSelfEndpoint: process.env.COMPANION_SELF_ENDPOINT, uploadUrls: uploadUrls ? uploadUrls.split(',') : null, - secret: getSecret('COMPANION_SECRET') || generateSecret(), - preAuthSecret: getSecret('COMPANION_PREAUTH_SECRET') || generateSecret(), + secret: getSecret('COMPANION_SECRET'), + preAuthSecret: getSecret('COMPANION_PREAUTH_SECRET'), allowLocalUrls: process.env.COMPANION_ALLOW_LOCAL_URLS === 'true', // cookieDomain is kind of a hack to support distributed systems. This should be improved but we never got so far. cookieDomain: process.env.COMPANION_COOKIE_DOMAIN, @@ -115,32 +150,29 @@ const getConfigFromEnv = () => { clientSocketConnectTimeout: process.env.COMPANION_CLIENT_SOCKET_CONNECT_TIMEOUT ? parseInt(process.env.COMPANION_CLIENT_SOCKET_CONNECT_TIMEOUT, 10) : undefined, metrics: process.env.COMPANION_HIDE_METRICS !== 'true', + loggerProcessName: process.env.COMPANION_LOGGER_PROCESS_NAME, + corsOrigins: getCorsOrigins(), } } /** - * Tries to read the secret from a file if the according environment variable is set. - * Otherwise it falls back to the standard secret environment variable. - * - * @param {string} baseEnvVar + * Returns the config path specified via cli arguments * * @returns {string} */ -const getSecret = (baseEnvVar) => { - const secretFile = process.env[`${baseEnvVar}_FILE`] - return secretFile - ? fs.readFileSync(secretFile).toString() - : process.env[baseEnvVar] -} +const getConfigPath = () => { + let configPath -/** - * Auto-generates server secret - * - * @returns {string} - */ -const generateSecret = () => { - logger.warn('auto-generating server secret because none was specified', 'startup.secret') - return crypto.randomBytes(64).toString('hex') + for (let i = process.argv.length - 1; i >= 0; i--) { + const isConfigFlag = process.argv[i] === '-c' || process.argv[i] === '--config' + const flagHasValue = i + 1 <= process.argv.length + if (isConfigFlag && flagHasValue) { + configPath = process.argv[i + 1] + break + } + } + + return configPath } /** @@ -158,31 +190,13 @@ const getConfigFromFile = () => { } /** - * Returns the config path specified via cli arguments - * - * @returns {string} - */ -const getConfigPath = () => { - let configPath - - for (let i = process.argv.length - 1; i >= 0; i--) { - const isConfigFlag = process.argv[i] === '-c' || process.argv[i] === '--config' - const flagHasValue = i + 1 <= process.argv.length - if (isConfigFlag && flagHasValue) { - configPath = process.argv[i + 1] - break - } - } - - return configPath -} - -/** + * Reads all companion configuration set via environment variables + * and via the config file path * - * @param {string} url + * @returns {object} */ -exports.hasProtocol = (url) => { - return url.startsWith('http://') || url.startsWith('https://') +exports.getCompanionOptions = (options = {}) => { + return merge({}, getConfigFromEnv(), getConfigFromFile(), options) } exports.buildHelpfulStartupMessage = (companionOptions) => { diff --git a/packages/@uppy/companion/src/standalone/index.js b/packages/@uppy/companion/src/standalone/index.js index 5336d759f1..93e45a8950 100644 --- a/packages/@uppy/companion/src/standalone/index.js +++ b/packages/@uppy/companion/src/standalone/index.js @@ -11,25 +11,20 @@ const connectRedis = require('connect-redis') const logger = require('../server/logger') const redis = require('../server/redis') const companion = require('../companion') -const helper = require('./helper') +const { getCompanionOptions, generateSecret, buildHelpfulStartupMessage } = require('./helper') /** * Configures an Express app for running Companion standalone * * @returns {object} */ -module.exports = function server (inputCompanionOptions = {}) { - let corsOrigins - if (process.env.COMPANION_CLIENT_ORIGINS) { - corsOrigins = process.env.COMPANION_CLIENT_ORIGINS - .split(',') - .map((url) => (helper.hasProtocol(url) ? url : `${process.env.COMPANION_PROTOCOL || 'http'}://${url}`)) - } else if (process.env.COMPANION_CLIENT_ORIGINS_REGEX) { - corsOrigins = new RegExp(process.env.COMPANION_CLIENT_ORIGINS_REGEX) - } +module.exports = function server (inputCompanionOptions) { + const companionOptions = getCompanionOptions(inputCompanionOptions) + + companion.setLoggerProcessName(companionOptions) - const moreCompanionOptions = { ...inputCompanionOptions, corsOrigins } - const companionOptions = helper.getCompanionOptions(moreCompanionOptions) + if (!companionOptions.secret) companionOptions.secret = generateSecret() + if (!companionOptions.preAuthSecret) companionOptions.preAuthSecret = generateSecret() const app = express() @@ -98,6 +93,7 @@ module.exports = function server (inputCompanionOptions = {}) { const { query, censored } = censorQuery(rawQuery) return censored ? `${parsed.href.split('?')[0]}?${qs.stringify(query)}` : parsed.href } + return undefined }) router.use(bodyParser.json()) @@ -141,7 +137,7 @@ module.exports = function server (inputCompanionOptions = {}) { if (process.env.COMPANION_HIDE_WELCOME !== 'true') { router.get('/', (req, res) => { res.setHeader('Content-Type', 'text/plain') - res.send(helper.buildHelpfulStartupMessage(companionOptions)) + res.send(buildHelpfulStartupMessage(companionOptions)) }) } diff --git a/packages/@uppy/companion/src/standalone/start-server.js b/packages/@uppy/companion/src/standalone/start-server.js index 18b17a6db8..2bc25214b1 100755 --- a/packages/@uppy/companion/src/standalone/start-server.js +++ b/packages/@uppy/companion/src/standalone/start-server.js @@ -3,16 +3,13 @@ const companion = require('../companion') // @ts-ignore const { version } = require('../../package.json') const standalone = require('.') -const { getURLBuilder } = require('../server/helpers/utils') +const logger = require('../server/logger') const port = process.env.COMPANION_PORT || process.env.PORT || 3020 -const { app, companionOptions } = standalone() +const { app } = standalone() companion.socket(app.listen(port)) -const buildURL = getURLBuilder(companionOptions) - -/* eslint-disable no-console */ -console.log(`Welcome to Companion! v${version}`) -console.log(`Listening on ${buildURL('/', false)}`) +logger.info(`Welcome to Companion! v${version}`) +logger.info(`Listening on http://localhost:${port}`) diff --git a/website/src/docs/companion.md b/website/src/docs/companion.md index 6f1b43d769..5cc0139438 100644 --- a/website/src/docs/companion.md +++ b/website/src/docs/companion.md @@ -191,6 +191,8 @@ export COMPANION_PATH="/SERVER/PATH/TO/WHERE/COMPANION/LIVES" export COMPANION_HIDE_WELCOME="true" # disables the metrics page, defaults to false export COMPANION_HIDE_METRICS="true" +# prefix all log entries with this value - useful for multiple instances +export COMPANION_LOGGER_PROCESS_NAME="companion" # use this in place of COMPANION_PATH if the server path should not be # handled by the express.js app, but maybe by an external server configuration diff --git a/yarn.lock b/yarn.lock index e095db10ba..206b82e78d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -15584,6 +15584,7 @@ __metadata: cypress: ^10.0.0 cypress-terminal-report: ^4.1.2 deep-freeze: ^0.0.1 + execa: ^6.1.0 parcel: ^2.0.1 prompts: ^2.4.2 react: ^18.1.0