feat: enable some settings to be marked unsafe, settable in the repositories themselves

Author: jonathanmorley

docs/sample-settings/sample-deployment-settings.yml

@@ -27,3 +27,8 @@ overridevalidators:
       Some error
     script: |
       return true
+unsafeFields:
+  # You can specify the fields that are allowed to be controlled by individual repositories
+  - /repository/description
+  - /repository/allow_auto_merge
+  

index.js

@@ -258,41 +258,48 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
     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 synch...`)
+        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)
+      }
+    }
   })
 
   robot.on('create', async context => {

lib/configManager.js

@@ -10,25 +10,20 @@ 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 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
       if (Array.isArray(response.data)) {
         return null
       }
-
       // we don't handle symlinks or submodule
       // - https://developer.github.com/v3/repos/contents/#response-if-content-is-a-symlink
       // - https://developer.github.com/v3/repos/contents/#response-if-content-is-a-submodule
@@ -45,14 +40,18 @@ module.exports = class ConfigManager {
   }
 
   /**
-   * Loads a file from GitHub
+   * Loads the settings 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)
+    return ConfigManager.loadYaml(this.context.octokit, {
+      owner: this.context.repo().owner,
+      repo: env.ADMIN_REPO,
+      path: filePath,
+      ref: this.ref
+    })
   }
 }

lib/deploymentConfig.js

@@ -11,6 +11,7 @@ class DeploymentConfig {
   // static config
   static configvalidators = {}
   static overridevalidators = {}
+  static unsafeFields = []
 
   static {
     const deploymentConfigPath = process.env.DEPLOYMENT_CONFIG_FILE ? process.env.DEPLOYMENT_CONFIG_FILE : 'deployment-settings.yml'
@@ -36,6 +37,15 @@ class DeploymentConfig {
         this.configvalidators[validator.plugin] = { isValid: f, error: validator.error }
       }
     }
+
+    const unsafeFields = this.config.unsafeFields
+    if (unsafeFields) {
+      if (this.isIterable(unsafeFields)) {
+        this.unsafeFields = unsafeFields
+      } else {
+        throw new Error('unsafeFields must be an array')
+      }
+    }
   }
 
   static isNonEmptyArray (obj) {

lib/settings.js

@@ -1,7 +1,10 @@
 const path = require('path')
 const { Eta } = require('eta')
+const jsonPointer = require('json-pointer')
+const lodashSet = require('lodash.set')
 const commetMessageTemplate = require('./commentmessage')
 const errorTemplate = require('./error')
+const ConfigManager = require('./configManager')
 const Glob = require('./glob')
 const NopCommand = require('./nopcommand')
 const MergeDeep = require('./mergeDeep')
@@ -298,11 +301,6 @@ ${this.results.reduce((x, y) => {
 
   async updateRepos(repo) {
     this.subOrgConfigs = this.subOrgConfigs || await this.getSubOrgConfigs()
-    let repoConfig = this.config.repository
-    if (repoConfig) {
-      repoConfig = Object.assign(repoConfig, { name: repo.repo, org: repo.owner })
-    }
-
     const subOrgConfig = this.getSubOrgConfig(repo.repo)
 
     // If suborg config has been updated then only restrict to the repos for that suborg
@@ -313,6 +311,14 @@ ${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)}`)
 
+    let repoConfig = this.config.repository
+
+    // Overlay with repo information
+    if (repoConfig) {
+      repoConfig = Object.assign(repoConfig, { name: repo.repo, org: repo.owner })
+    }
+
+    // Overlay with suborg
     if (subOrgConfig) {
       let suborgRepoConfig = subOrgConfig.repository
       if (suborgRepoConfig) {
@@ -321,19 +327,26 @@ ${this.results.reduce((x, y) => {
       }
     }
 
-    // Overlay repo config
+    // Overlay with centralized repo config
     // RepoConfigs should be preloaded but checking anyway
     const overrideRepoConfig = this.repoConfigs[`${repo.repo}.yml`]?.repository || this.repoConfigs[`${repo.repo}.yaml`]?.repository
     if (overrideRepoConfig) {
       repoConfig = this.mergeDeep.mergeDeep({}, repoConfig, overrideRepoConfig)
     }
+
+    // Overlay with decentralized repo config
+    const unsafeRepoOverrideConfig = (await this.getUnsafeRepoConfig(repo))?.repository
+    if (unsafeRepoOverrideConfig) {
+      repoConfig = this.mergeDeep.mergeDeep({}, repoConfig, unsafeRepoOverrideConfig)
+    }
+
     const {shouldContinue, nopCommands} = await new Archive(this.nop, this.github, repo, repoConfig, this.log).sync()
     if (nopCommands) this.appendToResults(nopCommands)
     if (shouldContinue) {
       if (repoConfig) {
         try {
           this.log.debug(`found a matching repoconfig for this repo ${JSON.stringify(repoConfig)}`)
-          const childPlugins = this.childPluginsList(repo)
+          const childPlugins = await 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)
@@ -356,7 +369,7 @@ ${this.results.reduce((x, y) => {
         }
       } 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)
+        const childPlugins = await 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)
@@ -379,26 +392,59 @@ ${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 repoConfig = await ConfigManager.loadYaml(this.github, {
+      ...repo,
+      path: '.github/settings.yml'
+    })
+
+    const { unsafeFields } = this.config
+
+    const result = {}
+    for (const unsafeField of unsafeFields) {
+      let value
+      try {
+        value = jsonPointer.get(repoConfig, 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.getRepoOverrideConfig(repoName)
-    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)}`)
 

package-lock.json

@@ -30,7 +30,9 @@
     "deepmerge": "^4.3.1",
     "eta": "^3.5.0",
     "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": "^4.1.2",
     "probot": "^13.4.4"