Skip to content

Commit 16feaad

Browse files
CopilotdecyjphrCopilot
authored
fix: migrate all octokit API calls to .rest namespace for probot v14 compatibility (#949)
* fix: override NODE_ENV=development in functional test and add retry logic Co-authored-by: decyjphr <57544838+decyjphr@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/safe-settings/sessions/97b10121-f26e-44c0-86e6-3ede047fe176 * fix: add --fail to curl and || true to docker logs in functional test Co-authored-by: decyjphr <57544838+decyjphr@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/safe-settings/sessions/97b10121-f26e-44c0-86e6-3ede047fe176 * fix: migrate all octokit API calls to .rest namespace for probot v14 compatibility and revert Dockerfile ENV HOST Co-authored-by: decyjphr <57544838+decyjphr@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/safe-settings/sessions/179c9d77-8ca0-4098-9017-8a255df170f9 * Update lib/plugins/repository.js Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix: address review feedback - NopCommand repo arg, milestones mock, teams test error propagation Co-authored-by: decyjphr <57544838+decyjphr@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/safe-settings/sessions/d9420b15-2cca-40a3-a30b-869df73487f1 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: decyjphr <57544838+decyjphr@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 34ce7a1 commit 16feaad

27 files changed

Lines changed: 310 additions & 266 deletions

‎.github/workflows/create-pre-release.yml‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,10 @@ jobs:
7171
- name: Run Functional Tests
7272
id: functionaltest
7373
run: |
74-
docker run --env APP_ID=${{ secrets.APP_ID }} --env PRIVATE_KEY=${{ secrets.PRIVATE_KEY }} --env WEBHOOK_SECRET=${{ secrets.WEBHOOK_SECRET }} -d -p 3000:3000 ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:main-enterprise
74+
CONTAINER_ID=$(docker run --env APP_ID=${{ secrets.APP_ID }} --env PRIVATE_KEY=${{ secrets.PRIVATE_KEY }} --env WEBHOOK_SECRET=${{ secrets.WEBHOOK_SECRET }} --env NODE_ENV=development -d -p 3000:3000 ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:main-enterprise)
7575
sleep 10
76-
curl http://localhost:3000
76+
docker logs $CONTAINER_ID || true
77+
curl --fail --retry 5 --retry-delay 3 --retry-connrefused http://localhost:3000
7778
- run: echo "${{ github.ref }}"
7879
- name: Tag a final release
7980
id: prerelease

‎Dockerfile‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
FROM node:22-alpine
22
WORKDIR /opt/safe-settings
33
ENV NODE_ENV production
4-
ENV HOST=0.0.0.0
54
## Set the Labels
65
LABEL version="1.0" \
76
description="Probot app which is a modified version of Settings Probot GitHub App" \

‎index.js‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
199199
async function createCheckRun (context, pull_request, head_sha, head_branch) {
200200
const { payload } = context
201201
// robot.log.debug(`Check suite was requested! for ${context.repo()} ${pull_request.number} ${head_sha} ${head_branch}`)
202-
const res = await context.octokit.checks.create({
202+
const res = await context.octokit.rest.checks.create({
203203
owner: payload.repository.owner.login,
204204
repo: payload.repository.name,
205205
name: 'Safe-setting validator',
@@ -211,13 +211,13 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
211211
async function info () {
212212
const github = await robot.auth()
213213
const installations = await github.paginate(
214-
github.apps.listInstallations.endpoint.merge({ per_page: 100 })
214+
github.rest.apps.listInstallations.endpoint.merge({ per_page: 100 })
215215
)
216216
robot.log.debug(`installations: ${JSON.stringify(installations)}`)
217217
if (installations.length > 0) {
218218
const installation = installations[0]
219219
const github = await robot.auth(installation.id)
220-
const app = await github.apps.getAuthenticated()
220+
const app = await github.rest.apps.getAuthenticated()
221221
appSlug = app.data.slug
222222
robot.log.debug(`Validated the app is configured properly = \n${JSON.stringify(app.data, null, 2)}`)
223223
}
@@ -228,7 +228,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
228228
const github = await robot.auth()
229229

230230
const installations = await github.paginate(
231-
github.apps.listInstallations.endpoint.merge({ per_page: 100 })
231+
github.rest.apps.listInstallations.endpoint.merge({ per_page: 100 })
232232
)
233233

234234
if (installations.length > 0) {
@@ -577,11 +577,11 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
577577
output: { title: 'Starting NOP', summary: 'initiating...' }
578578
}
579579
robot.log.debug(`Updating check run ${JSON.stringify(params)}`)
580-
await context.octokit.checks.update(params)
580+
await context.octokit.rest.checks.update(params)
581581

582582
params = Object.assign(context.repo(), { pull_number: pull_request.number })
583583

584-
const changes = await context.octokit.pulls.listFiles(params)
584+
const changes = await context.octokit.rest.pulls.listFiles(params)
585585
const files = changes.data.map(f => { return f.filename })
586586

587587
const settingsModified = files.includes(Settings.FILE_PATH)
@@ -609,7 +609,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
609609
output: { title: 'No Safe-settings changes detected', summary: 'No changes detected' }
610610
}
611611
robot.log.debug(`Completing check run ${JSON.stringify(params)}`)
612-
await context.octokit.checks.update(params)
612+
await context.octokit.rest.checks.update(params)
613613
})
614614

615615
robot.on('repository.created', async context => {

‎lib/configManager.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ module.exports = class ConfigManager {
1919
try {
2020
const repo = { owner: this.context.repo().owner, repo: env.ADMIN_REPO }
2121
const params = Object.assign(repo, { path: filePath, ref: this.ref })
22-
const response = await this.context.octokit.repos.getContent(params).catch(e => {
22+
const response = await this.context.octokit.rest.repos.getContent(params).catch(e => {
2323
this.log.error(`Error getting settings ${e}`)
2424
})
2525

‎lib/plugins/archive.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ module.exports = class Archive {
1111

1212
async getRepo () {
1313
try {
14-
const { data } = await this.github.repos.get({
14+
const { data } = await this.github.rest.repos.get({
1515
owner: this.repo.owner,
1616
repo: this.repo.repo
1717
})
@@ -32,13 +32,13 @@ module.exports = class Archive {
3232
return new NopCommand(
3333
this.constructor.name,
3434
this.repo,
35-
this.github.repos.update.endpoint(this.settings),
35+
this.github.rest.repos.update.endpoint(this.settings),
3636
change,
3737
'INFO'
3838
)
3939
}
4040

41-
const { data } = await this.github.repos.update({
41+
const { data } = await this.github.rest.repos.update({
4242
owner: this.repo.owner,
4343
repo: this.repo.repo,
4444
archived

‎lib/plugins/autolinks.js‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ module.exports = class Autolinks extends Diffable {
88
// }
99

1010
async find () {
11-
const { data } = await this.github.repos.listAutolinks(this.repo)
11+
const { data } = await this.github.rest.repos.listAutolinks(this.repo)
1212
return data
1313
}
1414

@@ -43,13 +43,13 @@ module.exports = class Autolinks extends Diffable {
4343
return new NopCommand(
4444
this.constructor.name,
4545
this.repo,
46-
this.github.repos.createAutolink.endpoint(attrs),
46+
this.github.rest.repos.createAutolink.endpoint(attrs),
4747
'Add autolink'
4848
)
4949
}
5050

5151
try {
52-
return this.github.repos.createAutolink(attrs)
52+
return this.github.rest.repos.createAutolink(attrs)
5353
} catch (e) {
5454
if (e?.response?.data?.errors?.[0]?.code === 'already_exists') {
5555
this.log.debug(`Did not update ${key_prefix}, as it already exists`)
@@ -68,10 +68,10 @@ module.exports = class Autolinks extends Diffable {
6868
return new NopCommand(
6969
this.constructor.name,
7070
this.repo,
71-
this.github.repos.deleteAutolink.endpoint(attrs),
71+
this.github.rest.repos.deleteAutolink.endpoint(attrs),
7272
'Remove autolink'
7373
)
7474
}
75-
return this.github.repos.deleteAutolink(attrs)
75+
return this.github.rest.repos.deleteAutolink(attrs)
7676
}
7777
}

‎lib/plugins/branches.js‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ module.exports = class Branches extends ErrorStash {
2323

2424
sync () {
2525
const resArray = []
26-
return this.github.repos.get(this.repo).then((currentRepo) => {
26+
return this.github.rest.repos.get(this.repo).then((currentRepo) => {
2727
return Promise.all(
2828
this.branches
2929
.filter(branch => branch.protection !== undefined)
@@ -39,12 +39,12 @@ module.exports = class Branches extends ErrorStash {
3939
const params = Object.assign({}, p)
4040
if (this.nop) {
4141
resArray.push(
42-
new NopCommand(this.constructor.name, this.repo, this.github.repos.deleteBranchProtection.endpoint(params), 'Delete Branch Protection')
42+
new NopCommand(this.constructor.name, this.repo, this.github.rest.repos.deleteBranchProtection.endpoint(params), 'Delete Branch Protection')
4343
)
4444
return Promise.resolve(resArray)
4545
}
4646

47-
return this.github.repos.deleteBranchProtection(params).catch(e => { return [] })
47+
return this.github.rest.repos.deleteBranchProtection(params).catch(e => { return [] })
4848
} else {
4949
// Branch protection is not empty
5050
let p = Object.assign(this.repo, { branch: branch.name })
@@ -54,7 +54,7 @@ module.exports = class Branches extends ErrorStash {
5454
}
5555
// Hack to handle closures and keep params from changing
5656
const params = Object.assign({}, p)
57-
return this.github.repos.getBranchProtection(params).then((result) => {
57+
return this.github.rest.repos.getBranchProtection(params).then((result) => {
5858
const mergeDeep = new MergeDeep(this.log, this.github, ignorableFields)
5959
const changes = mergeDeep.compareDeep({ branch: { protection: this.reformatAndReturnBranchProtection(result.data) } }, { branch: { protection: Overrides.removeOverrides(overrides, branch.protection, result.data) } })
6060
const results = { msg: `Followings changes will be applied to the branch protection for ${params.branch.name} branch`, additions: changes.additions, modifications: changes.modifications, deletions: changes.deletions }
@@ -76,24 +76,24 @@ module.exports = class Branches extends ErrorStash {
7676
Object.assign(params, branch.protection, { headers: previewHeaders })
7777

7878
if (this.nop) {
79-
resArray.push(new NopCommand(this.constructor.name, this.repo, this.github.repos.updateBranchProtection.endpoint(params), 'Add Branch Protection'))
79+
resArray.push(new NopCommand(this.constructor.name, this.repo, this.github.rest.repos.updateBranchProtection.endpoint(params), 'Add Branch Protection'))
8080
return Promise.resolve(resArray)
8181
}
8282
this.log.debug(`Adding branch protection ${JSON.stringify(params)}`)
83-
return this.github.repos.updateBranchProtection(params).then(res => this.log.debug(`Branch protection applied successfully ${JSON.stringify(res.url)}`)).catch(e => { this.logError(`Error applying branch protection ${JSON.stringify(e)}`); return [] })
83+
return this.github.rest.repos.updateBranchProtection(params).then(res => this.log.debug(`Branch protection applied successfully ${JSON.stringify(res.url)}`)).catch(e => { this.logError(`Error applying branch protection ${JSON.stringify(e)}`); return [] })
8484
}).catch((e) => {
8585
if (e.status === 404) {
8686
Object.assign(params, Overrides.removeOverrides(overrides, branch.protection, {}), { headers: previewHeaders })
8787
if (this.nop) {
88-
resArray.push(new NopCommand(this.constructor.name, this.repo, this.github.repos.updateBranchProtection.endpoint(params), 'Add Branch Protection'))
88+
resArray.push(new NopCommand(this.constructor.name, this.repo, this.github.rest.repos.updateBranchProtection.endpoint(params), 'Add Branch Protection'))
8989
return Promise.resolve(resArray)
9090
}
9191
this.log.debug(`Adding branch protection ${JSON.stringify(params)}`)
92-
return this.github.repos.updateBranchProtection(params).then(res => this.log.debug(`Branch protection applied successfully ${JSON.stringify(res.url)}`)).catch(e => { this.logError(`Error applying branch protection ${JSON.stringify(e)}`); return [] })
92+
return this.github.rest.repos.updateBranchProtection(params).then(res => this.log.debug(`Branch protection applied successfully ${JSON.stringify(res.url)}`)).catch(e => { this.logError(`Error applying branch protection ${JSON.stringify(e)}`); return [] })
9393
} else {
9494
this.logError(e)
9595
if (this.nop) {
96-
resArray.push(new NopCommand(this.constructor.name, this.repo, this.github.repos.updateBranchProtection.endpoint(params), `${e}`, 'ERROR'))
96+
resArray.push(new NopCommand(this.constructor.name, this.repo, this.github.rest.repos.updateBranchProtection.endpoint(params), `${e}`, 'ERROR'))
9797
return Promise.resolve(resArray)
9898
}
9999
}

‎lib/plugins/collaborators.js‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ module.exports = class Collaborators extends Diffable {
2020
// 'direct' means all collaborators with permissions to an organization-owned repository, regardless of organization membership status. (includes outside collaborators)
2121
// 'all' means all collaborators the authenticated user can see.
2222
// We are using 'direct' to avoid double listing users outside collaborators and team members.
23-
return Promise.all([this.github.repos.listCollaborators({ repo: this.repo.repo, owner: this.repo.owner, affiliation: 'direct' }),
24-
this.github.repos.listInvitations({ repo: this.repo.repo, owner: this.repo.owner })])
23+
return Promise.all([this.github.rest.repos.listCollaborators({ repo: this.repo.repo, owner: this.repo.owner, affiliation: 'direct' }),
24+
this.github.rest.repos.listInvitations({ repo: this.repo.repo, owner: this.repo.owner })])
2525
.then(res => {
2626
const mapCollaborator = user => {
2727
return {
@@ -74,10 +74,10 @@ module.exports = class Collaborators extends Diffable {
7474
const data = Object.assign({}, attrs, this.repo)
7575
if (this.nop) {
7676
return Promise.resolve([
77-
new NopCommand(this.constructor.name, this.repo, this.github.repos.addCollaborator.endpoint(data), 'Add Collaborators')
77+
new NopCommand(this.constructor.name, this.repo, this.github.rest.repos.addCollaborator.endpoint(data), 'Add Collaborators')
7878
])
7979
}
80-
return this.github.repos.addCollaborator(data)
80+
return this.github.rest.repos.addCollaborator(data)
8181
}
8282

8383
updateInvite (invitation_id, permissions) {
@@ -89,31 +89,31 @@ module.exports = class Collaborators extends Diffable {
8989
}, this.repo)
9090
if (this.nop) {
9191
return Promise.resolve([
92-
new NopCommand(this.constructor.name, this.repo, this.github.repos.updateInvitation.endpoint(data), 'Update Invitation')
92+
new NopCommand(this.constructor.name, this.repo, this.github.rest.repos.updateInvitation.endpoint(data), 'Update Invitation')
9393
])
9494
}
95-
return this.github.repos.updateInvitation(data)
95+
return this.github.rest.repos.updateInvitation(data)
9696
}
9797

9898
remove (existing) {
9999
if (existing.pendinginvite) {
100100
const data = Object.assign({ invitation_id: existing.invitation_id }, this.repo)
101101
if (this.nop) {
102102
return Promise.resolve([
103-
new NopCommand(this.constructor.name, this.repo, this.github.repos.deleteInvitation.endpoint(data),
103+
new NopCommand(this.constructor.name, this.repo, this.github.rest.repos.deleteInvitation.endpoint(data),
104104
'Delete Invitation')
105105
])
106106
}
107-
return this.github.repos.deleteInvitation(data)
107+
return this.github.rest.repos.deleteInvitation(data)
108108
} else {
109109
const data = Object.assign({ username: existing.username }, this.repo)
110110
if (this.nop) {
111111
return Promise.resolve([
112-
new NopCommand(this.constructor.name, this.repo, this.github.repos.removeCollaborator.endpoint(data),
112+
new NopCommand(this.constructor.name, this.repo, this.github.rest.repos.removeCollaborator.endpoint(data),
113113
'Remove Collaborator')
114114
])
115115
}
116-
return this.github.repos.removeCollaborator(data)
116+
return this.github.rest.repos.removeCollaborator(data)
117117
}
118118
}
119119
}

‎lib/plugins/custom_properties.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ module.exports = class CustomProperties extends Diffable {
2525
this.log.debug(`Getting all custom properties for the repo ${repoFullName}`)
2626

2727
const customProperties = await this.github.paginate(
28-
this.github.repos.getCustomPropertiesValues,
28+
this.github.rest.repos.getCustomPropertiesValues,
2929
{
3030
owner,
3131
repo,
@@ -82,14 +82,14 @@ module.exports = class CustomProperties extends Diffable {
8282
return new NopCommand(
8383
this.constructor.name,
8484
this.repo,
85-
this.github.repos.createOrUpdateCustomPropertiesValues.endpoint(params),
85+
this.github.rest.repos.createOrUpdateCustomPropertiesValues.endpoint(params),
8686
`${operation} Custom Property`
8787
)
8888
}
8989

9090
try {
9191
this.log.debug(`${operation} Custom Property "${name}" for the repo ${repoFullName}`)
92-
await this.github.repos.createOrUpdateCustomPropertiesValues(params)
92+
await this.github.rest.repos.createOrUpdateCustomPropertiesValues(params)
9393
this.log.debug(`Successfully ${operation.toLowerCase()}d Custom Property "${name}" for the repo ${repoFullName}`)
9494
} catch (e) {
9595
this.logError(`Error during ${operation} Custom Property "${name}" for the repo ${repoFullName}: ${e.message || e}`)

‎lib/plugins/labels.js‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ module.exports = class Labels extends Diffable {
3434

3535
find () {
3636
this.log.debug(`Finding labels for ${JSON.stringify(this.wrapAttrs({ per_page: 100 }))}`)
37-
const options = this.github.issues.listLabelsForRepo.endpoint.merge(this.wrapAttrs({ per_page: 100 }))
38-
return this.github.repos.get(this.repo).then(() => {
37+
const options = this.github.rest.issues.listLabelsForRepo.endpoint.merge(this.wrapAttrs({ per_page: 100 }))
38+
return this.github.rest.repos.get(this.repo).then(() => {
3939
return this.github.paginate(options)
4040
})
4141
.catch(e => {
@@ -63,20 +63,20 @@ module.exports = class Labels extends Diffable {
6363
delete attrs.oldname
6464
if (this.nop) {
6565
return Promise.resolve([
66-
new NopCommand(this.constructor.name, this.repo, this.github.issues.updateLabel.endpoint(this.wrapAttrs(attrs)), 'Update label')
66+
new NopCommand(this.constructor.name, this.repo, this.github.rest.issues.updateLabel.endpoint(this.wrapAttrs(attrs)), 'Update label')
6767
])
6868
}
69-
return this.github.issues.updateLabel(this.wrapAttrs(attrs))
69+
return this.github.rest.issues.updateLabel(this.wrapAttrs(attrs))
7070
}
7171

7272
add (attrs) {
7373
if (this.nop) {
7474
return Promise.resolve([
75-
new NopCommand(this.constructor.name, this.repo, this.github.issues.createLabel.endpoint(this.wrapAttrs(attrs)), 'Create label')
75+
new NopCommand(this.constructor.name, this.repo, this.github.rest.issues.createLabel.endpoint(this.wrapAttrs(attrs)), 'Create label')
7676
])
7777
}
7878
this.log.debug(`Creating labels for ${JSON.stringify(attrs, null, 4)}`)
79-
return this.github.issues.createLabel(this.wrapAttrs(attrs)).catch(e => this.logError(` ${JSON.stringify(e)}`))
79+
return this.github.rest.issues.createLabel(this.wrapAttrs(attrs)).catch(e => this.logError(` ${JSON.stringify(e)}`))
8080
}
8181

8282
remove (existing) {
@@ -85,10 +85,10 @@ module.exports = class Labels extends Diffable {
8585
}
8686
if (this.nop) {
8787
return Promise.resolve([
88-
new NopCommand(this.constructor.name, this.repo, this.github.issues.deleteLabel.endpoint(this.wrapAttrs({ name: existing.name })), 'Delete label')
88+
new NopCommand(this.constructor.name, this.repo, this.github.rest.issues.deleteLabel.endpoint(this.wrapAttrs({ name: existing.name })), 'Delete label')
8989
])
9090
}
91-
return this.github.issues.deleteLabel(this.wrapAttrs({ name: existing.name }))
91+
return this.github.rest.issues.deleteLabel(this.wrapAttrs({ name: existing.name }))
9292
}
9393

9494
wrapAttrs (attrs) {

0 commit comments

Comments
 (0)