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

Updated tests #684

Draft
wants to merge 23 commits into
base: main-enterprise
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
f534c6e
add initial code for preventing multiple suborg config for repos
decyjphr Aug 8, 2024
04434a2
detect conflict even when a single suborg config is changed
decyjphr Aug 29, 2024
775b7ca
remove duplicate errors
decyjphr Sep 17, 2024
87b8a95
Merge branch 'main-enterprise' into yj-repo-in-many-suborgs
decyjphr Sep 17, 2024
60f8fd3
dont add nulls and undefined to results
decyjphr Sep 17, 2024
336d739
add some JSDoc types
Gramatus Sep 15, 2024
e826f40
fix some buggy logging statements
Gramatus Sep 15, 2024
2b90f8b
add missing owner.login property in push.settings.json
Gramatus Sep 15, 2024
f94f6e4
change sender type in repository.edited.json
Gramatus Sep 15, 2024
520f6e8
use standard probot logger in index.test.js
Gramatus Sep 15, 2024
6eff20c
fix index.test.js
Gramatus Sep 15, 2024
585c7da
use probot logger in branches.test.js
Gramatus Sep 15, 2024
8bc6886
return data when updating branch protection
Gramatus Sep 15, 2024
489546b
fix broken branch test
Gramatus Sep 15, 2024
0bb2713
use probot logger in milestones.test.js
Gramatus Sep 15, 2024
5178080
include `title` in `NAME_FIELDS`
Gramatus Sep 15, 2024
16925d6
fix broken milestones test
Gramatus Sep 15, 2024
996af69
refact: changed to async/await and removed some returns
Gramatus Oct 5, 2024
cb401ec
refact: avoid repeating code in `updateRepos`
Gramatus Oct 5, 2024
cac9b84
FORK_ONLY: fix some errors found by ts checker
Gramatus Oct 5, 2024
9b0c66b
chore: autoformatted according to rules after #664
Gramatus Oct 5, 2024
444cefe
fix: update eslint ecmaVersion to 13
Gramatus Sep 15, 2024
b4ea065
reapply eslint to tests, still supporting jest
Gramatus Sep 15, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions .eslintrc.json
Original file line number Diff line number Diff line change
@@ -1,16 +1,23 @@
{
"env": {
"browser": true,
"node": true,
"commonjs": true,
"es2021": true
},
"extends": [
"standard"
"eslint:recommended"
],
"parserOptions": {
"ecmaVersion": 12
"ecmaVersion": 13
},
"rules": {
},
"ignorePatterns": ["test/**/*.js"]
"overrides": [
{
"files": ["test/**/*.js"],
"env": {
"jest": true
}
}
]
}
56 changes: 30 additions & 26 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,15 @@ const env = require('./lib/env')

let deploymentConfig


module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => {
/**
* @import { Probot, ApplicationFunctionOptions, ProbotOctokit } from "probot"
* @param {Probot} robot
* @param {ApplicationFunctionOptions} probotOptions
*/
module.exports = (robot, _, Settings = require('./lib/settings')) => {
let appName = 'safe-settings'
let appSlug = 'safe-settings'
async function syncAllSettings (nop, context, repo = context.repo(), ref) {
async function syncAllSettings(nop, context, repo = context.repo(), ref) {
try {
deploymentConfig = await loadYamlFileSystem()
robot.log.debug(`deploymentConfig is ${JSON.stringify(deploymentConfig)}`)
Expand Down Expand Up @@ -42,7 +46,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
}
}

async function syncSubOrgSettings (nop, context, suborg, repo = context.repo(), ref) {
async function syncSubOrgSettings(nop, context, suborg, repo = context.repo(), ref) {
try {
deploymentConfig = await loadYamlFileSystem()
robot.log.debug(`deploymentConfig is ${JSON.stringify(deploymentConfig)}`)
Expand All @@ -67,7 +71,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
}
}

async function syncSettings (nop, context, repo = context.repo(), ref) {
async function syncSettings(nop, context, repo = context.repo(), ref) {
try {
deploymentConfig = await loadYamlFileSystem()
robot.log.debug(`deploymentConfig is ${JSON.stringify(deploymentConfig)}`)
Expand All @@ -92,7 +96,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
}
}

async function renameSync (nop, context, repo = context.repo(), rename, ref) {
async function renameSync(nop, context, repo = context.repo(), rename, ref) {
try {
deploymentConfig = await loadYamlFileSystem()
robot.log.debug(`deploymentConfig is ${JSON.stringify(deploymentConfig)}`)
Expand All @@ -101,7 +105,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
const config = Object.assign({}, deploymentConfig, runtimeConfig)
const renameConfig = Object.assign({}, config, rename)
robot.log.debug(`config for ref ${ref} is ${JSON.stringify(config)}`)
return Settings.sync(nop, context, repo, renameConfig, ref )
return Settings.sync(nop, context, repo, renameConfig, ref)
} catch (e) {
if (nop) {
let filename = env.SETTINGS_FILE_PATH
Expand All @@ -123,7 +127,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
*
* @return The parsed YAML file
*/
async function loadYamlFileSystem () {
async function loadYamlFileSystem() {
if (deploymentConfig === undefined) {
const deploymentConfigPath = env.DEPLOYMENT_CONFIG_FILE
if (fs.existsSync(deploymentConfigPath)) {
Expand All @@ -135,7 +139,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
return deploymentConfig
}

function getAllChangedSubOrgConfigs (payload) {
function getAllChangedSubOrgConfigs(payload) {
const settingPattern = new Glob(`${env.CONFIG_PATH}/suborgs/*.yml`)
// Changes will be an array of files that were added
const added = payload.commits.map(c => {
Expand All @@ -159,7 +163,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
return configs
}

function getAllChangedRepoConfigs (payload, owner) {
function getAllChangedRepoConfigs(payload, owner) {
const settingPattern = new Glob(`${env.CONFIG_PATH}/repos/*.yml`)
// Changes will be an array of files that were added
const added = payload.commits.map(c => {
Expand All @@ -182,7 +186,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
return configs
}

function getChangedRepoConfigName (glob, files, owner) {
function getChangedRepoConfigName(glob, files, owner) {
const modifiedFiles = files.filter(s => {
robot.log.debug(JSON.stringify(s))
return (s.search(glob) >= 0)
Expand All @@ -193,7 +197,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
})
}

function getChangedSubOrgConfigName (glob, files) {
function getChangedSubOrgConfigName(glob, files) {
const modifiedFiles = files.filter(s => {
robot.log.debug(JSON.stringify(s))
return (s.search(glob) >= 0)
Expand All @@ -205,7 +209,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
})
}

async function createCheckRun (context, pull_request, head_sha, head_branch) {
async function createCheckRun(context, pull_request, head_sha) {
const { payload } = context
// robot.log.debug(`Check suite was requested! for ${context.repo()} ${pull_request.number} ${head_sha} ${head_branch}`)
const res = await context.octokit.checks.create({
Expand All @@ -229,12 +233,12 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
const app = await github.apps.getAuthenticated()
appName = app.data.name
appSlug = app.data.slug
robot.log.debug(`Validated the app is configured properly = \n${JSON.stringify(app.data, null, 2)}`)
robot.log.debug(`Validated the app ${appName} is configured properly = \n${JSON.stringify(app.data, null, 2)}`)
}
}


async function syncInstallation () {
async function syncInstallation() {
robot.log.trace('Fetching installations')
const github = await robot.auth()

Expand Down Expand Up @@ -383,7 +387,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
robot.on('repository.edited', async context => {
const { payload } = context
const { sender } = payload
robot.log.debug('repository.edited payload from ', JSON.stringify(sender))
robot.log.debug(sender, 'repository.edited payload from')

if (sender.type === 'Bot') {
robot.log.debug('Repository Edited by a Bot')
Expand All @@ -395,7 +399,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
})

robot.on('repository.renamed', async context => {
if (env.BLOCK_REPO_RENAME_BY_HUMAN!== 'true') {
if (env.BLOCK_REPO_RENAME_BY_HUMAN !== 'true') {
robot.log.debug(`"env.BLOCK_REPO_RENAME_BY_HUMAN" is 'false' by default. Repo rename is not managed by Safe-settings. Continue with the default behavior.`)
return
}
Expand All @@ -414,7 +418,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
const newPath = `.github/repos/${payload.repository.name}.yml`
robot.log.debug(oldPath)
try {
const repofile = await context.octokit.request('GET /repos/{owner}/{repo}/contents/{path}', {
const repofile = await context.octokit.request('GET /repos/{owner}/{repo}/contents/{path}', {
owner: payload.repository.owner.login,
repo: env.ADMIN_REPO,
path: oldPath,
Expand All @@ -439,11 +443,11 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
} catch (error) {
if (error.status === 404) {
// if the a config file does not exist, create one from the old one
const update = await context.octokit.request('PUT /repos/{owner}/{repo}/contents/{path}', {
await context.octokit.request('PUT /repos/{owner}/{repo}/contents/{path}', {
owner: payload.repository.owner.login,
repo: env.ADMIN_REPO,
path: newPath,
name: `${payload.repository.name}.yml`,
name: `${payload.repository.name}.yml`,
content: content,
message: `Repo Renamed and safe-settings renamed the file from ${payload.changes.repository.name.from} to ${payload.repository.name}`,
sha: repofile.data.sha,
Expand All @@ -455,21 +459,21 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
} else {
robot.log.error(error)
}
}
}

} catch (error) {
if (error.status === 404) {
//nop
} else {
} else {
robot.log.error(error)
}
}
}
return
} else {
robot.log.debug('Repository Edited by a Human')
// Create a repository config to reset the name back to the previous name
const rename = {repository: { name: payload.changes.repository.name.from, oldname: payload.repository.name}}
const repo = {repo: payload.changes.repository.name.from, owner: payload.repository.owner.login}
const rename = { repository: { name: payload.changes.repository.name.from, oldname: payload.repository.name } }
const repo = { repo: payload.changes.repository.name.from, owner: payload.repository.owner.login }
return renameSync(false, context, repo, rename)
}
})
Expand Down Expand Up @@ -663,7 +667,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
syncInstallation()
})
}

// Get info about the app
info()

Expand Down
2 changes: 1 addition & 1 deletion lib/mergeDeep.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const mergeBy = require('./mergeArrayBy')
const DeploymentConfig = require('./deploymentConfig')

const NAME_FIELDS = ['name', 'username', 'actor_id', 'login', 'type', 'key_prefix']
const NAME_FIELDS = ['name', 'username', 'actor_id', 'login', 'type', 'key_prefix', 'title']
const NAME_USERNAME_PROPERTY = item => NAME_FIELDS.find(prop => Object.prototype.hasOwnProperty.call(item, prop))
const GET_NAME_USERNAME_PROPERTY = item => { if (NAME_USERNAME_PROPERTY(item)) return item[NAME_USERNAME_PROPERTY(item)] }

Expand Down
10 changes: 9 additions & 1 deletion lib/plugins/branches.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,15 @@ module.exports = class Branches extends ErrorStash {
return Promise.resolve(resArray)
}
this.log.debug(`Adding branch protection ${JSON.stringify(params)}`)
return this.github.repos.updateBranchProtection(params).then(res => this.log(`Branch protection applied successfully ${JSON.stringify(res.url)}`)).catch(e => { this.logError(`Error applying branch protection ${JSON.stringify(e)}`); return [] })
return this.github.repos.updateBranchProtection(params)
.then(res => {
this.log.info(`Branch protection applied successfully ${JSON.stringify(res.url)}`)
return res.url
})
.catch(e => {
this.logError(`Error applying branch protection ${JSON.stringify(e)}`)
return []
})
}).catch((e) => {
if (e.status === 404) {
Object.assign(params, branch.protection, { headers: previewHeaders })
Expand Down
5 changes: 3 additions & 2 deletions lib/plugins/diffable.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,14 @@ const MergeDeep = require('../mergeDeep')
const NopCommand = require('../nopcommand')
const ignorableFields = ['id', 'node_id', 'default', 'url']
module.exports = class Diffable extends ErrorStash {
constructor (nop, github, repo, entries, log, errors) {
constructor (nop, github, repo, entries, log, errors, filterNameFields) {
super(errors)
this.github = github
this.repo = repo
this.entries = entries
this.log = log
this.nop = nop
this.filterNameFields = filterNameFields ?? MergeDeep.NAME_FIELDS
}

filterEntries () {
Expand Down Expand Up @@ -102,7 +103,7 @@ module.exports = class Diffable extends ErrorStash {
}
}
// Delete any diffable that now only has name and no other attributes
filteredEntries = filteredEntries.filter(entry => Object.keys(entry).filter(key => !MergeDeep.NAME_FIELDS.includes(key)).length !== 0)
filteredEntries = filteredEntries.filter(entry => Object.keys(entry).filter(key => !this.filterNameFields.includes(key)).length !== 0)

const changes = []

Expand Down
5 changes: 3 additions & 2 deletions lib/plugins/milestones.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
const MergeDeep = require('../mergeDeep')
const Diffable = require('./diffable')

module.exports = class Milestones extends Diffable {
constructor (...args) {
super(...args)
constructor (nop, github, repo, entries, log, errors) {
super(nop, github, repo, entries, log, errors, MergeDeep.NAME_FIELDS.filter(i => i !== 'title'))

if (this.entries) {
this.entries.forEach(milestone => {
Expand Down
4 changes: 2 additions & 2 deletions lib/plugins/rulesets.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@ module.exports = class Rulesets extends Diffable {
// Find all Rulesets for this org
find () {
if (this.scope === 'org') {
this.log.debug(`Getting all rulesets for the org ${this.org}`)
this.log.debug(`Getting all rulesets for the org ${this.repo.owner}`)

const listOptions = this.github.request.endpoint.merge('GET /orgs/{org}/rulesets', {
org: this.repo.owner,
headers: version
})
this.log(listOptions)
this.log.debug(listOptions)
return this.github.paginate(listOptions)
.then(res => {
const rulesets = res.map(ruleset => {
Expand Down
Loading