diff --git a/.rubocop.yml b/.rubocop.yml index f47daf25..4c9b65d9 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,6 +1,8 @@ AllCops: DisplayCopNames: true DisplayStyleGuide: true + SuggestExtensions: false + TargetRubyVersion: 2.4 Exclude: - "module/**/*" - "vendor/**/*" @@ -25,9 +27,6 @@ Style/TrailingUnderscoreVariable: Lint/NonLocalExitFromIterator: Enabled: false -Style/TrailingUnderscoreVariable: - Enabled: false - Naming/ClassAndModuleCamelCase: Enabled: false @@ -46,6 +45,9 @@ Style/StringLiterals: Style/FormatString: EnforcedStyle: percent +Style/FormatStringToken: + EnforcedStyle: unannotated + Style/SpecialGlobalVars: Enabled: false @@ -70,9 +72,6 @@ Layout/SpaceInsideBlockBraces: Layout/SpaceInsideHashLiteralBraces: EnforcedStyle: no_space -Style/Documentation: - Severity: warning - Style/EachWithObject: Enabled: false @@ -87,6 +86,7 @@ Style/DoubleNegation: Style/Documentation: Enabled: false + Severity: warning Style/PerlBackrefs: Enabled: false @@ -106,7 +106,7 @@ Lint/UnusedMethodArgument: Lint/AssignmentInCondition: Enabled: false -Metrics/LineLength: +Layout/LineLength: Max: 180 Metrics/PerceivedComplexity: @@ -133,9 +133,6 @@ Metrics/ModuleLength: Metrics/BlockLength: Enabled: false -Performance/Casecmp: - Enabled: false - Style/FrozenStringLiteralComment: Enabled: false @@ -145,21 +142,18 @@ Style/SymbolArray: Security/MarshalLoad: Enabled: false -Layout/IndentHeredoc: +Layout/HeredocIndentation: Enabled: false # @todo use safe_load but its a big change Security/YAMLLoad: Enabled: false -Performance/RegexpMatch: - Enabled: false - Style/SafeNavigation: Enabled: false -Lint/RescueWithoutErrorClass: +Style/RescueStandardError: Enabled: false -Performance/StringReplacement: +Style/OptionalBooleanParameter: Enabled: false diff --git a/Gemfile b/Gemfile index 0709c82c..6b835788 100755 --- a/Gemfile +++ b/Gemfile @@ -1,6 +1,5 @@ source ENV["GEM_SOURCE"] || "https://rubygems.org" -gem "json", "~> 2.1.0" gem "systemu", "~> 2.6.4" gem "rdoc" gem "yarjuf", "~> 1.0" @@ -8,4 +7,4 @@ gem "rspec", "~> 2.11.0" gem "mocha", "~> 0.10.0" gem "mcollective-test" gem "rake", "< 11.0" -gem "rubocop", "0.51.0" +gem "rubocop", "1.6.1" diff --git a/Rakefile b/Rakefile index 86a6af5b..0b2342b0 100644 --- a/Rakefile +++ b/Rakefile @@ -28,5 +28,6 @@ end desc "Run spec tests" task :test do - sh "cd spec && rake" + sh "bundle exec rubocop --config .rubocop.yml lib" + sh "bundle exec rspec --colour --backtrace" end diff --git a/lib/mcollective/agent/rpcutil.ddl b/lib/mcollective/agent/rpcutil.ddl index bd1c4e9a..34fe8d60 100644 --- a/lib/mcollective/agent/rpcutil.ddl +++ b/lib/mcollective/agent/rpcutil.ddl @@ -1,220 +1,220 @@ -metadata :name => "rpcutil", +metadata :name => "rpcutil", :description => "General helpful actions that expose stats and internals to SimpleRPC clients", - :author => "R.I.Pienaar ", - :license => "Apache License, Version 2.0", - :version => "1.0.0", - :url => "https://choria.io/", - :timeout => 10 + :author => "R.I.Pienaar ", + :license => "Apache License, Version 2.0", + :version => "1.0.0", + :url => "https://choria.io/", + :timeout => 10 action "collective_info", :description => "Info about the main and sub collectives" do - display :always + display :always - output :main_collective, - :description => "The main Collective", - :display_as => "Main Collective" + output :main_collective, + :description => "The main Collective", + :display_as => "Main Collective" - output :collectives, - :description => "All Collectives", - :display_as => "All Collectives" + output :collectives, + :description => "All Collectives", + :display_as => "All Collectives" - summarize do - aggregate summary(:collectives) - end + summarize do + aggregate summary(:collectives) + end end action "inventory", :description => "System Inventory" do - display :always + display :always - output :agents, - :description => "List of agent names", - :display_as => "Agents" + output :agents, + :description => "List of agent names", + :display_as => "Agents" - output :facts, - :description => "List of facts and values", - :display_as => "Facts" + output :facts, + :description => "List of facts and values", + :display_as => "Facts" - output :classes, - :description => "List of classes on the system", - :display_as => "Classes" + output :classes, + :description => "List of classes on the system", + :display_as => "Classes" - output :version, - :description => "MCollective Version", - :display_as => "Version" + output :version, + :description => "MCollective Version", + :display_as => "Version" - output :main_collective, - :description => "The main Collective", - :display_as => "Main Collective" + output :main_collective, + :description => "The main Collective", + :display_as => "Main Collective" - output :collectives, - :description => "All Collectives", - :display_as => "All Collectives" + output :collectives, + :description => "All Collectives", + :display_as => "All Collectives" - output :data_plugins, - :description => "List of data plugin names", - :display_as => "Data Plugins" + output :data_plugins, + :description => "List of data plugin names", + :display_as => "Data Plugins" end action "get_fact", :description => "Retrieve a single fact from the fact store" do - display :always - - input :fact, - :prompt => "The name of the fact", - :description => "The fact to retrieve", - :type => :string, - :validation => '^[\w\-\.]+$', - :optional => false, - :maxlength => 40 - - output :fact, - :description => "The name of the fact being returned", - :display_as => "Fact" - - output :value, - :description => "The value of the fact", - :display_as => "Value" - - summarize do - aggregate summary(:value) - end + display :always + + input :fact, + :prompt => "The name of the fact", + :description => "The fact to retrieve", + :type => :string, + :validation => '^[\w\-\.]+$', + :optional => false, + :maxlength => 40 + + output :fact, + :description => "The name of the fact being returned", + :display_as => "Fact" + + output :value, + :description => "The value of the fact", + :display_as => "Value" + + summarize do + aggregate summary(:value) + end end action "get_facts", :description => "Retrieve multiple facts from the fact store" do - display :always - - input :facts, - :prompt => "Comma-separated list of facts", - :description => "Facts to retrieve", - :type => :string, - :validation => '^\s*[\w\.\-]+(\s*,\s*[\w\.\-]+)*$', - :optional => false, - :maxlength => 200 - - output :values, - :description => "List of values of the facts", - :display_as => "Values" + display :always + + input :facts, + :prompt => "Comma-separated list of facts", + :description => "Facts to retrieve", + :type => :string, + :validation => '^\s*[\w\.\-]+(\s*,\s*[\w\.\-]+)*$', + :optional => false, + :maxlength => 200 + + output :values, + :description => "List of values of the facts", + :display_as => "Values" end action "daemon_stats", :description => "Get statistics from the running daemon" do - display :always + display :always - output :threads, - :description => "List of threads active in the daemon", - :display_as => "Threads" + output :threads, + :description => "List of threads active in the daemon", + :display_as => "Threads" - output :agents, - :description => "List of agents loaded", - :display_as => "Agents" + output :agents, + :description => "List of agents loaded", + :display_as => "Agents" - output :pid, - :description => "Process ID of the daemon", - :display_as => "PID" + output :pid, + :description => "Process ID of the daemon", + :display_as => "PID" - output :times, - :description => "Processor time consumed by the daemon", - :display_as => "Times" + output :times, + :description => "Processor time consumed by the daemon", + :display_as => "Times" - output :validated, - :description => "Messages that passed security validation", - :display_as => "Security Validated" + output :validated, + :description => "Messages that passed security validation", + :display_as => "Security Validated" - output :unvalidated, - :description => "Messages that failed security validation", - :display_as => "Failed Security" + output :unvalidated, + :description => "Messages that failed security validation", + :display_as => "Failed Security" - output :passed, - :description => "Passed filter checks", - :display_as => "Passed Filter" + output :passed, + :description => "Passed filter checks", + :display_as => "Passed Filter" - output :filtered, - :description => "Didn't pass filter checks", - :display_as => "Failed Filter" + output :filtered, + :description => "Didn't pass filter checks", + :display_as => "Failed Filter" - output :starttime, - :description => "Time the server started", - :display_as => "Start Time" + output :starttime, + :description => "Time the server started", + :display_as => "Start Time" - output :total, - :description => "Total messages received", - :display_as => "Total Messages" + output :total, + :description => "Total messages received", + :display_as => "Total Messages" - output :replies, - :description => "Replies sent back to clients", - :display_as => "Replies" + output :replies, + :description => "Replies sent back to clients", + :display_as => "Replies" - output :configfile, - :description => "Config file used to start the daemon", - :display_as => "Config File" + output :configfile, + :description => "Config file used to start the daemon", + :display_as => "Config File" - output :version, - :description => "MCollective Version", - :display_as => "Version" + output :version, + :description => "MCollective Version", + :display_as => "Version" - output :ttlexpired, - :description => "Messages that did pass TTL checks", - :display_as => "TTL Expired" + output :ttlexpired, + :description => "Messages that did pass TTL checks", + :display_as => "TTL Expired" - summarize do - aggregate summary(:version) - aggregate summary(:agents) - end + summarize do + aggregate summary(:version) + aggregate summary(:agents) + end end action "agent_inventory", :description => "Inventory of all agents on the server" do - display :always + display :always - output :agents, - :description => "List of agents on the server", - :display_as => "Agents" + output :agents, + :description => "List of agents on the server", + :display_as => "Agents" end action "get_config_item", :description => "Get the active value of a specific config property" do - display :always - - input :item, - :prompt => "Configuration Item", - :description => "The item to retrieve from the server", - :type => :string, - :validation => '^.+$', - :optional => false, - :maxlength => 50 - - output :item, - :description => "The config property being retrieved", - :display_as => "Property" - - output :value, - :description => "The value that is in use", - :display_as => "Value" - - summarize do - aggregate summary(:value) - end + display :always + + input :item, + :prompt => "Configuration Item", + :description => "The item to retrieve from the server", + :type => :string, + :validation => "^.+$", + :optional => false, + :maxlength => 50 + + output :item, + :description => "The config property being retrieved", + :display_as => "Property" + + output :value, + :description => "The value that is in use", + :display_as => "Value" + + summarize do + aggregate summary(:value) + end end action "get_data", :description => "Get data from a data plugin" do - display :always - - input :source, - :prompt => "Data Source", - :description => "The data plugin to retrieve information from", - :type => :string, - :validation => '^\w+$', - :optional => false, - :maxlength => 50 - - input :query, - :prompt => "Query", - :description => "The query argument to supply to the data plugin", - :type => :string, - :validation => '^.+$', - :optional => true, - :maxlength => 200 + display :always + + input :source, + :prompt => "Data Source", + :description => "The data plugin to retrieve information from", + :type => :string, + :validation => '^\w+$', + :optional => false, + :maxlength => 50 + + input :query, + :prompt => "Query", + :description => "The query argument to supply to the data plugin", + :type => :string, + :validation => "^.+$", + :optional => true, + :maxlength => 200 end action "ping", :description => "Responds to requests for PING with PONG" do - display :always + display :always - output :pong, - :description => "The local timestamp", - :display_as => "Timestamp" + output :pong, + :description => "The local timestamp", + :display_as => "Timestamp" end diff --git a/lib/mcollective/agents.rb b/lib/mcollective/agents.rb index 4d39d4ae..19523ba3 100644 --- a/lib/mcollective/agents.rb +++ b/lib/mcollective/agents.rb @@ -42,6 +42,7 @@ def loadagents def loadagent(agentname) agentfile = findagentfile(agentname) return false unless agentfile + classname = class_for_agent(agentname) PluginManager.delete("#{agentname}_agent") @@ -62,16 +63,16 @@ def loadagent(agentname) Util.subscribe(Util.make_subscriptions(agentname, :broadcast)) unless @@agents.include?(agentname) @@agents[agentname] = {:file => agentfile} - return true + true else Log.debug("Not activating agent #{agentname} due to agent policy in activate? method") - return false + false end rescue Exception # rubocop:disable Lint/RescueException Log.error("Loading agent %s failed: %s" % [agentname, $!]) PluginManager.delete("%s_agent" % agentname) - return false + false end end @@ -87,14 +88,14 @@ def activate_agent?(agent) klass = Kernel.const_get("MCollective").const_get("Agent").const_get(agent.capitalize) if klass.respond_to?("activate?") - return klass.activate? + klass.activate? else Log.debug("#{klass} does not have an activate? method, activating as default") - return true + true end rescue Exception # rubocop:disable Lint/RescueException Log.warn("Agent activation check for %s #{agent} failed: %s: %s" % [agent, $!.class, $!]) - return false + false end # searches the libdirs for agents diff --git a/lib/mcollective/aggregate.rb b/lib/mcollective/aggregate.rb index d1f11a9b..44e1324f 100644 --- a/lib/mcollective/aggregate.rb +++ b/lib/mcollective/aggregate.rb @@ -25,7 +25,7 @@ def create_functions format = (arguments.delete(:format) if arguments) || nil begin @functions << load_function(agg[:function]).new(output, arguments, format, @action) - rescue Exception => e + rescue Exception => e # rubocop:disable Lint/RescueException Log.error("Cannot create aggregate function '%s': %s" % [output, e]) @failed << {:name => output, :type => :startup} end @@ -47,7 +47,7 @@ def call_functions(reply) Log.debug("Calling aggregate function %s for result" % function) begin function.process_result(reply[:data][function.output_name], reply) - rescue Exception => e + rescue Exception => e # rubocop:disable Lint/RescueException Log.error("Could not process aggregate function for '%s': %s" % [function.output_name, e]) @failed << {:name => function.output_name, :type => :process_result} @functions.delete(function) @@ -60,7 +60,7 @@ def summarize summary = @functions.map do |function| begin function.summarize - rescue Exception => e + rescue Exception => e # rubocop:disable Lint/RescueException Log.error("Could not summarize aggregate result for '%s': %s" % [function.output_name, e]) @failed << {:name => function.output_name, :type => :summarize} nil @@ -78,7 +78,7 @@ def load_function(function_name) PluginManager.loadclass("MCollective::Aggregate::%s" % function_name) unless Aggregate.const_defined?(function_name) Aggregate.const_get(function_name) - rescue Exception + rescue Exception # rubocop:disable Lint/RescueException raise("Aggregate function file '%s.rb' cannot be loaded" % function_name.downcase) end end diff --git a/lib/mcollective/aggregate/average.rb b/lib/mcollective/aggregate/average.rb index 5e56d2e1..8810fa96 100644 --- a/lib/mcollective/aggregate/average.rb +++ b/lib/mcollective/aggregate/average.rb @@ -1,6 +1,6 @@ module MCollective class Aggregate - class Average y[1]}.reverse.each do |value| + @result[:value].sort {|x, y| x[1] <=> y[1]}.reverse.each do |value| result.puts @aggregate_format % [value[0], value[1]] end diff --git a/lib/mcollective/aggregate/result/numeric_result.rb b/lib/mcollective/aggregate/result/numeric_result.rb index ffa77ba8..1d511467 100644 --- a/lib/mcollective/aggregate/result/numeric_result.rb +++ b/lib/mcollective/aggregate/result/numeric_result.rb @@ -1,11 +1,11 @@ module MCollective class Aggregate module Result - class NumericResult nil, - :usage => [], - :cli_arguments => [], + @application_options = {:description => nil, + :usage => [], + :cli_arguments => [], :exclude_arg_sections => []} end end @@ -101,7 +101,7 @@ def validate_option(blk, name, value) validation_result = blk.call(value) unless validation_result == true - STDERR.puts "Validation of #{name} failed: #{validation_result}" + warn "Validation of #{name} failed: #{validation_result}" exit 1 end end @@ -123,7 +123,7 @@ def clioptions(help) post_option_parser(configuration) if respond_to?(:post_option_parser) - return options + options rescue Exception # rubocop:disable Lint/RescueException application_failure($!) end @@ -205,14 +205,15 @@ def validate_cli_options application_cli_arguments.each do |carg| # Check for required arguments next unless carg[:required] + unless configuration[carg[:name]] validation_passed = false - STDERR.puts "The #{carg[:name]} option is mandatory" + warn "The #{carg[:name]} option is mandatory" end end unless validation_passed - STDERR.puts "\nPlease run with --help for detailed help" + warn "\nPlease run with --help for detailed help" exit 1 end end @@ -242,23 +243,23 @@ def application_cli_arguments # Handles failure, if we're far enough in the initialization # phase it will log backtraces if its in verbose mode only - def application_failure(e, err_dest=STDERR) + def application_failure(err, err_dest=$stderr) # peole can use exit() anywhere and not get nasty backtraces as a result - if e.is_a?(SystemExit) + if err.is_a?(SystemExit) disconnect - raise(e) + raise(err) end if options[:verbose] - err_dest.puts "\nThe %s application failed to run: %s\n" % [Util.colorize(:bold, $0), Util.colorize(:red, e.to_s)] + err_dest.puts "\nThe %s application failed to run: %s\n" % [Util.colorize(:bold, $0), Util.colorize(:red, err.to_s)] else - err_dest.puts "\nThe %s application failed to run, use -v for full error backtrace details: %s\n" % [Util.colorize(:bold, $0), Util.colorize(:red, e.to_s)] + err_dest.puts "\nThe %s application failed to run, use -v for full error backtrace details: %s\n" % [Util.colorize(:bold, $0), Util.colorize(:red, err.to_s)] end if options.nil? || options[:verbose] - e.backtrace.first << Util.colorize(:red, " <----") - err_dest.puts "\n%s %s" % [Util.colorize(:red, e.to_s), Util.colorize(:bold, "(#{e.class})")] - e.backtrace.each {|l| err_dest.puts "\tfrom #{l}"} + err.backtrace.first << Util.colorize(:red, " <----") + err_dest.puts "\n%s %s" % [Util.colorize(:red, err.to_s), Util.colorize(:bold, "(#{err.class})")] + err.backtrace.each {|l| err_dest.puts "\tfrom #{l}"} end disconnect @@ -289,13 +290,13 @@ def run def disconnect MCollective::PluginManager["connector_plugin"].disconnect - rescue # rubocop:disable Lint/HandleExceptions + rescue # rubocop:disable Lint/SuppressedException end # Fake abstract class that logs if the user tries to use an application without # supplying a main override method. def main - STDERR.puts "Applications need to supply a 'main' method" + warn "Applications need to supply a 'main' method" exit 1 end @@ -305,9 +306,7 @@ def halt_code(stats) :okcount => 0, :failcount => 0}.merge(stats.to_hash) - if request_stats[:discoverytime] == 0 && request_stats[:responses] == 0 - return 4 - end + return 4 if request_stats[:discoverytime] == 0 && request_stats[:responses] == 0 if request_stats[:discovered] > 0 if request_stats[:responses] == 0 diff --git a/lib/mcollective/application/completion.rb b/lib/mcollective/application/completion.rb index efa9ed56..1a60058f 100644 --- a/lib/mcollective/application/completion.rb +++ b/lib/mcollective/application/completion.rb @@ -1,5 +1,5 @@ module MCollective - class Application::Completion 1 diff --git a/lib/mcollective/application/describe_filter.rb b/lib/mcollective/application/describe_filter.rb index f37a416f..090d22e5 100644 --- a/lib/mcollective/application/describe_filter.rb +++ b/lib/mcollective/application/describe_filter.rb @@ -1,5 +1,5 @@ module MCollective - class Application::Describe_filter=|=|=~|=)/ op = $1 - k,v = token.values[0].split(op) + k, v = token.values[0].split(op) puts indent + get_fact_string(k, v, op) else puts indent + get_class_string(token.values[0]) end - elsif token.keys[0] == "fstatement" + when "fstatement" v = token.values[0] - result_string = indent + "Execute the Data Query '#{v["name"]}'" - if v["params"] - result_string += " with parameters (#{v["params"]})" - end + result_string = indent + "Execute the Data Query '#{v['name']}'" + result_string += " with parameters (#{v['params']})" if v["params"] result_string += ". " - result_string += "Check if the query's '#{v["value"]}' value #{v["operator"]} '#{v["r_compare"]}' " + result_string += "Check if the query's '#{v['value']}' value #{v['operator']} '#{v['r_compare']}' " puts result_string - elsif token.keys[0] == "(" - puts indent + "(" + when "(" + puts "#{indent}(" old_indent = indent indent *= 2 - elsif token.keys[0] == ")" + when ")" indent = old_indent - puts indent + ")" + puts "#{indent})" else puts indent + token.keys[0].upcase end @@ -46,7 +45,7 @@ def describe_f_filter(facts) puts "-F filter expands to the following fact comparisons:" puts facts.each do |f| - puts " " + get_fact_string(f[:fact], f[:value], f[:operator]) + puts " #{get_fact_string(f[:fact], f[:value], f[:operator])}" end end @@ -54,30 +53,31 @@ def describe_c_filter(classes) puts "-C filter expands to the following class checks:" puts classes.each do |c| - puts " " + get_class_string(c) + puts " #{get_class_string(c)}" end end def main - if !(@options[:filter]["fact"].empty?) + unless @options[:filter]["fact"].empty? describe_f_filter(@options[:filter]["fact"]) puts end - if !(@options[:filter]["cf_class"].empty?) + unless @options[:filter]["cf_class"].empty? describe_c_filter(@options[:filter]["cf_class"]) puts end - if !(@options[:filter]["compound"].empty?) + unless @options[:filter]["compound"].empty? describe_s_filter(@options[:filter]["compound"][0]) puts end end private - def get_fact_string(fact, value, op) - "Check if fact '#{fact}' #{op} '#{value}'" + + def get_fact_string(fact, value, oper) + "Check if fact '#{fact}' #{oper} '#{value}'" end def get_class_string(classname) diff --git a/lib/mcollective/application/facts.rb b/lib/mcollective/application/facts.rb index 28d3b70b..428eb383 100644 --- a/lib/mcollective/application/facts.rb +++ b/lib/mcollective/application/facts.rb @@ -1,8 +1,8 @@ -class MCollective::Application::Facts 0 + configuration[:fact] = ARGV.shift unless ARGV.empty? end def validate_configuration(configuration) @@ -18,15 +18,15 @@ def show_single_fact_report(fact, facts, verbose=false) facts.keys.sort.each do |k| printf(" %-#{field_size}s found %d times\n", k, facts[k].size) - if verbose - puts + next unless verbose - facts[k].sort.each do |f| - puts(" #{f}") - end + puts - puts + facts[k].sort.each do |f| + puts(" #{f}") end + + puts end end @@ -50,11 +50,11 @@ def main if facts.include?(value) facts[value] << resp[:senderid] else - facts[value] = [ resp[:senderid] ] + facts[value] = [resp[:senderid]] end end - rescue Exception => e - STDERR.puts "Could not parse facts for #{resp[:senderid]}: #{e.class}: #{e}" + rescue Exception => e # rubocop:disable Lint/RescueException + warn "Could not parse facts for #{resp[:senderid]}: #{e.class}: #{e}" end end diff --git a/lib/mcollective/application/find.rb b/lib/mcollective/application/find.rb index 92c83a24..b8084bb3 100644 --- a/lib/mcollective/application/find.rb +++ b/lib/mcollective/application/find.rb @@ -1,4 +1,4 @@ -class MCollective::Application::Find 0 ? exit(0) : exit(1) + !nodes.empty? ? exit(0) : exit(1) end end diff --git a/lib/mcollective/application/help.rb b/lib/mcollective/application/help.rb index 09395eb1..560e909e 100644 --- a/lib/mcollective/application/help.rb +++ b/lib/mcollective/application/help.rb @@ -1,10 +1,10 @@ module MCollective - class Application::Help 0 + configuration[:application] = ARGV.shift unless ARGV.empty? end def main @@ -17,7 +17,7 @@ def main Applications.list.sort.each do |app| begin puts " %-15s %s" % [app, Applications[app].application_description] - rescue + rescue # rubocop:disable Lint/SuppressedException end end diff --git a/lib/mcollective/application/inventory.rb b/lib/mcollective/application/inventory.rb index 096796f6..2b457e28 100644 --- a/lib/mcollective/application/inventory.rb +++ b/lib/mcollective/application/inventory.rb @@ -1,22 +1,24 @@ -class MCollective::Application::Inventory "Script to run", - :arguments => ["--script SCRIPT"] + :description => "Script to run", + :arguments => ["--script SCRIPT"] option :collectives, - :description => "List all known collectives", - :arguments => ["--list-collectives", "--lc"], - :default => false, - :type => :bool + :description => "List all known collectives", + :arguments => ["--list-collectives", "--lc"], + :default => false, + :type => :bool option :collectivemap, - :description => "Create a DOT graph of all collectives", - :arguments => ["--collective-graph MAP", "--cg MAP", "--map MAP"] + :description => "Create a DOT graph of all collectives", + :arguments => ["--collective-graph MAP", "--cg MAP", "--map MAP"] + + attr_writer :page_length, :page_heading, :page_body def post_option_parser(configuration) - configuration[:node] = ARGV.shift if ARGV.size > 0 + configuration[:node] = ARGV.shift unless ARGV.empty? end def validate_configuration(configuration) @@ -26,7 +28,7 @@ def validate_configuration(configuration) end # Get all the known collectives and nodes that belong to them - def get_collectives + def fetch_collectives util = rpcclient("rpcutil") util.progress = false @@ -34,17 +36,15 @@ def get_collectives nodes = 0 total = 0 - util.collective_info do |r, cinfo| - begin - if cinfo[:data] && cinfo[:data][:collectives] - cinfo[:data][:collectives].each do |collective| - collectives[collective] ||= [] - collectives[collective] << cinfo[:sender] - end - - nodes += 1 - total += 1 + util.collective_info do |_r, cinfo| + if cinfo[:data] && cinfo[:data][:collectives] + cinfo[:data][:collectives].each do |collective| + collectives[collective] ||= [] + collectives[collective] << cinfo[:sender] end + + nodes += 1 + total += 1 end end @@ -55,21 +55,21 @@ def get_collectives def collectives_map(file) File.open(file, "w") do |graph| puts "Retrieving collective info...." - collectives = get_collectives + collectives = fetch_collectives - graph.puts 'graph {' + graph.puts "graph {" collectives[:collectives].keys.sort.each do |collective| - graph.puts ' subgraph "%s" {' % [ collective ] + graph.puts ' subgraph "%s" {' % [collective] collectives[:collectives][collective].each do |member| - graph.puts ' "%s" -- "%s"' % [ member, collective ] + graph.puts ' "%s" -- "%s"' % [member, collective] end - graph.puts ' }' + graph.puts " }" end - graph.puts '}' + graph.puts "}" puts "Graph of #{collectives[:total_nodes]} nodes has been written to #{file}" end @@ -77,21 +77,21 @@ def collectives_map(file) # Prints a report of all known sub collectives def collectives_report - collectives = get_collectives + collectives = fetch_collectives - puts " %-30s %s" % [ "Collective", "Nodes" ] - puts " %-30s %s" % [ "==========", "=====" ] + puts " %-30s %s" % ["Collective", "Nodes"] + puts " %-30s %s" % ["==========", "====="] - collectives[:collectives].sort_by {|key,count| count.size}.each do |collective| - puts " %-30s %d" % [ collective[0], collective[1].size ] + collectives[:collectives].sort_by {|_key, count| count.size}.each do |collective| + puts " %-30s %d" % [collective[0], collective[1].size] end puts - puts " %30s %d" % [ "Total nodes:", collectives[:nodes] ] + puts " %30s %d" % ["Total nodes:", collectives[:nodes]] puts end - def node_inventory + def node_inventory # rubocop:disable Metrics/MethodLength node = configuration[:node] util = rpcclient("rpcutil") @@ -101,16 +101,14 @@ def node_inventory nodestats = util.custom_request("daemon_stats", {}, node, {"identity" => node}).first unless nodestats - STDERR.puts "Did not receive any results from node #{node}" + warn "Did not receive any results from node #{node}" exit 1 end - unless nodestats[:statuscode] == 0 - STDERR.puts "Failed to retrieve daemon_stats from #{node}: #{nodestats[:statusmsg]}" - else + if nodestats[:statuscode] == 0 util.custom_request("inventory", {}, node, {"identity" => node}).each do |resp| unless resp[:statuscode] == 0 - STDERR.puts "Failed to retrieve inventory for #{node}: #{resp[:statusmsg]}" + warn "Failed to retrieve inventory for #{node}: #{resp[:statusmsg]}" next end @@ -140,7 +138,7 @@ def node_inventory puts puts " Agents:" - if data[:agents].size > 0 + if !data[:agents].empty? data[:agents].sort.in_groups_of(3, "") do |agents| puts " %-15s %-15s %-15s" % agents end @@ -151,9 +149,9 @@ def node_inventory puts puts " Data Plugins:" - if data[:data_plugins].size > 0 + if !data[:data_plugins].empty? data[:data_plugins].sort.in_groups_of(3, "") do |plugins| - puts " %-15s %-15s %-15s" % plugins.map{|p| p.gsub("_data", "")} + puts " %-15s %-15s %-15s" % plugins.map {|p| p.gsub("_data", "")} end else puts " No data plugins installed" @@ -162,10 +160,10 @@ def node_inventory puts puts " Configuration Management Classes:" - if data[:classes].size > 0 + if !data[:classes].empty? field_size = MCollective::Util.field_size(data[:classes], 30) fields_num = MCollective::Util.field_number(field_size) - format = " " + (" %-#{field_size}s" * fields_num) + format = " #{" %-#{field_size}s" * fields_num}" data[:classes].sort.in_groups_of(fields_num, "") do |klasses| puts format % klasses @@ -177,8 +175,8 @@ def node_inventory puts puts " Facts:" - if data[:facts].size > 0 - data[:facts].sort_by{|f| f[0]}.each do |f| + if !data[:facts].empty? + data[:facts].sort_by {|f| f[0]}.each do |f| puts " #{f[0]} => #{f[1]}" end else @@ -186,10 +184,12 @@ def node_inventory end break - rescue Exception => e - STDERR.puts "Failed to display node inventory: #{e.class}: #{e}" + rescue Exception => e # rubocop:disable Lint/RescueException + warn "Failed to display node inventory: #{e.class}: #{e}" end end + else + warn "Failed to retrieve daemon_stats from #{node}: #{nodestats[:statusmsg]}" end halt util.stats @@ -220,18 +220,6 @@ def agents @node[:agents] end - def page_length(len) - @page_length = len - end - - def page_heading(fmt) - @page_heading = fmt - end - - def page_body(fmt) - @page_body = fmt - end - # Expects a simple printf style format and apply it to # each node: # @@ -251,11 +239,11 @@ def inventory(&blk) util = rpcclient("rpcutil") util.progress = false - util.inventory do |t, resp| + util.inventory do |_t, resp| @node = {:identity => resp[:sender], - :facts => resp[:data][:facts], - :classes => resp[:data][:classes], - :agents => resp[:data][:agents]} + :facts => resp[:data][:facts], + :classes => resp[:data][:classes], + :agents => resp[:data][:agents]} puts @fmt % @flds.call end @@ -289,7 +277,7 @@ def inventory(&blk) # BODY # end def formatted_inventory(&blk) - require 'formatr' + require "formatr" raise "Need to give a block to formatted_inventory" unless block_given? @@ -306,14 +294,14 @@ def formatted_inventory(&blk) util.inventory do |t, resp| @node = {:identity => resp[:sender], - :facts => resp[:data][:facts], - :classes => resp[:data][:classes], - :agents => resp[:data][:agents]} + :facts => resp[:data][:facts], + :classes => resp[:data][:classes], + :agents => resp[:data][:agents]} body_fmt.printFormat(binding) end - rescue Exception => e - STDERR.puts "Could not create report: #{e.class}: #{e}" + rescue Exception => e # rubocop:disable Lint/RescueException + warn "Could not create report: #{e.class}: #{e}" exit 1 end @@ -326,7 +314,7 @@ def formatted_inventory(&blk) def main if configuration[:script] if File.exist?(configuration[:script]) - eval(File.read(configuration[:script])) + eval(File.read(configuration[:script])) # rubocop:disable Security/Eval else raise "Could not find script to run: #{configuration[:script]}" end diff --git a/lib/mcollective/application/ping.rb b/lib/mcollective/application/ping.rb index 262782b6..15b12849 100644 --- a/lib/mcollective/application/ping.rb +++ b/lib/mcollective/application/ping.rb @@ -1,13 +1,12 @@ -# encoding: utf-8 module MCollective - class Application::Ping "Shows a graph of ping distribution", - :arguments => ["--graph", "-g"], - :default => false, - :type => :bool + :description => "Shows a graph of ping distribution", + :arguments => ["--graph", "-g"], + :default => false, + :type => :bool # Convert the times structure into a array representing # buckets of responses in 50 ms intervals. Return a small @@ -15,7 +14,7 @@ class Application::Ping 0 - sum = times.inject(0){|acc,i|acc +i} + if !times.empty? + sum = times.inject(0) {|acc, i| acc + i} avg = sum / times.length.to_f puts "%d replies max: %.2f min: %.2f avg: %.2f %s" % [times.size, times.max, times.min, avg, spark(times)] diff --git a/lib/mcollective/application/plugin.rb b/lib/mcollective/application/plugin.rb index 49c6879a..837c7427 100644 --- a/lib/mcollective/application/plugin.rb +++ b/lib/mcollective/application/plugin.rb @@ -1,6 +1,5 @@ module MCollective - class Application::Plugin 'Plugin name', - :arguments => ['-n', '--name NAME'], - :type => String + :description => "Plugin name", + :arguments => ["-n", "--name NAME"], + :type => String option :postinstall, - :description => 'Post install script', - :arguments => ['--postinstall POSTINSTALL'], - :type => String + :description => "Post install script", + :arguments => ["--postinstall POSTINSTALL"], + :type => String option :preinstall, - :description => 'Pre install script', - :arguments => ['--preinstall PREINSTALL'], - :type => String + :description => "Pre install script", + :arguments => ["--preinstall PREINSTALL"], + :type => String option :revision, - :description => 'Revision number', - :arguments => ['--revision REVISION'], - :type => String + :description => "Revision number", + :arguments => ["--revision REVISION"], + :type => String option :iteration, - :description => 'DEPRECATED - Use --revision instead', - :arguments => ['--iteration ITERATION'], - :type => String + :description => "DEPRECATED - Use --revision instead", + :arguments => ["--iteration ITERATION"], + :type => String option :vendor, - :description => 'Vendor name', - :arguments => ['--vendor VENDOR'], - :type => String + :description => "Vendor name", + :arguments => ["--vendor VENDOR"], + :type => String option :pluginpath, - :description => 'MCollective plugin path', - :arguments => ['--pluginpath PATH'], - :type => String + :description => "MCollective plugin path", + :arguments => ["--pluginpath PATH"], + :type => String option :mcname, - :description => 'MCollective type (mcollective, pe-mcollective) that the packages depend on', - :arguments => ['--mcname NAME'], - :type => String + :description => "MCollective type (mcollective, pe-mcollective) that the packages depend on", + :arguments => ["--mcname NAME"], + :type => String option :mcversion, - :description => 'Version of MCollective that the packages depend on', - :arguments => ['--mcversion MCVERSION'], - :type => String + :description => "Version of MCollective that the packages depend on", + :arguments => ["--mcversion MCVERSION"], + :type => String option :dependency, - :description => 'Adds a dependency to the plugin', - :arguments => ['--dependency DEPENDENCIES'], - :type => :array + :description => "Adds a dependency to the plugin", + :arguments => ["--dependency DEPENDENCIES"], + :type => :array option :format, - :description => 'Package output format. Defaults to rpmpackage or debpackage', - :arguments => ['--format OUTPUTFORMAT'], - :type => String + :description => "Package output format. Defaults to rpmpackage or debpackage", + :arguments => ["--format OUTPUTFORMAT"], + :type => String option :sign, - :description => 'Embed a signature in the package', - :arguments => ['--sign'], - :type => :boolean + :description => "Embed a signature in the package", + :arguments => ["--sign"], + :type => :boolean option :rpctemplate, - :description => 'Template to use.', - :arguments => ['--template HELPTEMPLATE'], - :type => String + :description => "Template to use.", + :arguments => ["--template HELPTEMPLATE"], + :type => String option :description, - :description => 'Plugin description', - :arguments => ['--description DESCRIPTION'], - :type => String + :description => "Plugin description", + :arguments => ["--description DESCRIPTION"], + :type => String option :author, - :description => 'The author of the plugin', - :arguments => ['--author AUTHOR'], - :type => String + :description => "The author of the plugin", + :arguments => ["--author AUTHOR"], + :type => String option :license, - :description => 'The license under which the plugin is distributed', - :arguments => ['--license LICENSE'], - :type => String + :description => "The license under which the plugin is distributed", + :arguments => ["--license LICENSE"], + :type => String option :version, - :description => 'The version of the plugin', - :arguments => ['--pluginversion VERSION'], - :type => String + :description => "The version of the plugin", + :arguments => ["--pluginversion VERSION"], + :type => String option :url, - :description => 'Url at which information about the plugin can be found', - :arguments => ['--url URL'], - :type => String + :description => "Url at which information about the plugin can be found", + :arguments => ["--url URL"], + :type => String option :timeout, :description => "The plugin's timeout", - :arguments => ['--timeout TIMEOUT'], - :type => Integer + :arguments => ["--timeout TIMEOUT"], + :type => Integer option :actions, - :description => 'Actions to be generated for an Agent Plugin', - :arguments => ['--actions [ACTIONS]'], - :type => Array + :description => "Actions to be generated for an Agent Plugin", + :arguments => ["--actions [ACTIONS]"], + :type => Array option :outputs, - :description => 'Outputs to be generated for an Data Plugin', - :arguments => ['--outputs [OUTPUTS]'], - :type => Array + :description => "Outputs to be generated for an Data Plugin", + :arguments => ["--outputs [OUTPUTS]"], + :type => Array option :keep_artifacts, :description => "Don't remove artifacts after building packages", - :arguments => ['--keep-artifacts'], - :type => :boolean + :arguments => ["--keep-artifacts"], + :type => :boolean option :module_template, :description => "Path to the template used by the modulepackager", - :arguments => ['--module-template PATH'], - :type => String + :arguments => ["--module-template PATH"], + :type => String # Handle alternative format that optparser can't parse. def post_option_parser(configuration) @@ -141,16 +140,16 @@ def post_option_parser(configuration) if configuration[:action] == "generate" unless ARGV[0] && ARGV[0].match(/(actions|outputs)=(.+)/i) - unless configuration[:pluginname] - configuration[:pluginname] = ARGV.delete_at(0) - else + if configuration[:pluginname] # rubocop:disable Metrics/BlockNesting ARGV.delete_at(0) + else + configuration[:pluginname] = ARGV.delete_at(0) end end ARGV.each do |argument| if argument.match(/(actions|outputs)=(.+)/i) - configuration[$1.downcase.to_sym]= $2.split(",") + configuration[$1.downcase.to_sym] = $2.split(",") else raise "Could not parse --arg '#{argument}'" end @@ -168,7 +167,7 @@ def info_command # Generate a plugin skeleton def generate_command - raise "undefined plugin type. cannot generate plugin. valid types are 'agent' and 'data'" if configuration["target"] == '.' + raise "undefined plugin type. cannot generate plugin. valid types are 'agent' and 'data'" if configuration["target"] == "." unless configuration[:pluginname] puts "No plugin name specified. Using 'new_plugin'" @@ -178,15 +177,16 @@ def generate_command load_plugin_config_values case configuration[:target].downcase - when 'agent' + when "agent" Generators::AgentGenerator.new(configuration[:pluginname], configuration[:actions], configuration[:pluginname], configuration[:description], configuration[:author], configuration[:license], configuration[:version], configuration[:url], configuration[:timeout]) - when 'data' + when "data" raise "data plugin must have at least one output" unless configuration[:outputs] + Generators::DataGenerator.new(configuration[:pluginname], configuration[:outputs], configuration[:pluginname], - configuration[:description], configuration[:author], configuration[:license], - configuration[:version], configuration[:url], configuration[:timeout]) + configuration[:description], configuration[:author], configuration[:license], + configuration[:version], configuration[:url], configuration[:timeout]) else raise "invalid plugin type. cannot generate plugin '#{configuration[:target]}'" end @@ -196,11 +196,11 @@ def generate_command def package_command if configuration[:sign] && Config.instance.pluginconf.include?("debian_packager.keyname") configuration[:sign] = Config.instance.pluginconf["debian_packager.keyname"] - configuration[:sign] = "\"#{configuration[:sign]}\"" unless configuration[:sign].match(/\".*\"/) + configuration[:sign] = "\"#{configuration[:sign]}\"" unless configuration[:sign].match(/".*"/) end plugin = prepare_plugin - (configuration[:pluginpath] = configuration[:pluginpath] + "/") if (configuration[:pluginpath] && !configuration[:pluginpath].match(/^.*\/$/)) + (configuration[:pluginpath] = "#{configuration[:pluginpath]}/") if configuration[:pluginpath] && !configuration[:pluginpath].match(/^.*\/$/) packager = PluginPackager["#{configuration[:format].capitalize}Packager"] packager.new(plugin, configuration[:pluginpath], configuration[:sign], options[:verbose], configuration[:keep_artifacts], @@ -219,11 +219,11 @@ def load_plugin_ddl(plugin, type) end end - return nil + nil end # Show application list and plugin help - def doc_command + def doc_command # rubocop:disable Metrics/MethodLength known_plugin_types = [ ["Agents", :agent], ["Aggregate", :aggregate], @@ -240,12 +240,12 @@ def doc_command found_plugin_type = nil known_plugin_types.each do |plugin_type| - PluginManager.find(plugin_type[1], "ddl").each do |ddl| - pluginname = ddl.gsub(/_#{plugin_type[1]}$/, "") - if pluginname == configuration[:target] - abort "Duplicate plugin name found, please specify a full path like agent/rpcutil" if found_plugin_type - found_plugin_type = plugin_type[1] - end + PluginManager.find(plugin_type[1], "ddl").each do |ddlf| + pluginname = ddlf.gsub(/_#{plugin_type[1]}$/, "") + + abort "Duplicate plugin name found, please specify a full path like agent/rpcutil" if pluginname == configuration[:target] && found_plugin_type + + found_plugin_type = plugin_type[1] if pluginname == configuration[:target] end end @@ -256,7 +256,7 @@ def doc_command if ddl puts ddl.help(configuration[:rpctemplate]) else - abort "Could not find a '%s' plugin named '%s'" % configuration[:target].split('/') + abort "Could not find a '%s' plugin named '%s'" % configuration[:target].split("/") end else @@ -268,17 +268,17 @@ def doc_command known_plugin_types.each do |plugin_type| puts "%s:" % plugin_type[0] - PluginManager.find(plugin_type[1], "ddl").each do |ddl| + PluginManager.find(plugin_type[1], "ddl").each do |ddlf| begin - help = DDL.new(ddl, plugin_type[1], false) + help = DDL.new(ddlf, plugin_type[1], false) next unless help.client_activated? help.loadddlfile - pluginname = ddl.gsub(/_#{plugin_type[1]}$/, "") + pluginname = ddlf.gsub(/_#{plugin_type[1]}$/, "") puts " %-25s %s" % [pluginname, help.meta[:description]] rescue => e - load_errors << [plugin_type[1], ddl, e] + load_errors << [plugin_type[1], ddlf, e] end end @@ -305,7 +305,7 @@ def prepare_plugin if configuration[:dependency] && configuration[:dependency].size == 1 configuration[:dependency] = configuration[:dependency][0].split(" ") elsif configuration[:dependency] - configuration[:dependency].map!{|dep| {:name => dep, :version => nil}} + configuration[:dependency].map! {|dep| {:name => dep, :version => nil}} end mcdependency = { @@ -315,7 +315,7 @@ def prepare_plugin # Deprecation warning for --iteration if configuration[:iteration] - puts 'Warning. The --iteration flag has been deprecated. Please use --revision instead.' + puts "Warning. The --iteration flag has been deprecated. Please use --revision instead." configuration[:revision] = configuration[:iteration] unless configuration[:revision] end @@ -330,12 +330,12 @@ def plugin_directory_exists?(plugin_type) def set_plugin_type if plugin_directory_exists?("agent") || plugin_directory_exists?("application") configuration[:plugintype] = "AgentDefinition" - return "Agent" + "Agent" elsif plugin_directory_exists?(plugintype = identify_plugin) configuration[:plugintype] = "StandardDefinition" - return plugintype + plugintype else - raise RuntimeError, "target directory is not a valid mcollective plugin" + raise "target directory is not a valid mcollective plugin" end end @@ -348,8 +348,8 @@ def identify_plugin File.directory?(file) && file.match(/(connector|facts|registration|security|audit|pluginpackager|data|discovery|validator)/) end - raise RuntimeError, "more than one plugin type detected in directory" if plugintype.size > 1 - raise RuntimeError, "no plugins detected in directory" if plugintype.size < 1 + raise "more than one plugin type detected in directory" if plugintype.size > 1 + raise "no plugins detected in directory" if plugintype.empty? File.basename(plugintype[0]) end @@ -367,15 +367,15 @@ def load_plugin_config_values end def main - abort "No action specified, please run 'mco help plugin' for help" unless configuration.include?(:action) + abort "No action specified, please run 'mco help plugin' for help" unless configuration.include?(:action) - cmd = "#{configuration[:action]}_command" + cmd = "#{configuration[:action]}_command" - if respond_to? cmd - send cmd - else - abort "Invalid action #{configuration[:action]}, please run 'mco help plugin' for help." - end + if respond_to? cmd + send cmd + else + abort "Invalid action #{configuration[:action]}, please run 'mco help plugin' for help." + end end end end diff --git a/lib/mcollective/application/rpc.rb b/lib/mcollective/application/rpc.rb index 330a5b38..c6a4a36c 100644 --- a/lib/mcollective/application/rpc.rb +++ b/lib/mcollective/application/rpc.rb @@ -1,33 +1,33 @@ -class MCollective::Application::Rpc --action [--argument --argument ...]" usage "mco rpc [options] [filters] [ ...]" option :show_results, - :description => "Do not process results, just send request", - :arguments => ["--no-results", "--nr"], - :default => true, - :type => :bool + :description => "Do not process results, just send request", + :arguments => ["--no-results", "--nr"], + :default => true, + :type => :bool option :agent, - :description => "Agent to call", - :arguments => ["-a", "--agent AGENT"] + :description => "Agent to call", + :arguments => ["-a", "--agent AGENT"] option :action, - :description => "Action to call", - :arguments => ["--action ACTION"] + :description => "Action to call", + :arguments => ["--action ACTION"] option :arguments, - :description => "Arguments to pass to agent", - :arguments => ["--arg", "--argument ARGUMENT"], - :type => :array, - :default => [], - :validate => Proc.new {|val| val.match(/^(.+?)=(.+)$/) ? true : "Could not parse --arg #{val} should be of the form key=val" } + :description => "Arguments to pass to agent", + :arguments => ["--arg", "--argument ARGUMENT"], + :type => :array, + :default => [], + :validate => proc {|val| val.match(/^(.+?)=(.+)$/) ? true : "Could not parse --arg #{val} should be of the form key=val" } def post_option_parser(configuration) # handle the alternative format that optparse cant parse - unless (configuration.include?(:agent) && configuration.include?(:action)) + unless configuration.include?(:agent) && configuration.include?(:action) if ARGV.length >= 2 configuration[:agent] = ARGV[0] ARGV.delete_at(0) @@ -37,15 +37,15 @@ def post_option_parser(configuration) ARGV.each do |v| if v =~ /^(.+?)=(.+)$/ - configuration[:arguments] = [] unless configuration.include?(:arguments) + configuration[:arguments] ||= [] configuration[:arguments] << v else - STDERR.puts("Could not parse --arg #{v}") + warn("Could not parse --arg #{v}") exit(1) end end else - STDERR.puts("No agent, action and arguments specified") + warn("No agent, action and arguments specified") exit(1) end end @@ -55,24 +55,22 @@ def post_option_parser(configuration) configuration[:arguments] = {} args.each do |v| - if v =~ /^(.+?)=(.+)$/ - configuration[:arguments][$1.to_sym] = $2 - end + configuration[:arguments][$1.to_sym] = $2 if v =~ /^(.+?)=(.+)$/ end end def string_to_ddl_type(arguments, ddl) return if ddl.empty? - arguments.keys.each do |key| - if ddl[:input].keys.include?(key) - case ddl[:input][key][:type] - when :boolean - arguments[key] = MCollective::DDL.string_to_boolean(arguments[key]) + arguments.each_key do |key| + next unless ddl[:input].keys.include?(key) - when :number, :integer, :float - arguments[key] = MCollective::DDL.string_to_number(arguments[key]) - end + case ddl[:input][key][:type] + when :boolean + arguments[key] = MCollective::DDL.string_to_boolean(arguments[key]) + + when :number, :integer, :float + arguments[key] = MCollective::DDL.string_to_number(arguments[key]) end end end @@ -89,11 +87,11 @@ def main if mc.reply_to configuration[:arguments][:process_results] = true - puts "Request sent with id: " + mc.send(configuration[:action], configuration[:arguments]) + " replies to #{mc.reply_to}" + puts "Request sent with id: %s replies to %s" % [mc.send(configuration[:action], configuration[:arguments]), mc.reply_to] elsif !configuration[:show_results] configuration[:arguments][:process_results] = false - puts "Request sent with id: " + mc.send(configuration[:action], configuration[:arguments]) + puts "Request sent with id: #{mc.send(configuration[:action], configuration[:arguments])}" else discover_args = {:verbose => true} @@ -103,7 +101,7 @@ def main printrpc mc.send(configuration[:action], configuration[:arguments]) - printrpcstats :summarize => true, :caption => "#{configuration[:agent]}##{configuration[:action]} call stats" if mc.discover.size > 0 + printrpcstats :summarize => true, :caption => "#{configuration[:agent]}##{configuration[:action]} call stats" unless mc.discover.empty? halt mc.stats end diff --git a/lib/mcollective/applications.rb b/lib/mcollective/applications.rb index 76b62f9c..000566d1 100644 --- a/lib/mcollective/applications.rb +++ b/lib/mcollective/applications.rb @@ -12,11 +12,11 @@ def self.run(appname) load_application(appname) rescue Exception => e # rubocop:disable Lint/RescueException e.backtrace.first << Util.colorize(:red, " <----") - STDERR.puts "Application '%s' failed to load:" % appname - STDERR.puts - STDERR.puts Util.colorize(:red, " %s (%s)" % [$!, $!.class]) - STDERR.puts - STDERR.puts " %s" % [$!.backtrace.join("\n ")] + warn "Application '%s' failed to load:" % appname + $stderr.puts + warn Util.colorize(:red, " %s (%s)" % [$!, $!.class]) + $stderr.puts + warn " %s" % [$!.backtrace.join("\n ")] exit 1 end @@ -40,7 +40,7 @@ def self.list rescue SystemExit exit 1 rescue Exception # rubocop:disable Lint/RescueException - STDERR.puts("Failed to generate application list: %s: %s" % [$!.class, $!]) + warn("Failed to generate application list: %s: %s" % [$!.class, $!]) exit 1 end @@ -49,13 +49,14 @@ def self.list def self.filter_extra_options(opts) words = Shellwords.shellwords(opts) words.each_with_index do |word, idx| - if word == "-c" + case word + when "-c" return "--config=#{words[idx + 1]}" - elsif word == "--config" + when "--config" return "--config=#{words[idx + 1]}" - elsif word =~ /\-c=/ + when /-c=/ return word - elsif word =~ /\-\-config=/ + when /--config=/ return word end end diff --git a/lib/mcollective/cache.rb b/lib/mcollective/cache.rb index 4c16dbb3..658223a2 100644 --- a/lib/mcollective/cache.rb +++ b/lib/mcollective/cache.rb @@ -122,14 +122,12 @@ def self.ttl(cache_name, key) end end - def self.synchronize(cache_name) + def self.synchronize(cache_name, &block) raise("No cache called '%s'" % cache_name) unless @cache.include?(cache_name) raise("No block supplied to synchronize") unless block_given? - @cache_locks[cache_name].synchronize do - yield - end + @cache_locks[cache_name].synchronize(&block) end def self.invalidate!(cache_name, key) diff --git a/lib/mcollective/client.rb b/lib/mcollective/client.rb index e9d2973c..6c453739 100644 --- a/lib/mcollective/client.rb +++ b/lib/mcollective/client.rb @@ -7,10 +7,11 @@ def initialize(options) @config = Config.instance @options = nil - if options.is_a?(String) + case options + when String # String is the path to a config file @config.loadconfig(options) unless @config.configured - elsif options.is_a?(Hash) + when Hash @config.loadconfig(options[:config]) unless @config.configured @options = options @connection_timeout = options[:connection_timeout] @@ -124,9 +125,7 @@ def receive(requestid=nil) reply.decode! - unless reply.requestid == requestid - raise(MsgDoesNotMatchRequestID, "Message reqid #{reply.requestid} does not match our reqid #{requestid}") - end + raise(MsgDoesNotMatchRequestID, "Message reqid #{reply.requestid} does not match our reqid #{requestid}") unless reply.requestid == requestid Log.debug("Received reply to #{reply.requestid} from #{reply.payload[:senderid]}") rescue SecurityValidationFailed => e @@ -171,7 +170,7 @@ def req(body, agent=nil, options=false, waitfor=[], &block) request = createreq(body, agent, @options[:filter]) publish_timeout = @options[:publish_timeout] || @config.publish_timeout stat = {:starttime => Time.now.to_f, :discoverytime => 0, :blocktime => 0, :totaltime => 0} - STDOUT.sync = true + $stdout.sync = true hosts_responded = 0 begin @@ -180,7 +179,7 @@ def req(body, agent=nil, options=false, waitfor=[], &block) else hosts_responded = unthreaded_req(request, publish_timeout, timeout, waitfor, &block) end - rescue Interrupt # rubocop:disable Lint/HandleExceptions + rescue Interrupt # rubocop:disable Lint/SuppressedException ensure unsubscribe(agent, :reply) end @@ -277,9 +276,7 @@ def start_receiver(requestid, waitfor, timeout, &block) end rescue Timeout::Error if waitfor.is_a?(Array) - unless unfinished.empty? - Log.warn("Could not receive all responses. Did not receive responses from #{unfinished.keys.join(', ')}") - end + Log.warn("Could not receive all responses. Did not receive responses from #{unfinished.keys.join(', ')}") unless unfinished.empty? elsif waitfor > hosts_responded Log.warn("Could not receive all responses. Expected : #{waitfor}. Received : #{hosts_responded}") end diff --git a/lib/mcollective/config.rb b/lib/mcollective/config.rb index d99f1106..28a109c4 100644 --- a/lib/mcollective/config.rb +++ b/lib/mcollective/config.rb @@ -5,19 +5,13 @@ class Config attr_accessor :mode - attr_reader :daemonize, :pluginconf, :configured - attr_reader :logfile, :keeplogs, :max_log_size, :loglevel, :logfacility - attr_reader :identity, :connector, :securityprovider, :factsource - attr_reader :registration, :registerinterval, :classesfile - attr_reader :rpcauditprovider, :rpcaudit, :configdir, :rpcauthprovider - attr_reader :rpcauthorization, :color, :configfile - attr_reader :rpclimitmethod, :logger_type, :fact_cache_time, :collectives - attr_reader :main_collective, :ssl_cipher, :registration_collective - attr_reader :direct_addressing, :direct_addressing_threshold, :ttl - attr_reader :default_discovery_method, :default_discovery_options - attr_reader :publish_timeout, :threaded, :soft_shutdown, :activate_agents - attr_reader :registration_splay, :discovery_timeout, :soft_shutdown_timeout - attr_reader :connection_timeout, :default_batch_size, :default_batch_sleep_time + attr_reader :daemonize, :pluginconf, :configured, :logfile, :keeplogs, :max_log_size, :loglevel, :logfacility, + :identity, :connector, :securityprovider, :factsource, :registration, :registerinterval, :classesfile, + :rpcauditprovider, :rpcaudit, :configdir, :rpcauthprovider, :rpcauthorization, :color, :configfile, + :rpclimitmethod, :logger_type, :fact_cache_time, :collectives, :main_collective, :ssl_cipher, :registration_collective, + :direct_addressing, :direct_addressing_threshold, :ttl, :default_discovery_method, :default_discovery_options, + :publish_timeout, :threaded, :soft_shutdown, :activate_agents, :registration_splay, :discovery_timeout, :soft_shutdown_timeout, + :connection_timeout, :default_batch_size, :default_batch_sleep_time def initialize @configured = false @@ -34,6 +28,7 @@ def loadconfig(configfile) # rubocop:disable Metrics/MethodLength next if line =~ /^#|^$/ next unless line =~ /(.+?)\s*=\s*(.+)/ + key = $1.strip val = $2 @@ -133,7 +128,7 @@ def loadconfig(configfile) # rubocop:disable Metrics/MethodLength read_plugin_config_dir("#{@configdir}/plugin.d") - raise 'Identities can only match /\w\.\-/' unless @identity =~ /^[\w\.\-]+$/ + raise 'Identities can only match /\w\.\-/' unless @identity =~ /^[\w.\-]+$/ @configured = true @@ -145,9 +140,7 @@ def loadconfig(configfile) # rubocop:disable Metrics/MethodLength $LOAD_PATH.unshift dir end - if @logger_type == "syslog" - raise "The sylog logger is not usable on the Windows platform" if Util.windows? - end + raise "The sylog logger is not usable on the Windows platform" if @logger_type == "syslog" && Util.windows? unless configfile =~ /server/ PluginManager.loadclass("Mcollective::Facts::#{@factsource}_facts") @@ -216,7 +209,7 @@ def read_plugin_config_dir(dir) return unless File.directory?(dir) Dir.new(dir).each do |pluginconfigfile| - next unless pluginconfigfile =~ /^([\w]+).cfg$/ + next unless pluginconfigfile =~ /^(\w+).cfg$/ plugin = $1 File.open("#{dir}/#{pluginconfigfile}", "r").each do |line| @@ -224,6 +217,7 @@ def read_plugin_config_dir(dir) line.gsub!(/\s*$/, "") next if line =~ /^#|^$/ next unless line =~ /(.+?)\s*=\s*(.+)/ + key = $1.strip val = $2 @pluginconf["#{plugin}.#{key}"] = val diff --git a/lib/mcollective/connector/base.rb b/lib/mcollective/connector/base.rb index 8399f417..38ed2816 100644 --- a/lib/mcollective/connector/base.rb +++ b/lib/mcollective/connector/base.rb @@ -18,8 +18,9 @@ module Connector class Base def self.inherited(klass) plugin_name = klass.to_s.split("::").last.downcase - ddl = DDL.new(plugin_name, :connector) + DDL.new(plugin_name, :connector) PluginManager << {:type => "connector_plugin", :class => klass.to_s} + super end end end diff --git a/lib/mcollective/data.rb b/lib/mcollective/data.rb index fcfcd9df..06ae48a5 100644 --- a/lib/mcollective/data.rb +++ b/lib/mcollective/data.rb @@ -28,12 +28,16 @@ def self.[](plugin) end # Data.package("httpd").architecture - def self.method_missing(method, *args) # rubocop:disable Style/MethodMissing: + def self.method_missing(method, *args) super unless PluginManager.include?(pluginname(method)) PluginManager[pluginname(method)].lookup(args.first) end + def self.respond_to_missing?(method, *) + PluginManager.include?(pluginname(method)) || super + end + def self.ddl(plugin) DDL.new(pluginname(plugin), :data) end @@ -55,7 +59,8 @@ def self.ddl_validate(ddl, argument) ddl.validate_input_argument(input, :query, argument) else raise("No data plugin argument was declared in the %s DDL but an input was supplied" % name) if argument - return true + + true end end @@ -82,7 +87,7 @@ def self.ddl_transform_input(ddl, input) when :number, :integer, :float return DDL.string_to_number(input) end - rescue # rubocop:disable Lint/HandleExceptions + rescue # rubocop:disable Lint/SuppressedException end input diff --git a/lib/mcollective/data/agent_data.rb b/lib/mcollective/data/agent_data.rb index 8a04d376..cc598920 100644 --- a/lib/mcollective/data/agent_data.rb +++ b/lib/mcollective/data/agent_data.rb @@ -1,6 +1,6 @@ module MCollective module Data - class Agent_data type, :class => klass.to_s, :single_instance => false} + super end def initialize @@ -22,9 +23,9 @@ def initialize def lookup(what) ddl_validate(what) - Log.debug("Doing data query %s for '%s'" % [ @name, what ]) + Log.debug("Doing data query %s for '%s'" % [@name, what]) - Timeout::timeout(@timeout) do + Timeout.timeout(@timeout) do query_data(what) end @@ -40,7 +41,7 @@ def lookup(what) end def self.query(&block) - self.module_eval { define_method("query_data", &block) } + module_eval { define_method("query_data", &block) } end def ddl_validate(what) @@ -58,10 +59,10 @@ def self.activate_when(&block) # Always be active unless a specific block is given with activate_when def self.activate? - return true + true end - def startup_hook;end + def startup_hook; end end end end diff --git a/lib/mcollective/data/collective_data.rb b/lib/mcollective/data/collective_data.rb index c4b4496f..ca2b0f3a 100644 --- a/lib/mcollective/data/collective_data.rb +++ b/lib/mcollective/data/collective_data.rb @@ -1,6 +1,6 @@ module MCollective module Data - class Collective_data nil}) # Sets the display preference to either :ok, :failed, :flatten or :always # operates on action level def display(pref) - if pref == :flatten - Log.warn("The display option :flatten is being deprecated and will be removed in the next minor release.") - end + Log.warn("The display option :flatten is being deprecated and will be removed in the next minor release.") if pref == :flatten # defaults to old behavior, complain if its supplied and invalid - unless [:ok, :failed, :flatten, :always].include?(pref) - raise "Display preference #{pref} is not valid, should be :ok, :failed, :flatten or :always" - end + raise "Display preference #{pref} is not valid, should be :ok, :failed, :flatten or :always" unless [:ok, :failed, :flatten, :always].include?(pref) action = @current_entity @entities[action][:display] = pref @@ -223,23 +219,15 @@ def symbolize_basic_input_arguments(input, arguments) # agent should be allowed based on action name and inputs. def validate_rpc_request(action, arguments) # is the action known? - unless actions.include?(action) - raise DDLValidationError, "Attempted to call action #{action} for #{@pluginname} but it's not declared in the DDL" - end + raise DDLValidationError, "Attempted to call action #{action} for #{@pluginname} but it's not declared in the DDL" unless actions.include?(action) input = action_interface(action)[:input] || {} compatible_args = symbolize_basic_input_arguments(input, arguments) input.each_key do |key| - unless input[key][:optional] - unless compatible_args.include?(key) - raise DDLValidationError, "Action #{action} needs a #{key} argument" - end - end + raise DDLValidationError, "Action #{action} needs a #{key} argument" if !input[key][:optional] && !compatible_args.include?(key) - if compatible_args.include?(key) - validate_input_argument(input, key, compatible_args[key]) - end + validate_input_argument(input, key, compatible_args[key]) if compatible_args.include?(key) end true diff --git a/lib/mcollective/ddl/base.rb b/lib/mcollective/ddl/base.rb index 96c295bf..29d79263 100644 --- a/lib/mcollective/ddl/base.rb +++ b/lib/mcollective/ddl/base.rb @@ -64,6 +64,7 @@ def help(template=nil) def usage(usage_text) @usage = usage_text end + # rubocop:enable Lint/DuplicateMethods, Style/TrivialAccessors def template_for_plugintype case @plugintype @@ -80,9 +81,7 @@ def template_for_plugintype end def loadddlfile - if @config.mode == :client && !client_activated? - raise("%s/%s is disabled, cannot load DDL file" % [@plugintype, @pluginname]) - end + raise("%s/%s is disabled, cannot load DDL file" % [@plugintype, @pluginname]) if @config.mode == :client && !client_activated? if ddlfile = findddlfile instance_eval(File.read(ddlfile), ddlfile, 1) @@ -111,10 +110,10 @@ def client_activated? end def validate_requirements - if requirement = @requirements[:mcollective] - if Util.versioncmp(Util.mcollective_version, requirement) < 0 - raise DDLValidationError, "%s plugin '%s' requires MCollective version %s or newer" % [@plugintype.to_s.capitalize, @pluginname, requirement] - end + requirement = @requirements[:mcollective] + + if requirement && (Util.versioncmp(Util.mcollective_version, requirement) < 0) + raise DDLValidationError, "%s plugin '%s' requires MCollective version %s or newer" % [@plugintype.to_s.capitalize, @pluginname, requirement] end true @@ -147,7 +146,7 @@ def validate_input_argument(input, key, argument) Validator.validate(argument, input[key][:type]) end - return true + true rescue => e raise DDLValidationError, "Cannot validate input %s: %s" % [key, e.to_s], e.backtrace end @@ -196,8 +195,8 @@ def output(argument, properties) action = @current_entity @entities[action][:output][argument] = {:description => properties[:description], - :display_as => properties[:display_as], - :default => properties[:default]} + :display_as => properties[:display_as], + :default => properties[:default]} @entities[action][:output][argument][:type] = properties[:type] if properties[:type] end @@ -208,9 +207,7 @@ def requires(requirement) valid_requirements = [:mcollective] requirement.each_key do |key| - unless valid_requirements.include?(key) - raise "Requirement %s is not a valid requirement, only %s is supported" % [key, valid_requirements.join(", ")] - end + raise "Requirement %s is not a valid requirement, only %s is supported" % [key, valid_requirements.join(", ")] unless valid_requirements.include?(key) @requirements[key] = requirement[key] end diff --git a/lib/mcollective/discovery.rb b/lib/mcollective/discovery.rb index 0858cda0..408d3e25 100644 --- a/lib/mcollective/discovery.rb +++ b/lib/mcollective/discovery.rb @@ -29,9 +29,7 @@ def discovery_method raise "Unknown discovery method %s" % method unless has_method?(method) - unless method == "mc" - raise "Custom discovery methods require direct addressing mode" unless Config.instance.direct_addressing - end + raise "Custom discovery methods require direct addressing mode" if method != "mc" && !Config.instance.direct_addressing method end @@ -49,9 +47,7 @@ def ddl # if the discovery method got changed we might have an old DDL cached # this will detect that and reread the correct DDL from disk - unless @ddl.meta[:name] == discovery_method - @ddl = DDL.new(discovery_method, :discovery) - end + @ddl = DDL.new(discovery_method, :discovery) unless @ddl.meta[:name] == discovery_method @ddl end @@ -62,32 +58,22 @@ def ddl def check_capabilities(filter) capabilities = ddl.discovery_interface[:capabilities] - unless capabilities.include?(:classes) - raise "Cannot use class filters while using the '%s' discovery method" % discovery_method unless filter["cf_class"].empty? - end + raise "Cannot use class filters while using the '%s' discovery method" % discovery_method if !capabilities.include?(:classes) && !filter["cf_class"].empty? - unless capabilities.include?(:facts) - raise "Cannot use fact filters while using the '%s' discovery method" % discovery_method unless filter["fact"].empty? - end + raise "Cannot use fact filters while using the '%s' discovery method" % discovery_method if !capabilities.include?(:facts) && !filter["fact"].empty? - unless capabilities.include?(:identity) - raise "Cannot use identity filters while using the '%s' discovery method" % discovery_method unless filter["identity"].empty? - end + raise "Cannot use identity filters while using the '%s' discovery method" % discovery_method if !capabilities.include?(:identity) && !filter["identity"].empty? - unless capabilities.include?(:compound) - raise "Cannot use compound filters while using the '%s' discovery method" % discovery_method unless filter["compound"].empty? - end + raise "Cannot use compound filters while using the '%s' discovery method" % discovery_method if !capabilities.include?(:compound) && !filter["compound"].empty? end # checks if compound filters are used and then forces the 'mc' discovery plugin def force_discovery_method_by_filter(filter) - unless discovery_method == "mc" - unless filter["compound"].empty? - Log.info "Switching to mc discovery method because compound filters are used" - @client.options[:discovery_method] = "mc" + if discovery_method != "mc" && !filter["compound"].empty? + Log.info "Switching to mc discovery method because compound filters are used" + @client.options[:discovery_method] = "mc" - return true - end + return true end false @@ -134,9 +120,9 @@ def discover(filter, timeout, limit) discovered = discovery_class.discover(filter, discovery_timeout(timeout, filter), limit, @client) if limit > 0 - return discovered[0, limit] + discovered[0, limit] else - return discovered + discovered end end end diff --git a/lib/mcollective/discovery/flatfile.rb b/lib/mcollective/discovery/flatfile.rb index ffc33bcd..aca719f2 100644 --- a/lib/mcollective/discovery/flatfile.rb +++ b/lib/mcollective/discovery/flatfile.rb @@ -5,10 +5,10 @@ module MCollective class Discovery class Flatfile def self.discover(filter, timeout, limit=0, client=nil) - unless client.options[:discovery_options].empty? - file = client.options[:discovery_options].first - else + if client.options[:discovery_options].empty? raise "The flatfile discovery method needs a path to a text file" + else + file = client.options[:discovery_options].first end raise "Cannot read the file %s specified as discovery source" % file unless File.readable?(file) @@ -18,16 +18,15 @@ def self.discover(filter, timeout, limit=0, client=nil) File.readlines(file).each do |host| host = host.chomp.strip - if host.empty? || host.match(/^#/) - next - end - raise 'Identities can only match /^[\w\.\-]+$/' unless host.match(/^[\w\.\-]+$/) + next if host.empty? || host.match(/^#/) + raise 'Identities can only match /^[\w\.\-]+$/' unless host.match(/^[\w.\-]+$/) + hosts << host end # this plugin only supports identity filters, do regex matches etc against # the list found in the flatfile - if !(filter["identity"].empty?) + if !filter["identity"].empty? filter["identity"].each do |identity| identity = Regexp.new(identity.gsub("\/", "")) if identity.match("^/") diff --git a/lib/mcollective/discovery/mc.rb b/lib/mcollective/discovery/mc.rb index efd1d828..55dce9f4 100644 --- a/lib/mcollective/discovery/mc.rb +++ b/lib/mcollective/discovery/mc.rb @@ -16,8 +16,8 @@ def self.discover(filter, timeout, limit, client) return hosts if limit > 0 && hosts.size == limit end end - rescue Timeout::Error => e - rescue Exception => e + rescue Timeout::Error # rubocop:disable Lint/SuppressedException + rescue Exception # rubocop:disable Lint/RescueException raise ensure client.unsubscribe("discovery", :reply) diff --git a/lib/mcollective/discovery/stdin.rb b/lib/mcollective/discovery/stdin.rb index 006ed61f..ef3d86de 100644 --- a/lib/mcollective/discovery/stdin.rb +++ b/lib/mcollective/discovery/stdin.rb @@ -1,52 +1,54 @@ # discovers against stdin instead of the traditional network discovery # the input must be a flat file with a node name per line which should match identities as configured, # or it should be a json string as output by the -j option of mco rpc -require 'mcollective/rpc/helpers' +require "mcollective/rpc/helpers" module MCollective class Discovery class Stdin def self.discover(filter, timeout, limit=0, client=nil) - unless client.options[:discovery_options].empty? - type = client.options[:discovery_options].first.downcase + if client.options[:discovery_options].empty? + type = "auto" else - type = 'auto' + type = client.options[:discovery_options].first.downcase end discovered = [] - file = STDIN.read + file = $stdin.read - if file =~ /^\s*$/ - raise("data piped on STDIN contained only whitespace - could not discover hosts from it.") - end + raise("data piped on STDIN contained only whitespace - could not discover hosts from it.") if file =~ /^\s*$/ - if type == 'auto' + if type == "auto" if file =~ /^\s*\[/ - type = 'json' + type = "json" else - type = 'text' + type = "text" end end Log.debug("Parsing STDIN input as type %s" % type) - if type == 'json' + case type + when "json" hosts = RPC::Helpers.extract_hosts_from_json(file) - elsif type == 'text' + when "text" hosts = file.split("\n") else raise("stdin discovery plugin only knows the types auto/text/json, not \"#{type}\"") end hosts.map do |host| - raise 'Identities can only match /\w\.\-/' unless host.match(/^[\w\.\-]+$/) + raise 'Identities can only match /\w\.\-/' unless host.match(/^[\w.\-]+$/) + host end # this plugin only supports identity filters, do regex matches etc against # the list found in the flatfile - unless filter["identity"].empty? + if filter["identity"].empty? + discovered = hosts + else filter["identity"].each do |identity| identity = Regexp.new(identity.gsub("\/", "")) if identity.match("^/") @@ -56,8 +58,6 @@ def self.discover(filter, timeout, limit=0, client=nil) discovered << identity end end - else - discovered = hosts end discovered @@ -65,4 +65,3 @@ def self.discover(filter, timeout, limit=0, client=nil) end end end - diff --git a/lib/mcollective/exceptions.rb b/lib/mcollective/exceptions.rb index 90318ccd..531a4b1a 100644 --- a/lib/mcollective/exceptions.rb +++ b/lib/mcollective/exceptions.rb @@ -1,12 +1,19 @@ module MCollective # Exceptions for the RPC system class DDLValidationError < RuntimeError; end + class ValidatorError < RuntimeError; end + class ClientTimeoutError < RuntimeError; end + class MsgDoesNotMatchRequestID < RuntimeError; end + class MsgTTLExpired < RuntimeError; end + class NotTargettedAtUs < RuntimeError; end + class RPCError < StandardError; end + class SecurityValidationFailed < RuntimeError; end class BackoffSuggestion < StandardError @@ -14,15 +21,21 @@ class BackoffSuggestion < StandardError def initialize(backoff=nil) @backoff = backoff + super end end class MessageNotReceived < BackoffSuggestion; end + class UnexpectedMessageType < BackoffSuggestion; end class InvalidRPCData < RPCError; end + class MissingRPCData < RPCError; end + class RPCAborted < RPCError; end + class UnknownRPCAction < RPCError; end + class UnknownRPCError < RPCError; end end diff --git a/lib/mcollective/facts/base.rb b/lib/mcollective/facts/base.rb index 50692d3c..3e24151a 100644 --- a/lib/mcollective/facts/base.rb +++ b/lib/mcollective/facts/base.rb @@ -17,6 +17,7 @@ def initialize # Registers new fact sources into the plugin manager def self.inherited(klass) PluginManager << {:type => "facts_plugin", :class => klass.to_s} + super end # Returns the value of a single fact @@ -27,7 +28,7 @@ def get_fact(fact=nil) @mutex.synchronize do begin - if (Time.now.to_i - @last_facts_load > cache_time.to_i ) || force_reload? + if (Time.now.to_i - @last_facts_load > cache_time.to_i) || force_reload? Log.debug("Resetting facter cache, now: #{Time.now.to_i} last-known-good: #{@last_facts_load}") tfacts = load_facts_from_source @@ -42,7 +43,7 @@ def get_fact(fact=nil) else Log.debug("Using cached facts now: #{Time.now.to_i} last-known-good: #{@last_facts_load}") end - rescue Exception => e + rescue Exception => e # rubocop:disable Lint/RescueException Log.error("Failed to load facts: #{e.class}: #{e}") # Avoid loops where failing fact loads cause huge CPU @@ -55,17 +56,16 @@ def get_fact(fact=nil) end end - # If you do not supply a specific fact all facts will be returned if fact.nil? - return @facts + @facts else @facts.include?(fact) ? @facts[fact] : nil end end # Returns all facts - def get_facts + def get_facts # rubocop:disable Naming/AccessorMethodName get_fact(nil) end @@ -84,15 +84,15 @@ def force_reload? def normalize_facts(value) case value when Array - return value.map { |v| normalize_facts(v) } + value.map { |v| normalize_facts(v) } when Hash new_hash = {} - value.each do |k,v| + value.each do |k, v| new_hash[k.to_s] = normalize_facts(v) end - return new_hash + new_hash else - return value.to_s + value.to_s end end end diff --git a/lib/mcollective/facts/yaml_facts.rb b/lib/mcollective/facts/yaml_facts.rb index 9161c7bd..8ecf180a 100644 --- a/lib/mcollective/facts/yaml_facts.rb +++ b/lib/mcollective/facts/yaml_facts.rb @@ -1,13 +1,13 @@ module MCollective module Facts - require 'yaml' + require "yaml" # A factsource that reads a hash of facts from a YAML file # # Multiple files can be specified seperated with a : in the # config file, they will be merged with later files overriding # earlier ones in the list. - class Yaml_facts e + rescue Exception => e # rubocop:disable Lint/RescueException Log.error("Failed to load yaml facts from #{file}: #{e.class}: #{e}") end end @@ -49,13 +49,13 @@ def force_reload? @yaml_file_mtimes[file] ||= File.stat(file).mtime mtime = File.stat(file).mtime - if mtime > @yaml_file_mtimes[file] - @yaml_file_mtimes[file] = mtime + next unless mtime > @yaml_file_mtimes[file] - Log.debug("Forcing fact reload due to age of #{file}") + @yaml_file_mtimes[file] = mtime - return true - end + Log.debug("Forcing fact reload due to age of #{file}") + + return true end false diff --git a/lib/mcollective/generators.rb b/lib/mcollective/generators.rb index ca7a5a49..6d88906b 100644 --- a/lib/mcollective/generators.rb +++ b/lib/mcollective/generators.rb @@ -1,7 +1,7 @@ module MCollective module Generators - require "mcollective/generators/base.rb" - require "mcollective/generators/data_generator.rb" - require "mcollective/generators/agent_generator.rb" + require "mcollective/generators/base" + require "mcollective/generators/data_generator" + require "mcollective/generators/agent_generator" end end diff --git a/lib/mcollective/generators/agent_generator.rb b/lib/mcollective/generators/agent_generator.rb index 577125ed..d7eb6089 100644 --- a/lib/mcollective/generators/agent_generator.rb +++ b/lib/mcollective/generators/agent_generator.rb @@ -1,11 +1,10 @@ module MCollective module Generators - class AgentGenerator name, :description => description, @@ -23,23 +24,21 @@ def create_plugin_string end def write_plugins - begin - Dir.mkdir @plugin_name - dirname = File.join(@plugin_name, @mod_name.downcase) - Dir.mkdir dirname - puts "Created plugin directory : #{@plugin_name}" + Dir.mkdir @plugin_name + dirname = File.join(@plugin_name, @mod_name.downcase) + Dir.mkdir dirname + puts "Created plugin directory : #{@plugin_name}" - File.open(File.join(dirname, "#{@plugin_name}.ddl"), "w"){|f| f.puts @ddl} - puts "Created DDL file : #{File.join(dirname, "#{@plugin_name}.ddl")}" + File.open(File.join(dirname, "#{@plugin_name}.ddl"), "w") {|f| f.puts @ddl} + puts "Created DDL file : #{File.join(dirname, "#{@plugin_name}.ddl")}" - File.open(File.join(dirname, "#{@plugin_name}.rb"), "w"){|f| f.puts @plugin} - puts "Created #{@mod_name} file : #{File.join(dirname, "#{@plugin_name}.rb")}" - rescue Errno::EEXIST - raise "cannot generate '#{@plugin_name}' : plugin directory already exists." - rescue Exception => e - FileUtils.rm_rf(@plugin_name) if File.directory?(@plugin_name) - raise "cannot generate plugin - #{e}" - end + File.open(File.join(dirname, "#{@plugin_name}.rb"), "w") {|f| f.puts @plugin} + puts "Created #{@mod_name} file : #{File.join(dirname, "#{@plugin_name}.rb")}" + rescue Errno::EEXIST + raise "cannot generate '#{@plugin_name}' : plugin directory already exists." + rescue Exception => e # rubocop:disable Lint/RescueException + FileUtils.rm_rf(@plugin_name) if File.directory?(@plugin_name) + raise "cannot generate plugin - #{e}" end end end diff --git a/lib/mcollective/generators/data_generator.rb b/lib/mcollective/generators/data_generator.rb index a7d7285a..035f2c02 100644 --- a/lib/mcollective/generators/data_generator.rb +++ b/lib/mcollective/generators/data_generator.rb @@ -1,11 +1,10 @@ module MCollective module Generators - class DataGenerator \"%DESCRIPTION%\",\n"] query_text += "%9s%s" % [" ", ":display_as => \"%DESCRIPTION%\"\n"] @@ -39,7 +38,7 @@ def create_plugin_content content_text = "%6s%s" % [" ", "query do |what|\n"] @outputs.each do |output| - content_text += "%8s%s" % [" ", "result[:#{output}] = nil\n"] + content_text += "%8s%s" % [" ", "result[:#{output}] = nil\n"] end content_text += "%6s%s" % [" ", "end\n"] diff --git a/lib/mcollective/log.rb b/lib/mcollective/log.rb index 983701a1..e3af883d 100644 --- a/lib/mcollective/log.rb +++ b/lib/mcollective/log.rb @@ -60,7 +60,7 @@ def log(level, msg) else t = Time.new.strftime("%H:%M:%S") - STDERR.puts "#{t}: #{level}: #{from}: #{msg}" + warn "#{t}: #{level}: #{from}: #{msg}" end end @@ -98,7 +98,7 @@ def configure(logger=nil) @logger.start rescue Exception => e # rubocop:disable Lint/RescueException @configured = false - STDERR.puts "Could not start logger: #{e.class} #{e}" + warn "Could not start logger: #{e.class} #{e}" end # figures out the filename that called us diff --git a/lib/mcollective/logger/base.rb b/lib/mcollective/logger/base.rb index 498e4460..db090b0f 100644 --- a/lib/mcollective/logger/base.rb +++ b/lib/mcollective/logger/base.rb @@ -29,7 +29,7 @@ def cycle_level end # Sets a new level and record it in @active_level - def set_level(level) + def set_level(level) # rubocop:disable Naming/AccessorMethodName set_logging_level(level) @active_level = level.to_sym end @@ -47,6 +47,7 @@ def reopen end private + def map_level(level) raise "Logger class do not know how to handle #{level} messages" unless valid_levels.include?(level.to_sym) @@ -54,7 +55,7 @@ def map_level(level) end # Gets the next level in the list, cycles down to the firt once it reaches the end - def get_next_level + def get_next_level # rubocop:disable Naming/AccessorMethodName # if all else fails, always go to debug mode nextlvl = :debug diff --git a/lib/mcollective/logger/console_logger.rb b/lib/mcollective/logger/console_logger.rb index 8be429d6..a4b18a62 100644 --- a/lib/mcollective/logger/console_logger.rb +++ b/lib/mcollective/logger/console_logger.rb @@ -1,7 +1,7 @@ module MCollective module Logger # Implements a syslog based logger using the standard ruby syslog class - class Console_logger :info, - :warn => :warning, + {:info => :info, + :warn => :warning, :debug => :debug, :fatal => :crit, :error => :err} end - def log(level, from, msg, normal_output=STDERR, last_resort_output=STDERR) + def log(level, from, msg, normal_output=$stderr, last_resort_output=$stderr) if @known_levels.index(level) >= @known_levels.index(@active_level) time = Time.new.strftime("%Y/%m/%d %H:%M:%S") @@ -41,20 +41,20 @@ def color(level) colors = {:error => Util.color(:red), :fatal => Util.color(:red), - :warn => Util.color(:yellow), - :info => Util.color(:green), + :warn => Util.color(:yellow), + :info => Util.color(:green), :reset => Util.color(:reset)} if colorize - return colors[level] || "" + colors[level] || "" else - return "" + "" end end # Helper to return a string in specific color def colorize(level, msg) - "%s%s%s" % [ color(level), msg, color(:reset) ] + "%s%s%s" % [color(level), msg, color(:reset)] end end end diff --git a/lib/mcollective/logger/file_logger.rb b/lib/mcollective/logger/file_logger.rb index 3768344a..21d5c75b 100644 --- a/lib/mcollective/logger/file_logger.rb +++ b/lib/mcollective/logger/file_logger.rb @@ -1,4 +1,4 @@ -require 'logger' +require "logger" module MCollective module Logger @@ -9,7 +9,7 @@ module Logger # - config.logfile # - config.keeplogs defaults to 2097152 # - config.max_log_size defaults to 5 - class File_logger e + rescue Exception => e # rubocop:disable Lint/RescueException @logger.level = ::Logger::DEBUG log(:error, "", "Could not set logging to #{level} using debug instead: #{e.class} #{e}") end def valid_levels - {:info => ::Logger::INFO, - :warn => ::Logger::WARN, + {:info => ::Logger::INFO, + :warn => ::Logger::WARN, :debug => ::Logger::DEBUG, :fatal => ::Logger::FATAL, :error => ::Logger::ERROR} @@ -39,7 +39,7 @@ def log(level, from, msg) rescue # if this fails we probably cant show the user output at all, # STDERR it as last resort - STDERR.puts("#{level}: #{msg}") + warn("#{level}: #{msg}") end def reopen diff --git a/lib/mcollective/logger/syslog_logger.rb b/lib/mcollective/logger/syslog_logger.rb index e2b1d9ab..8c40c2fe 100644 --- a/lib/mcollective/logger/syslog_logger.rb +++ b/lib/mcollective/logger/syslog_logger.rb @@ -1,8 +1,8 @@ module MCollective module Logger # Implements a syslog based logger using the standard ruby syslog class - class Syslog_logger e - STDERR.puts "Invalid syslog facility #{facility} supplied, reverting to USER" - Syslog::LOG_USER - end + Syslog.const_get("LOG_#{facility.upcase}") + rescue NameError + warn "Invalid syslog facility #{facility} supplied, reverting to USER" + Syslog::LOG_USER end - def set_logging_level(level) + def set_logging_level(level) # rubocop:disable Naming/AccessorMethodName # noop end def valid_levels - {:info => :info, - :warn => :warning, + {:info => :info, + :warn => :warning, :debug => :debug, :fatal => :crit, :error => :err} end def log(level, from, msg) - if @known_levels.index(level) >= @known_levels.index(@active_level) - Syslog.send(map_level(level), "#{from} #{msg}") - end + Syslog.send(map_level(level), "#{from} #{msg}") if @known_levels.index(level) >= @known_levels.index(@active_level) rescue # if this fails we probably cant show the user output at all, # STDERR it as last resort - STDERR.puts("#{level}: #{msg}") + warn("#{level}: #{msg}") end end end diff --git a/lib/mcollective/matcher.rb b/lib/mcollective/matcher.rb index c86d1949..0f51ac92 100644 --- a/lib/mcollective/matcher.rb +++ b/lib/mcollective/matcher.rb @@ -102,9 +102,9 @@ def self.execute_function(function_hash) # If data field has not been set we set the comparison result to nil eval_result = nil end - return eval_result + eval_result else - return result + result end rescue NoMethodError Log.debug("cannot execute discovery function '#{function_hash['name']}'. data plugin not found") @@ -113,9 +113,10 @@ def self.execute_function(function_hash) # Evaluates a compound statement def self.eval_compound_statement(expression) - if expression.values.first =~ /^\// + case expression.values.first + when /^\// Util.has_cf_class?(expression.values.first) - elsif expression.values.first =~ />=|<=|=|<|>/ + when />=|<=|=|<|>/ optype = expression.values.first.match(/>=|<=|=|<|>/) name, value = expression.values.first.split(optype[0]) if value.split("")[0] == "/" @@ -160,20 +161,21 @@ def self.eval_compound_fstatement(function_hash) # rubocop:disable Metrics/Metho result = l_compare.match(r_compare) # Flip return value for != operator if function_hash["operator"] == "!=~" - return !result + !result else - return !!result + !!result end else Log.debug("Cannot do a regex check on a non string value.") - return false + false end # Otherwise do a normal comparison while taking the type into account else if l_compare.is_a? String r_compare = r_compare.to_s elsif r_compare.is_a? String - if l_compare.is_a? Numeric + case l_compare + when Numeric r_compare = r_compare.strip begin r_compare = Integer(r_compare) @@ -184,7 +186,7 @@ def self.eval_compound_fstatement(function_hash) # rubocop:disable Metrics/Metho raise(ArgumentError, "invalid numeric value: #{r_compare}") end end - elsif l_compare.is_a?(TrueClass) || l_compare.is_a?(FalseClass) + when TrueClass, FalseClass r_compare = r_compare.strip if r_compare == true.to_s # rubocop:disable Metrics/BlockNesting r_compare = true @@ -201,8 +203,8 @@ def self.eval_compound_fstatement(function_hash) # rubocop:disable Metrics/Metho else raise(ArgumentError, "invalid operator: #{operator}") end - result = l_compare.send(operator, r_compare) - return result + l_compare.send(operator, r_compare) + end end @@ -210,9 +212,7 @@ def self.eval_compound_fstatement(function_hash) # rubocop:disable Metrics/Metho def self.create_compound_callstack(call_string) callstack = Matcher::Parser.new(call_string).execution_stack callstack.each_with_index do |statement, i| - if statement.keys.first == "fstatement" - callstack[i]["fstatement"] = create_function_hash(statement.values.first) - end + callstack[i]["fstatement"] = create_function_hash(statement.values.first) if statement.keys.first == "fstatement" end callstack end diff --git a/lib/mcollective/matcher/parser.rb b/lib/mcollective/matcher/parser.rb index 83c9f62d..3d577681 100644 --- a/lib/mcollective/matcher/parser.rb +++ b/lib/mcollective/matcher/parser.rb @@ -1,3 +1,4 @@ +# rubocop:disable Metrics/BlockNesting module MCollective module Matcher class Parser @@ -10,9 +11,9 @@ def initialize(args) @token_errors = [] @paren_errors = [] parse - exit_with_token_errors if @token_errors.size > 0 - exit_with_parse_errors if @parse_errors.size > 0 - exit_with_paren_errors if @paren_errors.size > 0 + exit_with_token_errors unless @token_errors.empty? + exit_with_parse_errors unless @parse_errors.empty? + exit_with_paren_errors unless @paren_errors.empty? end # Exit and highlight any malformed tokens @@ -31,7 +32,7 @@ def exit_with_parse_errors @scanner.arguments[i] = Util.colorize(:red, @scanner.arguments[i]) end end - raise "Parse errors found while parsing -S input #{ @scanner.arguments.join}" + raise "Parse errors found while parsing -S input #{@scanner.arguments.join}" end def exit_with_paren_errors @@ -42,13 +43,12 @@ def exit_with_paren_errors end # Parse the input string, one token at a time a contruct the call stack - def parse + def parse # rubocop:disable Metrics/MethodLength pre_index = @scanner.token_index - p_token,p_token_value = nil - c_token,c_token_value = @scanner.get_token - parenth = 0 + p_token = nil + c_token, c_token_value = @scanner.get_token - while (c_token != nil) + until c_token.nil? @scanner.token_index += 1 n_token, n_token_value = @scanner.get_token @@ -58,67 +58,56 @@ def parse @token_errors << c_token_value when "and" - unless (n_token =~ /not|fstatement|statement|\(/) || (scanner.token_index == scanner.arguments.size) && !(n_token == nil) + unless (n_token =~ /not|fstatement|statement|\(/) || (scanner.token_index == scanner.arguments.size) && !n_token.nil? @parse_errors << [pre_index, scanner.token_index] end - if p_token == nil + case p_token + when nil @parse_errors << [pre_index - c_token.size, scanner.token_index] - elsif (p_token == "and" || p_token == "or") + when "and", "or" @parse_errors << [pre_index - 1 - p_token.size, pre_index - 1] end when "or" - unless (n_token =~ /not|fstatement|statement|\(/) || (scanner.token_index == scanner.arguments.size) && !(n_token == nil) + unless (n_token =~ /not|fstatement|statement|\(/) || (scanner.token_index == scanner.arguments.size) && !n_token.nil? @parse_errors << [pre_index, scanner.token_index] end - if p_token == nil + case p_token + when nil @parse_errors << [pre_index - c_token.size, scanner.token_index] - elsif (p_token == "and" || p_token == "or") + when "and", "or" @parse_errors << [pre_index - 1 - p_token.size, pre_index - 1] end when "not" - unless n_token =~ /fstatement|statement|\(|not/ && !(n_token == nil) - @parse_errors << [pre_index, scanner.token_index] - end + @parse_errors << [pre_index, scanner.token_index] unless n_token =~ /fstatement|statement|\(|not/ && !n_token.nil? - when "statement","fstatement" - unless n_token =~ /and|or|\)/ - unless scanner.token_index == scanner.arguments.size - @parse_errors << [pre_index, scanner.token_index] - end - end + when "statement", "fstatement" + @parse_errors << [pre_index, scanner.token_index] if n_token !~ /and|or|\)/ && scanner.token_index != scanner.arguments.size when ")" - unless (n_token =~ /|and|or|not|\(/) - unless(scanner.token_index == scanner.arguments.size) - @parse_errors << [pre_index, scanner.token_index] - end - end - unless @paren_errors.empty? - @paren_errors.pop + @parse_errors << [pre_index, scanner.token_index] if n_token !~ /|and|or|not|\(/ && scanner.token_index != scanner.arguments.size + if @paren_errors.empty? + @paren_errors.push(n_token.nil? ? scanner.token_index - 1 : scanner.token_index - n_token_value.size) else - @paren_errors.push((n_token.nil?) ? scanner.token_index - 1: scanner.token_index - n_token_value.size) + @paren_errors.pop end when "(" - unless n_token =~ /fstatement|statement|not|\(/ - @parse_errors << [pre_index, scanner.token_index] - end - @paren_errors.push((n_token.nil?) ? scanner.token_index - 1: scanner.token_index - n_token_value.size) + @parse_errors << [pre_index, scanner.token_index] unless n_token =~ /fstatement|statement|not|\(/ + @paren_errors.push(n_token.nil? ? scanner.token_index - 1 : scanner.token_index - n_token_value.size) else @parse_errors << [pre_index, scanner.token_index] end - unless n_token == " " ||c_token == "bad_token" - @execution_stack << {c_token => c_token_value} - end + @execution_stack << {c_token => c_token_value} unless n_token == " " || c_token == "bad_token" - p_token, p_token_value = c_token, c_token_value - c_token, c_token_value = n_token, n_token_value + p_token = c_token + c_token = n_token + c_token_value = n_token_value end pre_index = @scanner.token_index end @@ -126,3 +115,4 @@ def parse end end end +# rubocop:enable Metrics/BlockNesting diff --git a/lib/mcollective/matcher/scanner.rb b/lib/mcollective/matcher/scanner.rb index 0c336dae..c7a23238 100644 --- a/lib/mcollective/matcher/scanner.rb +++ b/lib/mcollective/matcher/scanner.rb @@ -11,33 +11,31 @@ def initialize(arguments) end # Scans the input string and identifies single language tokens - def get_token - if @token_index >= @arguments.size - return nil - end + def get_token # rubocop:disable Naming/AccessorMethodName + return nil if @token_index >= @arguments.size case @arguments[@token_index] when "(" - return "(", "(" + ["(", "("] when ")" - return ")", ")" + [")", ")"] when "n" if (@arguments[@token_index + 1] == "o") && (@arguments[@token_index + 2] == "t") && ((@arguments[@token_index + 3] == " ") || (@arguments[@token_index + 3] == "(")) @token_index += 2 - return "not", "not" + ["not", "not"] else gen_statement end when "!" - return "not", "not" + ["not", "not"] when "a" if (@arguments[@token_index + 1] == "n") && (@arguments[@token_index + 2] == "d") && ((@arguments[@token_index + 3] == " ") || (@arguments[@token_index + 3] == "(")) @token_index += 2 - return "and", "and" + ["and", "and"] else gen_statement end @@ -45,13 +43,13 @@ def get_token when "o" if (@arguments[@token_index + 1] == "r") && ((@arguments[@token_index + 2] == " ") || (@arguments[@token_index + 2] == "(")) @token_index += 1 - return "or", "or" + ["or", "or"] else gen_statement end when " " - return " ", " " + [" ", " "] else gen_statement @@ -59,21 +57,24 @@ def get_token end private + # Helper generates a statement token - def gen_statement + def gen_statement # rubocop:disable Metrics/MethodLength func = false current_token_value = "" j = @token_index escaped = false begin - if (@arguments[j] == "/") - begin + case @arguments[j] + when "/" + loop do current_token_value << @arguments[j] j += 1 - end until (j >= @arguments.size) || (@arguments[j] =~ /\s/) - elsif (@arguments[j] =~ /=|<|>/) - while !(@arguments[j] =~ /=|<|>/) + break if (j >= @arguments.size) || (@arguments[j] =~ /\s/) + end + when /=|<|>/ + while @arguments[j] !~ /=|<|>/ current_token_value << @arguments[j] j += 1 end @@ -82,27 +83,28 @@ def gen_statement j += 1 if @arguments[j] == "/" - begin + loop do current_token_value << @arguments[j] j += 1 if @arguments[j] == "/" current_token_value << "/" break end - end until (j >= @arguments.size) || (@arguments[j] =~ /\//) + break if (j >= @arguments.size) || (@arguments[j] =~ /\//) + end while (j < @arguments.size) && ((@arguments[j] != " ") && (@arguments[j] != ")")) current_token_value << @arguments[j] j += 1 end end else - begin + loop do # Identify and tokenize regular expressions by ignoring everything between /'s - if @arguments[j] == '/' - current_token_value << '/' + if @arguments[j] == "/" + current_token_value << "/" j += 1 - while(j < @arguments.size && @arguments[j] != '/') - if @arguments[j] == '\\' + while j < @arguments.size && @arguments[j] != "/" + if @arguments[j] == '\\' # rubocop:disable Metrics/BlockNesting # eat the escape char current_token_value << @arguments[j] j += 1 @@ -116,7 +118,8 @@ def gen_statement break end - if @arguments[j] == "(" + case @arguments[j] + when "(" func = true current_token_value << @arguments[j] @@ -124,29 +127,29 @@ def gen_statement while j < @arguments.size current_token_value << @arguments[j] - if @arguments[j] == ')' + if @arguments[j] == ")" # rubocop:disable Metrics/BlockNesting j += 1 break end j += 1 end - elsif @arguments[j] == '"' || @arguments[j] == "'" + when '"', "'" escaped = true escaped_with = @arguments[j] j += 1 # step over first " or ' @white_spaces += 1 # identified "..." or '...' + # rubocop:disable Metrics/BlockNesting while j < @arguments.size - if @arguments[j] == '\\' + case @arguments[j] + when '\\' # eat the escape char but don't add it to the token, or we # end up with \\\" j += 1 @white_spaces += 1 - unless j < @arguments.size - break - end - elsif @arguments[j] == escaped_with + break unless j < @arguments.size + when escaped_with j += 1 @white_spaces += 1 break @@ -154,24 +157,22 @@ def gen_statement current_token_value << @arguments[j] j += 1 end + # rubocop:enable Metrics/BlockNesting else current_token_value << @arguments[j] j += 1 end - if(@arguments[j] == ' ') - break if(is_klass?(j) && !(@arguments[j-1] =~ /=|<|>/)) - end + break if @arguments[j] == " " && (is_klass?(j) && @arguments[j - 1] !~ /=|<|>/) - if( (@arguments[j] == ' ') && (@seperation_counter < 2) && !(current_token_value.match(/^.+(=|<|>).+$/)) ) - if((index = lookahead(j))) - j = index - end + if (@arguments[j] == " ") && (@seperation_counter < 2) && !current_token_value.match(/^.+(=|<|>).+$/) && (index = lookahead(j)) + j = index end - end until (j >= @arguments.size) || (@arguments[j] =~ /\s|\)/) + break if (j >= @arguments.size) || (@arguments[j] =~ /\s|\)/) + end @seperation_counter = 0 end - rescue Exception => e + rescue Exception => e # rubocop:disable Lint/RescueException raise "An exception was raised while trying to tokenize '#{current_token_value} - #{e}'" end @@ -180,61 +181,55 @@ def gen_statement # bar( if current_token_value.match(/.+?\($/) - return "bad_token", [@token_index - current_token_value.size + 1, @token_index] + ["bad_token", [@token_index - current_token_value.size + 1, @token_index]] # /foo/=bar elsif current_token_value.match(/^\/.+?\/(<|>|=).+/) - return "bad_token", [@token_index - current_token_value.size + 1, @token_index] + ["bad_token", [@token_index - current_token_value.size + 1, @token_index]] elsif current_token_value.match(/^.+?\/(<|>|=).+/) - return "bad_token", [@token_index - current_token_value.size + 1, @token_index] - else - if func - if current_token_value.match(/^.+?\((\s*(')[^']*(')\s*(,\s*(')[^']*('))*)?\)(\.[a-zA-Z0-9_]+)?((!=|<=|>=|=|>|<).+)?$/) || - current_token_value.match(/^.+?\((\s*(")[^"]*(")\s*(,\s*(")[^"]*("))*)?\)(\.[a-zA-Z0-9_]+)?((!=|<=|>=|=|>|<).+)?$/) - return "fstatement", current_token_value - else - return "bad_token", [@token_index - current_token_value.size + 1, @token_index] - end + ["bad_token", [@token_index - current_token_value.size + 1, @token_index]] + elsif func + if current_token_value.match(/^.+?\((\s*(')[^']*(')\s*(,\s*(')[^']*('))*)?\)(\.[a-zA-Z0-9_]+)?((!=|<=|>=|=|>|<).+)?$/) || + current_token_value.match(/^.+?\((\s*(")[^"]*(")\s*(,\s*(")[^"]*("))*)?\)(\.[a-zA-Z0-9_]+)?((!=|<=|>=|=|>|<).+)?$/) + ["fstatement", current_token_value] else - if escaped - return "statement", current_token_value - end - slash_err = false - current_token_value.split('').each do |c| - if c == '/' - slash_err = !slash_err - end - end - return "bad_token", [@token_index - current_token_value.size + 1, @token_index] if slash_err - return "statement", current_token_value + ["bad_token", [@token_index - current_token_value.size + 1, @token_index]] + end + else + return "statement", current_token_value if escaped + + slash_err = false + current_token_value.split("").each do |c| + slash_err = !slash_err if c == "/" end + return "bad_token", [@token_index - current_token_value.size + 1, @token_index] if slash_err + + ["statement", current_token_value] end end # Deal with special puppet class statement - def is_klass?(j) - while(j < @arguments.size && @arguments[j] == ' ') - j += 1 - end + def is_klass?(klass) + klass += 1 while klass < @arguments.size && @arguments[klass] == " " - if @arguments[j] =~ /=|<|>/ - return false + if @arguments[klass] =~ /=|<|>/ + false else - return true + true end end # Eat spaces while looking for the next comparison symbol def lookahead(index) index += 1 - while(index <= @arguments.size) + while index <= @arguments.size @white_spaces += 1 - unless(@arguments[index] =~ /\s/) - @seperation_counter +=1 + unless @arguments[index] =~ /\s/ + @seperation_counter += 1 return index end index += 1 end - return nil + nil end end end diff --git a/lib/mcollective/message.rb b/lib/mcollective/message.rb index 70d76194..4540d4fb 100644 --- a/lib/mcollective/message.rb +++ b/lib/mcollective/message.rb @@ -2,8 +2,7 @@ module MCollective # container for a message, its headers, agent, collective and other meta data class Message attr_reader :message, :request, :validated, :msgtime, :payload, :type, :expected_msgid, :reply_to - attr_accessor :headers, :agent, :collective, :filter - attr_accessor :requestid, :discovered_hosts, :options, :ttl + attr_accessor :headers, :agent, :collective, :filter, :requestid, :discovered_hosts, :options, :ttl VALIDTYPES = [:message, :request, :direct_request, :reply].freeze @@ -79,9 +78,7 @@ def type=(type) if type == :direct_request raise "Direct requests is not enabled using the direct_addressing config option" unless Config.instance.direct_addressing - unless @discovered_hosts && !@discovered_hosts.empty? - raise "Can only set type to :direct_request if discovered_hosts have been set" - end + raise "Can only set type to :direct_request if discovered_hosts have been set" unless @discovered_hosts && !@discovered_hosts.empty? # clear out the filter, custom discovery sources might interpret the filters # different than the remote mcollectived and in directed mode really the only @@ -109,6 +106,7 @@ def reply_to=(target) # at us. def expected_msgid=(msgid) raise "Can only store the expected msgid for reply messages" unless @type == :reply + @expected_msgid = msgid end @@ -132,7 +130,7 @@ def base64? def description cid = "" - cid += payload[:callerid] + "@" if payload.include?(:callerid) + cid += "#{payload[:callerid]}@" if payload.include?(:callerid) cid += payload[:senderid] "#{requestid} for agent '#{agent}' in collective '#{collective}' from #{cid}" @@ -163,6 +161,7 @@ def validate_compound_filter(compound_filter) compound_filter.each do |filter| filter.each do |statement| next unless statement["fstatement"] + functionname = statement["fstatement"]["name"] pluginname = Data.pluginname(functionname) value = statement["fstatement"]["value"] @@ -175,9 +174,7 @@ def validate_compound_filter(compound_filter) Data.ddl_validate(ddl, statement["fstatement"]["params"]) - unless value && Data.ddl_has_output?(ddl, value) - raise(DDLValidationError, "Data plugin '%s()' does not return a '%s' value" % [functionname, value]) - end + raise(DDLValidationError, "Data plugin '%s()' does not return a '%s' value" % [functionname, value]) unless value && Data.ddl_has_output?(ddl, value) end end end @@ -194,15 +191,15 @@ def decode! else # We're in the client, log and carry on as best we can - # Note: mc_sender is unverified. The verified identity is in the + # NOTE: mc_sender is unverified. The verified identity is in the # payload we just failed to decode Log.warn("Failed to decode a message from '#{headers['mc_sender']}': #{e}") return end end - if type == :request - raise "callerid in request is not valid, surpressing reply to potentially forged request" unless PluginManager["security_plugin"].valid_callerid?(payload[:callerid]) + if type == :request && !PluginManager["security_plugin"].valid_callerid?(payload[:callerid]) + raise "callerid in request is not valid, surpressing reply to potentially forged request" end [:collective, :agent, :filter, :requestid, :ttl, :msgtime].each do |prop| diff --git a/lib/mcollective/monkey_patches.rb b/lib/mcollective/monkey_patches.rb index 79a62880..9a71765a 100644 --- a/lib/mcollective/monkey_patches.rb +++ b/lib/mcollective/monkey_patches.rb @@ -67,8 +67,8 @@ class String unless method_defined?(:bytes) def bytes(&block) # This should not be necessary, really ... - require "enumerator" return to_enum(:each_byte) unless block_given? + each_byte(&block) end end @@ -141,9 +141,7 @@ class OpenSSL::SSL::SSLContext # rubocop:disable Style/ClassAndModuleChildren end # ruby 1.8.5 doesn't define this constant, but has it on by default - if defined?(OpenSSL::SSL::OP_DONT_INSERT_EMPTY_FRAGMENTS) - DEFAULT_PARAMS[:options] |= OpenSSL::SSL::OP_DONT_INSERT_EMPTY_FRAGMENTS - end + DEFAULT_PARAMS[:options] |= OpenSSL::SSL::OP_DONT_INSERT_EMPTY_FRAGMENTS if defined?(OpenSSL::SSL::OP_DONT_INSERT_EMPTY_FRAGMENTS) DEFAULT_PARAMS[:ciphers] << ":!SSLv2" if DEFAULT_PARAMS[:ciphers] diff --git a/lib/mcollective/optionparser.rb b/lib/mcollective/optionparser.rb index 1c3e0c6d..808621db 100644 --- a/lib/mcollective/optionparser.rb +++ b/lib/mcollective/optionparser.rb @@ -155,6 +155,7 @@ def add_common_options @parser.on("--dm", "--disc-method METHOD", "Which discovery method to use") do |v| raise "Discovery method is already set by a competing option" if @options[:discovery_method] && @options[:discovery_method] != v + @options[:discovery_method] = v end diff --git a/lib/mcollective/pluginmanager.rb b/lib/mcollective/pluginmanager.rb index f37bf2e8..261f8456 100644 --- a/lib/mcollective/pluginmanager.rb +++ b/lib/mcollective/pluginmanager.rb @@ -91,7 +91,7 @@ def self.[](plugin) # use eval to create an instance of a class def self.create_instance(klass) - eval("#{klass}.new") # rubocop:disable Security/Eval + eval("#{klass}.new") # rubocop:disable Security/Eval, Style/EvalWithLocation rescue Exception => e # rubocop:disable Lint/RescueException raise("Could not create instance of plugin #{klass}: #{e}") end @@ -146,9 +146,7 @@ def self.find_and_load(type, extension="rb") extension = ".#{extension}" unless extension =~ /^\./ klasses = find(type, extension).map do |plugin| - if block_given? - next unless yield(plugin) - end + next if block_given? && !yield(plugin) "%s::%s::%s" % ["MCollective", type.capitalize, plugin.capitalize] end.compact @@ -159,7 +157,7 @@ def self.find_and_load(type, extension="rb") # Loads a class from file by doing some simple search/replace # on class names and then doing a require. def self.loadclass(klass, squash_failures=false) - fname = klass.gsub("::", "/").downcase + ".rb" + fname = "#{klass.gsub('::', '/').downcase}.rb" Log.debug("Loading #{klass} from #{fname}") diff --git a/lib/mcollective/pluginpackager.rb b/lib/mcollective/pluginpackager.rb index 65fadd16..1a642429 100644 --- a/lib/mcollective/pluginpackager.rb +++ b/lib/mcollective/pluginpackager.rb @@ -84,9 +84,7 @@ def self.filter_dependencies(prefix, dependencies) # Return the path to a plugin's core directories def self.get_plugin_path(target) - if File.exist?(File.join(target, "lib", "mcollective")) - return File.join(target, "lib", "mcollective") - end + return File.join(target, "lib", "mcollective") if File.exist?(File.join(target, "lib", "mcollective")) target end diff --git a/lib/mcollective/pluginpackager/agent_definition.rb b/lib/mcollective/pluginpackager/agent_definition.rb index 8850ad00..ca52c1a4 100644 --- a/lib/mcollective/pluginpackager/agent_definition.rb +++ b/lib/mcollective/pluginpackager/agent_definition.rb @@ -2,8 +2,7 @@ module MCollective module PluginPackager # MCollective Agent Plugin package class AgentDefinition - attr_accessor :path, :packagedata, :metadata, :target_path, :vendor, :revision, :preinstall - attr_accessor :plugintype, :dependencies, :postinstall, :mcname, :mcversion + attr_accessor :path, :packagedata, :metadata, :target_path, :vendor, :revision, :preinstall, :plugintype, :dependencies, :postinstall, :mcname, :mcversion def initialize(configuration, mcdependency, plugintype) @plugintype = plugintype @@ -53,9 +52,7 @@ def agent agent[:files] = (Dir.glob(File.join(agentdir, "**", "**")) - ddls) agent[:plugindependency] = {:name => "#{@mcname}-#{@metadata[:name]}-common", :version => @metadata[:version], :revision => @revision} - if @metadata[:provider] == "external" - agent[:executable_files] << File.join(agentdir, @agent_name) - end + agent[:executable_files] << File.join(agentdir, @agent_name) if @metadata[:provider] == "external" agent end @@ -95,9 +92,7 @@ def common end # We fail if there is no ddl file present - if common[:files].grep(/^.*\.ddl$/).empty? - raise "cannot create package - No ddl file found in #{File.join(@path, 'agent')}" - end + raise "cannot create package - No ddl file found in #{File.join(@path, 'agent')}" if common[:files].grep(/^.*\.ddl$/).empty? common[:files].uniq! diff --git a/lib/mcollective/pluginpackager/forge_packager.rb b/lib/mcollective/pluginpackager/forge_packager.rb index c93459e1..893bc5bd 100644 --- a/lib/mcollective/pluginpackager/forge_packager.rb +++ b/lib/mcollective/pluginpackager/forge_packager.rb @@ -40,7 +40,7 @@ def create_packages puts("Completed building module for %s" % module_name) rescue - STDERR.puts("Failed to build plugin module: %s: %s" % [$!.class, $!.to_s]) + warn("Failed to build plugin module: %s: %s" % [$!.class, $!.to_s]) ensure if @keep_artifacts puts("Keeping build artifacts") @@ -191,12 +191,12 @@ def render_templates end def render_template(infile, outfile) - erb = ERB.new(File.read(infile), nil, "-") + erb = ERB.new(File.read(infile), 0, "%") File.open(outfile, "w") do |f| f.puts erb.result(binding) end rescue - STDERR.puts("Could not render template %s to %s" % [infile, outfile]) + warn("Could not render template %s to %s" % [infile, outfile]) raise end @@ -218,9 +218,7 @@ def assert_new_enough_pdk actual_version = s.stdout.chomp required_version = "1.12.0" - if Util.versioncmp(actual_version, required_version) < 0 - raise("Cannot build package. pdk #{required_version} or greater required. We have #{actual_version}.") - end + raise("Cannot build package. pdk #{required_version} or greater required. We have #{actual_version}.") if Util.versioncmp(actual_version, required_version) < 0 end def run_build @@ -230,7 +228,7 @@ def run_build end end rescue - STDERR.puts("Build process has failed") + warn("Build process has failed") raise end @@ -238,14 +236,14 @@ def move_package package_file = File.join(@tmpdir, "pkg", module_file_name) FileUtils.cp(package_file, ".") rescue - STDERR.puts("Could not copy package to working directory") + warn("Could not copy package to working directory") raise end def cleanup_tmpdirs FileUtils.rm_r(@tmpdir) if File.directory?(@tmpdir) rescue - STDERR.puts("Could not remove temporary build directory %s" % [@tmpdir]) + warn("Could not remove temporary build directory %s" % [@tmpdir]) raise end end diff --git a/lib/mcollective/pluginpackager/standard_definition.rb b/lib/mcollective/pluginpackager/standard_definition.rb index 114f0cfd..24874ed5 100644 --- a/lib/mcollective/pluginpackager/standard_definition.rb +++ b/lib/mcollective/pluginpackager/standard_definition.rb @@ -1,8 +1,7 @@ module MCollective module PluginPackager class StandardDefinition - attr_accessor :path, :packagedata, :metadata, :target_path, :vendor, :revision - attr_accessor :plugintype, :preinstall, :postinstall, :dependencies, :mcname, :mcversion + attr_accessor :path, :packagedata, :metadata, :target_path, :vendor, :revision, :plugintype, :preinstall, :postinstall, :dependencies, :mcname, :mcversion def initialize(configuration, mcdependency, plugintype) @plugintype = plugintype diff --git a/lib/mcollective/registration/base.rb b/lib/mcollective/registration/base.rb index edad1ddb..118adf52 100644 --- a/lib/mcollective/registration/base.rb +++ b/lib/mcollective/registration/base.rb @@ -10,6 +10,7 @@ class Base # Register plugins that inherits base def self.inherited(klass) PluginManager << {:type => "registration_plugin", :class => klass.to_s} + super end # Creates a background thread that periodically send a registration notice. @@ -44,7 +45,7 @@ def target_collective collective = main_collective end - return collective + collective end def interval @@ -52,15 +53,15 @@ def interval end def publish(message) - unless message - Log.debug("Skipping registration due to nil body") - else + if message req = Message.new(message, nil, {:type => :request, :agent => "registration", :collective => target_collective, :filter => msg_filter}) req.encode! Log.debug("Sending registration #{req.requestid} to collective #{req.collective}") req.publish + else + Log.debug("Skipping registration due to nil body") end end @@ -69,22 +70,23 @@ def body end private + def publish_thread(connnection) if config.registration_splay - splay_delay = rand(interval) - Log.debug("registration_splay enabled. Registration will start in #{splay_delay} seconds") - sleep splay_delay - end + splay_delay = rand(interval) + Log.debug("registration_splay enabled. Registration will start in #{splay_delay} seconds") + sleep splay_delay + end - loop do - begin - publish(body) - sleep interval - rescue Exception => e - Log.error("Sending registration message failed: #{e}") - sleep interval - end + loop do + begin + publish(body) + sleep interval + rescue Exception => e # rubocop:disable Lint/RescueException + Log.error("Sending registration message failed: #{e}") + sleep interval end + end end end end diff --git a/lib/mcollective/rpc.rb b/lib/mcollective/rpc.rb index 1bf42eff..b65023dd 100644 --- a/lib/mcollective/rpc.rb +++ b/lib/mcollective/rpc.rb @@ -21,13 +21,11 @@ module RPC def rpcoptions oparser = MCollective::Optionparser.new({:verbose => false, :progress_bar => true}, "filter") - options = oparser.parse do |parser, opts| + oparser.parse do |parser, opts| yield(parser, opts) if block_given? Helpers.add_simplerpc_options(parser, opts) end - - options end # Wrapper to create clients, supposed to be used as @@ -86,7 +84,7 @@ def rpcclient(agent, flags={}) if block_given? yield(rpc) else - return rpc + rpc end end diff --git a/lib/mcollective/rpc/actionrunner.rb b/lib/mcollective/rpc/actionrunner.rb index 04501707..41876e53 100644 --- a/lib/mcollective/rpc/actionrunner.rb +++ b/lib/mcollective/rpc/actionrunner.rb @@ -35,11 +35,11 @@ def initialize(command, request, format=:json) def run unless canrun?(command) - Log.warn("Cannot run #{to_s}") - raise RPCAborted, "Cannot execute #{to_s}" + Log.warn("Cannot run #{self}") + raise RPCAborted, "Cannot execute #{self}" end - Log.debug("Running #{to_s}") + Log.debug("Running #{self}") request_file = saverequest(request) reply_file = tempfile("reply") @@ -51,13 +51,13 @@ def run Log.debug("#{command} exited with #{runner.status.exitstatus}") - stderr.each_line {|l| Log.error("#{to_s}: #{l.chomp}")} unless stderr.empty? - stdout.each_line {|l| Log.info("#{to_s}: #{l.chomp}")} unless stdout.empty? + stderr.each_line {|l| Log.error("#{self}: #{l.chomp}")} unless stderr.empty? + stdout.each_line {|l| Log.info("#{self}: #{l.chomp}")} unless stdout.empty? {:exitstatus => runner.status.exitstatus, - :stdout => runner.stdout, - :stderr => runner.stderr, - :data => load_results(reply_file.path)} + :stdout => runner.stdout, + :stderr => runner.stderr, + :data => load_results(reply_file.path)} ensure request_file.close! if request_file.respond_to?("close!") reply_file.close! if reply_file.respond_to?("close") @@ -65,7 +65,7 @@ def run def shell(command, infile, outfile) env = {"MCOLLECTIVE_REQUEST_FILE" => infile, - "MCOLLECTIVE_REPLY_FILE" => outfile} + "MCOLLECTIVE_REPLY_FILE" => outfile} Shell.new("#{command} #{infile} #{outfile}", :cwd => Dir.tmpdir, :stdout => stdout, :stderr => stderr, :environment => env) end @@ -78,20 +78,20 @@ def load_results(file) if respond_to?("load_#{format}_results") tempdata = send("load_#{format}_results", file) - tempdata.each_pair do |k,v| + tempdata.each_pair do |k, v| data[k.to_sym] = v end end data - rescue Exception => e + rescue Exception # rubocop:disable Lint/RescueException {} end def load_json_results(file) return {} unless File.readable?(file) - JSON.load(File.read(file)) || {} + JSON.parse(File.read(file)) || {} rescue JSON::ParserError {} end @@ -119,7 +119,7 @@ def canrun?(command) end def to_s - "%s#%s command: %s" % [ agent, action, command ] + "%s#%s command: %s" % [agent, action, command] end def tempfile(prefix) @@ -127,15 +127,13 @@ def tempfile(prefix) end def path_to_command(command) - if Util.absolute_path?(command) - return command - end + return command if Util.absolute_path?(command) Config.instance.libdir.each do |libdir| command_file_old = File.join(libdir, "agent", agent, command) command_file_new = File.join(libdir, "mcollective", "agent", agent, command) - command_file_old_exists = File.exists?(command_file_old) - command_file_new_exists = File.exists?(command_file_new) + command_file_old_exists = File.exist?(command_file_old) + command_file_new_exists = File.exist?(command_file_new) if command_file_new_exists && command_file_old_exists Log.debug("Found 'implemented_by' scripts found in two locations #{command_file_old} and #{command_file_new}") diff --git a/lib/mcollective/rpc/agent.rb b/lib/mcollective/rpc/agent.rb index 64d9d98a..6fdca034 100644 --- a/lib/mcollective/rpc/agent.rb +++ b/lib/mcollective/rpc/agent.rb @@ -53,8 +53,7 @@ def load_ddl @ddl = DDL.new(@agent_name, :agent) @meta = @ddl.meta @timeout = @meta[:timeout] || 10 - - rescue Exception => e + rescue Exception => e # rubocop:disable Lint/RescueException Log.error("Failed to load DDL for the '%s' agent, DDLs are required: %s: %s" % [@agent_name, e.class, e.to_s]) raise DDLValidationError end @@ -89,35 +88,29 @@ def handlemsg(msg, connection) end rescue RPCAborted => e @reply.fail e.to_s, 1 - rescue UnknownRPCAction => e @reply.fail e.to_s, 2 - rescue MissingRPCData => e @reply.fail e.to_s, 3 - rescue InvalidRPCData, DDLValidationError => e @reply.fail e.to_s, 4 - rescue UnknownRPCError => e Log.error("%s#%s failed: %s: %s" % [@agent_name, @request.action, e.class, e.to_s]) Log.error(e.backtrace.join("\n\t")) @reply.fail e.to_s, 5 - - rescue Exception => e + rescue Exception => e # rubocop:disable Lint/RescueException Log.error("%s#%s failed: %s: %s" % [@agent_name, @request.action, e.class, e.to_s]) Log.error(e.backtrace.join("\n\t")) @reply.fail e.to_s, 5 - end after_processing_hook if @request.should_respond? - return @reply.to_hash + @reply.to_hash else Log.debug("Client did not request a response, surpressing reply") - return nil + nil end end @@ -137,7 +130,7 @@ def handlemsg(msg, connection) # File.exist?("/usr/bin/puppet") # end def self.activate? - agent_name = self.to_s.split("::").last.downcase + agent_name = to_s.split("::").last.downcase config = Config.instance Log.debug("Starting default activation checks for #{agent_name}") @@ -146,14 +139,12 @@ def self.activate? should_activate = config.activate_agents # Check agent specific state to determine if agent should be loaded - should_activate = Util.str_to_bool(config.pluginconf.fetch("#{agent_name}.activate_agent", - should_activate)) + should_activate = Util.str_to_bool(config.pluginconf.fetch("#{agent_name}.activate_agent", + should_activate)) - unless should_activate - Log.debug("Found plugin configuration '#{agent_name}.activate_agent' with value '#{should_activate}'") - end + Log.debug("Found plugin configuration '#{agent_name}.activate_agent' with value '#{should_activate}'") unless should_activate - return should_activate + should_activate end # Returns an array of actions this agent support @@ -163,7 +154,6 @@ def self.actions end end - private # Runs a command via the MC::Shell wrapper, options are as per MC::Shell # # The simplest use is: @@ -206,22 +196,18 @@ def run(command, options={}) [:stderr, :stdout].each do |k| if options.include?(k) if options[k].is_a?(Symbol) - reply[ options[k] ] = "" - shellopts[k] = reply[ options[k] ] + reply[options[k]] = "" + shellopts[k] = reply[options[k]] + elsif options[k].respond_to?("<<") + shellopts[k] = options[k] else - if options[k].respond_to?("<<") - shellopts[k] = options[k] - else - reply.fail! "#{k} should support << while calling run(#{command})" - end + reply.fail! "#{k} should support << while calling run(#{command})" end end end [:stdin, :cwd, :environment, :timeout].each do |k| - if options.include?(k) - shellopts[k] = options[k] - end + shellopts[k] = options[k] if options.include?(k) end shell = Shell.new(command, shellopts) @@ -263,7 +249,7 @@ def self.activate_when(&block) def self.action(name, &block) raise "Need to pass a body for the action" unless block_given? - self.module_eval { define_method("#{name}_action", &block) } + module_eval { define_method("#{name}_action", &block) } end # Helper that creates a method on the class that will call your authorization @@ -272,16 +258,18 @@ def self.authorized_by(plugin) plugin = plugin.to_s.capitalize # turns foo_bar into FooBar - plugin = plugin.to_s.split("_").map {|v| v.capitalize}.join + plugin = plugin.to_s.split("_").map(&:capitalize).join pluginname = "MCollective::Util::#{plugin}" PluginManager.loadclass(pluginname) unless MCollective::Util.constants.include?(plugin) + # rubocop:disable Style/EvalWithLocation class_eval(" def authorization_hook(request) #{pluginname}.authorize(request) end ") + # rubocop:enable Style/EvalWithLocation end # Validates a data member, if validation is a regex then it will try to match it @@ -304,7 +292,7 @@ def validate(key, validation) Validator.validate(@request[key], validation) rescue ValidatorError => e - raise InvalidRPCData, "Input %s did not pass validation: %s" % [ key, e.message ] + raise InvalidRPCData, "Input %s did not pass validation: %s" % [key, e.message] end # convenience wrapper around Util#shellescape @@ -323,10 +311,8 @@ def implemented_by(command, type=:json) reply.data.merge!(res[:data]) - if res[:exitstatus] > 0 - reply.fail "Failed to run #{command}: #{res[:stderr]}", res[:exitstatus] - end - rescue Exception => e + reply.fail "Failed to run #{command}: #{res[:stderr]}", res[:exitstatus] if res[:exitstatus] > 0 + rescue Exception => e # rubocop:disable Lint/RescueException Log.warn("Unhandled #{e.class} exception during #{request.agent}##{request.action}: #{e}") reply.fail! "Unexpected failure calling #{command}: #{e.class}: #{e}" end @@ -338,15 +324,13 @@ def implemented_by(command, type=:json) # This will not be called right when the daemon starts up, we use # lazy loading and initialization so it will only be called the first # time a request for this agent arrives. - def startup_hook - end + def startup_hook; end # Called just after a message was received from the middleware before # it gets passed to the handlers. @request and @reply will already be # set, the msg passed is the message as received from the normal # mcollective runner and the connection is the actual connector. - def before_processing_hook(msg, connection) - end + def before_processing_hook(msg, connection); end # Called at the end of processing just before the response gets sent # to the middleware. @@ -356,8 +340,7 @@ def before_processing_hook(msg, connection) # it is outside of the block is so you'll have access to even status codes # set by the exception handlers. If you do raise an exception it will just # be passed onto the runner and processing will fail. - def after_processing_hook - end + def after_processing_hook; end # Gets called right after a request was received and calls audit plugins # @@ -366,7 +349,7 @@ def after_processing_hook # do not care for the auditing in a specific agent. def audit_request(msg, connection) PluginManager["rpcaudit_plugin"].audit_request(msg, connection) if @config.rpcaudit - rescue Exception => e + rescue Exception => e # rubocop:disable Lint/RescueException Log.warn("Audit failed - #{e} - continuing to process message") end end diff --git a/lib/mcollective/rpc/audit.rb b/lib/mcollective/rpc/audit.rb index 72924220..9287e0cb 100644 --- a/lib/mcollective/rpc/audit.rb +++ b/lib/mcollective/rpc/audit.rb @@ -28,6 +28,7 @@ module RPC class Audit def self.inherited(klass) PluginManager << {:type => "rpcaudit_plugin", :class => klass.to_s} + super end def audit_request(request, connection) diff --git a/lib/mcollective/rpc/client.rb b/lib/mcollective/rpc/client.rb index 19ec96c2..de9b0e8e 100644 --- a/lib/mcollective/rpc/client.rb +++ b/lib/mcollective/rpc/client.rb @@ -4,10 +4,10 @@ module RPC # and just brings in a lot of convention and standard approached. class Client attr_accessor :timeout, :verbose, :filter, :config, :progress, :ttl, :reply_to - attr_reader :client, :stats, :ddl, :agent, :limit_targets, :limit_method, :output_format, :batch_size, :batch_sleep_time, :batch_mode - attr_reader :discovery_options, :discovery_method, :default_discovery_method, :limit_seed + attr_reader :client, :stats, :ddl, :agent, :limit_targets, :limit_method, :output_format, :batch_size, :batch_sleep_time, :batch_mode, :discovery_options, + :discovery_method, :default_discovery_method, :limit_seed - @@initial_options = nil + @@initial_options = nil # rubocop:disable Style/ClassVars # Creates a stub for a remote agent, you can pass in an options array in the flags # which will then be used else it will just create a default options array with @@ -17,7 +17,7 @@ class Client # # You typically would not call this directly you'd use MCollective::RPC#rpcclient instead # which is a wrapper around this that can be used as a Mixin - def initialize(agent, flags = {}) + def initialize(agent, flags={}) # rubocop:disable Metrics/MethodLength if flags.include?(:options) initial_options = flags[:options] @@ -25,22 +25,20 @@ def initialize(agent, flags = {}) initial_options = Marshal.load(@@initial_options) else - oparser = MCollective::Optionparser.new({ :verbose => false, - :progress_bar => true, - :mcollective_limit_targets => false, - :batch_size => nil, - :batch_sleep_time => 1 }, + oparser = MCollective::Optionparser.new({:verbose => false, + :progress_bar => true, + :mcollective_limit_targets => false, + :batch_size => nil, + :batch_sleep_time => 1}, "filter") initial_options = oparser.parse do |parser, opts| - if block_given? - yield(parser, opts) - end + yield(parser, opts) if block_given? Helpers.add_simplerpc_options(parser, opts) end - @@initial_options = Marshal.dump(initial_options) + @@initial_options = Marshal.dump(initial_options) # rubocop:disable Style/ClassVars end @initial_options = initial_options @@ -109,21 +107,21 @@ def initialize(agent, flags = {}) if initial_options[:stderr] @stderr = initial_options[:stderr] else - @stderr = STDERR + @stderr = $stderr @stderr.sync = true end if initial_options[:stdout] @stdout = initial_options[:stdout] else - @stdout = STDOUT + @stdout = $stdout @stdout.sync = true end if initial_options[:stdin] @stdin = initial_options[:stdin] else - @stdin = STDIN + @stdin = $stdin end end @@ -159,12 +157,12 @@ def help(template) def new_request(action, data) callerid = PluginManager["security_plugin"].callerid - raise 'callerid received from security plugin is not valid' unless PluginManager["security_plugin"].valid_callerid?(callerid) + raise "callerid received from security plugin is not valid" unless PluginManager["security_plugin"].valid_callerid?(callerid) - {:agent => @agent, + {:agent => @agent, :action => action, :caller => callerid, - :data => data} + :data => data} end # For the provided arguments and action the input arguments get @@ -238,7 +236,7 @@ def validate_request(action, args) # # This will do everything exactly as normal but communicate to only 5 # agents at a time - def method_missing(method_name, *args, &block) + def method_missing(method_name, *args, &block) # rubocop:disable Style/MissingRespondToMissing # set args to an empty hash if nothings given args = args[0] args = {} if args.nil? @@ -256,7 +254,6 @@ def method_missing(method_name, *args, &block) # if a global batch size is set just use that else set it # in the case that it was passed as an argument - batch_mode = args.include?(:batch_size) || @batch_mode batch_size = args.delete(:batch_size) || @batch_size batch_sleep_time = args.delete(:batch_sleep_time) || @batch_sleep_time @@ -308,7 +305,7 @@ def method_missing(method_name, *args, &block) # hash as a filter, this will force that request to be a directly addressed # request which technically does not need filters. If you try to use this # mode with direct addressing disabled an exception will be raise - def custom_request(action, args, expected_agents, filter = {}, &block) + def custom_request(action, args, expected_agents, filter={}, &block) validate_request(action, args) if filter == {} && !Config.instance.direct_addressing @@ -324,9 +321,7 @@ def custom_request(action, args, expected_agents, filter = {}, &block) # we could just use the merge method but I want to be sure # we dont merge in stuff that isnt actually valid ["identity", "fact", "agent", "cf_class", "compound"].each do |ftype| - if filter.include?(ftype) - custom_filter[ftype] = [filter[ftype], custom_filter[ftype]].flatten - end + custom_filter[ftype] = [filter[ftype], custom_filter[ftype]].flatten if filter.include?(ftype) end # ensure that all filters at least restrict the call to the agent we're a proxy for @@ -341,9 +336,7 @@ def custom_request(action, args, expected_agents, filter = {}, &block) # If a specific reply-to was set then from the client perspective this should # be a fire and forget request too since no response will ever reach us - it # will go to the reply-to destination - if args[:process_results] == false || @reply_to - return fire_and_forget_request(action, args, custom_filter) - end + return fire_and_forget_request(action, args, custom_filter) if args[:process_results] == false || @reply_to # Now do a call pretty much exactly like in method_missing except with our own # options and discovery magic @@ -358,7 +351,8 @@ def custom_request(action, args, expected_agents, filter = {}, &block) def discovery_timeout return @discovery_timeout if @discovery_timeout - return @client.discoverer.ddl.meta[:timeout] + + @client.discoverer.ddl.meta[:timeout] end def discovery_timeout=(timeout) @@ -423,12 +417,11 @@ def fact_filter(fact, value=nil, operator="=") if value.nil? parsed = Util.parse_fact_string(fact) - @filter["fact"] = @filter["fact"] | [parsed] unless parsed == false else parsed = Util.parse_fact_string("#{fact}#{operator}#{value}") - @filter["fact"] = @filter["fact"] | [parsed] unless parsed == false end + @filter["fact"] = @filter["fact"] | [parsed] unless parsed == false @filter["fact"].compact! reset end @@ -449,7 +442,7 @@ def identity_filter(identity) # Set a compound filter def compound_filter(filter) - @filter["compound"] = @filter["compound"] | [Matcher.create_compound_callstack(filter)] + @filter["compound"] = @filter["compound"] | [Matcher.create_compound_callstack(filter)] reset end @@ -477,9 +470,9 @@ def reset_filter # # Then we override discovery to try to grok the data on STDIN def detect_and_set_stdin_discovery - if self.default_discovery_method && !@stdin.tty? && !@stdin.eof? - self.discovery_method = 'stdin' - self.discovery_options = 'auto' + if default_discovery_method && !@stdin.tty? && !@stdin.eof? + self.discovery_method = "stdin" + self.discovery_options = "auto" end end @@ -499,7 +492,7 @@ def detect_and_set_stdin_discovery # # Use reset to force a new discovery def discover(flags={}) - flags.keys.each do |key| + flags.each_key do |key| raise "Unknown option #{key} passed to discover" unless [:verbose, :hosts, :nodes, :json].include?(key) end @@ -565,8 +558,8 @@ def discover(flags={}) # and if we're configured to use the first found hosts as the # limit method then pass in the limit thus minimizing the amount # of work we do in the discover phase and speeding it up significantly - filter = @filter.merge({'collective' => @collective}) - if @limit_method == :first and @limit_targets.is_a?(Integer) + filter = @filter.merge({"collective" => @collective}) + if (@limit_method == :first) && @limit_targets.is_a?(Integer) @discovered_agents = @client.discover(filter, discovery_timeout, @limit_targets) else @discovered_agents = @client.discover(filter, discovery_timeout) @@ -604,10 +597,10 @@ def options end # Sets the collective we are communicating with - def collective=(c) - raise "Unknown collective #{c}" unless Config.instance.collectives.include?(c) + def collective=(collective) + raise "Unknown collective #{collective}" unless Config.instance.collectives.include?(collective) - @collective = c + @collective = collective @client.options = options reset end @@ -616,7 +609,7 @@ def collective=(c) # used to restrict how many nodes we'll target # Limit targets can be reset by passing nil or false def limit_targets=(limit) - if !limit + unless limit @limit_targets = nil return end @@ -646,9 +639,7 @@ def limit_method=(method) # Sets the batch size, if the size is set to 0 that will disable batch mode def batch_size=(limit) - unless Config.instance.direct_addressing - raise "Can only set batch size if direct addressing is supported" - end + raise "Can only set batch size if direct addressing is supported" unless Config.instance.direct_addressing validate_batch_size(limit) @@ -721,21 +712,20 @@ def load_aggregate_functions(action, ddl) return nil unless ddl return nil unless ddl.action_interface(action).keys.include?(:aggregate) - return Aggregate.new(ddl.action_interface(action)) - + Aggregate.new(ddl.action_interface(action)) rescue => e Log.error("Failed to load aggregate functions, calculating summaries disabled: %s: %s (%s)" % [e.backtrace.first, e.to_s, e.class]) - return nil + nil end def aggregate_reply(reply, aggregate) return nil unless aggregate aggregate.call_functions(reply) - return aggregate - rescue Exception => e + aggregate + rescue Exception => e # rubocop:disable Lint/RescueException Log.error("Failed to calculate aggregate summaries for reply from %s, calculating summaries disabled: %s: %s (%s)" % [reply[:senderid], e.backtrace.first, e.to_s, e.class]) - return nil + nil end def rpc_result_from_reply(agent, action, reply) @@ -763,7 +753,7 @@ def fire_and_forget_request(action, args, filter=nil) req = new_request(action.to_s, args) - filter = options[:filter] unless filter + filter ||= options[:filter] message = Message.new(req, nil, {:agent => @agent, :type => :request, :collective => @collective, :filter => filter, :options => options}) message.reply_to = @reply_to if @reply_to @@ -788,8 +778,8 @@ def fire_and_forget_request(action, args, filter=nil) # the concept of identity to mean something else so we should pass the full # identity filter to them def identity_filter_discovery_optimization - if options[:filter]["identity"].size > 0 && @discovery_method == "mc" - regex_filters = options[:filter]["identity"].select{|i| i.match("^\/")}.size + if !options[:filter]["identity"].empty? && @discovery_method == "mc" + regex_filters = options[:filter]["identity"].select {|i| i.match("^\/")}.size if regex_filters == 0 @discovered_agents = options[:filter]["identity"].clone @@ -805,9 +795,10 @@ def identity_filter_discovery_optimization # from normal call_agent. # # This is used by method_missing and works only with direct addressing mode - def call_agent_batched(action, args, opts, batch_size, sleep_time, &block) + def call_agent_batched(action, args, opts, batch_size, sleep_time, &block) # rubocop:disable Metrics/MethodLength raise "Batched requests requires direct addressing" unless Config.instance.direct_addressing raise "Cannot bypass result processing for batched requests" if args[:process_results] == false + validate_batch_size(batch_size) sleep_time = Float(sleep_time) @@ -820,7 +811,7 @@ def call_agent_batched(action, args, opts, batch_size, sleep_time, &block) results = [] respcount = 0 - if discovered.size > 0 + if !discovered.empty? req = new_request(action.to_s, args) aggregate = load_aggregate_functions(action, @ddl) @@ -831,7 +822,7 @@ def call_agent_batched(action, args, opts, batch_size, sleep_time, &block) @stdout.print twirl.twirl(respcount, discovered.size) end - if (batch_size =~ /^(\d+)%$/) + if batch_size =~ /^(\d+)%$/ # determine batch_size as a percentage of the discovered array's size batch_size = (discovered.size / 100.0 * Integer($1)).ceil else @@ -869,9 +860,7 @@ def call_agent_batched(action, args, opts, batch_size, sleep_time, &block) end end - if @initial_options[:sort] - results.sort! - end + results.sort! if @initial_options[:sort] @stats.noresponsefrom.concat @client.stats[:noresponsefrom] @stats.unexpectedresponsefrom.concat @client.stats[:unexpectedresponsefrom] @@ -881,9 +870,7 @@ def call_agent_batched(action, args, opts, batch_size, sleep_time, &block) @stats.discoverytime += @client.stats[:discoverytime] processed_nodes += hosts.length - if (discovered.length > processed_nodes) - sleep sleep_time - end + sleep sleep_time if discovered.length > processed_nodes end @stats.aggregate_summary = aggregate.summarize if aggregate @@ -899,9 +886,9 @@ def call_agent_batched(action, args, opts, batch_size, sleep_time, &block) @stdout.print("\n") if @progress if block_given? - return stats + stats else - return [results].flatten + [results].flatten end end @@ -942,7 +929,7 @@ def call_agent(action, args, opts, disc=:auto, &block) results = [] respcount = 0 - if discovered.size > 0 + if !discovered.empty? message.type = :direct_request if @force_direct_request if @progress && !block_given? @@ -967,9 +954,7 @@ def call_agent(action, args, opts, disc=:auto, &block) end end - if @initial_options[:sort] - results.sort! - end + results.sort! if @initial_options[:sort] @stats.aggregate_summary = aggregate.summarize if aggregate @stats.aggregate_failures = aggregate.failed if aggregate @@ -985,9 +970,9 @@ def call_agent(action, args, opts, disc=:auto, &block) @stdout.print("\n\n") if @progress if block_given? - return stats + stats else - return [results].flatten + [results].flatten end end @@ -1025,25 +1010,23 @@ def process_results_with_block(action, resp, block, aggregate) @stats.time_block_execution :start case block.arity - when 1 - block.call(resp) - when 2 - block.call(resp, result) + when 1 + block.call(resp) + when 2 + block.call(resp, result) end @stats.time_block_execution :end - return aggregate + aggregate end private def determine_batch_mode(batch_size) - if (batch_size != 0 && batch_size != "0") - return true - end + return true if batch_size != 0 && batch_size != "0" - return false + false end # Validate the bach_size based on the following criteria @@ -1051,12 +1034,11 @@ def determine_batch_mode(batch_size) # batch_size is a string of digits # batch_size is of type Integer def validate_batch_size(batch_size) - if (batch_size.is_a?(Integer)) + case batch_size + when Integer return - elsif (batch_size.is_a?(String)) - if ((batch_size =~ /^(\d+)%$/ && Integer($1) != 0) || batch_size =~ /^(\d+)$/) - return - end + when String + return if (batch_size =~ /^(\d+)%$/ && Integer($1) != 0) || batch_size =~ /^(\d+)$/ end raise("batch_size must be an integer or match a percentage string (e.g. '24%'") diff --git a/lib/mcollective/rpc/helpers.rb b/lib/mcollective/rpc/helpers.rb index f6fd8b36..d650ca2f 100644 --- a/lib/mcollective/rpc/helpers.rb +++ b/lib/mcollective/rpc/helpers.rb @@ -26,9 +26,7 @@ def self.extract_hosts_from_json(json) hosts.map do |host| raise "JSON host list is not an array of Hashes" unless host.is_a?(Hash) - unless host.include?("sender") || host.include?("certname") - raise "JSON host list does not have senders in it" - end + raise "JSON host list does not have senders in it" unless host.include?("sender") || host.include?("certname") host["sender"] || host["certname"] end.uniq @@ -39,6 +37,7 @@ def self.extract_hosts_from_json(json) def self.extract_hosts_from_array(hosts) [hosts].flatten.map do |host| raise "#{host} should be a string" unless host.is_a?(String) + host.chomp end end @@ -61,7 +60,7 @@ def self.extract_hosts_from_array(hosts) # If you've asked it to flatten the result it will not print sender # hostnames, it will just print the result as if it's one huge result, # handy for things like showing a combined mailq. - def self.rpcresults(result, flags = {}) + def self.rpcresults(result, flags={}) flags = {:verbose => false, :flatten => false, :format => :console, :force_display_mode => false}.merge(flags) result_text = "" @@ -70,55 +69,49 @@ def self.rpcresults(result, flags = {}) # if running in verbose mode, just use the old style print # no need for all the DDL helpers obfuscating the result if flags[:format] == :json - if STDOUT.tty? + if $stdout.tty? result_text = JSON.pretty_generate(result) else result_text = result.to_json end + elsif flags[:verbose] + result_text = old_rpcresults(result, flags) else - if flags[:verbose] - result_text = old_rpcresults(result, flags) - else - [result].flatten.each do |r| - begin - ddl ||= DDL.new(r.agent).action_interface(r.action.to_s) + [result].flatten.each do |r| + begin + ddl ||= DDL.new(r.agent).action_interface(r.action.to_s) - sender = r[:sender] - status = r[:statuscode] - message = r[:statusmsg] - result = r[:data] + sender = r[:sender] + status = r[:statuscode] + message = r[:statusmsg] + result = r[:data] - if flags[:force_display_mode] - display = flags[:force_display_mode] - else - display = ddl[:display] - end + if flags[:force_display_mode] + display = flags[:force_display_mode] + else + display = ddl[:display] + end - # appand the results only according to what the DDL says - case display - when :ok - if status == 0 - result_text << text_for_result(sender, status, message, result, ddl) - end + # appand the results only according to what the DDL says + case display + when :ok + result_text << text_for_result(sender, status, message, result, ddl) if status == 0 - when :failed - if status > 0 - result_text << text_for_result(sender, status, message, result, ddl) - end + when :failed + result_text << text_for_result(sender, status, message, result, ddl) if status > 0 - when :always - result_text << text_for_result(sender, status, message, result, ddl) + when :always + result_text << text_for_result(sender, status, message, result, ddl) - when :flatten - Log.warn("The display option :flatten is being deprecated and will be removed in the next minor release") - result_text << text_for_flattened_result(status, result) + when :flatten + Log.warn("The display option :flatten is being deprecated and will be removed in the next minor release") + result_text << text_for_flattened_result(status, result) - end - rescue Exception => e - # no DDL so just do the old style print unchanged for - # backward compat - result_text = old_rpcresults(result, flags) end + rescue Exception # rubocop:disable Lint/RescueException + # no DDL so just do the old style print unchanged for + # backward compat + result_text = old_rpcresults(result, flags) end end end @@ -151,7 +144,7 @@ def self.text_for_result(sender, status, msg, result, ddl) end end - result.keys.sort_by{|k| k}.each do |k| + result.keys.sort.each do |k| # get all the output fields nicely lined up with a # 3 space front padding begin @@ -169,6 +162,7 @@ def self.text_for_result(sender, status, msg, result, ddl) if [String, Numeric].include?(result[k].class) lines = result[k].to_s.split("\n") + # rubocop:disable Metrics/BlockNesting if lines.empty? result_text << "\n" else @@ -178,6 +172,7 @@ def self.text_for_result(sender, status, msg, result, ddl) result_text << "#{padtxt}#{line}\n" end end + # rubocop:enable Metrics/BlockNesting else padding = " " * (lengths.max + 5) result_text << " " << result[k].pretty_inspect.split("\n").join("\n" << padding) << "\n" @@ -188,7 +183,7 @@ def self.text_for_result(sender, status, msg, result, ddl) # data by default since the DDL will supply all the defaults # it just doesnt look right else - result_text << "\n\t" + result.pretty_inspect.split("\n").join("\n\t") + result_text << "\n\t#{result.pretty_inspect.split("\n").join("\n\t")}" end end @@ -201,16 +196,16 @@ def self.text_for_flattened_result(status, result) result_text = "" if status <= 1 - unless result.is_a?(String) - result_text << result.pretty_inspect - else + if result.is_a?(String) result_text << result + else + result_text << result.pretty_inspect end end end # Backward compatible display block for results without a DDL - def self.old_rpcresults(result, flags = {}) + def self.old_rpcresults(result, flags={}) result_text = "" if flags[:flatten] @@ -218,10 +213,10 @@ def self.old_rpcresults(result, flags = {}) if r[:statuscode] <= 1 data = r[:data] - unless data.is_a?(String) - result_text << data.pretty_inspect - else + if data.is_a?(String) result_text << data + else + result_text << data.pretty_inspect end else result_text << r.pretty_inspect @@ -250,9 +245,7 @@ def self.old_rpcresults(result, flags = {}) result_text << " #{r[:statusmsg]}" end else - unless r[:statuscode] == 0 - result_text << "%-40s %s\n" % [r[:sender], Util.colorize(:red, r[:statusmsg])] - end + result_text << "%-40s %s\n" % [r[:sender], Util.colorize(:red, r[:statusmsg])] unless r[:statuscode] == 0 end end end @@ -266,32 +259,32 @@ def self.add_simplerpc_options(parser, options) parser.separator "RPC Options" # add SimpleRPC specific options to all clients that use our library - parser.on('--np', '--no-progress', 'Do not show the progress bar') do |v| + parser.on("--np", "--no-progress", "Do not show the progress bar") do |_v| options[:progress_bar] = false end - parser.on('--one', '-1', 'Send request to only one discovered nodes') do |v| + parser.on("--one", "-1", "Send request to only one discovered nodes") do |_v| options[:mcollective_limit_targets] = 1 end - parser.on('--batch SIZE', 'Do requests in batches') do |v| + parser.on("--batch SIZE", "Do requests in batches") do |v| # validate batch string. Is it x% where x > 0 or is it an integer - if ((v =~ /^(\d+)%$/ && Integer($1) != 0) || v =~ /^(\d+)$/) + if (v =~ /^(\d+)%$/ && Integer($1) != 0) || v =~ /^(\d+)$/ options[:batch_size] = v else - raise(::OptionParser::InvalidArgument.new(v)) + raise ::OptionParser::InvalidArgument, v end end - parser.on('--batch-sleep SECONDS', Float, 'Sleep time between batches') do |v| + parser.on("--batch-sleep SECONDS", Float, "Sleep time between batches") do |v| options[:batch_sleep_time] = v end - parser.on('--limit-seed NUMBER', Integer, 'Seed value for deterministic random batching') do |v| + parser.on("--limit-seed NUMBER", Integer, "Seed value for deterministic random batching") do |v| options[:limit_seed] = v end - parser.on('--limit-nodes COUNT', '--ln', '--limit', 'Send request to only a subset of nodes, can be a percentage') do |v| + parser.on("--limit-nodes COUNT", "--ln", "--limit", "Send request to only a subset of nodes, can be a percentage") do |v| raise "Invalid limit specified: #{v} valid limits are /^\d+%*$/" unless v =~ /^\d+%*$/ if v =~ /^\d+$/ @@ -301,12 +294,12 @@ def self.add_simplerpc_options(parser, options) end end - parser.on('--json', '-j', 'Produce JSON output') do |v| + parser.on("--json", "-j", "Produce JSON output") do |_v| options[:progress_bar] = false options[:output_format] = :json end - parser.on('--display MODE', 'Influence how results are displayed. One of ok, all or failed') do |v| + parser.on("--display MODE", "Influence how results are displayed. One of ok, all or failed") do |v| if v == "all" options[:force_display_mode] = :always else diff --git a/lib/mcollective/rpc/progress.rb b/lib/mcollective/rpc/progress.rb index 8c2f50bb..091ed1a0 100644 --- a/lib/mcollective/rpc/progress.rb +++ b/lib/mcollective/rpc/progress.rb @@ -13,7 +13,7 @@ module RPC # * [ ==================================================> ] 100 / 100 class Progress def initialize(size=nil) - @twirl = ['|', '/', '-', "\\", '|', '/', '-', "\\"] + @twirl = ["|", "/", "-", "\\", "|", "/", "-", "\\"] @twirldex = 0 if size @@ -56,7 +56,7 @@ def twirl(current, total) @twirldex == 7 ? @twirldex = 0 : @twirldex += 1 - return txt + txt end end end diff --git a/lib/mcollective/rpc/reply.rb b/lib/mcollective/rpc/reply.rb index 0dc8b5a4..c191212f 100644 --- a/lib/mcollective/rpc/reply.rb +++ b/lib/mcollective/rpc/reply.rb @@ -13,19 +13,17 @@ def initialize(action, ddl) begin initialize_data - rescue Exception => e - Log.warn("Could not pre-populate reply data from the DDL: %s: %s" % [e.class, e.to_s ]) + rescue Exception => e # rubocop:disable Lint/RescueException + Log.warn("Could not pre-populate reply data from the DDL: %s: %s" % [e.class, e.to_s]) end end def initialize_data - unless @ddl.actions.include?(@action) - raise "No action '%s' defined for agent '%s' in the DDL" % [@action, @ddl.pluginname] - end + raise "No action '%s' defined for agent '%s' in the DDL" % [@action, @ddl.pluginname] unless @ddl.actions.include?(@action) interface = @ddl.action_interface(@action) - interface[:output].keys.each do |output| + interface[:output].each_key do |output| # must deep clone this data to avoid accidental updates of the DDL in cases where the # default is for example a string and someone does << on it @data[output] = Marshal.load(Marshal.dump(interface[:output][output].fetch(:default, nil))) @@ -44,20 +42,20 @@ def fail!(msg, code=1) @statuscode = code case code - when 1 - raise RPCAborted, msg + when 1 + raise RPCAborted, msg - when 2 - raise UnknownRPCAction, msg + when 2 + raise UnknownRPCAction, msg - when 3 - raise MissingRPCData, msg + when 3 + raise MissingRPCData, msg - when 4 - raise InvalidRPCData, msg + when 4 + raise InvalidRPCData, msg - else - raise UnknownRPCError, msg + else + raise UnknownRPCError, msg end end @@ -78,9 +76,9 @@ def fetch(key, default) # Returns a compliant Hash of the reply that should be sent # over the middleware def to_hash - return {:statuscode => @statuscode, - :statusmsg => @statusmsg, - :data => @data} + {:statuscode => @statuscode, + :statusmsg => @statusmsg, + :data => @data} end end end diff --git a/lib/mcollective/rpc/request.rb b/lib/mcollective/rpc/request.rb index dc9dc095..4c0397d9 100644 --- a/lib/mcollective/rpc/request.rb +++ b/lib/mcollective/rpc/request.rb @@ -57,12 +57,14 @@ def should_respond? # If data is a hash, gives easy access to its members, else returns nil def [](key) return nil unless @data.is_a?(Hash) - return @data[compatible_key(key)] + + @data[compatible_key(key)] end def fetch(key, default) return nil unless @data.is_a?(Hash) - return @data.fetch(compatible_key(key), default) + + @data.fetch(compatible_key(key), default) end def to_hash @@ -76,10 +78,10 @@ def validate! @ddl.validate_rpc_request(@action, @data) end - def to_json - to_hash.merge!({:sender => @sender, + def to_json(*_args) + to_hash.merge!({:sender => @sender, :callerid => @callerid, - :uniqid => @uniqid}).to_json + :uniqid => @uniqid}).to_json end end end diff --git a/lib/mcollective/rpc/result.rb b/lib/mcollective/rpc/result.rb index 257a2f6c..10a1147a 100644 --- a/lib/mcollective/rpc/result.rb +++ b/lib/mcollective/rpc/result.rb @@ -40,12 +40,10 @@ def convert_data_based_on_ddl return if interface.fetch(:output, {}).empty? - interface[:output].each do |output, properties| + interface[:output].each do |output, _properties| next if data.include?(output) - if output.is_a?(Symbol) && data.include?(output.to_s) - data[output] = data.delete(output.to_s) - end + data[output] = data.delete(output.to_s) if output.is_a?(Symbol) && data.include?(output.to_s) end end @@ -69,17 +67,17 @@ def fetch(key, default) @results.fetch(compatible_key(key), default) end - def each - @results.each_pair {|k,v| yield(k,v) } + def each(&block) + @results.each_pair(&block) end - def to_json(*a) + def to_json(*result) {:agent => @agent, :action => @action, :sender => self[:sender], :statuscode => self[:statuscode], :statusmsg => self[:statusmsg], - :data => data}.to_json(*a) + :data => data}.to_json(*result) end def <=>(other) diff --git a/lib/mcollective/rpc/stats.rb b/lib/mcollective/rpc/stats.rb index 1b0ad4c2..676d110a 100644 --- a/lib/mcollective/rpc/stats.rb +++ b/lib/mcollective/rpc/stats.rb @@ -2,9 +2,8 @@ module MCollective module RPC # Class to wrap all the stats and to keep track of some timings class Stats - attr_accessor :noresponsefrom, :unexpectedresponsefrom, :starttime, :discoverytime, :blocktime, :responses - attr_accessor :totaltime, :discovered, :discovered_nodes, :okcount, :failcount, :noresponsefrom - attr_accessor :responsesfrom, :requestid, :aggregate_summary, :ddl, :aggregate_failures + attr_accessor :noresponsefrom, :unexpectedresponsefrom, :starttime, :discoverytime, :blocktime, :responses, :totaltime, :discovered, :discovered_nodes, :okcount, + :failcount, :responsesfrom, :requestid, :aggregate_summary, :ddl, :aggregate_failures def initialize reset @@ -17,7 +16,7 @@ def reset @responsesfrom = [] @responses = 0 @starttime = Time.now.to_f - @discoverytime = 0 unless @discoverytime + @discoverytime ||= 0 @blocktime = 0 @totaltime = 0 @discovered = 0 @@ -31,18 +30,18 @@ def reset # returns a hash of our stats def to_hash - {:noresponsefrom => @noresponsefrom, + {:noresponsefrom => @noresponsefrom, :unexpectedresponsefrom => @unexpectedresponsefrom, - :starttime => @starttime, - :discoverytime => @discoverytime, - :blocktime => @blocktime, - :responses => @responses, - :totaltime => @totaltime, - :discovered => @discovered, - :discovered_nodes => @discovered_nodes, - :okcount => @okcount, - :requestid => @requestid, - :failcount => @failcount, + :starttime => @starttime, + :discoverytime => @discoverytime, + :blocktime => @blocktime, + :responses => @responses, + :totaltime => @totaltime, + :discovered => @discovered, + :discovered_nodes => @discovered_nodes, + :okcount => @okcount, + :requestid => @requestid, + :failcount => @failcount, :aggregate_summary => @aggregate_summary, :aggregate_failures => @aggregate_failures} end @@ -82,9 +81,10 @@ def client_stats=(stats) # Utility to time discovery from :start to :end def time_discovery(action) - if action == :start + case action + when :start @discovery_start = Time.now.to_f - elsif action == :end + when :end @discoverytime = Time.now.to_f - @discovery_start else raise("Uknown discovery action #{action}") @@ -95,9 +95,10 @@ def time_discovery(action) # helper to time block execution time def time_block_execution(action) - if action == :start + case action + when :start @block_start = Time.now.to_f - elsif action == :end + when :end @blocktime += Time.now.to_f - @block_start else raise("Uknown block action #{action}") @@ -156,16 +157,16 @@ def text_for_aggregates result.puts Util.colorize(:bold, "Summary of %s:" % display_as) result.puts - unless aggregate_report == "" - result.puts aggregate.to_s.split("\n").map{|x| " " + x}.join("\n") - else + if aggregate_report == "" result.puts Util.colorize(:yellow, " No aggregate summary could be computed") + else + result.puts aggregate.to_s.split("\n").map {|x| " #{x}"}.join("\n") end result.puts end @aggregate_failures.each do |failed| - case(failed[:type]) + case (failed[:type]) when :startup message = "exception raised while processing startup hook" when :create @@ -187,65 +188,55 @@ def text_for_aggregates # Returns a blob of text representing the request status based on the # stats contained in this class - def report(caption = "rpc stats", summarize = true, verbose = false) + def report(caption="rpc stats", summarize=true, verbose=false) result_text = [] if verbose - if @aggregate_summary.size > 0 && summarize - result_text << text_for_aggregates - else - result_text << "" - end + if !@aggregate_summary.empty? && summarize + result_text << text_for_aggregates + else + result_text << "" + end result_text << Util.colorize(:yellow, "---- #{caption} ----") if @discovered @responses < @discovered ? color = :red : color = :reset - result_text << " Nodes: %s / %s" % [ Util.colorize(color, @discovered), Util.colorize(color, @responses) ] + result_text << " Nodes: %s / %s" % [Util.colorize(color, @discovered), Util.colorize(color, @responses)] else result_text << " Nodes: #{@responses}" end @failcount < 0 ? color = :red : color = :reset - result_text << " Pass / Fail: %s / %s" % [Util.colorize(color, @okcount), Util.colorize(color, @failcount) ] + result_text << " Pass / Fail: %s / %s" % [Util.colorize(color, @okcount), Util.colorize(color, @failcount)] result_text << " Start Time: %s" % [Time.at(@starttime)] result_text << " Discovery Time: %.2fms" % [@discoverytime * 1000] result_text << " Agent Time: %.2fms" % [@blocktime * 1000] result_text << " Total Time: %.2fms" % [@totaltime * 1000] - else - if @discovered - @responses < @discovered ? color = :red : color = :green - - if @aggregate_summary.size + @aggregate_failures.size > 0 && summarize - result_text << text_for_aggregates - else - result_text << "" - end + elsif @discovered + @responses < @discovered ? color = :red : color = :green - result_text << "Finished processing %s / %s hosts in %.2f ms" % [Util.colorize(color, @responses), Util.colorize(color, @discovered), @blocktime * 1000] + if @aggregate_summary.size + @aggregate_failures.size > 0 && summarize + result_text << text_for_aggregates else - result_text << "Finished processing %s hosts in %.2f ms" % [Util.colorize(:bold, @responses), @blocktime * 1000] + result_text << "" end + + result_text << "Finished processing %s / %s hosts in %.2f ms" % [Util.colorize(color, @responses), Util.colorize(color, @discovered), @blocktime * 1000] + else + result_text << "Finished processing %s hosts in %.2f ms" % [Util.colorize(:bold, @responses), @blocktime * 1000] end no_response_r = no_response_report unexpected_response_r = unexpected_response_report - if no_response_r || unexpected_response_r - result_text << "" - end + result_text << "" if no_response_r || unexpected_response_r - if no_response_r != "" - result_text << "" << no_response_r - end + result_text << "" << no_response_r if no_response_r != "" - if unexpected_response_r != "" - result_text << "" << unexpected_response_r - end + result_text << "" << unexpected_response_r if unexpected_response_r != "" - if no_response_r || unexpected_response_r - result_text << "" - end + result_text << "" if no_response_r || unexpected_response_r result_text.join("\n") end @@ -254,13 +245,13 @@ def report(caption = "rpc stats", summarize = true, verbose = false) def no_response_report result_text = StringIO.new - if @noresponsefrom.size > 0 + unless @noresponsefrom.empty? result_text.puts Util.colorize(:red, "No response from:") result_text.puts field_size = Util.field_size(@noresponsefrom, 30) fields_num = Util.field_number(field_size) - format = " " + ( " %-#{field_size}s" * fields_num ) + format = " #{" %-#{field_size}s" * fields_num}" @noresponsefrom.sort.in_groups_of(fields_num) do |c| result_text.puts format % c @@ -274,13 +265,13 @@ def no_response_report def unexpected_response_report result_text = StringIO.new - if @unexpectedresponsefrom.size > 0 + unless @unexpectedresponsefrom.empty? result_text.puts Util.colorize(:red, "Unexpected response from:") result_text.puts field_size = Util.field_size(@unexpectedresponsefrom, 30) fields_num = Util.field_number(field_size) - format = " " + ( " %-#{field_size}s" * fields_num ) + format = " #{" %-#{field_size}s" * fields_num}" @unexpectedresponsefrom.sort.in_groups_of(fields_num) do |c| result_text.puts format % c diff --git a/lib/mcollective/security/base.rb b/lib/mcollective/security/base.rb index bcedf721..dc760fba 100644 --- a/lib/mcollective/security/base.rb +++ b/lib/mcollective/security/base.rb @@ -31,6 +31,7 @@ class Base # Register plugins that inherits base def self.inherited(klass) PluginManager << {:type => "security_plugin", :class => klass.to_s} + super end # Initializes configuration and logging as well as prepare a zero'd hash of stats @@ -52,18 +53,18 @@ def initialize # - identity - the configured identity of the system # # TODO: Support REGEX and/or multiple filter keys to be AND'd - def validate_filter?(filter) + def validate_filter?(filter) # rubocop:disable Metrics/MethodLength failed = 0 passed = 0 passed = 1 if Util.empty_filter?(filter) - filter.keys.each do |key| + filter.each_key do |key| case key when /puppet_class|cf_class/ filter[key].each do |f| Log.debug("Checking for class #{f}") - if Util.has_cf_class?(f) then + if Util.has_cf_class?(f) Log.debug("Passing based on configuration management class #{f}") passed += 1 else @@ -80,34 +81,34 @@ def validate_filter?(filter) begin compound.each do |expression| case expression.keys.first - when "statement" - truth_values << Matcher.eval_compound_statement(expression).to_s - when "fstatement" - truth_values << Matcher.eval_compound_fstatement(expression.values.first) - when "and" - truth_values << "&&" - when "or" - truth_values << "||" - when "(" - truth_values << "(" - when ")" - truth_values << ")" - when "not" - truth_values << "!" + when "statement" + truth_values << Matcher.eval_compound_statement(expression).to_s + when "fstatement" + truth_values << Matcher.eval_compound_fstatement(expression.values.first) + when "and" + truth_values << "&&" + when "or" + truth_values << "||" + when "(" + truth_values << "(" + when ")" + truth_values << ")" + when "not" + truth_values << "!" end end - result = eval(truth_values.join(" ")) + result = eval(truth_values.join(" ")) # rubocop:disable Security/Eval rescue DDLValidationError result = false end if result Log.debug("Passing based on class and fact composition") - passed +=1 + passed += 1 else Log.debug("Failing based on class and fact composition") - failed +=1 + failed += 1 end end @@ -136,7 +137,7 @@ def validate_filter?(filter) when "identity" unless filter[key].empty? # Identity filters should not be 'and' but 'or' as each node can only have one identity - matched = filter[key].select{|f| Util.has_identity?(f)}.size + matched = filter[key].select {|f| Util.has_identity?(f)}.size if matched == 1 Log.debug("Passing based on identity") @@ -154,13 +155,13 @@ def validate_filter?(filter) @stats.passed - return true + true else Log.debug("Message failed the filter checks") @stats.filtered - return false + false end end @@ -194,12 +195,10 @@ def create_request(reqid, filter, msg, initiated_by, target_agent, target_collec # Mostly used by security plugins to figure out if they should do the hard work of decrypting # etc messages that would only later on be ignored def should_process_msg?(msg, msgid) - if msg.expected_msgid - unless msg.expected_msgid == msgid - msgtext = "Got a message with id %s but was expecting %s, ignoring message" % [msgid, msg.expected_msgid] - Log.debug msgtext - raise MsgDoesNotMatchRequestID, msgtext - end + if msg.expected_msgid && msg.expected_msgid != msgid + msgtext = "Got a message with id %s but was expecting %s, ignoring message" % [msgid, msg.expected_msgid] + Log.debug msgtext + raise MsgDoesNotMatchRequestID, msgtext end true @@ -211,7 +210,7 @@ def should_process_msg?(msg, msgid) # callerids are generally in the form uid=123 or cert=foo etc so we do that # here but security plugins could override this for some complex uses def valid_callerid?(id) - !!id.match(/^[\w]+=[\w\.\-]+$/) + !!id.match(/^\w+=[\w.\-]+$/) end # Returns a unique id for the caller, by default we just use the unix diff --git a/lib/mcollective/shell.rb b/lib/mcollective/shell.rb index 4d7d6d28..36da3d9c 100644 --- a/lib/mcollective/shell.rb +++ b/lib/mcollective/shell.rb @@ -38,18 +38,22 @@ def initialize(command, options={}) case opt.to_s when "stdout" raise "stdout should support <<" unless val.respond_to?("<<") + @stdout = val when "stderr" raise "stderr should support <<" unless val.respond_to?("<<") + @stderr = val when "stdin" raise "stdin should be a String" unless val.is_a?(String) + @stdin = val when "cwd" raise "Directory #{val} does not exist" unless File.directory?(val) + @cwd = val when "environment" @@ -62,6 +66,7 @@ def initialize(command, options={}) when "timeout" raise "timeout should be a positive integer or the symbol :on_thread_exit symbol" unless val.eql?(:on_thread_exit) || (val.is_a?(Integer) && val > 0) + @timeout = val end end @@ -69,10 +74,10 @@ def initialize(command, options={}) # Actually does the systemu call passing in the correct environment, stdout and stderr def runcommand - opts = {"env" => @environment, + opts = {"env" => @environment, "stdout" => @stdout, "stderr" => @stderr, - "cwd" => @cwd} + "cwd" => @cwd} opts["stdin"] = @stdin if @stdin @@ -108,8 +113,8 @@ def runcommand # only wait if the parent thread is dead Process.waitpid(cid) unless thread.alive? end - rescue SystemExit # rubocop:disable Lint/HandleExceptions - rescue Errno::ESRCH # rubocop:disable Lint/HandleExceptions + rescue SystemExit # rubocop:disable Lint/SuppressedException + rescue Errno::ESRCH # rubocop:disable Lint/SuppressedException rescue Errno::ECHILD Log.warn("Could not reap process '#{cid}'.") rescue Exception => e # rubocop:disable Lint/RescueException diff --git a/lib/mcollective/ssl.rb b/lib/mcollective/ssl.rb index 9ad8c5ca..7da7f096 100644 --- a/lib/mcollective/ssl.rb +++ b/lib/mcollective/ssl.rb @@ -166,7 +166,7 @@ def aes_decrypt(key, crypt_string) # Signs a string using the private key def sign(string, base64=false) - sig = @private_key.sign(OpenSSL::Digest::SHA1.new, string) + sig = @private_key.sign(OpenSSL::Digest.new("SHA1"), string) base64 ? base64_encode(sig) : sig end @@ -175,7 +175,7 @@ def sign(string, base64=false) def verify_signature(signature, string, base64=false) signature = base64_decode(signature) if base64 - @public_key.verify(OpenSSL::Digest::SHA1.new, signature, string) + @public_key.verify(OpenSSL::Digest.new("SHA1"), signature, string) end # base 64 encode a string @@ -196,6 +196,7 @@ def self.base64_decode(string) # The Base 64 character set is A-Z a-z 0-9 + / = # Also allow for whitespace, but raise if we get anything else raise(ArgumentError, "invalid base64") if string !~ /^[A-Za-z0-9+\/=\s]+$/ + Base64.decode64(string) end @@ -248,7 +249,8 @@ def read_key(type, key=nil, passphrase=nil) raise "Could not find key #{key}" unless File.exist?(key) raise "#{type} key file '#{key}' is empty" if File.zero?(key) - if type == :public + case type + when :public begin key = OpenSSL::PKey::RSA.new(File.read(key)) rescue OpenSSL::PKey::RSAError @@ -271,9 +273,9 @@ def read_key(type, key=nil, passphrase=nil) # See http://bugs.ruby-lang.org/issues/4550 OpenSSL.errors if Util.ruby_version =~ /^1.8/ - return key - elsif type == :private - return OpenSSL::PKey::RSA.new(File.read(key), passphrase) + key + when :private + OpenSSL::PKey::RSA.new(File.read(key), passphrase) else raise "Can only load :public or :private keys" end diff --git a/lib/mcollective/util.rb b/lib/mcollective/util.rb index df90e1e3..d4ca7766 100644 --- a/lib/mcollective/util.rb +++ b/lib/mcollective/util.rb @@ -10,12 +10,12 @@ def self.has_agent?(agent) if agent.is_a?(Regexp) if !Agents.agentlist.grep(agent).empty? - return true + true else - return false + false end else - return Agents.agentlist.include?(agent) + Agents.agentlist.include?(agent) end end @@ -41,9 +41,10 @@ def self.has_cf_class?(klass) begin File.readlines(cfile).each do |k| - if klass.is_a?(Regexp) + case klass + when Regexp return true if k.chomp.match(klass) - elsif k.chomp == klass + when k.chomp return true end end @@ -74,11 +75,11 @@ def self.has_fact?(fact, value, operator) fact = fact.clone case fact when Array - return fact.any? { |element| test_fact_value(element, value, operator)} + fact.any? { |element| test_fact_value(element, value, operator)} when Hash - return fact.keys.any? { |element| test_fact_value(element, value, operator)} + fact.keys.any? { |element| test_fact_value(element, value, operator)} else - return test_fact_value(fact, value, operator) + test_fact_value(fact, value, operator) end end @@ -104,7 +105,7 @@ def self.test_fact_value(fact, value, operator) value = Float(value) # rubocop:disable Lint/UselessAssignment end - return true if eval("fact #{operator} value") # rubocop:disable Security/Eval + return true if eval("fact #{operator} value") # rubocop:disable Security/Eval, Style/EvalWithLocation end false @@ -118,9 +119,10 @@ def self.test_fact_value(fact, value, operator) def self.has_identity?(identity) identity = Regexp.new(identity.gsub("\/", "")) if identity.start_with?("/") - if identity.is_a?(Regexp) + case identity + when Regexp return Config.instance.identity.match(identity) - elsif Config.instance.identity == identity + when Config.instance.identity return true end @@ -135,9 +137,9 @@ def self.empty_filter?(filter) # Creates an empty filter def self.empty_filter { - "fact" => [], + "fact" => [], "cf_class" => [], - "agent" => [], + "agent" => [], "identity" => [], "compound" => [] } @@ -161,7 +163,7 @@ def self.mcollective_config_paths_for_user # File.expand_path will raise if HOME isn't set, catch it user_path = File.expand_path("~/.mcollective") config_paths << user_path - rescue Exception # rubocop:disable Lint/RescueException, Lint/HandleExceptions + rescue Exception # rubocop:disable Lint/RescueException, Lint/SuppressedException end if windows? @@ -182,7 +184,7 @@ def self.choria_config_paths_for_user # File.expand_path will raise if HOME isn't set, catch it user_path = File.expand_path("~/.choriarc") config_paths << user_path - rescue Exception # rubocop:disable Lint/RescueException, Lint/HandleExceptions + rescue Exception # rubocop:disable Lint/RescueException, Lint/SuppressedException end if windows? @@ -217,14 +219,14 @@ def self.config_file_for_user # Creates a standard options hash def self.default_options { - :verbose => false, - :disctimeout => nil, - :timeout => 5, - :config => config_file_for_user, - :collective => nil, - :discovery_method => nil, + :verbose => false, + :disctimeout => nil, + :timeout => 5, + :config => config_file_for_user, + :collective => nil, + :discovery_method => nil, :discovery_options => Config.instance.default_discovery_options, - :filter => empty_filter + :filter => empty_filter } end @@ -273,15 +275,16 @@ def self.loadclass(klass) # Parse a fact filter string like foo=bar into the tuple hash thats needed def self.parse_fact_string(fact) - if fact =~ /^([^ ]+?)[ ]*=>[ ]*(.+)/ + case fact + when /^([^ ]+?) *=> *(.+)/ {:fact => $1, :value => $2, :operator => ">="} - elsif fact =~ /^([^ ]+?)[ ]*=<[ ]*(.+)/ + when /^([^ ]+?) *=< *(.+)/ {:fact => $1, :value => $2, :operator => "<="} - elsif fact =~ /^([^ ]+?)[ ]*(<=|>=|<|>|!=|==|=~)[ ]*(.+)/ + when /^([^ ]+?) *(<=|>=|<|>|!=|==|=~) *(.+)/ {:fact => $1, :value => $3, :operator => $2} - elsif fact =~ /^(.+?)[ ]*=[ ]*\/(.+)\/$/ + when /^(.+?) *= *\/(.+)\/$/ {:fact => $1, :value => "/#{$2}/", :operator => "=~"} - elsif fact =~ /^([^= ]+?)[ ]*=[ ]*(.+)/ + when /^([^= ]+?) *= *(.+)/ {:fact => $1, :value => $2, :operator => "=="} else raise "Could not parse fact #{fact} it does not appear to be in a valid format" @@ -326,9 +329,9 @@ def self.color(code) } if colorize - return colors[code] || "" + colors[code] || "" else - return "" + "" end end @@ -379,9 +382,7 @@ def self.align_text(text, console_cols=nil, preamble=5) text.each_with_index do |line, i| whitespace = 0 - while whitespace < line.length && line[whitespace].chr == " " - whitespace += 1 - end + whitespace += 1 while whitespace < line.length && line[whitespace].chr == " " # If the current line is empty, indent it so that a snippet # from the previous line is aligned correctly. @@ -435,21 +436,21 @@ def self.align_text(text, console_cols=nil, preamble=5) # # Returns [0, 0] if it can't figure it out or if you're # not running on a tty - def self.terminal_dimensions(stdout=STDOUT, environment=ENV) + def self.terminal_dimensions(stdout=$stdout, environment=ENV) return [0, 0] unless stdout.tty? return [80, 40] if Util.windows? if environment["COLUMNS"] && environment["LINES"] - return [environment["COLUMNS"].to_i, environment["LINES"].to_i] + [environment["COLUMNS"].to_i, environment["LINES"].to_i] elsif environment["TERM"] && command_in_path?("tput") - return [`tput cols`.to_i, `tput lines`.to_i] + [`tput cols`.to_i, `tput lines`.to_i] elsif command_in_path?("stty") - return `stty size`.scan(/\d+/).map(&:to_i) + `stty size`.scan(/\d+/).map(&:to_i) else - return [0, 0] + [0, 0] end rescue [0, 0] @@ -490,6 +491,7 @@ def self.versioncmp(version_a, version_b) elsif b == "." then return 1 elsif a =~ /^\d+$/ && b =~ /^\d+$/ return a.to_s.upcase <=> b.to_s.upcase if a =~ /^0/ || b =~ /^0/ + return a.to_i <=> b.to_i else return a.upcase <=> b.upcase @@ -516,10 +518,11 @@ def self.absolute_path?(path, separator=File::SEPARATOR, alt_separator=File::ALT # Any other value will return FalseClass def self.str_to_bool(val) clean_val = val.to_s.strip - if clean_val =~ /^(1|yes|true|y|t)$/i - return true - elsif clean_val =~ /^(0|no|false|n|f)$/i - return false + case clean_val + when /^(1|yes|true|y|t)$/i + true + when /^(0|no|false|n|f)$/i + false else raise("Cannot convert string value '#{clean_val}' into a boolean.") end @@ -531,8 +534,7 @@ def self.templatepath(template_file) template_path = File.join(config_dir, template_file) return template_path if File.exist?(template_path) - template_path = File.join("/etc/mcollective", template_file) - template_path + File.join("/etc/mcollective", template_file) end # subscribe to the direct addressing queue @@ -563,6 +565,7 @@ def self.get_hidden_input_on_windows # rubocop:disable Naming/AccessorMethodName while char = Win32API.new("crtdll", "_getch", [], "I").Call break if [10, 13].include?(char) # return or newline + if [127, 8].include?(char) # backspace and delete input.slice!(-1, 1) unless input.empty? else @@ -574,17 +577,13 @@ def self.get_hidden_input_on_windows # rubocop:disable Naming/AccessorMethodName end def self.get_hidden_input_on_unix # rubocop:disable Naming/AccessorMethodName - unless $stdin.tty? - raise "Could not hook to stdin to hide input. If using SSH, try using -t flag while connecting to server." - end - unless system "stty -echo -icanon" - raise "Could not hide input using stty command." - end + raise "Could not hook to stdin to hide input. If using SSH, try using -t flag while connecting to server." unless $stdin.tty? + raise "Could not hide input using stty command." unless system "stty -echo -icanon" + input = $stdin.gets ensure - unless system "stty echo icanon" - raise "Could not enable echoing of input. Try executing `stty echo icanon` to debug." - end + raise "Could not enable echoing of input. Try executing `stty echo icanon` to debug." unless system "stty echo icanon" + input end diff --git a/lib/mcollective/validator.rb b/lib/mcollective/validator.rb index d8d0751e..2203cee3 100644 --- a/lib/mcollective/validator.rb +++ b/lib/mcollective/validator.rb @@ -30,7 +30,7 @@ def self.[](klass) end # Allows validation plugins to be called like module methods : Validator.validate() - def self.method_missing(method, *args, &block) # rubocop:disable Style/MethodMissing + def self.method_missing(method, *args, &block) if has_validator?(method) Validator[method].validate(*args) else @@ -38,6 +38,10 @@ def self.method_missing(method, *args, &block) # rubocop:disable Style/MethodMis end end + def self.respond_to_missing?(method, *) + has_validator?(method) + end + def self.has_validator?(validator) const_defined?(validator_class(validator)) end @@ -48,6 +52,7 @@ def self.validator_class(validator) def self.load_validators? return true if @last_load.nil? + (@last_load - Time.now.to_i) > 300 end diff --git a/lib/mcollective/validator/length_validator.rb b/lib/mcollective/validator/length_validator.rb index 78ecb939..7a9bcbef 100644 --- a/lib/mcollective/validator/length_validator.rb +++ b/lib/mcollective/validator/length_validator.rb @@ -2,9 +2,7 @@ module MCollective module Validator class LengthValidator def self.validate(validator, length) - if (validator.size > length) && (length > 0) - raise ValidatorError, "Input string is longer than #{length} character(s)" - end + raise ValidatorError, "Input string is longer than #{length} character(s)" if (validator.size > length) && (length > 0) end end end diff --git a/spec/unit/mcollective/application_spec.rb b/spec/unit/mcollective/application_spec.rb index d2e64eb1..b34d07e9 100755 --- a/spec/unit/mcollective/application_spec.rb +++ b/spec/unit/mcollective/application_spec.rb @@ -108,7 +108,7 @@ module MCollective end it "should print an error to STDERR on error" do - IO.any_instance.expects(:puts).with("Validation of key failed: failed").at_least_once + Application.any_instance.expects(:warn).with("Validation of key failed: failed").at_least_once Application.any_instance.stubs("exit").returns(true) a = Application.new @@ -116,8 +116,8 @@ module MCollective end it "should exit on valdation error" do - IO.any_instance.expects(:puts).at_least_once - Application.any_instance.stubs("exit").returns(true) + Application.any_instance.expects(:warn).at_least_once + Application.any_instance.stubs(:exit).returns(true) a = Application.new a.validate_option(Proc.new {|v| "failed"}, "key", 1) @@ -218,9 +218,9 @@ module MCollective end it "should support validation" do - IO.any_instance.expects(:puts).with("Validation of foo failed: failed").at_least_once - Application.any_instance.stubs("exit").returns(true) - Application.any_instance.stubs("main").returns(true) + Application.any_instance.expects(:warn).with("Validation of foo failed: failed").at_least_once + Application.any_instance.stubs(:exit).returns(true) + Application.any_instance.stubs(:main).returns(true) Application.option :foo, :description => "meh", @@ -254,11 +254,11 @@ module MCollective end it "should enforce required options" do - Application.any_instance.stubs("exit").returns(true) - Application.any_instance.stubs("main").returns(true) - OptionParser.any_instance.stubs("parse!").returns(true) - IO.any_instance.expects(:puts).with(anything).at_least_once - IO.any_instance.expects(:puts).with("The foo option is mandatory").at_least_once + Application.any_instance.stubs(:exit).returns(true) + Application.any_instance.stubs(:main).returns(true) + OptionParser.any_instance.stubs(:parse!).returns(true) + Application.any_instance.expects(:warn).with(anything).at_least_once + Application.any_instance.expects(:warn).with("The foo option is mandatory").at_least_once ARGV.clear ARGV << "--foo=bar" @@ -370,7 +370,7 @@ module MCollective describe "#main" do it "should detect applications without a #main" do - IO.any_instance.expects(:puts).with("Applications need to supply a 'main' method") + Application.any_instance.expects(:warn).with("Applications need to supply a 'main' method") expect { Application.new.run diff --git a/spec/unit/mcollective/applications_spec.rb b/spec/unit/mcollective/applications_spec.rb index a5657275..170636fd 100755 --- a/spec/unit/mcollective/applications_spec.rb +++ b/spec/unit/mcollective/applications_spec.rb @@ -124,7 +124,7 @@ module MCollective it "should print a friendly error and exit on failure" do Applications.expects("load_config").raises(Exception) - IO.any_instance.expects(:puts).with(regexp_matches(/Failed to generate application list/)).once + Applications.expects(:warn).with(regexp_matches(/Failed to generate application list/)).once expect { Applications.list.should diff --git a/spec/unit/mcollective/data/fstat_data_spec.rb b/spec/unit/mcollective/data/fstat_data_spec.rb index a1a93aac..8c383104 100644 --- a/spec/unit/mcollective/data/fstat_data_spec.rb +++ b/spec/unit/mcollective/data/fstat_data_spec.rb @@ -34,13 +34,13 @@ module Data end it "should detect missing files" do - File.expects(:exists?).with("/nonexisting").returns(false) + File.expects(:exist?).with("/nonexisting").returns(false) @plugin.query_data("/nonexisting") @plugin.result.output.should == "not present" end it "should provide correct file stats" do - File.expects(:exists?).with("rspec").returns(true) + File.expects(:exist?).with("rspec").returns(true) File.expects(:symlink?).with("rspec").returns(false) File.expects(:stat).with("rspec").returns(@stat) File.expects(:read).with("rspec").returns("rspec") @@ -67,7 +67,7 @@ module Data end it "should provide correct link stats" do - File.expects(:exists?).with("rspec").returns(true) + File.expects(:exist?).with("rspec").returns(true) File.expects(:symlink?).with("rspec").returns(true) File.expects(:lstat).with("rspec").returns(@stat) @@ -80,7 +80,7 @@ module Data end it "should provide correct directory stats" do - File.expects(:exists?).with("rspec").returns(true) + File.expects(:exist?).with("rspec").returns(true) File.expects(:symlink?).with("rspec").returns(false) File.expects(:stat).with("rspec").returns(@stat) @@ -93,7 +93,7 @@ module Data end it "should provide correct socket stats" do - File.expects(:exists?).with("rspec").returns(true) + File.expects(:exist?).with("rspec").returns(true) File.expects(:symlink?).with("rspec").returns(false) File.expects(:stat).with("rspec").returns(@stat) @@ -106,7 +106,7 @@ module Data end it "should provide correct chardev stats" do - File.expects(:exists?).with("rspec").returns(true) + File.expects(:exist?).with("rspec").returns(true) File.expects(:symlink?).with("rspec").returns(false) File.expects(:stat).with("rspec").returns(@stat) @@ -119,7 +119,7 @@ module Data end it "should provide correct blockdev stats" do - File.expects(:exists?).with("rspec").returns(true) + File.expects(:exist?).with("rspec").returns(true) File.expects(:symlink?).with("rspec").returns(false) File.expects(:stat).with("rspec").returns(@stat) diff --git a/spec/unit/mcollective/discovery_spec.rb b/spec/unit/mcollective/discovery_spec.rb index 4e3a4097..61038523 100644 --- a/spec/unit/mcollective/discovery_spec.rb +++ b/spec/unit/mcollective/discovery_spec.rb @@ -167,7 +167,7 @@ module MCollective end end - describe "#descovery_method" do + describe "#discovery_method" do it "should default to 'mc'" do @client.expects(:options).returns({}) @discovery.discovery_method.should == "mc" diff --git a/spec/unit/mcollective/logger/syslog_logger_spec.rb b/spec/unit/mcollective/logger/syslog_logger_spec.rb index fb84f495..585d8bde 100644 --- a/spec/unit/mcollective/logger/syslog_logger_spec.rb +++ b/spec/unit/mcollective/logger/syslog_logger_spec.rb @@ -46,7 +46,7 @@ module Logger it "should set LOG_USER for unknown facilities" do logger = Syslog_logger.new - IO.any_instance.expects(:puts).with("Invalid syslog facility rspec supplied, reverting to USER") + logger.expects(:warn).with("Invalid syslog facility rspec supplied, reverting to USER") logger.syslog_facility("rspec").should == Syslog::LOG_USER end end @@ -75,7 +75,7 @@ module Logger it "should resort to STDERR output if all else fails" do logger = Syslog_logger.new - IO.any_instance.expects(:puts).with("error: rspec").once + logger.expects(:warn).with("error: rspec").once logger.log(:error, "rspec", "rspec") end diff --git a/spec/unit/mcollective/rpc/actionrunner_spec.rb b/spec/unit/mcollective/rpc/actionrunner_spec.rb index 89891cb2..e93fbf20 100755 --- a/spec/unit/mcollective/rpc/actionrunner_spec.rb +++ b/spec/unit/mcollective/rpc/actionrunner_spec.rb @@ -193,10 +193,10 @@ module RPC action_in_last_dir = File.join(File::SEPARATOR, "libdir2", "agent", "spectester", "action.sh") action_in_last_dir_new = File.join(File::SEPARATOR, "libdir2", "mcollective", "agent", "spectester", "action.sh") - File.expects("exists?").with(action_in_first_dir).returns(true) - File.expects("exists?").with(action_in_first_dir_new).returns(false) - File.expects("exists?").with(action_in_last_dir).never - File.expects("exists?").with(action_in_last_dir_new).never + File.expects(:exist?).with(action_in_first_dir).returns(true) + File.expects(:exist?).with(action_in_first_dir_new).returns(false) + File.expects(:exist?).with(action_in_last_dir).never + File.expects(:exist?).with(action_in_last_dir_new).never ActionRunner.new("action.sh", @req, :json).command.should == action_in_first_dir end @@ -208,10 +208,10 @@ module RPC action_in_last_dir = File.join(File::SEPARATOR, "libdir2", "agent", "spectester", "action.sh") action_in_last_dir_new = File.join(File::SEPARATOR, "libdir2", "mcollective", "agent", "spectester", "action.sh") - File.expects("exists?").with(action_in_first_dir).returns(false) - File.expects("exists?").with(action_in_first_dir_new).returns(false) - File.expects("exists?").with(action_in_last_dir).returns(true) - File.expects("exists?").with(action_in_last_dir_new).returns(false) + File.expects(:exist?).with(action_in_first_dir).returns(false) + File.expects(:exist?).with(action_in_first_dir_new).returns(false) + File.expects(:exist?).with(action_in_last_dir).returns(true) + File.expects(:exist?).with(action_in_last_dir_new).returns(false) ActionRunner.new("action.sh", @req, :json).command.should == action_in_last_dir end @@ -221,8 +221,8 @@ module RPC action_in_new_dir = File.join(File::SEPARATOR, "libdir1", "mcollective", "agent", "spectester", "action.sh") action_in_old_dir = File.join(File::SEPARATOR, "libdir1", "agent", "spectester", "action.sh") - File.expects("exists?").with(action_in_new_dir).returns(true) - File.expects("exists?").with(action_in_old_dir).returns(false) + File.expects(:exist?).with(action_in_new_dir).returns(true) + File.expects(:exist?).with(action_in_old_dir).returns(false) ActionRunner.new("action.sh", @req, :json).command.should == action_in_new_dir end @@ -232,8 +232,8 @@ module RPC action_in_new_dir = File.join(File::SEPARATOR, "libdir1", "mcollective", "agent", "spectester", "action.sh") action_in_old_dir = File.join(File::SEPARATOR, "libdir1", "agent", "spectester", "action.sh") - File.expects("exists?").with(action_in_new_dir).returns(true) - File.expects("exists?").with(action_in_old_dir).returns(true) + File.expects(:exist?).with(action_in_new_dir).returns(true) + File.expects(:exist?).with(action_in_old_dir).returns(true) ActionRunner.new("action.sh", @req, :json).command.should == action_in_new_dir end diff --git a/spec/unit/mcollective/rpc/client_spec.rb b/spec/unit/mcollective/rpc/client_spec.rb index d4021454..4f5ae5a8 100644 --- a/spec/unit/mcollective/rpc/client_spec.rb +++ b/spec/unit/mcollective/rpc/client_spec.rb @@ -809,6 +809,7 @@ module RPC discovered = mock discovered.stubs(:size).returns(1) + discovered.stubs(:empty?).returns(false) discovered.expects(:in_groups_of).with(10).raises("spec pass") client.instance_variable_set("@client", @coreclient)