Skip to content

feat: validate config with schema #857

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.

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

Open
wants to merge 5 commits into
base: main-enterprise
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion lib/env.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,6 @@ module.exports = {
CREATE_PR_COMMENT: process.env.CREATE_PR_COMMENT || 'true',
CREATE_ERROR_ISSUE: process.env.CREATE_ERROR_ISSUE || 'true',
BLOCK_REPO_RENAME_BY_HUMAN: process.env.BLOCK_REPO_RENAME_BY_HUMAN || 'false',
FULL_SYNC_NOP: process.env.FULL_SYNC_NOP === 'true'
FULL_SYNC_NOP: process.env.FULL_SYNC_NOP === 'true',
VALIDATE_CONFIG_SCHEMA: process.env.VALIDATE_CONFIG_SCHEMA === 'true'
}
57 changes: 52 additions & 5 deletions lib/settings.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,21 @@
const path = require('path')
const { Eta } = require('eta')
const commetMessageTemplate = require('./commentmessage')
const commentMessageTemplate = require('./commentmessage')
const errorTemplate = require('./error')
const Glob = require('./glob')
const NopCommand = require('./nopcommand')
const MergeDeep = require('./mergeDeep')
const Archive = require('./plugins/archive')
const env = require('./env')
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
const yaml = require('js-yaml');
const Ajv = require('ajv/dist/2020')
const schema = require('../schema/dereferenced/settings.json');

const ajv = new Ajv({
strict: false
})

class Settings {
static fileCache = {};
Expand Down Expand Up @@ -99,6 +104,34 @@ class Settings {
}
}
this.mergeDeep = new MergeDeep(this.log, this.github, [], this.configvalidators, this.overridevalidators)

const filePath = path.posix.join(env.CONFIG_PATH, env.SETTINGS_FILE_PATH)
const validation = this.validateConfig(this.config)

if (validation instanceof Error) {
this.appendToResults([{
type: 'ERROR',
plugin: 'settings',
repo: this.repo.repo,
action: {
msg: `${filePath} fails schema validation:\n${validation.message}`
}
}])

this.logError(`${filePath} fails schema validation:\n${validation.message}`)
}
}

validateConfig (config) {
if (env.VALIDATE_CONFIG_SCHEMA) {
const valid = ajv.validate(schema, config)

if (!valid) {
return new Error(ajv.errorsText(ajv.errors))
}
}

return true
}

// Create a check in the Admin repo for safe-settings.
Expand Down Expand Up @@ -232,7 +265,7 @@ class Settings {
<tbody>
`

const renderedCommentMessage = await eta.renderString(commetMessageTemplate, stats)
const renderedCommentMessage = await eta.renderString(commentMessageTemplate, stats)

if (env.CREATE_PR_COMMENT === 'true') {
const summary = `
Expand All @@ -246,7 +279,7 @@ ${this.results.reduce((x, y) => {
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) {
} else if (y.action?.additions === null && y.action.deletions === null && y.action.modifications === null) {
return `${x}`
} else {
if (y.action === undefined) {
Expand Down Expand Up @@ -644,7 +677,7 @@ ${this.results.reduce((x, y) => {
/**
* If repo param is null load configs for all repos
* If repo param is null and suborg change, load configs for suborg repos only
* If repo partam is not null, load the config for a specific repo
* If repo param is not null, load the config for a specific repo
* @param {*} repo repo param
* @returns repoConfigs object
*/
Expand Down Expand Up @@ -844,6 +877,20 @@ ${this.results.reduce((x, y) => {
}
}

const validation = this.validateConfig(content)

if (validation instanceof Error) {
this.appendToResults([{
type: 'ERROR',
plugin: 'settings',
repo: this.repo.repo,
action: {
msg: `${namespacedFilepath} fails schema validation:\n${validation.message}`
}
}])
this.logError(`${namespacedFilepath} fails schema validation:\n${validation.message}`)
}

return content
} catch (e) {
if (e.status === 404) {
Expand Down
102 changes: 88 additions & 14 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
"dependencies": {
"@apidevtools/json-schema-ref-parser": "^12.0.2",
"@probot/adapter-aws-lambda-serverless": "^4.0.3",
"ajv": "^8.17.1",
"deepmerge": "^4.3.1",
"eta": "^3.5.0",
"js-yaml": "^4.1.0",
Expand Down
47 changes: 47 additions & 0 deletions test/unit/lib/settings-validation.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/* eslint-disable no-undef */
const Settings = require('../../../lib/settings')

jest.mock('../../../lib/env', () => ({
VALIDATE_CONFIG_SCHEMA: true,
CONFIG_PATH: '.github',
SETTINGS_FILE_PATH: 'settings.yml'
}))

const context = {
log: {
debug: jest.fn(),
info: jest.fn(),
error: jest.fn()
},
payload: {
installation: {
id: 123
}
}
}

describe('Settings Validation Tests', () => {
it('should validate config schema when VALIDATE_CONFIG_SCHEMA is true', async () => {
const settings = new Settings(true, context, {}, {
repositories: {
has_wiki: 'nonsense'
}
}, 'main', 'github')

expect(settings.results).toContainEqual(expect.objectContaining({
action: {
msg: expect.stringContaining('has_wiki must be boolean')
}
}))
})

it('should pass valid config', async () => {
const settings = new Settings(false, context, {}, {
repositories: {
has_wiki: true
}
}, 'main', 'github')

expect(settings.results).toEqual([])
})
}) // Settings Tests
2 changes: 0 additions & 2 deletions test/unit/lib/settings.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,6 @@ repository:
}
}



mockRepo = { owner: 'test', repo: 'test-repo' }
mockRef = 'main'
mockSubOrg = 'frontend'
Expand Down