Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Safe settings/unsafe #7

Merged
merged 4 commits into from
Nov 15, 2024
Merged

Safe settings/unsafe #7

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f4704cd
feat: allow some settings to be marked 'unsafe', to allow overriding …
jonathanmorley Oct 14, 2024
4d5d6bc
fix: remove confirmation of changes. It doesnt work with the PR check…
jonathanmorley Oct 15, 2024
fdf9990
fix: change await back to return
jonathanmorley Oct 15, 2024
84ca3d5
fix: typo
jonathanmorley Nov 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 58 additions & 45 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,18 @@ 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 {
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)
Expand All @@ -42,13 +43,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) {
Expand All @@ -67,13 +69,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) {
Expand Down Expand Up @@ -258,47 +261,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
Expand Down Expand Up @@ -394,6 +407,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.`)
Expand Down Expand Up @@ -474,7 +488,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
Expand Down Expand Up @@ -503,6 +517,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
Expand All @@ -522,6 +537,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
Expand All @@ -543,16 +559,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
Expand Down
46 changes: 23 additions & 23 deletions lib/configManager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
}
}
79 changes: 42 additions & 37 deletions lib/deploymentConfig.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
20 changes: 14 additions & 6 deletions lib/mergeDeep.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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) {
Expand Down Expand Up @@ -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
}

Expand Down
Loading