Skip to content

Commit

Permalink
core: honor projectId detected by google-auth-library
Browse files Browse the repository at this point in the history
  • Loading branch information
stephenplusplus committed Nov 15, 2016
1 parent 86755a7 commit 09aeac1
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 22 deletions.
5 changes: 4 additions & 1 deletion packages/bigtable/system-test/bigtable.js
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,10 @@ describe('Bigtable', function() {

it('should get the tables metadata', function(done) {
TABLE.getMetadata(function(err, metadata) {
assert.strictEqual(metadata.name, TABLE.id);
assert.strictEqual(
metadata.name,
TABLE.id.replace('{{projectId}}', bigtable.projectId)
);
done();
});
});
Expand Down
2 changes: 1 addition & 1 deletion packages/common/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
"duplexify": "^3.2.0",
"ent": "^2.2.0",
"extend": "^3.0.0",
"google-auto-auth": "^0.3.0",
"google-auto-auth": "^0.4.0",
"google-proto-files": "^0.8.0",
"grpc": "^1.0.0",
"is": "^3.0.1",
Expand Down
27 changes: 27 additions & 0 deletions packages/common/src/grpc-service.js
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,13 @@ GrpcService.prototype.request = function(protoOpts, reqOpts, callback) {
request: function(_, onResponse) {
respError = null;

try {
reqOpts = util.decorateRequest(reqOpts, { projectId: self.projectId });
} catch (e) {
onResponse(e);
return;
}

service[protoOpts.method](reqOpts, metadata, grpcOpts, function(e, resp) {
if (e) {
respError = GrpcService.decorateError_(e);
Expand Down Expand Up @@ -351,6 +358,13 @@ GrpcService.prototype.requestStream = function(protoOpts, reqOpts) {
shouldRetryFn: GrpcService.shouldRetryRequest_,

request: function() {
try {
reqOpts = util.decorateRequest(reqOpts, { projectId: self.projectId });
} catch (e) {
stream.destroy(e);
return;
}

return service[protoOpts.method](reqOpts, self.grpcMetadata, grpcOpts)
.on('metadata', function() {
// retry-request requires a server response before it starts emitting
Expand Down Expand Up @@ -416,6 +430,15 @@ GrpcService.prototype.requestWritableStream = function(protoOpts, reqOpts) {
grpcOpts.deadline = GrpcService.createDeadline_(protoOpts.timeout);
}

try {
reqOpts = util.decorateRequest(reqOpts, { projectId: self.projectId });
} catch (e) {
setImmediate(function() {
stream.destroy(e);
});
return stream;
}

var grpcStream = service[protoOpts.method](reqOpts, grpcOpts)
.on('status', function(status) {
var grcpStatus = GrpcService.decorateStatus_(status);
Expand Down Expand Up @@ -724,6 +747,8 @@ GrpcService.structToObj_ = function(struct) {
* @param {?error} callback.err - An error getting an auth client.
*/
GrpcService.prototype.getGrpcCredentials_ = function(callback) {
var self = this;

this.authClient.getAuthClient(function(err, authClient) {
if (err) {
callback(err);
Expand All @@ -735,6 +760,8 @@ GrpcService.prototype.getGrpcCredentials_ = function(callback) {
grpc.credentials.createFromGoogleCredential(authClient)
);

self.projectId = authClient.projectId;

callback(null, credentials);
});
};
Expand Down
3 changes: 1 addition & 2 deletions packages/common/src/service.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,13 @@ function Service(config, options) {
});

this.makeAuthenticatedRequest = util.makeAuthenticatedRequestFactory(reqCfg);

this.authClient = this.makeAuthenticatedRequest.authClient;
this.baseUrl = config.baseUrl;
this.getCredentials = this.makeAuthenticatedRequest.getCredentials;
this.globalInterceptors = arrify(options.interceptors_);
this.interceptors = [];
this.packageJson = config.packageJson;
this.projectId = options.projectId;
this.projectId = options.projectId || '{{projectId}}';
this.projectIdRequired = config.projectIdRequired !== false;
this.Promise = options.promise || Promise;
}
Expand Down
38 changes: 27 additions & 11 deletions packages/common/src/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,8 @@ util.shouldRetryRequest = shouldRetryRequest;
/**
* Get a function for making authenticated requests.
*
* @throws {Error} If a projectId is requested, but not able to be detected.
*
* @param {object} config - Configuration object.
* @param {boolean=} config.autoRetry - Automatically retry requests if the
* response is related to rate limits or certain intermittent server errors.
Expand Down Expand Up @@ -344,6 +346,17 @@ function makeAuthenticatedRequestFactory(config) {
}

function onAuthenticated(err, authenticatedReqOpts) {
if (!err) {
try {
authenticatedReqOpts = util.decorateRequest(
authenticatedReqOpts,
extend({ projectId: authClient.projectId }, config)
);
} catch(e) {
err = util.missingProjectIdError;
}
}

if (err) {
if (stream) {
stream.destroy(err);
Expand All @@ -354,8 +367,6 @@ function makeAuthenticatedRequestFactory(config) {
return;
}

authenticatedReqOpts = util.decorateRequest(authenticatedReqOpts, config);

if (options && options.onAuthenticated) {
options.onAuthenticated(null, authenticatedReqOpts);
} else {
Expand Down Expand Up @@ -478,6 +489,19 @@ function decorateRequest(reqOpts, config) {
delete reqOpts.json.autoPaginateVal;
}

for (var opt in reqOpts) {
if (is.string(reqOpts[opt])) {
if (reqOpts[opt].indexOf('{{projectId}}') > -1) {
if (!config.projectId) {
throw util.missingProjectIdError;
}
reqOpts[opt] = reqOpts[opt].replace(/{{projectId}}/g, config.projectId);
}
} else if (is.object(reqOpts[opt])) {
decorateRequest(reqOpts[opt], config);
}
}

return reqOpts;
}

Expand Down Expand Up @@ -530,8 +554,6 @@ util.extendGlobalConfig = extendGlobalConfig;
/**
* Merge and validate API configurations.
*
* @throws {Error} If a projectId is not specified.
*
* @param {object} globalContext - gcloud-level context.
* @param {object} globalContext.config_ - gcloud-level configuration.
* @param {object} localConfig - Service-level configurations.
Expand All @@ -545,13 +567,7 @@ function normalizeArguments(globalContext, localConfig, options) {

var globalConfig = globalContext && globalContext.config_;

var config = util.extendGlobalConfig(globalConfig, localConfig);

if (options.projectIdRequired !== false && !config.projectId) {
throw util.missingProjectIdError;
}

return config;
return util.extendGlobalConfig(globalConfig, localConfig);
}

util.normalizeArguments = normalizeArguments;
Expand Down
7 changes: 0 additions & 7 deletions system-test/env.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,6 @@

'use strict';

if (!process.env.GCLOUD_TESTS_PROJECT_ID && !process.env.GCLOUD_TESTS_KEY) {
throw new Error([
'To run the system tests, you need to set some environment variables.',
'Please check the Contributing guide for instructions.'
].join('\n'));
}

module.exports = {
projectId: process.env.GCLOUD_TESTS_PROJECT_ID,
keyFilename: process.env.GCLOUD_TESTS_KEY
Expand Down

0 comments on commit 09aeac1

Please sign in to comment.