Merge pull request #2043 from CodeNow/ASAP-fix-branching-on-clusters

Fix add branch multi-hook

lib/models/services/cluster-config-service.js

@@ -1557,38 +1557,39 @@ module.exports = class ClusterConfigService {
    * Checks if the given instance has a Docker Compose config, and if it does, check if compose files were changed.
    * If it doesn't, this throws a BaseSchema.NotFoundError
    *
-   * @param {Instance} instance                - Instance to look up
-   * @param {Object}   githubPushInfo          - Github webhook push data
-   * @param {String}   githubPushInfo.repo     - Github repositories full name (org/repo)
-   * @param {String}   githubPushInfo.commit   - Commit for this push
-   * @param {String}   githubPushInfo.bpUserId - BigPoppa user id
+   * @param {SessionUser} sessionUser             - sessionUser
+   * @param {Instance}    instance                - Instance to look up
+   * @param {Object}      githubPushInfo          - Github webhook push data
+   * @param {String}      githubPushInfo.repo     - Github repositories full name (org/repo)
+   * @param {String}      githubPushInfo.commit   - Commit for this push
+   * @param {String}      githubPushInfo.bpUserId - BigPoppa user id
    *
    * @resolves {undefined} resolves when the update config job has been created
    *
    * @throws InputClusterConfig.NotChangedError - When the config's sha's match, so this won't be done
    * @throws InputClusterConfig.NotFoundError - When no config is found to match the given instance
    */
-  static checkIfComposeFilesChanged (instance, githubPushInfo) {
+  static checkIfComposeFilesChanged (sessionUser, instance, githubPushInfo) {
     const log = ClusterConfigService.log.child({
       method: 'checkIfComposeFilesChanged',
       instance, githubPushInfo
     })
     log.info('called')
     return Promise.props({
       aic: AutoIsolationService.fetchAutoIsolationForInstance(instance._id),
-      clusterConfig: ClusterConfigService.fetchConfigByInstanceId(instance._id)
+      clusterConfig: ClusterConfigService.fetchComposeInfoByInstanceId(sessionUser, instance._id, githubPushInfo)
     })
       .then(results => {
-        const clusterConfig = results.clusterConfig
+        const clusterConfig = results.clusterConfig.clusterOpts
         const currentPaths = clusterConfig.files.map(pluck('path'))
         // We found a cluster, so fetch the current one, and see if it changed
         return UserService.getByBpId(githubPushInfo.bpUserId)
           .then(bpUser => {
             return ClusterConfigService.fetchFilesFromGithub(
               bpUser,
-              githubPushInfo.repo,
+              clusterConfig.repo,
               currentPaths,
-              githubPushInfo.commit
+              clusterConfig.commit
             )
           })
           .then(newComposeFiles => {
@@ -1626,22 +1627,24 @@ module.exports = class ClusterConfigService {
   /**
    * If you need to do an updateCluster job, use this method.  It checks if the compose file has changed
    * before creating the job.
-   * @param {Instance} instance                - instance to be updated
-   * @param {Object}   githubPushInfo          - parsed githook data
-   * @param {String}   githubPushInfo.bpUserId - BigPoppa user id
+   *
+   * @param {SessionUser} sessionUser             - SessionUser
+   * @param {Instance}    instance                - instance to be updated
+   * @param {Object}      githubPushInfo          - parsed githook data
+   * @param {String}      githubPushInfo.bpUserId - BigPoppa user id
    *
    * @resolves {undefined}
    *
    * @throws InputClusterConfig.NotChangedError - When the config's sha's match, so this won't be done
    * @throws InputClusterConfig.NotFoundError - When no config is found to match the given instance
    */
-  static checkFileChangeAndCreateUpdateJob (instance, githubPushInfo) {
+  static checkFileChangeAndCreateUpdateJob (sessionUser, instance, githubPushInfo) {
     const log = ClusterConfigService.log.child({
       method: 'checkFileChangeAndCreateUpdateJob',
       instance, githubPushInfo
     })
     log.info('called')
-    return ClusterConfigService.checkIfComposeFilesChanged(instance, githubPushInfo)
+    return ClusterConfigService.checkIfComposeFilesChanged(sessionUser, instance, githubPushInfo)
       .then(() => rabbitMQ.updateCluster({
         instanceId: instance._id.toString(),
         pushInfo: githubPushInfo

lib/models/services/webhook-service.js

@@ -79,7 +79,10 @@ WebhookService.updateComposeOrAutoDeploy = function (instance, githubPushInfo) {
     method: 'updateComposeOrAutoDeploy'
   })
   log.info('called')
-  return ClusterConfigService.checkFileChangeAndCreateUpdateJob(instance, githubPushInfo)
+  const committerUsername = keypather.get(githubPushInfo, 'commitPusher')
+
+  return UserService.getCompleteUserByGithubUsername(committerUsername)
+    .then((user) => ClusterConfigService.checkFileChangeAndCreateUpdateJob(user, instance, githubPushInfo))
     .catch((err) => {
       log.warn({ err }, 'Cluster could not be checked or updated')
       return BuildService.createAndBuildContextVersion(instance, githubPushInfo, 'autodeploy')
@@ -150,13 +153,22 @@ WebhookService.autoFork = function (autoDeployedInstances, githubPushInfo) {
     method: 'autoFork'
   })
   log.info('called')
-
-  return Instance.findMasterPodsToAutoFork(githubPushInfo.repo, githubPushInfo.branch, autoDeployedInstances)
+  const committerUsername = keypather.get(githubPushInfo, 'commitPusher')
+  return Promise.props({
+    sessionUser: UserService.getCompleteUserByGithubUsername(committerUsername),
+    instances: Instance.findMasterPodsToAutoFork(
+      githubPushInfo.repo,
+      githubPushInfo.branch,
+      autoDeployedInstances
+    )
+  })
     .catch(function (err) {
       log.error({ err }, 'error attempting to fetch autoForking candidates')
       throw err
     })
-    .then(function (instances) {
+    .then(function (results) {
+      const instances = results.instances
+      const sessionUser = results.sessionUser
       if (!instances.length) {
         log.info('found no instances to fork')
         return null
@@ -166,7 +178,7 @@ WebhookService.autoFork = function (autoDeployedInstances, githubPushInfo) {
           return IsolationService.autoIsolate(newInstances, githubPushInfo)
         })
         .each(newInstance =>
-          ClusterConfigService.checkFileChangeAndCreateUpdateJob(newInstance, githubPushInfo)
+          ClusterConfigService.checkFileChangeAndCreateUpdateJob(sessionUser, newInstance, githubPushInfo)
             .catch(err => {
               log.warn({ err }, 'Cluster could not be checked or updated')
             })

lib/routes/instances/index.js

@@ -526,7 +526,7 @@ module.exports.forkInstance = function (req) {
       return IsolationService.autoIsolate([newInstance], githubPushInfo)
     })
     .tap(newInstance =>
-      ClusterConfigService.checkFileChangeAndCreateUpdateJob(newInstance, githubPushInfo)
+      ClusterConfigService.checkFileChangeAndCreateUpdateJob(req.sessionUser, newInstance, githubPushInfo)
         .catch(err => {
           log.warn({err}, 'Cluster could not be checked or updated')
         })

unit/models/services/cluster-config-service.js

@@ -1840,16 +1840,35 @@ describe('Cluster Config Service Unit Tests', function () {
     const instance = {
       _id: instanceId
     }
-
+    const testSessionUser = {
+      _id: 'id',
+      accounts: {
+        github: {
+          id: testUserGithubId,
+          accessToken: 'some-token'
+        },
+        login: 'login',
+        username: 'best'
+      },
+      bigPoppaUser: {
+        id: testUserBpId,
+        organizations: [{
+          name: testOrgName,
+          lowerName: testOrgName.toLowerCase(),
+          id: testOrgBpId,
+          githubId: testOrgGithubId
+        }]
+      }
+    }
     beforeEach(function (done) {
-      sinon.stub(ClusterConfigService, 'fetchConfigByInstanceId').resolves(clusterConfig)
+      sinon.stub(ClusterConfigService, 'fetchComposeInfoByInstanceId').resolves({ clusterOpts: clusterConfig })
       sinon.stub(AutoIsolationService, 'fetchAutoIsolationForInstance').resolves(aic)
       sinon.stub(UserService, 'getByBpId').resolves(userModel)
       sinon.stub(ClusterConfigService, 'fetchFilesFromGithub').resolves(changedClusterConfigFiles)
       done()
     })
     afterEach(function (done) {
-      ClusterConfigService.fetchConfigByInstanceId.restore()
+      ClusterConfigService.fetchComposeInfoByInstanceId.restore()
       AutoIsolationService.fetchAutoIsolationForInstance.restore()
       UserService.getByBpId.restore()
       ClusterConfigService.fetchFilesFromGithub.restore()
@@ -1858,8 +1877,8 @@ describe('Cluster Config Service Unit Tests', function () {
     describe('errors', function () {
       it('should return error if fetchConfigByInstanceId failed', function (done) {
         const error = new Error('Some error')
-        ClusterConfigService.fetchConfigByInstanceId.rejects(error)
-        ClusterConfigService.checkIfComposeFilesChanged(instance, githubPushInfo)
+        ClusterConfigService.fetchComposeInfoByInstanceId.rejects(error)
+        ClusterConfigService.checkIfComposeFilesChanged(testSessionUser, instance, githubPushInfo)
           .asCallback(function (err) {
             expect(err).to.exist()
             expect(err.message).to.equal(error.message)
@@ -1869,7 +1888,7 @@ describe('Cluster Config Service Unit Tests', function () {
       it('should return error if UserService.getByBpId failed', function (done) {
         const error = new Error('Some error')
         UserService.getByBpId.rejects(error)
-        ClusterConfigService.checkIfComposeFilesChanged(instance, githubPushInfo)
+        ClusterConfigService.checkIfComposeFilesChanged(testSessionUser, instance, githubPushInfo)
           .asCallback(function (err) {
             expect(err).to.exist()
             expect(err.message).to.equal(error.message)
@@ -1879,7 +1898,7 @@ describe('Cluster Config Service Unit Tests', function () {
       it('should return error if ClusterConfigService.fetchFilesFromGithub failed', function (done) {
         const error = new Error('Some error')
         ClusterConfigService.fetchFilesFromGithub.rejects(error)
-        ClusterConfigService.checkIfComposeFilesChanged(instance, githubPushInfo)
+        ClusterConfigService.checkIfComposeFilesChanged(testSessionUser, instance, githubPushInfo)
           .asCallback(function (err) {
             expect(err).to.exist()
             expect(err.message).to.equal(error.message)
@@ -1890,11 +1909,11 @@ describe('Cluster Config Service Unit Tests', function () {
     describe('success', function () {
       it('should run successfully', function () {
         ClusterConfigService.fetchFilesFromGithub.resolves(changedClusterConfigFiles)
-        return ClusterConfigService.checkIfComposeFilesChanged(instance, githubPushInfo)
+        return ClusterConfigService.checkIfComposeFilesChanged(testSessionUser, instance, githubPushInfo)
       })
       it('should return InputClusterConfig.NotChangedError if shas match', function (done) {
         ClusterConfigService.fetchFilesFromGithub.resolves(clusterConfigFiles)
-        ClusterConfigService.checkIfComposeFilesChanged(instance, githubPushInfo)
+        ClusterConfigService.checkIfComposeFilesChanged(testSessionUser, instance, githubPushInfo)
           .then(function () {
             done(new Error('Expecting NotChangedError'))
           })
@@ -1905,7 +1924,7 @@ describe('Cluster Config Service Unit Tests', function () {
       it('should not return InputClusterConfig.NotChangedError if aic is parent\'s', function () {
         ClusterConfigService.fetchFilesFromGithub.resolves(changedClusterConfigFiles)
         AutoIsolationService.fetchAutoIsolationForInstance.resolves(parentAic)
-        return ClusterConfigService.checkIfComposeFilesChanged(instance, githubPushInfo)
+        return ClusterConfigService.checkIfComposeFilesChanged(testSessionUser, instance, githubPushInfo)
       })
     })
   })

unit/models/services/webhook-service.js

@@ -184,18 +184,27 @@ describe('Webhook Service Unit Tests', function () {
       repo: 'theRepo',
       branch: 'theBranch'
     }
+    const user = {
+      _id: 'some-id',
+      allowed: true,
+      bigPoppaUser: {
+        id: 'bp-id'
+      }
+    }
     var instance
     beforeEach(function (done) {
       instance = {
         _id: 'sdasdsaddgfasdfgasdfasdf'
       }
       sinon.stub(BuildService, 'createAndBuildContextVersion')
+      sinon.stub(UserService, 'getCompleteUserByGithubUsername').resolves(user)
       sinon.stub(ClusterConfigService, 'checkIfComposeFilesChanged').rejects(new Error())
       sinon.stub(rabbitMQ, 'updateCluster').resolves()
       done()
     })
     afterEach(function (done) {
       BuildService.createAndBuildContextVersion.restore()
+      UserService.getCompleteUserByGithubUsername.restore()
       ClusterConfigService.checkIfComposeFilesChanged.restore()
       rabbitMQ.updateCluster.restore()
       done()
@@ -255,6 +264,13 @@ describe('Webhook Service Unit Tests', function () {
       repo: 'theRepo',
       branch: 'theBranch'
     }
+    const user = {
+      _id: 'some-id',
+      allowed: true,
+      bigPoppaUser: {
+        id: 'bp-id'
+      }
+    }
     var instances
     var contextIds
     beforeEach(function (done) {
@@ -266,15 +282,17 @@ describe('Webhook Service Unit Tests', function () {
       contextIds = ['sadsadasdsad', 'sdgfddfsgdfsgsdfgdsfg']
       sinon.stub(Instance, 'findMasterPodsToAutoFork')
       sinon.stub(ClusterConfigService, 'checkIfComposeFilesChanged').resolves()
+      sinon.stub(UserService, 'getCompleteUserByGithubUsername').resolves(user)
       sinon.stub(InstanceForkService, 'autoFork')
       sinon.stub(IsolationService, 'autoIsolate')
-      sinon.stub(rabbitMQ, 'updateCluster').resolves
+      sinon.stub(rabbitMQ, 'updateCluster').resolves()
       done()
     })
     afterEach(function (done) {
       Instance.findMasterPodsToAutoFork.restore()
       ClusterConfigService.checkIfComposeFilesChanged.restore()
       InstanceForkService.autoFork.restore()
+      UserService.getCompleteUserByGithubUsername.restore()
       IsolationService.autoIsolate.restore()
       rabbitMQ.updateCluster.restore()
       done()
@@ -387,11 +405,13 @@ describe('Webhook Service Unit Tests', function () {
             sinon.assert.calledTwice(ClusterConfigService.checkIfComposeFilesChanged)
             sinon.assert.calledWithExactly(
               ClusterConfigService.checkIfComposeFilesChanged,
+              user,
               forkedInstances[0],
               githubPushInfo
             )
             sinon.assert.calledWithExactly(
               ClusterConfigService.checkIfComposeFilesChanged,
+              user,
               forkedInstances[1],
               githubPushInfo
             )

unit/routes/instances.js

@@ -127,6 +127,7 @@ describe('/instances', () => {
         .then(function () {
           sinon.assert.calledOnce(ClusterConfigService.checkFileChangeAndCreateUpdateJob)
           sinon.assert.calledWith(ClusterConfigService.checkFileChangeAndCreateUpdateJob,
+            mockReq.sessionUser,
             mockForkedInstance,
             {
               repo: 'repoName',