From eb4b2a158121daa1c14a549d35292b59d3bc6b1e Mon Sep 17 00:00:00 2001 From: Nathan219 Date: Thu, 23 Feb 2017 17:26:29 -0800 Subject: [PATCH 1/9] Charon calls GET instance/id/dependencies/hostname Check if the incoming hostname is an alias, if it's not, do the normal dep logic --- lib/models/mongo/instance.js | 90 ++++++++++++++++++++++++++++-------- 1 file changed, 72 insertions(+), 18 deletions(-) diff --git a/lib/models/mongo/instance.js b/lib/models/mongo/instance.js index 49eba3e2a..7dadd7650 100644 --- a/lib/models/mongo/instance.js +++ b/lib/models/mongo/instance.js @@ -1162,33 +1162,87 @@ InstanceSchema.methods.getDependencies = function (params, cb) { return Instance.findByIdAsync(this._id) .tap(function (instance) { if (!instance) { - // the update failed - throw Boom.notFound('This instance could not be found!', { + throw new Instance.NotFoundError('This instance could not be found!', { instance: self._id.toString() }) } }) - .then(function (instance) { - return instance.dependencies || [] - }) - .filter(function removeSelf (dep) { - return self._id.toString() !== dep.id - }) - .filter(function filterByHostname (dep) { - if (!params.hostname) { - return true - } - return params.hostname === dep.elasticHostname + .then(instance => { + // this method will throw if there isn't a hostname, or an alias. So use that to control + // the flow of this + return instance.convertAliasToDependency(params.hostname) + .then(dependency => [dependency]) + .catch(Instance.NotFoundError, () => { + // It wasn't an alias, so maybe it's a dep? + let dependencies = instance.dependencies || [] + if (params.hostname) { + // Only get the dependency that matches the hostname + dependencies = dependencies.filter(dep => params.hostname === dep.elasticHostname) + } else { + // Remove self from the list (if it exists) + dependencies = dependencies.filter(dep => self._id.toString() !== dep.id) + } + // Annotate dependencies with additional instance information (currently + // only adding network information for charon) + return Promise + .map(dependencies, dep => Instance.findByIdAsync(dep.instanceId)) + .filter(exists) + }) }) - .map(function fetchInstance (dep) { - // Annotate dependencies with additional instance information (currently - // only adding network information for charon) - return Instance.findByIdAsync(dep.instanceId) + .catch(Instance.NotFoundError, err => { + // the update failed + throw Boom.notFound('This instance could not be found!', { err }) }) - .filter(exists) .asCallback(cb) } +/** + * Given an alias (hostname), find the instance being referred to, and resolve it + * + * @param {String} alias - Hostname that some instance is using to reference another instance + * @resolves {Instance} - Dependent Instance model referenced by the given alias + * @throws Instance.NotFoundError - When no alias is given + * @throws Instance.NotFoundError - When the alias isn't present in this Instance + * @throws Instance.NotFoundError - When we're looking for a dependency that isn't a masterpod nor isolated + * @throws Instance.NotFoundError - When the Dependency Instance fetch fails + * + */ +InstanceSchema.methods.convertAliasToDependency = function (alias) { + var log = logger.log.child({ + instanceId: keypather.get(this, '_id'), + aliases: keypather.get(this, 'aliases'), + alias, + method: 'InstanceSchema.methods.convertAliasesToDeps' + }) + log.info('called') + return Promise.try(() => { + if (!alias) { + throw new Instance.NotFoundError('No alias was given') + } + const base64Alias = new Buffer(alias).toString('base64') + const aliasModel = this.aliases[base64Alias] + if (!aliasModel) { + throw new Instance.NotFoundError('Alias was not found') + } + const query = { + 'contextVersion.context._id': aliasModel.contextId + } + if (this.masterPod) { + query.masterPod = true + } else if (this.isolation) { + query.isolation = this.isolation + } else { + throw new Instance.NotFoundError('Not masterpod and not isolated!') + } + return Instance.findOneBy(query) + .then(instance => { + if (!instance) { + throw new Instance.NotFoundError('Instance Dependency (by alias) could not be found', query) + } + }) + }) +} + /** * Fetch all of the MasterPods that should be autoForked, given the list of instances which should * be autoDeployed. By using this list, we can find all of the child instances that were updated. From cdfaea4ca849ce5a4d393ce4173eab7da7858e19 Mon Sep 17 00:00:00 2001 From: Nathan219 Date: Mon, 27 Feb 2017 16:37:01 -0800 Subject: [PATCH 2/9] nits --- lib/models/mongo/instance.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/models/mongo/instance.js b/lib/models/mongo/instance.js index 7dadd7650..d0564db2e 100644 --- a/lib/models/mongo/instance.js +++ b/lib/models/mongo/instance.js @@ -1158,12 +1158,11 @@ InstanceSchema.methods.getDependencies = function (params, cb) { cb = params params = {} } - var self = this return Instance.findByIdAsync(this._id) - .tap(function (instance) { + .tap(instance => { if (!instance) { throw new Instance.NotFoundError('This instance could not be found!', { - instance: self._id.toString() + instance: this._id.toString() }) } }) @@ -1180,7 +1179,7 @@ InstanceSchema.methods.getDependencies = function (params, cb) { dependencies = dependencies.filter(dep => params.hostname === dep.elasticHostname) } else { // Remove self from the list (if it exists) - dependencies = dependencies.filter(dep => self._id.toString() !== dep.id) + dependencies = dependencies.filter(dep => this._id.toString() !== dep.id) } // Annotate dependencies with additional instance information (currently // only adding network information for charon) @@ -1235,7 +1234,7 @@ InstanceSchema.methods.convertAliasToDependency = function (alias) { throw new Instance.NotFoundError('Not masterpod and not isolated!') } return Instance.findOneBy(query) - .then(instance => { + .tap(instance => { if (!instance) { throw new Instance.NotFoundError('Instance Dependency (by alias) could not be found', query) } From 5a337a845dbfd1afeab995826dcda0db9ccd4a10 Mon Sep 17 00:00:00 2001 From: Nathan219 Date: Mon, 27 Feb 2017 17:50:40 -0800 Subject: [PATCH 3/9] fixing a lot of the logic Adding more specific errors Add unit tests --- lib/models/mongo/instance.js | 30 +++++----- unit/models/mongo/instances.js | 104 ++++++++++++++++++++++++++++++++- 2 files changed, 119 insertions(+), 15 deletions(-) diff --git a/lib/models/mongo/instance.js b/lib/models/mongo/instance.js index d0564db2e..1e621586f 100644 --- a/lib/models/mongo/instance.js +++ b/lib/models/mongo/instance.js @@ -1171,7 +1171,7 @@ InstanceSchema.methods.getDependencies = function (params, cb) { // the flow of this return instance.convertAliasToDependency(params.hostname) .then(dependency => [dependency]) - .catch(Instance.NotFoundError, () => { + .catch(Instance.NotFoundError, Instance.IncorrectStateError, () => { // It wasn't an alias, so maybe it's a dep? let dependencies = instance.dependencies || [] if (params.hostname) { @@ -1199,11 +1199,14 @@ InstanceSchema.methods.getDependencies = function (params, cb) { * Given an alias (hostname), find the instance being referred to, and resolve it * * @param {String} alias - Hostname that some instance is using to reference another instance - * @resolves {Instance} - Dependent Instance model referenced by the given alias - * @throws Instance.NotFoundError - When no alias is given - * @throws Instance.NotFoundError - When the alias isn't present in this Instance - * @throws Instance.NotFoundError - When we're looking for a dependency that isn't a masterpod nor isolated - * @throws Instance.NotFoundError - When the Dependency Instance fetch fails + * + * @resolves {Instance} - Dependent Instance model referenced by the given alias + * + * @throws Instance.IncorrectStateError - When we're looking for a dependency in an instance + * isn't a masterpod nor isolated + * @throws Instance.NotFoundError - When no alias is given + * @throws Instance.NotFoundError - When the alias isn't present in this Instance + * @throws Instance.NotFoundError - When the Dependency Instance fetch fails * */ InstanceSchema.methods.convertAliasToDependency = function (alias) { @@ -1216,27 +1219,28 @@ InstanceSchema.methods.convertAliasToDependency = function (alias) { log.info('called') return Promise.try(() => { if (!alias) { - throw new Instance.NotFoundError('No alias was given') + throw new Instance.NotFoundError({ alias }) } const base64Alias = new Buffer(alias).toString('base64') const aliasModel = this.aliases[base64Alias] if (!aliasModel) { - throw new Instance.NotFoundError('Alias was not found') + throw new Instance.NotFoundError({ alias, base64Alias, aliases: this.aliases }) } const query = { 'contextVersion.context._id': aliasModel.contextId } if (this.masterPod) { query.masterPod = true - } else if (this.isolation) { - query.isolation = this.isolation + } else if (this.isolated) { + query.isolated = this.isolated } else { - throw new Instance.NotFoundError('Not masterpod and not isolated!') + // This shouldn't happen, so if it does, alert Nathan + throw new Instance.IncorrectStateError('be masterPod or isolated', 'neither') } - return Instance.findOneBy(query) + return Instance.findOneByAsync(query) .tap(instance => { if (!instance) { - throw new Instance.NotFoundError('Instance Dependency (by alias) could not be found', query) + throw new Instance.NotFoundError(query) } }) }) diff --git a/unit/models/mongo/instances.js b/unit/models/mongo/instances.js index e913e52d7..83d7db4e2 100644 --- a/unit/models/mongo/instances.js +++ b/unit/models/mongo/instances.js @@ -2152,7 +2152,7 @@ describe('Instance Model Tests', function () { testContextVersionId, testContainerId, testContainerInfo - ).asCallback((err) => { + ).asCallback(err => { expect(err).to.be.an.instanceOf(Instance.NotFoundError) done() }) @@ -2165,7 +2165,7 @@ describe('Instance Model Tests', function () { testContextVersionId, testContainerId, testContainerInfo - ).asCallback((err) => { + ).asCallback(err => { if (err) { return done(err) } sinon.assert.calledOnce(Instance.findOneAndUpdateAsync) sinon.assert.calledWith(Instance.findOneAndUpdateAsync, { @@ -2187,4 +2187,104 @@ describe('Instance Model Tests', function () { }) }) }) // end markAsCreating + + describe('convertAliasToDependency', function () { + let instance + const key = 'hello' + const base64Key = new Buffer(key).toString('base64') + const contextId = 'asdasasdasd' + const isolatedId = '12312312=321' + + beforeEach(function (done) { + instance = mongoFactory.createNewInstance() + instance._doc.aliases[base64Key] = { + id: key, + contextId: contextId + } + instance._doc.masterPod = true + sinon.stub(Instance, 'findOneByAsync').resolves() + done() + }) + + afterEach(function (done) { + Instance.findOneByAsync.restore() + done() + }) + describe('errors', function () { + describe('NotFound', function () { + it('should throw when no alias given', (done) => { + instance.convertAliasToDependency() + .catch(err => { + expect(err).to.be.an.instanceOf(Instance.NotFoundError) + }) + .asCallback(done) + }) + it('should throw when alias not in instance', (done) => { + instance.convertAliasToDependency('asdasdas') + .catch(err => { + expect(err).to.be.an.instanceOf(Instance.NotFoundError) + expect(err.data.aliases).to.equal(instance._doc.aliases) + }) + .asCallback(done) + }) + it('should throw when instance not masterPod nor isolated', (done) => { + delete instance._doc.masterPod + delete instance._doc.isolated + instance.convertAliasToDependency(key) + .catch(err => { + expect(err).to.be.an.instanceOf(Instance.IncorrectStateError) + }) + .asCallback(done) + }) + it('should throw when instance not returned from Mongo', (done) => { + instance.convertAliasToDependency(key) + .catch(err => { + expect(err).to.be.an.instanceOf(Instance.NotFoundError) + }) + .asCallback(done) + }) + }) + }) + describe('Success', function () { + let depInstance + beforeEach(function (done) { + depInstance = mongoFactory.createNewInstance('dep') + Instance.findOneByAsync.resolves(depInstance) + done() + }) + describe('masterPod', function () { + it('should return instance when alias matches', (done) => { + instance.convertAliasToDependency(key) + .then(dep => { + expect(dep._id).to.equal(depInstance._id) + sinon.assert.calledOnce(Instance.findOneByAsync) + sinon.assert.calledWith(Instance.findOneByAsync, { + 'contextVersion.context._id': contextId, + masterPod: true + }) + }) + .asCallback(done) + }) + }) + describe('isolated', function () { + beforeEach(function (done) { + delete instance._doc.masterPod + instance._doc.isolated = isolatedId + done() + }) + it('should return instance when alias matches', (done) => { + instance.convertAliasToDependency(key) + .then(dep => { + expect(dep._id).to.equal(depInstance._id) + sinon.assert.calledOnce(Instance.findOneByAsync) + sinon.assert.calledWith(Instance.findOneByAsync, { + 'contextVersion.context._id': contextId, + isolated: isolatedId + }) + }) + .asCallback(done) + }) + }) + }) + }) }) From b2ea7cb74353ff7208fec5fcc01648f0738864ab Mon Sep 17 00:00:00 2001 From: Nathan219 Date: Mon, 20 Mar 2017 14:12:53 -0700 Subject: [PATCH 4/9] Remove amazon ending on Aliases --- configs/.env.development | 1 + lib/routes/instances/dependencies/index.js | 3 +++ 2 files changed, 4 insertions(+) diff --git a/configs/.env.development b/configs/.env.development index ed70fbf61..da11b46b5 100644 --- a/configs/.env.development +++ b/configs/.env.development @@ -25,3 +25,4 @@ REDIS_IPADDRESS=127.0.0.1 REDIS_PORT=6379 S3_CONTEXT_RESOURCE_BUCKET=runnable.context.resources.development ALLOW_ALL_CORS=true +AWS_ALIAS_HOST=us-west-2.compute.internal \ No newline at end of file diff --git a/lib/routes/instances/dependencies/index.js b/lib/routes/instances/dependencies/index.js index ab701d0b2..b6827bf29 100644 --- a/lib/routes/instances/dependencies/index.js +++ b/lib/routes/instances/dependencies/index.js @@ -23,12 +23,15 @@ app.all('/instances/:id/dependencies*', }) }) +var AWS_ALIAS_HOST = new RegExp('.' + process.env.AWS_ALIAS_HOST + '$', 'i') + app.get('/instances/:id/dependencies', mw.query('hostname').pick(), mw.query('hostname').require().then( mw.query('hostname').string(), mw.query().set('hostname', 'query.hostname.toLowerCase()')), function (req, res, next) { + req.query.hostname = req.query.hostname.replace(AWS_ALIAS_HOST, '') req.instance.getDependenciesAsync(req.query) .then(function (deps) { res.send(200, deps) From 0612cdf8c784d0e3d1e6f8b5700ed105f5eaa135 Mon Sep 17 00:00:00 2001 From: Nathan219 Date: Mon, 20 Mar 2017 14:21:46 -0700 Subject: [PATCH 5/9] need escape . --- lib/routes/instances/dependencies/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/routes/instances/dependencies/index.js b/lib/routes/instances/dependencies/index.js index b6827bf29..544d4fde3 100644 --- a/lib/routes/instances/dependencies/index.js +++ b/lib/routes/instances/dependencies/index.js @@ -23,7 +23,7 @@ app.all('/instances/:id/dependencies*', }) }) -var AWS_ALIAS_HOST = new RegExp('.' + process.env.AWS_ALIAS_HOST + '$', 'i') +const AWS_ALIAS_HOST = new RegExp('\.' + process.env.AWS_ALIAS_HOST + '$', 'i') app.get('/instances/:id/dependencies', mw.query('hostname').pick(), From 520ebf4def5c468270a1a29952c5a4b13dd4c974 Mon Sep 17 00:00:00 2001 From: Nathan219 Date: Mon, 20 Mar 2017 14:33:23 -0700 Subject: [PATCH 6/9] fix cleaning up hostname --- lib/routes/instances/dependencies/index.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/routes/instances/dependencies/index.js b/lib/routes/instances/dependencies/index.js index 544d4fde3..52d00f864 100644 --- a/lib/routes/instances/dependencies/index.js +++ b/lib/routes/instances/dependencies/index.js @@ -29,7 +29,10 @@ app.get('/instances/:id/dependencies', mw.query('hostname').pick(), mw.query('hostname').require().then( mw.query('hostname').string(), - mw.query().set('hostname', 'query.hostname.toLowerCase()')), + function (req, res, next) { + req.query.hostname = req.query.hostname.replace(AWS_ALIAS_HOST, '').toLowerCase() + next() + }), function (req, res, next) { req.query.hostname = req.query.hostname.replace(AWS_ALIAS_HOST, '') req.instance.getDependenciesAsync(req.query) From f893195fbdfb4637e5bb05f50038a8c406412104 Mon Sep 17 00:00:00 2001 From: Nathan219 Date: Mon, 20 Mar 2017 14:42:15 -0700 Subject: [PATCH 7/9] didn't remove the original (This is why we test) --- lib/routes/instances/dependencies/index.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/routes/instances/dependencies/index.js b/lib/routes/instances/dependencies/index.js index 52d00f864..625fdf8c6 100644 --- a/lib/routes/instances/dependencies/index.js +++ b/lib/routes/instances/dependencies/index.js @@ -34,7 +34,6 @@ app.get('/instances/:id/dependencies', next() }), function (req, res, next) { - req.query.hostname = req.query.hostname.replace(AWS_ALIAS_HOST, '') req.instance.getDependenciesAsync(req.query) .then(function (deps) { res.send(200, deps) From cfcb1bef3811792f60a019cb473952d6027d4b55 Mon Sep 17 00:00:00 2001 From: Nathan219 Date: Mon, 20 Mar 2017 15:28:12 -0700 Subject: [PATCH 8/9] fix the instance fetch --- lib/models/mongo/instance.js | 4 ++-- unit/models/mongo/instances.js | 18 +++++++++--------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/models/mongo/instance.js b/lib/models/mongo/instance.js index 0d485ca85..4e2c570e4 100644 --- a/lib/models/mongo/instance.js +++ b/lib/models/mongo/instance.js @@ -1227,7 +1227,7 @@ InstanceSchema.methods.convertAliasToDependency = function (alias) { throw new Instance.NotFoundError({ alias, base64Alias, aliases: this.aliases }) } const query = { - 'contextVersion.context._id': aliasModel.contextId + 'contextVersion.context': aliasModel.contextId } if (this.masterPod) { query.masterPod = true @@ -1237,7 +1237,7 @@ InstanceSchema.methods.convertAliasToDependency = function (alias) { // This shouldn't happen, so if it does, alert Nathan throw new Instance.IncorrectStateError('be masterPod or isolated', 'neither') } - return Instance.findOneByAsync(query) + return Instance.findOneAsync(query) .tap(instance => { if (!instance) { throw new Instance.NotFoundError(query) diff --git a/unit/models/mongo/instances.js b/unit/models/mongo/instances.js index 0a863a506..17a42230f 100644 --- a/unit/models/mongo/instances.js +++ b/unit/models/mongo/instances.js @@ -2249,12 +2249,12 @@ describe('Instance Model Tests', function () { contextId: contextId } instance._doc.masterPod = true - sinon.stub(Instance, 'findOneByAsync').resolves() + sinon.stub(Instance, 'findOneAsync').resolves() done() }) afterEach(function (done) { - Instance.findOneByAsync.restore() + Instance.findOneAsync.restore() done() }) describe('errors', function () { @@ -2296,7 +2296,7 @@ describe('Instance Model Tests', function () { let depInstance beforeEach(function (done) { depInstance = mongoFactory.createNewInstance('dep') - Instance.findOneByAsync.resolves(depInstance) + Instance.findOneAsync.resolves(depInstance) done() }) describe('masterPod', function () { @@ -2304,9 +2304,9 @@ describe('Instance Model Tests', function () { instance.convertAliasToDependency(key) .then(dep => { expect(dep._id).to.equal(depInstance._id) - sinon.assert.calledOnce(Instance.findOneByAsync) - sinon.assert.calledWith(Instance.findOneByAsync, { - 'contextVersion.context._id': contextId, + sinon.assert.calledOnce(Instance.findOneAsync) + sinon.assert.calledWith(Instance.findOneAsync, { + 'contextVersion.context': contextId, masterPod: true }) }) @@ -2323,9 +2323,9 @@ describe('Instance Model Tests', function () { instance.convertAliasToDependency(key) .then(dep => { expect(dep._id).to.equal(depInstance._id) - sinon.assert.calledOnce(Instance.findOneByAsync) - sinon.assert.calledWith(Instance.findOneByAsync, { - 'contextVersion.context._id': contextId, + sinon.assert.calledOnce(Instance.findOneAsync) + sinon.assert.calledWith(Instance.findOneAsync, { + 'contextVersion.context': contextId, isolated: isolatedId }) }) From d967846f5e72de924f476b15c95633644a3bf811 Mon Sep 17 00:00:00 2001 From: Nathan219 Date: Mon, 20 Mar 2017 17:36:15 -0700 Subject: [PATCH 9/9] Make kahn happy --- configs/.env.development | 2 +- lib/models/mongo/instance.js | 56 +++++++++++++++++------------------- 2 files changed, 28 insertions(+), 30 deletions(-) diff --git a/configs/.env.development b/configs/.env.development index da11b46b5..5afce8f17 100644 --- a/configs/.env.development +++ b/configs/.env.development @@ -25,4 +25,4 @@ REDIS_IPADDRESS=127.0.0.1 REDIS_PORT=6379 S3_CONTEXT_RESOURCE_BUCKET=runnable.context.resources.development ALLOW_ALL_CORS=true -AWS_ALIAS_HOST=us-west-2.compute.internal \ No newline at end of file +AWS_ALIAS_HOST=us-west-2.compute.internal diff --git a/lib/models/mongo/instance.js b/lib/models/mongo/instance.js index 4e2c570e4..0cd9a15b2 100644 --- a/lib/models/mongo/instance.js +++ b/lib/models/mongo/instance.js @@ -1209,7 +1209,7 @@ InstanceSchema.methods.getDependencies = function (params, cb) { * @throws Instance.NotFoundError - When the Dependency Instance fetch fails * */ -InstanceSchema.methods.convertAliasToDependency = function (alias) { +InstanceSchema.methods.convertAliasToDependency = Promise.method(function (alias) { var log = logger.log.child({ instanceId: keypather.get(this, '_id'), aliases: keypather.get(this, 'aliases'), @@ -1217,34 +1217,32 @@ InstanceSchema.methods.convertAliasToDependency = function (alias) { method: 'InstanceSchema.methods.convertAliasesToDeps' }) log.info('called') - return Promise.try(() => { - if (!alias) { - throw new Instance.NotFoundError({ alias }) - } - const base64Alias = new Buffer(alias).toString('base64') - const aliasModel = this.aliases[base64Alias] - if (!aliasModel) { - throw new Instance.NotFoundError({ alias, base64Alias, aliases: this.aliases }) - } - const query = { - 'contextVersion.context': aliasModel.contextId - } - if (this.masterPod) { - query.masterPod = true - } else if (this.isolated) { - query.isolated = this.isolated - } else { - // This shouldn't happen, so if it does, alert Nathan - throw new Instance.IncorrectStateError('be masterPod or isolated', 'neither') - } - return Instance.findOneAsync(query) - .tap(instance => { - if (!instance) { - throw new Instance.NotFoundError(query) - } - }) - }) -} + if (!alias) { + throw new Instance.NotFoundError({ alias }) + } + const base64Alias = new Buffer(alias).toString('base64') + const aliasModel = this.aliases[base64Alias] + if (!aliasModel) { + throw new Instance.NotFoundError({ alias, base64Alias, aliases: this.aliases }) + } + const query = { + 'contextVersion.context': aliasModel.contextId + } + if (this.masterPod) { + query.masterPod = true + } else if (this.isolated) { + query.isolated = this.isolated + } else { + // This shouldn't happen, so if it does, alert Nathan + throw new Instance.IncorrectStateError('be masterPod or isolated', 'neither') + } + return Instance.findOneAsync(query) + .tap(instance => { + if (!instance) { + throw new Instance.NotFoundError(query) + } + }) +}) /** * Fetch all of the MasterPods that should be autoForked, given the list of instances which should