chore(jenkins): refactor jenkins-status using events

Author: mmarchini

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')

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)
+  }
+}

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)
+}

test/integration/push-jenkins-update.test.js

@@ -14,74 +14,89 @@ require('../../scripts/jenkins-status')(app, events)
 tap.test('Sends POST requests to https://api.github.com/repos/nodejs/node/statuses/<SHA>', (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)
     })
 })
 
 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)
     })
 })
 
 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)
     })
 })
 
 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)