joyent/node-triton#28 profile opts (-i, -a, -k, -u) aren't being applied to profiles other than the env profile

Profile/CloudAPI top-level CLI options are now applied to the current
profile. Also clean up loading of the 'env' profile a bit so that
special casing of that is more hidden in "config.js".

Also add support for the TRITON_URL, TRITON_TLS_INSECURE and
TRITON_ACCOUNT envvars. (I didn't add TRITON_KEY_ID because a coming
change will do better than that.)

Fixes #28, #24.

lib/cli.js

@@ -64,18 +64,28 @@ var OPTIONS = [
     },
 
     {
-        group: 'CloudApi Options'
+        group: 'CloudAPI Options'
     },
-    // XXX SDC_USER support. I don't grok the node-smartdc/README.md discussion
+
-    //      of SDC_USER.
+    /*
+     * Environment variable integration.
+     *
+     * While dashdash supports integrated envvar parsing with options
+     * we don't use that with `triton` because (a) we want to apply *option*
+     * usage (but not envvars) to profiles other than the default 'env'
+     * profile, and (b) we want to support `TRITON_*` *and* `SDC_*` envvars,
+     * which dashdash doesn't support.
+     *
+     * See <https://github.com/joyent/node-triton/issues/28> for some details.
+     */
     {
         names: ['account', 'a'],
         type: 'string',
-        env: 'SDC_ACCOUNT',
+        help: 'Account (login name). Environment: TRITON_ACCOUNT=ACCOUNT ' +
-        help: 'TritonApi account (login name)',
+            'or SDC_ACCOUNT=ACCOUNT.',
         helpArg: 'ACCOUNT'
     },
-    // XXX
+    // TODO: subuser/RBAC support
     //{
     //    names: ['subuser', 'user'],
     //    type: 'string',
@@ -93,15 +103,13 @@ var OPTIONS = [
     {
         names: ['keyId', 'k'],
         type: 'string',
-        env: 'SDC_KEY_ID',
+        help: 'SSH key fingerprint. Environment: SDC_KEY_ID=FINGERPRINT.',
-        help: 'SSH key fingerprint',
         helpArg: 'FINGERPRINT'
     },
     {
         names: ['url', 'u'],
         type: 'string',
-        env: 'SDC_URL',
+        help: 'CloudAPI URL. Environment: TRITON_URL=URL or SDC_URL=URL.',
-        help: 'CloudApi URL',
         helpArg: 'URL'
     },
     {
@@ -115,9 +123,10 @@ var OPTIONS = [
     {
         names: ['insecure', 'i'],
         type: 'bool',
-        help: 'Do not validate SSL certificate',
+        help: 'Do not validate the CloudAPI SSL certificate. Environment: ' +
+            'TRITON_TLS_INSECURE=1, SDC_TLS_INSECURE=1 (or the deprecated ' +
+            'SDC_TESTING=1).',
         'default': false,
-        env: 'SDC_TLS_INSECURE'  // Deprecated SDC_TESTING supported below.
     }
 ];
 
@@ -170,13 +179,13 @@ util.inherits(CLI, Cmdln);
 
 CLI.prototype.init = function (opts, args, callback) {
     var self = this;
+    this.opts = opts;
 
     if (opts.version) {
         console.log(this.name, pkg.version);
         callback(false);
         return;
     }
-    this.opts = opts;
 
     this.log = bunyan.createLogger({
         name: this.name,
@@ -190,10 +199,13 @@ CLI.prototype.init = function (opts, args, callback) {
         this.showErrStack = true;
     }
 
-    if (!opts.url && opts.J) {
+    if (opts.url && opts.J) {
+        callback(new errors.UsageError(
+            'cannot use both "--url" and "-J" options'));
+    } else if (opts.J) {
         opts.url = format('https://%s.api.joyent.com', opts.J);
     }
-    this.envProfile = mod_config.loadEnvProfile(opts);
+
     this.configDir = CONFIG_DIR;
 
     this.__defineGetter__('tritonapi', function () {
@@ -202,16 +214,13 @@ CLI.prototype.init = function (opts, args, callback) {
                 configDir: self.configDir
             });
             self.log.trace({config: config}, 'loaded config');
+
             var profileName = opts.profile || config.profile || 'env';
-            var profile;
+            var profile = mod_config.loadProfile({
-            if (profileName === 'env') {
-                profile = self.envProfile;
-            } else {
-                profile = mod_config.loadProfile({
                 configDir: self.configDir,
                 name: profileName
             });
-            }
+            self._applyProfileOverrides(profile);
             self.log.trace({profile: profile}, 'loaded profile');
 
             self._tritonapi = new TritonApi({
@@ -228,6 +237,19 @@ CLI.prototype.init = function (opts, args, callback) {
 };
 
 
+/*
+ * Apply overrides from CLI options to the given profile object *in place*.
+ */
+CLI.prototype._applyProfileOverrides =
+        function _applyProfileOverrides(profile) {
+    var self = this;
+    ['account', 'url', 'keyId', 'insecure'].forEach(function (field) {
+        if (self.opts.hasOwnProperty(field)) {
+            profile[field] = self.opts[field];
+        }
+    });
+};
+
 
 // Meta
 CLI.prototype.do_completion = require('./do_completion');

lib/config.js

@@ -205,29 +205,30 @@ function setConfigVar(opts, cb) {
 // --- Profiles
 
 /**
- * Load the special 'env' profile, which handles some details of getting
+ * Load the special 'env' profile, which handles details of getting
- * values from envvars. *Most* of that is done already via the
+ * values from envvars. Typically we'd piggyback on dashdash's env support
- * `opts` dashdash Options object.
+ * <https://github.com/trentm/node-dashdash#environment-variable-integration>.
+ * However, per the "Environment variable integration" comment in cli.js, we
+ * do that manually.
  *
  * @returns {Object} The 'env' profile.
  */
-function loadEnvProfile(opts) {
+function _loadEnvProfile() {
-    // XXX support keyId being a priv or pub key path, a la imgapi-cli
-    // XXX Add TRITON_* envvars.
     var envProfile = {
-        name: 'env',
+        name: 'env'
-        account: opts.account,
-        url: opts.url,
-        keyId: opts.keyId,
-        insecure: opts.insecure
     };
-    // If --insecure not given, look at envvar(s) for that.
+
-    var specifiedInsecureOpt = opts._order.filter(
+    envProfile.account = process.env.TRITON_ACCOUNT || process.env.SDC_ACCOUNT;
-        function (opt) { return opt.key === 'insecure'; }).length > 0;
+    envProfile.url = process.env.TRITON_URL || process.env.SDC_URL;
-    if (!specifiedInsecureOpt && process.env.SDC_TESTING) {
+    envProfile.keyId = process.env.SDC_KEY_ID;
+    if (process.env.TRITON_TLS_INSECURE) {
         envProfile.insecure = common.boolFromString(
-            process.env.SDC_TESTING,
+            process.env.TRITON_TLS_INSECURE);
-            false, '"SDC_TESTING" envvar');
+    } else if (process.env.SDC_TLS_INSECURE) {
+        envProfile.insecure = common.boolFromString(
+            process.env.SDC_TLS_INSECURE);
+    } else if (process.env.SDC_TESTING) { // deprecated
+        envProfile.insecure = common.boolFromString(process.env.SDC_TESTING);
     }
 
     _validateProfile(envProfile);
@@ -263,16 +264,22 @@ function loadProfile(opts) {
     assert.string(opts.configDir, 'opts.configDir');
     assert.string(opts.name, 'opts.name');
 
+    if (opts.name === 'env') {
+        return _loadEnvProfile();
+    } else {
         var profilePath = path.resolve(opts.configDir, 'profiles.d',
             opts.name + '.json');
         return _profileFromPath(profilePath, opts.name);
+    }
 }
 
 function loadAllProfiles(opts) {
     assert.string(opts.configDir, 'opts.configDir');
     assert.object(opts.log, 'opts.log');
 
-    var profiles = [];
+    var profiles = [
+        _loadEnvProfile()
+    ];
 
     var d = path.join(opts.configDir, 'profiles.d');
     var files = fs.readdirSync(d);
@@ -285,7 +292,8 @@ function loadAllProfiles(opts) {
         var name = path.basename(file).slice(0, - path.extname(file).length);
         if (name.toLowerCase() === 'env') {
             // Skip the special 'env'.
-            opts.log.debug('skip reserved name "env" profile: %s', file);
+            opts.log.warn({profilePath: file},
+                'invalid "env" profile; skipping');
             return;
         }
         try {
@@ -305,7 +313,6 @@ function loadAllProfiles(opts) {
 module.exports = {
     loadConfig: loadConfig,
     setConfigVar: setConfigVar,
-    loadEnvProfile: loadEnvProfile,
     loadProfile: loadProfile,
     loadAllProfiles: loadAllProfiles
 };

lib/do_profiles.js

@@ -14,7 +14,7 @@ var sortDefault = 'name';
 var columnsDefault = 'name,curr,account,url';
 var columnsDefaultLong = 'name,curr,account,url,insecure,keyId';
 
-function _listProfiles(_, opts, cb) {
+function _listProfiles(opts, args, cb) {
     var columns = columnsDefault;
     if (opts.o) {
         columns = opts.o;
@@ -35,21 +35,31 @@ function _listProfiles(_, opts, cb) {
     } catch (e) {
         return cb(e);
     }
-    profiles.push(this.envProfile);
+
+    // Current profile: Set 'curr' field. Apply CLI overrides.
+    for (i = 0; i < profiles.length; i++) {
+        var profile = profiles[i];
+        if (profile.name === this.tritonapi.profile.name) {
+            this._applyProfileOverrides(profile);
+            if (opts.json) {
+                profile.curr = true;
+            } else {
+                profile.curr = '*'; // tabular
+            }
+        } else {
+            if (opts.json) {
+                profile.curr = false;
+            } else {
+                profile.curr = ''; // tabular
+            }
+        }
+    }
 
     // Display.
     var i;
     if (opts.json) {
-        for (i = 0; i < profiles.length; i++) {
-            profiles[i].curr = (profiles[i].name ===
-                this.tritonapi.profile.name);
-        }
         common.jsonStream(profiles);
     } else {
-        for (i = 0; i < profiles.length; i++) {
-            profiles[i].curr = (profiles[i].name === this.tritonapi.profile.name
-                ? '*' : '');
-        }
         tabula(profiles, {
             skipHeader: opts.H,
             columns: columns,
@@ -59,7 +69,12 @@ function _listProfiles(_, opts, cb) {
     cb();
 }
 
-function _currentProfile(profile, opts, cb) {
+function _currentProfile(opts, args, cb) {
+    var profile = mod_config.loadProfile({
+        configDir: this.configDir,
+        name: opts.current
+    });
+
     if (this.tritonapi.profile.name === profile.name) {
         console.log('"%s" is already the current profile', profile.name);
         return cb();
@@ -95,6 +110,7 @@ function do_profiles(subcmd, opts, args, cb) {
         return;
     }
 
+    // Which action?
     var actions = [];
     if (opts.add) { actions.push('add'); }
     if (opts.current) { actions.push('current'); }
@@ -110,20 +126,19 @@ function do_profiles(subcmd, opts, args, cb) {
         action = actions[0];
     }
 
-    var name;
+    // Arg count validation.
     switch (action) {
-    case 'add':
+    //case 'add':
-        if (args.length === 1) {
+    //    if (args.length === 1) {
-            name = args[0];
+    //        name = args[0];
-        } else if (args.length > 1) {
+    //    } else if (args.length > 1) {
-            return cb(new errors.UsageError('too many args'));
+    //        return cb(new errors.UsageError('too many args'));
-        }
+    //    }
-        break;
+    //    break;
     case 'list':
     case 'current':
-    case 'edit':
+    //case 'edit':
-    case 'delete':
+    //case 'delete':
-        name = opts.current || opts.edit || opts['delete'];
         if (args.length > 0) {
             return cb(new errors.UsageError('too many args'));
         }
@@ -132,18 +147,6 @@ function do_profiles(subcmd, opts, args, cb) {
         throw new Error('unknown action: ' + action);
     }
 
-    var profile;
-    if (name) {
-        if (name === 'env') {
-            profile = this.envProfile;
-        } else {
-            profile = mod_config.loadProfile({
-                configDir: this.configDir,
-                name: name
-            });
-        }
-    }
-
     var func = {
         list: _listProfiles,
         current: _currentProfile
@@ -152,7 +155,7 @@ function do_profiles(subcmd, opts, args, cb) {
         //edit: _editProfile,
         //'delete': _deleteProfile
     }[action].bind(this);
-    func(profile, opts, cb);
+    func(opts, args, cb);
 }
 
 do_profiles.options = [