add initial code for preventing multiple suborg config for repos

Author: decyjphr

.eslintrc.json

@@ -1,11 +1,11 @@
 {
     "env": {
-        "browser": true,
+        "node": true,
         "commonjs": true,
         "es2021": true
     },
     "extends": [
-        "standard"
+        "eslint:recommended"
     ],
     "parserOptions": {
         "ecmaVersion": 12

lib/settings.js

@@ -24,7 +24,7 @@ class Settings {
     }
   }
 
-  static async syncSubOrgs (nop, context, suborg, repo, config, ref) {
+  static async syncSubOrgs(nop, context, suborg, repo, config, ref) {
     const settings = new Settings(nop, context, repo, config, ref, suborg)
     try {
       await settings.loadConfigs()
@@ -36,7 +36,7 @@ class Settings {
     }
   }
 
-  static async sync (nop, context, repo, config, ref) {
+  static async sync(nop, context, repo, config, ref) {
     const settings = new Settings(nop, context, repo, config, ref)
     try {
       await settings.loadConfigs(repo)
@@ -51,13 +51,13 @@ class Settings {
     }
   }
 
-  static async handleError (nop, context, repo, config, ref, nopcommand) {
+  static async handleError(nop, context, repo, config, ref, nopcommand) {
     const settings = new Settings(nop, context, repo, config, ref)
     settings.appendToResults([nopcommand])
     await settings.handleResults()
   }
 
-  constructor (nop, context, repo, config, ref, suborg) {
+  constructor(nop, context, repo, config, ref, suborg) {
     this.ref = ref
     this.context = context
     this.installation_id = context.payload.installation.id
@@ -96,7 +96,7 @@ class Settings {
   }
 
   // Create a check in the Admin repo for safe-settings.
-  async createCheckRun () {
+  async createCheckRun() {
     const startTime = new Date()
     let conclusion = 'success'
     let details = `Run on: \`${new Date().toISOString()}\``
@@ -142,7 +142,7 @@ class Settings {
       })
   }
 
-  logError (msg) {
+  logError(msg) {
     this.log.error(msg)
     this.errors.push({
       owner: this.repo.owner,
@@ -152,7 +152,7 @@ class Settings {
     })
   }
 
-  async handleResults () {
+  async handleResults() {
     const { payload } = this.context
 
     // Create a checkrun if not in nop mode
@@ -226,23 +226,23 @@ class Settings {
 #### :robot: Safe-Settings config changes detected:
 
 ${this.results.reduce((x, y) => {
-  if (!y) {
-    return x
-  }
-  if (y.type === 'ERROR') {
-    error = true
-    return `${x}
+        if (!y) {
+          return x
+        }
+        if (y.type === 'ERROR') {
+          error = true
+          return `${x}
 <tr><td> ❗ ${y.action.msg} </td><td> ${y.plugin} </td><td> ${prettify(y.repo)} </td><td> ${prettify(y.action.additions)} </td><td> ${prettify(y.action.deletions)} </td><td> ${prettify(y.action.modifications)} </td><tr>`
-  } else if (y.action.additions === null && y.action.deletions === null && y.action.modifications === null) {
-    return `${x}`
-  } else {
-    if (y.action === undefined) {
-      return `${x}`
-    }
-    return `${x}
+        } else if (y.action.additions === null && y.action.deletions === null && y.action.modifications === null) {
+          return `${x}`
+        } else {
+          if (y.action === undefined) {
+            return `${x}`
+          }
+          return `${x}
 <tr><td> ✋ </td><td> ${y.plugin} </td><td> ${prettify(y.repo)} </td><td> ${prettify(y.action.additions)} </td><td> ${prettify(y.action.deletions)} </td><td> ${prettify(y.action.modifications)} </td><tr>`
-  }
-}, table)}
+        }
+      }, table)}
 `
 
       const pullRequest = payload.check_run.check_suite.pull_requests[0]
@@ -272,12 +272,12 @@ ${this.results.reduce((x, y) => {
     await this.github.checks.update(params)
   }
 
-  async loadConfigs (repo) {
+  async loadConfigs(repo) {
     this.subOrgConfigs = await this.getSubOrgConfigs()
     this.repoConfigs = await this.getRepoConfigs(repo)
   }
 
-  async updateOrg () {
+  async updateOrg() {
     const rulesetsConfig = this.config.rulesets
     if (rulesetsConfig) {
       const RulesetsPlugin = Settings.PLUGINS.rulesets
@@ -287,7 +287,7 @@ ${this.results.reduce((x, y) => {
     }
   }
 
-  async updateRepos (repo) {
+  async updateRepos(repo) {
     this.subOrgConfigs = this.subOrgConfigs || await this.getSubOrgConfigs()
     let repoConfig = this.config.repository
     if (repoConfig) {
@@ -353,15 +353,15 @@ ${this.results.reduce((x, y) => {
     }
   }
 
-  async updateAll () {
+  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 => {
       this.appendToResults(res)
     })
   }
 
-  getSubOrgConfig (repoName) {
+  getSubOrgConfig(repoName) {
     if (this.subOrgConfigs) {
       for (const k of Object.keys(this.subOrgConfigs)) {
         const repoPattern = new Glob(k)
@@ -374,13 +374,13 @@ ${this.results.reduce((x, y) => {
   }
 
   // Remove Org specific configs from the repo config
-  returnRepoSpecificConfigs (config) {
+  returnRepoSpecificConfigs(config) {
     const newConfig = Object.assign({}, config) // clone
     delete newConfig.rulesets
     return newConfig
   }
 
-  childPluginsList (repo) {
+  childPluginsList(repo) {
     const repoName = repo.repo
     const subOrgOverrideConfig = this.getSubOrgConfig(repoName)
     this.log.debug(`suborg config for ${repoName}  is ${JSON.stringify(subOrgOverrideConfig)}`)
@@ -412,7 +412,7 @@ ${this.results.reduce((x, y) => {
     return childPlugins
   }
 
-  validate (section, baseConfig, overrideConfig) {
+  validate(section, baseConfig, overrideConfig) {
     const configValidator = this.configvalidators[section]
     if (configValidator) {
       this.log.debug(`Calling configvalidator for key ${section} `)
@@ -431,7 +431,7 @@ ${this.results.reduce((x, y) => {
     }
   }
 
-  isRestricted (repoName) {
+  isRestricted(repoName) {
     const restrictedRepos = this.config.restrictedRepos
     // Skip configuring any restricted repos
     if (Array.isArray(restrictedRepos)) {
@@ -463,11 +463,11 @@ ${this.results.reduce((x, y) => {
     return false
   }
 
-  includesRepo (repoName, restrictedRepos) {
+  includesRepo(repoName, restrictedRepos) {
     return restrictedRepos.filter((restrictedRepo) => { return RegExp(restrictedRepo).test(repoName) }).length > 0
   }
 
-  async eachRepositoryRepos (github, restrictedRepos, log) {
+  async eachRepositoryRepos(github, restrictedRepos, log) {
     log.debug('Fetching repositories')
     return github.paginate('GET /installation/repositories').then(repositories => {
       return Promise.all(repositories.map(repository => {
@@ -488,7 +488,7 @@ ${this.results.reduce((x, y) => {
    * @param params Params to fetch the file with
    * @return The parsed YAML file
    */
-  async loadConfigMap (params) {
+  async loadConfigMap(params) {
     try {
       this.log.debug(` In loadConfigMap ${JSON.stringify(params)}`)
       const response = await this.github.repos.getContent(params).catch(e => {
@@ -535,7 +535,7 @@ ${this.results.reduce((x, y) => {
    * @param params Params to fetch the file with
    * @return The parsed YAML file
    */
-  async getRepoConfigMap () {
+  async getRepoConfigMap() {
     try {
       this.log.debug(` In getRepoConfigMap ${JSON.stringify(this.repo)}`)
       // GitHub getContent api has a hard limit of returning 1000 entries without
@@ -602,7 +602,7 @@ ${this.results.reduce((x, y) => {
    * @param params Params to fetch the file with
    * @return The parsed YAML file
    */
-  async getSubOrgConfigMap () {
+  async getSubOrgConfigMap() {
     try {
       this.log.debug(` In getSubOrgConfigMap ${JSON.stringify(this.repo)}`)
       const repo = { owner: this.repo.owner, repo: env.ADMIN_REPO }
@@ -629,7 +629,7 @@ ${this.results.reduce((x, y) => {
    * @param {*} repo repo param
    * @returns repoConfigs object
    */
-  async getRepoConfigs (repo) {
+  async getRepoConfigs(repo) {
     try {
       const overridePaths = await this.getRepoConfigMap()
       const repoConfigs = {}
@@ -681,7 +681,7 @@ ${this.results.reduce((x, y) => {
    * @param params Params to fetch the file with
    * @return The parsed YAML file
    */
-  async getSubOrgConfigs () {
+  async getSubOrgConfigs() {
     try {
       if (this.subOrgConfigMap) {
         this.log.debug(`SubOrg config was changed and the associated overridePaths is = ${JSON.stringify(this.subOrgConfigMap)}`)
@@ -698,7 +698,19 @@ ${this.results.reduce((x, y) => {
         subOrgConfigs[override.name] = data
         if (data.suborgrepos) {
           data.suborgrepos.forEach(repository => {
-            subOrgConfigs[repository] = data
+            this.storeSubOrgConfig(subOrgConfigs, override.path, repository, data)
+
+            // In case support for multiple suborg configs for the same repo is required, merge the configs.
+            //
+            // Planned for the future to support multiple suborgrepos for the same repo
+            //
+            // if (existingConfigForRepo) {
+            //   subOrgConfigs[repository] = this.mergeDeep.mergeDeep({}, existingConfigForRepo, data)
+            // } else {
+            //   subOrgConfigs[repository] = data
+            // }
+
+            subOrgConfigs[repository] = Object.assign({}, data, { source: override.path })
           })
         }
         if (data.suborgteams) {
@@ -708,7 +720,7 @@ ${this.results.reduce((x, y) => {
           await Promise.all(promises).then(res => {
             res.forEach(r => {
               r.forEach(e => {
-                subOrgConfigs[e.name] = data
+                this.storeSubOrgConfig(subOrgConfigs, override.path, e.name, data)
               })
             })
           })
@@ -720,7 +732,7 @@ ${this.results.reduce((x, y) => {
           await Promise.all(promises).then(res => {
             res.forEach(r => {
               r.forEach(e => {
-                subOrgConfigs[e.repository_name] = data
+                this.storeSubOrgConfig(subOrgConfigs, override.path, e.repository_name, data)
               })
             })
           })
@@ -739,13 +751,21 @@ ${this.results.reduce((x, y) => {
     }
   }
 
+  storeSubOrgConfig(subOrgConfigs, overridePath, repoName, data) {
+    const existingConfigForRepo = subOrgConfigs[repoName]
+    if (existingConfigForRepo && existingConfigForRepo.source !== overridePath) {
+      throw new Error(`Multiple suborg configs for ${repoName} in ${overridePath} and ${existingConfigForRepo?.source}`)
+    }
+    subOrgConfigs[repoName] = Object.assign({}, data, { source: overridePath })
+  }
+
   /**
    * Loads a file from GitHub
    *
    * @param params Params to fetch the file with
    * @return The parsed YAML file
    */
-  async loadYaml (filePath) {
+  async loadYaml(filePath) {
     try {
       const repo = { owner: this.repo.owner, repo: env.ADMIN_REPO }
       const params = Object.assign(repo, { path: filePath, ref: this.ref })
@@ -782,13 +802,13 @@ ${this.results.reduce((x, y) => {
     }
   }
 
-  appendToResults (res) {
+  appendToResults(res) {
     if (this.nop) {
       this.results = this.results.concat(res.flat(3))
     }
   }
 
-  async getReposForTeam (teamslug) {
+  async getReposForTeam(teamslug) {
     const options = this.github.rest.teams.listReposInOrg.endpoint.merge({
       org: this.repo.owner,
       team_slug: teamslug,
@@ -797,20 +817,19 @@ ${this.results.reduce((x, y) => {
     return this.github.paginate(options)
   }
 
-  async getReposForCustomProperty (customPropertyTuple) {
-    const name=Object.keys(customPropertyTuple)[0]
+  async getReposForCustomProperty(customPropertyTuple) {
+    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) {
+  isObject(item) {
     return (item && typeof item === 'object' && !Array.isArray(item))
   }
 
-  isIterable (obj) {
+  isIterable(obj) {
     // checks for null and undefined
     if (obj == null) {
       return false