Update CI setup

.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

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

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

bundler.d/ansible.rb

@@ -1 +1,3 @@
+# frozen_string_literal: true
+
 gem 'smart_proxy_ansible'

lib/smart_proxy_ansible.rb

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'smart_proxy_dynflow'
 
 module Proxy

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|

lib/smart_proxy_ansible/configuration_loader.rb

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 module Proxy::Ansible
   class ConfigurationLoader
     def load_classes

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

lib/smart_proxy_ansible/http_config.ru

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'smart_proxy_ansible/api'
 
 map "/ansible" do

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}"

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

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}"

lib/smart_proxy_ansible/roles_reader.rb

@@ -1,15 +1,19 @@
+# frozen_string_literal: true
+
 require_relative 'exception'
 
 module Proxy
   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
 

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?

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

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

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

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