From 4b80625a2020c050f7ab6747f4b5e8467618e7e1 Mon Sep 17 00:00:00 2001 From: Thomas Watson Date: Wed, 28 Aug 2024 16:45:35 +0200 Subject: [PATCH] [test] allow FakeAgent to respond to remote config requests (#4626) Allow integration tests to mock the remote config abilities of the agent. This is done by first setting up a new remote config "file" using the new `FakeAgent#addRemoteConfig` method before running the main part of the integration test. This primes the fake agent to respond with this config for http requests to the `/v0.7/config` endpoint. --- integration-tests/helpers/fake-agent.js | 298 ++++++++++++++++++ .../{helpers.js => helpers/index.js} | 158 +--------- .../src/appsec/remote_config/manager.js | 5 +- .../test/appsec/remote_config/manager.spec.js | 50 +-- 4 files changed, 322 insertions(+), 189 deletions(-) create mode 100644 integration-tests/helpers/fake-agent.js rename integration-tests/{helpers.js => helpers/index.js} (69%) diff --git a/integration-tests/helpers/fake-agent.js b/integration-tests/helpers/fake-agent.js new file mode 100644 index 00000000000..86c6890bf00 --- /dev/null +++ b/integration-tests/helpers/fake-agent.js @@ -0,0 +1,298 @@ +'use strict' + +const { createHash } = require('crypto') +const EventEmitter = require('events') +const http = require('http') +const express = require('express') +const bodyParser = require('body-parser') +const msgpack = require('msgpack-lite') +const codec = msgpack.createCodec({ int64: true }) +const upload = require('multer')() + +module.exports = class FakeAgent extends EventEmitter { + constructor (port = 0) { + super() + this.port = port + this._rcFiles = {} + this._rcTargetsVersion = 0 + } + + async start () { + return new Promise((resolve, reject) => { + const timeoutObj = setTimeout(() => { + reject(new Error('agent timed out starting up')) + }, 10000) + this.server = http.createServer(buildExpressServer(this)) + this.server.on('error', reject) + this.server.listen(this.port, () => { + this.port = this.server.address().port + clearTimeout(timeoutObj) + resolve(this) + }) + }) + } + + stop () { + return new Promise((resolve) => { + this.server.on('close', resolve) + this.server.close() + }) + } + + /** + * Add a config object to be returned by the fake Remote Config endpoint. + * @param {Object} config - Object containing the Remote Config "file" and metadata + * @param {number} [config.orgId=2] - The Datadog organization ID + * @param {string} config.product - The Remote Config product name + * @param {string} config.id - The Remote Config config ID + * @param {string} [config.name] - The Remote Config "name". Defaults to the sha256 hash of `config.id` + * @param {Object} config.config - The Remote Config "file" object + */ + addRemoteConfig (config) { + config = { ...config } + config.orgId = config.orgId || 2 + config.name = config.name || createHash('sha256').update(config.id).digest('hex') + config.config = JSON.stringify(config.config) + config.path = `datadog/${config.orgId}/${config.product}/${config.id}/${config.name}` + config.fileHash = createHash('sha256').update(config.config).digest('hex') + config.meta = { + custom: { v: 1 }, + hashes: { sha256: config.fileHash }, + length: config.config.length + } + + this._rcFiles[config.id] = config + this._rcTargetsVersion++ + } + + /** + * Update an existing config object + * @param {string} id - The Remote Config config ID + * @param {Object} config - The Remote Config "file" object + */ + updateRemoteConfig (id, config) { + config = JSON.stringify(config) + config = Object.assign( + this._rcFiles[id], + { + config, + fileHash: createHash('sha256').update(config).digest('hex') + } + ) + config.meta.custom.v++ + config.meta.hashes.sha256 = config.fileHash + config.meta.length = config.config.length + this._rcTargetsVersion++ + } + + /** + * Remove a specific config object + * @param {string} id - The ID of the config object that should be removed + */ + removeRemoteConfig (id) { + delete this._rcFiles[id] + this._rcTargetsVersion++ + } + + /** + * Remove any existing config added by calls to FakeAgent#addRemoteConfig. + */ + resetRemoteConfig () { + this._rcFiles = {} + this._rcTargetsVersion++ + } + + // **resolveAtFirstSuccess** - specific use case for Next.js (or any other future libraries) + // where multiple payloads are generated, and only one is expected to have the proper span (ie next.request), + // but it't not guaranteed to be the last one (so, expectedMessageCount would not be helpful). + // It can still fail if it takes longer than `timeout` duration or if none pass the assertions (timeout still called) + assertMessageReceived (fn, timeout, expectedMessageCount = 1, resolveAtFirstSuccess) { + timeout = timeout || 30000 + let resultResolve + let resultReject + let msgCount = 0 + const errors = [] + + const timeoutObj = setTimeout(() => { + const errorsMsg = errors.length === 0 ? '' : `, additionally:\n${errors.map(e => e.stack).join('\n')}\n===\n` + resultReject(new Error(`timeout${errorsMsg}`, { cause: { errors } })) + }, timeout) + + const resultPromise = new Promise((resolve, reject) => { + resultResolve = () => { + clearTimeout(timeoutObj) + resolve() + } + resultReject = (e) => { + clearTimeout(timeoutObj) + reject(e) + } + }) + + const messageHandler = msg => { + try { + msgCount += 1 + fn(msg) + if (resolveAtFirstSuccess || msgCount === expectedMessageCount) { + resultResolve() + this.removeListener('message', messageHandler) + } + } catch (e) { + errors.push(e) + } + } + this.on('message', messageHandler) + + return resultPromise + } + + assertTelemetryReceived (fn, timeout, requestType, expectedMessageCount = 1) { + timeout = timeout || 30000 + let resultResolve + let resultReject + let msgCount = 0 + const errors = [] + + const timeoutObj = setTimeout(() => { + const errorsMsg = errors.length === 0 ? '' : `, additionally:\n${errors.map(e => e.stack).join('\n')}\n===\n` + resultReject(new Error(`timeout${errorsMsg}`, { cause: { errors } })) + }, timeout) + + const resultPromise = new Promise((resolve, reject) => { + resultResolve = () => { + clearTimeout(timeoutObj) + resolve() + } + resultReject = (e) => { + clearTimeout(timeoutObj) + reject(e) + } + }) + + const messageHandler = msg => { + if (msg.payload.request_type !== requestType) return + msgCount += 1 + try { + fn(msg) + if (msgCount === expectedMessageCount) { + resultResolve() + } + } catch (e) { + errors.push(e) + } + if (msgCount === expectedMessageCount) { + this.removeListener('telemetry', messageHandler) + } + } + this.on('telemetry', messageHandler) + + return resultPromise + } +} + +function buildExpressServer (agent) { + const app = express() + + app.use(bodyParser.raw({ limit: Infinity, type: 'application/msgpack' })) + app.use(bodyParser.json({ limit: Infinity, type: 'application/json' })) + + app.put('/v0.4/traces', (req, res) => { + if (req.body.length === 0) return res.status(200).send() + res.status(200).send({ rate_by_service: { 'service:,env:': 1 } }) + agent.emit('message', { + headers: req.headers, + payload: msgpack.decode(req.body, { codec }) + }) + }) + + app.post('/v0.7/config', (req, res) => { + const { + client: { products, state }, + cached_target_files: cachedTargetFiles + } = req.body + + if (state.has_error) { + // Print the error sent by the client in case it's useful in debugging tests + console.error(state.error) // eslint-disable-line no-console + } + + for (const { apply_error: error } of state.config_states) { + if (error) { + // Print the error sent by the client in case it's useful in debugging tests + console.error(error) // eslint-disable-line no-console + } + } + + if (agent._rcTargetsVersion === state.targets_version) { + // If the state hasn't changed since the last time the client asked, just return an empty result + res.json({}) + return + } + + if (Object.keys(agent._rcFiles).length === 0) { + // All config files have been removed, but the client has not yet been informed. + // Return this custom result to let the client know. + res.json({ client_configs: [] }) + return + } + + // The actual targets object is much more complicated, + // but the Node.js tracer currently only cares about the following properties. + const targets = { + signed: { + custom: { opaque_backend_state: 'foo' }, + targets: {}, + version: agent._rcTargetsVersion + } + } + const targetFiles = [] + const clientConfigs = [] + + const files = Object.values(agent._rcFiles).filter(({ product }) => products.includes(product)) + + for (const { path, fileHash, meta, config } of files) { + clientConfigs.push(path) + targets.signed.targets[path] = meta + + // skip files already cached by the client so we don't send them more than once + if (cachedTargetFiles.some((cached) => + path === cached.path && + fileHash === cached.hashes.find((e) => e.algorithm === 'sha256').hash + )) continue + + targetFiles.push({ path, raw: base64(config) }) + } + + // The real response object also contains a `roots` property which has been omitted here since it's not currently + // used by the Node.js tracer. + res.json({ + targets: clientConfigs.length === 0 ? undefined : base64(targets), + target_files: targetFiles, + client_configs: clientConfigs + }) + }) + + app.post('/profiling/v1/input', upload.any(), (req, res) => { + res.status(200).send() + agent.emit('message', { + headers: req.headers, + payload: req.body, + files: req.files + }) + }) + + app.post('/telemetry/proxy/api/v2/apmtelemetry', (req, res) => { + res.status(200).send() + agent.emit('telemetry', { + headers: req.headers, + payload: req.body + }) + }) + + return app +} + +function base64 (strOrObj) { + const str = typeof strOrObj === 'string' ? strOrObj : JSON.stringify(strOrObj) + return Buffer.from(str).toString('base64') +} diff --git a/integration-tests/helpers.js b/integration-tests/helpers/index.js similarity index 69% rename from integration-tests/helpers.js rename to integration-tests/helpers/index.js index ae24451bea3..49a04544322 100644 --- a/integration-tests/helpers.js +++ b/integration-tests/helpers/index.js @@ -1,11 +1,6 @@ 'use strict' const { promisify } = require('util') -const express = require('express') -const bodyParser = require('body-parser') -const msgpack = require('msgpack-lite') -const codec = msgpack.createCodec({ int64: true }) -const EventEmitter = require('events') const childProcess = require('child_process') const { fork, spawn } = childProcess const exec = promisify(childProcess.exec) @@ -13,156 +8,13 @@ const http = require('http') const fs = require('fs') const os = require('os') const path = require('path') -const rimraf = promisify(require('rimraf')) -const id = require('../packages/dd-trace/src/id') -const upload = require('multer')() const assert = require('assert') +const rimraf = promisify(require('rimraf')) +const FakeAgent = require('./fake-agent') +const id = require('../../packages/dd-trace/src/id') const hookFile = 'dd-trace/loader-hook.mjs' -class FakeAgent extends EventEmitter { - constructor (port = 0) { - super() - this.port = port - } - - async start () { - const app = express() - app.use(bodyParser.raw({ limit: Infinity, type: 'application/msgpack' })) - app.use(bodyParser.json({ limit: Infinity, type: 'application/json' })) - app.put('/v0.4/traces', (req, res) => { - if (req.body.length === 0) return res.status(200).send() - res.status(200).send({ rate_by_service: { 'service:,env:': 1 } }) - this.emit('message', { - headers: req.headers, - payload: msgpack.decode(req.body, { codec }) - }) - }) - app.post('/profiling/v1/input', upload.any(), (req, res) => { - res.status(200).send() - this.emit('message', { - headers: req.headers, - payload: req.body, - files: req.files - }) - }) - app.post('/telemetry/proxy/api/v2/apmtelemetry', (req, res) => { - res.status(200).send() - this.emit('telemetry', { - headers: req.headers, - payload: req.body - }) - }) - - return new Promise((resolve, reject) => { - const timeoutObj = setTimeout(() => { - reject(new Error('agent timed out starting up')) - }, 10000) - this.server = http.createServer(app) - this.server.on('error', reject) - this.server.listen(this.port, () => { - this.port = this.server.address().port - clearTimeout(timeoutObj) - resolve(this) - }) - }) - } - - stop () { - return new Promise((resolve) => { - this.server.on('close', resolve) - this.server.close() - }) - } - - // **resolveAtFirstSuccess** - specific use case for Next.js (or any other future libraries) - // where multiple payloads are generated, and only one is expected to have the proper span (ie next.request), - // but it't not guaranteed to be the last one (so, expectedMessageCount would not be helpful). - // It can still fail if it takes longer than `timeout` duration or if none pass the assertions (timeout still called) - assertMessageReceived (fn, timeout, expectedMessageCount = 1, resolveAtFirstSuccess) { - timeout = timeout || 30000 - let resultResolve - let resultReject - let msgCount = 0 - const errors = [] - - const timeoutObj = setTimeout(() => { - const errorsMsg = errors.length === 0 ? '' : `, additionally:\n${errors.map(e => e.stack).join('\n')}\n===\n` - resultReject(new Error(`timeout${errorsMsg}`, { cause: { errors } })) - }, timeout) - - const resultPromise = new Promise((resolve, reject) => { - resultResolve = () => { - clearTimeout(timeoutObj) - resolve() - } - resultReject = (e) => { - clearTimeout(timeoutObj) - reject(e) - } - }) - - const messageHandler = msg => { - try { - msgCount += 1 - fn(msg) - if (resolveAtFirstSuccess || msgCount === expectedMessageCount) { - resultResolve() - this.removeListener('message', messageHandler) - } - } catch (e) { - errors.push(e) - } - } - this.on('message', messageHandler) - - return resultPromise - } - - assertTelemetryReceived (fn, timeout, requestType, expectedMessageCount = 1) { - timeout = timeout || 30000 - let resultResolve - let resultReject - let msgCount = 0 - const errors = [] - - const timeoutObj = setTimeout(() => { - const errorsMsg = errors.length === 0 ? '' : `, additionally:\n${errors.map(e => e.stack).join('\n')}\n===\n` - resultReject(new Error(`timeout${errorsMsg}`, { cause: { errors } })) - }, timeout) - - const resultPromise = new Promise((resolve, reject) => { - resultResolve = () => { - clearTimeout(timeoutObj) - resolve() - } - resultReject = (e) => { - clearTimeout(timeoutObj) - reject(e) - } - }) - - const messageHandler = msg => { - if (msg.payload.request_type !== requestType) return - msgCount += 1 - try { - fn(msg) - if (msgCount === expectedMessageCount) { - resultResolve() - } - } catch (e) { - errors.push(e) - } - if (msgCount === expectedMessageCount) { - this.removeListener('telemetry', messageHandler) - } - } - this.on('telemetry', messageHandler) - - return resultPromise - } -} - async function runAndCheckOutput (filename, cwd, expectedOut) { const proc = spawn('node', [filename], { cwd, stdio: 'pipe' }) const pid = proc.pid @@ -246,7 +98,7 @@ async function runAndCheckWithTelemetry (filename, expectedOut, ...expectedTelem language_version: process.versions.node, runtime_name: 'nodejs', runtime_version: process.versions.node, - tracer_version: require('../package.json').version, + tracer_version: require('../../package.json').version, pid: Number(pid) } } @@ -347,7 +199,7 @@ async function createSandbox (dependencies = [], isGitRepo = false, function telemetryForwarder (expectedTelemetryPoints) { process.env.DD_TELEMETRY_FORWARDER_PATH = - path.join(__dirname, 'telemetry-forwarder.sh') + path.join(__dirname, '..', 'telemetry-forwarder.sh') process.env.FORWARDER_OUT = path.join(__dirname, `forwarder-${Date.now()}.out`) let retries = 0 diff --git a/packages/dd-trace/src/appsec/remote_config/manager.js b/packages/dd-trace/src/appsec/remote_config/manager.js index 9cc636cd302..5b0044e2c71 100644 --- a/packages/dd-trace/src/appsec/remote_config/manager.js +++ b/packages/dd-trace/src/appsec/remote_config/manager.js @@ -120,7 +120,10 @@ class RemoteConfigManager extends EventEmitter { const options = { url: this.url, method: 'POST', - path: '/v0.7/config' + path: '/v0.7/config', + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } } request(this.getPayload(), options, (err, data, statusCode) => { diff --git a/packages/dd-trace/test/appsec/remote_config/manager.spec.js b/packages/dd-trace/test/appsec/remote_config/manager.spec.js index 4c4d2ad1ccd..8e5fdc6b516 100644 --- a/packages/dd-trace/test/appsec/remote_config/manager.spec.js +++ b/packages/dd-trace/test/appsec/remote_config/manager.spec.js @@ -178,8 +178,16 @@ describe('RemoteConfigManager', () => { }) describe('poll', () => { + let expectedPayload + beforeEach(() => { sinon.stub(rc, 'parseConfig') + expectedPayload = { + url: rc.url, + method: 'POST', + path: '/v0.7/config', + headers: { 'Content-Type': 'application/json; charset=utf-8' } + } }) it('should request and do nothing when received status 404', (cb) => { @@ -188,11 +196,7 @@ describe('RemoteConfigManager', () => { const payload = JSON.stringify(rc.state) rc.poll(() => { - expect(request).to.have.been.calledOnceWith(payload, { - url: rc.url, - method: 'POST', - path: '/v0.7/config' - }) + expect(request).to.have.been.calledOnceWith(payload, expectedPayload) expect(log.error).to.not.have.been.called expect(rc.parseConfig).to.not.have.been.called cb() @@ -206,11 +210,7 @@ describe('RemoteConfigManager', () => { const payload = JSON.stringify(rc.state) rc.poll(() => { - expect(request).to.have.been.calledOnceWith(payload, { - url: rc.url, - method: 'POST', - path: '/v0.7/config' - }) + expect(request).to.have.been.calledOnceWith(payload, expectedPayload) expect(log.error).to.have.been.calledOnceWithExactly(err) expect(rc.parseConfig).to.not.have.been.called cb() @@ -223,11 +223,7 @@ describe('RemoteConfigManager', () => { const payload = JSON.stringify(rc.state) rc.poll(() => { - expect(request).to.have.been.calledOnceWith(payload, { - url: rc.url, - method: 'POST', - path: '/v0.7/config' - }) + expect(request).to.have.been.calledOnceWith(payload, expectedPayload) expect(log.error).to.not.have.been.called expect(rc.parseConfig).to.have.been.calledOnceWithExactly({ a: 'b' }) cb() @@ -243,11 +239,7 @@ describe('RemoteConfigManager', () => { const payload = JSON.stringify(rc.state) rc.poll(() => { - expect(request).to.have.been.calledOnceWith(payload, { - url: rc.url, - method: 'POST', - path: '/v0.7/config' - }) + expect(request).to.have.been.calledOnceWith(payload, expectedPayload) expect(rc.parseConfig).to.have.been.calledOnceWithExactly({ a: 'b' }) expect(log.error).to.have.been .calledOnceWithExactly('Could not parse remote config response: Error: Unable to parse config') @@ -258,11 +250,7 @@ describe('RemoteConfigManager', () => { rc.poll(() => { expect(request).to.have.been.calledTwice - expect(request.secondCall).to.have.been.calledWith(payload2, { - url: rc.url, - method: 'POST', - path: '/v0.7/config' - }) + expect(request.secondCall).to.have.been.calledWith(payload2, expectedPayload) expect(rc.parseConfig).to.have.been.calledOnce expect(log.error).to.have.been.calledOnce expect(rc.state.client.state.has_error).to.be.false @@ -278,11 +266,7 @@ describe('RemoteConfigManager', () => { const payload = JSON.stringify(rc.state) rc.poll(() => { - expect(request).to.have.been.calledOnceWith(payload, { - url: rc.url, - method: 'POST', - path: '/v0.7/config' - }) + expect(request).to.have.been.calledOnceWith(payload, expectedPayload) expect(log.error).to.not.have.been.called expect(rc.parseConfig).to.not.have.been.called cb() @@ -299,11 +283,7 @@ describe('RemoteConfigManager', () => { expect(JSON.parse(payload).client.client_tracer.extra_services).to.deep.equal(extraServices) rc.poll(() => { - expect(request).to.have.been.calledOnceWith(payload, { - url: rc.url, - method: 'POST', - path: '/v0.7/config' - }) + expect(request).to.have.been.calledOnceWith(payload, expectedPayload) cb() }) })