From f4704cd21fa4d6f5af8fe94d38208b1b2bf62403 Mon Sep 17 00:00:00 2001 From: Jonathan Morley Date: Mon, 14 Oct 2024 12:31:02 -0400 Subject: [PATCH 1/4] feat: allow some settings to be marked 'unsafe', to allow overriding in the repository --- index.js | 102 +-- lib/configManager.js | 46 +- lib/deploymentConfig.js | 79 +-- lib/mergeDeep.js | 20 +- lib/plugins/branches.js | 5 +- lib/plugins/diffable.js | 5 +- lib/plugins/environments.js | 622 +++++++++--------- lib/plugins/labels.js | 6 +- lib/plugins/repository.js | 67 +- lib/plugins/rulesets.js | 4 +- lib/plugins/validator.js | 2 +- lib/settings.js | 184 +++--- package-lock.json | 20 + package.json | 2 + test/unit/lib/mergeDeep.test.js | 73 +- test/unit/lib/plugins/autolinks.test.js | 3 +- test/unit/lib/plugins/branches.test.js | 3 +- test/unit/lib/plugins/collaborators.test.js | 3 +- .../lib/plugins/custom_properties.test.js | 3 +- test/unit/lib/plugins/labels.test.js | 3 +- test/unit/lib/plugins/repository.test.js | 3 +- test/unit/lib/plugins/teams.test.js | 3 +- test/unit/lib/settings.test.js | 42 +- test/unit/lib/validator.test.js | 27 +- 24 files changed, 705 insertions(+), 622 deletions(-) diff --git a/index.js b/index.js index 7e4ef032..0983bfcc 100644 --- a/index.js +++ b/index.js @@ -13,13 +13,15 @@ let deploymentConfig module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => { let appName = 'safe-settings' let appSlug = 'safe-settings' + + // All repos _could_ be affected, so sync everything async function syncAllSettings (nop, context, repo = context.repo(), ref) { try { - deploymentConfig = await loadYamlFileSystem() - robot.log.debug(`deploymentConfig is ${JSON.stringify(deploymentConfig)}`) + robot.log.info('Synchronizing all settings') + const configManager = new ConfigManager(context, ref) - const runtimeConfig = await configManager.loadGlobalSettingsYaml() - const config = Object.assign({}, deploymentConfig, runtimeConfig) + const config = await configManager.loadGlobalSettingsYaml() + robot.log.debug(`config for ref ${ref} is ${JSON.stringify(config)}`) if (ref) { return Settings.syncAll(nop, context, repo, config, ref) @@ -42,13 +44,14 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => } } + // Only the suborg has been modified, so only sync that async function syncSubOrgSettings (nop, context, suborg, repo = context.repo(), ref) { try { - deploymentConfig = await loadYamlFileSystem() - robot.log.debug(`deploymentConfig is ${JSON.stringify(deploymentConfig)}`) + robot.log.info(`Synchronizing settings for suborg: ${suborg}`) + const configManager = new ConfigManager(context, ref) - const runtimeConfig = await configManager.loadGlobalSettingsYaml() - const config = Object.assign({}, deploymentConfig, runtimeConfig) + const config = await configManager.loadGlobalSettingsYaml() + robot.log.debug(`config for ref ${ref} is ${JSON.stringify(config)}`) return Settings.syncSubOrgs(nop, context, suborg, repo, config, ref) } catch (e) { @@ -67,13 +70,14 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => } } + // Only the repository has been modified, so only sync that async function syncSettings (nop, context, repo = context.repo(), ref) { try { - deploymentConfig = await loadYamlFileSystem() - robot.log.debug(`deploymentConfig is ${JSON.stringify(deploymentConfig)}`) + robot.log.info(`Synchronizing settings for repo: ${repo}`) + const configManager = new ConfigManager(context, ref) - const runtimeConfig = await configManager.loadGlobalSettingsYaml() - const config = Object.assign({}, deploymentConfig, runtimeConfig) + const config = await configManager.loadGlobalSettingsYaml() + robot.log.debug(`config for ref ${ref} is ${JSON.stringify(config)}`) return Settings.sync(nop, context, repo, config, ref) } catch (e) { @@ -258,47 +262,57 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => return null } + // On any push robot.on('push', async context => { const { payload } = context const { repository } = payload - const adminRepo = repository.name === env.ADMIN_REPO - if (!adminRepo) { - return - } - const defaultBranch = payload.ref === 'refs/heads/' + repository.default_branch if (!defaultBranch) { robot.log.debug('Not working on the default branch, returning...') return } - const settingsModified = payload.commits.find(commit => { - return commit.added.includes(Settings.FILE_NAME) || - commit.modified.includes(Settings.FILE_NAME) - }) - if (settingsModified) { - robot.log.debug(`Changes in '${Settings.FILE_NAME}' detected, doing a full synch...`) - return syncAllSettings(false, context) - } + const adminRepo = repository.name === env.ADMIN_REPO + if (adminRepo) { + const settingsModified = payload.commits.find(commit => { + return commit.added.includes(Settings.FILE_NAME) || + commit.modified.includes(Settings.FILE_NAME) + }) + if (settingsModified) { + robot.log.debug(`Changes in '${Settings.FILE_NAME}' detected, doing a full sync...`) + return syncAllSettings(false, context) + } - const repoChanges = getAllChangedRepoConfigs(payload, context.repo().owner) - if (repoChanges.length > 0) { - return Promise.all(repoChanges.map(repo => { - return syncSettings(false, context, repo) - })) - } + const repoChanges = getAllChangedRepoConfigs(payload, context.repo().owner) + if (repoChanges.length > 0) { + return Promise.all(repoChanges.map(repo => { + return syncSettings(false, context, repo) + })) + } - const changes = getAllChangedSubOrgConfigs(payload) - if (changes.length) { - return Promise.all(changes.map(suborg => { - return syncSubOrgSettings(false, context, suborg) - })) - } + const changes = getAllChangedSubOrgConfigs(payload) + if (changes.length) { + return Promise.all(changes.map(suborg => { + return syncSubOrgSettings(false, context, suborg) + })) + } - robot.log.debug(`No changes in '${Settings.FILE_NAME}' detected, returning...`) + robot.log.debug(`No changes in '${Settings.FILE_NAME}' detected, returning...`) + } else { + const settingsModified = payload.commits.find(commit => { + return commit.added.includes('.github/settings.yml') || + commit.modified.includes('.github/settings.yml') + }) + if (settingsModified) { + robot.log.debug(`Changes in '.github/settings.yml' detected, doing a sync for ${repository.name}...`) + return syncSettings(false, context) + } + } }) + // Invoke syncSettings for events that could cause drift + robot.on('create', async context => { const { payload } = context const { sender } = payload @@ -394,6 +408,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => return syncSettings(false, context) }) + // Renames need some extra handling robot.on('repository.renamed', async context => { 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.`) @@ -474,7 +489,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => } }) - + // Validate settings when PR checks are needed robot.on('check_suite.requested', async context => { const { payload } = context const { repository } = payload @@ -503,6 +518,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => return createCheckRun(context, pull_request, headSha, headBranch) }) + // Validate settings when PR checks are needed robot.on('pull_request.opened', async context => { robot.log.debug('Pull_request opened !') const { payload } = context @@ -522,6 +538,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => return createCheckRun(context, pull_request, payload.pull_request.head.sha, payload.pull_request.head.ref) }) + // Validate settings when PR checks are needed robot.on('pull_request.reopened', async context => { robot.log.debug('Pull_request REopened !') const { payload } = context @@ -543,16 +560,13 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => return createCheckRun(context, pull_request, payload.pull_request.head.sha, payload.pull_request.head.ref) }) + // Validate settings when PR checks are needed robot.on(['check_suite.rerequested'], async context => { robot.log.debug('Check suite was rerequested!') return createCheckRun(context) }) - robot.on(['check_suite.rerequested'], async context => { - robot.log.debug('Check suite was rerequested!') - return createCheckRun(context) - }) - + // Validate settings when PR checks are needed robot.on(['check_run.created'], async context => { robot.log.debug('Check run was created!') const { payload } = context diff --git a/lib/configManager.js b/lib/configManager.js index 58f5bb43..7b3cbe71 100644 --- a/lib/configManager.js +++ b/lib/configManager.js @@ -10,18 +10,30 @@ module.exports = class ConfigManager { } /** -* Loads a file from GitHub -* -* @param params Params to fetch the file with -* @return The parsed YAML file -*/ - async loadYaml (filePath) { + * Loads the settings file from GitHub + * + * @return The parsed YAML file + */ + async loadGlobalSettingsYaml () { + const CONFIG_PATH = env.CONFIG_PATH + const filePath = path.posix.join(CONFIG_PATH, env.SETTINGS_FILE_PATH) + return ConfigManager.loadYaml(this.context.octokit, { + owner: this.context.repo().owner, + repo: env.ADMIN_REPO, + path: filePath, + ref: this.ref + }) + } + + /** + * Loads a file from GitHub + * + * @param params Params to fetch the file with + * @return The parsed YAML file + */ + static async loadYaml (octokit, params) { try { - const repo = { owner: this.context.repo().owner, repo: env.ADMIN_REPO } - const params = Object.assign(repo, { path: filePath, ref: this.ref }) - const response = await this.context.octokit.repos.getContent(params).catch(e => { - this.log.error(`Error getting settings ${e}`) - }) + const response = await octokit.repos.getContent(params) // Ignore in case path is a folder // - https://developer.github.com/v3/repos/contents/#response-if-content-is-a-directory @@ -43,16 +55,4 @@ module.exports = class ConfigManager { throw e } } - - /** - * Loads a file from GitHub - * - * @param params Params to fetch the file with - * @return The parsed YAML file - */ - async loadGlobalSettingsYaml () { - const CONFIG_PATH = env.CONFIG_PATH - const filePath = path.posix.join(CONFIG_PATH, env.SETTINGS_FILE_PATH) - return this.loadYaml(filePath) - } } diff --git a/lib/deploymentConfig.js b/lib/deploymentConfig.js index f8fdeb27..435914fe 100644 --- a/lib/deploymentConfig.js +++ b/lib/deploymentConfig.js @@ -4,51 +4,56 @@ const env = require('./env') /** * Class representing a deployment config. - * It is a singleton (class object) for the deployment settings. - * The settings are loaded from the deployment-settings.yml file during initialization and stored as static properties. + * The settings are loaded from the deployment-settings.yml file during initialization and stored as fields. */ class DeploymentConfig { - //static config - static configvalidators = {} - static overridevalidators = {} - - static { - const deploymentConfigPath = process.env.DEPLOYMENT_CONFIG_FILE ? process.env.DEPLOYMENT_CONFIG_FILE : 'deployment-settings.yml' - if (fs.existsSync(deploymentConfigPath)) { - this.config = yaml.load(fs.readFileSync(deploymentConfigPath)) - } else { - this.config = { restrictedRepos: ['admin', '.github', 'safe-settings'] } - } - - const overridevalidators = this.config.overridevalidators - if (this.isIterable(overridevalidators)) { - for (const validator of overridevalidators) { - // eslint-disable-next-line no-new-func - const f = new Function('baseconfig', 'overrideconfig', 'githubContext', validator.script) - this.overridevalidators[validator.plugin] = { canOverride: f, error: validator.error } - } - } - const configvalidators = this.config.configvalidators - if (this.isIterable(configvalidators)) { - for (const validator of configvalidators) { - // eslint-disable-next-line no-new-func - const f = new Function('baseconfig', 'githubContext', validator.script) - this.configvalidators[validator.plugin] = { isValid: f, error: validator.error } - } - } + constructor () { + const deploymentConfigPath = process.env.DEPLOYMENT_CONFIG_FILE ? process.env.DEPLOYMENT_CONFIG_FILE : 'deployment-settings.yml' + + let config = {} + if (fs.existsSync(deploymentConfigPath)) { + config = yaml.load(fs.readFileSync(deploymentConfigPath)) + } + + this.overrideValidators = {} + if (config.overridevalidators) { + if (!Array.isArray(config.overridevalidators)) { + throw new Error('overridevalidators must be an array') + } + for (const validator of config.overridevalidators) { + // eslint-disable-next-line no-new-func + const f = new Function('baseconfig', 'overrideconfig', 'githubContext', validator.script) + this.overrideValidators[validator.plugin] = { canOverride: f, error: validator.error } + } } - static isIterable (obj) { - // checks for null and undefined - if (obj == null) { - return false - } - return typeof obj[Symbol.iterator] === 'function' + this.configValidators = {} + if (config.configvalidators) { + if (!Array.isArray(config.configvalidators)) { + throw new Error('configvalidators must be an array') } + for (const validator of config.configvalidators) { + // eslint-disable-next-line no-new-func + const f = new Function('baseconfig', 'githubContext', validator.script) + this.configvalidators[validator.plugin] = { isValid: f, error: validator.error } + } + } + + this.restrictedRepos = config.restrictedRepos ?? ['admin', '.github', 'safe-settings'] - constructor (nop, context, repo, config, ref, suborg) { + this.unsafeFields = [] + // eslint-disable-next-line dot-notation + if (config['unsafe_fields']) { + // eslint-disable-next-line dot-notation + if (!Array.isArray(config['unsafe_fields'])) { + throw new Error('unsafe_fields must be an array') + } + // eslint-disable-next-line dot-notation + this.unsafeFields = config['unsafe_fields'] } + } } + DeploymentConfig.FILE_NAME = `${env.CONFIG_PATH}/settings.yml` module.exports = DeploymentConfig diff --git a/lib/mergeDeep.js b/lib/mergeDeep.js index 054a5b34..54c0ca5f 100644 --- a/lib/mergeDeep.js +++ b/lib/mergeDeep.js @@ -1,17 +1,16 @@ const mergeBy = require('./mergeArrayBy') -const DeploymentConfig = require('./deploymentConfig') const NAME_FIELDS = ['name', 'username', 'actor_id', 'login', 'type', 'key_prefix'] 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)] } class MergeDeep { - constructor (log, github, ignorableFields = [], configvalidators = {}, overridevalidators = {}) { + constructor (log, github, ignorableFields = [], deploymentConfig) { this.log = log this.github = github this.ignorableFields = ignorableFields - this.configvalidators = DeploymentConfig.configvalidators - this.overridevalidators = DeploymentConfig.overridevalidators + this.configvalidators = deploymentConfig.configValidators + this.overridevalidators = deploymentConfig.overrideValidators } isObjectNotArray (item) { @@ -336,11 +335,20 @@ class MergeDeep { this.validateConfig(key, target[key]) } } else { - // Not calling validators when target[key] is primitive or empty - target[key] = source[key] + // Handle nulls differently than undefined + if (source[key] === null) { + // nulls will be respected, and remove the value + delete target[key] + } else if (source[key] === undefined) { + // undefined will be ignored + } else { + // Not calling validators when target[key] is primitive + target[key] = source[key] + } } } } + this.log.debug(`returning ${JSON.stringify(target)}`) return target } diff --git a/lib/plugins/branches.js b/lib/plugins/branches.js index cac0aadf..e153d296 100644 --- a/lib/plugins/branches.js +++ b/lib/plugins/branches.js @@ -5,10 +5,11 @@ const ignorableFields = [] const previewHeaders = { accept: 'application/vnd.github.hellcat-preview+json,application/vnd.github.luke-cage-preview+json,application/vnd.github.zzzax-preview+json' } module.exports = class Branches extends ErrorStash { - constructor (nop, github, repo, settings, log, errors) { + constructor (nop, github, repo, settings, deploymentConfig, log, errors) { super(errors) this.github = github this.repo = repo + this.deploymentConfig = deploymentConfig this.branches = settings this.log = log this.nop = nop @@ -48,7 +49,7 @@ module.exports = class Branches extends ErrorStash { // Hack to handle closures and keep params from changing const params = Object.assign({}, p) return this.github.repos.getBranchProtection(params).then((result) => { - const mergeDeep = new MergeDeep(this.log, this.github, ignorableFields) + const mergeDeep = new MergeDeep(this.log, this.github, ignorableFields, this.deploymentConfig) const changes = mergeDeep.compareDeep({ branch: { protection: this.reformatAndReturnBranchProtection(result.data) } }, { branch: { protection: branch.protection } }) 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 } this.log.debug(`Result of compareDeep = ${results}`) diff --git a/lib/plugins/diffable.js b/lib/plugins/diffable.js index 8db6a0ef..940753f1 100644 --- a/lib/plugins/diffable.js +++ b/lib/plugins/diffable.js @@ -24,11 +24,12 @@ 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, deploymentConfig, log, errors) { super(errors) this.github = github this.repo = repo this.entries = entries + this.deploymentConfig = deploymentConfig this.log = log this.nop = nop } @@ -80,7 +81,7 @@ module.exports = class Diffable extends ErrorStash { return this.find().then(existingRecords => { this.log.debug(` ${JSON.stringify(existingRecords, null, 2)} \n\n ${JSON.stringify(filteredEntries, null, 2)} `) - const mergeDeep = new MergeDeep(this.log, this.github, ignorableFields) + const mergeDeep = new MergeDeep(this.log, this.github, ignorableFields, this.deploymentConfig) const compare = mergeDeep.compareDeep(existingRecords, filteredEntries) const results = { msg: 'Changes found', additions: compare.additions, modifications: compare.modifications, deletions: compare.deletions } this.log.debug(`Results of comparing ${this.constructor.name} diffable target ${JSON.stringify(existingRecords)} with source ${JSON.stringify(filteredEntries)} is ${results}`) diff --git a/lib/plugins/environments.js b/lib/plugins/environments.js index 4ceaa9ff..352c5c2c 100644 --- a/lib/plugins/environments.js +++ b/lib/plugins/environments.js @@ -1,363 +1,359 @@ +/* eslint-disable camelcase */ const Diffable = require('./diffable') -const MergeDeep = require('../mergeDeep') const NopCommand = require('../nopcommand') module.exports = class Environments extends Diffable { - constructor(...args) { - super(...args) - - if (this.entries) { - // Force all names to lowercase to avoid comparison issues. - this.entries.forEach(environment => { - environment.name = environment.name.toLowerCase(); - if(environment.variables) { - environment.variables.forEach(variable => { - variable.name = variable.name.toLowerCase(); - }); - } - }) + constructor (...args) { + super(...args) + + if (this.entries) { + // Force all names to lowercase to avoid comparison issues. + this.entries.forEach(environment => { + environment.name = environment.name.toLowerCase() + if (environment.variables) { + environment.variables.forEach(variable => { + variable.name = variable.name.toLowerCase() + }) } + }) + } + } + + async find () { + const { data: { environments } } = await this.github.request('GET /repos/:org/:repo/environments', { + org: this.repo.owner, + repo: this.repo.repo + }) + + const environments_mapped = [] + + for (const environment of environments) { + const mapped = { + name: environment.name.toLowerCase(), + repo: this.repo.repo, + wait_timer: (environment.protection_rules.find(rule => rule.type === 'wait_timer') || { wait_timer: 0 }).wait_timer, + prevent_self_review: (environment.protection_rules.find(rule => rule.type === 'required_reviewers') || { prevent_self_review: false }).prevent_self_review, + reviewers: (environment.protection_rules.find(rule => rule.type === 'required_reviewers') || { reviewers: [] }).reviewers.map(reviewer => ({ id: reviewer.reviewer.id, type: reviewer.type })), + deployment_branch_policy: environment.deployment_branch_policy === null + ? null + : { + protected_branches: (environment.deployment_branch_policy || { protected_branches: false }).protected_branches, + custom_branch_policies: (environment.deployment_branch_policy || { custom_branch_policies: false }).custom_branch_policies && (await this.github.request('GET /repos/:org/:repo/environments/:environment_name/deployment-branch-policies', { + org: this.repo.owner, + repo: this.repo.repo, + environment_name: environment.name + })).data.branch_policies.map(policy => ({ + name: policy.name + })) + }, + variables: (await this.github.request('GET /repos/:org/:repo/environments/:environment_name/variables', { + org: this.repo.owner, + repo: this.repo.repo, + environment_name: environment.name + })).data.variables.map(variable => ({ name: variable.name.toLowerCase(), value: variable.value })), + deployment_protection_rules: (await this.github.request('GET /repos/:org/:repo/environments/:environment_name/deployment_protection_rules', { + org: this.repo.owner, + repo: this.repo.repo, + environment_name: environment.name + })).data.custom_deployment_protection_rules.map(rule => ({ + app_id: rule.app.id, + id: rule.id + })) + } + environments_mapped.push(mapped) + // console.log(mapped); } - async find() { - const { data: { environments } } = await this.github.request('GET /repos/:org/:repo/environments', { - org: this.repo.owner, - repo: this.repo.repo - }); + return environments_mapped + } - let environments_mapped = []; + comparator (existing, attrs) { + return existing.name === attrs.name + } - for(let environment of environments) { - const mapped = { - name: environment.name.toLowerCase(), - repo: this.repo.repo, - wait_timer: (environment.protection_rules.find(rule => rule.type === 'wait_timer') || { wait_timer: 0 }).wait_timer, - prevent_self_review: (environment.protection_rules.find(rule => rule.type === 'required_reviewers') || { prevent_self_review: false }).prevent_self_review, - reviewers: (environment.protection_rules.find(rule => rule.type === 'required_reviewers') || { reviewers: [] }).reviewers.map(reviewer => ({id: reviewer.reviewer.id, type: reviewer.type})), - deployment_branch_policy: environment.deployment_branch_policy === null ? null : { - protected_branches: (environment.deployment_branch_policy || { protected_branches: false }).protected_branches, - custom_branch_policies: (environment.deployment_branch_policy || { custom_branch_policies: false }).custom_branch_policies && (await this.github.request('GET /repos/:org/:repo/environments/:environment_name/deployment-branch-policies', { - org: this.repo.owner, - repo: this.repo.repo, - environment_name: environment.name - })).data.branch_policies.map(policy => ({ - name: policy.name - })) - }, - variables: (await this.github.request('GET /repos/:org/:repo/environments/:environment_name/variables', { - org: this.repo.owner, - repo: this.repo.repo, - environment_name: environment.name - })).data.variables.map(variable => ({name: variable.name.toLowerCase(), value: variable.value})), - deployment_protection_rules: (await this.github.request('GET /repos/:org/:repo/environments/:environment_name/deployment_protection_rules', { - org: this.repo.owner, - repo: this.repo.repo, - environment_name: environment.name - })).data.custom_deployment_protection_rules.map(rule => ({ - app_id: rule.app.id, - id: rule.id - })) - } - environments_mapped.push(mapped); - //console.log(mapped); - } + getChanged (existing, attrs) { + if (!attrs.wait_timer) attrs.wait_timer = 0 + if (!attrs.prevent_self_review) attrs.prevent_self_review = false + if (!attrs.reviewers) attrs.reviewers = [] + if (!attrs.deployment_branch_policy) attrs.deployment_branch_policy = null + if (!attrs.variables) attrs.variables = [] + if (!attrs.deployment_protection_rules) attrs.deployment_protection_rules = [] - return environments_mapped; - } + const wait_timer = existing.wait_timer !== attrs.wait_timer + const prevent_self_review = existing.prevent_self_review !== attrs.prevent_self_review + const reviewers = JSON.stringify(existing.reviewers.sort((x1, x2) => x1.id - x2.id)) !== JSON.stringify(attrs.reviewers.sort((x1, x2) => x1.id - x2.id)) - comparator(existing, attrs) { - return existing.name === attrs.name + let existing_custom_branch_policies = existing.deployment_branch_policy === null ? null : existing.deployment_branch_policy.custom_branch_policies + if (typeof (existing_custom_branch_policies) === 'object' && existing_custom_branch_policies !== null) { + existing_custom_branch_policies = existing_custom_branch_policies.sort() } - - getChanged(existing, attrs) { - if (!attrs.wait_timer) attrs.wait_timer = 0; - if (!attrs.prevent_self_review) attrs.prevent_self_review = false; - if (!attrs.reviewers) attrs.reviewers = []; - if (!attrs.deployment_branch_policy) attrs.deployment_branch_policy = null; - if(!attrs.variables) attrs.variables = []; - if(!attrs.deployment_protection_rules) attrs.deployment_protection_rules = []; - - const wait_timer = existing.wait_timer !== attrs.wait_timer; - const prevent_self_review = existing.prevent_self_review !== attrs.prevent_self_review; - const reviewers = JSON.stringify(existing.reviewers.sort((x1, x2) => x1.id - x2.id)) !== JSON.stringify(attrs.reviewers.sort((x1, x2) => x1.id - x2.id)); - - let existing_custom_branch_policies = existing.deployment_branch_policy === null ? null : existing.deployment_branch_policy.custom_branch_policies; - if(typeof(existing_custom_branch_policies) === 'object' && existing_custom_branch_policies !== null) { - existing_custom_branch_policies = existing_custom_branch_policies.sort(); - } - let attrs_custom_branch_policies = attrs.deployment_branch_policy === null ? null : attrs.deployment_branch_policy.custom_branch_policies; - if(typeof(attrs_custom_branch_policies) === 'object' && attrs_custom_branch_policies !== null) { - attrs_custom_branch_policies = attrs_custom_branch_policies.sort(); - } - let deployment_branch_policy; - if(existing.deployment_branch_policy === attrs.deployment_branch_policy) { - deployment_branch_policy = false; - } - else { - deployment_branch_policy = ( - (existing.deployment_branch_policy === null && attrs.deployment_branch_policy !== null) || + let attrs_custom_branch_policies = attrs.deployment_branch_policy === null ? null : attrs.deployment_branch_policy.custom_branch_policies + if (typeof (attrs_custom_branch_policies) === 'object' && attrs_custom_branch_policies !== null) { + attrs_custom_branch_policies = attrs_custom_branch_policies.sort() + } + let deployment_branch_policy + if (existing.deployment_branch_policy === attrs.deployment_branch_policy) { + deployment_branch_policy = false + } else { + deployment_branch_policy = ( + (existing.deployment_branch_policy === null && attrs.deployment_branch_policy !== null) || (existing.deployment_branch_policy !== null && attrs.deployment_branch_policy === null) || (existing.deployment_branch_policy.protected_branches !== attrs.deployment_branch_policy.protected_branches) || (JSON.stringify(existing_custom_branch_policies) !== JSON.stringify(attrs_custom_branch_policies)) - ); - } - - const variables = JSON.stringify(existing.variables.sort((x1, x2) => x1.name - x2.name)) !== JSON.stringify(attrs.variables.sort((x1, x2) => x1.name - x2.name)); - const deployment_protection_rules = JSON.stringify(existing.deployment_protection_rules.map(x => ({app_id: x.app_id})).sort((x1, x2) => x1.app_id - x2.app_id)) !== JSON.stringify(attrs.deployment_protection_rules.map(x => ({app_id: x.app_id})).sort((x1, x2) => x1.app_id - x2.app_id)); + ) + } - return {wait_timer, prevent_self_review, reviewers, deployment_branch_policy, variables, deployment_protection_rules}; + const variables = JSON.stringify(existing.variables.sort((x1, x2) => x1.name - x2.name)) !== JSON.stringify(attrs.variables.sort((x1, x2) => x1.name - x2.name)) + const deployment_protection_rules = JSON.stringify(existing.deployment_protection_rules.map(x => ({ app_id: x.app_id })).sort((x1, x2) => x1.app_id - x2.app_id)) !== JSON.stringify(attrs.deployment_protection_rules.map(x => ({ app_id: x.app_id })).sort((x1, x2) => x1.app_id - x2.app_id)) + + return { wait_timer, prevent_self_review, reviewers, deployment_branch_policy, variables, deployment_protection_rules } + } + + changed (existing, attrs) { + const { wait_timer, prevent_self_review, reviewers, deployment_branch_policy, variables, deployment_protection_rules } = this.getChanged(existing, attrs) + + return wait_timer || prevent_self_review || reviewers || deployment_branch_policy || variables || deployment_protection_rules + } + + async update (existing, attrs) { + const { wait_timer, prevent_self_review, reviewers, deployment_branch_policy, variables, deployment_protection_rules } = this.getChanged(existing, attrs) + + if (wait_timer || prevent_self_review || reviewers || deployment_branch_policy) { + await this.github.request('PUT /repos/:org/:repo/environments/:environment_name', { + org: this.repo.owner, + repo: this.repo.repo, + environment_name: attrs.name, + wait_timer: attrs.wait_timer, + prevent_self_review: attrs.prevent_self_review, + reviewers: attrs.reviewers, + deployment_branch_policy: attrs.deployment_branch_policy === null + ? null + : { + protected_branches: attrs.deployment_branch_policy.protected_branches, + custom_branch_policies: !!attrs.deployment_branch_policy.custom_branch_policies + } + }) } - changed(existing, attrs) { - const {wait_timer, prevent_self_review, reviewers, deployment_branch_policy, variables, deployment_protection_rules} = this.getChanged(existing, attrs); + if (deployment_branch_policy && attrs.deployment_branch_policy && attrs.deployment_branch_policy.custom_branch_policies) { + const existingPolicies = (await this.github.request('GET /repos/:org/:repo/environments/:environment_name/deployment-branch-policies', { + org: this.repo.owner, + repo: this.repo.repo, + environment_name: attrs.name + })).data.branch_policies + + for (const policy of existingPolicies) { + await this.github.request('DELETE /repos/:org/:repo/environments/:environment_name/deployment-branch-policies/:branch_policy_id', { + org: this.repo.owner, + repo: this.repo.repo, + environment_name: attrs.name, + branch_policy_id: policy.id + }) + } - return wait_timer || prevent_self_review || reviewers || deployment_branch_policy || variables || deployment_protection_rules; + for (const policy of attrs.deployment_branch_policy.custom_branch_policies) { + await this.github.request('POST /repos/:org/:repo/environments/:environment_name/deployment-branch-policies', { + org: this.repo.owner, + repo: this.repo.repo, + environment_name: attrs.name, + name: policy + }) + } } - async update(existing, attrs) { - const {wait_timer, prevent_self_review, reviewers, deployment_branch_policy, variables, deployment_protection_rules} = this.getChanged(existing, attrs); - - if(wait_timer || prevent_self_review || reviewers || deployment_branch_policy) { - await this.github.request(`PUT /repos/:org/:repo/environments/:environment_name`, { - org: this.repo.owner, - repo: this.repo.repo, - environment_name: attrs.name, - wait_timer: attrs.wait_timer, - prevent_self_review: attrs.prevent_self_review, - reviewers: attrs.reviewers, - deployment_branch_policy: attrs.deployment_branch_policy === null ? null : { - protected_branches: attrs.deployment_branch_policy.protected_branches, - custom_branch_policies: !!attrs.deployment_branch_policy.custom_branch_policies - } + if (variables) { + let existingVariables = [...existing.variables] + + for (const variable of attrs.variables) { + const existingVariable = existingVariables.find((_var) => _var.name === variable.name) + if (existingVariable) { + existingVariables = existingVariables.filter(_var => _var.name !== variable.name) + if (existingVariable.value !== variable.value) { + await this.github.request('PATCH /repos/:org/:repo/environments/:environment_name/variables/:variable_name', { + org: this.repo.owner, + repo: this.repo.repo, + environment_name: attrs.name, + variable_name: variable.name, + value: variable.value }) + } + } else { + await this.github.request('POST /repos/:org/:repo/environments/:environment_name/variables', { + org: this.repo.owner, + repo: this.repo.repo, + environment_name: attrs.name, + name: variable.name, + value: variable.value + }) } + } - if(deployment_branch_policy && attrs.deployment_branch_policy && attrs.deployment_branch_policy.custom_branch_policies) { - const existingPolicies = (await this.github.request('GET /repos/:org/:repo/environments/:environment_name/deployment-branch-policies', { - org: this.repo.owner, - repo: this.repo.repo, - environment_name: attrs.name - })).data.branch_policies; - - for(let policy of existingPolicies) { - await this.github.request('DELETE /repos/:org/:repo/environments/:environment_name/deployment-branch-policies/:branch_policy_id', { - org: this.repo.owner, - repo: this.repo.repo, - environment_name: attrs.name, - branch_policy_id: policy.id - }); - } - - for(let policy of attrs.deployment_branch_policy.custom_branch_policies) { - await this.github.request('POST /repos/:org/:repo/environments/:environment_name/deployment-branch-policies', { - org: this.repo.owner, - repo: this.repo.repo, - environment_name: attrs.name, - name: policy - }); - } - } - - if(variables) { - let existingVariables = [...existing.variables]; - - for(let variable of attrs.variables) { - const existingVariable = existingVariables.find((_var) => _var.name === variable.name); - if(existingVariable) { - existingVariables = existingVariables.filter(_var => _var.name !== variable.name); - if(existingVariable.value !== variable.value) { - await this.github.request(`PATCH /repos/:org/:repo/environments/:environment_name/variables/:variable_name`, { - org: this.repo.owner, - repo: this.repo.repo, - environment_name: attrs.name, - variable_name: variable.name, - value: variable.value - }); - } - } - else { - await this.github.request(`POST /repos/:org/:repo/environments/:environment_name/variables`, { - org: this.repo.owner, - repo: this.repo.repo, - environment_name: attrs.name, - name: variable.name, - value: variable.value - }); - } - } - - for(let variable of existingVariables) { - await this.github.request('DELETE /repos/:org/:repo/environments/:environment_name/variables/:variable_name', { - org: this.repo.owner, - repo: this.repo.repo, - environment_name: attrs.name, - variable_name: variable.name - }); - } - } - - if(deployment_protection_rules) { - let existingRules = [...existing.deployment_protection_rules]; + for (const variable of existingVariables) { + await this.github.request('DELETE /repos/:org/:repo/environments/:environment_name/variables/:variable_name', { + org: this.repo.owner, + repo: this.repo.repo, + environment_name: attrs.name, + variable_name: variable.name + }) + } + } - for(let rule of attrs.deployment_protection_rules) { - const existingRule = existingRules.find((_rule) => _rule.id === rule.id); + if (deployment_protection_rules) { + const existingRules = [...existing.deployment_protection_rules] - if(!existingRule) { - await this.github.request(`POST /repos/:org/:repo/environments/:environment_name/deployment_protection_rules`, { - org: this.repo.owner, - repo: this.repo.repo, - environment_name: attrs.name, - integration_id: rule.app_id - }); - } - } + for (const rule of attrs.deployment_protection_rules) { + const existingRule = existingRules.find((_rule) => _rule.id === rule.id) - for(let rule of existingRules) { - await this.github.request('DELETE /repos/:org/:repo/environments/:environment_name/deployment_protection_rules/:rule_id', { - org: this.repo.owner, - repo: this.repo.repo, - environment_name: attrs.name, - rule_id: rule.id - }); - } - } - } - - async add(attrs) { - await this.github.request(`PUT /repos/:org/:repo/environments/:environment_name`, { + if (!existingRule) { + await this.github.request('POST /repos/:org/:repo/environments/:environment_name/deployment_protection_rules', { org: this.repo.owner, repo: this.repo.repo, environment_name: attrs.name, - wait_timer: attrs.wait_timer, - prevent_self_review: attrs.prevent_self_review, - reviewers: attrs.reviewers, - deployment_branch_policy: attrs.deployment_branch_policy == null ? null : { - protected_branches: !!attrs.deployment_branch_policy.protected_branches, - custom_branch_policies: !!attrs.deployment_branch_policy.custom_branch_policies - } - }); - - if(attrs.deployment_branch_policy && attrs.deployment_branch_policy.custom_branch_policies) { - - for(let policy of attrs.deployment_branch_policy.custom_branch_policies) { - await this.github.request('POST /repos/:org/:repo/environments/:environment_name/deployment-branch-policies', { - org: this.repo.owner, - repo: this.repo.repo, - environment_name: attrs.name, - name: policy.name - }); - } - + integration_id: rule.app_id + }) } + } - if(attrs.variables) { - - for(let variable of attrs.variables) { - await this.github.request(`POST /repos/:org/:repo/environments/:environment_name/variables`, { - org: this.repo.owner, - repo: this.repo.repo, - environment_name: attrs.name, - name: variable.name, - value: variable.value - }); - } - + for (const rule of existingRules) { + await this.github.request('DELETE /repos/:org/:repo/environments/:environment_name/deployment_protection_rules/:rule_id', { + org: this.repo.owner, + repo: this.repo.repo, + environment_name: attrs.name, + rule_id: rule.id + }) + } + } + } + + async add (attrs) { + await this.github.request('PUT /repos/:org/:repo/environments/:environment_name', { + org: this.repo.owner, + repo: this.repo.repo, + environment_name: attrs.name, + wait_timer: attrs.wait_timer, + prevent_self_review: attrs.prevent_self_review, + reviewers: attrs.reviewers, + deployment_branch_policy: attrs.deployment_branch_policy == null + ? null + : { + protected_branches: !!attrs.deployment_branch_policy.protected_branches, + custom_branch_policies: !!attrs.deployment_branch_policy.custom_branch_policies } + }) + + if (attrs.deployment_branch_policy && attrs.deployment_branch_policy.custom_branch_policies) { + for (const policy of attrs.deployment_branch_policy.custom_branch_policies) { + await this.github.request('POST /repos/:org/:repo/environments/:environment_name/deployment-branch-policies', { + org: this.repo.owner, + repo: this.repo.repo, + environment_name: attrs.name, + name: policy.name + }) + } + } - if(attrs.deployment_protection_rules) { + if (attrs.variables) { + for (const variable of attrs.variables) { + await this.github.request('POST /repos/:org/:repo/environments/:environment_name/variables', { + org: this.repo.owner, + repo: this.repo.repo, + environment_name: attrs.name, + name: variable.name, + value: variable.value + }) + } + } - for(let rule of attrs.deployment_protection_rules) { - await this.github.request(`POST /repos/:org/:repo/environments/:environment_name/deployment_protection_rules`, { - org: this.repo.owner, - repo: this.repo.repo, - environment_name: attrs.name, - integration_id: rule.app_id - }); + if (attrs.deployment_protection_rules) { + for (const rule of attrs.deployment_protection_rules) { + await this.github.request('POST /repos/:org/:repo/environments/:environment_name/deployment_protection_rules', { + org: this.repo.owner, + repo: this.repo.repo, + environment_name: attrs.name, + integration_id: rule.app_id + }) + } + } + } + + async remove (existing) { + await this.github.request('DELETE /repos/:org/:repo/environments/:environment_name', { + org: this.repo.owner, + repo: this.repo.repo, + environment_name: existing.name + }) + } + + sync () { + const resArray = [] + if (this.entries) { + const filteredEntries = this.filterEntries() + return this.find().then(existingRecords => { + // Remove any null or undefined values from the diffables (usually comes from repo override) + for (const entry of filteredEntries) { + for (const key of Object.keys(entry)) { + if (entry[key] === null || entry[key] === undefined) { + delete entry[key] } - + } } - } + // For environments, we want to keep the entries with only name defined. - async remove(existing) { - await this.github.request(`DELETE /repos/:org/:repo/environments/:environment_name`, { - org: this.repo.owner, - repo: this.repo.repo, - environment_name: existing.name - }); - } + const changes = [] - sync () { - const resArray = [] - if (this.entries) { - let filteredEntries = this.filterEntries() - return this.find().then(existingRecords => { - - // Remove any null or undefined values from the diffables (usually comes from repo override) - for (const entry of filteredEntries) { - for (const key of Object.keys(entry)) { - if (entry[key] === null || entry[key] === undefined) { - delete entry[key] + existingRecords.forEach(x => { + if (!filteredEntries.find(y => this.comparator(x, y))) { + const change = this.remove(x).then(res => { + if (this.nop) { + return resArray.push(res) } - } + return res + }) + changes.push(change) } - // For environments, we want to keep the entries with only name defined. - - const changes = [] - - existingRecords.forEach(x => { - if (!filteredEntries.find(y => this.comparator(x, y))) { - const change = this.remove(x).then(res => { - if (this.nop) { - return resArray.push(res) - } - return res - }) - changes.push(change) - } + }) + + filteredEntries.forEach(attrs => { + const existing = existingRecords.find(record => { + return this.comparator(record, attrs) }) - filteredEntries.forEach(attrs => { - const existing = existingRecords.find(record => { - return this.comparator(record, attrs) + if (!existing) { + const change = this.add(attrs).then(res => { + if (this.nop) { + return resArray.push(res) + } + return res }) + changes.push(change) + } else if (this.changed(existing, attrs)) { + const change = this.update(existing, attrs).then(res => { + if (this.nop) { + return resArray.push(res) + } + return res + }) + changes.push(change) + } + }) - if (!existing) { - const change = this.add(attrs).then(res => { - if (this.nop) { - return resArray.push(res) - } - return res - }) - changes.push(change) - } else if (this.changed(existing, attrs)) { - const change = this.update(existing, attrs).then(res => { - if (this.nop) { - return resArray.push(res) - } - return res - }) - changes.push(change) - } - }) - - if (this.nop) { + if (this.nop) { + return Promise.resolve(resArray) + } + return Promise.all(changes) + }).catch(e => { + if (this.nop) { + if (e.status === 404) { + // Ignore 404s which can happen in dry-run as the repo may not exist. return Promise.resolve(resArray) - } - return Promise.all(changes) - }).catch(e => { - if (this.nop) { - if (e.status === 404) { - // Ignore 404s which can happen in dry-run as the repo may not exist. - return Promise.resolve(resArray) - } else { - resArray.push(new NopCommand(this.constructor.name, this.repo, null, `error ${e} in ${this.constructor.name} for repo: ${JSON.stringify(this.repo)} entries ${JSON.stringify(this.entries)}`, 'ERROR')) - return Promise.resolve(resArray) - } } else { - this.logError(`Error ${e} in ${this.constructor.name} for repo: ${JSON.stringify(this.repo)} entries ${JSON.stringify(this.entries)}`) + resArray.push(new NopCommand(this.constructor.name, this.repo, null, `error ${e} in ${this.constructor.name} for repo: ${JSON.stringify(this.repo)} entries ${JSON.stringify(this.entries)}`, 'ERROR')) + return Promise.resolve(resArray) } - }) - } + } else { + this.logError(`Error ${e} in ${this.constructor.name} for repo: ${JSON.stringify(this.repo)} entries ${JSON.stringify(this.entries)}`) + } + }) } - + } } diff --git a/lib/plugins/labels.js b/lib/plugins/labels.js index 49558cc0..5086cf55 100644 --- a/lib/plugins/labels.js +++ b/lib/plugins/labels.js @@ -3,19 +3,19 @@ const NopCommand = require('../nopcommand') const previewHeaders = { accept: 'application/vnd.github.symmetra-preview+json' } module.exports = class Labels extends Diffable { - constructor (nop, github, repo, entries, log, errors) { + constructor (nop, github, repo, entries, deploymentConfig, log, errors) { const { include, exclude = [] } = entries if (Array.isArray(include)) { // Config is object with include array // labels: // include: // - name: label - super(nop, github, repo, include, log, errors) + super(nop, github, repo, include, deploymentConfig, log, errors) } else { // Config is array // labels: // - name: label - super(nop, github, repo, entries, log, errors) + super(nop, github, repo, entries, deploymentConfig, log, errors) } this.exclude = exclude.filter(item => item?.name).map(item => new RegExp(item.name)) diff --git a/lib/plugins/repository.js b/lib/plugins/repository.js index 42bbd28e..371d25a1 100644 --- a/lib/plugins/repository.js +++ b/lib/plugins/repository.js @@ -40,13 +40,13 @@ const ignorableFields = [ ] module.exports = class Repository extends ErrorStash { - constructor (nop, github, repo, settings, installationId, log, errors) { + constructor (nop, github, repo, settings, deploymentSettings, log, errors) { super(errors) - this.installationId = installationId this.github = github - this.settings = Object.assign({ mediaType: { previews: ['nebula-preview'] } }, settings, repo) - this.topics = this.settings.topics - this.security = this.settings.security + this.settings = settings + this.deploymentSettings = deploymentSettings + this.topics = settings.topics + this.security = settings.security this.repo = repo this.log = log this.nop = nop @@ -57,28 +57,28 @@ module.exports = class Repository extends ErrorStash { delete this.settings.template } - sync () { + async sync () { const resArray = [] - this.log.debug(`Syncing Repo ${this.settings.name}`) - this.settings.name = this.settings.name || this.settings.repo + + const mergeDeep = new MergeDeep(this.log, this.github, ignorableFields, this.deploymentSettings) + + this.log.debug(`Syncing Repo ${this.repo.owner}/${this.repo.repo}`) // let hasChanges = false // let hasTopicChanges = false - return this.github.repos.get(this.repo) + await this.github.repos.get(this.repo) .then(resp => { - const mergeDeep = new MergeDeep(this.log, this.github, ignorableFields) - const changes = mergeDeep.compareDeep(resp.data, this.settings) // hasChanges = changes.additions.length > 0 || changes.modifications.length > 0 const topicChanges = mergeDeep.compareDeep({ entries: resp.data.topics }, { entries: this.topics }) // hasTopicChanges = topicChanges.additions.length > 0 || topicChanges.modifications.length > 0 - //const results = JSON.stringify(changes, null, 2) + // const results = JSON.stringify(changes, null, 2) const results = { msg: `${this.constructor.name} settings changes`, additions: changes.additions, modifications: changes.modifications, deletions: changes.deletions } - this.log.debug(`Result of comparing repo for changes = ${results}`) + this.log.debug(`Result of comparing repo for changes = ${JSON.stringify(results, null, 2)}`) - //const topicResults = JSON.stringify(topicChanges, null, 2) + // const topicResults = JSON.stringify(topicChanges, null, 2) const topicResults = { msg: `${this.constructor.name} settings changes for topics`, additions: topicChanges.additions, modifications: topicChanges.modifications, deletions: topicChanges.deletions } this.log.debug(`Result of comparing topics for changes source ${JSON.stringify(resp.data.topics)} target ${JSON.stringify(this.topics)} = ${topicResults}`) @@ -91,7 +91,7 @@ module.exports = class Repository extends ErrorStash { const promises = [] if (topicChanges.hasChanges) { promises.push(this.updatetopics(resp.data, resArray)) - } + } if (changes.hasChanges) { this.log.debug('There are repo changes') let updateDefaultBranchPromise = Promise.resolve() @@ -153,6 +153,14 @@ module.exports = class Repository extends ErrorStash { this.logError(` Error ${JSON.stringify(e)}`) } }) + + this.log.debug(`Confirming synchronization of repository for ${this.repo.owner}/${this.repo.repo}`) + const { data: upstream } = await this.github.repos.get(this.repo) + + const changes = mergeDeep.compareDeep(upstream, this.settings) + + if (changes.hasChanges) throw new Error(`Synchronization of repository for ${this.repo.owner}/${this.repo.repo} failed: ${JSON.stringify(changes)}`) + this.log.debug(`Confirmed synchronization of repository for ${this.repo.owner}/${this.repo.repo}`) } updateDefaultBranch (oldname, newname, resArray) { @@ -189,7 +197,7 @@ module.exports = class Repository extends ErrorStash { }) } - renameBranch(oldname, newname, resArray) { + renameBranch (oldname, newname, resArray) { this.log.error(`Branch ${newname} does not exist. So renaming the current default branch ${oldname} to ${newname}`) const parms = { owner: this.settings.owner, @@ -205,19 +213,22 @@ module.exports = class Repository extends ErrorStash { } } - updaterepo (resArray) { - this.log.debug(`Updating repo with settings ${JSON.stringify(this.topics)} ${JSON.stringify(this.settings)}`) + async updaterepo (resArray) { + this.log.debug(`Updating repo with settings: ${JSON.stringify(this.settings)}`) if (this.nop) { resArray.push(new NopCommand(this.constructor.name, this.repo, this.github.repos.update.endpoint(this.settings), 'Update Repo')) return Promise.resolve(resArray) } - return this.github.repos.update(this.settings) + await this.github.repos.update({ + ...this.repo, + ...this.settings + }) } updatetopics (repoData, resArray) { const parms = { - owner: this.settings.owner, - repo: this.settings.repo, + owner: this.repo.owner, + repo: this.repo.repo, names: this.topics, mediaType: { previews: ['mercy'] @@ -227,12 +238,12 @@ module.exports = class Repository extends ErrorStash { if (this.topics) { // if (repoData.data?.topics.length !== this.topics.length || // !repoData.data?.topics.every(t => this.topics.includes(t))) { - this.log.debug(`Updating repo with topics ${this.topics.join(',')}`) - if (this.nop) { - resArray.push((new NopCommand(this.constructor.name, this.repo, this.github.repos.replaceAllTopics.endpoint(parms), 'Update Topics'))) - return Promise.resolve(resArray) - } - return this.github.repos.replaceAllTopics(parms) + this.log.debug(`Updating repo with topics ${this.topics.join(',')}`) + if (this.nop) { + resArray.push((new NopCommand(this.constructor.name, this.repo, this.github.repos.replaceAllTopics.endpoint(parms), 'Update Topics'))) + return Promise.resolve(resArray) + } + return this.github.repos.replaceAllTopics(parms) // } else { // this.log.debug(`no need to update topics for ${repoData.data.name}`) // if (this.nop) { @@ -277,7 +288,7 @@ module.exports = class Repository extends ErrorStash { } else { this.log.debug(`no need to update security for ${repoData.name}`) if (this.nop) { - //resArray.push((new NopCommand(this.constructor.name, this.repo, null, `no need to update security for ${repoData.name}`))) + // resArray.push((new NopCommand(this.constructor.name, this.repo, null, `no need to update security for ${repoData.name}`))) return Promise.resolve([]) } } diff --git a/lib/plugins/rulesets.js b/lib/plugins/rulesets.js index 6846e1ce..a3724367 100644 --- a/lib/plugins/rulesets.js +++ b/lib/plugins/rulesets.js @@ -7,8 +7,8 @@ const version = { 'X-GitHub-Api-Version': '2022-11-28' } module.exports = class Rulesets extends Diffable { - constructor (nop, github, repo, entries, log, errors, scope) { - super(nop, github, repo, entries, log, errors) + constructor (nop, github, repo, entries, deploymentConfig, log, errors, scope) { + super(nop, github, repo, entries, deploymentConfig, log, errors) this.github = github this.repo = repo this.rulesets = entries diff --git a/lib/plugins/validator.js b/lib/plugins/validator.js index 45f04d60..d9c90ebe 100644 --- a/lib/plugins/validator.js +++ b/lib/plugins/validator.js @@ -1,6 +1,6 @@ const NopCommand = require('../nopcommand') module.exports = class Validator { - constructor (nop, github, repo, settings, log) { + constructor (nop, github, repo, settings, deploymentConfig, log) { this.github = github this.pattern = settings.pattern // this.regex = /[a-zA-Z0-9_-]+_\w[a-zA-Z0-9_-]+.*/gm diff --git a/lib/settings.js b/lib/settings.js index 961aa4c6..6d337798 100644 --- a/lib/settings.js +++ b/lib/settings.js @@ -6,6 +6,10 @@ const Glob = require('./glob') const NopCommand = require('./nopcommand') const MergeDeep = require('./mergeDeep') const env = require('./env') +const DeploymentConfig = require('./deploymentConfig.js') +const ConfigManager = require('./configManager.js') +const jsonPointer = require('json-pointer') +const lodashSet = require('lodash.set') const CONFIG_PATH = env.CONFIG_PATH const eta = new Eta({ views: path.join(__dirname) }) const SCOPE = { ORG: 'org', REPO: 'repo' } // Determine if the setting is a org setting or repo setting @@ -73,26 +77,8 @@ class Settings { this.log = context.log this.results = [] this.errors = [] - this.configvalidators = {} - this.overridevalidators = {} - const overridevalidators = config.overridevalidators - if (this.isIterable(overridevalidators)) { - for (const validator of overridevalidators) { - // eslint-disable-next-line no-new-func - const f = new Function('baseconfig', 'overrideconfig', 'githubContext', validator.script) - this.overridevalidators[validator.plugin] = { canOverride: f, error: validator.error } - } - } - const configvalidators = config.configvalidators - if (this.isIterable(configvalidators)) { - for (const validator of configvalidators) { - this.log.debug(`Logging each script: ${typeof validator.script}`) - // eslint-disable-next-line no-new-func - const f = new Function('baseconfig', 'githubContext', validator.script) - this.configvalidators[validator.plugin] = { isValid: f, error: validator.error } - } - } - this.mergeDeep = new MergeDeep(this.log, this.github, [], this.configvalidators, this.overridevalidators) + this.settings = new DeploymentConfig() + this.mergeDeep = new MergeDeep(this.log, this.github, [], this.settings) } // Create a check in the Admin repo for safe-settings. @@ -105,7 +91,7 @@ class Settings { if (this.errors.length > 0) { conclusion = 'failure' summary = 'Safe-Settings finished with errors.' - details = await eta.renderString(errorTemplate, this.errors) + details = eta.renderString(errorTemplate, this.errors) } // Use the latest commit to create the check against @@ -304,59 +290,32 @@ ${this.results.reduce((x, y) => { this.log.debug(`Process normally... Not a SubOrg config change or SubOrg config was changed and this repo is part of it. ${JSON.stringify(repo)} suborg config ${JSON.stringify(this.subOrgConfigMap)}`) - if (subOrgConfig) { - let suborgRepoConfig = subOrgConfig.repository - if (suborgRepoConfig) { - suborgRepoConfig = Object.assign(suborgRepoConfig, { name: repo.repo, org: repo.owner }) - repoConfig = this.mergeDeep.mergeDeep({}, repoConfig, suborgRepoConfig) - } - } + try { + const pluginConfigs = await this.childPluginsList(repo) + this.log.debug(`Plugin Configs: ${JSON.stringify(pluginConfigs, null, 2)}`) - // Overlay repo config - // RepoConfigs should be preloaded but checking anyway - const overrideRepoConfig = this.repoConfigs[`${repo.repo}.yml`]?.repository - if (overrideRepoConfig) { - repoConfig = this.mergeDeep.mergeDeep({}, repoConfig, overrideRepoConfig) - } - if (repoConfig) { - try { - this.log.debug(`found a matching repoconfig for this repo ${JSON.stringify(repoConfig)}`) - const childPlugins = this.childPluginsList(repo) - const RepoPlugin = Settings.PLUGINS.repository - return new RepoPlugin(this.nop, this.github, repo, repoConfig, this.installation_id, this.log, this.errors).sync().then(res => { - this.appendToResults(res) - return Promise.all( - childPlugins.map(([Plugin, config]) => { - return new Plugin(this.nop, this.github, repo, config, this.log, this.errors).sync() - })) - }).then(res => { - this.appendToResults(res) - }) - } catch (e) { - if (this.nop) { - const nopcommand = new NopCommand(this.constructor.name, this.repo, null, `${e}`, 'ERROR') - this.log.error(`NOPCOMMAND ${JSON.stringify(nopcommand)}`) - this.appendToResults([nopcommand]) - // throw e - } else { - throw e - } + const plugins = await Promise.all(pluginConfigs.map(([Plugin, config]) => + new Plugin(this.nop, this.github, repo, config, this.settings, this.log, this.errors).sync() + )) + + this.appendToResults(plugins) + } catch (e) { + if (this.nop) { + const nopcommand = new NopCommand(this.constructor.name, this.repo, null, `${e}`, 'ERROR') + this.log.error(`NOPCOMMAND ${JSON.stringify(nopcommand)}`) + this.appendToResults([nopcommand]) + // throw e + } else { + throw e } - } else { - this.log.debug(`Didnt find any a matching repoconfig for this repo ${JSON.stringify(repo)} in ${JSON.stringify(this.repoConfigs)}`) - const childPlugins = this.childPluginsList(repo) - return Promise.all(childPlugins.map(([Plugin, config]) => { - return new Plugin(this.nop, this.github, repo, config, this.log, this.errors).sync().then(res => { - this.appendToResults(res) - }) - })) } } async updateAll () { // this.subOrgConfigs = this.subOrgConfigs || await this.getSubOrgConfigs(this.github, this.repo, this.log) // this.repoConfigs = this.repoConfigs || await this.getRepoConfigs(this.github, this.repo, this.log) - return this.eachRepositoryRepos(this.github, this.config.restrictedRepos, this.log).then(res => { + return this.eachRepositoryRepos(this.github, this.settings.restrictedRepos).then(res => { + this.log.debug(res) this.appendToResults(res) }) } @@ -366,26 +325,61 @@ ${this.results.reduce((x, y) => { for (const k of Object.keys(this.subOrgConfigs)) { const repoPattern = new Glob(k) if (repoName.search(repoPattern) >= 0) { - return this.subOrgConfigs[k] + const subOrgConfig = this.subOrgConfigs[k] + // Coerce 'repositories' to 'repository' + subOrgConfig.repository = subOrgConfig.repositories + delete subOrgConfig.repositories + return subOrgConfig } } } return undefined } + async getUnsafeRepoConfig (repo) { + const unsafeSettings = await ConfigManager.loadYaml(this.github, { + ...repo, + path: '.github/settings.yml' + }) + + const { unsafeFields } = this.settings + + const result = {} + for (const unsafeField of unsafeFields) { + let value + try { + value = jsonPointer.get(unsafeSettings, unsafeField) + } catch {} + + if (value !== undefined) { + lodashSet(result, jsonPointer.parse(unsafeField), value) + } + } + + return result + } + // Remove Org specific configs from the repo config returnRepoSpecificConfigs (config) { const newConfig = Object.assign({}, config) // clone delete newConfig.rulesets + + // Coerce 'repositories' to 'repository' + newConfig.repository = newConfig.repositories + delete newConfig.repositories + return newConfig } - childPluginsList (repo) { + async childPluginsList (repo) { const repoName = repo.repo + const subOrgOverrideConfig = this.getSubOrgConfig(repoName) - this.log.debug(`suborg config for ${repoName} is ${JSON.stringify(subOrgOverrideConfig)}`) + this.log.debug(`suborg config for ${repoName} is ${JSON.stringify(subOrgOverrideConfig)}`) const repoOverrideConfig = this.repoConfigs[`${repoName}.yml`] || {} - const overrideConfig = this.mergeDeep.mergeDeep({}, this.returnRepoSpecificConfigs(this.config), subOrgOverrideConfig, repoOverrideConfig) + const unsafeRepoOverrideConfig = await this.getUnsafeRepoConfig(repo) + + const overrideConfig = this.mergeDeep.mergeDeep({}, this.returnRepoSpecificConfigs(this.config), subOrgOverrideConfig, repoOverrideConfig, unsafeRepoOverrideConfig) this.log.debug(`consolidated config is ${JSON.stringify(overrideConfig)}`) @@ -400,20 +394,19 @@ ${this.results.reduce((x, y) => { } else { this.validate(section, baseConfig, config) } - if (section !== 'repositories' && section !== 'repository') { - // Ignore any config that is not a plugin - if (section in Settings.PLUGINS) { - this.log.debug(`Found section ${section} in the config. Creating plugin...`) - const Plugin = Settings.PLUGINS[section] - childPlugins.push([Plugin, config]) - } + + // Ignore any config that is not a plugin + if (section in Settings.PLUGINS) { + this.log.debug(`Found section ${section} in the config. Creating plugin...`) + const Plugin = Settings.PLUGINS[section] + childPlugins.push([Plugin, config]) } } return childPlugins } validate (section, baseConfig, overrideConfig) { - const configValidator = this.configvalidators[section] + const configValidator = this.settings.configValidators[section] if (configValidator) { this.log.debug(`Calling configvalidator for key ${section} `) if (!configValidator.isValid(overrideConfig, this.github)) { @@ -421,7 +414,7 @@ ${this.results.reduce((x, y) => { throw new Error(configValidator.error) } } - const overridevalidator = this.overridevalidators[section] + const overridevalidator = this.settings.overrideValidators[section] if (overridevalidator) { this.log.debug(`Calling overridevalidator for key ${section} `) if (!overridevalidator.canOverride(baseConfig, overrideConfig, this.github)) { @@ -432,12 +425,12 @@ ${this.results.reduce((x, y) => { } isRestricted (repoName) { - const restrictedRepos = this.config.restrictedRepos + const restrictedRepos = this.settings.restrictedRepos // Skip configuring any restricted repos if (Array.isArray(restrictedRepos)) { // For backward compatibility support the old format if (restrictedRepos.includes(repoName)) { - this.log.debug(`Skipping retricted repo ${repoName}`) + this.log.debug(`Skipping restricted repo ${repoName}`) return true } else { this.log.debug(`${repoName} not in restricted repos ${restrictedRepos}`) @@ -467,19 +460,16 @@ ${this.results.reduce((x, y) => { return restrictedRepos.filter((restrictedRepo) => { return RegExp(restrictedRepo).test(repoName) }).length > 0 } - async eachRepositoryRepos (github, restrictedRepos, log) { - log.debug('Fetching repositories') - return github.paginate('GET /installation/repositories').then(repositories => { - return Promise.all(repositories.map(repository => { - if (this.isRestricted(repository.name)) { - return null - } + async eachRepositoryRepos (github, restrictedRepos) { + this.log.debug('Fetching repositories') + const repositories = await github.paginate('GET /installation/repositories') - const { owner, name } = repository - return this.updateRepos({ owner: owner.login, repo: name }) - }) - ) - }) + return Promise.all(repositories.map(repository => { + if (this.isRestricted(repository.name)) return null + + const { owner, name } = repository + return this.updateRepos({ owner: owner.login, repo: name }) + })) } /** @@ -689,6 +679,8 @@ ${this.results.reduce((x, y) => { const overridePaths = this.subOrgConfigMap || await this.getSubOrgConfigMap() const subOrgConfigs = {} + this.log.debug(`OverridePaths: ${JSON.stringify(overridePaths, null, 2)}`) + for (const override of overridePaths) { const data = await this.loadYaml(override.path) this.log.debug(`data = ${JSON.stringify(data)}`) @@ -713,6 +705,7 @@ ${this.results.reduce((x, y) => { }) }) } + this.log.debug(`data: ${JSON.stringify(data, null, 2)}`) if (data.suborgproperties) { const promises = data.suborgproperties.map((customProperty) => { return this.getReposForCustomProperty(customProperty) @@ -798,14 +791,13 @@ ${this.results.reduce((x, y) => { } async getReposForCustomProperty (customPropertyTuple) { - const name=Object.keys(customPropertyTuple)[0] + const name = Object.keys(customPropertyTuple)[0] let q = `props.${name}:${customPropertyTuple[name]}` q = encodeURIComponent(q) const options = this.github.request.endpoint((`/orgs/${this.repo.owner}/properties/values?repository_query=${q}`)) return this.github.paginate(options) } - isObject (item) { return (item && typeof item === 'object' && !Array.isArray(item)) } @@ -819,7 +811,7 @@ ${this.results.reduce((x, y) => { } } -function prettify(obj) { +function prettify (obj) { if (obj === null || obj === undefined) { return '' } diff --git a/package-lock.json b/package-lock.json index 9baba6f5..0c83d5d0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13,7 +13,9 @@ "deepmerge": "^4.3.1", "eta": "^3.0.3", "js-yaml": "^4.1.0", + "json-pointer": "^0.6.2", "lodash": "^4.17.21", + "lodash.set": "^4.3.2", "node-cron": "^3.0.2", "octokit": "^3.1.2", "probot": "^12.3.3" @@ -5143,6 +5145,11 @@ "is-callable": "^1.1.3" } }, + "node_modules/foreach": { + "version": "2.0.6", + "resolved": "https://registry.npmjs.org/foreach/-/foreach-2.0.6.tgz", + "integrity": "sha512-k6GAGDyqLe9JaebCsFCoudPPWfihKu8pylYXRlqP1J7ms39iPoTtk2fviNglIeQEwdh0bQeKJ01ZPyuyQvKzwg==" + }, "node_modules/form-data": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/form-data/-/form-data-4.0.0.tgz", @@ -7882,6 +7889,14 @@ "integrity": "sha512-xyFwyhro/JEof6Ghe2iz2NcXoj2sloNsWr/XsERDK/oiPCfaNhl5ONfp+jQdAZRQQ0IJWNzH9zIZF7li91kh2w==", "dev": true }, + "node_modules/json-pointer": { + "version": "0.6.2", + "resolved": "https://registry.npmjs.org/json-pointer/-/json-pointer-0.6.2.tgz", + "integrity": "sha512-vLWcKbOaXlO+jvRy4qNd+TI1QUPZzfJj1tpJ3vAXDych5XJf93ftpUKe5pKCrzyIIwgBJcOcCVRUfqQP25afBw==", + "dependencies": { + "foreach": "^2.0.4" + } + }, "node_modules/json-schema-traverse": { "version": "0.4.1", "resolved": "https://registry.npmjs.org/json-schema-traverse/-/json-schema-traverse-0.4.1.tgz", @@ -8195,6 +8210,11 @@ "resolved": "https://registry.npmjs.org/lodash.once/-/lodash.once-4.1.1.tgz", "integrity": "sha512-Sb487aTOCr9drQVL8pIxOzVhafOjZN9UU54hiN8PU3uAiSV7lx1yYNpbNmex2PK6dSJoNTSJUUswT651yww3Mg==" }, + "node_modules/lodash.set": { + "version": "4.3.2", + "resolved": "https://registry.npmjs.org/lodash.set/-/lodash.set-4.3.2.tgz", + "integrity": "sha512-4hNPN5jlm/N/HLMCO43v8BXKq9Z7QdAGc/VGrRD61w8gN9g/6jF9A4L1pbUgBLCffi0w9VsXfTOij5x8iTyFvg==" + }, "node_modules/loose-envify": { "version": "1.4.0", "resolved": "https://registry.npmjs.org/loose-envify/-/loose-envify-1.4.0.tgz", diff --git a/package.json b/package.json index dbad5350..ad45cfcf 100644 --- a/package.json +++ b/package.json @@ -27,7 +27,9 @@ "deepmerge": "^4.3.1", "eta": "^3.0.3", "js-yaml": "^4.1.0", + "json-pointer": "^0.6.2", "lodash": "^4.17.21", + "lodash.set": "^4.3.2", "node-cron": "^3.0.2", "octokit": "^3.1.2", "probot": "^12.3.3" diff --git a/test/unit/lib/mergeDeep.test.js b/test/unit/lib/mergeDeep.test.js index c114377e..354509e9 100644 --- a/test/unit/lib/mergeDeep.test.js +++ b/test/unit/lib/mergeDeep.test.js @@ -1,4 +1,5 @@ /* eslint-disable no-undef */ +const DeploymentConfig = require('../../../lib/deploymentConfig') const MergeDeep = require('../../../lib/mergeDeep') const YAML = require('js-yaml') const log = require('pino')('test.log') @@ -188,7 +189,8 @@ branches: const mergeDeep = new MergeDeep( log, mockReturnGitHubContext, - ignorableFields + ignorableFields, + new DeploymentConfig() ) const merged = mergeDeep.compareDeep(target, source) console.log(`source ${JSON.stringify(source, null, 2)}`) @@ -250,7 +252,8 @@ branches: const mergeDeep = new MergeDeep( log, mockReturnGitHubContext, - ignorableFields + ignorableFields, + new DeploymentConfig() ) const merged = mergeDeep.compareDeep(undefined, source) console.log(`diffs ${JSON.stringify(merged, null, 2)}`) @@ -299,7 +302,8 @@ branches: const mergeDeep = new MergeDeep( log, mockReturnGitHubContext, - ignorableFields + ignorableFields, + new DeploymentConfig() ) const merged = mergeDeep.compareDeep({}, source) console.log(`diffs ${JSON.stringify(merged, null, 2)}`) @@ -421,7 +425,8 @@ branches: const mergeDeep = new MergeDeep( log, mockReturnGitHubContext, - ignorableFields + ignorableFields, + new DeploymentConfig() ) const merged = mergeDeep.compareDeep(target, source) console.log(`source ${JSON.stringify(source, null, 2)}`) @@ -452,7 +457,8 @@ branches: const mergeDeep = new MergeDeep( log, mockReturnGitHubContext, - ignorableFields + ignorableFields, + new DeploymentConfig() ) const merged = mergeDeep.compareDeep(target, source) // console.log(`diffs ${JSON.stringify(merged, null, 2)}`) @@ -505,7 +511,8 @@ branches: const mergeDeep = new MergeDeep( log, mockReturnGitHubContext, - ignorableFields + ignorableFields, + new DeploymentConfig() ) const merged = mergeDeep.compareDeep(target, source) // console.log(`diffs ${JSON.stringify(merged, null, 2)}`) @@ -538,7 +545,8 @@ branches: const mergeDeep = new MergeDeep( log, mockReturnGitHubContext, - ignorableFields + ignorableFields, + new DeploymentConfig() ) const merged = mergeDeep.compareDeep(target, source) console.log(`diffs ${JSON.stringify(merged, null, 2)}`) @@ -571,7 +579,8 @@ branches: const mergeDeep = new MergeDeep( log, mockReturnGitHubContext, - ignorableFields + ignorableFields, + new DeploymentConfig() ) const merged = mergeDeep.compareDeep(target, source) console.log(`diffs ${JSON.stringify(merged, null, 2)}`) @@ -647,7 +656,8 @@ branches: const mergeDeep = new MergeDeep( log, mockReturnGitHubContext, - ignorableFields + ignorableFields, + new DeploymentConfig() ) const merged = mergeDeep.compareDeep(target, source) // console.log(`diffs ${JSON.stringify(merged, null, 2)}`) @@ -688,7 +698,8 @@ branches: const mergeDeep = new MergeDeep( log, mockReturnGitHubContext, - ignorableFields + ignorableFields, + new DeploymentConfig() ) const merged = mergeDeep.compareDeep(target, source) // console.log(`diffs ${JSON.stringify(merged, null, 2)}`) @@ -768,7 +779,8 @@ branches: const mergeDeep = new MergeDeep( log, mockReturnGitHubContext, - ignorableFields + ignorableFields, + new DeploymentConfig() ) const merged = mergeDeep.compareDeep(target, source) // console.log(`diffs ${JSON.stringify(merged, null, 2)}`) @@ -818,7 +830,8 @@ entries: const mergeDeep = new MergeDeep( log, mockReturnGitHubContext, - ignorableFields + ignorableFields, + new DeploymentConfig() ) const merged = mergeDeep.compareDeep(target, source) console.log(`diffs ${JSON.stringify(merged, null, 2)}`) @@ -866,7 +879,8 @@ entries: const mergeDeep = new MergeDeep( log, mockReturnGitHubContext, - ignorableFields + ignorableFields, + new DeploymentConfig() ) const merged = mergeDeep.compareDeep(target, source) console.log(`diffs ${JSON.stringify(merged, null, 2)}`) @@ -881,7 +895,8 @@ entries: const mergeDeep = new MergeDeep( log, mockReturnGitHubContext, - ignorableFields + ignorableFields, + new DeploymentConfig() ) const target = { teams: [ @@ -910,7 +925,8 @@ entries: const mergeDeep = new MergeDeep( log, mockReturnGitHubContext, - ignorableFields + ignorableFields, + new DeploymentConfig() ) const target = { teams: [ @@ -938,7 +954,8 @@ entries: const mergeDeep = new MergeDeep( log, mockReturnGitHubContext, - ignorableFields + ignorableFields, + new DeploymentConfig() ) const target = { required_pull_request_reviews: { @@ -972,7 +989,8 @@ entries: const mergeDeep = new MergeDeep( log, mockReturnGitHubContext, - ignorableFields + ignorableFields, + new DeploymentConfig() ) const target = { required_pull_request_reviews: { @@ -1006,7 +1024,8 @@ entries: const mergeDeep = new MergeDeep( log, mockReturnGitHubContext, - ignorableFields + ignorableFields, + new DeploymentConfig() ) const target = [ { username: 'collaborator-1' }, @@ -1029,7 +1048,8 @@ entries: const mergeDeep = new MergeDeep( log, mockReturnGitHubContext, - ignorableFields + ignorableFields, + new DeploymentConfig() ) const target = [] const source = [] @@ -1075,7 +1095,8 @@ entries: const mergeDeep = new MergeDeep( log, mockReturnGitHubContext, - ignorableFields + ignorableFields, + new DeploymentConfig() ) const merged = mergeDeep.compareDeep(target, source) console.log(`diffs ${JSON.stringify(merged, null, 2)}`) @@ -1305,7 +1326,8 @@ entries: const mergeDeep = new MergeDeep( log, mockReturnGitHubContext, - ignorableFields + ignorableFields, + new DeploymentConfig() ) const merged = mergeDeep.compareDeep(target, source) expect(merged.hasChanges).toBeFalsy() @@ -1358,7 +1380,8 @@ entries: const mergeDeep = new MergeDeep( log, mockReturnGitHubContext, - ignorableFields + ignorableFields, + new DeploymentConfig() ) const merged = mergeDeep.compareDeep(target, source) console.log(`source ${JSON.stringify(source, null, 2)}`) @@ -1448,7 +1471,8 @@ entries: const mergeDeep = new MergeDeep( log, mockReturnGitHubContext, - ignorableFields + ignorableFields, + new DeploymentConfig() ) const merged = mergeDeep.compareDeep(target, source) console.log(`source ${JSON.stringify(source, null, 2)}`) @@ -1604,7 +1628,8 @@ branches: const mergeDeep = new MergeDeep( log, mockReturnGitHubContext, - ignorableFields + ignorableFields, + new DeploymentConfig() ) const merged = mergeDeep.compareDeep(target, source) console.log(`source ${JSON.stringify(source, null, 2)}`) diff --git a/test/unit/lib/plugins/autolinks.test.js b/test/unit/lib/plugins/autolinks.test.js index 63d82927..d9d9d3aa 100644 --- a/test/unit/lib/plugins/autolinks.test.js +++ b/test/unit/lib/plugins/autolinks.test.js @@ -1,4 +1,5 @@ const Autolinks = require('../../../../lib/plugins/autolinks') +const DeploymentConfig = require('../../../../lib/deploymentConfig') describe('Autolinks', () => { const repo = { owner: 'owner', repo: 'repo' } @@ -8,7 +9,7 @@ describe('Autolinks', () => { const log = { debug: jest.fn(), error: console.error } const nop = false const errors = [] - return new Autolinks(nop, github, repo, config, log, errors) + return new Autolinks(nop, github, repo, config, new DeploymentConfig(), log, errors) } beforeEach(() => { diff --git a/test/unit/lib/plugins/branches.test.js b/test/unit/lib/plugins/branches.test.js index 68290ea4..0cfaa77f 100644 --- a/test/unit/lib/plugins/branches.test.js +++ b/test/unit/lib/plugins/branches.test.js @@ -2,6 +2,7 @@ const { when } = require('jest-when') const Branches = require('../../../../lib/plugins/branches') +const DeploymentConfig = require('../../../../lib/deploymentConfig') describe('Branches', () => { let github @@ -12,7 +13,7 @@ describe('Branches', () => { function configure (config) { const noop = false const errors = [] - return new Branches(noop, github, { owner: 'bkeepers', repo: 'test' }, config, log, errors) + return new Branches(noop, github, { owner: 'bkeepers', repo: 'test' }, config, new DeploymentConfig(), log, errors) } beforeEach(() => { diff --git a/test/unit/lib/plugins/collaborators.test.js b/test/unit/lib/plugins/collaborators.test.js index 359dd461..81026c33 100644 --- a/test/unit/lib/plugins/collaborators.test.js +++ b/test/unit/lib/plugins/collaborators.test.js @@ -1,11 +1,12 @@ const Collaborators = require('../../../../lib/plugins/collaborators') +const DeploymentConfig = require('../../../../lib/deploymentConfig') describe('Collaborators', () => { let github function configure (config) { const log = { debug: jest.fn(), error: console.error } - return new Collaborators(undefined, github, { owner: 'bkeepers', repo: 'test' }, config, log) + return new Collaborators(undefined, github, { owner: 'bkeepers', repo: 'test' }, config, new DeploymentConfig(), log) } beforeEach(() => { diff --git a/test/unit/lib/plugins/custom_properties.test.js b/test/unit/lib/plugins/custom_properties.test.js index 69568a28..7ee70380 100644 --- a/test/unit/lib/plugins/custom_properties.test.js +++ b/test/unit/lib/plugins/custom_properties.test.js @@ -1,4 +1,5 @@ const CustomProperties = require('../../../../lib/plugins/custom_properties') +const DeploymentConfig = require('../../../../lib/deploymentConfig') describe('CustomProperties', () => { let github @@ -8,7 +9,7 @@ describe('CustomProperties', () => { function configure (config) { const nop = false; const errors = [] - return new CustomProperties(nop, github, { owner: 'bkeepers', repo: 'test' }, config, log, errors) + return new CustomProperties(nop, github, { owner: 'bkeepers', repo: 'test' }, config, new DeploymentConfig(), log, errors) } beforeEach(() => { diff --git a/test/unit/lib/plugins/labels.test.js b/test/unit/lib/plugins/labels.test.js index a94ffb5b..4378208b 100644 --- a/test/unit/lib/plugins/labels.test.js +++ b/test/unit/lib/plugins/labels.test.js @@ -1,4 +1,5 @@ const Labels = require('../../../../lib/plugins/labels') +const DeploymentConfig = require('../../../../lib/deploymentConfig') describe('Labels', () => { let github @@ -6,7 +7,7 @@ describe('Labels', () => { function configure(config) { const nop = false; - return new Labels(nop, github, { owner: 'bkeepers', repo: 'test' }, config, log) + return new Labels(nop, github, { owner: 'bkeepers', repo: 'test' }, config, new DeploymentConfig(), log) } beforeEach(() => { diff --git a/test/unit/lib/plugins/repository.test.js b/test/unit/lib/plugins/repository.test.js index b7af3912..654998ce 100644 --- a/test/unit/lib/plugins/repository.test.js +++ b/test/unit/lib/plugins/repository.test.js @@ -1,4 +1,5 @@ const Repository = require('../../../../lib/plugins/repository') +const DeploymentConfig = require('../../../../lib/deploymentConfig') describe('Repository', () => { const github = { @@ -19,7 +20,7 @@ describe('Repository', () => { function configure (config) { const noop = false const errors = [] - return new Repository(noop, github, { owner: 'bkeepers', repo: 'test' }, config, 1, log, errors) + return new Repository(noop, github, { owner: 'bkeepers', repo: 'test' }, config, new DeploymentConfig(), log, errors) } describe('sync', () => { diff --git a/test/unit/lib/plugins/teams.test.js b/test/unit/lib/plugins/teams.test.js index f289421e..2bf5ebc8 100644 --- a/test/unit/lib/plugins/teams.test.js +++ b/test/unit/lib/plugins/teams.test.js @@ -1,6 +1,7 @@ const { when } = require('jest-when') const any = require('@travi/any') const Teams = require('../../../../lib/plugins/teams') +const DeploymentConfig = require('../../../../lib/deploymentConfig') describe('Teams', () => { let github @@ -17,7 +18,7 @@ describe('Teams', () => { function configure (config) { const log = { debug: jest.fn(), error: console.error } const errors = [] - return new Teams(undefined, github, { owner: 'bkeepers', repo: 'test' }, config, log, errors) + return new Teams(undefined, github, { owner: 'bkeepers', repo: 'test' }, config, new DeploymentConfig(), log, errors) } beforeEach(() => { diff --git a/test/unit/lib/settings.test.js b/test/unit/lib/settings.test.js index c289f563..8b3952b4 100644 --- a/test/unit/lib/settings.test.js +++ b/test/unit/lib/settings.test.js @@ -1,6 +1,9 @@ /* eslint-disable no-undef */ const Settings = require('../../../lib/settings') +const DeploymentConfig = require('../../../lib/deploymentConfig') + +jest.mock('../../../lib/deploymentConfig') describe('Settings Tests', () => { let stubContext @@ -42,10 +45,10 @@ describe('Settings Tests', () => { describe('restrictedRepos', () => { describe('restrictedRepos not defined', () => { beforeEach(() => { - stubConfig = { - restrictedRepos: { - } - } + DeploymentConfig.mockImplementation(() => ({ + restrictedRepos: {} + })) + stubConfig = {} }) it('Allow repositories being configured', () => { @@ -64,11 +67,12 @@ describe('Settings Tests', () => { describe('restrictedRepos.exclude defined', () => { beforeEach(() => { - stubConfig = { + DeploymentConfig.mockImplementation(() => ({ restrictedRepos: { exclude: ['foo', '.*-test$', '^personal-.*$'] } - } + })) + stubConfig = {} }) it('Skipping excluded repository from being configured', () => { @@ -91,11 +95,12 @@ describe('Settings Tests', () => { describe('restrictedRepos.include defined', () => { beforeEach(() => { - stubConfig = { + DeploymentConfig.mockImplementation(() => ({ restrictedRepos: { include: ['foo', '.*-test$', '^personal-.*$'] } - } + })) + stubConfig = {} }) it('Allowing repository from being configured', () => { @@ -116,25 +121,12 @@ describe('Settings Tests', () => { }) }) - describe('restrictedRepos not defined', () => { - it('Throws TypeError if restrictedRepos not defined', () => { - stubConfig = {} - settings = createSettings(stubConfig) - expect(() => settings.isRestricted('my-repo')).toThrow('Cannot read properties of undefined (reading \'include\')') - }) - - it('Throws TypeError if restrictedRepos is null', () => { - stubConfig = { - restrictedRepos: null - } - settings = createSettings(stubConfig) - expect(() => settings.isRestricted('my-repo')).toThrow('Cannot read properties of null (reading \'include\')') - }) - + describe('restrictedRepos empty', () => { it('Allowing all repositories if restrictedRepos is empty', () => { - stubConfig = { + DeploymentConfig.mockImplementation(() => ({ restrictedRepos: [] - } + })) + stubConfig = {} settings = createSettings(stubConfig) expect(settings.isRestricted('my-repo')).toEqual(false) }) diff --git a/test/unit/lib/validator.test.js b/test/unit/lib/validator.test.js index 56a1b87a..c34746db 100644 --- a/test/unit/lib/validator.test.js +++ b/test/unit/lib/validator.test.js @@ -20,8 +20,11 @@ describe('Validator Tests', () => { console.log(`Branch config validator, baseconfig ${baseconfig}`) return false }) - DeploymentConfig.overridevalidators = { branches: { canOverride: overrideMock, error: 'Branch overrideValidators.error' } } - DeploymentConfig.configvalidators = { branches: { isValid: configMock, error: 'Branch configValidators.error' } } + + const deploymentConfig = { + overrideValidators: { branches: { canOverride: overrideMock, error: 'Branch overrideValidators.error' } }, + configValidators: { branches: { isValid: configMock, error: 'Branch configValidators.error' } } + } const overrideconfig = YAML.load(` branches: @@ -55,7 +58,7 @@ describe('Validator Tests', () => { try { const ignorableFields = [] - const mergeDeep = new MergeDeep(log, {}, ignorableFields) + const mergeDeep = new MergeDeep(log, {}, ignorableFields, deploymentConfig) mergeDeep.mergeDeep(baseconfig, overrideconfig) // const merged = mergeDeep.mergeDeep(baseconfig, overrideconfig) // expect(() => mergeDeep.mergeDeep(baseconfig, overrideconfig)).toThrow('you are using the wrong JDK'); @@ -77,8 +80,11 @@ describe('Validator Tests', () => { console.log(`Repo config validator, baseconfig ${baseconfig}`) return false }) - DeploymentConfig.overridevalidators = { repository: { canOverride: overrideMock, error: 'Repo overrideValidators.error' } } - DeploymentConfig.configvalidators = { repository: { isValid: configMock, error: 'Repo configValidators.error' } } + + const deploymentConfig = { + overrideValidators: { repository: { canOverride: overrideMock, error: 'Repo overrideValidators.error' } }, + configValidators: { repository: { isValid: configMock, error: 'Repo configValidators.error' } } + } const overrideconfig = YAML.load(` repository: @@ -113,7 +119,7 @@ describe('Validator Tests', () => { try { const ignorableFields = [] - const mergeDeep = new MergeDeep(log, {}, ignorableFields) + const mergeDeep = new MergeDeep(log, {}, ignorableFields, deploymentConfig) mergeDeep.mergeDeep(baseconfig, overrideconfig) } catch (err) { expect(err).toBeDefined() @@ -132,8 +138,11 @@ describe('Validator Tests', () => { console.log(`Repo config validator, baseconfig ${baseconfig}`) return false }) - DeploymentConfig.overridevalidators = { repository: { canOverride: overrideMock, error: 'Repo overrideValidators.error' } } - DeploymentConfig.configvalidators = { repository: { isValid: configMock, error: 'Repo configValidators.error' } } + + const deploymentConfig = { + overrideValidators: { repository: { canOverride: overrideMock, error: 'Repo overrideValidators.error' } }, + configValidators: { repository: { isValid: configMock, error: 'Repo configValidators.error' } } + } const overrideconfig = YAML.load(` repository: @@ -168,7 +177,7 @@ describe('Validator Tests', () => { try { const ignorableFields = [] - const mergeDeep = new MergeDeep(log, {}, ignorableFields) + const mergeDeep = new MergeDeep(log, {}, ignorableFields, deploymentConfig) mergeDeep.mergeDeep(baseconfig, overrideconfig) } catch (err) { expect(err).toBeDefined() From 4d5d6bc39bf5fcb43b55b5a74e38250c7293648c Mon Sep 17 00:00:00 2001 From: Jonathan Morley Date: Tue, 15 Oct 2024 09:48:56 -0400 Subject: [PATCH 2/4] fix: remove confirmation of changes. It doesnt work with the PR checks right now --- lib/plugins/repository.js | 8 -------- 1 file changed, 8 deletions(-) diff --git a/lib/plugins/repository.js b/lib/plugins/repository.js index 371d25a1..921c7f84 100644 --- a/lib/plugins/repository.js +++ b/lib/plugins/repository.js @@ -153,14 +153,6 @@ module.exports = class Repository extends ErrorStash { this.logError(` Error ${JSON.stringify(e)}`) } }) - - this.log.debug(`Confirming synchronization of repository for ${this.repo.owner}/${this.repo.repo}`) - const { data: upstream } = await this.github.repos.get(this.repo) - - const changes = mergeDeep.compareDeep(upstream, this.settings) - - if (changes.hasChanges) throw new Error(`Synchronization of repository for ${this.repo.owner}/${this.repo.repo} failed: ${JSON.stringify(changes)}`) - this.log.debug(`Confirmed synchronization of repository for ${this.repo.owner}/${this.repo.repo}`) } updateDefaultBranch (oldname, newname, resArray) { From fdf9990bbc7dd5b3e495e91f6bcf7cc21aff0386 Mon Sep 17 00:00:00 2001 From: Jonathan Morley Date: Tue, 15 Oct 2024 13:29:38 -0400 Subject: [PATCH 3/4] fix: change await back to return --- index.js | 3 +-- lib/plugins/repository.js | 2 +- lib/settings.js | 3 --- 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/index.js b/index.js index 0983bfcc..908f12fa 100644 --- a/index.js +++ b/index.js @@ -9,11 +9,10 @@ const env = require('./lib/env') let deploymentConfig - module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => { let appName = 'safe-settings' let appSlug = 'safe-settings' - + // All repos _could_ be affected, so sync everything async function syncAllSettings (nop, context, repo = context.repo(), ref) { try { diff --git a/lib/plugins/repository.js b/lib/plugins/repository.js index 921c7f84..96d79ba8 100644 --- a/lib/plugins/repository.js +++ b/lib/plugins/repository.js @@ -65,7 +65,7 @@ module.exports = class Repository extends ErrorStash { this.log.debug(`Syncing Repo ${this.repo.owner}/${this.repo.repo}`) // let hasChanges = false // let hasTopicChanges = false - await this.github.repos.get(this.repo) + return this.github.repos.get(this.repo) .then(resp => { const changes = mergeDeep.compareDeep(resp.data, this.settings) // hasChanges = changes.additions.length > 0 || changes.modifications.length > 0 diff --git a/lib/settings.js b/lib/settings.js index 6d337798..cb3b3309 100644 --- a/lib/settings.js +++ b/lib/settings.js @@ -679,8 +679,6 @@ ${this.results.reduce((x, y) => { const overridePaths = this.subOrgConfigMap || await this.getSubOrgConfigMap() const subOrgConfigs = {} - this.log.debug(`OverridePaths: ${JSON.stringify(overridePaths, null, 2)}`) - for (const override of overridePaths) { const data = await this.loadYaml(override.path) this.log.debug(`data = ${JSON.stringify(data)}`) @@ -705,7 +703,6 @@ ${this.results.reduce((x, y) => { }) }) } - this.log.debug(`data: ${JSON.stringify(data, null, 2)}`) if (data.suborgproperties) { const promises = data.suborgproperties.map((customProperty) => { return this.getReposForCustomProperty(customProperty) From 84ca3d5c3ac61df00b364216e1554d5e85684c9f Mon Sep 17 00:00:00 2001 From: Jonathan Morley Date: Thu, 14 Nov 2024 17:38:53 -0500 Subject: [PATCH 4/4] fix: typo --- lib/deploymentConfig.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/deploymentConfig.js b/lib/deploymentConfig.js index 435914fe..e757f9fd 100644 --- a/lib/deploymentConfig.js +++ b/lib/deploymentConfig.js @@ -35,7 +35,7 @@ class DeploymentConfig { for (const validator of config.configvalidators) { // eslint-disable-next-line no-new-func const f = new Function('baseconfig', 'githubContext', validator.script) - this.configvalidators[validator.plugin] = { isValid: f, error: validator.error } + this.configValidators[validator.plugin] = { isValid: f, error: validator.error } } }