From 93ac4e92c06d33682c564fdc27d46d3b51139e67 Mon Sep 17 00:00:00 2001 From: Mary Marchini Date: Sun, 2 Aug 2020 15:39:50 -0700 Subject: [PATCH] chore(jenkins): refactor jenkins-status using events --- app.js | 1 + lib/jenkins-events.js | 79 ++++++++++++++++ scripts/jenkins-status.js | 96 +++++--------------- test/integration/push-jenkins-update.test.js | 68 ++++++++------ 4 files changed, 146 insertions(+), 98 deletions(-) create mode 100644 lib/jenkins-events.js diff --git a/app.js b/app.js index b0b2308d..c5c496f4 100644 --- a/app.js +++ b/app.js @@ -30,6 +30,7 @@ app.use(bunyanMiddleware({ })) require('./lib/github-events')(app, events) +require('./lib/jenkins-events')(app, events) app.use(function logUnhandledErrors (err, req, res, next) { logger.error(err, 'Unhandled error while responding to incoming HTTP request') diff --git a/lib/jenkins-events.js b/lib/jenkins-events.js new file mode 100644 index 00000000..d46991c8 --- /dev/null +++ b/lib/jenkins-events.js @@ -0,0 +1,79 @@ +'use strict' + +const pushJenkinsUpdate = require('../lib/push-jenkins-update') + +const debug = require('debug')('jenkins-events') +const enabledRepos = ['citgm', 'http-parser', 'node', 'node-auto-test'] + +const listOfKnownJenkinsIps = process.env.JENKINS_WORKER_IPS ? process.env.JENKINS_WORKER_IPS.split(',') : [] + +function isKnownJenkinsIp (req) { + const ip = req.connection.remoteAddress.split(':').pop() + + if (listOfKnownJenkinsIps.length && !listOfKnownJenkinsIps.includes(ip)) { + req.log.warn({ ip }, 'Ignoring, not allowed to push Jenkins updates') + return false + } + + return true +} + +function isRelatedToPullRequest (gitRef) { + // refs/pull/12345/head vs refs/heads/v8.x-staging/head + return gitRef.includes('/pull/') +} + +module.exports = (app, events) => { + app.post('/:repo/jenkins/:event', async (req, res) => { + const isValid = pushJenkinsUpdate.validate(req.body) + const repo = req.params.repo + const event = req.params.event + const owner = req.body.owner || process.env.JENKINS_DEFAULT_GH_OWNER || 'nodejs' + + if (!isValid) { + return res.status(400).end('Invalid payload') + } + + if (!isRelatedToPullRequest(req.body.ref)) { + return res.status(400).end('Will only push builds related to pull requests') + } + + if (!enabledRepos.includes(repo)) { + return res.status(400).end('Invalid repository') + } + + if (!isKnownJenkinsIp(req)) { + return res.status(401).end('Invalid Jenkins IP') + } + + const data = { + ...req.body, + owner, + repo, + event + } + + try { + await app.emitJenkinsEvent(event, data, req.log) + res.status(200) + } catch (err) { + req.log.error(err, 'Error while emitting Jenkins event') + res.status(500) + } + + res.end() + }) + + app.emitJenkinsEvent = function emitJenkinsEvent (event, data, logger) { + const { identifier } = data + + // create unique logger which is easily traceable throughout the entire app + // by having e.g. "nodejs/nodejs.org/#1337" part of every subsequent log statement + data.logger = logger.child({ identifier, event }, true) + + data.logger.info('Emitting Jenkins event') + debug(data) + + return events.emit(`jenkins.${event}`, data) + } +} diff --git a/scripts/jenkins-status.js b/scripts/jenkins-status.js index 9b307fae..f9feaf0a 100644 --- a/scripts/jenkins-status.js +++ b/scripts/jenkins-status.js @@ -1,84 +1,36 @@ 'use strict' const pushJenkinsUpdate = require('../lib/push-jenkins-update') -const enabledRepos = ['citgm', 'http-parser', 'node'] -const jenkinsIpWhitelist = process.env.JENKINS_WORKER_IPS ? process.env.JENKINS_WORKER_IPS.split(',') : [] +function handleJenkinsStart (event) { + const { repo, owner } = event -function isJenkinsIpWhitelisted (req) { - const ip = req.connection.remoteAddress.split(':').pop() - - if (jenkinsIpWhitelist.length && !jenkinsIpWhitelist.includes(ip)) { - req.log.warn({ ip }, 'Ignoring, not allowed to push Jenkins updates') - return false - } - - return true -} - -function isRelatedToPullRequest (gitRef) { - // refs/pull/12345/head vs refs/heads/v8.x-staging/head - return gitRef.includes('/pull/') -} - -module.exports = function (app) { - app.post('/:repo/jenkins/start', (req, res) => { - const isValid = pushJenkinsUpdate.validate(req.body) - const repo = req.params.repo - - if (!isValid) { - return res.status(400).end('Invalid payload') - } - - if (!isRelatedToPullRequest(req.body.ref)) { - return res.status(400).end('Will only push builds related to pull requests') - } - - if (!enabledRepos.includes(repo)) { - return res.status(400).end('Invalid repository') - } - - if (!isJenkinsIpWhitelisted(req)) { - return res.status(401).end('Invalid Jenkins IP') + pushJenkinsUpdate.pushStarted({ + owner, + repo, + logger: event.logger + }, event, (err) => { + if (err) { + event.logger.error(err, 'Error while handling Jenkins start event') } - - pushJenkinsUpdate.pushStarted({ - owner: 'nodejs', - repo, - logger: req.log - }, req.body, (err) => { - const statusCode = err !== null ? 500 : 201 - res.status(statusCode).end() - }) }) +} - app.post('/:repo/jenkins/end', (req, res) => { - const isValid = pushJenkinsUpdate.validate(req.body) - const repo = req.params.repo - - if (!isValid) { - return res.status(400).end('Invalid payload') - } - - if (!isRelatedToPullRequest(req.body.ref)) { - return res.status(400).end('Will only push builds related to pull requests') - } - - if (!enabledRepos.includes(repo)) { - return res.status(400).end('Invalid repository') - } +function handleJenkinsStop (event) { + const { repo, owner } = event - if (!isJenkinsIpWhitelisted(req)) { - return res.status(401).end('Invalid Jenkins IP') + pushJenkinsUpdate.pushEnded({ + owner, + repo, + logger: event.logger + }, event, (err) => { + if (err) { + event.logger.error(err, 'Error while handling Jenkins end event') } - - pushJenkinsUpdate.pushEnded({ - owner: 'nodejs', - repo, - logger: req.log - }, req.body, (err) => { - const statusCode = err !== null ? 500 : 201 - res.status(statusCode).end() - }) }) } + +module.exports = function (_, event) { + event.on('jenkins.start', handleJenkinsStart) + event.on('jenkins.end', handleJenkinsStop) +} diff --git a/test/integration/push-jenkins-update.test.js b/test/integration/push-jenkins-update.test.js index ee3a66bf..d163d540 100644 --- a/test/integration/push-jenkins-update.test.js +++ b/test/integration/push-jenkins-update.test.js @@ -14,21 +14,25 @@ require('../../scripts/jenkins-status')(app, events) tap.test('Sends POST requests to https://api.github.com/repos/nodejs/node/statuses/', (t) => { const jenkinsPayload = readFixture('success-payload.json') - const prCommitsScope = setupGetCommitsMock('node') - const scope = nock('https://api.github.com') + setupGetCommitsMock('node') + .on('replied', (req, interceptor) => { + t.doesNotThrow(() => interceptor.scope.done()) + }) + nock('https://api.github.com') .filteringPath(ignoreQueryParams) .post('/repos/nodejs/node/statuses/8a5fec2a6bade91e544a30314d7cf21f8a200de1') .reply(201) + .on('replied', (req, interceptor) => { + t.doesNotThrow(() => interceptor.scope.done()) + }) - t.plan(1) + t.plan(3) supertest(app) .post('/node/jenkins/start') .send(jenkinsPayload) - .expect(201) + .expect(200) .end((err, res) => { - prCommitsScope.done() - scope.done() t.equal(err, null) }) }) @@ -36,21 +40,25 @@ tap.test('Sends POST requests to https://api.github.com/repos/nodejs/node/status tap.test('Allows repository name to be provided with URL parameter when pushing job started', (t) => { const jenkinsPayload = readFixture('pending-payload.json') - const prCommitsScope = setupGetCommitsMock('citgm') - const scope = nock('https://api.github.com') + setupGetCommitsMock('citgm') + .on('replied', (req, interceptor) => { + t.doesNotThrow(() => interceptor.scope.done()) + }) + nock('https://api.github.com') .filteringPath(ignoreQueryParams) .post('/repos/nodejs/citgm/statuses/8a5fec2a6bade91e544a30314d7cf21f8a200de1') .reply(201) + .on('replied', (req, interceptor) => { + t.doesNotThrow(() => interceptor.scope.done()) + }) - t.plan(1) + t.plan(3) supertest(app) .post('/citgm/jenkins/start') .send(jenkinsPayload) - .expect(201) + .expect(200) .end((err, res) => { - prCommitsScope.done() - scope.done() t.equal(err, null) }) }) @@ -58,21 +66,25 @@ tap.test('Allows repository name to be provided with URL parameter when pushing tap.test('Allows repository name to be provided with URL parameter when pushing job ended', (t) => { const jenkinsPayload = readFixture('success-payload.json') - const prCommitsScope = setupGetCommitsMock('citgm') - const scope = nock('https://api.github.com') + setupGetCommitsMock('citgm') + .on('replied', (req, interceptor) => { + t.doesNotThrow(() => interceptor.scope.done()) + }) + nock('https://api.github.com') .filteringPath(ignoreQueryParams) .post('/repos/nodejs/citgm/statuses/8a5fec2a6bade91e544a30314d7cf21f8a200de1') .reply(201) + .on('replied', (req, interceptor) => { + t.doesNotThrow(() => interceptor.scope.done()) + }) - t.plan(1) + t.plan(3) supertest(app) .post('/citgm/jenkins/end') .send(jenkinsPayload) - .expect(201) + .expect(200) .end((err, res) => { - prCommitsScope.done() - scope.done() t.equal(err, null) }) }) @@ -80,8 +92,11 @@ tap.test('Allows repository name to be provided with URL parameter when pushing tap.test('Forwards payload provided in incoming POST to GitHub status API', (t) => { const fixture = readFixture('success-payload.json') - const prCommitsScope = setupGetCommitsMock('node') - const scope = nock('https://api.github.com') + setupGetCommitsMock('node') + .on('replied', (req, interceptor) => { + t.doesNotThrow(() => interceptor.scope.done()) + }) + nock('https://api.github.com') .filteringPath(ignoreQueryParams) .post('/repos/nodejs/node/statuses/8a5fec2a6bade91e544a30314d7cf21f8a200de1', { state: 'success', @@ -90,16 +105,17 @@ tap.test('Forwards payload provided in incoming POST to GitHub status API', (t) target_url: 'https://ci.nodejs.org/job/node-test-commit-osx/3157/' }) .reply(201) + .on('replied', (req, interceptor) => { + t.doesNotThrow(() => interceptor.scope.done()) + }) - t.plan(1) + t.plan(3) supertest(app) .post('/node/jenkins/start') .send(fixture) - .expect(201) + .expect(200) .end((err, res) => { - prCommitsScope.done() - scope.done() t.equal(err, null) }) }) @@ -123,7 +139,7 @@ tap.test('Posts a CI comment in the related PR when Jenkins build is named node- supertest(app) .post('/node/jenkins/start') .send(fixture) - .expect(201) + .expect(200) .end((err, res) => { commentScope.done() t.equal(err, null) @@ -151,7 +167,7 @@ tap.test('Posts a CI comment in the related PR when Jenkins build is named node- supertest(app) .post('/node/jenkins/start') .send(fixture) - .expect(201) + .expect(200) .end((err, res) => { commentScope.done() t.equal(err, null)