diff --git a/.rubocop.yml b/.rubocop.yml new file mode 100644 index 0000000..5835705 --- /dev/null +++ b/.rubocop.yml @@ -0,0 +1,37 @@ +inherit_from: .rubocop_todo.yml + +require: + - rubocop-minitest + - rubocop-performance + - rubocop-rake + +AllCops: + NewCops: enable + TargetRubyVersion: 2.5 + Exclude: + - 'vendor/**/*' + +Gemspec/RequireMFA: + Enabled: false + +Layout/LineLength: + Exclude: + - 'test/**/*.rb' + +Metrics: + Enabled: false + +Style/ClassAndModuleChildren: + Enabled: false + +Style/Documentation: + Enabled: false + +Style/HashSyntax: + Enabled: false + +Style/IfUnlessModifier: + Enabled: false + +Style/StringLiterals: + Enabled: false diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml new file mode 100644 index 0000000..e69de29 diff --git a/Gemfile b/Gemfile index 6642277..26cffac 100644 --- a/Gemfile +++ b/Gemfile @@ -1,16 +1,26 @@ +# frozen_string_literal: true + source 'https://rubygems.org' gemspec group :development do - gem 'smart_proxy', git: 'https://github.com/theforeman/smart-proxy', - branch: 'develop' - #gem 'smart_proxy', path: '../smart-proxy' gem 'pry' gem 'pry-byebug' end +group :rubocop do + gem 'rubocop', '~> 1.28.0', require: false + gem 'rubocop-minitest', require: false + gem 'rubocop-performance', require: false + gem 'rubocop-rake', require: false +end + group :test do - gem 'minitest' - gem 'mocha' + gem 'minitest', require: false + gem 'minitest-reporters', '~> 1.4', require: false + gem 'mocha', '~> 1', require: false + gem 'rake', '~> 13.0', require: false + gem 'smart_proxy', github: 'theforeman/smart-proxy', branch: 'develop' + gem 'webmock', '~> 3', require: false end diff --git a/Rakefile b/Rakefile index b3a8da5..9f4be17 100644 --- a/Rakefile +++ b/Rakefile @@ -1,13 +1,19 @@ +# frozen_string_literal: true + require 'rake' require 'rake/testtask' +require 'rubocop/rake_task' begin require 'bundler/gem_tasks' rescue LoadError + # This is optional end +RuboCop::RakeTask.new + desc 'Default: run unit tests.' -task default: :test +task default: %i[rubocop test] desc 'Test Ansible plugin' Rake::TestTask.new(:test) do |t| @@ -17,3 +23,8 @@ Rake::TestTask.new(:test) do |t| t.test_files = FileList['test/**/*_test.rb'] t.verbose = true end + +namespace :jenkins do + desc nil # No description means it's not listed in rake -T + task unit: :test +end diff --git a/bundler.d/ansible.rb b/bundler.d/ansible.rb index 4c7e4f2..518c962 100644 --- a/bundler.d/ansible.rb +++ b/bundler.d/ansible.rb @@ -1 +1,3 @@ +# frozen_string_literal: true + gem 'smart_proxy_ansible' diff --git a/lib/smart_proxy_ansible.rb b/lib/smart_proxy_ansible.rb index 119a983..4a6fb95 100644 --- a/lib/smart_proxy_ansible.rb +++ b/lib/smart_proxy_ansible.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'smart_proxy_dynflow' module Proxy diff --git a/lib/smart_proxy_ansible/api.rb b/lib/smart_proxy_ansible/api.rb index 67776ee..84ef862 100644 --- a/lib/smart_proxy_ansible/api.rb +++ b/lib/smart_proxy_ansible/api.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Proxy module Ansible # API endpoints. Most of the code should be calling other classes, @@ -40,11 +42,13 @@ class Api < Sinatra::Base def extract_variables(role_name) variables = {} - role_name_parts = role_name.split('.') - if role_name_parts.count == 3 + parts = role_name.split('.') + if parts.count == 3 ReaderHelper.collections_paths.split(':').each do |path| - variables[role_name] = VariablesExtractor - .extract_variables("#{path}/ansible_collections/#{role_name_parts[0]}/#{role_name_parts[1]}/roles/#{role_name_parts[2]}") if variables[role_name].nil? || variables[role_name].empty? + if variables[role_name].nil? || variables[role_name].empty? + role_path = "#{path}/ansible_collections/#{parts[0]}/#{parts[1]}/roles/#{parts[2]}" + variables[role_name] = VariablesExtractor.extract_variables(role_path) + end end else RolesReader.roles_path.split(':').each do |path| diff --git a/lib/smart_proxy_ansible/configuration_loader.rb b/lib/smart_proxy_ansible/configuration_loader.rb index 354b955..5566d81 100644 --- a/lib/smart_proxy_ansible/configuration_loader.rb +++ b/lib/smart_proxy_ansible/configuration_loader.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Proxy::Ansible class ConfigurationLoader def load_classes diff --git a/lib/smart_proxy_ansible/exception.rb b/lib/smart_proxy_ansible/exception.rb index da9093c..1319a75 100644 --- a/lib/smart_proxy_ansible/exception.rb +++ b/lib/smart_proxy_ansible/exception.rb @@ -5,17 +5,18 @@ module Ansible # Taken from Foreman core, this class creates an error code for any # exception class Exception < ::StandardError - def initialize(message, *params) + def initialize(message, *params) # rubocop:todo Lint/MissingSuper @message = message @params = params end def self.calculate_error_code(classname, message) return 'ERF00-0000' if classname.nil? || message.nil? + basename = classname.split(':').last class_hash = Zlib.crc32(basename) % 100 msg_hash = Zlib.crc32(message) % 10_000 - format 'ERF%02d-%04d', class_hash, msg_hash + format 'ERF%02d-%04d', class_hash, msg_hash # rubocop:todo Style/FormatStringToken end def code diff --git a/lib/smart_proxy_ansible/http_config.ru b/lib/smart_proxy_ansible/http_config.ru index f4fef03..9164316 100644 --- a/lib/smart_proxy_ansible/http_config.ru +++ b/lib/smart_proxy_ansible/http_config.ru @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'smart_proxy_ansible/api' map "/ansible" do diff --git a/lib/smart_proxy_ansible/playbooks_reader.rb b/lib/smart_proxy_ansible/playbooks_reader.rb index f88db8b..45dafd7 100644 --- a/lib/smart_proxy_ansible/playbooks_reader.rb +++ b/lib/smart_proxy_ansible/playbooks_reader.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Proxy module Ansible # Implements the logic needed to read the playbooks and associated information @@ -22,10 +24,12 @@ def get_playbooks_names(collections_path) def read_collection_playbooks(collections_path, playbooks_to_import = nil) Dir.glob("#{collections_path}/ansible_collections/*/*/playbooks/*").map do |path| name = ReaderHelper.playbook_or_role_full_name(path) + next unless playbooks_to_import.nil? || playbooks_to_import.include?(name) + { name: name, playbooks_content: File.read(path) - } if playbooks_to_import.nil? || playbooks_to_import.include?(name) + } end.compact rescue Errno::ENOENT, Errno::EACCES => e message = "Could not read Ansible playbooks #{collections_path} - #{e.message}" diff --git a/lib/smart_proxy_ansible/plugin.rb b/lib/smart_proxy_ansible/plugin.rb index 24ef5a8..9693ac5 100644 --- a/lib/smart_proxy_ansible/plugin.rb +++ b/lib/smart_proxy_ansible/plugin.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Proxy module Ansible # Calls for the smart-proxy API to register the plugin @@ -6,7 +8,6 @@ class Plugin < Proxy::Plugin settings_file 'ansible.yml' plugin :ansible, Proxy::Ansible::VERSION default_settings :ansible_dir => Dir.home - # :working_dir => nil load_classes ::Proxy::Ansible::ConfigurationLoader load_validators :validate_settings => ::Proxy::Ansible::ValidateSettings diff --git a/lib/smart_proxy_ansible/reader_helper.rb b/lib/smart_proxy_ansible/reader_helper.rb index d8f4a7a..9f04628 100644 --- a/lib/smart_proxy_ansible/reader_helper.rb +++ b/lib/smart_proxy_ansible/reader_helper.rb @@ -1,10 +1,12 @@ +# frozen_string_literal: true + module Proxy module Ansible # Helper for Playbooks Reader class ReaderHelper class << self - DEFAULT_COLLECTIONS_PATHS = '/etc/ansible/collections:/usr/share/ansible/collections'.freeze - DEFAULT_CONFIG_FILE = '/etc/ansible/ansible.cfg'.freeze + DEFAULT_COLLECTIONS_PATHS = '/etc/ansible/collections:/usr/share/ansible/collections' + DEFAULT_CONFIG_FILE = '/etc/ansible/ansible.cfg' def collections_paths config_path(path_from_config('collections_paths'), DEFAULT_COLLECTIONS_PATHS) @@ -21,9 +23,7 @@ def config_path(config_line, default) end def path_from_config(config_key) - File.readlines(DEFAULT_CONFIG_FILE).select do |line| - line =~ /^\s*#{config_key}/ - end + File.readlines(DEFAULT_CONFIG_FILE).grep(/^\s*#{config_key}/) rescue Errno::ENOENT, Errno::EACCES => e RolesReader.logger.debug(e.backtrace) message = "Could not read Ansible config file #{DEFAULT_CONFIG_FILE} - #{e.message}" diff --git a/lib/smart_proxy_ansible/roles_reader.rb b/lib/smart_proxy_ansible/roles_reader.rb index b690caf..2749388 100644 --- a/lib/smart_proxy_ansible/roles_reader.rb +++ b/lib/smart_proxy_ansible/roles_reader.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require_relative 'exception' module Proxy @@ -5,11 +7,13 @@ module Ansible # Implements the logic needed to read the roles and associated information class RolesReader class << self - DEFAULT_ROLES_PATH = '/etc/ansible/roles:/usr/share/ansible/roles'.freeze + DEFAULT_ROLES_PATH = '/etc/ansible/roles:/usr/share/ansible/roles' def list_roles roles = roles_path.split(':').map { |path| read_roles(path) }.flatten - collection_roles = ReaderHelper.collections_paths.split(':').map { |path| read_collection_roles(path) }.flatten + collection_roles = ReaderHelper.collections_paths.split(':').map do |path| + read_collection_roles(path) + end.flatten roles + collection_roles end diff --git a/lib/smart_proxy_ansible/runner/ansible_runner.rb b/lib/smart_proxy_ansible/runner/ansible_runner.rb index 8ba51d0..7338268 100644 --- a/lib/smart_proxy_ansible/runner/ansible_runner.rb +++ b/lib/smart_proxy_ansible/runner/ansible_runner.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'shellwords' require 'yaml' @@ -81,6 +83,7 @@ def process_artifacts File.basename(f) end return unless @uuid + job_event_dir = File.join(@root, 'artifacts', @uuid, 'job_events') loop do files = Dir["#{job_event_dir}/*.json"].map do |file| @@ -89,6 +92,7 @@ def process_artifacts end files_with_nums = files.select { |(_, num)| num && num >= @counter }.sort_by(&:last) break if files_with_nums.empty? + logger.debug("[foreman_ansible] - processing event files: #{files_with_nums.map(&:first).inspect}}") files_with_nums.map(&:first).each { |event_file| handle_event_file(event_file) } @counter = files_with_nums.last.last + 1 @@ -124,7 +128,7 @@ def hostname_for_event(event) def handle_host_event(hostname, event) log_event("for host: #{hostname.inspect}", event) - publish_data_for(hostname, event['stdout'] + "\n", 'stdout') if event['stdout'] + publish_data_for(hostname, "#{event['stdout']}\n", 'stdout') if event['stdout'] case event['event'] when 'runner_on_ok' publish_exit_status_for(hostname, 0) if @exit_statuses[hostname].nil? @@ -151,7 +155,7 @@ def handle_broadcast_data(event) end end else - broadcast_data(event['stdout'] + "\n", 'stdout') + broadcast_data("#{event['stdout']}\n", 'stdout') end end @@ -197,6 +201,7 @@ def start_ansible_runner def cmdline cmd_args = [tags_cmd, check_cmd].reject(&:empty?) return nil unless cmd_args.any? + cmd_args.join(' ') end @@ -210,7 +215,7 @@ def check_cmd end def verbosity - '-' + 'v' * @verbosity_level.to_i + "-#{'v' * @verbosity_level.to_i}" end def verbose? @@ -229,9 +234,7 @@ def prepare_directory_structure end def log_event(description, event) - # TODO: replace this ugly code with block variant once https://github.com/Dynflow/dynflow/pull/323 - # arrives in production - logger.debug("[foreman_ansible] - handling event #{description}: #{JSON.pretty_generate(event)}") if logger.level <= ::Logger::DEBUG + logger.debug { "[foreman_ansible] - handling event #{description}: #{JSON.pretty_generate(event)}" } end # Each per-host task has inventory only for itself, we must @@ -255,6 +258,7 @@ def rebuild_inventory(input) def working_dir return @root if @root + dir = Proxy::Ansible::Plugin.settings[:working_dir] @tmp_working_dir = true if dir.nil? diff --git a/lib/smart_proxy_ansible/task_launcher/ansible_runner.rb b/lib/smart_proxy_ansible/task_launcher/ansible_runner.rb index be0ff9a..81dc801 100644 --- a/lib/smart_proxy_ansible/task_launcher/ansible_runner.rb +++ b/lib/smart_proxy_ansible/task_launcher/ansible_runner.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'fileutils' require 'smart_proxy_dynflow/task_launcher/abstract' require 'smart_proxy_dynflow/task_launcher/batch' @@ -25,25 +27,9 @@ def self.runner_class # apart when debugging def transform_input(input) action_input = super['action_input'] - { 'action_input' => { 'name' => action_input['name'], :task_id => action_input[:task_id], :runner_id => action_input[:runner_id] } } + { 'action_input' => { 'name' => action_input['name'], :task_id => action_input[:task_id], + :runner_id => action_input[:runner_id] } } end - - # def self.input_format - # { - # $UUID => { - # :execution_plan_id => $EXECUTION_PLAN_UUID, - # :run_step_id => Integer, - # :input => { - # :action_class => Class, - # :action_input => { - # "ansible_inventory"=> String, - # "hostname"=>"127.0.0.1", - # "script"=>"---\n- hosts: all\n tasks:\n - shell: |\n true\n register: out\n - debug: var=out" - # } - # } - # } - # } - # end end end end diff --git a/lib/smart_proxy_ansible/validate_settings.rb b/lib/smart_proxy_ansible/validate_settings.rb index 8d54c7a..2ebeb82 100644 --- a/lib/smart_proxy_ansible/validate_settings.rb +++ b/lib/smart_proxy_ansible/validate_settings.rb @@ -1,7 +1,11 @@ +# frozen_string_literal: true + module Proxy::Ansible class ValidateSettings < ::Proxy::PluginValidators::Base def validate!(settings) - raise NotExistingWorkingDirException.new("Working directory does not exist") unless settings[:working_dir].nil? || File.directory?(File.expand_path(settings[:working_dir])) + return if settings[:working_dir].nil? || File.directory?(File.expand_path(settings[:working_dir])) + + raise NotExistingWorkingDirException, "Working directory does not exist" end end end diff --git a/lib/smart_proxy_ansible/variables_extractor.rb b/lib/smart_proxy_ansible/variables_extractor.rb index 8ba375b..47313ba 100644 --- a/lib/smart_proxy_ansible/variables_extractor.rb +++ b/lib/smart_proxy_ansible/variables_extractor.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'yaml' module Proxy @@ -13,9 +15,10 @@ def extract_variables(role_path) begin loaded_yaml = YAML.load_file(role_file) rescue Psych::SyntaxError - raise ReadVariablesException.new "#{role_file} is not YAML file" + raise ReadVariablesException, "#{role_file} is not YAML file" end - raise ReadVariablesException.new "Could not parse YAML file: #{role_file}" unless loaded_yaml.is_a? Hash + raise ReadVariablesException, "Could not parse YAML file: #{role_file}" unless loaded_yaml.is_a? Hash + memo.merge loaded_yaml end end diff --git a/lib/smart_proxy_ansible/version.rb b/lib/smart_proxy_ansible/version.rb index abc66c1..2474e19 100644 --- a/lib/smart_proxy_ansible/version.rb +++ b/lib/smart_proxy_ansible/version.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Proxy # Version, this allows the proxy and other plugins know # what version of the Ansible plugin is running diff --git a/smart_proxy_ansible.gemspec b/smart_proxy_ansible.gemspec index 5022c1a..dde18db 100644 --- a/smart_proxy_ansible.gemspec +++ b/smart_proxy_ansible.gemspec @@ -1,5 +1,6 @@ -# -*- encoding: utf-8 -*- -lib = File.expand_path('../lib', __FILE__) +# frozen_string_literal: true + +lib = File.expand_path('lib', __dir__) $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib) require 'smart_proxy_ansible/version' @@ -10,9 +11,7 @@ Gem::Specification.new do |gem| gem.email = ['inecas@redhat.com', 'dlobatog@redhat.com'] gem.homepage = 'https://github.com/theforeman/smart_proxy_ansible' gem.summary = 'Smart-Proxy Ansible plugin' - gem.description = <<-EOS - Smart-Proxy ansible plugin - EOS + gem.description = 'Smart-Proxy ansible plugin' gem.files = Dir['bundler.d/ansible.rb', 'settings.d/**/*', @@ -26,12 +25,6 @@ Gem::Specification.new do |gem| gem.license = 'GPL-3.0' gem.required_ruby_version = '>= 2.5' - gem.add_development_dependency 'rake', '~> 13.0' - gem.add_development_dependency('mocha', '~> 1') - gem.add_development_dependency('webmock', '~> 3') - gem.add_development_dependency('rack-test', '~> 0') - gem.add_development_dependency('logger') - gem.add_development_dependency('smart_proxy') gem.add_runtime_dependency('net-ssh') gem.add_runtime_dependency('smart_proxy_dynflow', '~> 0.8') gem.add_runtime_dependency('smart_proxy_remote_execution_ssh', '~> 0.4') diff --git a/test/playbooks_reader_test.rb b/test/playbooks_reader_test.rb index 8190de1..e19c932 100644 --- a/test/playbooks_reader_test.rb +++ b/test/playbooks_reader_test.rb @@ -1,10 +1,11 @@ +# frozen_string_literal: true + require 'test_helper' require_relative '../lib/smart_proxy_ansible/playbooks_reader' require_relative '../lib/smart_proxy_ansible/exception' require_relative '../lib/smart_proxy_ansible/reader_helper' class PlaybooksReaderTest < Minitest::Test - describe 'playbooks method' do let(:fixtures) { JSON.parse(File.read(File.join(__dir__, 'fixtures/playbooks_reader_data.json'))) } let(:ansible_config) { fixtures['ansible_config'] } @@ -70,8 +71,8 @@ class PlaybooksReaderTest < Minitest::Test res = Proxy::Ansible::PlaybooksReader.playbooks_names assert_equal Array, res.class assert_equal 2, res.count - assert not(res.first.match(/.ya?ml/)) - assert not(res.last.match(/.ya?ml/)) + refute_match res.first, /.ya?ml/ + refute_match res.last, /.ya?ml/ end end end diff --git a/test/roles_reader_test.rb b/test/roles_reader_test.rb index cfb2b4a..c618222 100644 --- a/test/roles_reader_test.rb +++ b/test/roles_reader_test.rb @@ -12,7 +12,7 @@ class RolesReaderTest < Minitest::Test def self.expect_content_config(ansible_cfg_content) Proxy::Ansible::ReaderHelper.expects(:path_from_config) - .returns(ansible_cfg_content) + .returns(ansible_cfg_content) end describe '#roles_path' do diff --git a/test/test_helper.rb b/test/test_helper.rb index 020e218..15134e9 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -1,8 +1,13 @@ +# frozen_string_literal: true + require 'minitest/autorun' require 'minitest/spec' require 'mocha/minitest' require 'smart_proxy_for_testing' +require "minitest/reporters" +Minitest::Reporters.use! + module Minitest # Modifications to allow a 'test 'nameoftest' do' syntax class Test @@ -10,8 +15,9 @@ class << self def test(name, &block) test_name = "test_#{name.gsub(/\s+/, '_')}".to_sym defined = method_defined? test_name - fail "#{test_name} is already defined in #{self}" if defined - if block_given? + raise "#{test_name} is already defined in #{self}" if defined + + if block define_method(test_name, &block) else define_method(test_name) do diff --git a/test/variables_extractor_test.rb b/test/variables_extractor_test.rb index e07b6f5..45c9f55 100644 --- a/test/variables_extractor_test.rb +++ b/test/variables_extractor_test.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'test_helper' require_relative '../lib/smart_proxy_ansible/variables_extractor' require_relative '../lib/smart_proxy_ansible/exception'