From f00ff120a546f492e83f12f90c3860c967c41600 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Fri, 9 Sep 2022 16:20:13 +0200 Subject: [PATCH 01/23] Use minitest-reporters and declare jenkins:unit This makes it possible to properly report unit tests in CI. --- Gemfile | 1 + Rakefile | 5 +++++ test/test_helper.rb | 3 +++ 3 files changed, 9 insertions(+) diff --git a/Gemfile b/Gemfile index 6642277..c4de343 100644 --- a/Gemfile +++ b/Gemfile @@ -12,5 +12,6 @@ end group :test do gem 'minitest' + gem 'minitest-reporters' gem 'mocha' end diff --git a/Rakefile b/Rakefile index b3a8da5..ca47371 100644 --- a/Rakefile +++ b/Rakefile @@ -17,3 +17,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/test/test_helper.rb b/test/test_helper.rb index 020e218..978d130 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -3,6 +3,9 @@ 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 From 0b36b7b81347883e6f0eed1f29aabf7eb70b9449 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Fri, 9 Sep 2022 16:42:22 +0200 Subject: [PATCH 02/23] Create a bundler group for testing This separates the pure development dependencies (which can be skipped in CI) and those needed for tests. --- Gemfile | 12 ++++++------ smart_proxy_ansible.gemspec | 6 ------ 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/Gemfile b/Gemfile index c4de343..b571a8d 100644 --- a/Gemfile +++ b/Gemfile @@ -3,15 +3,15 @@ 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 :test do - gem 'minitest' - gem 'minitest-reporters' - 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/smart_proxy_ansible.gemspec b/smart_proxy_ansible.gemspec index 5022c1a..5a51900 100644 --- a/smart_proxy_ansible.gemspec +++ b/smart_proxy_ansible.gemspec @@ -26,12 +26,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') From f82073b204c2b5f90a63620b94db0766e948a1c5 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Fri, 9 Sep 2022 23:04:13 +0200 Subject: [PATCH 03/23] Remove commented code --- lib/smart_proxy_ansible/plugin.rb | 1 - .../task_launcher/ansible_runner.rb | 17 ----------------- 2 files changed, 18 deletions(-) diff --git a/lib/smart_proxy_ansible/plugin.rb b/lib/smart_proxy_ansible/plugin.rb index 24ef5a8..84e44df 100644 --- a/lib/smart_proxy_ansible/plugin.rb +++ b/lib/smart_proxy_ansible/plugin.rb @@ -6,7 +6,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/task_launcher/ansible_runner.rb b/lib/smart_proxy_ansible/task_launcher/ansible_runner.rb index be0ff9a..8c9682c 100644 --- a/lib/smart_proxy_ansible/task_launcher/ansible_runner.rb +++ b/lib/smart_proxy_ansible/task_launcher/ansible_runner.rb @@ -27,23 +27,6 @@ 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] } } 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 From aa3ed408fed3a4cc6486334bcf8602e6ae536524 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Fri, 9 Sep 2022 22:54:22 +0200 Subject: [PATCH 04/23] More efficient debug logging Dynflow 1.2.3 introduced the ability to call a block when the log level is correct. This is useful for expensive calculations. --- lib/smart_proxy_ansible/runner/ansible_runner.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/smart_proxy_ansible/runner/ansible_runner.rb b/lib/smart_proxy_ansible/runner/ansible_runner.rb index 8ba51d0..4afeccd 100644 --- a/lib/smart_proxy_ansible/runner/ansible_runner.rb +++ b/lib/smart_proxy_ansible/runner/ansible_runner.rb @@ -229,9 +229,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 From 9757f5df9e5c7777ae89286fcbb2b23993cbb91f Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Fri, 9 Sep 2022 22:31:27 +0200 Subject: [PATCH 05/23] Fix Layout/EmptyLineAfterGuardClause cop --- lib/smart_proxy_ansible/exception.rb | 1 + lib/smart_proxy_ansible/runner/ansible_runner.rb | 4 ++++ lib/smart_proxy_ansible/variables_extractor.rb | 1 + test/test_helper.rb | 1 + 4 files changed, 7 insertions(+) diff --git a/lib/smart_proxy_ansible/exception.rb b/lib/smart_proxy_ansible/exception.rb index da9093c..17a7a9d 100644 --- a/lib/smart_proxy_ansible/exception.rb +++ b/lib/smart_proxy_ansible/exception.rb @@ -12,6 +12,7 @@ def initialize(message, *params) 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 diff --git a/lib/smart_proxy_ansible/runner/ansible_runner.rb b/lib/smart_proxy_ansible/runner/ansible_runner.rb index 4afeccd..88959a2 100644 --- a/lib/smart_proxy_ansible/runner/ansible_runner.rb +++ b/lib/smart_proxy_ansible/runner/ansible_runner.rb @@ -81,6 +81,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 +90,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 @@ -197,6 +199,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 @@ -253,6 +256,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/variables_extractor.rb b/lib/smart_proxy_ansible/variables_extractor.rb index 8ba375b..e775cc5 100644 --- a/lib/smart_proxy_ansible/variables_extractor.rb +++ b/lib/smart_proxy_ansible/variables_extractor.rb @@ -16,6 +16,7 @@ def extract_variables(role_path) raise ReadVariablesException.new "#{role_file} is not YAML file" end raise ReadVariablesException.new "Could not parse YAML file: #{role_file}" unless loaded_yaml.is_a? Hash + memo.merge loaded_yaml end end diff --git a/test/test_helper.rb b/test/test_helper.rb index 978d130..525ed8b 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -14,6 +14,7 @@ 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? define_method(test_name, &block) else From 7ac867e31672ee857a721f61334e3c0924390ca8 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Fri, 9 Sep 2022 22:31:59 +0200 Subject: [PATCH 06/23] Fix Layout/EmptyLineAfterMagicComment cop --- smart_proxy_ansible.gemspec | 1 + 1 file changed, 1 insertion(+) diff --git a/smart_proxy_ansible.gemspec b/smart_proxy_ansible.gemspec index 5a51900..5667bef 100644 --- a/smart_proxy_ansible.gemspec +++ b/smart_proxy_ansible.gemspec @@ -1,4 +1,5 @@ # -*- encoding: utf-8 -*- + lib = File.expand_path('../lib', __FILE__) $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib) require 'smart_proxy_ansible/version' From 83a3057e30d1bf23f038994aa0429af7f6b0e7b0 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Fri, 9 Sep 2022 22:32:20 +0200 Subject: [PATCH 07/23] Fix Layout/EmptyLinesAroundClassBody cop --- test/playbooks_reader_test.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/test/playbooks_reader_test.rb b/test/playbooks_reader_test.rb index 8190de1..d716d5e 100644 --- a/test/playbooks_reader_test.rb +++ b/test/playbooks_reader_test.rb @@ -4,7 +4,6 @@ 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'] } From 81892a2c79c4f4462954ccf1e27cbd3b8a589c19 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Fri, 9 Sep 2022 22:32:41 +0200 Subject: [PATCH 08/23] Fix Layout/MultilineMethodCallIndentation cop --- test/roles_reader_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 From 45aac50f69334b4e9be3cfca4188d43ec32304c0 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Fri, 9 Sep 2022 22:33:10 +0200 Subject: [PATCH 09/23] Fix Lint/AmbiguousOperatorPrecedence cop --- lib/smart_proxy_ansible/runner/ansible_runner.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/smart_proxy_ansible/runner/ansible_runner.rb b/lib/smart_proxy_ansible/runner/ansible_runner.rb index 88959a2..e204141 100644 --- a/lib/smart_proxy_ansible/runner/ansible_runner.rb +++ b/lib/smart_proxy_ansible/runner/ansible_runner.rb @@ -213,7 +213,7 @@ def check_cmd end def verbosity - '-' + 'v' * @verbosity_level.to_i + '-' + ('v' * @verbosity_level.to_i) end def verbose? From 324a4802d7a2068c262a7875b82ca6ebb95ac101 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Fri, 9 Sep 2022 22:33:24 +0200 Subject: [PATCH 10/23] Fix Minitest/RefuteFalse cop --- test/playbooks_reader_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/playbooks_reader_test.rb b/test/playbooks_reader_test.rb index d716d5e..c74f2eb 100644 --- a/test/playbooks_reader_test.rb +++ b/test/playbooks_reader_test.rb @@ -69,8 +69,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 From 21274b580a8978dfaf97f60f900b7a9134131e06 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Fri, 9 Sep 2022 22:33:38 +0200 Subject: [PATCH 11/23] Fix Performance/BlockGivenWithExplicitBlock cop --- test/test_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_helper.rb b/test/test_helper.rb index 525ed8b..c986051 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -15,7 +15,7 @@ def test(name, &block) defined = method_defined? test_name fail "#{test_name} is already defined in #{self}" if defined - if block_given? + if block define_method(test_name, &block) else define_method(test_name) do From 978f660f63997bf44b52583255a643836250d058 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Fri, 9 Sep 2022 22:36:00 +0200 Subject: [PATCH 12/23] Fix Style/Encoding cop --- smart_proxy_ansible.gemspec | 2 -- 1 file changed, 2 deletions(-) diff --git a/smart_proxy_ansible.gemspec b/smart_proxy_ansible.gemspec index 5667bef..fd89503 100644 --- a/smart_proxy_ansible.gemspec +++ b/smart_proxy_ansible.gemspec @@ -1,5 +1,3 @@ -# -*- encoding: utf-8 -*- - lib = File.expand_path('../lib', __FILE__) $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib) require 'smart_proxy_ansible/version' From 77930ac2ed94771ddbab7dd33f7627782ff008e7 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Fri, 9 Sep 2022 22:36:13 +0200 Subject: [PATCH 13/23] Fix Style/ExpandPathArguments cop --- smart_proxy_ansible.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/smart_proxy_ansible.gemspec b/smart_proxy_ansible.gemspec index fd89503..6bf0951 100644 --- a/smart_proxy_ansible.gemspec +++ b/smart_proxy_ansible.gemspec @@ -1,4 +1,4 @@ -lib = File.expand_path('../lib', __FILE__) +lib = File.expand_path('lib', __dir__) $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib) require 'smart_proxy_ansible/version' From 335e973aa401431944cad8ad24a3ce2a871eb1c7 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Fri, 9 Sep 2022 22:37:01 +0200 Subject: [PATCH 14/23] Fix Style/FrozenStringLiteralComment cop --- Gemfile | 2 ++ Rakefile | 2 ++ bundler.d/ansible.rb | 2 ++ lib/smart_proxy_ansible.rb | 2 ++ lib/smart_proxy_ansible/api.rb | 2 ++ lib/smart_proxy_ansible/configuration_loader.rb | 2 ++ lib/smart_proxy_ansible/http_config.ru | 2 ++ lib/smart_proxy_ansible/playbooks_reader.rb | 2 ++ lib/smart_proxy_ansible/plugin.rb | 2 ++ lib/smart_proxy_ansible/reader_helper.rb | 6 ++++-- lib/smart_proxy_ansible/roles_reader.rb | 4 +++- lib/smart_proxy_ansible/runner/ansible_runner.rb | 2 ++ lib/smart_proxy_ansible/task_launcher/ansible_runner.rb | 2 ++ lib/smart_proxy_ansible/validate_settings.rb | 2 ++ lib/smart_proxy_ansible/variables_extractor.rb | 2 ++ lib/smart_proxy_ansible/version.rb | 2 ++ smart_proxy_ansible.gemspec | 2 ++ test/playbooks_reader_test.rb | 2 ++ test/test_helper.rb | 2 ++ test/variables_extractor_test.rb | 2 ++ 20 files changed, 43 insertions(+), 3 deletions(-) diff --git a/Gemfile b/Gemfile index b571a8d..d1bcd14 100644 --- a/Gemfile +++ b/Gemfile @@ -1,3 +1,5 @@ +# frozen_string_literal: true + source 'https://rubygems.org' gemspec diff --git a/Rakefile b/Rakefile index ca47371..dbd816c 100644 --- a/Rakefile +++ b/Rakefile @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'rake' require 'rake/testtask' 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..b570f37 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, 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/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..25151b5 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 diff --git a/lib/smart_proxy_ansible/plugin.rb b/lib/smart_proxy_ansible/plugin.rb index 84e44df..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 diff --git a/lib/smart_proxy_ansible/reader_helper.rb b/lib/smart_proxy_ansible/reader_helper.rb index d8f4a7a..38ce6b7 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) diff --git a/lib/smart_proxy_ansible/roles_reader.rb b/lib/smart_proxy_ansible/roles_reader.rb index b690caf..d55675f 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,7 +7,7 @@ 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 diff --git a/lib/smart_proxy_ansible/runner/ansible_runner.rb b/lib/smart_proxy_ansible/runner/ansible_runner.rb index e204141..a343743 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' diff --git a/lib/smart_proxy_ansible/task_launcher/ansible_runner.rb b/lib/smart_proxy_ansible/task_launcher/ansible_runner.rb index 8c9682c..88775d8 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' diff --git a/lib/smart_proxy_ansible/validate_settings.rb b/lib/smart_proxy_ansible/validate_settings.rb index 8d54c7a..e9b6472 100644 --- a/lib/smart_proxy_ansible/validate_settings.rb +++ b/lib/smart_proxy_ansible/validate_settings.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Proxy::Ansible class ValidateSettings < ::Proxy::PluginValidators::Base def validate!(settings) diff --git a/lib/smart_proxy_ansible/variables_extractor.rb b/lib/smart_proxy_ansible/variables_extractor.rb index e775cc5..5c29d8d 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 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 6bf0951..6e11070 100644 --- a/smart_proxy_ansible.gemspec +++ b/smart_proxy_ansible.gemspec @@ -1,3 +1,5 @@ +# frozen_string_literal: true + lib = File.expand_path('lib', __dir__) $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib) require 'smart_proxy_ansible/version' diff --git a/test/playbooks_reader_test.rb b/test/playbooks_reader_test.rb index c74f2eb..e19c932 100644 --- a/test/playbooks_reader_test.rb +++ b/test/playbooks_reader_test.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'test_helper' require_relative '../lib/smart_proxy_ansible/playbooks_reader' require_relative '../lib/smart_proxy_ansible/exception' diff --git a/test/test_helper.rb b/test/test_helper.rb index c986051..395ac68 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'minitest/autorun' require 'minitest/spec' require 'mocha/minitest' 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' From 9acc835f0ec1eec5b47a3d900ed8395fc91d9368 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Fri, 9 Sep 2022 22:39:10 +0200 Subject: [PATCH 15/23] Fix Style/MultilineIfModifier cop --- lib/smart_proxy_ansible/api.rb | 6 ++++-- lib/smart_proxy_ansible/playbooks_reader.rb | 4 +++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/smart_proxy_ansible/api.rb b/lib/smart_proxy_ansible/api.rb index b570f37..f205171 100644 --- a/lib/smart_proxy_ansible/api.rb +++ b/lib/smart_proxy_ansible/api.rb @@ -45,8 +45,10 @@ def extract_variables(role_name) role_name_parts = role_name.split('.') if role_name_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/#{role_name_parts[0]}/#{role_name_parts[1]}/roles/#{role_name_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/playbooks_reader.rb b/lib/smart_proxy_ansible/playbooks_reader.rb index 25151b5..45dafd7 100644 --- a/lib/smart_proxy_ansible/playbooks_reader.rb +++ b/lib/smart_proxy_ansible/playbooks_reader.rb @@ -24,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}" From 9a15aa7d94fd00fbcbd610aa4bc12cc94ebec443 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Fri, 9 Sep 2022 22:41:57 +0200 Subject: [PATCH 16/23] Fix Style/RaiseArgs cop --- lib/smart_proxy_ansible/validate_settings.rb | 2 +- lib/smart_proxy_ansible/variables_extractor.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/smart_proxy_ansible/validate_settings.rb b/lib/smart_proxy_ansible/validate_settings.rb index e9b6472..5fadd46 100644 --- a/lib/smart_proxy_ansible/validate_settings.rb +++ b/lib/smart_proxy_ansible/validate_settings.rb @@ -3,7 +3,7 @@ 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])) + raise NotExistingWorkingDirException, "Working directory does not exist" unless settings[:working_dir].nil? || File.directory?(File.expand_path(settings[:working_dir])) end end end diff --git a/lib/smart_proxy_ansible/variables_extractor.rb b/lib/smart_proxy_ansible/variables_extractor.rb index 5c29d8d..47313ba 100644 --- a/lib/smart_proxy_ansible/variables_extractor.rb +++ b/lib/smart_proxy_ansible/variables_extractor.rb @@ -15,9 +15,9 @@ 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 From d39c337fe5669b834bc874e1cf3177713b3e18ca Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Fri, 9 Sep 2022 22:42:18 +0200 Subject: [PATCH 17/23] Fix Style/SelectByRegexp cop --- lib/smart_proxy_ansible/reader_helper.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/smart_proxy_ansible/reader_helper.rb b/lib/smart_proxy_ansible/reader_helper.rb index 38ce6b7..9f04628 100644 --- a/lib/smart_proxy_ansible/reader_helper.rb +++ b/lib/smart_proxy_ansible/reader_helper.rb @@ -23,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}" From ffd59c317b4858fe4ee3478367cf6a657f708289 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Fri, 9 Sep 2022 22:42:42 +0200 Subject: [PATCH 18/23] Fix Style/SignalException cop --- test/test_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_helper.rb b/test/test_helper.rb index 395ac68..15134e9 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -15,7 +15,7 @@ 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 + raise "#{test_name} is already defined in #{self}" if defined if block define_method(test_name, &block) From 514e974bb54f4319f1da30d245cfbd915fd795f1 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Fri, 9 Sep 2022 22:43:05 +0200 Subject: [PATCH 19/23] Fix Style/StringConcatenation cop --- lib/smart_proxy_ansible/runner/ansible_runner.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/smart_proxy_ansible/runner/ansible_runner.rb b/lib/smart_proxy_ansible/runner/ansible_runner.rb index a343743..7338268 100644 --- a/lib/smart_proxy_ansible/runner/ansible_runner.rb +++ b/lib/smart_proxy_ansible/runner/ansible_runner.rb @@ -128,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? @@ -155,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 @@ -215,7 +215,7 @@ def check_cmd end def verbosity - '-' + ('v' * @verbosity_level.to_i) + "-#{'v' * @verbosity_level.to_i}" end def verbose? From 14debf01f4001076b6413f807a3c4da441f0b988 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Fri, 9 Sep 2022 22:46:16 +0200 Subject: [PATCH 20/23] Fix Naming/HeredocDelimiterNaming cop --- smart_proxy_ansible.gemspec | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/smart_proxy_ansible.gemspec b/smart_proxy_ansible.gemspec index 6e11070..dde18db 100644 --- a/smart_proxy_ansible.gemspec +++ b/smart_proxy_ansible.gemspec @@ -11,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/**/*', From b84ff88c1a52dda783259d76fd413ecf697cf9cd Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Fri, 9 Sep 2022 22:46:31 +0200 Subject: [PATCH 21/23] Fix Lint/SuppressedException cop --- Rakefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Rakefile b/Rakefile index dbd816c..8594dda 100644 --- a/Rakefile +++ b/Rakefile @@ -6,6 +6,7 @@ require 'rake/testtask' begin require 'bundler/gem_tasks' rescue LoadError + # This is optional end desc 'Default: run unit tests.' From 9ca078570da10dd3f6c0eb439a72e6cf43ca47e8 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Fri, 9 Sep 2022 22:55:59 +0200 Subject: [PATCH 22/23] Fix Layout/LineLength cop --- lib/smart_proxy_ansible/api.rb | 6 +++--- lib/smart_proxy_ansible/roles_reader.rb | 4 +++- lib/smart_proxy_ansible/task_launcher/ansible_runner.rb | 3 ++- lib/smart_proxy_ansible/validate_settings.rb | 4 +++- 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/lib/smart_proxy_ansible/api.rb b/lib/smart_proxy_ansible/api.rb index f205171..84ef862 100644 --- a/lib/smart_proxy_ansible/api.rb +++ b/lib/smart_proxy_ansible/api.rb @@ -42,11 +42,11 @@ 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| if variables[role_name].nil? || variables[role_name].empty? - role_path = "#{path}/ansible_collections/#{role_name_parts[0]}/#{role_name_parts[1]}/roles/#{role_name_parts[2]}" + role_path = "#{path}/ansible_collections/#{parts[0]}/#{parts[1]}/roles/#{parts[2]}" variables[role_name] = VariablesExtractor.extract_variables(role_path) end end diff --git a/lib/smart_proxy_ansible/roles_reader.rb b/lib/smart_proxy_ansible/roles_reader.rb index d55675f..2749388 100644 --- a/lib/smart_proxy_ansible/roles_reader.rb +++ b/lib/smart_proxy_ansible/roles_reader.rb @@ -11,7 +11,9 @@ class << self 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/task_launcher/ansible_runner.rb b/lib/smart_proxy_ansible/task_launcher/ansible_runner.rb index 88775d8..81dc801 100644 --- a/lib/smart_proxy_ansible/task_launcher/ansible_runner.rb +++ b/lib/smart_proxy_ansible/task_launcher/ansible_runner.rb @@ -27,7 +27,8 @@ 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 end end diff --git a/lib/smart_proxy_ansible/validate_settings.rb b/lib/smart_proxy_ansible/validate_settings.rb index 5fadd46..2ebeb82 100644 --- a/lib/smart_proxy_ansible/validate_settings.rb +++ b/lib/smart_proxy_ansible/validate_settings.rb @@ -3,7 +3,9 @@ module Proxy::Ansible class ValidateSettings < ::Proxy::PluginValidators::Base def validate!(settings) - raise NotExistingWorkingDirException, "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 From cbd7ea53a4f3e92579d79c91840a919a62af13fe Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Fri, 9 Sep 2022 16:48:34 +0200 Subject: [PATCH 23/23] Set up Rubocop --- .rubocop.yml | 37 ++++++++++++++++++++++++++++ .rubocop_todo.yml | 0 Gemfile | 7 ++++++ Rakefile | 5 +++- lib/smart_proxy_ansible/exception.rb | 4 +-- 5 files changed, 50 insertions(+), 3 deletions(-) create mode 100644 .rubocop.yml create mode 100644 .rubocop_todo.yml 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 d1bcd14..26cffac 100644 --- a/Gemfile +++ b/Gemfile @@ -9,6 +9,13 @@ group :development do 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', require: false gem 'minitest-reporters', '~> 1.4', require: false diff --git a/Rakefile b/Rakefile index 8594dda..9f4be17 100644 --- a/Rakefile +++ b/Rakefile @@ -2,6 +2,7 @@ require 'rake' require 'rake/testtask' +require 'rubocop/rake_task' begin require 'bundler/gem_tasks' @@ -9,8 +10,10 @@ 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| diff --git a/lib/smart_proxy_ansible/exception.rb b/lib/smart_proxy_ansible/exception.rb index 17a7a9d..1319a75 100644 --- a/lib/smart_proxy_ansible/exception.rb +++ b/lib/smart_proxy_ansible/exception.rb @@ -5,7 +5,7 @@ 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 @@ -16,7 +16,7 @@ def self.calculate_error_code(classname, message) 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