Skip to content

Commit 85c96cd

Browse files
dschodscho
authored
Merge pull request #40 from dscho/safer-add-relnote
Make `/add relnote` safer to use
2 parents 1e53e13 + 1df9095 commit 85c96cd

9 files changed

Lines changed: 149 additions & 25 deletions

‎GitForWindowsHelper/azure-pipelines.js‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ const triggerAzurePipeline = async (context, token, organization, project, build
1010
'parameters': JSON.stringify(parameters),
1111
}
1212

13-
const httpsRequest = require('./https-request')
13+
const { httpsRequest } = require('./https-request')
1414
return await httpsRequest(
1515
context,
1616
'dev.azure.com',
@@ -49,7 +49,7 @@ const listReleases = async (context, token, organization, project) => {
4949
'Authorization': 'Basic ' + auth,
5050
}
5151

52-
const httpsRequest = require('./https-request')
52+
const { httpsRequest } = require('./https-request')
5353
return await httpsRequest(
5454
context,
5555
'vsrm.dev.azure.com',
@@ -67,7 +67,7 @@ const getRelease = async (context, token, organization, project, releaseId) => {
6767
'Authorization': 'Basic ' + auth,
6868
}
6969

70-
const httpsRequest = require('./https-request')
70+
const { httpsRequest } = require('./https-request')
7171
return await httpsRequest(
7272
context,
7373
'vsrm.dev.azure.com',
@@ -120,7 +120,7 @@ const createRelease = async (
120120
properties: { ReleaseCreationSource: "GitForWindowsHelper" },
121121
}
122122

123-
const httpsRequest = require('./https-request')
123+
const { httpsRequest } = require('./https-request')
124124
return await httpsRequest(
125125
context,
126126
'vsrm.dev.azure.com',

‎GitForWindowsHelper/component-updates.js‎

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@ const isMSYSPackage = package_name => {
4242
&& !package_name.startsWith('mingw-w64-')
4343
}
4444

45+
const packageNeedsBothMSYSAndMINGW = package_name => {
46+
return ['openssl', 'curl', 'gnutls', 'pcre2'].includes(package_name)
47+
}
48+
4549
const needsSeparateARM64Build = package_name => {
4650
if (package_name === 'git-extra') return true
4751
return package_name.startsWith('mingw-w64-') && ![
@@ -93,18 +97,48 @@ const guessReleaseNotes = async (context, issue) => {
9397
const url = await matchURL()
9498
if (!url) throw new Error(`Could not determine URL from issue ${issue.number}`)
9599

96-
package_name = prettyPackageName(package_name.replace(/^mingw-w64-/, ''))
100+
const prettyName = prettyPackageName(package_name.replace(/^mingw-w64-/, ''))
97101

98102
return {
99103
type: 'feature',
100-
message: `Comes with [${package_name} v${version}](${url}).`
104+
message: `Comes with [${prettyName} v${version}](${url}).`,
105+
package: package_name,
106+
version
107+
}
108+
}
109+
110+
const pacmanRepositoryBaseURL = 'https://wingit.blob.core.windows.net/'
111+
112+
const pacmanRepositoryURLs = (package_name, version) =>
113+
isMSYSPackage(package_name)
114+
? [
115+
`${pacmanRepositoryBaseURL}i686/${package_name}-${version}-1-i686.pkg.tar.xz`,
116+
`${pacmanRepositoryBaseURL}x86-64/${package_name}-${version}-1-x86_64.pkg.tar.xz`
117+
] : [
118+
`${pacmanRepositoryBaseURL}i686/${package_name.replace(/^mingw-w64/, '$&-i686')}-${version}-1-any.pkg.tar.xz`,
119+
`${pacmanRepositoryBaseURL}x86-64/${package_name.replace(/^mingw-w64/, '$&-x86_64')}-${version}-1-any.pkg.tar.xz`
120+
]
121+
122+
const getMissingDeployments = async (package_name, version) => {
123+
const urls = []
124+
const msysName = package_name.replace(/^mingw-w64-/, '')
125+
if (packageNeedsBothMSYSAndMINGW(msysName)) {
126+
urls.push(...pacmanRepositoryURLs(msysName, version))
127+
urls.push(...pacmanRepositoryURLs(`mingw-w64-${msysName}`, version))
128+
} else {
129+
urls.push(...pacmanRepositoryURLs(package_name, version))
101130
}
131+
const { doesURLReturn404 } = require('./https-request')
132+
const result = await Promise.all(urls.map(async url => doesURLReturn404(url)))
133+
return urls.filter((_, index) => result[index])
102134
}
103135

104136
module.exports = {
105137
guessComponentUpdateDetails,
106138
guessReleaseNotes,
107139
prettyPackageName,
108140
isMSYSPackage,
109-
needsSeparateARM64Build
141+
packageNeedsBothMSYSAndMINGW,
142+
needsSeparateARM64Build,
143+
getMissingDeployments
110144
}

‎GitForWindowsHelper/github-api-request-as-app.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ module.exports = async (context, requestMethod, requestPath, body) => {
2929

3030
const token = `${headerAndPayload}.${signature}`
3131

32-
const httpsRequest = require('./https-request')
32+
const { httpsRequest } = require('./https-request')
3333
return await httpsRequest(
3434
context,
3535
null,

‎GitForWindowsHelper/github-api-request.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
module.exports = async (context, token, method, requestPath, payload) => {
2-
const httpsRequest = require('./https-request')
2+
const { httpsRequest } = require('./https-request')
33
const headers = token ? { Authorization: `Bearer ${token}` } : null
44
const answer = await httpsRequest(context, null, method, requestPath, payload, headers)
55
if (answer.error) throw answer.error

‎GitForWindowsHelper/https-request.js‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
const gently = require('./gently')
22

3-
module.exports = async (context, hostname, method, requestPath, body, headers) => {
3+
const httpsRequest = async (context, hostname, method, requestPath, body, headers) => {
44
headers = {
55
'User-Agent': 'GitForWindowsHelper/0.0',
66
Accept: 'application/json',
@@ -60,3 +60,29 @@ module.exports = async (context, hostname, method, requestPath, body, headers) =
6060
}
6161
})
6262
}
63+
64+
const doesURLReturn404 = async url => {
65+
const match = url.match(/^https:\/\/([^/]+?)(:\d+)?(\/.*)?$/)
66+
if (!match) throw new Error(`Could not parse URL ${url}`)
67+
68+
const https = require('https')
69+
const options = {
70+
method: 'HEAD',
71+
host: match[1],
72+
port: Number.parseInt(match[2] || '443'),
73+
path: match[3] || '/'
74+
}
75+
return new Promise((resolve, reject) => {
76+
https.request(options, res => {
77+
if (res.error) reject(res.error)
78+
else if (res.statusCode === 404) resolve(true)
79+
else if (res.statusCode === 200) resolve(false)
80+
else reject(`Unexpected statusCode: ${res.statusCode}`)
81+
}).end()
82+
})
83+
}
84+
85+
module.exports = {
86+
httpsRequest,
87+
doesURLReturn404
88+
}

‎GitForWindowsHelper/search.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* https://docs.github.com/en/search-github/searching-on-github/searching-issues-and-pull-requests
44
*/
55
const searchIssues = async (context, searchTerms) => {
6-
const httpsRequest = require('./https-request');
6+
const { httpsRequest } = require('./https-request');
77
const answer = await httpsRequest(
88
context,
99
'api.github.com',

‎GitForWindowsHelper/slash-commands.js‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ module.exports = async (context, req) => {
5454

5555
await checkPermissions()
5656

57-
const { guessComponentUpdateDetails } = require('./component-updates')
57+
const { guessComponentUpdateDetails, packageNeedsBothMSYSAndMINGW } = require('./component-updates')
5858
const { package_name, version } = guessComponentUpdateDetails(req.body.issue.title, req.body.issue.body)
5959

6060
await thumbsUp()
@@ -96,7 +96,7 @@ module.exports = async (context, req) => {
9696
);
9797
({ html_url: commentURL, id: commentId } = await appendToIssueComment(context, await getToken(), owner, repo, commentId, `The${packageType ? ` ${packageType}` : ''} workflow run [was started](${answer.html_url})`))
9898
}
99-
if (!['openssl', 'curl', 'gnutls', 'pcre2'].includes(package_name)) {
99+
if (!packageNeedsBothMSYSAndMINGW(package_name)) {
100100
await openPR(package_name)
101101
} else {
102102
await openPR(package_name, 'MSYS')
@@ -462,10 +462,18 @@ module.exports = async (context, req) => {
462462

463463
await checkPermissions()
464464

465+
const { appendToIssueComment } = require('./issues')
465466
let [ , , , type, message ] = relNotesMatch
466467
if (!type) {
467-
const { guessReleaseNotes } = require('./component-updates');
468-
({ type, message } = await guessReleaseNotes(context, req.body.issue))
468+
const { guessReleaseNotes, getMissingDeployments } = require('./component-updates');
469+
let package_name, version
470+
({ type, message, package: package_name, version } = await guessReleaseNotes(context, req.body.issue))
471+
const missingDeployments = await getMissingDeployments(package_name, version)
472+
if (missingDeployments.length > 0) {
473+
const message = `The following deployment(s) are missing:\n\n* ${missingDeployments.join('\n* ')}`
474+
await appendToIssueComment(context, await getToken(), owner, repo, commentId, message)
475+
throw new Error(message)
476+
}
469477
}
470478

471479
await thumbsUp()
@@ -482,7 +490,6 @@ module.exports = async (context, req) => {
482490
message
483491
}
484492
)
485-
const { appendToIssueComment } = require('./issues')
486493
const answer2 = await appendToIssueComment(context, await getToken(), owner, repo, commentId, `The workflow run [was started](${answer.html_url})`)
487494
return `I edited the comment: ${answer2.html_url}`
488495
}

‎__tests__/component-updates.test.js‎

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
const {
22
guessComponentUpdateDetails,
3-
guessReleaseNotes
3+
guessReleaseNotes,
4+
getMissingDeployments
45
} = require('../GitForWindowsHelper/component-updates')
56

67
const bashTicketBody = `# [New bash version] Bash-5.2 patch 15: fix too-aggressive optimizing forks out of subshell commands
@@ -63,7 +64,9 @@ test('guessReleaseNotes()', async () => {
6364
body: bashTicketBody
6465
})).toEqual({
6566
type: 'feature',
66-
message: 'Comes with [Bash v5.2.15](https://git.savannah.gnu.org/cgit/bash.git/commit/?id=ec8113b9861375e4e17b3307372569d429dec814).'
67+
message: 'Comes with [Bash v5.2.15](https://git.savannah.gnu.org/cgit/bash.git/commit/?id=ec8113b9861375e4e17b3307372569d429dec814).',
68+
package: 'bash',
69+
version: '5.2.15'
6770
})
6871

6972
expect(await guessReleaseNotes(context, {
@@ -76,7 +79,9 @@ Added the security advisory.[GNUTLS-SA-2020-07-14](security-new.html#GNUTLS-SA-2
7679
http://www.gnutls.org/news.html#2023-02-10`
7780
})).toEqual({
7881
type: 'feature',
79-
message: 'Comes with [GNU TLS v3.8.0](https://lists.gnupg.org/pipermail/gnutls-help/2023-February/004816.html).'
82+
message: 'Comes with [GNU TLS v3.8.0](https://lists.gnupg.org/pipermail/gnutls-help/2023-February/004816.html).',
83+
package: 'gnutls',
84+
version: '3.8.0'
8085
})
8186

8287
expect(await guessReleaseNotes(context, {
@@ -85,7 +90,9 @@ http://www.gnutls.org/news.html#2023-02-10`
8590
body: `\nhttps://github.com/Perl/perl5/releases/tag/v5.36.1`
8691
})).toEqual({
8792
type: 'feature',
88-
message: 'Comes with [Perl v5.36.1](http://search.cpan.org/dist/perl-5.36.1/pod/perldelta.pod).'
93+
message: 'Comes with [Perl v5.36.1](http://search.cpan.org/dist/perl-5.36.1/pod/perldelta.pod).',
94+
package: 'perl',
95+
version: '5.36.1'
8996
})
9097

9198
expect(await guessReleaseNotes(context, {
@@ -94,7 +101,9 @@ http://www.gnutls.org/news.html#2023-02-10`
94101
body: `\nhttps://github.com/curl/curl/releases/tag/curl-8_1_1`
95102
})).toEqual({
96103
type: 'feature',
97-
message: 'Comes with [cURL v8.1.1](https://curl.se/changes.html#8_1_1).'
104+
message: 'Comes with [cURL v8.1.1](https://curl.se/changes.html#8_1_1).',
105+
package: 'curl',
106+
version: '8.1.1'
98107
})
99108

100109
expect(await guessReleaseNotes(context, {
@@ -103,7 +112,9 @@ http://www.gnutls.org/news.html#2023-02-10`
103112
body: `\nhttps://github.com/openssl/openssl/releases/tag/OpenSSL_1_1_1u`
104113
})).toEqual({
105114
type: 'feature',
106-
message: 'Comes with [OpenSSL v1.1.1u](https://www.openssl.org/news/openssl-1.1.1-notes.html).'
115+
message: 'Comes with [OpenSSL v1.1.1u](https://www.openssl.org/news/openssl-1.1.1-notes.html).',
116+
package: 'openssl',
117+
version: '1.1.1u'
107118
})
108119

109120
expect(await guessReleaseNotes(context, {
@@ -112,6 +123,18 @@ http://www.gnutls.org/news.html#2023-02-10`
112123
body: `\nhttps://github.com/openssl/openssl/releases/tag/openssl-3.1.1`
113124
})).toEqual({
114125
type: 'feature',
115-
message: 'Comes with [OpenSSL v3.1.1](https://www.openssl.org/news/openssl-3.1-notes.html).'
126+
message: 'Comes with [OpenSSL v3.1.1](https://www.openssl.org/news/openssl-3.1-notes.html).',
127+
package: 'openssl',
128+
version: '3.1.1'
129+
})
130+
})
131+
132+
test('getMissingDeployments()', async () => {
133+
const missingURL = 'https://wingit.blob.core.windows.net/x86-64/curl-8.1.2-1-x86_64.pkg.tar.xz'
134+
const mockDoesURLReturn404 = jest.fn(url => url === missingURL)
135+
jest.mock('../GitForWindowsHelper/https-request', () => {
136+
return { doesURLReturn404: mockDoesURLReturn404 }
116137
})
117-
})
138+
139+
expect(await getMissingDeployments('curl', '8.1.2')).toEqual([missingURL])
140+
})

‎__tests__/index.test.js‎

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,11 @@ function extend (a, ...list) {
168168
}
169169

170170
const testIssueComment = (comment, bodyExtra_, fn) => {
171+
let note = comment
172+
if (typeof comment === 'object') {
173+
note = comment.note
174+
comment = comment.comment
175+
}
171176
if (!fn) {
172177
fn = bodyExtra_
173178
bodyExtra_= undefined
@@ -201,7 +206,7 @@ const testIssueComment = (comment, bodyExtra_, fn) => {
201206
'x-github-event': 'issue_comment'
202207
})
203208

204-
test(`test ${comment}`, async () => {
209+
test(`test ${comment}${note ? ` (${note})` : ''}`, async () => {
205210
try {
206211
await fn(context)
207212
} catch (e) {
@@ -465,6 +470,12 @@ The workflow run [was started](dispatched-workflow-build-and-deploy.yml).`)
465470
expect(dispatchedWorkflows.map(e => e.payload.inputs.architecture)).toEqual(['aarch64'])
466471
})
467472

473+
const missingURL = 'https://wingit.blob.core.windows.net/x86-64/mingw-w64-x86_64-git-lfs-3.4.0-1-any.pkg.tar.xz'
474+
const mockDoesURLReturn404 = jest.fn(url => url === missingURL)
475+
jest.mock('../GitForWindowsHelper/https-request', () => {
476+
return { doesURLReturn404: mockDoesURLReturn404 }
477+
})
478+
468479
testIssueComment('/add release note', {
469480
issue: {
470481
number: 4281,
@@ -487,13 +498,36 @@ The workflow run [was started](dispatched-workflow-add-release-note.yml)`,
487498
})
488499
expect(mockGetInstallationAccessToken).toHaveBeenCalledTimes(1)
489500
expect(mockGitHubApiRequestAsApp).not.toHaveBeenCalled()
501+
expect(mockDoesURLReturn404).toHaveBeenCalledTimes(4)
490502
expect(dispatchedWorkflows).toHaveLength(1)
491503
expect(dispatchedWorkflows[0].payload.inputs).toEqual({
492504
message: 'Comes with [GNU TLS v3.8.0](https://lists.gnupg.org/pipermail/gnutls-help/2023-February/004816.html).',
493505
type: 'feature'
494506
})
495507
})
496508

509+
testIssueComment({ comment: '/add release note', note: 'missing deployment' }, {
510+
issue: {
511+
number: 4523,
512+
labels: [{ name: 'component-update' }],
513+
title: '[New git-lfs version] v3.4.0',
514+
body: `https://github.com/git-lfs/git-lfs/releases/tag/v3.4.0`
515+
}
516+
}, async (context) => {
517+
expect(await index(context, context.req)).toBeUndefined()
518+
expect(context.res).toEqual({
519+
body: `The following deployment(s) are missing:
520+
521+
* https://wingit.blob.core.windows.net/x86-64/mingw-w64-x86_64-git-lfs-3.4.0-1-any.pkg.tar.xz`,
522+
headers: undefined,
523+
status: 500
524+
})
525+
expect(mockGetInstallationAccessToken).toHaveBeenCalledTimes(1)
526+
expect(mockGitHubApiRequestAsApp).not.toHaveBeenCalled()
527+
expect(mockDoesURLReturn404).toHaveBeenCalledTimes(2)
528+
expect(dispatchedWorkflows).toHaveLength(0)
529+
})
530+
497531
test('a completed `tag-git` run triggers `git-artifacts` runs', async () => {
498532
const context = makeContext({
499533
action: 'completed',

0 commit comments

Comments
 (0)