Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor [JenkinsBuild] #3302

Merged
merged 10 commits into from
Apr 19, 2019
29 changes: 29 additions & 0 deletions services/jenkins/jenkins-base.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
'use strict'

const { BaseJsonService } = require('..')
const serverSecrets = require('../../lib/server-secrets')

module.exports = class JenkinsBase extends BaseJsonService {
async fetch({
url,
schema,
qs,
errorMessages = { 404: 'instance or job not found' },
}) {
const options = { qs, strictSSL: false }
calebcartwright marked this conversation as resolved.
Show resolved Hide resolved

if (serverSecrets.jenkins_user) {
options.auth = {
user: serverSecrets.jenkins_user,
pass: serverSecrets.jenkins_pass,
}
}

return this._requestJson({
url,
options,
schema,
errorMessages,
})
}
}
16 changes: 16 additions & 0 deletions services/jenkins/jenkins-build-redirect.service.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
'use strict'

const { redirector } = require('..')

module.exports = [
redirector({
category: 'build',
route: {
base: 'jenkins-ci',
pattern: 's/:protocol(http|https)/:host+/:job+',
},
transformPath: ({ protocol, host, job }) =>
`/jenkins/s/${protocol}/${host}/${job}`,
dateAdded: new Date('2019-04-14'),
}),
]
19 changes: 19 additions & 0 deletions services/jenkins/jenkins-build-redirect.tester.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
'use strict'

const { ServiceTester } = require('../tester')

const t = (module.exports = new ServiceTester({
id: 'JenkinsBuildRedirect',
title: 'JenkinsBuildRedirect',
pathPrefix: '/jenkins-ci/s',
}))

t.create('jenkins ci')
.get('/https/updates.jenkins-ci.org/job/foo.svg', {
followRedirect: false,
})
.expectStatus(301)
.expectHeader(
'Location',
'/jenkins/s/https/updates.jenkins-ci.org/job/foo.svg'
)
152 changes: 76 additions & 76 deletions services/jenkins/jenkins-build.service.js
Original file line number Diff line number Diff line change
@@ -1,107 +1,107 @@
'use strict'

const LegacyService = require('../legacy-service')
const { makeBadgeData: getBadgeData } = require('../../lib/badge-data')
const serverSecrets = require('../../lib/server-secrets')
const Joi = require('joi')
const { renderBuildStatusBadge } = require('../build-status')
const JenkinsBase = require('./jenkins-base')

// This legacy service should be rewritten to use e.g. BaseJsonService.
//
// Tips for rewriting:
// https://github.com/badges/shields/blob/master/doc/rewriting-services.md
//
// Do not base new services on this code.
module.exports = class JenkinsBuild extends LegacyService {
// https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/model/BallColor.java#L56
const schema = Joi.object({
color: Joi.allow(
'red',
'red_anime',
'yellow',
'yellow_anime',
'blue',
'blue_anime',
// green included for backwards compatibility
'green',
'green_anime',
'grey',
'grey_anime',
'disabled',
'disabled_anime',
'aborted',
'aborted_anime',
'notbuilt',
'notbuilt_anime'
calebcartwright marked this conversation as resolved.
Show resolved Hide resolved
).required(),
}).required()

const colorStatusMap = {
red: 'failing',
red_anime: 'building',
yellow: 'unstable',
yellow_anime: 'building',
blue: 'passing',
blue_anime: 'building',
green: 'passing',
green_anime: 'building',
grey: 'not built',
grey_anime: 'building',
disabled: 'not built',
disabled_anime: 'building',
aborted: 'not built',
aborted_anime: 'building',
notbuilt: 'not built',
notbuilt_anime: 'building',
}

module.exports = class JenkinsBuild extends JenkinsBase {
static get category() {
return 'build'
}

static get defaultBadgeData() {
return {
label: 'build',
}
}

static get route() {
return {
base: 'jenkins/s',
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have any PR #'s handy, but we seem to be trending away from single letter aliases to more explicit route param names.

I'd like to do the same thing here (build or status instead of s). Any thoughts/objections?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy if you want to change it, but we'll want to maintain an alias/redirect for /s for existing users

We talked about formalising some of this stuff in docs in #3144 (comment) and then.. haven't got round to doing it yet. There's several things in that discussion that I want to get done at some point but haven't found the time. Having this stuff documented would make it easier when we're thinking

wait, what is our convention for a "build status" URL?

🙂

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll leave it in place for now. I plan on using some of the common bits in here to refactor the Jenkins test service so I'll consider the alias redirecting in that one

pattern: ':scheme(http|https)?/:host/:job*',
pattern: ':protocol(http|https)/:host/:job+',
}
}

static get examples() {
return [
{
title: 'Jenkins',
pattern: ':scheme/:host/:job',
namedParams: {
scheme: 'https',
protocol: 'https',
host: 'jenkins.qa.ubuntu.com',
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am also intentionally not including the new disableStrictSSL query param here as I do not believe this is something we want to advertise; it's a work around for a handful of exceptional circumstances only.

job:
'view/Precise/view/All%20Precise/job/precise-desktop-amd64_default',
},
staticPreview: {
label: 'build',
message: 'passing',
color: 'brightgreen',
},
staticPreview: renderBuildStatusBadge({ status: 'passing' }),
},
]
}

static registerLegacyRouteHandler({ camp, cache }) {
camp.route(
/^\/jenkins(?:-ci)?\/s\/(http(?:s)?)\/([^/]+)\/(.+)\.(svg|png|gif|jpg|json)$/,
cache((data, match, sendBadge, request) => {
const scheme = match[1] // http(s)
const host = match[2] // example.org:8080
const job = match[3] // folder/job
const format = match[4]
const options = {
json: true,
uri: `${scheme}://${host}/job/${job}/api/json?tree=color`,
}
if (job.indexOf('/') > -1) {
options.uri = `${scheme}://${host}/${job}/api/json?tree=color`
}
static render({ status }) {
if (status === 'unstable') {
return {
message: status,
color: 'yellow',
}
}

if (serverSecrets.jenkins_user) {
options.auth = {
user: serverSecrets.jenkins_user,
pass: serverSecrets.jenkins_pass,
}
}
return renderBuildStatusBadge({ status })
}

const badgeData = getBadgeData('build', data)
request(options, (err, res, json) => {
if (err !== null) {
badgeData.text[1] = 'inaccessible'
sendBadge(format, badgeData)
return
}
transform({ json }) {
return { status: colorStatusMap[json.color] }
}

try {
if (json.color === 'blue' || json.color === 'green') {
badgeData.colorscheme = 'brightgreen'
badgeData.text[1] = 'passing'
} else if (json.color === 'red') {
badgeData.colorscheme = 'red'
badgeData.text[1] = 'failing'
} else if (json.color === 'yellow') {
badgeData.colorscheme = 'yellow'
badgeData.text[1] = 'unstable'
} else if (
json.color === 'grey' ||
json.color === 'disabled' ||
json.color === 'aborted' ||
json.color === 'notbuilt'
) {
badgeData.colorscheme = 'lightgrey'
badgeData.text[1] = 'not built'
} else {
badgeData.colorscheme = 'lightgrey'
badgeData.text[1] = 'building'
}
sendBadge(format, badgeData)
} catch (e) {
badgeData.text[1] = 'invalid'
sendBadge(format, badgeData)
}
})
})
)
async handle({ protocol, host, job }) {
const jobPrefix = job.indexOf('/') > -1 ? '' : 'job/'
const json = await this.fetch({
url: `${protocol}://${host}/${jobPrefix}${job}/api/json`,
schema,
qs: { tree: 'color' },
})
const { status } = this.transform({ json })
return this.constructor.render({ status })
}
}
57 changes: 57 additions & 0 deletions services/jenkins/jenkins-build.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
'use strict'

const { test, forCases, given } = require('sazerac')
const { renderBuildStatusBadge } = require('../build-status')
const JenkinsBuild = require('./jenkins-build.service')

describe('JenkinsBuild', function() {
test(JenkinsBuild.prototype.transform, () => {
forCases([
given({ json: { color: 'red_anime' } }),
given({ json: { color: 'yellow_anime' } }),
given({ json: { color: 'blue_anime' } }),
given({ json: { color: 'green_anime' } }),
given({ json: { color: 'grey_anime' } }),
given({ json: { color: 'disabled_anime' } }),
given({ json: { color: 'aborted_anime' } }),
given({ json: { color: 'notbuilt_anime' } }),
]).expect({
status: 'building',
})
forCases([
given({ json: { color: 'grey' } }),
given({ json: { color: 'disabled' } }),
given({ json: { color: 'aborted' } }),
given({ json: { color: 'notbuilt' } }),
]).expect({
status: 'not built',
})
forCases([
given({ json: { color: 'blue' } }),
given({ json: { color: 'green' } }),
]).expect({
status: 'passing',
})
given({ json: { color: 'red' } }).expect({ status: 'failing' })
given({ json: { color: 'yellow' } }).expect({ status: 'unstable' })
})

test(JenkinsBuild.render, () => {
given({ status: 'unstable' }).expect({
message: 'unstable',
color: 'yellow',
})
given({ status: 'passing' }).expect(
renderBuildStatusBadge({ status: 'passing' })
)
given({ status: 'failing' }).expect(
renderBuildStatusBadge({ status: 'failing' })
)
given({ status: 'building' }).expect(
renderBuildStatusBadge({ status: 'building' })
)
given({ status: 'not built' }).expect(
renderBuildStatusBadge({ status: 'not built' })
)
})
})
53 changes: 53 additions & 0 deletions services/jenkins/jenkins-build.tester.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
'use strict'

const Joi = require('joi')
const sinon = require('sinon')
const serverSecrets = require('../../lib/server-secrets')
const { isBuildStatus } = require('../build-status')
const t = (module.exports = require('../tester').createServiceTester())

const isJenkinsBuildStatus = Joi.alternatives(
isBuildStatus,
Joi.string().allow('unstable')
)

t.create('build job not found')
.get('/https/updates.jenkins-ci.org/job/does-not-exist.json')
.expectBadge({ label: 'build', message: 'instance or job not found' })

t.create('build found (view)')
.get(
'/https/jenkins.qa.ubuntu.com/view/Precise/view/All%20Precise/job/precise-desktop-amd64_default.json'
)
.expectBadge({ label: 'build', message: isJenkinsBuildStatus })

t.create('build found (job)')
.get('/https/jenkins.ubuntu.com/server/curtin-vmtest-daily-x.json')
.expectBadge({ label: 'build', message: isJenkinsBuildStatus })

const user = 'admin'
const pass = 'password'

function mockCreds() {
serverSecrets['jenkins_user'] = undefined
serverSecrets['jenkins_pass'] = undefined
sinon.stub(serverSecrets, 'jenkins_user').value(user)
sinon.stub(serverSecrets, 'jenkins_pass').value(pass)
}

t.create('with mock credentials')
.before(mockCreds)
.get('/https/jenkins.ubuntu.com/server/job/curtin-vmtest-daily-x.json')
.intercept(nock =>
nock('https://jenkins.ubuntu.com/server/job/curtin-vmtest-daily-x')
.get(`/api/json?tree=color`)
// This ensures that the expected credentials from serverSecrets are actually being sent with the HTTP request.
// Without this the request wouldn't match and the test would fail.
.basicAuth({
user,
pass,
})
.reply(200, { color: 'blue' })
)
.finally(sinon.restore)
.expectBadge({ label: 'build', message: 'passing' })