From 5fe3982faf7e721656164084c579732097ff9dbe Mon Sep 17 00:00:00 2001 From: Gibson Fahnestock Date: Mon, 6 Nov 2017 14:39:39 +0000 Subject: [PATCH 1/5] pipeline: check inputs in post-build-status-update Make sure PR is a number, and the `POST_STATUS_TO_PR` is set. Refs: https://github.com/nodejs/build/pull/973#issuecomment-342130374 --- jenkins/pipelines/post-build-status-update.jenkinsfile | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/jenkins/pipelines/post-build-status-update.jenkinsfile b/jenkins/pipelines/post-build-status-update.jenkinsfile index 071be2568..01a0a566e 100644 --- a/jenkins/pipelines/post-build-status-update.jenkinsfile +++ b/jenkins/pipelines/post-build-status-update.jenkinsfile @@ -29,6 +29,14 @@ pipeline { } def sendBuildStatus(repo, identifier, status, url, commit, ref) { + + // Fail quietly if upstream job shouldn't be posting to GitHub. + if (!PR.isInteger() || POST_STATUS_TO_PR != 'true') { + println "Skipping status update (PR.isInteger(): ${PR.isInteger}, " + + "POST_STATUS_TO_PR: ${POST_STATUS_TO_PR}." + return + } + def path = "" def message = "" @@ -64,6 +72,7 @@ def sendBuildStatus(repo, identifier, status, url, commit, ref) { } def validateParams(params) { + // Fail loudly if upstream job is configured wrong. if (params.IDENTIFIER == '' || params.STATUS == '' || params.URL == '' || params.COMMIT == '' || params.REF == '') { error('All parameter fields are required.') From cb22bd2c4b1e7b743515178d73bfcfe446919a34 Mon Sep 17 00:00:00 2001 From: Gibson Fahnestock Date: Mon, 6 Nov 2017 15:15:02 +0000 Subject: [PATCH 2/5] squash: address comment --- .../post-build-status-update.jenkinsfile | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/jenkins/pipelines/post-build-status-update.jenkinsfile b/jenkins/pipelines/post-build-status-update.jenkinsfile index 01a0a566e..a8afdb03f 100644 --- a/jenkins/pipelines/post-build-status-update.jenkinsfile +++ b/jenkins/pipelines/post-build-status-update.jenkinsfile @@ -10,12 +10,12 @@ pipeline { agent { label 'jenkins-workspace' } parameters { - string(defaultValue: 'node', description: 'GitHub repository', name: 'REPO') - string(defaultValue: '', description: 'test/aix, linter, etc.', name: 'IDENTIFIER') - string(defaultValue: '', description: 'pending, success, unstable, failure', name: 'STATUS') - string(defaultValue: '', description: 'URL for upstream Jenkins job', name: 'URL') - string(defaultValue: '', description: 'Current commit being tested in upstream Jenkins job', name: 'COMMIT') - string(defaultValue: '', description: 'Current branch being tested in upstream Jenkins job', name: 'REF') + string(name: 'REPO', defaultValue: 'node', description: 'GitHub repository') + string(name: 'IDENTIFIER', defaultValue: '', description: 'test/aix, linter, etc.') + string(name: 'STATUS, 'defaultValue: '', description: 'pending, success, unstable, failure') + string(name: 'URL, 'defaultValue: '', description: 'URL for upstream Jenkins job') + string(name: 'COMMIT, 'defaultValue: '', description: 'Current commit being tested in upstream Jenkins job') + string(name: 'REF, 'defaultValue: '', description: 'Current branch being tested in upstream Jenkins job') } stages { @@ -31,9 +31,9 @@ pipeline { def sendBuildStatus(repo, identifier, status, url, commit, ref) { // Fail quietly if upstream job shouldn't be posting to GitHub. - if (!PR.isInteger() || POST_STATUS_TO_PR != 'true') { - println "Skipping status update (PR.isInteger(): ${PR.isInteger}, " + - "POST_STATUS_TO_PR: ${POST_STATUS_TO_PR}." + if (ref.contains('refs/pull/') || POST_STATUS_TO_PR != 'true') { + println "Skipping status update (ref: ${ref}, " + + "POST_STATUS_TO_PR: ${POST_STATUS_TO_PR})." return } From 618c62fbc9af5743c6b0457405861eab4b486aca Mon Sep 17 00:00:00 2001 From: Gibson Fahnestock Date: Mon, 6 Nov 2017 19:13:46 +0000 Subject: [PATCH 3/5] squash! suggestion --- .../post-build-status-update.jenkinsfile | 39 +++++++++---------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/jenkins/pipelines/post-build-status-update.jenkinsfile b/jenkins/pipelines/post-build-status-update.jenkinsfile index a8afdb03f..4afe4b772 100644 --- a/jenkins/pipelines/post-build-status-update.jenkinsfile +++ b/jenkins/pipelines/post-build-status-update.jenkinsfile @@ -16,30 +16,37 @@ pipeline { string(name: 'URL, 'defaultValue: '', description: 'URL for upstream Jenkins job') string(name: 'COMMIT, 'defaultValue: '', description: 'Current commit being tested in upstream Jenkins job') string(name: 'REF, 'defaultValue: '', description: 'Current branch being tested in upstream Jenkins job') + booleanParam(name: 'POST_STATUS_TO_PR', defaultValue: 'false', description: 'Whether the PR should be updated.') } stages { stage('Send status report') { steps { - validateParams(params) - sendBuildStatus(params.REPO, params.IDENTIFIER, params.STATUS, params.URL, params.COMMIT, params.REF) + sendBuildStatus(params) } } } } -def sendBuildStatus(repo, identifier, status, url, commit, ref) { +def sendBuildStatus(params) { + // Fail loudly if upstream job is configured wrong. + for (def i in params) { + if (i.value == '') { + error('All parameter fields are required, ${i.key} was blank.') + } + } // Fail quietly if upstream job shouldn't be posting to GitHub. - if (ref.contains('refs/pull/') || POST_STATUS_TO_PR != 'true') { - println "Skipping status update (ref: ${ref}, " + - "POST_STATUS_TO_PR: ${POST_STATUS_TO_PR})." + if (params.REF.contains('refs/pull/') || params.POST_STATUS_TO_PR != 'true') { + println "Skipping status update (REF: ${params.REF}, " + + "POST_STATUS_TO_PR: ${params.POST_STATUS_TO_PR})." return } def path = "" def message = "" + def status = params.STATUS if (status == "pending") { path = "start" message = "running tests" @@ -56,25 +63,17 @@ def sendBuildStatus(repo, identifier, status, url, commit, ref) { } def buildPayload = JsonOutput.toJson([ - 'identifier': identifier, - 'status': status, - 'url': url, - 'commit': commit, - 'ref': ref, + 'identifier': params.IDENTIFIER, + 'status': params.STATUS, + 'url': params.URL, + 'commit': params.COMMIT, + 'ref': params.REF, 'message': message ]) def script = "curl -s -o /dev/null --connect-timeout 5 -X POST " + "-H 'Content-Type: application/json' -d '${buildPayload}' " + - "http://github-bot.nodejs.org:3333/${repo}/jenkins/${path}" + "http://github-bot.nodejs.org:3333/${params.REPO}/jenkins/${path}" sh(returnStdout: true, script: script) } - -def validateParams(params) { - // Fail loudly if upstream job is configured wrong. - if (params.IDENTIFIER == '' || params.STATUS == '' || params.URL == '' || - params.COMMIT == '' || params.REF == '') { - error('All parameter fields are required.') - } -} From ce701632eaba23fe2c5e7df1e618eaef638300be Mon Sep 17 00:00:00 2001 From: Gibson Fahnestock Date: Mon, 6 Nov 2017 19:25:47 +0000 Subject: [PATCH 4/5] squash! try using .any() instead of the for loop --- jenkins/pipelines/post-build-status-update.jenkinsfile | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/jenkins/pipelines/post-build-status-update.jenkinsfile b/jenkins/pipelines/post-build-status-update.jenkinsfile index 4afe4b772..503df5f2c 100644 --- a/jenkins/pipelines/post-build-status-update.jenkinsfile +++ b/jenkins/pipelines/post-build-status-update.jenkinsfile @@ -30,10 +30,8 @@ pipeline { def sendBuildStatus(params) { // Fail loudly if upstream job is configured wrong. - for (def i in params) { - if (i.value == '') { - error('All parameter fields are required, ${i.key} was blank.') - } + if (params.any{ it.value == '' }) { + error('All parameter fields are required.') } // Fail quietly if upstream job shouldn't be posting to GitHub. From 18d6bc0534c5a0ad5a358f07fe6768d257810fb7 Mon Sep 17 00:00:00 2001 From: Gibson Fahnestock Date: Mon, 6 Nov 2017 21:40:56 +0000 Subject: [PATCH 5/5] squash! d'oh! --- jenkins/pipelines/post-build-status-update.jenkinsfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jenkins/pipelines/post-build-status-update.jenkinsfile b/jenkins/pipelines/post-build-status-update.jenkinsfile index 503df5f2c..25f5ccf71 100644 --- a/jenkins/pipelines/post-build-status-update.jenkinsfile +++ b/jenkins/pipelines/post-build-status-update.jenkinsfile @@ -35,7 +35,7 @@ def sendBuildStatus(params) { } // Fail quietly if upstream job shouldn't be posting to GitHub. - if (params.REF.contains('refs/pull/') || params.POST_STATUS_TO_PR != 'true') { + if (!params.REF.contains('refs/pull/') || params.POST_STATUS_TO_PR != 'true') { println "Skipping status update (REF: ${params.REF}, " + "POST_STATUS_TO_PR: ${params.POST_STATUS_TO_PR})." return