Merge pull request #4333 from chef/vault

no longer wait on node search to refresh vault but pass created ApiCient instead

8 files changed, 82 insertions, 53 deletions

diff --git a/lib/chef/api_client/registration.rb b/lib/chef/api_client/registration.rb
index 7875afde0f..bc941d5bfa 100644
--- a/lib/chef/api_client/registration.rb
+++ b/lib/chef/api_client/registration.rb
@@ -53,8 +53,9 @@ class Chef
       def run
         assert_destination_writable!
         retries = Config[:client_registration_retries] || 5
+        client = nil
         begin
-          create_or_update
+          client = api_client(create_or_update)
         rescue Net::HTTPFatalError => e
           # HTTPFatalError implies 5xx.
           raise if retries <= 0
@@ -64,6 +65,7 @@ class Chef
           retry
         end
         write_key
+        client
       end
 
       def assert_destination_writable!
@@ -106,6 +108,28 @@ class Chef
         response
       end
 
+      def api_client(response)
+        return response if response.is_a?(Chef::ApiClient)
+
+        client = Chef::ApiClient.new
+        client.name(name)
+        client.public_key(api_client_key(response, "public_key"))
+        client.private_key(api_client_key(response, "private_key"))
+        client
+      end
+
+      def api_client_key(response, key_name)
+        if response[key_name]
+          if response[key_name].respond_to?(:to_pem)
+            response[key_name].to_pem
+          else
+            response[key_name]
+          end
+        elsif response["chef_key"]
+          response["chef_key"][key_name]
+        end
+      end
+
       def put_data
         base_put_data = { :name => name, :admin => false }
         if self_generate_keys?
diff --git a/lib/chef/knife/bootstrap.rb b/lib/chef/knife/bootstrap.rb
index 30ea3cac6c..d958ddf336 100644
--- a/lib/chef/knife/bootstrap.rb
+++ b/lib/chef/knife/bootstrap.rb
@@ -377,7 +377,7 @@ class Chef
 
           client_builder.run
 
-          chef_vault_handler.run(node_name: config[:chef_node_name])
+          chef_vault_handler.run(client_builder.client)
 
           bootstrap_context.client_pem = client_builder.client_path
         else
diff --git a/lib/chef/knife/bootstrap/chef_vault_handler.rb b/lib/chef/knife/bootstrap/chef_vault_handler.rb
index f658957499..9d0dfec621 100644
--- a/lib/chef/knife/bootstrap/chef_vault_handler.rb
+++ b/lib/chef/knife/bootstrap/chef_vault_handler.rb
@@ -28,8 +28,8 @@ class Chef
         # @return [Chef::Knife::UI] ui object for output
         attr_accessor :ui
 
-        # @return [String] name of the node (technically name of the client)
-        attr_reader :node_name
+        # @return [Chef::ApiClient] vault client
+        attr_reader :client
 
         # @param knife_config [Hash] knife merged config, typically @config
         # @param ui [Chef::Knife::UI] ui object for output
@@ -38,18 +38,15 @@ class Chef
           @ui           = ui
         end
 
-        # Updates the chef vault items for the newly created node.
+        # Updates the chef vault items for the newly created client.
         #
-        # @param node_name [String] name of the node (technically name of the client)
-        # @todo: node_name should be mandatory (ruby 2.0 compat)
-        def run(node_name: nil)
+        # @param client [Chef::ApiClient] vault client
+        def run(client)
           return unless doing_chef_vault?
 
           sanity_check
 
-          @node_name = node_name
-
-          ui.info("Updating Chef Vault, waiting for client to be searchable..") while wait_for_client
+          @client = client
 
           update_bootstrap_vault_json!
         end
@@ -126,7 +123,7 @@ class Chef
         def update_vault(vault, item)
           require_chef_vault!
           bootstrap_vault_item = load_chef_bootstrap_vault_item(vault, item)
-          bootstrap_vault_item.clients("name:#{node_name}")
+          bootstrap_vault_item.clients(client)
           bootstrap_vault_item.save
         end
 
@@ -141,22 +138,18 @@ class Chef
 
         public :load_chef_bootstrap_vault_item  # for stubbing
 
-        # Helper used to spin waiting for the client to appear in search.
-        #
-        # @return [Boolean] true if the client is searchable
-        def wait_for_client
-          sleep 1
-          !Chef::Search::Query.new.search(:client, "name:#{node_name}")[0]
-        end
-
         # Helper to very lazily require the chef-vault gem
         def require_chef_vault!
           @require_chef_vault ||=
             begin
+              error_message = "Knife bootstrap needs version 2.6.0 or higher of the chef-vault gem to configure chef vault items"
               require 'chef-vault'
+              if Gem::Version.new(ChefVault::VERSION) < Gem::Version.new('2.6.0')
+                raise error_message
+              end
               true
             rescue LoadError
-              raise "Knife bootstrap cannot configure chef vault items when the chef-vault gem is not installed"
+              raise error_message
             end
         end
 
diff --git a/lib/chef/knife/bootstrap/client_builder.rb b/lib/chef/knife/bootstrap/client_builder.rb
index 7eb1e22628..6414ac5c72 100644
--- a/lib/chef/knife/bootstrap/client_builder.rb
+++ b/lib/chef/knife/bootstrap/client_builder.rb
@@ -34,6 +34,8 @@ class Chef
         attr_accessor :chef_config
         # @return [Chef::Knife::UI] ui object for output
         attr_accessor :ui
+        # @return [Chef::ApiClient] client saved on run
+        attr_reader :client
 
         # @param knife_config [Hash] Hash of knife config settings
         # @param chef_config [Hash] Hash of chef config settings
@@ -51,7 +53,7 @@ class Chef
 
           ui.info("Creating new client for #{node_name}")
 
-          create_client!
+          @client = create_client!
 
           ui.info("Creating new node for #{node_name}")
 
diff --git a/spec/unit/api_client/registration_spec.rb b/spec/unit/api_client/registration_spec.rb
index 58d64990f3..4cf1c20eaa 100644
--- a/spec/unit/api_client/registration_spec.rb
+++ b/spec/unit/api_client/registration_spec.rb
@@ -46,8 +46,10 @@ describe Chef::ApiClient::Registration do
   end
 
   let(:server_v10_response) do
-    {"uri" => "https://chef.local/clients/#{client_name}",
-     "private_key" => "--begin rsa key etc--"}
+    {
+      "uri" => "https://chef.local/clients/#{client_name}",
+      "private_key" => "--begin rsa key etc--"
+    }
   end
 
   # Server v11 includes `json_class` on all replies
@@ -69,7 +71,9 @@ describe Chef::ApiClient::Registration do
   let(:create_with_pkey_response) do
     {
       "uri" => "",
-      "public_key" => generated_public_key.to_pem
+      "chef_key" => {
+        "public_key" => generated_public_key.to_pem
+      }
     }
   end
 
@@ -108,7 +112,7 @@ describe Chef::ApiClient::Registration do
       expect(http_mock).to receive(:post).
         with("clients", expected_post_data).
         and_return(create_with_pkey_response)
-      expect(registration.create_or_update).to eq(create_with_pkey_response)
+      expect(registration.run.public_key).to eq(create_with_pkey_response["chef_key"]["public_key"])
       expect(registration.private_key).to eq(generated_private_key_pem)
     end
 
@@ -119,7 +123,7 @@ describe Chef::ApiClient::Registration do
       expect(http_mock).to receive(:put).
         with("clients/#{client_name}", expected_put_data).
         and_return(update_with_pkey_response)
-      expect(registration.create_or_update).to eq(update_with_pkey_response)
+      expect(registration.run.public_key).to eq(update_with_pkey_response["public_key"].to_pem)
       expect(registration.private_key).to eq(generated_private_key_pem)
     end
 
@@ -137,7 +141,7 @@ describe Chef::ApiClient::Registration do
         expect(http_mock).to receive(:put).
           with("clients/#{client_name}", expected_put_data).
           and_return(update_with_pkey_response)
-        expect(registration.create_or_update).to eq(update_with_pkey_response)
+        expect(registration.run.public_key).to eq(update_with_pkey_response["public_key"].to_pem)
         expect(registration.private_key).to eq(generated_private_key_pem)
       end
     end
@@ -161,7 +165,7 @@ describe Chef::ApiClient::Registration do
         expect(http_mock).to receive(:post).
           with("clients", expected_post_data).
           and_return(server_v10_response)
-        expect(registration.create_or_update).to eq(server_v10_response)
+        expect(registration.run.private_key).to eq(server_v10_response["private_key"])
         expect(registration.private_key).to eq("--begin rsa key etc--")
       end
 
@@ -171,7 +175,7 @@ describe Chef::ApiClient::Registration do
           expect(http_mock).to receive(:put).
             with("clients/#{client_name}", expected_put_data).
             and_return(server_v11_response)
-          expect(registration.create_or_update).to eq(server_v11_response)
+          expect(registration.run).to eq(server_v11_response)
           expect(registration.private_key).to eq("--begin rsa key etc--")
         end
       end
@@ -183,7 +187,7 @@ describe Chef::ApiClient::Registration do
           expect(http_mock).to receive(:put).
             with("clients/#{client_name}", expected_put_data).
             and_return(server_v10_response)
-          expect(registration.create_or_update).to eq(server_v10_response)
+          expect(registration.run.private_key).to eq(server_v10_response["private_key"])
           expect(registration.private_key).to eq("--begin rsa key etc--")
         end
       end
diff --git a/spec/unit/knife/bootstrap/chef_vault_handler_spec.rb b/spec/unit/knife/bootstrap/chef_vault_handler_spec.rb
index d8f84265b7..75e0ff6b00 100644
--- a/spec/unit/knife/bootstrap/chef_vault_handler_spec.rb
+++ b/spec/unit/knife/bootstrap/chef_vault_handler_spec.rb
@@ -27,7 +27,7 @@ describe Chef::Knife::Bootstrap::ChefVaultHandler do
 
   let(:knife_config) { {} }
 
-  let(:node_name) { "bevell.wat" }
+  let(:client) { Chef::ApiClient.new }
 
   let(:chef_vault_handler) {
     chef_vault_handler = Chef::Knife::Bootstrap::ChefVaultHandler.new(knife_config: knife_config, ui: ui)
@@ -50,9 +50,8 @@ describe Chef::Knife::Bootstrap::ChefVaultHandler do
     let(:bootstrap_vault_item) { double("ChefVault::Item") }
 
     before do
-      expect(chef_vault_handler).to receive(:wait_for_client).and_return(false)
       expect(chef_vault_handler).to receive(:require_chef_vault!).at_least(:once)
-      expect(bootstrap_vault_item).to receive(:clients).with("name:#{node_name}").at_least(:once)
+      expect(bootstrap_vault_item).to receive(:clients).with(client).at_least(:once)
       expect(bootstrap_vault_item).to receive(:save).at_least(:once)
     end
 
@@ -60,20 +59,20 @@ describe Chef::Knife::Bootstrap::ChefVaultHandler do
       it "sets a single item as a scalar" do
         knife_config[:bootstrap_vault_item] = { 'vault' => 'item1' }
         expect(chef_vault_handler).to receive(:load_chef_bootstrap_vault_item).with('vault', 'item1').and_return(bootstrap_vault_item)
-        chef_vault_handler.run(node_name: node_name)
+        chef_vault_handler.run(client)
       end
 
       it "sets a single item as an array" do
         knife_config[:bootstrap_vault_item] = { 'vault' => [ 'item1' ] }
         expect(chef_vault_handler).to receive(:load_chef_bootstrap_vault_item).with('vault', 'item1').and_return(bootstrap_vault_item)
-        chef_vault_handler.run(node_name: node_name)
+        chef_vault_handler.run(client)
       end
 
       it "sets two items as an array" do
         knife_config[:bootstrap_vault_item] = { 'vault' => [ 'item1', 'item2' ] }
         expect(chef_vault_handler).to receive(:load_chef_bootstrap_vault_item).with('vault', 'item1').and_return(bootstrap_vault_item)
         expect(chef_vault_handler).to receive(:load_chef_bootstrap_vault_item).with('vault', 'item2').and_return(bootstrap_vault_item)
-        chef_vault_handler.run(node_name: node_name)
+        chef_vault_handler.run(client)
       end
 
       it "sets two vaults from different hash keys" do
@@ -81,7 +80,7 @@ describe Chef::Knife::Bootstrap::ChefVaultHandler do
         expect(chef_vault_handler).to receive(:load_chef_bootstrap_vault_item).with('vault', 'item1').and_return(bootstrap_vault_item)
         expect(chef_vault_handler).to receive(:load_chef_bootstrap_vault_item).with('vault', 'item2').and_return(bootstrap_vault_item)
         expect(chef_vault_handler).to receive(:load_chef_bootstrap_vault_item).with('vault2', 'item3').and_return(bootstrap_vault_item)
-        chef_vault_handler.run(node_name: node_name)
+        chef_vault_handler.run(client)
       end
     end
 
@@ -89,20 +88,20 @@ describe Chef::Knife::Bootstrap::ChefVaultHandler do
       it "sets a single item as a scalar" do
         knife_config[:bootstrap_vault_json] = '{ "vault": "item1" }'
         expect(chef_vault_handler).to receive(:load_chef_bootstrap_vault_item).with('vault', 'item1').and_return(bootstrap_vault_item)
-        chef_vault_handler.run(node_name: node_name)
+        chef_vault_handler.run(client)
       end
 
       it "sets a single item as an array" do
         knife_config[:bootstrap_vault_json] = '{ "vault": [ "item1" ] }'
         expect(chef_vault_handler).to receive(:load_chef_bootstrap_vault_item).with('vault', 'item1').and_return(bootstrap_vault_item)
-        chef_vault_handler.run(node_name: node_name)
+        chef_vault_handler.run(client)
       end
 
       it "sets two items as an array" do
         knife_config[:bootstrap_vault_json] = '{ "vault": [ "item1", "item2" ] }'
         expect(chef_vault_handler).to receive(:load_chef_bootstrap_vault_item).with('vault', 'item1').and_return(bootstrap_vault_item)
         expect(chef_vault_handler).to receive(:load_chef_bootstrap_vault_item).with('vault', 'item2').and_return(bootstrap_vault_item)
-        chef_vault_handler.run(node_name: node_name)
+        chef_vault_handler.run(client)
       end
 
       it "sets two vaults from different hash keys" do
@@ -110,7 +109,7 @@ describe Chef::Knife::Bootstrap::ChefVaultHandler do
         expect(chef_vault_handler).to receive(:load_chef_bootstrap_vault_item).with('vault', 'item1').and_return(bootstrap_vault_item)
         expect(chef_vault_handler).to receive(:load_chef_bootstrap_vault_item).with('vault', 'item2').and_return(bootstrap_vault_item)
         expect(chef_vault_handler).to receive(:load_chef_bootstrap_vault_item).with('vault2', 'item3').and_return(bootstrap_vault_item)
-        chef_vault_handler.run(node_name: node_name)
+        chef_vault_handler.run(client)
       end
     end
 
@@ -125,20 +124,20 @@ describe Chef::Knife::Bootstrap::ChefVaultHandler do
       it "sets a single item as a scalar" do
         setup_file_contents('{ "vault": "item1" }')
         expect(chef_vault_handler).to receive(:load_chef_bootstrap_vault_item).with('vault', 'item1').and_return(bootstrap_vault_item)
-        chef_vault_handler.run(node_name: node_name)
+        chef_vault_handler.run(client)
       end
 
       it "sets a single item as an array" do
         setup_file_contents('{ "vault": [ "item1" ] }')
         expect(chef_vault_handler).to receive(:load_chef_bootstrap_vault_item).with('vault', 'item1').and_return(bootstrap_vault_item)
-        chef_vault_handler.run(node_name: node_name)
+        chef_vault_handler.run(client)
       end
 
       it "sets two items as an array" do
         setup_file_contents('{ "vault": [ "item1", "item2" ] }')
         expect(chef_vault_handler).to receive(:load_chef_bootstrap_vault_item).with('vault', 'item1').and_return(bootstrap_vault_item)
         expect(chef_vault_handler).to receive(:load_chef_bootstrap_vault_item).with('vault', 'item2').and_return(bootstrap_vault_item)
-        chef_vault_handler.run(node_name: node_name)
+        chef_vault_handler.run(client)
       end
 
       it "sets two vaults from different hash keys" do
@@ -146,7 +145,7 @@ describe Chef::Knife::Bootstrap::ChefVaultHandler do
         expect(chef_vault_handler).to receive(:load_chef_bootstrap_vault_item).with('vault', 'item1').and_return(bootstrap_vault_item)
         expect(chef_vault_handler).to receive(:load_chef_bootstrap_vault_item).with('vault', 'item2').and_return(bootstrap_vault_item)
         expect(chef_vault_handler).to receive(:load_chef_bootstrap_vault_item).with('vault2', 'item3').and_return(bootstrap_vault_item)
-        chef_vault_handler.run(node_name: node_name)
+        chef_vault_handler.run(client)
       end
     end
   end
diff --git a/spec/unit/knife/bootstrap/client_builder_spec.rb b/spec/unit/knife/bootstrap/client_builder_spec.rb
index e7232fe8d6..f17a6af878 100644
--- a/spec/unit/knife/bootstrap/client_builder_spec.rb
+++ b/spec/unit/knife/bootstrap/client_builder_spec.rb
@@ -107,17 +107,20 @@ describe Chef::Knife::Bootstrap::ClientBuilder do
   end
 
   context "#create_client!" do
+    let(:client) { Chef::ApiClient.new }
+
     before do
       # mock out the rest of #run
       expect(client_builder).to receive(:sanity_check)
       expect(client_builder).to receive(:create_node!)
     end
 
-    it "delegates everything to Chef::ApiClient::Registration" do
+    it "delegates everything to Chef::ApiClient::Registration and sets client" do
       reg_double = double("Chef::ApiClient::Registration")
       expect(Chef::ApiClient::Registration).to receive(:new).with(node_name, client_builder.client_path, http_api: rest).and_return(reg_double)
-      expect(reg_double).to receive(:run)
+      expect(reg_double).to receive(:run).and_return(client)
       client_builder.run
+      expect(client_builder.client).to eq(client)
     end
 
   end
@@ -128,7 +131,7 @@ describe Chef::Knife::Bootstrap::ClientBuilder do
     end
   end
 
-  context "#create_node!" do
+  context "#create_node!" do    
     before do
       # mock out the rest of #run
       expect(client_builder).to receive(:sanity_check)
diff --git a/spec/unit/knife/bootstrap_spec.rb b/spec/unit/knife/bootstrap_spec.rb
index 19146ea8e3..b4babdc14f 100644
--- a/spec/unit/knife/bootstrap_spec.rb
+++ b/spec/unit/knife/bootstrap_spec.rb
@@ -633,6 +633,7 @@ describe Chef::Knife::Bootstrap do
       allow(knife).to receive(:knife_ssh).and_return(knife_ssh)
       knife_ssh
     end
+    let(:client) { Chef::ApiClient.new }
 
     context "when running with a configured and present validation key" do
       before do
@@ -666,7 +667,8 @@ describe Chef::Knife::Bootstrap do
         knife.config[:bootstrap_vault_file] = "/not/our/responsibility/to/check/if/this/exists"
         expect(knife_ssh).to receive(:run)
         expect(knife.client_builder).to receive(:run)
-        expect(knife.chef_vault_handler).to receive(:run).with(node_name: knife.config[:chef_node_name])
+        expect(knife.client_builder).to receive(:client).and_return(client)
+        expect(knife.chef_vault_handler).to receive(:run).with(client)
         knife.run
       end
 
@@ -674,7 +676,8 @@ describe Chef::Knife::Bootstrap do
         knife.config[:bootstrap_vault_json] = '{ "vault" => "item" }'
         expect(knife_ssh).to receive(:run)
         expect(knife.client_builder).to receive(:run)
-        expect(knife.chef_vault_handler).to receive(:run).with(node_name: knife.config[:chef_node_name])
+        expect(knife.client_builder).to receive(:client).and_return(client)
+        expect(knife.chef_vault_handler).to receive(:run).with(client)
         knife.run
       end
 
@@ -682,7 +685,7 @@ describe Chef::Knife::Bootstrap do
         expect(File).to receive(:exist?).with(File.expand_path(Chef::Config[:validation_key])).and_return(true)
         expect(knife_ssh).to receive(:run)
         expect(knife.client_builder).not_to receive(:run)
-        expect(knife.chef_vault_handler).not_to receive(:run).with(node_name: knife.config[:chef_node_name])
+        expect(knife.chef_vault_handler).not_to receive(:run)
         knife.run
       end
 
@@ -702,7 +705,8 @@ describe Chef::Knife::Bootstrap do
       it "creates the client (and possibly adds chef-vault items)" do
         expect(knife_ssh).to receive(:run)
         expect(knife.client_builder).to receive(:run)
-        expect(knife.chef_vault_handler).to receive(:run).with(node_name: knife.config[:chef_node_name])
+        expect(knife.client_builder).to receive(:client).and_return(client)
+        expect(knife.chef_vault_handler).to receive(:run).with(client)
         knife.run
       end