From 975162ef2ccdcf568339c44ddf7bde461a77b3dd Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Mon, 19 Nov 2018 20:07:45 +0000 Subject: [PATCH] =?UTF-8?q?Bug=201508333=20-=20[BREAKING]=20do=20not=20use?= =?UTF-8?q?=20TASKCLUSTER=5F=E2=80=A6=20vars=20by=20default?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- README.md | 42 +++++++++++++++++------------------------- src/client.js | 25 ++++++++++++++++++++----- test/client_test.js | 12 +++++++----- test/creds_test.js | 36 ++++++++++++++++++++---------------- 4 files changed, 64 insertions(+), 51 deletions(-) diff --git a/README.md b/README.md index 04e5d9a..cb89de7 100644 --- a/README.md +++ b/README.md @@ -666,34 +666,18 @@ There is a number of configuration options for Client which affects invocation of API end-points. These are useful if using a non-default server, for example when setting up a staging area or testing locally. -### Configuring API Root URL +### Configuring API Root URL and Credentials + If you use the builtin API Client classes documented above you must configure the `rootUrl` when creating an instance of the client. As illustrated below: ```js var auth = new taskcluster.Auth({ - credentials: {...}, rootUrl: "http://whatever.com" }); ``` -This can also be set by setting a -`TASKCLUSTER_ROOT_URL` env var before importing taskcluster-client. You can also -use global config options as below: - -```js -// Configure default options -taskcluster.config({ - rootUrl: "https://somesite.com", -}); - -// No rootUrl needed here -var auth = new taskcluster.Auth(); -``` - -### Configuring Credentials -When creating an instance of a Client class the credentials can be provided -in options. For example: +You may also provide credentials. For example: ```js var auth = new taskcluster.Auth({ credentials: { @@ -702,26 +686,34 @@ var auth = new taskcluster.Auth({ } }); ``` +If the `clientId` and `accessToken` are not given, no credentials will be used. -You can also configure default options globally using -`taskcluster.config(options)`, as follows: +You can set either or both of these values as global config options as below: ```js // Configure default options taskcluster.config({ + rootUrl: "https://somesite.com", credentials: { clientId: '...', accessToken: '...' } }); -// No credentials needed here +// No rootUrl needed here var auth = new taskcluster.Auth(); ``` -If the `clientId` and `accessToken` are left empty we also check the -`TASKCLUSTER_CLIENT_ID` and `TASKCLUSTER_ACCESS_TOKEN` environment variables -to use as defaults (similar to how AWS, Azure, etc. handle authentication). +You can read credentials and rootUrl from the standard `TASKCLUSTER_…` +environment variables with `taskcluster.fromEnvVars()`: + +```js +var auth = new taskcluster.Auth({ + ...taskcluster.fromEnvVars(), +}); +// or (to get behavior like that in versions 11.0.0 and earlier): +taskcluster.config(taskcluster.fromEnvVars()); +``` ### Restricting Authorized Scopes If you wish to perform requests on behalf of a third-party that has small set of diff --git a/src/client.js b/src/client.js index c06e229..b891bd5 100644 --- a/src/client.js +++ b/src/client.js @@ -41,9 +41,9 @@ exports.agents = DEFAULT_AGENTS; // Default options stored globally for convenience var _defaultOptions = { credentials: { - clientId: process.env.TASKCLUSTER_CLIENT_ID, - accessToken: process.env.TASKCLUSTER_ACCESS_TOKEN, - certificate: process.env.TASKCLUSTER_CERTIFICATE, + clientId: undefined, + accessToken: undefined, + certificate: undefined, }, // Request time out (defaults to 30 seconds) timeout: 30 * 1000, @@ -60,7 +60,7 @@ var _defaultOptions = { maxDelay: 30 * 1000, // The prefix of any api calls. e.g. https://taskcluster.net/api/ - rootUrl: process.env.TASKCLUSTER_ROOT_URL, + rootUrl: undefined, // Fake methods, if given this will produce a fake client object. // Methods called won't make expected HTTP requests, but instead: @@ -177,7 +177,7 @@ exports.createClient = function(reference, name) { serviceVersion: 'v1', }, _defaultOptions); - assert(this._options.rootUrl, 'Must provide a rootUrl or set the env var TASKCLUSTER_ROOT_URL'); + assert(this._options.rootUrl, 'Must provide a rootUrl'); this._options.rootUrl = this._options.rootUrl.replace(/\/$/, ''); @@ -672,6 +672,21 @@ exports.config = function(options) { _defaultOptions = _.defaults({}, options, _defaultOptions); }; +exports.fromEnvVars = function() { + var results = {}; + for (let {env, path} of [ + {env: 'TASKCLUSTER_ROOT_URL', path: 'rootUrl'}, + {env: 'TASKCLUSTER_CLIENT_ID', path: 'credentials.clientId'}, + {env: 'TASKCLUSTER_ACCESS_TOKEN', path: 'credentials.accessToken'}, + {env: 'TASKCLUSTER_CERTIFICATE', path: 'credentials.certificate'}, + ]) { + if (process.env[env]) { + _.set(results, path, process.env[env]); + } + } + return results; +}; + /** * Construct a set of temporary credentials. * diff --git a/test/client_test.js b/test/client_test.js index f6c249e..250fdb2 100644 --- a/test/client_test.js +++ b/test/client_test.js @@ -235,12 +235,14 @@ suite('client requests/responses', function() { delete require.cache[clientPath]; const cleanClient = require(clientPath); const Fake = cleanClient.createClient(referenceNameStyle); - const fake = new Fake({ - credentials: { - clientId: 'nobody', - accessToken: 'nothing', + const fake = new Fake(Object.assign({}, + taskcluster.fromEnvVars(), { + credentials: { + clientId: 'nobody', + accessToken: 'nothing', + }, }, - }); + )); delete process.env.TASKCLUSTER_ROOT_URL; return fake; })(), diff --git a/test/creds_test.js b/test/creds_test.js index 1469714..6d9ddf2 100644 --- a/test/creds_test.js +++ b/test/creds_test.js @@ -343,37 +343,41 @@ suite('client credential handling', function() { }); suite('Get with credentials from environment variables', async () => { - let ACCESS_TOKEN = process.env.TASKCLUSTER_ACCESS_TOKEN; + let ROOT_URL = process.env.TASKCLUSTER_ROOT_URL; let CLIENT_ID = process.env.TASKCLUSTER_CLIENT_ID; + let ACCESS_TOKEN = process.env.TASKCLUSTER_ACCESS_TOKEN; // Ensure the client is removed from the require cache so it can be // reloaded from scratch. - let cleanClient = null; setup(() => { + process.env.TASKCLUSTER_ROOT_URL = 'https://taskcluster.net'; process.env.TASKCLUSTER_CLIENT_ID = 'tester'; process.env.TASKCLUSTER_ACCESS_TOKEN = 'no-secret'; - - // This is an absolute path to the client.js file. If this file is moved - // then this obviously will break. The intent is to re-require the file - // with the environment variables in place, since they are used at - // load time - let clientPath = path.resolve(__dirname, '..', 'src', 'client.js'); - delete require.cache[clientPath]; - cleanClient = require(clientPath); }); // Be a good citizen and cleanup after this test so we don't leak state. teardown(() => { - if (cleanClient.agents.http.destroy) { - cleanClient.agents.http.destroy(); - cleanClient.agents.https.destroy(); - } + process.env.TASKCLUSTER_ROOT_URL = ROOT_URL; process.env.TASKCLUSTER_CLIENT_ID = CLIENT_ID; process.env.TASKCLUSTER_ACCESS_TOKEN = ACCESS_TOKEN; }); - test('implicit credentials', async () => { - let client = new cleanClient.Auth({rootUrl: 'https://taskcluster.net'}); + test('fromEnvVars with only rootUrl', async () => { + delete process.env.TASKCLUSTER_CLIENT_ID; + delete process.env.TASKCLUSTER_ACCESS_TOKEN; + assert.deepEqual(taskcluster.fromEnvVars(), + {rootUrl: 'https://taskcluster.net'}); + }); + + test('fromEnvVars with only accessToken', async () => { + delete process.env.TASKCLUSTER_ROOT_URL; + delete process.env.TASKCLUSTER_CLIENT_ID; + assert.deepEqual(taskcluster.fromEnvVars(), + {credentials: {accessToken: 'no-secret'}}); + }); + + test('fromEnvVar credentials', async () => { + let client = new taskcluster.Auth(taskcluster.fromEnvVars()); assert.deepEqual( await client.testAuthenticate({ clientScopes: [],