diff --git a/src/controller.js b/src/controller.js index 89f15c8a..5b82e9d5 100644 --- a/src/controller.js +++ b/src/controller.js @@ -38,8 +38,8 @@ function Controller(config, debug) { /** @priavate {Debug} */ this.debug_ = debug; - /** @private {string} numeric project id */ - this.project_ = null; + /** @private {string} project id */ + this.project_ = config.projectId || process.env.GCLOUD_PROJECT; /** @private {string} debuggee id provided by the server once registered */ this.debuggeeId_ = null; @@ -68,13 +68,25 @@ Controller.prototype.init = function(uid, logger, callback) { that.uid_ = uid; that.nextWaitToken_ = null; - function complete(err, project) { - if (err) { - callback(err, project); - return; + // We need to figure out whether we are running on GCP. We can use our ability + // to access the metadata service as a test for that. + // TODO: change this to getProjectId in the future. + utils.getProjectNumber(function(err, metadataProject) { + // We should get an error if we are not on GCP. + that.onGCP = !err; + + // We prefer to use the locally available projectId as that is least + // surprising to users. + var project = that.project_ || metadataProject; + + // We if don't have a projectId by now, we fail with an error. + if (!project) { + return callback(err); + } else { + that.project_ = project; } - that.project_ = project; + // Locate the source context. fs.readFile('source-context.json', 'utf8', function(err, data) { try { that.sourceContext_ = JSON.parse(data); @@ -82,17 +94,8 @@ Controller.prototype.init = function(uid, logger, callback) { logger.warn('Malformed source-context.json file.'); // But we keep on going. } - callback(null, project); + return callback(null, project); }); - } - - utils.getProjectNumber(function(err, project) { - that.onGCP = !!project; - if (process.env.GCLOUD_PROJECT) { - complete(null, process.env.GCLOUD_PROJECT); - } else { - complete(err, project); - } }); }; @@ -132,13 +135,14 @@ Controller.prototype.register_ = function(errorMessage, callback) { uri: API + '/debuggees/register', method: 'POST', json: true, - body: { debuggee: debuggee } + body: {debuggee: debuggee} }; this.debug_.request(options, function(err, body, response) { if (err) { callback(err); } else if (response.statusCode !== 200) { - callback(new Error('unable to register, statusCode ' + response.statusCode)); + callback( + new Error('unable to register, statusCode ' + response.statusCode)); } else if (!body.debuggee) { callback(new Error('invalid response body from server')); } else if (body.debuggee.isDisabled) { @@ -153,18 +157,19 @@ Controller.prototype.register_ = function(errorMessage, callback) { /** * Fetch the list of breakpoints from the server. Assumes we have registered. - * @param {!function(?Error,Object=,Object=)} callback accepting (err, response, body) + * @param {!function(?Error,Object=,Object=)} callback accepting (err, response, + * body) */ Controller.prototype.listBreakpoints = function(callback) { var that = this; assert(that.debuggeeId_, 'should register first'); - var query = { success_on_timeout: true }; + var query = {success_on_timeout: true}; if (that.nextWaitToken_) { query.waitToken = that.nextWaitToken; } var uri = API + '/debuggees/' + encodeURIComponent(that.debuggeeId_) + - '/breakpoints?' + qs.stringify(query); + '/breakpoints?' + qs.stringify(query); that.debug_.request({uri: uri, json: true}, function(err, body, response) { if (!response) { callback(err || new Error('unknown error - request response missing')); @@ -176,7 +181,7 @@ Controller.prototype.listBreakpoints = function(callback) { return; } else if (response.statusCode !== 200) { callback(new Error('unable to list breakpoints, status code ' + - response.statusCode)); + response.statusCode)); return; } else { body = body || {}; @@ -191,34 +196,29 @@ Controller.prototype.listBreakpoints = function(callback) { * @param {!Breakpoint} breakpoint * @param {!Function} callback accepting (err, body) */ -Controller.prototype.updateBreakpoint = - function(breakpoint, callback) { - assert(this.debuggeeId_, 'should register first'); - - breakpoint.action = 'capture'; - breakpoint.isFinalState = true; - var options = { - uri: API + '/debuggees/' + encodeURIComponent(this.debuggeeId_) + - '/breakpoints/' + encodeURIComponent(breakpoint.id), - json: true, - method: 'PUT', - body: { - debuggeeId: this.debuggeeId_, - breakpoint: breakpoint - } - }; - - // We need to have a try/catch here because a JSON.stringify will be done - // by request. Some V8 debug mirror objects get a throw when we attempt to - // stringify them. The try-catch keeps it resilient and avoids crashing the - // user's app. - try { - this.debug_.request(options, function(err, body, response) { - callback(err, body); - }); - } catch (error) { - callback(error); - } +Controller.prototype.updateBreakpoint = function(breakpoint, callback) { + assert(this.debuggeeId_, 'should register first'); + + breakpoint.action = 'capture'; + breakpoint.isFinalState = true; + var options = { + uri: API + '/debuggees/' + encodeURIComponent(this.debuggeeId_) + + '/breakpoints/' + encodeURIComponent(breakpoint.id), + json: true, + method: 'PUT', + body: {debuggeeId: this.debuggeeId_, breakpoint: breakpoint} }; + // We need to have a try/catch here because a JSON.stringify will be done + // by request. Some V8 debug mirror objects get a throw when we attempt to + // stringify them. The try-catch keeps it resilient and avoids crashing the + // user's app. + try { + this.debug_.request(options, + function(err, body, response) { callback(err, body); }); + } catch (error) { + callback(error); + } +}; + module.exports = Controller; diff --git a/test/standalone/test-config-credentials.js b/test/standalone/test-config-credentials.js index 0ebabb1c..9928fb2f 100644 --- a/test/standalone/test-config-credentials.js +++ b/test/standalone/test-config-credentials.js @@ -23,13 +23,35 @@ var logger = require('@google/cloud-diagnostics-common').logger; var defaultConfig = require('../../src/config.js').debug; var Debuglet = require('../../src/agent/debuglet.js'); +var envProject = process.env.GCLOUD_PROJECT; + nock.disableNetConnect(); -process.env.GCLOUD_PROJECT = 0; + +function accept() { + return true; +} + +function nockOAuth2(validator) { + return nock('https://accounts.google.com') + .post('/o/oauth2/token', validator) + .reply(200, { + refresh_token: 'hello', + access_token: 'goodbye', + expiry_date: new Date(9999, 1, 1) + }); +} + +function nockRegister(validator) { + return nock('https://clouddebugger.googleapis.com') + .post('/v2/controller/debuggees/register', validator) + .reply(200); +} describe('test-config-credentials', function() { var debuglet = null; beforeEach(function() { + delete process.env.GCLOUD_PROJECT; assert.equal(debuglet, null); }); @@ -37,101 +59,113 @@ describe('test-config-credentials', function() { assert.ok(debuglet); debuglet.stop(); debuglet = null; + process.env.GCLOUD_PROJECT = envProject; }); + it('should use config.projectId in preference to the environment variable', + function(done) { + process.env.GCLOUD_PROJECT = 'should-not-be-used'; + + var config = extend({}, defaultConfig, { + projectId: 'project-via-config', + credentials: require('../fixtures/gcloud-credentials.json') + }); + var debug = require('../..')(config); + + // TODO: also make sure we don't request the project from metadata + // service. + + var scope = nockOAuth2(accept); + nockRegister(function(body) { + assert.ok(body.debuggee); + assert.equal(body.debuggee.project, 'project-via-config'); + scope.done(); + setImmediate(done); + return true; + }); + + debuglet = + new Debuglet(debug, config, logger.create(logger.WARN, 'testing')); + debuglet.start(); + }); it('should use the keyFilename field of the config object', function(done) { + process.env.GCLOUD_PROJECT = '0'; var credentials = require('../fixtures/gcloud-credentials.json'); var config = extend({}, defaultConfig, { keyFilename: path.join('test', 'fixtures', 'gcloud-credentials.json') }); var debug = require('../..')(config); - var scope = nock('https://accounts.google.com') - .post('/o/oauth2/token', function(body) { - assert.equal(body.client_id, credentials.client_id); - assert.equal(body.client_secret, credentials.client_secret); - assert.equal(body.refresh_token, credentials.refresh_token); - return true; - }).reply(200, { - refresh_token: 'hello', - access_token: 'goodbye', - expiry_date: new Date(9999, 1, 1) - }); - // Since we have to get an auth token, this always gets intercepted second - nock('https://clouddebugger.googleapis.com') - .post('/v2/controller/debuggees/register', function() { - scope.done(); - setImmediate(done); - return true; - }).reply(200); - debuglet = new Debuglet(debug, config, logger.create(logger.WARN, 'testing')); + var scope = nockOAuth2(function(body) { + assert.equal(body.client_id, credentials.client_id); + assert.equal(body.client_secret, credentials.client_secret); + assert.equal(body.refresh_token, credentials.refresh_token); + return true; + }); + // Since we have to get an auth token, this always gets intercepted second. + nockRegister(function() { + scope.done(); + setImmediate(done); + return true; + }); + debuglet = + new Debuglet(debug, config, logger.create(logger.WARN, 'testing')); debuglet.start(); }); it('should use the credentials field of the config object', function(done) { - var config = extend({}, defaultConfig, { - credentials: require('../fixtures/gcloud-credentials.json') - }); + process.env.GCLOUD_PROJECT = '0'; + var config = + extend({}, defaultConfig, + {credentials: require('../fixtures/gcloud-credentials.json')}); var debug = require('../..')(config); - var scope = nock('https://accounts.google.com') - .post('/o/oauth2/token', function(body) { - assert.equal(body.client_id, config.credentials.client_id); - assert.equal(body.client_secret, config.credentials.client_secret); - assert.equal(body.refresh_token, config.credentials.refresh_token); - return true; - }).reply(200, { - refresh_token: 'hello', - access_token: 'goodbye', - expiry_date: new Date(9999, 1, 1) - }); - // Since we have to get an auth token, this always gets intercepted second - nock('https://clouddebugger.googleapis.com') - .post('/v2/controller/debuggees/register', function() { - scope.done(); - setImmediate(done); - return true; - }).reply(200); + var scope = nockOAuth2(function(body) { + assert.equal(body.client_id, config.credentials.client_id); + assert.equal(body.client_secret, config.credentials.client_secret); + assert.equal(body.refresh_token, config.credentials.refresh_token); + return true; + }); + // Since we have to get an auth token, this always gets intercepted second. + nockRegister(function() { + scope.done(); + setImmediate(done); + return true; + }); debuglet = new Debuglet(debug, config, logger.create(undefined, 'testing')); debuglet.start(); }); it('should ignore keyFilename if credentials is provided', function(done) { + process.env.GCLOUD_PROJECT = '0'; var fileCredentials = require('../fixtures/gcloud-credentials.json'); var credentials = { - client_id: 'a', - client_secret: 'b', - refresh_token: 'c', - type: 'authorized_user' + client_id: 'a', + client_secret: 'b', + refresh_token: 'c', + type: 'authorized_user' }; var config = extend({}, defaultConfig, { keyFilename: path.join('test', 'fixtures', 'gcloud-credentials.json'), credentials: credentials }); var debug = require('../..')(config); - ['client_id', 'client_secret', 'refresh_token'].forEach(function (field) { + var scope = nockOAuth2(function(body) { + assert.equal(body.client_id, credentials.client_id); + assert.equal(body.client_secret, credentials.client_secret); + assert.equal(body.refresh_token, credentials.refresh_token); + return true; + }); + // Since we have to get an auth token, this always gets intercepted second. + nockRegister(function() { + scope.done(); + setImmediate(done); + return true; + }); + ['client_id', 'client_secret', 'refresh_token'].forEach(function(field) { assert(fileCredentials.hasOwnProperty(field)); assert(config.credentials.hasOwnProperty(field)); - assert.notEqual(config.credentials[field], - fileCredentials[field]); + assert.notEqual(config.credentials[field], fileCredentials[field]); }); - var scope = nock('https://accounts.google.com') - .post('/o/oauth2/token', function(body) { - assert.equal(body.client_id, credentials.client_id); - assert.equal(body.client_secret, credentials.client_secret); - assert.equal(body.refresh_token, credentials.refresh_token); - return true; - }).reply(200, { - refresh_token: 'hello', - access_token: 'goodbye', - expiry_date: new Date(9999, 1, 1) - }); - // Since we have to get an auth token, this always gets intercepted second - nock('https://clouddebugger.googleapis.com') - .post('/v2/controller/debuggees/register', function() { - scope.done(); - setImmediate(done); - return true; - }).reply(200); debuglet = new Debuglet(debug, config, logger.create(undefined, 'testing')); debuglet.start(); }); diff --git a/test/standalone/test-debuglet.js b/test/standalone/test-debuglet.js index f4f4bedb..a3295991 100644 --- a/test/standalone/test-debuglet.js +++ b/test/standalone/test-debuglet.js @@ -17,9 +17,10 @@ var assert = require('assert'); var request = require('../auth-request.js'); -var logger = require('@google/cloud-diagnostics-common').logger; -var config = require('../../src/config.js').debug; +var loggerModule = require('@google/cloud-diagnostics-common').logger; +var defaultConfig = require('../../src/config.js').debug; var Debuglet = require('../../src/agent/debuglet.js'); +var extend = require('extend'); var DEBUGGEE_ID = 'bar'; var API = 'https://clouddebugger.googleapis.com'; @@ -29,10 +30,8 @@ var BPS_PATH = '/v2/controller/debuggees/' + DEBUGGEE_ID + '/breakpoints'; var nock = require('nock'); nock.disableNetConnect(); -// Disable error logging during the tests. -config.logLevel = 0; +var logger = loggerModule.create(process.env.GCLOUD_DEBUG_LOGLEVEL || 0, 'test'); -var debuglet; var bp = { id: 'test', action: 'CAPTURE', @@ -44,25 +43,27 @@ var errorBp = { location: { path: 'fixtures/foo.js', line: 2 } }; var fakeDebug = { - request: request + request: request // avoid authentication that happens through + // google-auth-library. }; describe(__filename, function(){ + var debuglet; + beforeEach(function() { - process.env.GCLOUD_PROJECT = 0; - debuglet = new Debuglet( - fakeDebug, config, logger.create(config.logLevel, '@google/cloud-debug')); - debuglet.once('started', function() { - debuglet.debugletApi_.request_ = request; // Avoid authing. - }); + delete process.env.GCLOUD_PROJECT; }); afterEach(function() { + assert.ok(debuglet); debuglet.stop(); }); - it('should not start unless we know the project num', function(done) { - delete process.env.GCLOUD_PROJECT; + it('should not start when projectId is not available', function(done) { + debuglet = new Debuglet(fakeDebug, defaultConfig, logger); + + // The following mock is neccessary for the case when the test is running + // on GCP. In that case we will get the projectId from the metadata service. var scope = nock('http://metadata.google.internal') .get('/computeMetadata/v1/project/numeric-project-id') .reply(404); @@ -79,7 +80,10 @@ describe(__filename, function(){ }); it('should not crash without project num', function(done) { - delete process.env.GCLOUD_PROJECT; + debuglet = new Debuglet(fakeDebug, defaultConfig, logger); + + // The following mock is neccessary for the case when the test is running + // on GCP. In that case we will get the projectId from the metadata service. var scope = nock('http://metadata.google.internal') .get('/computeMetadata/v1/project/numeric-project-id') .reply(404); @@ -96,6 +100,7 @@ describe(__filename, function(){ it('should accept non-numeric GCLOUD_PROJECT', function(done) { process.env.GCLOUD_PROJECT='11020304f2934'; + debuglet = new Debuglet(fakeDebug, defaultConfig, logger); var scope = nock(API) .post(REGISTER_PATH) @@ -117,6 +122,7 @@ describe(__filename, function(){ it('should retry on failed registration', function(done) { this.timeout(10000); process.env.GCLOUD_PROJECT='11020304f2934'; + debuglet = new Debuglet(fakeDebug, defaultConfig, logger); var scope = nock(API) .post(REGISTER_PATH) @@ -142,6 +148,9 @@ describe(__filename, function(){ it('should error if a package.json doesn\'t exist'); it('should register successfully otherwise', function(done) { + var config = extend({}, defaultConfig, {projectId: 'fake-project'}); + debuglet = new Debuglet(fakeDebug, config, logger); + var scope = nock(API) .post(REGISTER_PATH) .reply(200, { @@ -162,6 +171,9 @@ describe(__filename, function(){ it('should pass source context to api if present', function(done) { process.chdir('test/fixtures'); + var config = extend({}, defaultConfig, {projectId: 'fake-project'}); + debuglet = new Debuglet(fakeDebug, config, logger); + var scope = nock(API) .post(REGISTER_PATH, function(body) { return body.debuggee.sourceContexts[0] && @@ -192,6 +204,9 @@ describe(__filename, function(){ it('should re-fetch breakpoints on error', function(done) { this.timeout(6000); + var config = extend({}, defaultConfig, {projectId: 'fake-project'}); + debuglet = new Debuglet(fakeDebug, config, logger); + var scope = nock(API) .post(REGISTER_PATH) .reply(200, { @@ -238,11 +253,11 @@ describe(__filename, function(){ it('should add a breakpoint'); it('should expire stale breakpoints', function(done) { - var oldTimeout = config.breakpointExpirationSec; - config.breakpointExpirationSec = 1; + var config = extend({}, defaultConfig, { + projectId: 'fake-project', + breakpointExpirationSec: 1 + }); this.timeout(6000); - var debuglet = new Debuglet( - fakeDebug, config, logger.create(config.logLevel, '@google/cloud-debug')); var scope = nock(API) .post(REGISTER_PATH) @@ -261,9 +276,7 @@ describe(__filename, function(){ }) .reply(200); - debuglet.once('started', function() { - debuglet.debugletApi_.request_ = request; // Avoid authing. - }); + debuglet = new Debuglet(fakeDebug, config, logger); debuglet.once('registered', function(id) { assert(id === DEBUGGEE_ID); setTimeout(function() { @@ -271,7 +284,6 @@ describe(__filename, function(){ setTimeout(function() { assert(!debuglet.activeBreakpointMap_.test); scope.done(); - config.breakpointExpirationSec = oldTimeout; done(); }, 1100); }, 500); @@ -288,13 +300,12 @@ describe(__filename, function(){ // the breakpoint listed as active. It validates that the breakpoint // is only expired with the server once. it('should not update expired breakpoints', function(done) { - var oldTimeout = config.breakpointExpirationSec; - var oldFetchRate = config.breakpointUpdateIntervalSec; - config.breakpointExpirationSec = 1; - config.breakpointUpdateIntervalSec = 1; + var config = extend({}, defaultConfig, { + projectId: 'fake-project', + breakpointExpirationSec: 1, + breakpointUpdateIntervalSec: 1 + }); this.timeout(6000); - var debuglet = new Debuglet( - fakeDebug, config, logger.create(config.logLevel, '@google/cloud-debug')); var scope = nock(API) .post(REGISTER_PATH) @@ -317,9 +328,7 @@ describe(__filename, function(){ breakpoints: [bp] }); - debuglet.once('started', function() { - debuglet.debugletApi_.request_ = request; // Avoid authing. - }); + debuglet = new Debuglet(fakeDebug, config, logger); debuglet.once('registered', function(id) { assert(id === DEBUGGEE_ID); setTimeout(function() { @@ -329,8 +338,6 @@ describe(__filename, function(){ // Fetcher disables if we re-update since endpoint isn't mocked twice assert(debuglet.fetcherActive_); scope.done(); - config.breakpointExpirationSec = oldTimeout; - config.breakpointUpdateIntervalSec = oldFetchRate; done(); }, 4500); }, 500);