Bug 1508333 - [BREAKING] do not use TASKCLUSTER_… vars by default

README.md

@@ -559,34 +559,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: {
@@ -595,26 +579,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

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.
  *

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;
       })(),

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: [],