PUBAPI-1233 Add firewalls to node-triton

PUBAPI-1234 Add snapshots to node-triton

CR updates.
- add 'triton fwrule update ID <TAB>' completion on updatable fields
- make ID arg first in ^^
- make 'triton fwrule create ...' have the rule enabled by default
- cosmetic help output tweaks
- allow 'triton fwrule update ...' to not *require* that the rule is
  updated. i.e you can update just the description

etc/triton-bash-completion-types.sh

@@ -12,4 +12,11 @@ function complete_tritonupdateaccountfield {
     local candidates
     candidates="{{UPDATE_ACCOUNT_FIELDS}}"
     compgen $compgen_opts -W "$candidates" -- "$word"
+}
+
+function complete_tritonupdatefwrulefield {
+    local word="$1"
+    local candidates
+    candidates="{{UPDATE_FWRULE_FIELDS}}"
+    compgen $compgen_opts -W "$candidates" -- "$word"
 }

lib/cloudapi2.js

@@ -1195,7 +1195,7 @@ function getMachineSnapshot(opts, cb) {
     assert.func(cb, 'cb');
 
     var endpoint = format('/%s/machines/%s/snapshots/%s', this.account, opts.id,
-                          encodeURIComponent(opts.name));
+        encodeURIComponent(opts.name));
     this._passThrough(endpoint, opts, cb);
 };
 
@@ -1218,7 +1218,7 @@ function startMachineFromSnapshot(opts, cb) {
     this._request({
         method: 'POST',
         path: format('/%s/machines/%s/snapshots/%s', this.account, opts.id,
-                     encodeURIComponent(opts.name)),
+            encodeURIComponent(opts.name)),
         data: opts
     }, function (err, req, res, body) {
         cb(err, body, res);
@@ -1244,7 +1244,7 @@ function deleteMachineSnapshot(opts, cb) {
     this._request({
         method: 'DELETE',
         path: format('/%s/machines/%s/snapshots/%s', this.account, opts.id,
-                     opts.name)
+            encodeURIComponent(opts.name))
     }, function (err, req, res) {
         cb(err, res);
     });
@@ -1259,6 +1259,7 @@ function deleteMachineSnapshot(opts, cb) {
  * @param {Object} options object containing:
  *      - {String} rule (required) the fwrule text.
  *      - {Boolean} enabled (optional) default to false.
+ *      - {String} description (optional)
  * @param {Function} callback of the form f(err, fwrule, res).
  */
 CloudApi.prototype.createFirewallRule =
@@ -1269,7 +1270,7 @@ function createFirewallRule(opts, cb) {
     assert.optionalBool(opts.enabled, 'opts.enabled');
 
     var data = {};
-    Object.keys(this.UPDATE_FIREWALL_RULE_FIELDS).forEach(function (attr) {
+    Object.keys(this.UPDATE_FWRULE_FIELDS).forEach(function (attr) {
         if (opts[attr] !== undefined)
             data[attr] = opts[attr];
     });
@@ -1320,7 +1321,7 @@ function getFirewallRule(id, cb) {
 
 
 // <updatable account field> -> <expected typeof>
-CloudApi.prototype.UPDATE_FIREWALL_RULE_FIELDS = {
+CloudApi.prototype.UPDATE_FWRULE_FIELDS = {
     enabled: 'boolean',
     rule: 'string',
     description: 'string'
@@ -1330,10 +1331,13 @@ CloudApi.prototype.UPDATE_FIREWALL_RULE_FIELDS = {
 /**
  * Updates a Firewall Rule.
  *
+ * Dev Note: That 'rule' is *required* here is lame. Hoping to change that
+ * in cloudapi.
+ *
  * @param {Object} opts object containing:
  *      - {UUID} id: The fwrule id. Required.
  *      - {String} rule: The fwrule text. Required.
- *      - {Boolean} enabled: Default to false. Optional.
+ *      - {Boolean} enabled: Optional.
  *      - {String} description: Description of the rule. Optional.
  * @param {Function} callback of the form `function (err, fwrule, res)`
  */
@@ -1347,7 +1351,7 @@ function updateFirewallRule(opts, cb) {
     assert.func(cb, 'cb');
 
     var data = {};
-    Object.keys(this.UPDATE_FIREWALL_RULE_FIELDS).forEach(function (attr) {
+    Object.keys(this.UPDATE_FWRULE_FIELDS).forEach(function (attr) {
         if (opts[attr] !== undefined)
             data[attr] = opts[attr];
     });

lib/do_completion.js

@@ -13,8 +13,9 @@
 var fs = require('fs');
 var path = require('path');
 
-var UPDATE_ACCOUNT_FIELDS
+var CloudApi = require('./cloudapi2').CloudApi;
-    = require('./cloudapi2').CloudApi.prototype.UPDATE_ACCOUNT_FIELDS;
+var UPDATE_ACCOUNT_FIELDS = CloudApi.prototype.UPDATE_ACCOUNT_FIELDS;
+var UPDATE_FWRULE_FIELDS = CloudApi.prototype.UPDATE_FWRULE_FIELDS;
 
 
 // Replace {{variable}} in `s` with the template data in `d`.
@@ -39,6 +40,8 @@ function do_completion(subcmd, opts, args, cb) {
             'utf8');
         var specExtra = renderTemplate(specExtraIn, {
             UPDATE_ACCOUNT_FIELDS: Object.keys(UPDATE_ACCOUNT_FIELDS).sort()
+                .map(function (field) { return field + '='; }).join(' '),
+            UPDATE_FWRULE_FIELDS: Object.keys(UPDATE_FWRULE_FIELDS).sort()
                 .map(function (field) { return field + '='; }).join(' ')
         });
         console.log(this.bashCompletion({specExtra: specExtra}));
@@ -61,7 +64,7 @@ do_completion.options = [
     }
 ];
 do_completion.help = [
-    'Output bash completion. See help output for installation.',
+    'Emit bash completion. See help for installation.',
     '',
     'Installation:',
     '    {{name}} completion > /usr/local/etc/bash_completion.d/{{name}} # Mac',

lib/do_fwrule/do_create.js

@@ -20,14 +20,13 @@ var errors = require('../errors');
 
 function do_create(subcmd, opts, args, cb) {
     assert.optionalString(opts.description, 'opts.description');
-    assert.optionalBool(opts.enabled, 'opts.enabled');
+    assert.optionalBool(opts.disabled, 'opts.disabled');
     assert.func(cb, 'cb');
 
     if (opts.help) {
         this.do_help('help', {}, [subcmd], cb);
         return;
     }
-
     if (args.length === 0) {
         cb(new errors.UsageError('missing <fwrule> argument'));
         return;
@@ -36,17 +35,24 @@ function do_create(subcmd, opts, args, cb) {
         return;
     }
 
-    opts.rule = args[0];
+    var createOpts = {
+        rule: args[0]
+    };
+    if (!opts.disabled) {
+        createOpts.enabled = true;
+    }
+    if (opts.description) {
+        createOpts.description = opts.description;
+    }
 
-    var cli = this.top;
+    this.top.tritonapi.cloudapi.createFirewallRule(createOpts,
-    cli.tritonapi.cloudapi.createFirewallRule(opts, function (err, fwrule) {
+            function (err, fwrule) {
         if (err) {
             cb(err);
             return;
         }
-
+        console.log('Created firewall rule %s%s', fwrule.id,
-        console.log('Created firewall rule %s', fwrule.id);
+            (!fwrule.enabled ? ' (disabled)' : ''));
-
         cb();
     });
 }
@@ -64,14 +70,16 @@ do_create.options = [
         help: 'JSON stream output.'
     },
     {
-        names: ['enabled', 'e'],
+        names: ['disabled', 'd'],
         type: 'bool',
-        help: 'If the firewall rule should be enabled upon creation.'
+        help: 'Disable the created firewall rule. By default a created '
+            + 'firewall rule is enabled. Use "triton fwrule enable" '
+            + 'to enable it later.'
     },
     {
-        names: ['description', 'd'],
+        names: ['description', 'D'],
         type: 'string',
-        helpArg: '<description>',
+        helpArg: '<desc>',
         help: 'Description of the firewall rule.'
     }
 ];
@@ -84,4 +92,8 @@ do_create.help = [
     '{{options}}'
 ].join('\n');
 
+do_create.helpOpts = {
+    helpCol: 25
+};
+
 module.exports = do_create;

lib/do_fwrule/do_update.js

@@ -17,8 +17,8 @@ var vasync = require('vasync');
 
 var common = require('../common');
 var errors = require('../errors');
-var UPDATE_FIREWALL_RULE_FIELDS
+var UPDATE_FWRULE_FIELDS
-    = require('../cloudapi2').CloudApi.prototype.UPDATE_FIREWALL_RULE_FIELDS;
+    = require('../cloudapi2').CloudApi.prototype.UPDATE_FWRULE_FIELDS;
 
 
 function do_update(subcmd, opts, args, cb) {
@@ -35,7 +35,7 @@ function do_update(subcmd, opts, args, cb) {
         return;
     }
 
-    var id = args.pop();
+    var id = args.shift();
 
     vasync.pipeline({arg: {}, funcs: [
         function gatherDataArgs(ctx, next) {
@@ -47,7 +47,7 @@ function do_update(subcmd, opts, args, cb) {
             try {
                 ctx.data = common.objFromKeyValueArgs(args, {
                     disableDotted: true,
-                    typeHintFromKey: UPDATE_FIREWALL_RULE_FIELDS
+                    typeHintFromKey: UPDATE_FWRULE_FIELDS
                 });
             } catch (err) {
                 next(err);
@@ -116,13 +116,12 @@ function do_update(subcmd, opts, args, cb) {
             for (var i = 0; i < keys.length; i++) {
                 var key = keys[i];
                 var value = ctx.data[key];
-                var type = UPDATE_FIREWALL_RULE_FIELDS[key];
+                var type = UPDATE_FWRULE_FIELDS[key];
                 if (!type) {
                     next(new errors.UsageError(format('unknown or ' +
                         'unupdateable field: %s (updateable fields are: %s)',
                         key,
-                        Object.keys(UPDATE_FIREWALL_RULE_FIELDS).sort().join(
+                        Object.keys(UPDATE_FWRULE_FIELDS).sort().join(', '))));
-                            ', '))));
                     return;
                 }
 
@@ -174,18 +173,18 @@ do_update.help = [
     'Update a firewall rule',
     '',
     'Usage:',
-    '     {{name}} update [FIELD=VALUE ...] <fwrule-id>',
+    '     {{name}} update <fwrule-id> [FIELD=VALUE ...]',
     '     {{name}} update -f <json-file> <fwrule-id>',
     '',
     '{{options}}',
 
     'Updateable fields:',
-    '    ' + Object.keys(UPDATE_FIREWALL_RULE_FIELDS).sort().map(function (f) {
+    '    ' + Object.keys(UPDATE_FWRULE_FIELDS).sort().map(function (f) {
-        return f + ' (' + UPDATE_FIREWALL_RULE_FIELDS[f] + ')';
+        return f + ' (' + UPDATE_FWRULE_FIELDS[f] + ')';
     }).join('\n    '),
     ''
 ].join('\n');
 
-do_update.completionArgtypes = ['tritonupdatefwrulefield'];
+do_update.completionArgtypes = ['tritonfwrule', 'tritonupdatefwrulefield'];
 
 module.exports = do_update;

lib/do_fwrule/index.js

@@ -22,18 +22,22 @@ function FirewallRuleCLI(top) {
 
     Cmdln.call(this, {
         name: top.name + ' fwrule',
-        desc: 'List, get, create and update Triton firewall rules.',
+        desc: 'List and manage Triton firewall rules.',
         helpSubcmds: [
             'help',
-            'create',
             'list',
             'get',
+            'create',
             'update',
             'delete',
+            { group: '' },
             'enable',
             'disable',
             'instances'
-        ]
+        ],
+        helpOpts: {
+            minHelpCol: 23
+        }
     });
 }
 util.inherits(FirewallRuleCLI, Cmdln);

lib/do_image/index.js

@@ -23,7 +23,7 @@ function ImageCLI(top) {
         name: top.name + ' image',
         /* BEGIN JSSTYLED */
         desc: [
-            'List, get, create and manage Triton images.'
+            'List and manage Triton images.'
         ].join('\n'),
         /* END JSSTYLED */
         helpOpts: {

lib/do_instance/do_snapshot/do_create.js

@@ -59,7 +59,8 @@ function do_create(subcmd, opts, args, cb) {
                     return;
                 }
 
-                console.log('Creating snapshot %s', snapshot.name);
+                console.log('Creating snapshot %s of instance %s',
+                    snapshot.name, createOpts.id);
                 ctx.name = snapshot.name;
                 ctx.instId = res.instId;
 

lib/do_instance/do_snapshot/index.js

@@ -31,7 +31,7 @@ function SnapshotCLI(top) {
             'delete'
         ],
         helpBody: 'Instances can be rolled back to a snapshot using\n' +
-                  '`triton instance start --snapshot=<snapname>`'
+                  '`triton instance start --snapshot=<snapname>`.'
     });
 }
 util.inherits(SnapshotCLI, Cmdln);

lib/do_instance/index.js

@@ -22,7 +22,7 @@ function InstanceCLI(top) {
         name: top.name + ' instance',
         /* BEGIN JSSTYLED */
         desc: [
-            'List, get, create and manage Triton instances.'
+            'List and manage Triton instances.'
         ].join('\n'),
         /* END JSSTYLED */
         helpOpts: {

lib/do_network/index.js

@@ -23,7 +23,7 @@ function NetworkCLI(top) {
         name: top.name + ' network',
         /* BEGIN JSSTYLED */
         desc: [
-            'List, get, create and update Triton networks.'
+            'List and manage Triton networks.'
         ].join('\n'),
         /* END JSSTYLED */
         helpOpts: {

lib/tritonapi.js

@@ -88,8 +88,10 @@ function _stepInstId(arg, next) {
 
 /**
  * A function appropriate for `vasync.pipeline` funcs that takes a `arg.id`
- * instance name, shortid or uuid, and determines the fwrule id (setting it
+ * fwrule shortid or uuid, and determines the fwrule id (setting it
  * as `arg.fwruleId`).
+ *
+ * If the fwrule *was* retrieved, that is set as `arg.fwrule`.
  */
 function _stepFwRuleId(arg, next) {
     assert.object(arg.client, 'arg.client');
@@ -1496,10 +1498,10 @@ function listInstanceFirewallRules(opts, cb) {
 
 
 /**
- * List all machines affected by a firewall rule.
+ * List all instances affected by a firewall rule.
  *
  * @param {Object} opts
- *      - {String} id: The fwrule ID, name, or short ID. Required.
+ *      - {String} id: The fwrule ID, or short ID. Required.
  * @param {Function} callback `function (err, instances, res)`
  */
 TritonApi.prototype.listFirewallRuleInstances =
@@ -1514,7 +1516,7 @@ function listFirewallRuleInstances(opts, cb) {
     vasync.pipeline({arg: {client: self, id: opts.id}, funcs: [
         _stepFwRuleId,
 
-        function listMachines(arg, next) {
+        function listInsts(arg, next) {
             self.cloudapi.listFirewallRuleMachines({
                 id: arg.fwruleId
             }, function (err, machines, _res) {
@@ -1532,37 +1534,70 @@ function listFirewallRuleInstances(opts, cb) {
 /**
  * Update a firewall rule.
  *
+ * Dev Note: Currently cloudapi UpdateFirewallRule *requires* the 'rule' field,
+ * which is overkill. `TritonApi.updateFirewallRule` adds sugar by making
+ * 'rule' optional.
+ *
  * @param {Object} opts
- *      - {String} id: The fwrule ID, name, or short ID. Required.
+ *      - {String} id: The fwrule ID, or short ID. Required.
- *      - {String} rule: The fwrule text. Required.
+ *      - {String} rule: The fwrule text. Optional.
  *      - {Boolean} enabled: Default to false. Optional.
  *      - {String} description: Description of the rule. Optional.
- * @param {Function} callback `function (err, instances, res)`
+ *      At least one of the fields must be provided.
+ * @param {Function} callback `function (err, fwrule, res)`
  */
 TritonApi.prototype.updateFirewallRule = function updateFirewallRule(opts, cb) {
+    // TODO: strict opts field validation
     assert.string(opts.id, 'opts.id');
-    assert.string(opts.rule, 'opts.rule');
+    assert.optionalString(opts.rule, 'opts.rule');
     assert.optionalBool(opts.enabled, 'opts.enabled');
     assert.optionalString(opts.description, 'opts.description');
+    assert.ok(opts.rule !== undefined || opts.enabled !== undefined ||
+        opts.description !== undefined, 'at least one of opts.rule, '
+        + 'opts.enabled, or opts.description is required');
     assert.func(cb, 'cb');
 
     var self = this;
     var res;
-    var fwrule;
+    var updatedFwrule;
+    var updateOpts = common.objCopy(opts);
 
     vasync.pipeline({arg: {client: self, id: opts.id}, funcs: [
         _stepFwRuleId,
 
+        /*
+         * CloudAPI currently requires the 'rule' field. We provide sugar here
+         * and fill it in for you.
+         */
+        function sugarFillRuleField(arg, next) {
+            if (updateOpts.rule) {
+                next();
+            } else if (arg.fwrule) {
+                updateOpts.rule = arg.fwrule.rule;
+                next();
+            } else {
+                self.getFirewallRule(arg.fwruleId, function (err, fwrule) {
+                    if (err) {
+                        next(err);
+                    } else {
+                        updateOpts.rule = fwrule.rule;
+                        next();
+                    }
+                });
+            }
+        },
+
         function updateRule(arg, next) {
-            opts.id = arg.fwruleId;
+            updateOpts.id = arg.fwruleId;
-            self.cloudapi.updateFirewallRule(opts, function (err, rule, _res) {
+            self.cloudapi.updateFirewallRule(updateOpts,
-                res = _res;
+                    function (err, fwrule, res_) {
-                fwrule = rule;
+                res = res_;
+                updatedFwrule = fwrule;
                 next(err);
             });
         }
     ]}, function (err) {
-        cb(err, fwrule, res);
+        cb(err, updatedFwrule, res);
     });
 };
 
@@ -1571,7 +1606,7 @@ TritonApi.prototype.updateFirewallRule = function updateFirewallRule(opts, cb) {
  * Enable a firewall rule.
  *
  * @param {Object} opts
- *      - {String} id: The fwrule ID, name, or short ID. Required.
+ *      - {String} id: The fwrule ID, or short ID. Required.
  * @param {Function} callback `function (err, fwrule, res)`
  */
 TritonApi.prototype.enableFirewallRule = function enableFirewallRule(opts, cb) {
@@ -1604,7 +1639,7 @@ TritonApi.prototype.enableFirewallRule = function enableFirewallRule(opts, cb) {
  * Disable a firewall rule.
  *
  * @param {Object} opts
- *      - {String} id: The fwrule ID, name, or short ID. Required.
+ *      - {String} id: The fwrule ID, or short ID. Required.
  * @param {Function} callback `function (err, fwrule, res)`
  */
 TritonApi.prototype.disableFirewallRule =
@@ -1638,7 +1673,7 @@ function disableFirewallRule(opts, cb) {
  * Delete a firewall rule.
  *
  * @param {Object} opts
- *      - {String} id: The fwrule ID, name, or short ID. Required.
+ *      - {String} id: The fwrule ID, or short ID. Required.
  * @param {Function} callback `function (err, res)`
  *
  */

test/integration/cli-fwrules.test.js

@@ -59,15 +59,49 @@ test('triton fwrule', OPTS, function (tt) {
         });
     });
 
+    tt.test('  triton fwrule create --disabled', function (t) {
+        var cmd = f('fwrule create -d "%s"', RULE);
+        h.triton(cmd, function (err, stdout, stderr) {
+            if (h.ifErr(t, err, 'triton fwrule create --disabled'))
+                return t.end();
+            /* JSSTYLED */
+            var expected = /^Created firewall rule ([a-f0-9-]{36}) \(disabled\)$/m;
+            var match = expected.exec(stdout);
+            t.ok(match, f('stdout matches %s: %j', expected, stdout));
+
+            var id = match[1];
+            t.ok(id);
+            ID = id.match(/^(.+?)-/)[1]; // convert to short ID
+
+            t.end();
+        });
+    });
+
+    tt.test('  triton fwrule get (disabled)', function (t) {
+        var cmd = 'fwrule get ' + ID;
+
+        h.triton(cmd, function (err, stdout, stderr) {
+            if (h.ifErr(t, err, 'triton fwrule get'))
+                return t.end();
+
+            var obj = JSON.parse(stdout);
+            t.equal(obj.rule, RULE, 'fwrule rule is correct');
+            t.equal(obj.enabled, false, 'fwrule is disabled');
+            t.end();
+        });
+    });
+
     tt.test('  triton fwrule create', function (t) {
-        var cmd = f('fwrule create -d "%s" "%s"', DESC, RULE);
+        var cmd = f('fwrule create -D "%s" "%s"', DESC, RULE);
 
         h.triton(cmd, function (err, stdout, stderr) {
             if (h.ifErr(t, err, 'triton fwrule create'))
                 return t.end();
 
-            var match = stdout.match('Created firewall rule (.+)');
+            /* JSSTYLED */
-            t.ok(match, 'fwrule made');
+            var expected = /^Created firewall rule ([a-f0-9-]{36})$/m;
+            var match = expected.exec(stdout);
+            t.ok(match, f('stdout matches %s: %j', expected, stdout));
 
             var id = match[1];
             t.ok(id);
@@ -87,8 +121,7 @@ test('triton fwrule', OPTS, function (tt) {
             var obj = JSON.parse(stdout);
             t.equal(obj.rule, RULE, 'fwrule rule is correct');
             t.equal(obj.description, DESC, 'fwrule was properly created');
-            t.equal(obj.enabled, false, 'fwrule enabled defaults to false');
+            t.equal(obj.enabled, true, 'fwrule enabled defaults to true');
-
             t.end();
         });
     });
@@ -120,10 +153,10 @@ test('triton fwrule', OPTS, function (tt) {
     });
 
     tt.test('  triton fwrule update', function (t) {
-        var cmd = 'fwrule update rule="' + RULE2 + '" ' + ID;
+        var cmd = 'fwrule update ' + ID + ' rule="' + RULE2 + '"';
 
         h.triton(cmd, function (err, stdout, stderr) {
-            if (h.ifErr(t, err, 'triton fwrule disable'))
+            if (h.ifErr(t, err, 'triton fwrule update'))
                 return t.end();
 
             t.ok(stdout.match('Updated firewall rule ' + ID +

test/integration/cli-snapshots.test.js

@@ -20,7 +20,7 @@ var test = require('tape');
 // --- Globals
 
 var SNAP_NAME = 'test-snapshot';
-var INST_ALIAS = f('nodetritontest-fwrules-%s', os.hostname());
+var INST_ALIAS = f('nodetritontest-snapshots-%s', os.hostname());
 var INST;
 var OPTS = {
     skip: !h.CONFIG.allowWriteActions