Fix incorrectly implemented show commands

CHANGELOG.md

@@ -8,6 +8,9 @@ Backwards incompatible changes:
 * Update `Gemfile.lock` to use Bundler 2.4.22
 * When ENV `HOME` is not set, the working directory is used for configs
   rather than `/.brightbox` to fix issues with containers
+* `show` commands require at least one argument to
+   prevent an issue where the wrong, summary API was
+   used resulting in missing data in tables
 
 Enhancements:
 

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    brightbox-cli (5.0.0.rc1)
+    brightbox-cli (5.0.0.rc2)
       fog-brightbox (>= 1.12.0)
       fog-core (< 2.0)
       gli (~> 2.21)

lib/brightbox-cli/commands/images/show.rb

@@ -4,7 +4,11 @@ module Brightbox
     cmd.arg_name "image-id..."
     cmd.command [:show] do |c|
       c.action do |global_options, _options, args|
-        images = Image.find_all_or_warn(args)
+        raise "You must specify image IDs to show" if args.empty?
+
+        images = Image.find_or_call(args) do |id|
+          raise "Couldn't find an image with ID #{id}"
+        end
 
         display_options = {
           :vertical => true,

lib/brightbox-cli/commands/lbs/show.rb

@@ -4,7 +4,11 @@ module Brightbox
     cmd.arg_name "lbs-id..."
     cmd.command [:show] do |c|
       c.action do |global_options, _options, args|
-        lbs = LoadBalancer.find_all_or_warn(args)
+        raise "You must specify load balancer IDs to show" if args.empty?
+
+        lbs = LoadBalancer.find_or_call(args) do |id|
+          raise "Couldn't find a load balancer with ID #{id}"
+        end
 
         table_opts = global_options.merge(
           :vertical => true,

lib/brightbox-cli/commands/servers/show.rb

@@ -4,7 +4,11 @@ module Brightbox
     cmd.arg_name "server-id..."
     cmd.command [:show] do |c|
       c.action do |global_options, _options, args|
-        servers = DetailedServer.find_all_or_warn(args)
+        raise "You must specify server IDs to show" if args.empty?
+
+        servers = DetailedServer.find_or_call(args) do |id|
+          raise "Couldn't find a server with ID #{id}"
+        end
 
         table_opts = global_options.merge(:vertical => true)
         render_table(servers, table_opts)

lib/brightbox-cli/commands/users/show.rb

@@ -4,7 +4,11 @@ module Brightbox
     cmd.arg_name "user-id..."
     cmd.command [:show] do |c|
       c.action do |global_options, _options, args|
-        users = User.find_all_or_warn(args)
+        raise "You must specify user IDs to show" if args.empty?
+
+        users = User.find_or_call(args) do |id|
+          raise "Couldn't find a user with ID #{id}"
+        end
 
         table_opts = global_options.merge(
           :vertical => true,

lib/brightbox-cli/commands/volumes/show.rb

@@ -5,7 +5,11 @@ module Brightbox
 
     cmd.command [:show] do |c|
       c.action do |global_options, _options, args|
-        volumes = Volume.find_all_or_warn(args)
+        raise "You must specify volume IDs to show" if args.empty?
+
+        volumes = Volume.find_or_call(args) do |id|
+          raise "Couldn't find a volume with ID #{id}"
+        end
 
         table_opts = global_options.merge(
           :vertical => true,

lib/brightbox-cli/version.rb

@@ -1,3 +1,3 @@
 module Brightbox
-  VERSION = "5.0.0.rc1".freeze unless defined?(Brightbox::VERSION)
+  VERSION = "5.0.0.rc2".freeze unless defined?(Brightbox::VERSION)
 end

spec/commands/images/show_spec.rb

@@ -17,35 +17,51 @@
     context "without arguments" do
       let(:argv) { %w[images show] }
 
+      it "reports missing IDs" do
+        expect { output }.to_not raise_error
+
+        aggregate_failures do
+          expect(stderr).to match("ERROR: You must specify image IDs to show")
+          expect(stdout).to eq("")
+        end
+      end
+    end
+
+    context "with identifier argument" do
+      let(:argv) { %w[images show img-11111] }
+
       before do
-        stub_request(:get, "#{api_url}/1.0/images?account_id=acc-12345")
+        expect(Brightbox::Image).to receive(:find)
+          .with("img-11111")
+          .and_call_original
+
+        stub_request(:get, "#{api_url}/1.0/images/img-11111?account_id=acc-12345")
           .to_return(
             status: 200,
-            body: [
-              {
-                id: "img-12345"
-              }
-            ].to_json
+            body: {
+              id: "img-11111",
+              min_ram: 2_048
+            }.to_json
           )
       end
 
       it "does not error" do
         expect { output }.to_not raise_error
 
-        expect(stderr).to match("")
-        expect(stderr).not_to match("ERROR")
-        expect(stdout).to match("img-12345")
+        aggregate_failures do
+          expect(stderr).to match("")
+          expect(stderr).not_to match("ERROR")
+          expect(stdout).to match("img-11111")
+
+          expect(stdout).to match("min_ram: 2048")
+        end
       end
     end
 
-    context "with identifier" do
-      let(:argv) { %w[images show img-11111] }
+    context "with multiple identifiers" do
+      let(:argv) { %w[images show img-11111 img-22222] }
 
       before do
-        expect(Brightbox::Image).to receive(:find)
-          .with("img-11111")
-          .and_call_original
-
         stub_request(:get, "#{api_url}/1.0/images/img-11111?account_id=acc-12345")
           .to_return(
             status: 200,
@@ -54,16 +70,30 @@
               min_ram: 2_048
             }.to_json
           )
+
+        stub_request(:get, "#{api_url}/1.0/images/img-22222?account_id=acc-12345")
+          .to_return(
+            status: 200,
+            body: {
+              id: "img-22222",
+              min_ram: 4_096
+            }.to_json
+          )
       end
 
       it "does not error" do
         expect { output }.to_not raise_error
 
-        expect(stderr).to match("")
-        expect(stderr).not_to match("ERROR")
-        expect(stdout).to match("img-11111")
+        aggregate_failures do
+          expect(stderr).to match("")
+          expect(stderr).not_to match("ERROR")
+
+          expect(stdout).to match("img-11111")
+          expect(stdout).to match("min_ram: 2048")
 
-        expect(stdout).to match("min_ram: 2048")
+          expect(stdout).to match("img-22222")
+          expect(stdout).to match("min_ram: 4096")
+        end
       end
     end
   end

spec/commands/lbs/show_spec.rb

@@ -11,29 +11,60 @@
       cache_access_token(config, "f83da712e6299cda953513ec07f7a754f747d727")
     end
 
-    context "without identifier argument" do
+    context "without arguments" do
       let(:argv) { %w[lbs show] }
+
+      it "reports missing IDs" do
+        expect { output }.to_not raise_error
+
+        aggregate_failures do
+          expect(stderr).to include("ERROR: You must specify load balancer IDs to show")
+          expect(stdout).to be_empty
+        end
+      end
+    end
+
+    context "with identifier argument" do
+      let(:argv) { %w[lbs show lba-12345] }
       let(:json_response) do
         <<~EOS
-        [
-          {
-            "id":"lba-12345",
-            "name":"app-lb1",
-            "status":"active",
-            "created_at":"2012-03-05T12:00:00Z",
-            "nodes":[
+        {
+          "id":"lba-12345",
+          "name":"app-lb1",
+          "status":"active",
+          "created_at":"2012-03-05T12:00:00Z",
+          "acme": {
+            "domains": [
               {
-                "id":"srv-12345",
-                "status":"active"
+                "identifier": "domain.test",
+                "status": "verified"
+              },
+              {
+                "identifier": "domain2.test",
+                "status": "verified"
               }
-            ]
-          }
-        ]
+            ],
+            "certificate": {
+              "domains": [
+                "domain.test"
+              ],
+              "expires_at": "2025-12-31T23:59:59Z",
+              "fingerprint": "fingerprint",
+              "issued_at": "2025-01-01T00:00:00Z"
+            }
+          },
+          "nodes":[
+            {
+              "id":"srv-12345",
+              "status":"active"
+            }
+          ]
+        }
         EOS
       end
 
       before do
-        stub_request(:get, "http://api.brightbox.localhost/1.0/load_balancers?account_id=acc-12345")
+        stub_request(:get, "http://api.brightbox.localhost/1.0/load_balancers/lba-12345?account_id=acc-12345")
           .to_return(:status => 200, :body => json_response)
       end
 
@@ -45,13 +76,18 @@
           expect(stdout).to include("name: app-lb1")
           expect(stdout).to include("created_at: 2012-03-05T12:00Z")
           expect(stdout).to include("nodes: srv-12345")
+          expect(stdout).to include("acme_domains: domain.test:verified,domain2.test:verified")
+          expect(stdout).to include("acme_cert_subjects: domain.test")
+          expect(stdout).to include("acme_cert_expires_at: 2025-12-31T23:59:59Z")
+          expect(stdout).to include("acme_cert_fingerprint: fingerprint")
+          expect(stdout).to include("acme_cert_issued_at: 2025-01-01T00:00:00Z")
         end
       end
     end
 
-    context "with identifier argument" do
-      let(:argv) { %w[lbs show lba-12345] }
-      let(:json_response) do
+    context "with multiple identifiers" do
+      let(:argv) { %w[lbs show lba-12345 lba-54321] }
+      let(:json_response_1) do
         <<~EOS
         {
           "id":"lba-12345",
@@ -87,13 +123,52 @@
         }
         EOS
       end
+      let(:json_response_2) do
+        <<~EOS
+        {
+          "id":"lba-54321",
+          "name":"app-lb2",
+          "status":"active",
+          "created_at":"2012-03-05T12:00:00Z",
+          "acme": {
+            "domains": [
+              {
+                "identifier": "domain.test",
+                "status": "verified"
+              },
+              {
+                "identifier": "domain2.test",
+                "status": "verified"
+              }
+            ],
+            "certificate": {
+              "domains": [
+                "domain.test"
+              ],
+              "expires_at": "2025-12-31T23:59:59Z",
+              "fingerprint": "fingerprint",
+              "issued_at": "2025-01-01T00:00:00Z"
+            }
+          },
+          "nodes":[
+            {
+              "id":"srv-54321",
+              "status":"active"
+            }
+          ]
+        }
+        EOS
+      end
 
       before do
         stub_request(:get, "http://api.brightbox.localhost/1.0/load_balancers/lba-12345?account_id=acc-12345")
-          .to_return(:status => 200, :body => json_response)
+          .to_return(:status => 200, :body => json_response_1)
+
+        stub_request(:get, "http://api.brightbox.localhost/1.0/load_balancers/lba-54321?account_id=acc-12345")
+          .to_return(:status => 200, :body => json_response_2)
       end
 
-      it "shows load balancer details" do
+      it "shows multiple load balancer details" do
         aggregate_failures do
           expect(stderr).to be_empty unless ENV["DEBUG"]
           expect(stdout).to include("id: lba-12345")
@@ -106,6 +181,17 @@
           expect(stdout).to include("acme_cert_expires_at: 2025-12-31T23:59:59Z")
           expect(stdout).to include("acme_cert_fingerprint: fingerprint")
           expect(stdout).to include("acme_cert_issued_at: 2025-01-01T00:00:00Z")
+
+          expect(stdout).to include("id: lba-54321")
+          expect(stdout).to include("status: active")
+          expect(stdout).to include("name: app-lb2")
+          expect(stdout).to include("created_at: 2012-03-05T12:00Z")
+          expect(stdout).to include("nodes: srv-54321")
+          expect(stdout).to include("acme_domains: domain.test:verified,domain2.test:verified")
+          expect(stdout).to include("acme_cert_subjects: domain.test")
+          expect(stdout).to include("acme_cert_expires_at: 2025-12-31T23:59:59Z")
+          expect(stdout).to include("acme_cert_fingerprint: fingerprint")
+          expect(stdout).to include("acme_cert_issued_at: 2025-01-01T00:00:00Z")
         end
       end
     end