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.

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

Add joi for config validation #30

Merged
merged 4 commits into from
Dec 8, 2017
Merged
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
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
"es6-promisify": "^5.0.0",
"find-up": "^2.1.0",
"inquirer": "^4.0.0",
"joi": "^12.0.0",
"lodash.get": "^4.4.2",
"lodash.isempty": "^4.4.0",
"mkdirp": "^0.5.1",
Expand Down
59 changes: 29 additions & 30 deletions src/lib/configs.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@ const fs = require('fs');
const isEmpty = require('lodash.isempty');
const get = require('lodash.get');
const stripJsonComments = require('strip-json-comments');
const Joi = require('joi');
const constants = require('./constants');
const env = require('./env');
const rpc = require('./rpc');
const prompts = require('./prompts');
const schemas = require('./schemas');

function maybeCreateGlobalConfigAndFolder() {
const REPOS_PATH = env.getReposPath();
Expand Down Expand Up @@ -43,48 +45,43 @@ class InvalidConfigError extends Error {
}
}

function validateGlobalConfig(config) {
const { username, accessToken } = config;
const GLOBAL_CONFIG_PATH = env.getGlobalConfigPath();

if (!username && !accessToken) {
throw new InvalidConfigError(
`Please add your Github username, and Github access token to the config: ${
GLOBAL_CONFIG_PATH
}`
);
}

if (!username) {
throw new InvalidConfigError(
`Please add your Github username to the config: ${GLOBAL_CONFIG_PATH}`
);
}
function validateGlobalConfig(config, filename) {
const { error } = Joi.validate(
config,
schemas.globalConfig,
schemas.joiOptions
);

if (!accessToken) {
if (error) {
throw new InvalidConfigError(
`Please add your Github access token to the config: ${GLOBAL_CONFIG_PATH}`
`The global config file (${filename}) is not valid:\n${schemas.formatError(
error
)}`
);
}

const isConfigValid = hasRestrictedPermissions(GLOBAL_CONFIG_PATH);
if (!isConfigValid) {
if (!hasRestrictedPermissions(filename)) {
throw new InvalidConfigError(
`Config file at ${
GLOBAL_CONFIG_PATH
} needs to have more restrictive permissions. Run the following to limit access to the file to just your user account:
chmod 600 "${GLOBAL_CONFIG_PATH}"\n`
`The global config file (${filename}) needs to have more restrictive permissions. Run the following to limit access to the file to just your user account:
chmod 600 "${filename}"\n`
);
}

return config;
}

function validateProjectConfig(config, filepath) {
const { upstream } = config;
if (!upstream) {
const { error } = Joi.validate(
config,
schemas.projectConfig,
schemas.joiOptions
);

if (error) {
throw new InvalidConfigError(
`Your config (${filepath}) must contain "upstream" property`
`The project config file (${filepath}) is not valid:\n${schemas.formatError(
error
)}`
);
}
return config;
Expand Down Expand Up @@ -121,7 +118,7 @@ function getGlobalConfig() {
const GLOBAL_CONFIG_PATH = env.getGlobalConfigPath();
return maybeCreateGlobalConfigAndFolder()
.then(() => readConfigFile(GLOBAL_CONFIG_PATH))
.then(validateGlobalConfig);
.then(config => validateGlobalConfig(config, GLOBAL_CONFIG_PATH));
}

function getProjectConfig() {
Expand Down Expand Up @@ -189,5 +186,7 @@ module.exports = {
maybeCreateGlobalConfig,
getCombinedConfig,
_getCombinedConfig,
mergeConfigs
mergeConfigs,
validateProjectConfig,
validateGlobalConfig
};
4 changes: 1 addition & 3 deletions src/lib/git.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,7 @@ function isIndexDirty(owner, repoName) {

function createAndCheckoutBranch(owner, repoName, baseBranch, featureBranch) {
return rpc.exec(
`git fetch origin ${baseBranch} && git branch ${featureBranch} origin/${
baseBranch
} --force && git checkout ${featureBranch} `,
`git fetch origin ${baseBranch} && git branch ${featureBranch} origin/${baseBranch} --force && git checkout ${featureBranch} `,
{
cwd: env.getRepoPath(owner, repoName)
}
Expand Down
22 changes: 7 additions & 15 deletions src/lib/github.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ function getCommits(owner, repoName, author) {
}

return axios(
`https://api.github.com/repos/${owner}/${
repoName
}/commits?${querystring.stringify(urlArgs)}`
`https://api.github.com/repos/${owner}/${repoName}/commits?${querystring.stringify(
urlArgs
)}`
)
.catch(handleError)
.then(res =>
Expand All @@ -38,9 +38,7 @@ function getCommits(owner, repoName, author) {

function getCommit(owner, repoName, sha) {
return axios(
`https://api.github.com/repos/${owner}/${repoName}/commits/${
sha
}?access_token=${accessToken}`
`https://api.github.com/repos/${owner}/${repoName}/commits/${sha}?access_token=${accessToken}`
)
.catch(handleError)
.then(res => ({
Expand All @@ -52,9 +50,7 @@ function getCommit(owner, repoName, sha) {
function createPullRequest(owner, repoName, payload) {
return axios
.post(
`https://api.github.com/repos/${owner}/${repoName}/pulls?access_token=${
accessToken
}`,
`https://api.github.com/repos/${owner}/${repoName}/pulls?access_token=${accessToken}`,
payload
)
.catch(handleError);
Expand All @@ -63,19 +59,15 @@ function createPullRequest(owner, repoName, payload) {
function addLabels(owner, repoName, pullNumber, labels) {
return axios
.post(
`https://api.github.com/repos/${owner}/${repoName}/issues/${
pullNumber
}/labels?access_token=${accessToken}`,
`https://api.github.com/repos/${owner}/${repoName}/issues/${pullNumber}/labels?access_token=${accessToken}`,
labels
)
.catch(handleError);
}

function getPullRequestByCommit(owner, repoName, commitSha) {
return axios(
`https://api.github.com/search/issues?q=repo:${owner}/${repoName}+${
commitSha
}&access_token=${accessToken}`
`https://api.github.com/search/issues?q=repo:${owner}/${repoName}+${commitSha}&access_token=${accessToken}`
)
.catch(handleError)
.then(res => get(res.data.items[0], 'number'));
Expand Down
49 changes: 49 additions & 0 deletions src/lib/schemas.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
const Joi = require('joi');

const joiOptions = {
abortEarly: false,
convert: false
};

const branchesSchema = Joi.array().items(
Joi.object().keys({
name: Joi.string().required(),
checked: Joi.bool()
}),
Joi.string()
);

const projectConfig = Joi.object().keys({
upstream: Joi.string().required(),
branches: branchesSchema,
own: Joi.bool(),
multipleCommits: Joi.bool(),
multipleBranches: Joi.bool(),
labels: Joi.array().items(Joi.string())
});

const globalConfig = Joi.object().keys({
username: Joi.string().required(),
accessToken: Joi.string().required(),
own: Joi.bool(),
multipleCommits: Joi.bool(),
multipleBranches: Joi.bool(),
projects: Joi.array().items(projectConfig)
});

const formatError = error => {
return error.details
.map(detail => {
const errorPath =
detail.path.length > 1 ? `(in ${detail.path.join('.')})` : '';
return ` - ${detail.message} ${errorPath}`;
})
.join('\n');
};

module.exports = {
joiOptions,
globalConfig,
projectConfig,
formatError
};
12 changes: 12 additions & 0 deletions test/__snapshots__/configs.test.js.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`config.js validateGlobalConfig should fail if config is invalid 1`] = `
"The global config file (/path/to/.backport/config.json) is not valid:
- \\"username\\" must be a string
- \\"accessToken\\" is required "
`;

exports[`config.js validateProjectConfig should fail if config is invalid 1`] = `
"The project config file (/path/to/.backportrc.json) is not valid:
- \\"upstream\\" must be a string "
`;
59 changes: 50 additions & 9 deletions test/configs.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ describe('config.js', () => {
expect.assertions(1);

return configs.getProjectConfig().catch(e => {
expect(e.message).toEqual(
'Your config (/path/to/config) must contain "upstream" property'
expect(e.message).toContain(
'The project config file (/path/to/config) is not valid'
);
});
});
Expand All @@ -65,13 +65,14 @@ describe('config.js', () => {
jest.spyOn(rpc, 'mkdirp').mockReturnValue(Promise.resolve());
jest.spyOn(rpc, 'writeFile').mockReturnValue(Promise.resolve());
jest.spyOn(rpc, 'statSync').mockReturnValue({ mode: 33152 });
jest
.spyOn(rpc, 'readFile')
.mockReturnValue(
Promise.resolve(
JSON.stringify({ accessToken: 'myAccessToken', username: 'sqren' })
)
);
jest.spyOn(rpc, 'readFile').mockReturnValue(
Promise.resolve(
JSON.stringify({
accessToken: 'myAccessToken',
username: 'sqren'
})
)
);
return configs.getGlobalConfig().then(res => {
this.res = res;
});
Expand Down Expand Up @@ -294,4 +295,44 @@ describe('config.js', () => {
});
});
});

describe('validateProjectConfig', () => {
it('should fail if config is invalid', () => {
expect(() =>
configs.validateProjectConfig(
{ upstream: 1337 },
'/path/to/.backportrc.json'
)
).toThrowErrorMatchingSnapshot();
});

it('should return valid config', () => {
const config = { upstream: 'elastic/kibana', branches: ['6.1', '6.x'] };
expect(
configs.validateProjectConfig(config, '/path/to/.backportrc.json')
).toBe(config);
});
});

describe('validateGlobalConfig', () => {
beforeEach(() => {
jest.spyOn(rpc, 'statSync').mockReturnValue({ mode: 33152 });
});

it('should fail if config is invalid', () => {
expect(() =>
configs.validateGlobalConfig(
{ username: 1337 },
'/path/to/.backport/config.json'
)
).toThrowErrorMatchingSnapshot();
});

it('should return valid config', () => {
const config = { username: 'sqren', accessToken: 'myAccessToken' };
expect(
configs.validateGlobalConfig(config, '/path/to/.backport/config.json')
).toBe(config);
});
});
});
Loading