-
Notifications
You must be signed in to change notification settings - Fork 114
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
ncu-ci: command to start CI for PRs #445
Conversation
Codecov Report
@@ Coverage Diff @@
## master #445 +/- ##
=======================================
Coverage 76.94% 76.94%
=======================================
Files 21 21
Lines 1501 1501
=======================================
Hits 1155 1155
Misses 346 346 Continue to review full report at Codecov.
|
Done. I also made some changes so that if we fail to start the CI for any reason, the process exits with status code != 0 :) |
I think this deserves a @nodejs/build ping to double-check if we're not doing anything we shouldn't in the Jenkins calls (for example, using a user token instead of the token defined in the Job configuration). |
Added some tests. Would be great if we could include this in the next release, as it is required for the Request CI Queue on nodejs/node :) |
I might not have all the context i might need as i'm not a member of @nodejs/build, but if no one else comments then i think we may be fine to go ahead and merge? |
That's what I'm thinking, yes. If something with this approach is not right/good, we can always improve on a future version (or remove if needed) |
I'm pretty sure @nodejs/jenkins-admins is a subset of @nodejs/build, but uh, pinging them anyway.... |
If folks want to help review but don't want/have time to dabble into ncu code, the main question is: is this the right/best way to start a Jenkins job via API? Below is a simplified version of the flow implemented in the PR: // this code probably doesn't work, but it shows
// which calls the PR makes to start a job via
// Jenkins API
// Use the token generated on Jenkins interface to
// authenticate.
const username = '***';
const personalJenkinsToken = '***';
const auth = Buffer
.from(`${username}:${personalJenkinsToken}`)
.toString('base64')
const headers = { Authorization: `Basic ${auth}` };
// Get breadcrumb for CSRF validation
const { crumb } = await (await fetch('https://ci.nodejs.org/crumbIssuer/api/json', { headers }));
// `/job/node-pull-request/build` expects a multipart
// request, so use FormData to generate the payload
const payload = new FormData();
payload.append('json', JSON.stringify({
parameter: [
{ name: 'CERTIFY_SAFE', value: 'on' },
{ name: 'TARGET_GITHUB_ORG', value: 'nodejs' },
{ name: 'TARGET_REPO_NAME', value: 'node' },
{ name: 'PR_ID', value: 123456 },
{ name: 'REBASE_ONTO', value: '<pr base branch>' },
{ name: 'DESCRIPTION_SETTER_DESCRIPTION', value: '' }
]
}));
// Send POST with breadcrump + auth on headers, and
// FormData payload on body
await fetch('https://ci.nodejs.org/job/node-pull-request/build', {
method: 'POST',
headers: {
'Jenkins-Crumb': crumb,
...headers
},
body: payload
}); |
Going to merge this just so we can move forward with it :) I think if there are any changes to the flow start we can update as needed! |
Can we get a new release with this included? :) @nodejs/node-core-utils |
found a bug which doesn't prevent from using the tool, but if the user doens't have permission to start CI it will show as if everything worked fine. I already have a fix locally and will send a PR soon. |
No description provided.