Add joi for config validation (#30)

package.json

@@ -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",

src/lib/configs.js

@@ -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();
@@ -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;
@@ -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() {
@@ -189,5 +186,7 @@ module.exports = {
   maybeCreateGlobalConfig,
   getCombinedConfig,
   _getCombinedConfig,
-  mergeConfigs
+  mergeConfigs,
+  validateProjectConfig,
+  validateGlobalConfig
 };

src/lib/git.js

@@ -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)
     }

src/lib/github.js

@@ -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 =>
@@ -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 => ({
@@ -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);
@@ -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'));

src/lib/schemas.js

@@ -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
+};

test/__snapshots__/configs.test.js.snap

@@ -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 "
+`;

test/configs.test.js

@@ -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'
           );
         });
       });
@@ -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;
       });
@@ -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);
+    });
+  });
 });