Skip to content

Commit

Permalink
Same repository that is part of multiple suborgs (#664)
Browse files Browse the repository at this point in the history 
* add initial code for preventing multiple suborg config for repos

* detect conflict even when a single suborg config is changed

* remove duplicate errors

* dont add nulls and undefined to results

* Update ESLint config for browser and standard
  • Loading branch information
@decyjphr
decyjphr authored Dec 21, 2024
1 parent 20a4508 commit a692dbf
Show file tree
Hide file tree
Showing 2 changed files with 226 additions and 61 deletions.
150 changes: 96 additions & 54 deletions lib/settings.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class Settings {
return 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()
Expand All @@ -37,7 +37,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)
Expand All @@ -52,13 +52,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
Expand Down Expand Up @@ -97,7 +97,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()}\``
Expand Down Expand Up @@ -143,7 +143,7 @@ class Settings {
})
}

logError (msg) {
logError(msg) {
this.log.error(msg)
this.errors.push({
owner: this.repo.owner,
Expand All @@ -153,7 +153,7 @@ class Settings {
})
}

async handleResults () {
async handleResults() {
const { payload } = this.context

// Create a checkrun if not in nop mode
Expand All @@ -163,6 +163,13 @@ class Settings {
return
}

//remove duplicate rows in this.results
this.results = this.results.filter((thing, index, self) => {
return index === self.findIndex((t) => {
return t.type === thing.type && t.repo === thing.repo && t.plugin === thing.plugin
})
})

let error = false
// Different logic
const stats = {
Expand Down Expand Up @@ -227,23 +234,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]
Expand Down Expand Up @@ -273,12 +280,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
Expand All @@ -288,7 +295,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) {
Expand Down Expand Up @@ -354,15 +361,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)
Expand All @@ -375,13 +382,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)}`)
Expand Down Expand Up @@ -413,7 +420,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} `)
Expand All @@ -432,7 +439,7 @@ ${this.results.reduce((x, y) => {
}
}

isRestricted (repoName) {
isRestricted(repoName) {
const restrictedRepos = this.config.restrictedRepos
// Skip configuring any restricted repos
if (Array.isArray(restrictedRepos)) {
Expand Down Expand Up @@ -464,11 +471,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 => {
Expand All @@ -489,7 +496,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 => {
Expand Down Expand Up @@ -536,7 +543,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
Expand Down Expand Up @@ -603,7 +610,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 }
Expand All @@ -630,7 +637,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 = {}
Expand Down Expand Up @@ -682,12 +689,11 @@ ${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)}`)
}
const overridePaths = this.subOrgConfigMap || await this.getSubOrgConfigMap()
// Get all suborg configs even though we might be here becuase of a suborg config change
// we will filter them out if request is due to a suborg config change
const overridePaths = await this.getSubOrgConfigMap()
const subOrgConfigs = {}

for (const override of overridePaths) {
Expand All @@ -699,7 +705,19 @@ ${this.results.reduce((x, y) => {
subOrgConfigs[override.name] = data
if (data.suborgrepos) {
data.suborgrepos.forEach(repository => {
subOrgConfigs[repository] = data
this.storeSubOrgConfigIfNoConflicts(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) {
Expand All @@ -709,7 +727,7 @@ ${this.results.reduce((x, y) => {
await Promise.all(promises).then(res => {
res.forEach(r => {
r.forEach(e => {
subOrgConfigs[e.name] = data
this.storeSubOrgConfigIfNoConflicts(subOrgConfigs, override.path, e.name, data)
})
})
})
Expand All @@ -721,12 +739,26 @@ ${this.results.reduce((x, y) => {
await Promise.all(promises).then(res => {
res.forEach(r => {
r.forEach(e => {
subOrgConfigs[e.repository_name] = data
this.storeSubOrgConfigIfNoConflicts(subOrgConfigs, override.path, e.repository_name, data)
})
})
})
}
}

// If this was result of a suborg config change, only return the repos that are part of the suborg config
if (this.subOrgConfigMap) {
this.log.debug(`SubOrg config was changed and the associated overridePaths is = ${JSON.stringify(this.subOrgConfigMap)}`)
// enumerate the properties of the subOrgConfigs object and delete the ones that are not part of the suborg
for (const [key, value] of Object.entries(subOrgConfigs)) {
if (!this.subOrgConfigMap.some((overridePath) => {
return overridePath.path === value.source
}
)) {
delete subOrgConfigs[key]
}
}
}
return subOrgConfigs
} catch (e) {
if (this.nop) {
Expand All @@ -740,13 +772,21 @@ ${this.results.reduce((x, y) => {
}
}

storeSubOrgConfigIfNoConflicts(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 })
Expand Down Expand Up @@ -783,13 +823,16 @@ ${this.results.reduce((x, y) => {
}
}

appendToResults (res) {
appendToResults(res) {
if (this.nop) {
this.results = this.results.concat(res.flat(3))
//Remove nulls and undefined from the results
const results = res.flat(3).filter(r => r)

this.results = this.results.concat(results)
}
}

async getReposForTeam (teamslug) {
async getReposForTeam(teamslug) {
const options = this.github.rest.teams.listReposInOrg.endpoint.merge({
org: this.repo.owner,
team_slug: teamslug,
Expand All @@ -798,20 +841,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
Expand Down
Loading

0 comments on commit a692dbf

Please sign in to comment.