From ecd7a7b0260b8f4647f6971661f42427b8ed32c5 Mon Sep 17 00:00:00 2001 From: Thom May Date: Fri, 20 Jan 2017 17:13:46 +0000 Subject: Ensure ssh search paginates correctly Also use partial search for much small on-the-wire sizes Signed-off-by: Thom May --- lib/chef/knife/ssh.rb | 61 +++++++++++++++++++++-------------- spec/functional/knife/ssh_spec.rb | 10 +++--- spec/unit/knife/ssh_spec.rb | 68 +++++++++++++++++++-------------------- 3 files changed, 74 insertions(+), 65 deletions(-) diff --git a/lib/chef/knife/ssh.rb b/lib/chef/knife/ssh.rb index 53801afffd..6797f6cab2 100644 --- a/lib/chef/knife/ssh.rb +++ b/lib/chef/knife/ssh.rb @@ -166,10 +166,10 @@ class Chef def configure_session list = config[:manual] ? @name_args[0].split(" ") : search_nodes if list.length == 0 - if @action_nodes.length == 0 + if @search_count == 0 ui.fatal("No nodes returned from search") else - ui.fatal("#{@action_nodes.length} #{@action_nodes.length > 1 ? "nodes" : "node"} found, " + + ui.fatal("#{@search_count} #{@search_count > 1 ? "nodes" : "node"} found, " + "but does not have the required attribute to establish the connection. " + "Try setting another attribute to open the connection using --attribute.") end @@ -184,41 +184,56 @@ class Chef # 2) configuration file # 3) cloud attribute # 4) fqdn - if config[:attribute] - Chef::Log.debug("Using node attribute '#{config[:attribute]}' as the ssh target") - attribute = config[:attribute] + if node["config"] + Chef::Log.debug("Using node attribute '#{config[:attribute]}' as the ssh target: #{node["config"]}") + node["config"] elsif Chef::Config[:knife][:ssh_attribute] - Chef::Log.debug("Using node attribute #{Chef::Config[:knife][:ssh_attribute]}") - attribute = Chef::Config[:knife][:ssh_attribute] - elsif node[:cloud] && - node[:cloud][:public_hostname] && - !node[:cloud][:public_hostname].empty? - Chef::Log.debug("Using node attribute 'cloud[:public_hostname]' automatically as the ssh target") - attribute = "cloud.public_hostname" + Chef::Log.debug("Using node attribute #{Chef::Config[:knife][:ssh_attribute]}: #{node["knife_config"]}") + node["knife_config"] + elsif node["cloud"] && + node["cloud"]["public_hostname"] && + !node["cloud"]["public_hostname"].empty? + Chef::Log.debug("Using node attribute 'cloud[:public_hostname]' automatically as the ssh target: #{node["cloud"]["public_hostname"]}") + node["cloud"]["public_hostname"] else # falling back to default of fqdn - Chef::Log.debug("Using node attribute 'fqdn' as the ssh target") - attribute = "fqdn" + Chef::Log.debug("Using node attribute 'fqdn' as the ssh target: #{node["fqdn"]}") + node["fqdn"] end - attribute end def search_nodes list = Array.new query = Chef::Search::Query.new - @action_nodes = query.search(:node, @name_args[0])[0] - @action_nodes.each do |item| + required_attributes = { fqdn: ["fqdn"], cloud: ["cloud"] } + + separator = ui.presenter.attribute_field_separator + + # if we've set an attribute to use on the command line + if config[:attribute] + required_attributes[:config] = config[:attribute].split(separator) + end + + # if we've configured an attribute in our config + if Chef::Config[:knife][:ssh_attribute] + required_attributes[:knife_config] = Chef::Config[:knife][:ssh_attribute].split(separator) + end + + @search_count = 0 + query.search(:node, @name_args[0], filter_result: required_attributes) do |item| + @search_count += 1 # we should skip the loop to next iteration if the item # returned by the search is nil next if item.nil? # next if we couldn't find the specified attribute in the # returned node object - host = extract_nested_value(item, get_ssh_attribute(item)) + host = get_ssh_attribute(item) next if host.nil? ssh_port = item[:cloud].nil? ? nil : item[:cloud][:public_ssh_port] srv = [host, ssh_port] list.push(srv) end + list end @@ -309,9 +324,9 @@ class Chef subsession ||= session command = fixup_sudo(command) command.force_encoding("binary") if command.respond_to?(:force_encoding) - subsession.open_channel do |ch| - ch.request_pty - ch.exec command do |ch, success| + subsession.open_channel do |chan| + chan.request_pty + chan.exec command do |ch, success| raise ArgumentError, "Cannot execute #{command}" unless success ch.on_data do |ichannel, data| print_data(ichannel[:host], data) @@ -532,10 +547,6 @@ class Chef config[:ssh_identity_file] = get_stripped_unfrozen_value(config[:ssh_identity_file] || config[:identity_file] || Chef::Config[:knife][:ssh_identity_file]) end - def extract_nested_value(data_structure, path_spec) - ui.presenter.extract_nested_value(data_structure, path_spec) - end - def run @longest = 0 diff --git a/spec/functional/knife/ssh_spec.rb b/spec/functional/knife/ssh_spec.rb index 065b646ac6..aea7585bb2 100644 --- a/spec/functional/knife/ssh_spec.rb +++ b/spec/functional/knife/ssh_spec.rb @@ -181,7 +181,7 @@ describe Chef::Knife::Ssh do it "uses the ssh_attribute" do @knife.run - expect(@knife.get_ssh_attribute(Chef::Node.new)).to eq("ec2.public_hostname") + expect(@knife.get_ssh_attribute({ "knife_config" => "ec2.public_hostname" })).to eq("ec2.public_hostname") end end @@ -193,11 +193,11 @@ describe Chef::Knife::Ssh do it "uses the default" do @knife.run - expect(@knife.get_ssh_attribute(Chef::Node.new)).to eq("fqdn") + expect(@knife.get_ssh_attribute({ "fqdn" => "fqdn" })).to eq("fqdn") end end - context "when -a ec2.public_ipv4 is provided" do + context "when -a ec2.public_public_hostname is provided" do before do setup_knife(["-a ec2.public_hostname", "*:*", "uptime"]) Chef::Config[:knife][:ssh_attribute] = nil @@ -276,8 +276,8 @@ describe Chef::Knife::Ssh do Chef::Config[:client_key] = nil Chef::Config[:chef_server_url] = "http://localhost:9000" - @api.get("/search/node?q=*:*&sort=X_CHEF_id_CHEF_X%20asc&start=0", 200) do - %({"total":1, "start":0, "rows":[{"name":"i-xxxxxxxx", "json_class":"Chef::Node", "automatic":{"fqdn":"the.fqdn", "ec2":{"public_hostname":"the_public_hostname"}},"recipes":[]}]}) + @api.post("/search/node?q=*:*&sort=X_CHEF_id_CHEF_X%20asc&start=0", 200) do + %({"total":1, "start":0, "rows":[{"data": {"fqdn":"the.fqdn", "config": "the_public_hostname", "knife_config": "the_public_hostname" }}]}) end end diff --git a/spec/unit/knife/ssh_spec.rb b/spec/unit/knife/ssh_spec.rb index 9263a0b8e5..6141a8a6df 100644 --- a/spec/unit/knife/ssh_spec.rb +++ b/spec/unit/knife/ssh_spec.rb @@ -21,54 +21,53 @@ require "net/ssh" require "net/ssh/multi" describe Chef::Knife::Ssh do - before(:each) do - Chef::Config[:client_key] = CHEF_SPEC_DATA + "/ssl/private_key.pem" - end + let(:query_result) { double("chef search results") } before do + Chef::Config[:client_key] = CHEF_SPEC_DATA + "/ssl/private_key.pem" @knife = Chef::Knife::Ssh.new @knife.merge_configs - @node_foo = Chef::Node.new - @node_foo.automatic_attrs[:fqdn] = "foo.example.org" - @node_foo.automatic_attrs[:ipaddress] = "10.0.0.1" + @node_foo = {} + @node_foo["fqdn"] = "foo.example.org" + @node_foo["ipaddress"] = "10.0.0.1" + @node_foo["cloud"] = {} + + @node_bar = {} + @node_bar["fqdn"] = "bar.example.org" + @node_bar["ipaddress"] = "10.0.0.2" + @node_bar["cloud"] = {} - @node_bar = Chef::Node.new - @node_bar.automatic_attrs[:fqdn] = "bar.example.org" - @node_bar.automatic_attrs[:ipaddress] = "10.0.0.2" end describe "#configure_session" do context "manual is set to false (default)" do before do @knife.config[:manual] = false - @query = Chef::Search::Query.new - end - - def configure_query(node_array) - allow(@query).to receive(:search).and_return([node_array]) - allow(Chef::Search::Query).to receive(:new).and_return(@query) + allow(query_result).to receive(:search).with(any_args).and_yield(@node_foo).and_yield(@node_bar) + allow(Chef::Search::Query).to receive(:new).and_return(query_result) end def self.should_return_specified_attributes it "returns an array of the attributes specified on the command line OR config file, if only one is set" do + @node_bar["config"] = "10.0.0.2" + @node_foo["config"] = "10.0.0.1" @knife.config[:attribute] = "ipaddress" Chef::Config[:knife][:ssh_attribute] = "ipaddress" # this value will be in the config file - configure_query([@node_foo, @node_bar]) expect(@knife).to receive(:session_from_list).with([["10.0.0.1", nil], ["10.0.0.2", nil]]) @knife.configure_session end it "returns an array of the attributes specified on the command line even when a config value is set" do + @node_bar["config"] = "10.0.0.2" + @node_foo["config"] = "10.0.0.1" Chef::Config[:knife][:ssh_attribute] = "config_file" # this value will be in the config file @knife.config[:attribute] = "ipaddress" # this is the value of the command line via #configure_attribute - configure_query([@node_foo, @node_bar]) expect(@knife).to receive(:session_from_list).with([["10.0.0.1", nil], ["10.0.0.2", nil]]) @knife.configure_session end end it "searchs for and returns an array of fqdns" do - configure_query([@node_foo, @node_bar]) expect(@knife).to receive(:session_from_list).with([ ["foo.example.org", nil], ["bar.example.org", nil], @@ -80,11 +79,10 @@ describe Chef::Knife::Ssh do context "when cloud hostnames are available" do before do - @node_foo.automatic_attrs[:cloud][:public_hostname] = "ec2-10-0-0-1.compute-1.amazonaws.com" - @node_bar.automatic_attrs[:cloud][:public_hostname] = "ec2-10-0-0-2.compute-1.amazonaws.com" + @node_foo["cloud"]["public_hostname"] = "ec2-10-0-0-1.compute-1.amazonaws.com" + @node_bar["cloud"]["public_hostname"] = "ec2-10-0-0-2.compute-1.amazonaws.com" end it "returns an array of cloud public hostnames" do - configure_query([@node_foo, @node_bar]) expect(@knife).to receive(:session_from_list).with([ ["ec2-10-0-0-1.compute-1.amazonaws.com", nil], ["ec2-10-0-0-2.compute-1.amazonaws.com", nil], @@ -97,12 +95,11 @@ describe Chef::Knife::Ssh do context "when cloud hostnames are available but empty" do before do - @node_foo.automatic_attrs[:cloud][:public_hostname] = "" - @node_bar.automatic_attrs[:cloud][:public_hostname] = "" + @node_foo["cloud"]["public_hostname"] = "" + @node_bar["cloud"]["public_hostname"] = "" end it "returns an array of fqdns" do - configure_query([@node_foo, @node_bar]) expect(@knife).to receive(:session_from_list).with([ ["foo.example.org", nil], ["bar.example.org", nil], @@ -114,7 +111,7 @@ describe Chef::Knife::Ssh do end it "should raise an error if no host are found" do - configure_query([ ]) + allow(query_result).to receive(:search).with(any_args) expect(@knife.ui).to receive(:fatal) expect(@knife).to receive(:exit).with(10) @knife.configure_session @@ -122,10 +119,8 @@ describe Chef::Knife::Ssh do context "when there are some hosts found but they do not have an attribute to connect with" do before do - allow(@query).to receive(:search).and_return([[@node_foo, @node_bar]]) - @node_foo.automatic_attrs[:fqdn] = nil - @node_bar.automatic_attrs[:fqdn] = nil - allow(Chef::Search::Query).to receive(:new).and_return(@query) + @node_foo["fqdn"] = nil + @node_bar["fqdn"] = nil end it "should raise a specific error (CHEF-3402)" do @@ -158,31 +153,34 @@ describe Chef::Knife::Ssh do before do Chef::Config[:knife][:ssh_attribute] = nil @knife.config[:attribute] = nil - @node_foo.automatic_attrs[:cloud][:public_hostname] = "ec2-10-0-0-1.compute-1.amazonaws.com" - @node_bar.automatic_attrs[:cloud][:public_hostname] = "" + @node_foo["cloud"]["public_hostname"] = "ec2-10-0-0-1.compute-1.amazonaws.com" + @node_bar["cloud"]["public_hostname"] = "" end it "should return fqdn by default" do - expect(@knife.get_ssh_attribute(Chef::Node.new)).to eq("fqdn") + expect(@knife.get_ssh_attribute({ "fqdn" => "fqdn" })).to eq("fqdn") end it "should return cloud.public_hostname attribute if available" do - expect(@knife.get_ssh_attribute(@node_foo)).to eq("cloud.public_hostname") + expect(@knife.get_ssh_attribute(@node_foo)).to eq("ec2-10-0-0-1.compute-1.amazonaws.com") end it "should favor to attribute_from_cli over config file and cloud" do @knife.config[:attribute] = "command_line" Chef::Config[:knife][:ssh_attribute] = "config_file" + @node_foo["config"] = "command_line" + @node_foo["knife_config"] = "config_file" expect( @knife.get_ssh_attribute(@node_foo)).to eq("command_line") end it "should favor config file over cloud and default" do Chef::Config[:knife][:ssh_attribute] = "config_file" + @node_foo["knife_config"] = "config_file" expect( @knife.get_ssh_attribute(@node_foo)).to eq("config_file") end it "should return fqdn if cloud.hostname is empty" do - expect( @knife.get_ssh_attribute(@node_bar)).to eq("fqdn") + expect( @knife.get_ssh_attribute(@node_bar)).to eq("bar.example.org") end end @@ -294,7 +292,7 @@ describe Chef::Knife::Ssh do describe "#run" do before do @query = Chef::Search::Query.new - expect(@query).to receive(:search).and_return([[@node_foo]]) + expect(@query).to receive(:search).and_yield(@node_foo) allow(Chef::Search::Query).to receive(:new).and_return(@query) allow(@knife).to receive(:ssh_command).and_return(exit_code) @knife.name_args = ["*:*", "false"] -- cgit v1.2.1