From 428708ef376884c3dc35aee1951cb8e1c55fac9f Mon Sep 17 00:00:00 2001 From: ytti Date: Fri, 22 Jun 2018 19:25:10 +0300 Subject: Refactor smells * reduce no/resolve_repo smell * reduce input/ssh/connect smell * reduce source/http load smell * reduce node/worker smell * reduce source/csv smell * reduce output/http smell Get's code climate from B to A, so I'm sure it's super duper important. --- lib/oxidized/input/ssh.rb | 57 ++++++++++++++++++---------------- lib/oxidized/node.rb | 41 ++++++++---------------- lib/oxidized/output/http.rb | 31 +++++++++++------- lib/oxidized/source/csv.rb | 21 ++++++++----- lib/oxidized/source/http.rb | 46 ++++++++++++++------------- lib/oxidized/worker.rb | 76 +++++++++++++++++++++++++-------------------- 6 files changed, 141 insertions(+), 131 deletions(-) diff --git a/lib/oxidized/input/ssh.rb b/lib/oxidized/input/ssh.rb index d321a11..82335f9 100644 --- a/lib/oxidized/input/ssh.rb +++ b/lib/oxidized/input/ssh.rb @@ -21,36 +21,10 @@ module Oxidized @output = '' @pty_options = { term: "vt100" } @node.model.cfg['ssh'].each { |cb| instance_exec(&cb) } - secure = Oxidized.config.input.ssh.secure @log = File.open(Oxidized::Config::Log + "/#{@node.ip}-ssh", 'w') if Oxidized.config.input.debug? - port = vars(:ssh_port) || 22 - - ssh_opts = { - port: port.to_i, - paranoid: secure, - keepalive: true, - password: @node.auth[:password], :timeout => Oxidized.config.timeout, - number_of_password_prompts: 0, - } - - auth_methods = vars(:auth_methods) || %w(none publickey password) - ssh_opts[:auth_methods] = auth_methods - Oxidized.logger.debug "AUTH METHODS::#{auth_methods}" - - if proxy_host = vars(:ssh_proxy) - proxy_command = "ssh " - proxy_command += "-o StrictHostKeyChecking=no " unless secure - proxy_command += "#{proxy_host} -W %h:%p" - proxy = Net::SSH::Proxy::Command.new(proxy_command) - ssh_opts[:proxy] = proxy - end - - ssh_opts[:keys] = vars(:ssh_keys).is_a?(Array) ? vars(:ssh_keys) : [vars(:ssh_keys)] if vars(:ssh_keys) - ssh_opts[:kex] = vars(:ssh_kex).split(/,\s*/) if vars(:ssh_kex) - ssh_opts[:encryption] = vars(:ssh_encryption).split(/,\s*/) if vars(:ssh_encryption) Oxidized.logger.debug "lib/oxidized/input/ssh.rb: Connecting to #{@node.name}" - @ssh = Net::SSH.start(@node.ip, @node.auth[:username], ssh_opts) + @ssh = Net::SSH.start(@node.ip, @node.auth[:username], get_ssh_opts) unless @exec shell_open @ssh begin @@ -142,5 +116,34 @@ module Oxidized end end end + + def get_ssh_opts + ssh_opts = { + port: (vars(:ssh_port) || 22).to_i, + paranoid: secure, + keepalive: true, + password: @node.auth[:password], timeout: Oxidized.config.timeout, + number_of_password_prompts: 0 + } + + auth_methods = vars(:auth_methods) || %w(none publickey password) + ssh_opts[:auth_methods] = auth_methods + Oxidized.logger.debug "AUTH METHODS::#{auth_methods}" + + if proxy_host = vars(:ssh_proxy) + proxy_command = "ssh " + proxy_command += "-o StrictHostKeyChecking=no " unless Oxidized.config.input.ssh.secure? + proxy_command += "#{proxy_host} -W %h:%p" + proxy = Net::SSH::Proxy::Command.new(proxy_command) + ssh_opts[:proxy] = proxy + end + + ssh_opts[:keys] = [vars(:ssh_keys)].flatten if vars(:ssh_keys) + ssh_opts[:kex] = vars(:ssh_kex).split(/,\s*/) if vars(:ssh_kex) + ssh_opts[:encryption] = vars(:ssh_encryption).split(/,\s*/) if vars(:ssh_encryption) + ssh_opts[:verbose] = Logger::DEBUG if Oxidized.config.input.debug? + + ssh_opts + end end end diff --git a/lib/oxidized/node.rb b/lib/oxidized/node.rb index 300221e..bc1a586 100644 --- a/lib/oxidized/node.rb +++ b/lib/oxidized/node.rb @@ -173,32 +173,18 @@ module Oxidized end def resolve_repo opt - if is_git? opt - remote_repo = Oxidized.config.output.git.repo - - if remote_repo.is_a?(::String) - if Oxidized.config.output.git.single_repo? || @group.nil? - remote_repo - else - File.join(File.dirname(remote_repo), @group + '.git') - end - else - remote_repo[@group] - end - elsif is_gitcrypt? opt - remote_repo = Oxidized.config.output.gitcrypt.repo - - if remote_repo.is_a?(::String) - if Oxidized.config.output.gitcrypt.single_repo? || @group.nil? - remote_repo - else - File.join(File.dirname(remote_repo), @group + '.git') - end + type = git_type opt + return nil unless type + + remote_repo = Oxidized.config.output.send(type).repo + if remote_repo.is_a?(::String) + if Oxidized.config.output.send(type).single_repo? || @group.nil? + remote_repo else - remote_repo[@group] + File.join(File.dirname(remote_repo), @group + '.git') end else - return + remote_repo[@group] end end @@ -237,12 +223,11 @@ module Oxidized value end - def is_git? opt - (opt[:output] || Oxidized.config.output.default) == 'git' + def git_type + type = opt[:output] || Oxidized.config.output.default + return nil unless type[0..2] == "git" + type end - def is_gitcrypt? opt - (opt[:output] || Oxidized.config.output.default) == 'gitcrypt' - end end end diff --git a/lib/oxidized/output/http.rb b/lib/oxidized/output/http.rb index 0467261..756b9ad 100644 --- a/lib/oxidized/output/http.rb +++ b/lib/oxidized/output/http.rb @@ -14,28 +14,19 @@ module Oxidized raise NoConfig, 'no output http config, edit ~/.config/oxidized/config' end end + require "net/http" require "uri" require "json" + def store node, outputs, opt = {} @commitref = nil - json = JSON.pretty_generate( - { - 'msg' => opt[:msg], - 'user' => opt[:user], - 'email' => opt[:email], - 'group' => opt[:group], - 'node' => node, - 'config' => outputs.to_cfg, - # actually we need to also iterate outputs, for other types like in gitlab. But most people don't use 'type' functionality. - } - ) uri = URI.parse @cfg.url http = Net::HTTP.new uri.host, uri.port # http.use_ssl = true if uri.scheme = 'https' req = Net::HTTP::Post.new(uri.request_uri, initheader = { 'Content-Type' => 'application/json' }) req.basic_auth @cfg.user, @cfg.password - req.body = json + req.body = generate_json(node, outputs, opt) response = http.request req case response.code.to_i @@ -50,5 +41,21 @@ module Oxidized Oxidized.logger.info "Configuration http backup for #{node} failed status: #{response.body}" end end + + private + + def generate_json node, outputs, opt + JSON.pretty_generate( + { + 'msg' => opt[:msg], + 'user' => opt[:user], + 'email' => opt[:email], + 'group' => opt[:group], + 'node' => node, + 'config' => outputs.to_cfg, + # actually we need to also iterate outputs, for other types like in gitlab. But most people don't use 'type' functionality. + } + ) + end end end diff --git a/lib/oxidized/source/csv.rb b/lib/oxidized/source/csv.rb index 7b771f6..769b1f7 100644 --- a/lib/oxidized/source/csv.rb +++ b/lib/oxidized/source/csv.rb @@ -20,14 +20,7 @@ module Oxidized def load _node_want = nil nodes = [] - file = File.expand_path(@cfg.file) - file = if @cfg.gpg? - crypto = GPGME::Crypto.new password: @cfg.gpg_password - file = crypto.decrypt(File.open(file)).to_s - else - open(file) - end - file.each_line do |line| + get_file.each_line do |line| next if line.match(/^\s*#/) data = line.chomp.split(@cfg.delimiter, -1) next if data.empty? @@ -49,5 +42,17 @@ module Oxidized end nodes end + + private + + def get_file + file = File.expand_path(@cfg.file) + if @cfg.gpg? + crypto = GPGME::Crypto.new password: @cfg.gpg_password + crypto.decrypt(File.open(file)).to_s + else + open(file) + end + end end end diff --git a/lib/oxidized/source/http.rb b/lib/oxidized/source/http.rb index bf1e74c..36fa764 100644 --- a/lib/oxidized/source/http.rb +++ b/lib/oxidized/source/http.rb @@ -17,28 +17,7 @@ module Oxidized def load node_want = nil nodes = [] - uri = URI.parse(@cfg.url) - http = Net::HTTP.new(uri.host, uri.port) - http.use_ssl = true if uri.scheme == 'https' - http.verify_mode = OpenSSL::SSL::VERIFY_NONE unless @cfg.secure - - # map headers - headers = {} - @cfg.headers.each do |header, value| - headers[header] = value - end - - req_uri = uri.request_uri - if node_want - req_uri = "#{req_uri}/#{node_want}" - end - request = Net::HTTP::Get.new(req_uri, headers) - if (@cfg.user? && @cfg.pass?) - request.basic_auth(@cfg.user, @cfg.pass) - end - - response = http.request(request) - data = JSON.parse(response.body) + data = JSON.parse(get_http) data = string_navigate(data, @cfg.hosts_location) if @cfg.hosts_location? data.each do |node| next if node.empty? @@ -72,5 +51,28 @@ module Oxidized end object end + + def get_http + uri = URI.parse(@cfg.url) + http = Net::HTTP.new(uri.host, uri.port) + http.use_ssl = true if uri.scheme == 'https' + http.verify_mode = OpenSSL::SSL::VERIFY_NONE unless @cfg.secure + + # map headers + headers = {} + @cfg.headers.each do |header, value| + headers[header] = value + end + + req_uri = uri.request_uri + if node_want + req_uri = "#{req_uri}/#{node_want}" + end + request = Net::HTTP::Get.new(req_uri, headers) + if (@cfg.user? && @cfg.pass?) + request.basic_auth(@cfg.user, @cfg.pass) + end + http.request(request).body + end end end diff --git a/lib/oxidized/worker.rb b/lib/oxidized/worker.rb index 74b046d..06e8689 100644 --- a/lib/oxidized/worker.rb +++ b/lib/oxidized/worker.rb @@ -36,48 +36,17 @@ module Oxidized Oxidized.logger.debug("lib/oxidized/worker.rb: #{@jobs.size} jobs running in parallel") unless @jobs.empty? end + def process job node = job.node node.last = job node.stats.add job @jobs.duration job.time node.running = false - if job.status == :success - @jobs_done += 1 # needed for :nodes_done hook - Oxidized.Hooks.handle :node_success, :node => node, - :job => job - msg = "update #{node.name}" - msg += " from #{node.from}" if node.from - msg += " with message '#{node.msg}'" if node.msg - output = node.output.new - if output.store node.name, job.config, - :msg => msg, :email => node.email, :user => node.user, :group => node.group - node.modified - Oxidized.logger.info "Configuration updated for #{node.group}/#{node.name}" - Oxidized.Hooks.handle :post_store, :node => node, - :job => job, - :commitref => output.commitref - end - node.reset + process_success node, job else - msg = "#{node.name} status #{job.status}" - if node.retry < Oxidized.config.retries - node.retry += 1 - msg += ", retry attempt #{node.retry}" - @nodes.next node.name - else - # Only increment the @jobs_done when we give up retries for a node (or success). - # As it would otherwise cause @jobs_done to be incremented with generic retries. - # This would cause :nodes_done hook to desync from running at the end of the nodelist and - # be fired when the @jobs_done > @nodes.count (could be mid-cycle on the next cycle). - @jobs_done += 1 - msg += ", retries exhausted, giving up" - node.retry = 0 - Oxidized.Hooks.handle :node_fail, :node => node, - :job => job - end - Oxidized.logger.warn msg + process_failure node, job end rescue NodeNotFound Oxidized.logger.warn "#{node.name} not found, removed while collecting?" @@ -85,6 +54,45 @@ module Oxidized private + def process_success node, job + @jobs_done += 1 # needed for :nodes_done hook + Oxidized.Hooks.handle :node_success, node: node, + job: job + msg = "update #{node.name}" + msg += " from #{node.from}" if node.from + msg += " with message '#{node.msg}'" if node.msg + output = node.output.new + if output.store node.name, job.config, + msg: msg, email: node.email, user: node.user, group: node.group + node.modified + Oxidized.logger.info "Configuration updated for #{node.group}/#{node.name}" + Oxidized.Hooks.handle :post_store, node: node, + job: job, + commitref: output.commitref + end + node.reset + end + + def process_failure node, job + msg = "#{node.name} status #{job.status}" + if node.retry < Oxidized.config.retries + node.retry += 1 + msg += ", retry attempt #{node.retry}" + @nodes.next node.name + else + # Only increment the @jobs_done when we give up retries for a node (or success). + # As it would otherwise cause @jobs_done to be incremented with generic retries. + # This would cause :nodes_done hook to desync from running at the end of the nodelist and + # be fired when the @jobs_done > @nodes.count (could be mid-cycle on the next cycle). + @jobs_done += 1 + msg += ", retries exhausted, giving up" + node.retry = 0 + Oxidized.Hooks.handle :node_fail, :node => node, + :job => job + end + Oxidized.logger.warn msg + end + def is_cycle_finished? if @jobs_done > @nodes.count true -- cgit v1.2.1