PUBAPI-1233/PUBAPI-1234 - code-review fixes.

lib/cli.js

@@ -217,8 +217,7 @@ function CLI() {
             'stop',
             'reboot',
             'ssh',
-            'snapshot',
+            { group: 'Images, Packages, Networks, Firewall Rules' },
-            { group: 'Images, Packages, Networks' },
             'image',
             'package',
             'network',
@@ -381,9 +380,6 @@ CLI.prototype.do_package = require('./do_package');
 CLI.prototype.do_networks = require('./do_networks');
 CLI.prototype.do_network = require('./do_network');
 
-// Snapshots
-CLI.prototype.do_snapshot = require('./do_snapshot');
-
 // Hidden commands
 CLI.prototype.do_cloudapi = require('./do_cloudapi');
 CLI.prototype.do_badger = require('./do_badger');

lib/cloudapi2.js

@@ -757,8 +757,8 @@ CloudApi.prototype.rebootMachine = function rebootMachine(uuid, callback) {
  * @param {String} id (required) The machine id.
  * @param {Function} callback of the form `function (err, machine, res)`
  */
-CloudApi.prototype.enableFirewall =
+CloudApi.prototype.enableMachineFirewall =
-function enableFirewall(uuid, callback) {
+function enableMachineFirewall(uuid, callback) {
     return this._doMachine('enable_firewall', uuid, callback);
 };
 
@@ -769,8 +769,8 @@ function enableFirewall(uuid, callback) {
  * @param {String} id (required) The machine id.
  * @param {Function} callback of the form `function (err, machine, res)`
  */
-CloudApi.prototype.disableFirewall =
+CloudApi.prototype.disableMachineFirewall =
-function disableFirewall(uuid, callback) {
+function disableMachineFirewall(uuid, callback) {
     return this._doMachine('disable_firewall', uuid, callback);
 };
 
@@ -1052,6 +1052,30 @@ CloudApi.prototype.deleteMachineTags = function deleteMachineTags(opts, cb) {
     });
 };
 
+/**
+ * <http://apidocs.joyent.com/cloudapi/#DeleteMachineTag>
+ *
+ * @param {Object} opts:
+ *      - @param {UUID} id: The machine UUID. Required.
+ *      - @param {String} tag: The tag name. Required.
+ * @param {Function} cb - `function (err, res)`
+ */
+CloudApi.prototype.deleteMachineTag = function deleteMachineTag(opts, cb) {
+    assert.object(opts, 'opts');
+    assert.uuid(opts.id, 'opts.id');
+    assert.string(opts.tag, 'opts.tag');
+    assert.ok(opts.tag, 'opts.tag cannot be empty');
+    assert.func(cb, 'cb');
+
+    this._request({
+        method: 'DELETE',
+        path: format('/%s/machines/%s/tags/%s', this.account, opts.id,
+                encodeURIComponent(opts.tag))
+    }, function (err, req, res) {
+        cb(err, res);
+    });
+};
+
 
 // --- snapshots
 
@@ -1074,10 +1098,14 @@ function createMachineSnapshot(opts, cb) {
     assert.optionalString(opts.name, 'opts.name');
     assert.func(cb, 'cb');
 
+    var data = {};
+    if (opts.name)
+        data.name = opts.name;
+
     this._request({
         method: 'POST',
         path: format('/%s/machines/%s/snapshots', this.account, opts.id),
-        data: opts
+        data: data
     }, function (err, req, res, body) {
         cb(err, body, res);
     });
@@ -1135,7 +1163,7 @@ function waitForSnapshotStates(opts, cb) {
  * Returns a list of snapshot objects.
  *
  * @param {Object} options object containing:
- *                   - {String} id (required) the machine's id.
+ *      - {String} id (required) the machine's id.
  * @param {Function} callback of the form f(err, snapshot, res).
  */
 CloudApi.prototype.listMachineSnapshots =
@@ -1155,8 +1183,8 @@ function listMachineSnapshots(opts, cb) {
  * Returns a snapshot object.
  *
  * @param {Object} options object containing:
- *                   - {String} id (required) the machine's id.
+ *      - {String} id (required) the machine's id.
- *                   - {String} name (required) the snapshot's name.
+ *      - {String} name (required) the snapshot's name.
  * @param {Function} callback of the form f(err, snapshot, res).
  */
 CloudApi.prototype.getMachineSnapshot =
@@ -1167,7 +1195,7 @@ function getMachineSnapshot(opts, cb) {
     assert.func(cb, 'cb');
 
     var endpoint = format('/%s/machines/%s/snapshots/%s', this.account, opts.id,
-                          opts.name);
+                          encodeURIComponent(opts.name));
     this._passThrough(endpoint, opts, cb);
 };
 
@@ -1176,8 +1204,8 @@ function getMachineSnapshot(opts, cb) {
  * Re/boots a machine from a snapshot.
  *
  * @param {Object} options object containing:
- *                   - {String} id (required) the machine's id.
+ *      - {String} id (required) the machine's id.
- *                   - {String} name (required) the snapshot's name.
+ *      - {String} name (required) the snapshot's name.
  * @param {Function} callback of the form f(err, res).
  */
 CloudApi.prototype.startMachineFromSnapshot =
@@ -1190,7 +1218,7 @@ function startMachineFromSnapshot(opts, cb) {
     this._request({
         method: 'POST',
         path: format('/%s/machines/%s/snapshots/%s', this.account, opts.id,
-                     opts.name),
+                     encodeURIComponent(opts.name)),
         data: opts
     }, function (err, req, res, body) {
         cb(err, body, res);
@@ -1202,8 +1230,8 @@ function startMachineFromSnapshot(opts, cb) {
  * Deletes a machine snapshot.
  *
  * @param {Object} options object containing:
- *                   - {String} id (required) the machine's id.
+ *      - {String} id (required) the machine's id.
- *                   - {String} name (required) the snapshot's name.
+ *      - {String} name (required) the snapshot's name.
  * @param {Function} callback of the form f(err, res).
  */
 CloudApi.prototype.deleteMachineSnapshot =
@@ -1229,8 +1257,8 @@ function deleteMachineSnapshot(opts, cb) {
  * Creates a Firewall Rule.
  *
  * @param {Object} options object containing:
- *                   - {String} rule (required) the fwrule text.
+ *      - {String} rule (required) the fwrule text.
- *                   - {Boolean} enabled (optional) default to false.
+ *      - {Boolean} enabled (optional) default to false.
  * @param {Function} callback of the form f(err, fwrule, res).
  */
 CloudApi.prototype.createFirewallRule =
@@ -1240,10 +1268,16 @@ function createFirewallRule(opts, cb) {
     assert.optionalString(opts.description, 'opts.description');
     assert.optionalBool(opts.enabled, 'opts.enabled');
 
+    var data = {};
+    Object.keys(this.UPDATE_FIREWALL_RULE_FIELDS).forEach(function (attr) {
+        if (opts[attr] !== undefined)
+            data[attr] = opts[attr];
+    });
+
     this._request({
         method: 'POST',
         path: format('/%s/fwrules', this.account),
-        data: opts
+        data: data
     }, function (err, req, res, body) {
         cb(err, body, res);
     });
@@ -1270,7 +1304,7 @@ function listFirewallRules(opts, cb) {
 /**
  * Retrieves a Firewall Rule.
  *
- * @param {String} id (required) The machine id.
+ * @param {UUID} id: The firewall rule id.
  * @param {Function} callback of the form `function (err, fwrule, res)`
  */
 CloudApi.prototype.getFirewallRule =
@@ -1297,9 +1331,10 @@ CloudApi.prototype.UPDATE_FIREWALL_RULE_FIELDS = {
  * Updates a Firewall Rule.
  *
  * @param {Object} opts object containing:
- *                   - {String} id (required) The fwrule id.
+ *      - {UUID} id: The fwrule id. Required.
- *                   - {String} rule (required) the fwrule text.
+ *      - {String} rule: The fwrule text. Required.
- *                   - {Boolean} enabled (optional) default to false.
+ *      - {Boolean} enabled: Default to false. Optional.
+ *      - {String} description: Description of the rule. Optional.
  * @param {Function} callback of the form `function (err, fwrule, res)`
  */
 CloudApi.prototype.updateFirewallRule =
@@ -1308,13 +1343,19 @@ function updateFirewallRule(opts, cb) {
     assert.uuid(opts.id, 'opts.id');
     assert.string(opts.rule, 'opts.rule');
     assert.optionalBool(opts.enabled, 'opts.enabled');
-    assert.optionalBool(opts.description, 'opts.description');
+    assert.optionalString(opts.description, 'opts.description');
     assert.func(cb, 'cb');
 
+    var data = {};
+    Object.keys(this.UPDATE_FIREWALL_RULE_FIELDS).forEach(function (attr) {
+        if (opts[attr] !== undefined)
+            data[attr] = opts[attr];
+    });
+
     this._request({
         method: 'POST',
         path: format('/%s/fwrules/%s', this.account, opts.id),
-        data: opts
+        data: data
     }, function (err, req, res, body) {
         cb(err, body, res);
     });
@@ -1324,17 +1365,19 @@ function updateFirewallRule(opts, cb) {
 /**
  * Enable a Firewall Rule.
  *
- * @param {String} id (required) The machine id.
+ * @param {Object} opts
+ *      - {UUID} id: The firewall id. Required.
  * @param {Function} callback of the form `function (err, fwrule, res)`
  */
 CloudApi.prototype.enableFirewallRule =
-function enableFirewallRule(id, cb) {
+function enableFirewallRule(opts, cb) {
-    assert.uuid(id, 'id');
+    assert.object(opts, 'opts');
+    assert.uuid(opts.id, 'opts.id');
     assert.func(cb, 'cb');
 
     this._request({
         method: 'POST',
-        path: format('/%s/fwrules/%s/enable', this.account, id),
+        path: format('/%s/fwrules/%s/enable', this.account, opts.id),
         data: {}
     }, function (err, req, res, body) {
         cb(err, body, res);
@@ -1345,17 +1388,19 @@ function enableFirewallRule(id, cb) {
 /**
  * Disable a Firewall Rule.
  *
- * @param {String} id (required) The machine id.
+ * @param {Object} opts
+ *      - {UUID} id: The firewall id. Required.
  * @param {Function} callback of the form `function (err, fwrule, res)`
  */
 CloudApi.prototype.disableFirewallRule =
-function disableFirewallRule(id, cb) {
+function disableFirewallRule(opts, cb) {
-    assert.uuid(id, 'id');
+    assert.object(opts, 'opts');
+    assert.uuid(opts.id, 'opts.id');
     assert.func(cb, 'cb');
 
     this._request({
         method: 'POST',
-        path: format('/%s/fwrules/%s/disable', this.account, id),
+        path: format('/%s/fwrules/%s/disable', this.account, opts.id),
         data: {}
     }, function (err, req, res, body) {
         cb(err, body, res);
@@ -1367,13 +1412,13 @@ function disableFirewallRule(id, cb) {
  * Remove a Firewall Rule.
  *
  * @param {Object} opts (object)
- *      - {String} id (required) for your firewall.
+ *      - {UUID} id: The firewall id. Required.
  * @param {Function} cb of the form `function (err, res)`
  */
 CloudApi.prototype.deleteFirewallRule =
 function deleteFirewallRule(opts, cb) {
     assert.object(opts, 'opts');
-    assert.string(opts.id, 'opts.id');
+    assert.uuid(opts.id, 'opts.id');
     assert.func(cb, 'cb');
 
     this._request({
@@ -1385,31 +1430,6 @@ function deleteFirewallRule(opts, cb) {
 };
 
 
-/**
- * <http://apidocs.joyent.com/cloudapi/#DeleteMachineTag>
- *
- * @param {Object} opts:
- *      - @param {UUID} id: The machine UUID. Required.
- *      - @param {String} tag: The tag name. Required.
- * @param {Function} cb - `function (err, res)`
- */
-CloudApi.prototype.deleteMachineTag = function deleteMachineTag(opts, cb) {
-    assert.object(opts, 'opts');
-    assert.uuid(opts.id, 'opts.id');
-    assert.string(opts.tag, 'opts.tag');
-    assert.ok(opts.tag, 'opts.tag cannot be empty');
-    assert.func(cb, 'cb');
-
-    this._request({
-        method: 'DELETE',
-        path: format('/%s/machines/%s/tags/%s', this.account, opts.id,
-                encodeURIComponent(opts.tag))
-    }, function (err, req, res) {
-        cb(err, res);
-    });
-};
-
-
 /**
  * Lists all the Firewall Rules affecting a given machine.
  *

lib/do_fwrule/do_create.js

@@ -5,7 +5,7 @@
  */
 
 /*
- * Copyright 2015 Joyent, Inc.
+ * Copyright 2016 Joyent, Inc.
  *
  * `triton fwrule create ...`
  */
@@ -29,10 +29,10 @@ function do_create(subcmd, opts, args, cb) {
     }
 
     if (args.length === 0) {
-        cb(new errors.UsageError('Missing RULE argument'));
+        cb(new errors.UsageError('missing <fwrule> argument'));
         return;
     } else if (args.length > 1) {
-        cb(new errors.UsageError('Incorrect number of arguments'));
+        cb(new errors.UsageError('incorrect number of arguments'));
         return;
     }
 
@@ -71,6 +71,7 @@ do_create.options = [
     {
         names: ['description', 'd'],
         type: 'string',
+        helpArg: '<description>',
         help: 'Description of the firewall rule.'
     }
 ];
@@ -78,7 +79,7 @@ do_create.help = [
     'Create a firewall rule.',
     '',
     'Usage:',
-    '     {{name}} create [<options>] RULE',
+    '    {{name}} create [<options>] <fwrule>',
     '',
     '{{options}}'
 ].join('\n');

lib/do_fwrule/do_delete.js

@@ -5,7 +5,7 @@
  */
 
 /*
- * Copyright 2015 Joyent, Inc.
+ * Copyright 2016 Joyent, Inc.
  *
  * `triton snapshot delete ...`
  */
@@ -27,7 +27,7 @@ function do_delete(subcmd, opts, args, cb) {
     }
 
     if (args.length < 1) {
-        cb(new errors.UsageError('missing FWRULE-ID argument(s)'));
+        cb(new errors.UsageError('missing <fwrule-id> argument(s)'));
         return;
     }
 
@@ -61,23 +61,16 @@ function do_delete(subcmd, opts, args, cb) {
             vasync.forEachParallel({
                 inputs: ruleIds,
                 func: function deleteOne(id, nextId) {
-                    cli.tritonapi.getFirewallRule(id, function (err, fwrule) {
+                    cli.tritonapi.deleteFirewallRule({
+                            id: id
+                    }, function (err) {
                         if (err) {
                             nextId(err);
                             return;
                         }
 
-                        cli.tritonapi.cloudapi.deleteFirewallRule({
+                        console.log('Deleted rule %s', id);
-                            id: fwrule.id
+                        nextId();
-                        }, function (err2) {
-                            if (err2) {
-                                nextId(err2);
-                                return;
-                            }
-
-                            console.log('Deleted rule %s', id);
-                            nextId();
-                        });
                     });
                 }
             }, next);
@@ -107,7 +100,7 @@ do_delete.help = [
     'Remove a firewall rule.',
     '',
     'Usage:',
-    '     {{name}} delete [<options>] FWRULE-ID [FWRULE-ID...]',
+    '    {{name}} delete [<options>] <fwrule-id> [<fwrule-id>...]',
     '',
     '{{options}}'
 ].join('\n');

lib/do_fwrule/do_disable.js

@@ -26,7 +26,7 @@ function do_disable(subcmd, opts, args, cb) {
     }
 
     if (args.length === 0) {
-        cb(new errors.UsageError('Missing FWRULE-ID argument(s)'));
+        cb(new errors.UsageError('Missing <fwrule-id> argument(s)'));
         return;
     }
 
@@ -35,34 +35,15 @@ function do_disable(subcmd, opts, args, cb) {
     vasync.forEachParallel({
         inputs: args,
         func: function disableOne(id, nextId) {
-            if (common.isUUID(id)) {
+            cli.tritonapi.disableFirewallRule({ id: id }, function (err) {
-                enable();
-                return;
-            }
-
-            // we need to look up the full UUID if the given id is a short id
-            cli.tritonapi.getFirewallRule(id, function onRule(err, fwrule) {
                 if (err) {
                     nextId(err);
                     return;
                 }
 
-                id = fwrule.id;
+                console.log('Disabled firewall rule %s', id);
-
+                nextId();
-                enable();
             });
-
-            function enable() {
-                cli.tritonapi.cloudapi.disableFirewallRule(id, function (err) {
-                    if (err) {
-                        nextId(err);
-                        return;
-                    }
-
-                    console.log('Disabled firewall rule %s', id);
-                    nextId();
-                });
-            }
         }
     }, cb);
 }
@@ -79,7 +60,7 @@ do_disable.help = [
     'Disable a specific firewall rule.',
     '',
     'Usage:',
-    '     {{name}} disable FWRULE-ID',
+    '    {{name}} disable <fwrule-id> [<fwrule-id>...]',
     '',
     '{{options}}'
 ].join('\n');

lib/do_fwrule/do_enable.js

@@ -26,7 +26,7 @@ function do_enable(subcmd, opts, args, cb) {
     }
 
     if (args.length === 0) {
-        cb(new errors.UsageError('Missing FWRULE-ID argument(s)'));
+        cb(new errors.UsageError('Missing <fwrule-id> argument(s)'));
         return;
     }
 
@@ -35,34 +35,15 @@ function do_enable(subcmd, opts, args, cb) {
     vasync.forEachParallel({
         inputs: args,
         func: function enableOne(id, nextId) {
-            if (common.isUUID(id)) {
+            cli.tritonapi.enableFirewallRule({ id: id }, function (err) {
-                enable();
-                return;
-            }
-
-            // we need to look up the full UUID if the given id is a short id
-            cli.tritonapi.getFirewallRule(id, function onRule(err, fwrule) {
                 if (err) {
                     nextId(err);
                     return;
                 }
 
-                id = fwrule.id;
+                console.log('Enabled firewall rule %s', id);
-
+                nextId();
-                enable();
             });
-
-            function enable() {
-                cli.tritonapi.cloudapi.enableFirewallRule(id, function (err) {
-                    if (err) {
-                        nextId(err);
-                        return;
-                    }
-
-                    console.log('Enabled firewall rule %s', id);
-                    nextId();
-                });
-            }
         }
     }, cb);
 }
@@ -79,7 +60,7 @@ do_enable.help = [
     'Enable a specific firewall rule.',
     '',
     'Usage:',
-    '     {{name}} enable FWRULE-ID [FWRULE-ID...]',
+    '    {{name}} enable <fwrule-id> [<fwrule-id>...]',
     '',
     '{{options}}'
 ].join('\n');

lib/do_fwrule/do_get.js

@@ -25,11 +25,10 @@ function do_get(subcmd, opts, args, cb) {
     }
 
     if (args.length === 0) {
-        var errMsg = 'missing FWRULE-ID argument';
+        cb(new errors.UsageError('missing <fwrule-id> argument'));
-        cb(new errors.UsageError(errMsg));
         return;
     } else if (args.length > 1) {
-        cb(new errors.UsageError('Incorrect number of arguments'));
+        cb(new errors.UsageError('incorrect number of arguments'));
         return;
     }
 
@@ -69,7 +68,7 @@ do_get.help = [
     'Show a specific firewall rule.',
     '',
     'Usage:',
-    '     {{name}} get FWRULE-ID',
+    '    {{name}} get <fwrule-id>',
     '',
     '{{options}}'
 ].join('\n');

lib/do_fwrule/do_instances.js

@@ -30,10 +30,10 @@ function do_instances(subcmd, opts, args, cb) {
     }
 
     if (args.length === 0) {
-        cb(new errors.UsageError('Missing FWRULE-ID argument'));
+        cb(new errors.UsageError('missing <fwrule-id> argument'));
         return;
     } else if (args.length > 1) {
-        cb(new errors.UsageError('Incorrect number of arguments'));
+        cb(new errors.UsageError('incorrect number of arguments'));
         return;
     }
 
@@ -67,7 +67,7 @@ function do_instances(subcmd, opts, args, cb) {
             });
         },
         function getTheMachines(next) {
-            tritonapi.cloudapi.listFirewallRuleMachines({
+            tritonapi.listFirewallRuleInstances({
                 id: id
             }, function (err, _insts) {
                 if (err) {
@@ -140,7 +140,7 @@ do_instances.help = [
     'List instances a firewall rule is applied to.',
     '',
     'Usage:',
-    '     {{name}} instances [<options>] FWRULE-ID',
+    '    {{name}} instances [<options>] <fwrule-id>',
     '',
     '{{options}}',
     '',

lib/do_fwrule/do_list.js

@@ -31,7 +31,7 @@ function do_list(subcmd, opts, args, cb) {
     }
 
     if (args.length > 0) {
-        cb(new errors.UsageError('Incorrect number of arguments'));
+        cb(new errors.UsageError('incorrect number of arguments'));
         return;
     }
 
@@ -63,7 +63,7 @@ function do_list(subcmd, opts, args, cb) {
             }
 
             tabula(rules, {
-                skipHeader: false,
+                skipHeader: opts.H,
                 columns: columns,
                 sort: sort
             });
@@ -88,7 +88,7 @@ do_list.help = [
     'Show all firewall rules.',
     '',
     'Usage:',
-    '     {{name}} list [<options>]',
+    '   {{name}} list [<options>]',
     '',
     '{{options}}'
 ].join('\n');

lib/do_fwrule/do_update.js

@@ -31,7 +31,7 @@ function do_update(subcmd, opts, args, cb) {
     var tritonapi = this.top.tritonapi;
 
     if (args.length === 0) {
-        cb(new errors.UsageError('Missing FWRULE-ID argument'));
+        cb(new errors.UsageError('missing <fwrule-id> argument'));
         return;
     }
 
@@ -136,30 +136,11 @@ function do_update(subcmd, opts, args, cb) {
             next();
         },
 
-        // we need to look up the full UUID if the given id is a short id
-        function getFullId(ctx, next) {
-            if (common.isUUID(id)) {
-                ctx.data.id = id;
-                next();
-                return;
-            }
-
-            tritonapi.getFirewallRule(id, function onRule(err, fwrule) {
-                if (err) {
-                    next(err);
-                    return;
-                }
-
-                ctx.data.id = fwrule.id;
-
-                next();
-            });
-        },
-
         function updateAway(ctx, next) {
             var data = ctx.data;
+            data.id = id;
 
-            tritonapi.cloudapi.updateFirewallRule(data, function (err) {
+            tritonapi.updateFirewallRule(data, function (err) {
                 if (err) {
                     next(err);
                     return;
@@ -184,7 +165,7 @@ do_update.options = [
     {
         names: ['file', 'f'],
         type: 'string',
-        helpArg: 'FILE',
+        helpArg: '<json-file>',
         help: 'A file holding a JSON file of updates, or "-" to read ' +
             'JSON from stdin.'
     }
@@ -193,8 +174,8 @@ do_update.help = [
     'Update a firewall rule',
     '',
     'Usage:',
-    '     {{name}} update [FIELD=VALUE ...] FWRULE-ID',
+    '     {{name}} update [FIELD=VALUE ...] <fwrule-id>',
-    '     {{name}} update -f JSON-FILE FWRULE-ID',
+    '     {{name}} update -f <json-file> <fwrule-id>',
     '',
     '{{options}}',
 

lib/do_fwrule/index.js

@@ -7,7 +7,7 @@
 /*
  * Copyright 2016 Joyent, Inc.
  *
- * `triton snapshot ...`
+ * `triton fwrule ...`
  */
 
 var Cmdln = require('cmdln').Cmdln;
@@ -22,10 +22,9 @@ function FirewallRuleCLI(top) {
 
     Cmdln.call(this, {
         name: top.name + ' fwrule',
-        desc: 'Firewall rule commands',
+        desc: 'List, get, create and update Triton firewall rules.',
         helpSubcmds: [
             'help',
-            { group: 'Key Resources' },
             'create',
             'list',
             'get',

lib/do_instance/do_fwrules.js

@@ -31,15 +31,19 @@ function do_fwrules(subcmd, opts, args, cb) {
     }
 
     if (args.length === 0) {
-        cb(new errors.UsageError('Missing INST argument'));
+        cb(new errors.UsageError('missing <inst> argument'));
         return;
     } else if (args.length > 1) {
-        cb(new errors.UsageError('Incorrect number of arguments'));
+        cb(new errors.UsageError('incorrect number of arguments'));
         return;
     }
 
+    var id = args[0];
+
     var cli = this.top;
-    cli.tritonapi.cloudapi.listFirewallRules({}, function onRules(err, rules) {
+    cli.tritonapi.listInstanceFirewallRules({
+        id: id
+    }, function onRules(err, rules) {
         if (err) {
             cb(err);
             return;
@@ -66,7 +70,7 @@ function do_fwrules(subcmd, opts, args, cb) {
             }
 
             tabula(rules, {
-                skipHeader: false,
+                skipHeader: opts.H,
                 columns: columns,
                 sort: sort
             });
@@ -91,11 +95,9 @@ do_fwrules.help = [
     'Show firewall rules applied to an instance.',
     '',
     'Usage:',
-    '     {{name}} fwrules [<options>] INST',
+    '     {{name}} fwrules [<options>] <inst>',
     '',
     '{{options}}'
 ].join('\n');
 
-//do_fwrules.aliases = ['fwrules'];
-
 module.exports = do_fwrules;

lib/do_instance/do_snapshot/do_create.js

@@ -14,9 +14,9 @@ var assert = require('assert-plus');
 var format = require('util').format;
 var vasync = require('vasync');
 
-var common = require('../common');
+var common = require('../../common');
-var distractions = require('../distractions');
+var distractions = require('../../distractions');
-var errors = require('../errors');
+var errors = require('../../errors');
 
 
 function do_create(subcmd, opts, args, cb) {
@@ -29,7 +29,7 @@ function do_create(subcmd, opts, args, cb) {
     }
 
     if (args.length === 0) {
-        cb(new errors.UsageError('missing INST argument'));
+        cb(new errors.UsageError('missing <inst> argument'));
         return;
     } else if (args.length > 1) {
         cb(new errors.UsageError('incorrect number of arguments'));
@@ -52,8 +52,8 @@ function do_create(subcmd, opts, args, cb) {
         function createSnapshot(ctx, next) {
             ctx.start = Date.now();
 
-            cli.tritonapi.cloudapi.createMachineSnapshot(createOpts,
+            cli.tritonapi.createInstanceSnapshot(createOpts,
-            function (err, snapshot) {
+            function (err, snapshot, res) {
                 if (err) {
                     next(err);
                     return;
@@ -61,6 +61,7 @@ function do_create(subcmd, opts, args, cb) {
 
                 console.log('Creating snapshot %s', snapshot.name);
                 ctx.name = snapshot.name;
+                ctx.instId = res.instId;
 
                 next();
             });
@@ -81,7 +82,7 @@ function do_create(subcmd, opts, args, cb) {
             var waiter = cloudapi.waitForSnapshotStates.bind(cloudapi);
 
             waiter({
-                id: inst,
+                id: ctx.instId,
                 name: ctx.name,
                 states: ['created', 'failed']
             }, function (err, snap) {
@@ -124,7 +125,7 @@ do_create.options = [
     {
         names: ['name', 'n'],
         type: 'string',
-        helpArg: 'SNAPSHOT-NAME',
+        helpArg: '<snapname>',
         help: 'An optional name for a snapshot.'
     },
     {
@@ -135,10 +136,10 @@ do_create.options = [
     }
 ];
 do_create.help = [
-    'Create a snapshot of a machine.',
+    'Create a snapshot of an instance.',
     '',
     'Usage:',
-    '     {{name}} create [<options>] INST',
+    '    {{name}} create [<options>] <inst>',
     '',
     '{{options}}',
     'Snapshot do not work for instances of type "kvm".'

lib/do_instance/do_snapshot/do_delete.js

@@ -14,9 +14,9 @@ var assert = require('assert-plus');
 var format = require('util').format;
 var vasync = require('vasync');
 
-var common = require('../common');
+var common = require('../../common');
-var distractions = require('../distractions');
+var distractions = require('../../distractions');
-var errors = require('../errors');
+var errors = require('../../errors');
 
 
 function do_delete(subcmd, opts, args, cb) {
@@ -28,7 +28,7 @@ function do_delete(subcmd, opts, args, cb) {
     }
 
     if (args.length < 2) {
-        cb(new errors.UsageError('missing INST and SNAPSHOT-NAME argument(s)'));
+        cb(new errors.UsageError('missing <inst> and <snapname> argument(s)'));
         return;
     }
 
@@ -36,7 +36,7 @@ function do_delete(subcmd, opts, args, cb) {
     var inst = args[0];
     var names = args.slice(1, args.length);
 
-    function wait(name, startTime, next) {
+    function wait(instId, name, startTime, next) {
         //  1 'wait': no distraction.
         // >1 'wait': distraction, pass in the N.
         var distraction;
@@ -48,7 +48,7 @@ function do_delete(subcmd, opts, args, cb) {
         var waiter = cloudapi.waitForSnapshotStates.bind(cloudapi);
 
         waiter({
-            id: inst,
+            id: instId,
             name: name,
             states: ['deleted']
         }, function (err, snap) {
@@ -100,19 +100,22 @@ function do_delete(subcmd, opts, args, cb) {
             vasync.forEachParallel({
                 inputs: names,
                 func: function deleteOne(name, nextName) {
-                    cli.tritonapi.cloudapi.deleteMachineSnapshot({
+                    cli.tritonapi.deleteInstanceSnapshot({
                         id: inst,
                         name: name
-                    }, function (err) {
+                    }, function (err, res) {
                         if (err) {
                             nextName(err);
                             return;
                         }
 
-                        console.log('Deleting snapshot "%s"', name);
+                        var instId = res.instId;
+
+                        var msg = 'Deleting snapshot "%s" of instance "%s"';
+                        console.log(msg, name, instId);
 
                         if (opts.wait) {
-                            wait(name, startTime, nextName);
+                            wait(instId, name, startTime, nextName);
                         } else {
                             nextName();
                         }
@@ -148,10 +151,10 @@ do_delete.options = [
     }
 ];
 do_delete.help = [
-    'Remove a snapshot from a machine.',
+    'Remove a snapshot from an instance.',
     '',
     'Usage:',
-    '     {{name}} delete [<options>] SNAPSHOT-NAME [SNAPSHOT-NAME...]',
+    '    {{name}} delete [<options>] <inst> <snapname> [<snapname>...]',
     '',
     '{{options}}'
 ].join('\n');

lib/do_instance/do_snapshot/do_get.js

@@ -12,8 +12,8 @@
 
 var assert = require('assert-plus');
 
-var common = require('../common');
+var common = require('../../common');
-var errors = require('../errors');
+var errors = require('../../errors');
 
 
 function do_get(subcmd, opts, args, cb) {
@@ -25,8 +25,7 @@ function do_get(subcmd, opts, args, cb) {
     }
 
     if (args.length < 2) {
-        var errMsg = 'missing INST and/or SNAPSHOT-NAME arguments';
+        cb(new errors.UsageError('missing <inst> and/or <snapname> arguments'));
-        cb(new errors.UsageError(errMsg));
         return;
     } else if (args.length > 2) {
         cb(new errors.UsageError('incorrect number of arguments'));
@@ -37,7 +36,7 @@ function do_get(subcmd, opts, args, cb) {
     var name = args[1];
     var cli = this.top;
 
-    cli.tritonapi.cloudapi.getMachineSnapshot({
+    cli.tritonapi.getInstanceSnapshot({
         id: id,
         name: name
     }, function onSnapshot(err, snapshot) {
@@ -70,10 +69,10 @@ do_get.options = [
     }
 ];
 do_get.help = [
-    'Show a specific snapshot of a machine.',
+    'Show a specific snapshot of an instance.',
     '',
     'Usage:',
-    '     {{name}} get INST SNAPSHOT-NAME',
+    '    {{name}} get <inst> <snapname>',
     '',
     '{{options}}'
 ].join('\n');

lib/do_instance/do_snapshot/do_list.js

@@ -13,11 +13,11 @@
 var assert = require('assert-plus');
 var tabula = require('tabula');
 
-var common = require('../common');
+var common = require('../../common');
-var errors = require('../errors');
+var errors = require('../../errors');
 
 
-var COLUMNS_DEFAULT = 'name,state';
+var COLUMNS_DEFAULT = 'name,state,created';
 var SORT_DEFAULT = 'name';
 
 
@@ -29,7 +29,10 @@ function do_list(subcmd, opts, args, cb) {
         return;
     }
 
-    if (args.length !== 1) {
+    if (args.length === 0) {
+        cb(new errors.UsageError('missing <inst> argument'));
+        return;
+    } else if (args.length > 1) {
         cb(new errors.UsageError('incorrect number of arguments'));
         return;
     }
@@ -37,7 +40,7 @@ function do_list(subcmd, opts, args, cb) {
     var cli = this.top;
     var machineId = args[0];
 
-    cli.tritonapi.cloudapi.listMachineSnapshots({
+    cli.tritonapi.listInstanceSnapshots({
         id: machineId
     }, function onSnapshots(err, snapshots) {
         if (err) {
@@ -60,7 +63,7 @@ function do_list(subcmd, opts, args, cb) {
             var sort = opts.s.split(',');
 
             tabula(snapshots, {
-                skipHeader: false,
+                skipHeader: opts.H,
                 columns: columns,
                 sort: sort
             });
@@ -82,10 +85,10 @@ do_list.options = [
 }));
 
 do_list.help = [
-    'Show all of a machines\'s snapshots.',
+    'Show all of an instance\'s snapshots.',
     '',
     'Usage:',
-    '     {{name}} list [<options>]',
+    '    {{name}} list [<options>] <inst>',
     '',
     '{{options}}'
 ].join('\n');

lib/do_instance/do_snapshot/index.js

@@ -5,7 +5,7 @@
  */
 
 /*
- * Copyright 2015 Joyent, Inc.
+ * Copyright 2016 Joyent, Inc.
  *
  * `triton snapshot ...`
  */
@@ -18,21 +18,20 @@ var util = require('util');
 // ---- CLI class
 
 function SnapshotCLI(top) {
-    this.top = top;
+    this.top = top.top;
 
     Cmdln.call(this, {
         name: top.name + ' snapshot',
-        desc: 'Machine snapshot commands',
+        desc: 'List, get, create and delete Triton instance snapshots.',
         helpSubcmds: [
             'help',
-            { group: 'Key Resources' },
             'create',
             'list',
             'get',
             'delete'
         ],
-        helpBody: 'Machines can be rolled back to a snapshot using\n' +
+        helpBody: 'Instances can be rolled back to a snapshot using\n' +
-                  '`triton instance start --snapshot=SNAPSHOT-NAME`'
+                  '`triton instance start --snapshot=<snapname>`'
     });
 }
 util.inherits(SnapshotCLI, Cmdln);

lib/do_instance/do_snapshots.js

@@ -0,0 +1,26 @@
+/*
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+/*
+ * Copyright 2016 Joyent, Inc.
+ *
+ * `triton instance snapshots ...` shortcut for
+ * `triton instance snapshot list ...`.
+ */
+
+function do_snapshots(subcmd, opts, args, callback) {
+    this.handlerFromSubcmd('snapshot').dispatch({
+        subcmd: 'list',
+        opts: opts,
+        args: args
+    }, callback);
+}
+
+do_snapshots.help = 'A shortcut for "triton instance snapshot list".';
+do_snapshots.options = require('./do_snapshot/do_list').options;
+do_snapshots.hidden = true;
+
+module.exports = do_snapshots;

lib/do_instance/index.js

@@ -43,6 +43,7 @@ function InstanceCLI(top) {
             'wait',
             'audit',
             'fwrules',
+            'snapshot',
             'tag'
         ]
     });
@@ -67,6 +68,8 @@ InstanceCLI.prototype.do_ssh = require('./do_ssh');
 InstanceCLI.prototype.do_wait = require('./do_wait');
 InstanceCLI.prototype.do_audit = require('./do_audit');
 InstanceCLI.prototype.do_fwrules = require('./do_fwrules');
+InstanceCLI.prototype.do_snapshot = require('./do_snapshot');
+InstanceCLI.prototype.do_snapshots = require('./do_snapshots');
 InstanceCLI.prototype.do_tag = require('./do_tag');
 InstanceCLI.prototype.do_tags = require('./do_tags');
 

lib/tritonapi.js

@@ -86,6 +86,29 @@ 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
+ * as `arg.fwruleId`).
+ */
+function _stepFwRuleId(arg, next) {
+    assert.object(arg.client, 'arg.client');
+    assert.string(arg.id, 'arg.id');
+
+    if (common.isUUID(arg.id)) {
+        arg.fwruleId = arg.id;
+        next();
+    } else {
+        arg.client.getFirewallRule(arg.id, function (err, fwrule) {
+            if (err) {
+                next(err);
+            } else {
+                arg.fwruleId = fwrule.id;
+                next();
+            }
+        });
+    }
+}
 
 //---- TritonApi class
 
@@ -563,53 +586,6 @@ TritonApi.prototype.getNetwork = function getNetwork(name, cb) {
 };
 
 
-/**
- * Get a firewall rule by ID, or short ID, in that order.
- *
- * If there is more than one firewall rule with that short ID, then this errors
- * out.
- */
-TritonApi.prototype.getFirewallRule = function getFirewallRule(id, cb) {
-    assert.string(id, 'id');
-    assert.func(cb, 'cb');
-
-    if (common.isUUID(id)) {
-        this.cloudapi.getFirewallRule(id, function (err, fwrule) {
-            if (err) {
-                if (err.restCode === 'ResourceNotFound') {
-                    err = new errors.ResourceNotFoundError(err,
-                        format('firewall rule with id %s was not found', id));
-                }
-                cb(err);
-            } else {
-                cb(null, fwrule);
-            }
-        });
-    } else {
-        this.cloudapi.listFirewallRules(function (err, fwrules) {
-            if (err) {
-                return cb(err);
-            }
-
-            var shortIdMatches = fwrules.filter(function (fwrule) {
-                return fwrule.id.slice(0, 8) === id;
-            });
-
-            if (shortIdMatches.length === 1) {
-                cb(null, shortIdMatches[0]);
-            } else if (shortIdMatches.length === 0) {
-                cb(new errors.ResourceNotFoundError(format(
-                    'no firewall rule with short id "%s" was found', id)));
-            } else {
-                cb(new errors.ResourceNotFoundError(
-                    format('"%s" is an ambiguous short id, with multiple ' +
-                           'matching firewall rules', id)));
-            }
-        });
-    }
-};
-
-
 /**
  * Get an instance.
  *
@@ -771,6 +747,157 @@ TritonApi.prototype.getInstance = function getInstance(opts, cb) {
 };
 
 
+// ---- instance snapshots
+
+/**
+ * Create a snapshot of an instance.
+ *
+ * @param {Object} opts
+ *      - {String} id: The instance ID, name, or short ID. Required.
+ *      - {String} name: The name for new snapshot. Optional.
+ * @param {Function} callback `function (err, snapshots, res)`
+ */
+TritonApi.prototype.createInstanceSnapshot =
+function createInstanceSnapshot(opts, cb) {
+    assert.string(opts.id, 'opts.id');
+    assert.optionalString(opts.name, 'opts.name');
+    assert.func(cb, 'cb');
+
+    var self = this;
+    var res;
+    var snapshot;
+
+    vasync.pipeline({arg: {client: self, id: opts.id}, funcs: [
+        _stepInstId,
+
+        function createSnapshot(arg, next) {
+            self.cloudapi.createMachineSnapshot({
+                id: arg.instId,
+                name: opts.name
+            }, function (err, snap, _res) {
+                res = _res;
+                res.instId = arg.instId; // gross hack, in case caller needs it
+                snapshot = snap;
+                next(err);
+            });
+        }
+    ]}, function (err) {
+        cb(err, snapshot, res);
+    });
+};
+
+
+/**
+ * List an instance's snapshots.
+ *
+ * @param {Object} opts
+ *      - {String} id: The instance ID, name, or short ID. Required.
+ * @param {Function} callback `function (err, snapshots, res)`
+ */
+TritonApi.prototype.listInstanceSnapshots =
+function listInstanceSnapshots(opts, cb) {
+    assert.string(opts.id, 'opts.id');
+    assert.func(cb, 'cb');
+
+    var self = this;
+    var res;
+    var snapshots;
+
+    vasync.pipeline({arg: {client: self, id: opts.id}, funcs: [
+        _stepInstId,
+
+        function listSnapshots(arg, next) {
+            self.cloudapi.listMachineSnapshots({
+                id: arg.instId,
+                name: opts.name
+            }, function (err, snaps, _res) {
+                res = _res;
+                res.instId = arg.instId; // gross hack, in case caller needs it
+                snapshots = snaps;
+                next(err);
+            });
+        }
+    ]}, function (err) {
+        cb(err, snapshots, res);
+    });
+};
+
+
+/**
+ * Get an instance's snapshot.
+ *
+ * @param {Object} opts
+ *      - {String} id: The instance ID, name, or short ID. Required.
+ *      - {String} name: The name of the snapshot. Required.
+ * @param {Function} callback `function (err, snapshot, res)`
+ */
+TritonApi.prototype.getInstanceSnapshot =
+function getInstanceSnapshot(opts, cb) {
+    assert.string(opts.id, 'opts.id');
+    assert.string(opts.name, 'opts.name');
+    assert.func(cb, 'cb');
+
+    var self = this;
+    var res;
+    var snapshot;
+
+    vasync.pipeline({arg: {client: self, id: opts.id}, funcs: [
+        _stepInstId,
+
+        function getSnapshot(arg, next) {
+            self.cloudapi.getMachineSnapshot({
+                id: arg.instId,
+                name: opts.name
+            }, function (err, _snap, _res) {
+                res = _res;
+                res.instId = arg.instId; // gross hack, in case caller needs it
+                snapshot = _snap;
+                next(err);
+            });
+        }
+    ]}, function (err) {
+        cb(err, snapshot, res);
+    });
+};
+
+
+/**
+ * Delete an instance's snapshot.
+ *
+ * @param {Object} opts
+ *      - {String} id: The instance ID, name, or short ID. Required.
+ *      - {String} name: The name of the snapshot. Required.
+ * @param {Function} callback `function (err, res)`
+ *
+ */
+TritonApi.prototype.deleteInstanceSnapshot =
+function deleteInstanceSnapshot(opts, cb) {
+    assert.string(opts.id, 'opts.id');
+    assert.string(opts.name, 'opts.name');
+    assert.func(cb, 'cb');
+
+    var self = this;
+    var res;
+
+    vasync.pipeline({arg: {client: self, id: opts.id}, funcs: [
+        _stepInstId,
+
+        function deleteSnapshot(arg, next) {
+            self.cloudapi.deleteMachineSnapshot({
+                id: arg.instId,
+                name: opts.name
+            }, function (err, _res) {
+                res = _res;
+                res.instId = arg.instId; // gross hack, in case caller needs it
+                next(err);
+            });
+        }
+    ]}, function (err) {
+        cb(err, res);
+    });
+};
+
+
 // ---- instance tags
 
 /**
@@ -1285,6 +1412,260 @@ function deleteAllInstanceTags(opts, cb) {
 };
 
 
+// ---- Firewall Rules
+
+/**
+ * Get a firewall rule by ID, or short ID, in that order.
+ *
+ * If there is more than one firewall rule with that short ID, then this errors
+ * out.
+ */
+TritonApi.prototype.getFirewallRule = function getFirewallRule(id, cb) {
+    assert.string(id, 'id');
+    assert.func(cb, 'cb');
+
+    if (common.isUUID(id)) {
+        this.cloudapi.getFirewallRule(id, function (err, fwrule) {
+            if (err) {
+                if (err.restCode === 'ResourceNotFound') {
+                    err = new errors.ResourceNotFoundError(err,
+                        format('firewall rule with id %s was not found', id));
+                }
+                cb(err);
+            } else {
+                cb(null, fwrule);
+            }
+        });
+    } else {
+        this.cloudapi.listFirewallRules({}, function (err, fwrules) {
+            if (err) {
+                return cb(err);
+            }
+
+            var shortIdMatches = fwrules.filter(function (fwrule) {
+                return fwrule.id.slice(0, 8) === id;
+            });
+
+            if (shortIdMatches.length === 1) {
+                cb(null, shortIdMatches[0]);
+            } else if (shortIdMatches.length === 0) {
+                cb(new errors.ResourceNotFoundError(format(
+                    'no firewall rule with short id "%s" was found', id)));
+            } else {
+                cb(new errors.ResourceNotFoundError(
+                    format('"%s" is an ambiguous short id, with multiple ' +
+                           'matching firewall rules', id)));
+            }
+        });
+    }
+};
+
+
+/**
+ * List all firewall rules affecting an instance.
+ *
+ * @param {Object} opts
+ *      - {String} id: The instance ID, name, or short ID. Required.
+ * @param {Function} callback `function (err, instances, res)`
+ */
+TritonApi.prototype.listInstanceFirewallRules =
+function listInstanceFirewallRules(opts, cb) {
+    assert.string(opts.id, 'opts.id');
+    assert.func(cb, 'cb');
+
+    var self = this;
+    var res;
+    var fwrules;
+
+    vasync.pipeline({arg: {client: self, id: opts.id}, funcs: [
+        _stepInstId,
+
+        function listRules(arg, next) {
+            self.cloudapi.listMachineFirewallRules({
+                id: arg.instId
+            }, function (err, rules, _res) {
+                res = _res;
+                fwrules = rules;
+                next(err);
+            });
+        }
+    ]}, function (err) {
+        cb(err, fwrules, res);
+    });
+};
+
+
+/**
+ * List all machines affected by a firewall rule.
+ *
+ * @param {Object} opts
+ *      - {String} id: The fwrule ID, name, or short ID. Required.
+ * @param {Function} callback `function (err, instances, res)`
+ */
+TritonApi.prototype.listFirewallRuleInstances =
+function listFirewallRuleInstances(opts, cb) {
+    assert.string(opts.id, 'opts.id');
+    assert.func(cb, 'cb');
+
+    var self = this;
+    var res;
+    var instances;
+
+    vasync.pipeline({arg: {client: self, id: opts.id}, funcs: [
+        _stepFwRuleId,
+
+        function listMachines(arg, next) {
+            self.cloudapi.listFirewallRuleMachines({
+                id: arg.fwruleId
+            }, function (err, machines, _res) {
+                res = _res;
+                instances = machines;
+                next(err);
+            });
+        }
+    ]}, function (err) {
+        cb(err, instances, res);
+    });
+};
+
+
+/**
+ * Update a firewall rule.
+ *
+ * @param {Object} opts
+ *      - {String} id: The fwrule ID, name, or short ID. Required.
+ *      - {String} rule: The fwrule text. Required.
+ *      - {Boolean} enabled: Default to false. Optional.
+ *      - {String} description: Description of the rule. Optional.
+ * @param {Function} callback `function (err, instances, res)`
+ */
+TritonApi.prototype.updateFirewallRule = function updateFirewallRule(opts, cb) {
+    assert.string(opts.id, 'opts.id');
+    assert.string(opts.rule, 'opts.rule');
+    assert.optionalBool(opts.enabled, 'opts.enabled');
+    assert.optionalString(opts.description, 'opts.description');
+    assert.func(cb, 'cb');
+
+    var self = this;
+    var res;
+    var fwrule;
+
+    vasync.pipeline({arg: {client: self, id: opts.id}, funcs: [
+        _stepFwRuleId,
+
+        function updateRule(arg, next) {
+            opts.id = arg.fwruleId;
+            self.cloudapi.updateFirewallRule(opts, function (err, rule, _res) {
+                res = _res;
+                fwrule = rule;
+                next(err);
+            });
+        }
+    ]}, function (err) {
+        cb(err, fwrule, res);
+    });
+};
+
+
+/**
+ * Enable a firewall rule.
+ *
+ * @param {Object} opts
+ *      - {String} id: The fwrule ID, name, or short ID. Required.
+ * @param {Function} callback `function (err, fwrule, res)`
+ */
+TritonApi.prototype.enableFirewallRule = function enableFirewallRule(opts, cb) {
+    assert.string(opts.id, 'opts.id');
+    assert.func(cb, 'cb');
+
+    var self = this;
+    var res;
+    var fwrule;
+
+    vasync.pipeline({arg: {client: self, id: opts.id}, funcs: [
+        _stepFwRuleId,
+
+        function enableRule(arg, next) {
+            self.cloudapi.enableFirewallRule({
+                id: arg.fwruleId
+            }, function (err, rule, _res) {
+                res = _res;
+                fwrule = rule;
+                next(err);
+            });
+        }
+    ]}, function (err) {
+        cb(err, fwrule, res);
+    });
+};
+
+
+/**
+ * Disable a firewall rule.
+ *
+ * @param {Object} opts
+ *      - {String} id: The fwrule ID, name, or short ID. Required.
+ * @param {Function} callback `function (err, fwrule, res)`
+ */
+TritonApi.prototype.disableFirewallRule =
+function disableFirewallRule(opts, cb) {
+    assert.string(opts.id, 'opts.id');
+    assert.func(cb, 'cb');
+
+    var self = this;
+    var res;
+    var fwrule;
+
+    vasync.pipeline({arg: {client: self, id: opts.id}, funcs: [
+        _stepFwRuleId,
+
+        function disableRule(arg, next) {
+            self.cloudapi.disableFirewallRule({
+                id: arg.fwruleId
+            }, function (err, rule, _res) {
+                res = _res;
+                fwrule = rule;
+                next(err);
+            });
+        }
+    ]}, function (err) {
+        cb(err, fwrule, res);
+    });
+};
+
+
+/**
+ * Delete a firewall rule.
+ *
+ * @param {Object} opts
+ *      - {String} id: The fwrule ID, name, or short ID. Required.
+ * @param {Function} callback `function (err, res)`
+ *
+ */
+TritonApi.prototype.deleteFirewallRule = function deleteFirewallRule(opts, cb) {
+    assert.string(opts.id, 'opts.id');
+    assert.func(cb, 'cb');
+
+    var self = this;
+    var res;
+
+    vasync.pipeline({arg: {client: self, id: opts.id}, funcs: [
+        _stepFwRuleId,
+
+        function deleteRule(arg, next) {
+            self.cloudapi.deleteFirewallRule({
+                id: arg.fwruleId
+            }, function (err, _res) {
+                res = _res;
+                next(err);
+            });
+        }
+    ]}, function (err) {
+        cb(err, res);
+    });
+};
+
+
 // ---- RBAC
 
 /**

test/integration/cli-fwrules.test.js

@@ -13,7 +13,8 @@
  */
 
 var h = require('./helpers');
-var format = require('util').format;
+var f = require('util').format;
+var os = require('os');
 var test = require('tape');
 
 // --- Globals
@@ -23,35 +24,43 @@ var RULE = 'FROM any TO vm $id ALLOW tcp PORT 80';
 var RULE2 = 'FROM any TO vm $id BLOCK tcp port 25';
 var INST;
 var ID;
-var FAKE_INST_UUID = '89bcb9de-f174-4f20-bfa8-27d9749e6a2c';
+var INST_ALIAS = f('nodetritontest-fwrules-%s', os.hostname());
+var OPTS = {
+    skip: !h.CONFIG.allowWriteActions
+};
 
 // --- Tests
 
-test('triton fwrule', function (tt) {
+if (OPTS.skip) {
-    tt.test('setup', function (t) {
+    console.error('** skipping %s tests', __filename);
-        h.triton('insts -j', function (err, stdout, stderr) {
+    console.error('** set "allowWriteActions" in test config to enable');
-            if (h.ifErr(t, err, 'triton insts'))
+}
+
+test('triton fwrule', OPTS, function (tt) {
+    h.printConfig(tt);
+
+    tt.test('  cleanup existing inst with alias ' + INST_ALIAS, function (t) {
+        h.deleteTestInst(t, INST_ALIAS, function (err) {
+            t.ifErr(err);
+            t.end();
+        });
+    });
+
+    tt.test('  setup: triton create', function (t) {
+        h.createTestInst(t, INST_ALIAS, function onInst(err2, instId) {
+            if (h.ifErr(t, err2, 'triton instance create'))
                 return t.end();
 
-            var rows = stdout.split('\n');
+            INST = instId;
-            try {
+            RULE = RULE.replace('$id', INST);
-                INST = JSON.parse(rows[0]).id;
+            RULE2 = RULE2.replace('$id', INST);
-                RULE = RULE.replace('$id', INST);
-                RULE2 = RULE2.replace('$id', INST);
-            } catch (e) {
-                // if we don't have a VM already running to test with, we'll
-                // run most tests with a fake UUID, and skip any tests that
-                // require an actual machine UUID
-                RULE = RULE.replace('$id', FAKE_INST_UUID);
-                RULE2 = RULE2.replace('$id', FAKE_INST_UUID);
-            }
 
             t.end();
         });
     });
 
-    tt.test(' triton fwrule create', function (t) {
+    tt.test('  triton fwrule create', function (t) {
-        var cmd = format('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'))
@@ -60,14 +69,15 @@ test('triton fwrule', function (tt) {
             var match = stdout.match('Created firewall rule (.+)');
             t.ok(match, 'fwrule made');
 
-            ID = match[1];
+            var id = match[1];
-            t.ok(ID);
+            t.ok(id);
+            ID = id.match(/^(.+?)-/)[1]; // convert to short ID
 
             t.end();
         });
     });
 
-    tt.test(' triton fwrule get', function (t) {
+    tt.test('  triton fwrule get', function (t) {
         var cmd = 'fwrule get ' + ID;
 
         h.triton(cmd, function (err, stdout, stderr) {
@@ -83,7 +93,7 @@ test('triton fwrule', function (tt) {
         });
     });
 
-    tt.test(' triton fwrule enable', function (t) {
+    tt.test('  triton fwrule enable', function (t) {
         var cmd = 'fwrule enable ' + ID;
 
         h.triton(cmd, function (err, stdout, stderr) {
@@ -96,7 +106,7 @@ test('triton fwrule', function (tt) {
         });
     });
 
-    tt.test(' triton fwrule disable', function (t) {
+    tt.test('  triton fwrule disable', function (t) {
         var cmd = 'fwrule disable ' + ID;
 
         h.triton(cmd, function (err, stdout, stderr) {
@@ -109,7 +119,7 @@ test('triton fwrule', function (tt) {
         });
     });
 
-    tt.test(' triton fwrule update', function (t) {
+    tt.test('  triton fwrule update', function (t) {
         var cmd = 'fwrule update rule="' + RULE2 + '" ' + ID;
 
         h.triton(cmd, function (err, stdout, stderr) {
@@ -123,7 +133,7 @@ test('triton fwrule', function (tt) {
         });
     });
 
-    tt.test(' triton fwrule list', function (t) {
+    tt.test('  triton fwrule list', function (t) {
         h.triton('fwrule list -l', function (err, stdout, stderr) {
             if (h.ifErr(t, err, 'triton fwrule list'))
                 return t.end();
@@ -144,7 +154,7 @@ test('triton fwrule', function (tt) {
         });
     });
 
-    tt.test(' triton fwrule instances', function (t) {
+    tt.test('  triton fwrule instances', function (t) {
         h.triton('fwrule instances -l ' + ID, function (err, stdout, stderr) {
             if (h.ifErr(t, err, 'triton fwrule instances'))
                 return t.end();
@@ -172,8 +182,8 @@ test('triton fwrule', function (tt) {
         });
     });
 
-    tt.test(' triton instance fwrules', function (t) {
+    tt.test('  triton instance fwrules', function (t) {
-        h.triton('instance fwrules -l ' + ID, function (err, stdout, stderr) {
+        h.triton('instance fwrules -l ' + INST, function (err, stdout, stderr) {
             if (h.ifErr(t, err, 'triton fwrule list'))
                 return t.end();
 
@@ -193,7 +203,7 @@ test('triton fwrule', function (tt) {
         });
     });
 
-    tt.test(' triton fwrule delete', function (t) {
+    tt.test('  triton fwrule delete', function (t) {
         var cmd = 'fwrule delete ' + ID + ' --force';
         h.triton(cmd, function (err, stdout, stderr) {
             if (h.ifErr(t, err, 'triton fwrule delete'))
@@ -204,4 +214,15 @@ test('triton fwrule', function (tt) {
             t.end();
         });
     });
+
+    /*
+     * Use a timeout, because '-w' on delete doesn't have a way to know if the
+     * attempt failed or if it is just taking a really long time.
+     */
+    tt.test('  cleanup: triton rm INST', {timeout: 10 * 60 * 1000},
+            function (t) {
+        h.deleteTestInst(t, INST_ALIAS, function () {
+            t.end();
+        });
+    });
 });

test/integration/cli-instance-tag.test.js

@@ -39,32 +39,14 @@ if (opts.skip) {
     console.error('** set "allowWriteActions" in test config to enable');
 }
 test('triton inst tag ...', opts, function (tt) {
-    tt.comment('Test config:');
+    h.printConfig(tt);
-    Object.keys(h.CONFIG).forEach(function (key) {
-        var value = h.CONFIG[key];
-        tt.comment(f('- %s: %j', key, value));
-    });
 
     var inst;
 
     tt.test('  cleanup: rm inst ' + INST_ALIAS + ' if exists', function (t) {
-        h.triton(['inst', 'get', '-j', INST_ALIAS],
+        h.deleteTestInst(t, INST_ALIAS, function (err) {
-                function (err, stdout, stderr) {
+            t.ifErr(err);
-            if (err) {
+            t.end();
-                if (err.code === 3) {  // `triton` code for ResourceNotFound
-                    t.ok(true, 'no pre-existing alias in the way');
-                    t.end();
-                } else {
-                    t.ifErr(err);
-                    t.end();
-                }
-            } else {
-                var oldInst = JSON.parse(stdout);
-                h.safeTriton(t, ['delete', '-w', oldInst.id], function (dErr) {
-                    t.ifError(dErr, 'deleted old inst ' + oldInst.id);
-                    t.end();
-                });
-            }
         });
     });
 

test/integration/cli-manage-workflow.test.js

@@ -40,30 +40,12 @@ if (opts.skip) {
     console.error('** set "allowWriteActions" in test config to enable');
 }
 test('triton manage workflow', opts, function (tt) {
-    tt.comment('Test config:');
+    h.printConfig(tt);
-    Object.keys(h.CONFIG).forEach(function (key) {
-        var value = h.CONFIG[key];
-        tt.comment(f('- %s: %j', key, value));
-    });
 
     tt.test('  cleanup existing inst with alias ' + INST_ALIAS, function (t) {
-        h.triton(['inst', 'get', '-j', INST_ALIAS],
+        h.deleteTestInst(t, INST_ALIAS, function (err) {
-                function (err, stdout, stderr) {
+            t.ifErr(err);
-            if (err) {
+            t.end();
-                if (err.code === 3) {  // `triton` code for ResourceNotFound
-                    t.ok(true, 'no pre-existing alias in the way');
-                    t.end();
-                } else {
-                    t.ifErr(err, err);
-                    t.end();
-                }
-            } else {
-                var inst = JSON.parse(stdout);
-                h.safeTriton(t, ['inst', 'rm', '-w', inst.id], function () {
-                    t.ok(true, 'deleted inst ' + inst.id);
-                    t.end();
-                });
-            }
         });
     });
 

test/integration/cli-snapshots.test.js

@@ -9,50 +9,56 @@
  */
 
 /*
- * Integration tests for `triton snapshot ...`
+ * Integration tests for `triton instance snapshot ...`
  */
 
 var h = require('./helpers');
+var f = require('util').format;
+var os = require('os');
 var test = require('tape');
 
 // --- Globals
 
 var SNAP_NAME = 'test-snapshot';
+var INST_ALIAS = f('nodetritontest-fwrules-%s', os.hostname());
 var INST;
-var DESTROY_INST = false;
+var OPTS = {
+    skip: !h.CONFIG.allowWriteActions
+};
 
 // --- Tests
 
-test('triton snapshot', function (tt) {
+if (OPTS.skip) {
-    tt.test('setup', function (t) {
+    console.error('** skipping %s tests', __filename);
-        h.triton('insts -j', function (err, stdout, stderr) {
+    console.error('** set "allowWriteActions" in test config to enable');
-            if (h.ifErr(t, err, 'triton insts'))
+}
-                return t.end();
 
-            var rows = stdout.split('\n');
+test('triton instance snapshot', OPTS, function (tt) {
+    h.printConfig(tt);
 
-            try {
+    tt.test('  cleanup existing inst with alias ' + INST_ALIAS, function (t) {
-                INST = JSON.parse(rows[0]).id;
+        h.deleteTestInst(t, INST_ALIAS, function (err) {
-                return t.end();
+            t.ifErr(err);
-            } catch (e) {
+            t.end();
-                h.createMachine(function onCreate(err2, instId) {
-                    if (h.ifErr(t, err2, 'triton instance create'))
-                        return t.end();
-
-                    INST = instId;
-                    DESTROY_INST = true;
-
-                    t.end();
-                });
-            }
         });
     });
 
-    tt.test(' triton snapshot create', function (t) {
+    tt.test('  setup: triton instance create', function (t) {
-        var cmd = 'snapshot create -w -n ' + SNAP_NAME + ' ' + INST;
+        h.createTestInst(t, INST_ALIAS, function onInst(err2, instId) {
+            if (h.ifErr(t, err2, 'triton instance create'))
+                return t.end();
+
+            INST = instId.match(/^(.+?)-/)[1]; // convert to short ID
+
+            t.end();
+        });
+    });
+
+    tt.test('  triton instance snapshot create', function (t) {
+        var cmd = 'instance snapshot create -w -n ' + SNAP_NAME + ' ' + INST;
 
         h.triton(cmd, function (err, stdout, stderr) {
-            if (h.ifErr(t, err, 'triton snapshot create'))
+            if (h.ifErr(t, err, 'triton instance snapshot create'))
                 return t.end();
 
             t.ok(stdout.match('Created snapshot "' + SNAP_NAME + '" in \\d+'),
@@ -62,11 +68,11 @@ test('triton snapshot', function (tt) {
         });
     });
 
-    tt.test(' triton snapshot get', function (t) {
+    tt.test('  triton instance snapshot get', function (t) {
-        var cmd = 'snapshot get ' + INST + ' ' + SNAP_NAME;
+        var cmd = 'instance snapshot get ' + INST + ' ' + SNAP_NAME;
 
         h.triton(cmd, function (err, stdout, stderr) {
-            if (h.ifErr(t, err, 'triton snapshot get'))
+            if (h.ifErr(t, err, 'triton instance snapshot get'))
                 return t.end();
 
             var obj = JSON.parse(stdout);
@@ -77,13 +83,15 @@ test('triton snapshot', function (tt) {
         });
     });
 
-    tt.test(' triton snapshot list', function (t) {
+    tt.test('  triton instance snapshot list', function (t) {
-        h.triton('snapshot list ' + INST, function (err, stdout, stderr) {
+        var cmd = 'instance snapshot list ' + INST;
-            if (h.ifErr(t, err, 'triton snapshot list'))
+
+        h.triton(cmd, function (err, stdout, stderr) {
+            if (h.ifErr(t, err, 'triton instance snapshot list'))
                 return t.end();
 
             var snaps = stdout.split('\n');
-            t.ok(snaps[0].match(/NAME\s+STATE/));
+            t.ok(snaps[0].match(/NAME\s+STATE\s+CREATED/));
             snaps.shift();
 
             t.ok(snaps.length >= 1, 'triton snap list expected snap num');
@@ -98,7 +106,7 @@ test('triton snapshot', function (tt) {
         });
     });
 
-    tt.test(' triton instance start --snapshot', function (t) {
+    tt.test('  triton instance start --snapshot', function (t) {
         var cmd = 'instance start ' + INST + ' -w --snapshot=' + SNAP_NAME;
 
         h.triton(cmd, function (err, stdout, stderr) {
@@ -111,10 +119,12 @@ test('triton snapshot', function (tt) {
         });
     });
 
-    tt.test(' triton snapshot delete', function (t) {
+    tt.test('  triton instance snapshot delete', function (t) {
-        var cmd = 'snapshot delete ' + INST + ' ' + SNAP_NAME + ' -w --force';
+        var cmd = 'instance snapshot delete  -w --force ' + INST + ' ' +
+                  SNAP_NAME;
+
         h.triton(cmd, function (err, stdout, stderr) {
-            if (h.ifErr(t, err, 'triton snapshot delete'))
+            if (h.ifErr(t, err, 'triton instance snapshot delete'))
                 return t.end();
 
             t.ok(stdout.match('Deleting snapshot "' + SNAP_NAME + '"',
@@ -126,11 +136,13 @@ test('triton snapshot', function (tt) {
         });
     });
 
-    tt.test('teardown', function (t) {
+    /*
-        if (!DESTROY_INST)
+     * Use a timeout, because '-w' on delete doesn't have a way to know if the
-            return t.end();
+     * attempt failed or if it is just taking a really long time.
-
+     */
-        h.triton('instance delete ' + INST, function () {
+    tt.test('  cleanup: triton instance rm INST', {timeout: 10 * 60 * 1000},
+            function (t) {
+        h.deleteTestInst(t, INST_ALIAS, function () {
             t.end();
         });
     });

test/integration/cli-subcommands.test.js

@@ -44,6 +44,11 @@ var subs = [
     ['instance wait'],
     ['instance audit'],
     ['instance fwrules'],
+    ['instance snapshot'],
+    ['instance snapshot create'],
+    ['instance snapshot list', 'instance snapshot ls', 'instance snapshots'],
+    ['instance snapshot get'],
+    ['instance snapshot delete', 'instance snapshot rm'],
     ['ssh'],
     ['network'],
     ['network list', 'networks'],
@@ -59,11 +64,6 @@ var subs = [
     ['package', 'pkg'],
     ['package get'],
     ['package list', 'packages', 'pkgs'],
-    ['snapshot'],
-    ['snapshot create'],
-    ['snapshot list', 'snapshot ls'],
-    ['snapshot get'],
-    ['snapshot delete', 'snapshot rm'],
     ['fwrule'],
     ['fwrule create'],
     ['fwrule list', 'fwrule ls'],

test/integration/helpers.js

@@ -258,46 +258,22 @@ function createClient() {
 
 
 /*
- * Create a small instance.
+ * Create a small test instance.
  */
-function createMachine(cb) {
+function createTestInst(t, name, cb) {
-    function jsonToObjs(jsons) {
+    getTestPkg(t, function (err, pkgId) {
-        return jsons.split('\n').map(function (json) {
+        t.ifErr(err);
-            try {
-                return JSON.parse(json);
-            } catch (e) {}
-        }).filter(function (obj) {
-            return obj;
-        });
-    }
 
-    triton('package list -j', function (err, pkgJson) {
+        getTestImg(t, function (err2, imgId) {
-        if (err)
+            t.ifErr(err2);
-            return cb(err);
 
-        // pick the smallest package (ram-wise)
+            var cmd = f('instance create -w -n %s %s %s', name, imgId, pkgId);
-        var pkgs = jsonToObjs(pkgJson);
+            triton(cmd, function (err3, stdout) {
-        var pkg = pkgs.sort(function (x, y) {
+                t.ifErr(err3, 'create test instance');
-            return (x.memory > y.memory) ? 1 : -1;
-        })[0];
-
-        triton('image list -j', function (err2, imgJson) {
-            if (err2)
-                return cb(err2);
-
-            // pick any smartos image
-            var imgs = jsonToObjs(imgJson);
-            var img = imgs.filter(function (i) {
-                return i.os === 'smartos';
-            })[0];
-
-            triton('instance create -w ' + img.id + ' ' + pkg.id,
-                   function (err3, stdout) {
-                if (err3)
-                    return cb(err3);
 
                 var match = stdout.match(/Created .+? \((.+)\)/);
                 var inst = match[1];
+
                 cb(null, inst);
             });
         });
@@ -305,6 +281,44 @@ function createMachine(cb) {
 }
 
 
+/*
+ * Remove test instance, if exists.
+ */
+function deleteTestInst(t, name, cb) {
+    triton(['inst', 'get', '-j', name], function (err, stdout, stderr) {
+        if (err) {
+            if (err.code === 3) {  // `triton` code for ResourceNotFound
+                t.ok(true, 'no pre-existing alias in the way');
+            } else {
+                t.ifErr(err);
+            }
+
+            return cb();
+        }
+
+        var oldInst = JSON.parse(stdout);
+
+        safeTriton(t, ['delete', '-w', oldInst.id], function (dErr) {
+            t.ifError(dErr, 'deleted old inst ' + oldInst.id);
+            cb();
+        });
+    });
+}
+
+
+/*
+ * Print out a listing of the test config.json values.
+ */
+function printConfig(t) {
+    t.comment('Test config:');
+
+    Object.keys(CONFIG).forEach(function (key) {
+        var value = CONFIG[key];
+        t.comment(f('- %s: %j', key, value));
+    });
+}
+
+
 // --- exports
 
 module.exports = {
@@ -312,10 +326,12 @@ module.exports = {
     triton: triton,
     safeTriton: safeTriton,
     createClient: createClient,
-    createMachine: createMachine,
+    createTestInst: createTestInst,
+    deleteTestInst: deleteTestInst,
     getTestImg: getTestImg,
     getTestPkg: getTestPkg,
     jsonStreamParse: jsonStreamParse,
+    printConfig: printConfig,
 
     ifErr: testcommon.ifErr
 };