Skip to content
This repository was archived by the owner on Apr 3, 2024. It is now read-only.

Commit f7de637

Browse files
ofrobotsofrobots
authored
fix precedence for where the projectId is acquired from (#193)
1 parent 638f902 commit f7de637

File tree

3 files changed

+192
-151
lines changed

3 files changed

+192
-151
lines changed

‎src/controller.js‎

Lines changed: 51 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@ function Controller(config, debug) {
3838
/** @priavate {Debug} */
3939
this.debug_ = debug;
4040

41-
/** @private {string} numeric project id */
42-
this.project_ = null;
41+
/** @private {string} project id */
42+
this.project_ = config.projectId || process.env.GCLOUD_PROJECT;
4343

4444
/** @private {string} debuggee id provided by the server once registered */
4545
this.debuggeeId_ = null;
@@ -68,31 +68,34 @@ Controller.prototype.init = function(uid, logger, callback) {
6868
that.uid_ = uid;
6969
that.nextWaitToken_ = null;
7070

71-
function complete(err, project) {
72-
if (err) {
73-
callback(err, project);
74-
return;
71+
// We need to figure out whether we are running on GCP. We can use our ability
72+
// to access the metadata service as a test for that.
73+
// TODO: change this to getProjectId in the future.
74+
utils.getProjectNumber(function(err, metadataProject) {
75+
// We should get an error if we are not on GCP.
76+
that.onGCP = !err;
77+
78+
// We prefer to use the locally available projectId as that is least
79+
// surprising to users.
80+
var project = that.project_ || metadataProject;
81+
82+
// We if don't have a projectId by now, we fail with an error.
83+
if (!project) {
84+
return callback(err);
85+
} else {
86+
that.project_ = project;
7587
}
76-
that.project_ = project;
7788

89+
// Locate the source context.
7890
fs.readFile('source-context.json', 'utf8', function(err, data) {
7991
try {
8092
that.sourceContext_ = JSON.parse(data);
8193
} catch (e) {
8294
logger.warn('Malformed source-context.json file.');
8395
// But we keep on going.
8496
}
85-
callback(null, project);
97+
return callback(null, project);
8698
});
87-
}
88-
89-
utils.getProjectNumber(function(err, project) {
90-
that.onGCP = !!project;
91-
if (process.env.GCLOUD_PROJECT) {
92-
complete(null, process.env.GCLOUD_PROJECT);
93-
} else {
94-
complete(err, project);
95-
}
9699
});
97100
};
98101

@@ -132,13 +135,14 @@ Controller.prototype.register_ = function(errorMessage, callback) {
132135
uri: API + '/debuggees/register',
133136
method: 'POST',
134137
json: true,
135-
body: { debuggee: debuggee }
138+
body: {debuggee: debuggee}
136139
};
137140
this.debug_.request(options, function(err, body, response) {
138141
if (err) {
139142
callback(err);
140143
} else if (response.statusCode !== 200) {
141-
callback(new Error('unable to register, statusCode ' + response.statusCode));
144+
callback(
145+
new Error('unable to register, statusCode ' + response.statusCode));
142146
} else if (!body.debuggee) {
143147
callback(new Error('invalid response body from server'));
144148
} else if (body.debuggee.isDisabled) {
@@ -153,18 +157,19 @@ Controller.prototype.register_ = function(errorMessage, callback) {
153157

154158
/**
155159
* Fetch the list of breakpoints from the server. Assumes we have registered.
156-
* @param {!function(?Error,Object=,Object=)} callback accepting (err, response, body)
160+
* @param {!function(?Error,Object=,Object=)} callback accepting (err, response,
161+
* body)
157162
*/
158163
Controller.prototype.listBreakpoints = function(callback) {
159164
var that = this;
160165
assert(that.debuggeeId_, 'should register first');
161-
var query = { success_on_timeout: true };
166+
var query = {success_on_timeout: true};
162167
if (that.nextWaitToken_) {
163168
query.waitToken = that.nextWaitToken;
164169
}
165170

166171
var uri = API + '/debuggees/' + encodeURIComponent(that.debuggeeId_) +
167-
'/breakpoints?' + qs.stringify(query);
172+
'/breakpoints?' + qs.stringify(query);
168173
that.debug_.request({uri: uri, json: true}, function(err, body, response) {
169174
if (!response) {
170175
callback(err || new Error('unknown error - request response missing'));
@@ -176,7 +181,7 @@ Controller.prototype.listBreakpoints = function(callback) {
176181
return;
177182
} else if (response.statusCode !== 200) {
178183
callback(new Error('unable to list breakpoints, status code ' +
179-
response.statusCode));
184+
response.statusCode));
180185
return;
181186
} else {
182187
body = body || {};
@@ -191,34 +196,29 @@ Controller.prototype.listBreakpoints = function(callback) {
191196
* @param {!Breakpoint} breakpoint
192197
* @param {!Function} callback accepting (err, body)
193198
*/
194-
Controller.prototype.updateBreakpoint =
195-
function(breakpoint, callback) {
196-
assert(this.debuggeeId_, 'should register first');
197-
198-
breakpoint.action = 'capture';
199-
breakpoint.isFinalState = true;
200-
var options = {
201-
uri: API + '/debuggees/' + encodeURIComponent(this.debuggeeId_) +
202-
'/breakpoints/' + encodeURIComponent(breakpoint.id),
203-
json: true,
204-
method: 'PUT',
205-
body: {
206-
debuggeeId: this.debuggeeId_,
207-
breakpoint: breakpoint
208-
}
209-
};
210-
211-
// We need to have a try/catch here because a JSON.stringify will be done
212-
// by request. Some V8 debug mirror objects get a throw when we attempt to
213-
// stringify them. The try-catch keeps it resilient and avoids crashing the
214-
// user's app.
215-
try {
216-
this.debug_.request(options, function(err, body, response) {
217-
callback(err, body);
218-
});
219-
} catch (error) {
220-
callback(error);
221-
}
199+
Controller.prototype.updateBreakpoint = function(breakpoint, callback) {
200+
assert(this.debuggeeId_, 'should register first');
201+
202+
breakpoint.action = 'capture';
203+
breakpoint.isFinalState = true;
204+
var options = {
205+
uri: API + '/debuggees/' + encodeURIComponent(this.debuggeeId_) +
206+
'/breakpoints/' + encodeURIComponent(breakpoint.id),
207+
json: true,
208+
method: 'PUT',
209+
body: {debuggeeId: this.debuggeeId_, breakpoint: breakpoint}
222210
};
223211

212+
// We need to have a try/catch here because a JSON.stringify will be done
213+
// by request. Some V8 debug mirror objects get a throw when we attempt to
214+
// stringify them. The try-catch keeps it resilient and avoids crashing the
215+
// user's app.
216+
try {
217+
this.debug_.request(options,
218+
function(err, body, response) { callback(err, body); });
219+
} catch (error) {
220+
callback(error);
221+
}
222+
};
223+
224224
module.exports = Controller;

‎test/standalone/test-config-credentials.js‎

Lines changed: 100 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -23,115 +23,149 @@ var logger = require('@google/cloud-diagnostics-common').logger;
2323
var defaultConfig = require('../../src/config.js').debug;
2424
var Debuglet = require('../../src/agent/debuglet.js');
2525

26+
var envProject = process.env.GCLOUD_PROJECT;
27+
2628
nock.disableNetConnect();
27-
process.env.GCLOUD_PROJECT = 0;
29+
30+
function accept() {
31+
return true;
32+
}
33+
34+
function nockOAuth2(validator) {
35+
return nock('https://accounts.google.com')
36+
.post('/o/oauth2/token', validator)
37+
.reply(200, {
38+
refresh_token: 'hello',
39+
access_token: 'goodbye',
40+
expiry_date: new Date(9999, 1, 1)
41+
});
42+
}
43+
44+
function nockRegister(validator) {
45+
return nock('https://clouddebugger.googleapis.com')
46+
.post('/v2/controller/debuggees/register', validator)
47+
.reply(200);
48+
}
2849

2950
describe('test-config-credentials', function() {
3051
var debuglet = null;
3152

3253
beforeEach(function() {
54+
delete process.env.GCLOUD_PROJECT;
3355
assert.equal(debuglet, null);
3456
});
3557

3658
afterEach(function() {
3759
assert.ok(debuglet);
3860
debuglet.stop();
3961
debuglet = null;
62+
process.env.GCLOUD_PROJECT = envProject;
4063
});
4164

65+
it('should use config.projectId in preference to the environment variable',
66+
function(done) {
67+
process.env.GCLOUD_PROJECT = 'should-not-be-used';
68+
69+
var config = extend({}, defaultConfig, {
70+
projectId: 'project-via-config',
71+
credentials: require('../fixtures/gcloud-credentials.json')
72+
});
73+
var debug = require('../..')(config);
74+
75+
// TODO: also make sure we don't request the project from metadata
76+
// service.
77+
78+
var scope = nockOAuth2(accept);
79+
nockRegister(function(body) {
80+
assert.ok(body.debuggee);
81+
assert.equal(body.debuggee.project, 'project-via-config');
82+
scope.done();
83+
setImmediate(done);
84+
return true;
85+
});
86+
87+
debuglet =
88+
new Debuglet(debug, config, logger.create(logger.WARN, 'testing'));
89+
debuglet.start();
90+
});
4291

4392
it('should use the keyFilename field of the config object', function(done) {
93+
process.env.GCLOUD_PROJECT = '0';
4494
var credentials = require('../fixtures/gcloud-credentials.json');
4595
var config = extend({}, defaultConfig, {
4696
keyFilename: path.join('test', 'fixtures', 'gcloud-credentials.json')
4797
});
4898
var debug = require('../..')(config);
49-
var scope = nock('https://accounts.google.com')
50-
.post('/o/oauth2/token', function(body) {
51-
assert.equal(body.client_id, credentials.client_id);
52-
assert.equal(body.client_secret, credentials.client_secret);
53-
assert.equal(body.refresh_token, credentials.refresh_token);
54-
return true;
55-
}).reply(200, {
56-
refresh_token: 'hello',
57-
access_token: 'goodbye',
58-
expiry_date: new Date(9999, 1, 1)
59-
});
60-
// Since we have to get an auth token, this always gets intercepted second
61-
nock('https://clouddebugger.googleapis.com')
62-
.post('/v2/controller/debuggees/register', function() {
63-
scope.done();
64-
setImmediate(done);
65-
return true;
66-
}).reply(200);
67-
debuglet = new Debuglet(debug, config, logger.create(logger.WARN, 'testing'));
99+
var scope = nockOAuth2(function(body) {
100+
assert.equal(body.client_id, credentials.client_id);
101+
assert.equal(body.client_secret, credentials.client_secret);
102+
assert.equal(body.refresh_token, credentials.refresh_token);
103+
return true;
104+
});
105+
// Since we have to get an auth token, this always gets intercepted second.
106+
nockRegister(function() {
107+
scope.done();
108+
setImmediate(done);
109+
return true;
110+
});
111+
debuglet =
112+
new Debuglet(debug, config, logger.create(logger.WARN, 'testing'));
68113
debuglet.start();
69114
});
70115

71116
it('should use the credentials field of the config object', function(done) {
72-
var config = extend({}, defaultConfig, {
73-
credentials: require('../fixtures/gcloud-credentials.json')
74-
});
117+
process.env.GCLOUD_PROJECT = '0';
118+
var config =
119+
extend({}, defaultConfig,
120+
{credentials: require('../fixtures/gcloud-credentials.json')});
75121
var debug = require('../..')(config);
76-
var scope = nock('https://accounts.google.com')
77-
.post('/o/oauth2/token', function(body) {
78-
assert.equal(body.client_id, config.credentials.client_id);
79-
assert.equal(body.client_secret, config.credentials.client_secret);
80-
assert.equal(body.refresh_token, config.credentials.refresh_token);
81-
return true;
82-
}).reply(200, {
83-
refresh_token: 'hello',
84-
access_token: 'goodbye',
85-
expiry_date: new Date(9999, 1, 1)
86-
});
87-
// Since we have to get an auth token, this always gets intercepted second
88-
nock('https://clouddebugger.googleapis.com')
89-
.post('/v2/controller/debuggees/register', function() {
90-
scope.done();
91-
setImmediate(done);
92-
return true;
93-
}).reply(200);
122+
var scope = nockOAuth2(function(body) {
123+
assert.equal(body.client_id, config.credentials.client_id);
124+
assert.equal(body.client_secret, config.credentials.client_secret);
125+
assert.equal(body.refresh_token, config.credentials.refresh_token);
126+
return true;
127+
});
128+
// Since we have to get an auth token, this always gets intercepted second.
129+
nockRegister(function() {
130+
scope.done();
131+
setImmediate(done);
132+
return true;
133+
});
94134
debuglet = new Debuglet(debug, config, logger.create(undefined, 'testing'));
95135
debuglet.start();
96136
});
97137

98138
it('should ignore keyFilename if credentials is provided', function(done) {
139+
process.env.GCLOUD_PROJECT = '0';
99140
var fileCredentials = require('../fixtures/gcloud-credentials.json');
100141
var credentials = {
101-
client_id: 'a',
102-
client_secret: 'b',
103-
refresh_token: 'c',
104-
type: 'authorized_user'
142+
client_id: 'a',
143+
client_secret: 'b',
144+
refresh_token: 'c',
145+
type: 'authorized_user'
105146
};
106147
var config = extend({}, defaultConfig, {
107148
keyFilename: path.join('test', 'fixtures', 'gcloud-credentials.json'),
108149
credentials: credentials
109150
});
110151
var debug = require('../..')(config);
111-
['client_id', 'client_secret', 'refresh_token'].forEach(function (field) {
152+
var scope = nockOAuth2(function(body) {
153+
assert.equal(body.client_id, credentials.client_id);
154+
assert.equal(body.client_secret, credentials.client_secret);
155+
assert.equal(body.refresh_token, credentials.refresh_token);
156+
return true;
157+
});
158+
// Since we have to get an auth token, this always gets intercepted second.
159+
nockRegister(function() {
160+
scope.done();
161+
setImmediate(done);
162+
return true;
163+
});
164+
['client_id', 'client_secret', 'refresh_token'].forEach(function(field) {
112165
assert(fileCredentials.hasOwnProperty(field));
113166
assert(config.credentials.hasOwnProperty(field));
114-
assert.notEqual(config.credentials[field],
115-
fileCredentials[field]);
167+
assert.notEqual(config.credentials[field], fileCredentials[field]);
116168
});
117-
var scope = nock('https://accounts.google.com')
118-
.post('/o/oauth2/token', function(body) {
119-
assert.equal(body.client_id, credentials.client_id);
120-
assert.equal(body.client_secret, credentials.client_secret);
121-
assert.equal(body.refresh_token, credentials.refresh_token);
122-
return true;
123-
}).reply(200, {
124-
refresh_token: 'hello',
125-
access_token: 'goodbye',
126-
expiry_date: new Date(9999, 1, 1)
127-
});
128-
// Since we have to get an auth token, this always gets intercepted second
129-
nock('https://clouddebugger.googleapis.com')
130-
.post('/v2/controller/debuggees/register', function() {
131-
scope.done();
132-
setImmediate(done);
133-
return true;
134-
}).reply(200);
135169
debuglet = new Debuglet(debug, config, logger.create(undefined, 'testing'));
136170
debuglet.start();
137171
});

0 commit comments

Comments
 (0)