Add RuboCop check GH actions

.github/workflows/ci.yml

@@ -0,0 +1,20 @@
+name: CI
+on:
+  pull_request:
+  push:
+    branches:
+      - master
+jobs:
+  rubocop:
+    runs-on: ubuntu-latest
+    env:
+      BUNDLE_SMART_PROXY: '0'
+    steps:
+      - uses: actions/checkout@v2
+      - name: Setup Ruby
+        uses: ruby/setup-ruby@v1
+        with:
+          ruby-version: 2.5
+          bundler-cache: true
+      - name: Run rubocop
+        run: bundle exec rubocop --format github

.rubocop.yml

@@ -0,0 +1,20 @@
+inherit_from: .rubocop_todo.yml
+
+inherit_gem:
+  theforeman-rubocop:
+    - default.yml
+
+inherit_mode:
+  merge:
+    - Exclude
+
+AllCops:
+  Exclude:
+    - 'vendor/bundle/**/*'
+
+Metrics/BlockLength:
+  Exclude:
+    - test/**/*
+
+Style/FrozenStringLiteralComment:
+  Enabled: false

.rubocop_todo.yml

@@ -0,0 +1,74 @@
+# This configuration was generated by
+# `rubocop --auto-gen-config`
+# on 2021-11-10 20:56:15 UTC using RuboCop version 1.22.3.
+# The point is for the user to remove these configuration records
+# one by one as the offenses are removed from the code base.
+# Note that changes in the inspected code, or installation of new
+# versions of RuboCop, may require this file to be generated again.
+
+# Offense count: 5
+# Configuration parameters: IgnoredMethods, CountRepeatedAttributes.
+Metrics/AbcSize:
+  Max: 32
+
+# Offense count: 2
+# Configuration parameters: CountComments, CountAsOne.
+Metrics/ClassLength:
+  Max: 201
+
+# Offense count: 3
+# Configuration parameters: IgnoredMethods.
+Metrics/CyclomaticComplexity:
+  Max: 13
+
+# Offense count: 7
+# Configuration parameters: CountComments, CountAsOne, ExcludedMethods, IgnoredMethods.
+Metrics/MethodLength:
+  Max: 16
+
+# Offense count: 2
+# Configuration parameters: IgnoredMethods.
+Metrics/PerceivedComplexity:
+  Max: 13
+
+# Offense count: 9
+# Cop supports --auto-correct.
+# Configuration parameters: EnforcedStyle.
+# SupportedStyles: nested, compact
+Style/ClassAndModuleChildren:
+  Exclude:
+    - 'lib/smart_proxy_ansible/actions.rb'
+    - 'lib/smart_proxy_ansible/remote_execution_core/ansible_runner.rb'
+    - 'lib/smart_proxy_ansible/runner/ansible_runner.rb'
+    - 'lib/smart_proxy_ansible/runner/command_creator.rb'
+    - 'lib/smart_proxy_ansible/runner/playbook.rb'
+    - 'lib/smart_proxy_ansible/task_launcher/ansible_runner.rb'
+    - 'lib/smart_proxy_ansible/task_launcher/playbook.rb'
+    - 'test/ansible_runner_test.rb'
+    - 'test/task_launcher_test.rb'
+
+# Offense count: 1
+# Configuration parameters: MinBodyLength.
+Style/GuardClause:
+  Exclude:
+    - 'lib/smart_proxy_ansible/runner/ansible_runner.rb'
+
+# Offense count: 2
+# Cop supports --auto-correct.
+Style/IfUnlessModifier:
+  Exclude:
+    - 'lib/smart_proxy_ansible/runner/ansible_runner.rb'
+
+# Offense count: 3
+# Cop supports --auto-correct.
+# Configuration parameters: Mode.
+Style/StringConcatenation:
+  Exclude:
+    - 'lib/smart_proxy_ansible/runner/ansible_runner.rb'
+
+# Offense count: 3
+# Cop supports --auto-correct.
+# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns.
+# URISchemes: http, https
+Layout/LineLength:
+  Max: 158

Gemfile

@@ -3,14 +3,19 @@ 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'
+
+  if (smart_proxy_path = ENV.fetch('SMART_PROXY_PATH', nil))
+    gem 'smart_proxy', path: smart_proxy_path
+  elsif ENV.fetch('BUNDLE_SMART_PROXY', '1') != '0'
+    gem 'smart_proxy', git: 'https://github.com/theforeman/smart-proxy',
+                       branch: 'develop'
+  end
 end
 
 group :test do
   gem 'minitest'
   gem 'mocha'
+  gem 'theforeman-rubocop', '~> 0.1.0.pre'
 end

Rakefile

@@ -1,10 +1,6 @@
 require 'rake'
 require 'rake/testtask'
-
-begin
-  require 'bundler/gem_tasks'
-rescue LoadError
-end
+require 'bundler/gem_tasks'
 
 desc 'Default: run unit tests.'
 task default: :test

lib/smart_proxy_ansible/api.rb

@@ -35,15 +35,15 @@ def extract_variables(role_name)
         role_name_parts = role_name.split('.')
         if role_name_parts.count == 3
           RolesReader.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].blank?
+              variables[role_name] = VariablesExtractor
+                                     .extract_variables("#{path}/ansible_collections/#{role_name_parts[0]}/#{role_name_parts[1]}/roles/#{role_name_parts[2]}")
+            end
           end
         else
           RolesReader.roles_path.split(':').each do |path|
             role_path = "#{path}/#{role_name}"
-            if File.directory?(role_path)
-              variables[role_name] ||= VariablesExtractor.extract_variables(role_path)
-            end
+            variables[role_name] ||= VariablesExtractor.extract_variables(role_path) if File.directory?(role_path)
           end
         end
         variables

lib/smart_proxy_ansible/exception.rb

@@ -6,16 +6,18 @@ module Ansible
     # exception
     class Exception < ::StandardError
       def initialize(message, *params)
+        super()
         @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%<clshash>02d-%<msghash>04d', clshash: class_hash, msghash: msg_hash
       end
 
       def code
@@ -35,7 +37,9 @@ def to_s
     end
 
     class ReadConfigFileException < Proxy::Ansible::Exception; end
+
     class ReadRolesException < Proxy::Ansible::Exception; end
+
     class ReadVariablesException < Proxy::Ansible::Exception; end
   end
 end

lib/smart_proxy_ansible/http_config.ru

@@ -1,5 +1,5 @@
 require 'smart_proxy_ansible/api'
 
-map "/ansible" do
+map '/ansible' do
   run Proxy::Ansible::Api
 end

lib/smart_proxy_ansible/plugin.rb

@@ -5,8 +5,8 @@ class Plugin < Proxy::Plugin
       rackup_path File.expand_path('http_config.ru', __dir__)
       settings_file 'ansible.yml'
       plugin :ansible, Proxy::Ansible::VERSION
-      default_settings :ansible_dir => Dir.home
-                       # :working_dir => nil
+      default_settings ansible_dir: Dir.home
+      # :working_dir => nil
 
       after_activation do
         require 'smart_proxy_dynflow'

lib/smart_proxy_ansible/remote_execution_core/ansible_runner.rb

@@ -10,12 +10,12 @@ class AnsibleRunner < ::Proxy::Dynflow::Runner::CommandRunner
       CONNECTION_PROMPT = 'Are you sure you want to continue connecting (yes/no)? '
 
       def initialize(options, suspended_action:)
-        super(options, :suspended_action => suspended_action)
+        super(options, suspended_action: suspended_action)
         @playbook_runner = Proxy::Ansible::Runner::Playbook.new(
           options['ansible_inventory'],
           options['script'],
           options,
-          :suspended_action => suspended_action
+          suspended_action: suspended_action
         )
       end
 

lib/smart_proxy_ansible/roles_reader.rb

@@ -58,7 +58,6 @@ def read_roles(roles_path)
 
         def glob_path(path)
           Dir.glob path
-
         end
 
         def read_collection_roles(collections_path)

lib/smart_proxy_ansible/runner/ansible_runner.rb

@@ -11,7 +11,7 @@ class AnsibleRunner < ::Proxy::Dynflow::Runner::Parent
       attr_reader :execution_timeout_interval, :command_pid
 
       def initialize(input, suspended_action:)
-        super input, :suspended_action => suspended_action
+        super input, suspended_action: suspended_action
         @inventory = rebuild_secrets(rebuild_inventory(input), input)
         action_input = input.values.first[:input][:action_input]
         @playbook = action_input[:script]
@@ -30,7 +30,7 @@ def start
         prepare_directory_structure
         write_inventory
         write_playbook
-        write_ssh_key if !@passphrase.nil? && !@passphrase.empty?
+        write_ssh_key if @passphrase.present?
         start_ansible_runner
       end
 
@@ -106,7 +106,7 @@ def handle_host_event(hostname, event)
       end
 
       def handle_broadcast_data(event)
-        log_event("broadcast", event)
+        log_event('broadcast', event)
         if event['event'] == 'playbook_on_stats'
           failures = event.dig('event_data', 'failures') || {}
           unreachable = event.dig('event_data', 'dark') || {}
@@ -150,7 +150,7 @@ def write_ssh_key
         # to match line asking for password, given the limitation to match only first 100 chars
         # and the fact the line contains dynamically created temp directory, the regexp
         # mentions only things that are always there, such as artifacts directory and the key name
-        secrets = YAML.dump({ "for.*/artifacts/.*/ssh_key_data:" => @passphrase })
+        secrets = YAML.dump({ 'for.*/artifacts/.*/ssh_key_data:' => @passphrase })
         File.write(passwords_path, secrets, perm: 0o600)
       end
 
@@ -201,7 +201,9 @@ def prepare_directory_structure
       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
+        if logger.level <= ::Logger::DEBUG
+          logger.debug("[foreman_ansible] - handling event #{description}: #{JSON.pretty_generate(event)}")
+        end
       end
 
       # Each per-host task has inventory only for itself, we must
@@ -241,7 +243,7 @@ def rebuild_secrets(inventory, input)
 
           new_secrets = {
             'ansible_password' => inventory['ssh_password'] || per_host['ansible_password'],
-            'ansible_become_password' => inventory['effective_user_password'] || per_host['ansible_become_password']
+            'ansible_become_password' => inventory['effective_user_password'] || per_host['ansible_become_password'],
           }
           inventory['_meta']['hostvars'][host].update(new_secrets)
         end

lib/smart_proxy_ansible/runner/playbook.rb

@@ -14,7 +14,7 @@ class Playbook < Proxy::Dynflow::Runner::CommandRunner
       attr_reader :command_out, :command_in, :command_pid
 
       def initialize(inventory, playbook, options = {}, suspended_action:)
-        super :suspended_action => suspended_action
+        super suspended_action: suspended_action
         @inventory = rebuild_secrets(inventory, options[:secrets])
         unknown_hosts.each do |host|
           add_to_known_hosts(host)
@@ -28,8 +28,8 @@ def start
         write_inventory
         write_playbook
         command = CommandCreator.new(inventory_file,
-                                     playbook_file,
-                                     @options).command
+          playbook_file,
+          @options).command
         logger.debug('[foreman_ansible] - Initializing Ansible Runner')
         Dir.chdir(@ansible_dir) do
           initialize_command(*command)
@@ -126,7 +126,7 @@ def rebuild_secrets(inventory, secrets)
 
           new_secrets = {
             'ansible_password' => inventory['ssh_password'] || per_host['ansible_password'],
-            'ansible_become_password' => inventory['effective_user_password'] || per_host['ansible_become_password']
+            'ansible_become_password' => inventory['effective_user_password'] || per_host['ansible_become_password'],
           }
           inventory['_meta']['hostvars'][name].update(new_secrets)
         end

lib/smart_proxy_ansible/task_launcher/playbook.rb

@@ -6,19 +6,19 @@ class Playbook < Proxy::Dynflow::TaskLauncher::Batch
       class PlaybookRunnerAction < Proxy::Dynflow::Action::Runner
         def initiate_runner
           additional_options = {
-            :step_id => run_step_id,
-            :uuid => execution_plan_id
+            step_id: run_step_id,
+            uuid: execution_plan_id,
           }
           ::Proxy::Ansible::RemoteExecutionCore::AnsibleRunner.new(
             input.merge(additional_options),
-            :suspended_action => suspended_action
+            suspended_action: suspended_action
           )
         end
       end
 
       def child_launcher(parent)
-        ::Proxy::Dynflow::TaskLauncher::Single.new(world, callback, :parent => parent,
-                                                                    :action_class_override => PlaybookRunnerAction)
+        ::Proxy::Dynflow::TaskLauncher::Single.new(world, callback, parent: parent,
+                                                                    action_class_override: PlaybookRunnerAction)
       end
     end
   end

lib/smart_proxy_ansible/variables_extractor.rb

@@ -9,16 +9,19 @@ def extract_variables(role_path)
           role_files = Dir.glob("#{role_path}/defaults/**/*.yml") +
                        Dir.glob("#{role_path}/defaults/**/*.yaml")
           role_files.reduce({}) do |memo, role_file|
-            loaded_yaml = {}
-            begin
-              loaded_yaml = YAML.load_file(role_file)
-            rescue Psych::SyntaxError
-              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
+            loaded_yaml = load_role_file(role_file)
+
             memo.merge loaded_yaml
           end
         end
+
+        def load_role_file(file)
+          loaded_yaml = YAML.load_file(file)
+          raise ReadVariablesException, "Could not parse YAML file: #{file}" unless loaded_yaml.is_a? Hash
+          loaded_yaml
+        rescue Psych::SyntaxError
+          raise ReadVariablesException, "#{file} is not YAML file"
+        end
       end
     end
   end

lib/smart_proxy_ansible/version.rb

@@ -2,6 +2,6 @@ module Proxy
   # Version, this allows the proxy and other plugins know
   # what version of the Ansible plugin is running
   module Ansible
-    VERSION = '3.2.0'
+    VERSION = '3.2.0'.freeze
   end
 end