From 439311dedfee2d07175f53d750a9bf0fdf1d19c9 Mon Sep 17 00:00:00 2001 From: Daniel Lobato Garcia Date: Wed, 26 Jul 2017 14:54:48 +0000 Subject: [PATCH 1/4] Fix rubocop issues --- .rubocop.yml | 6 +++ Rakefile | 2 +- app/controllers/ansible_roles_controller.rb | 2 +- .../api/v2/hosts_controller_extensions.rb | 3 +- .../foreman_ansible/play_host_roles.rb | 7 ++- .../foreman_ansible/play_hostgroup_roles.rb | 7 ++- app/models/ansible_role.rb | 2 +- app/models/host_ansible_role.rb | 2 +- app/models/hostgroup_ansible_role.rb | 2 +- app/services/foreman_ansible/fact_importer.rb | 2 +- app/services/foreman_ansible/fact_parser.rb | 10 ++-- .../foreman_ansible/ui_roles_importer.rb | 2 +- config/routes.rb | 1 + ...54057_automatically_set_role_timestamps.rb | 13 +++-- foreman_ansible_core.gemspec | 4 +- lib/foreman_ansible/engine.rb | 4 +- lib/foreman_ansible_core/command_creator.rb | 43 +++++++++++++++++ lib/foreman_ansible_core/playbook_runner.rb | 48 ++++--------------- 18 files changed, 92 insertions(+), 68 deletions(-) mode change 100644 => 100755 Rakefile create mode 100644 lib/foreman_ansible_core/command_creator.rb diff --git a/.rubocop.yml b/.rubocop.yml index 585dd2cd..bcbbd75a 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -19,3 +19,9 @@ Rails/ActionFilter: Style/RaiseArgs: EnforcedStyle: compact + +Style/SymbolArray: + EnforcedStyle: brackets + +Style/FormatStringToken: + EnforcedStyle: template diff --git a/Rakefile b/Rakefile old mode 100644 new mode 100755 index f6abf081..74f79046 --- a/Rakefile +++ b/Rakefile @@ -6,7 +6,7 @@ rescue LoadError end require 'rake/testtask' -Rake::TestTask.new("test:core") do |test| +Rake::TestTask.new('test:core') do |test| test_dir = File.join(File.dirname(__FILE__), 'test/lib') test.pattern = "#{test_dir}/**/*_test.rb" test.libs << test_dir diff --git a/app/controllers/ansible_roles_controller.rb b/app/controllers/ansible_roles_controller.rb index d6d260f1..7717a2bc 100644 --- a/app/controllers/ansible_roles_controller.rb +++ b/app/controllers/ansible_roles_controller.rb @@ -49,7 +49,7 @@ def create_importer end def no_changed_roles_message - return _('No changes in roles detected.') unless @proxy.present? + return _('No changes in roles detected.') if @proxy.blank? _('No changes in roles detected on %s.') % @proxy.name end end diff --git a/app/controllers/foreman_ansible/api/v2/hosts_controller_extensions.rb b/app/controllers/foreman_ansible/api/v2/hosts_controller_extensions.rb index 48b57f0e..07a83882 100644 --- a/app/controllers/foreman_ansible/api/v2/hosts_controller_extensions.rb +++ b/app/controllers/foreman_ansible/api/v2/hosts_controller_extensions.rb @@ -7,7 +7,8 @@ module HostsControllerExtensions include ForemanTasks::Triggers included do - api :POST, '/hosts/:id/play_roles', N_('Plays Ansible roles on a host') + api :POST, '/hosts/:id/play_roles', + N_('Plays Ansible roles on a host') param :id, String, :required => true def play_roles diff --git a/app/lib/actions/foreman_ansible/play_host_roles.rb b/app/lib/actions/foreman_ansible/play_host_roles.rb index 1c54f031..ffd0565a 100644 --- a/app/lib/actions/foreman_ansible/play_host_roles.rb +++ b/app/lib/actions/foreman_ansible/play_host_roles.rb @@ -23,10 +23,9 @@ def plan(host, proxy_selector = ::ForemanAnsible::ProxySelector.new, end def humanized_input - _('on host %{name} through %{proxy}') % { - :name => input.fetch(:host, {})[:name], - :proxy => running_proxy_name - } + format(_('on host %{name} through %{proxy}'), + :name => input.fetch(:host, {})[:name], + :proxy => running_proxy_name) end private diff --git a/app/lib/actions/foreman_ansible/play_hostgroup_roles.rb b/app/lib/actions/foreman_ansible/play_hostgroup_roles.rb index 5566dbd5..49a02264 100644 --- a/app/lib/actions/foreman_ansible/play_hostgroup_roles.rb +++ b/app/lib/actions/foreman_ansible/play_hostgroup_roles.rb @@ -23,10 +23,9 @@ def plan(hostgroup, proxy_selector = ::ForemanAnsible::ProxySelector.new, end def humanized_input - _('on host group %{name} through proxy %{proxy}') % { - :name => input.fetch(:hostgroup, {})[:name], - :proxy => running_proxy_name - } + format(_('on host group %{name} through proxy %{proxy}'), + :name => input.fetch(:hostgroup, {})[:name], + :proxy => running_proxy_name) end private diff --git a/app/models/ansible_role.rb b/app/models/ansible_role.rb index 77c999a7..567d960b 100644 --- a/app/models/ansible_role.rb +++ b/app/models/ansible_role.rb @@ -1,5 +1,5 @@ # Simple model to store basic info about the Ansible role -class AnsibleRole < ActiveRecord::Base +class AnsibleRole < ApplicationRecord include Authorizable self.include_root_in_json = false diff --git a/app/models/host_ansible_role.rb b/app/models/host_ansible_role.rb index 73ea8dca..b1a498b9 100644 --- a/app/models/host_ansible_role.rb +++ b/app/models/host_ansible_role.rb @@ -1,5 +1,5 @@ # Join model that hosts the connection between hosts and ansible_roles -class HostAnsibleRole < ActiveRecord::Base +class HostAnsibleRole < ApplicationRecord audited :associated_with => :host, :allow_mass_assignment => true belongs_to_host belongs_to :ansible_role diff --git a/app/models/hostgroup_ansible_role.rb b/app/models/hostgroup_ansible_role.rb index ca496ae1..7f20aa1c 100644 --- a/app/models/hostgroup_ansible_role.rb +++ b/app/models/hostgroup_ansible_role.rb @@ -1,5 +1,5 @@ # Join model that hosts the connection between hostgroups and ansible_roles -class HostgroupAnsibleRole < ActiveRecord::Base +class HostgroupAnsibleRole < ApplicationRecord belongs_to :hostgroup belongs_to :ansible_role diff --git a/app/services/foreman_ansible/fact_importer.rb b/app/services/foreman_ansible/fact_importer.rb index 2d923c39..d0f1388f 100644 --- a/app/services/foreman_ansible/fact_importer.rb +++ b/app/services/foreman_ansible/fact_importer.rb @@ -31,7 +31,7 @@ def add_new_facts end def add_missing_facts(imported_facts, parent = nil, prefix = '') - imported_facts.select! { |_fact_name, fact_value| !fact_value.nil? } + imported_facts.reject! { |_fact_name, fact_value| fact_value.nil? } imported_facts.each do |imported_name, imported_value| fact_fqn = fact_fqn(imported_name, prefix) diff --git a/app/services/foreman_ansible/fact_parser.rb b/app/services/foreman_ansible/fact_parser.rb index ce4f9486..863d56d1 100644 --- a/app/services/foreman_ansible/fact_parser.rb +++ b/app/services/foreman_ansible/fact_parser.rb @@ -18,19 +18,19 @@ def environment; end # Don't do anything as there's no env in Ansible def architecture name = facts[:ansible_architecture] || facts[:facter_architecture] - Architecture.where(:name => name).first_or_create unless name.blank? + Architecture.where(:name => name).first_or_create if name.present? end def model name = detect_fact([:ansible_product_name, :facter_virtual, :facter_productname, :facter_model, :model]) - Model.where(:name => name.strip).first_or_create unless name.blank? + Model.where(:name => name.strip).first_or_create if name.present? end def domain name = detect_fact([:ansible_domain, :facter_domain, :ohai_domain, :domain]) - Domain.where(:name => name).first_or_create unless name.blank? + Domain.where(:name => name).first_or_create if name.present? end def support_interfaces_parsing? @@ -65,12 +65,12 @@ def ipmi_interface; end private def ansible_interfaces - return [] unless facts[:ansible_interfaces].present? + return [] if facts[:ansible_interfaces].blank? facts[:ansible_interfaces].sort end def ip_from_interface(interface) - return unless facts[:"ansible_#{interface}"]['ipv4'].present? + return if facts[:"ansible_#{interface}"]['ipv4'].blank? facts[:"ansible_#{interface}"]['ipv4']['address'] end diff --git a/app/services/foreman_ansible/ui_roles_importer.rb b/app/services/foreman_ansible/ui_roles_importer.rb index ba8ebeb6..ea60b062 100644 --- a/app/services/foreman_ansible/ui_roles_importer.rb +++ b/app/services/foreman_ansible/ui_roles_importer.rb @@ -6,7 +6,7 @@ def import! end def finish_import(changes) - return unless changes.present? + return if changes.blank? create_new_roles changes['new'] if changes['new'] delete_old_roles changes['obsolete'] if changes['obsolete'] end diff --git a/config/routes.rb b/config/routes.rb index 92e6984a..321e7063 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,3 +1,4 @@ +# rubocop:disable BlockLength Rails.application.routes.draw do scope '/ansible' do constraints(:id => %r{[^\/]+}) do diff --git a/db/migrate/20161122154057_automatically_set_role_timestamps.rb b/db/migrate/20161122154057_automatically_set_role_timestamps.rb index df3050fc..0ab76d13 100644 --- a/db/migrate/20161122154057_automatically_set_role_timestamps.rb +++ b/db/migrate/20161122154057_automatically_set_role_timestamps.rb @@ -1,11 +1,16 @@ +# Creation and updates timestamps for Ansible Roles class AutomaticallySetRoleTimestamps < ActiveRecord::Migration def up - change_column :ansible_roles, :created_at, :datetime, :null => true, :default => nil - change_column :ansible_roles, :updated_at, :datetime, :null => true, :default => nil + change_column :ansible_roles, :created_at, :datetime, :null => true, + :default => nil + change_column :ansible_roles, :updated_at, :datetime, :null => true, + :default => nil end def down - change_column :ansible_roles, :created_at, :datetime, :null => false, :default => Time.now.utc - change_column :ansible_roles, :updated_at, :datetime, :null => false, :default => Time.now.utc + change_column :ansible_roles, :created_at, :datetime, + :null => false, :default => Time.now.utc + change_column :ansible_roles, :updated_at, :datetime, + :null => false, :default => Time.now.utc end end diff --git a/foreman_ansible_core.gemspec b/foreman_ansible_core.gemspec index 8c0e4237..7a5bc155 100644 --- a/foreman_ansible_core.gemspec +++ b/foreman_ansible_core.gemspec @@ -9,8 +9,8 @@ Gem::Specification.new do |s| s.homepage = 'https://github.com/theforeman/foreman_ansible' s.summary = 'Ansible integration with Foreman (theforeman.org): core bits' s.description = < :finisher_hook do Foreman::Plugin.register :foreman_ansible do requires_foreman '>= 1.15' @@ -98,7 +100,7 @@ class Engine < ::Rails::Engine end initializer 'foreman_ansible.assets.precompile' do |app| - app.config.assets.precompile += %w(foreman_ansible/Ansible.png) + app.config.assets.precompile += %w[foreman_ansible/Ansible.png] end initializer 'foreman_ansible.configure_assets', :group => :assets do diff --git a/lib/foreman_ansible_core/command_creator.rb b/lib/foreman_ansible_core/command_creator.rb new file mode 100644 index 00000000..d8a763f8 --- /dev/null +++ b/lib/foreman_ansible_core/command_creator.rb @@ -0,0 +1,43 @@ +module ForemanAnsibleCore + # Creates the actual command to be passed to foreman_tasks_core to run + class CommandCreator + attr_reader :command + + def initialize(inventory_file, playbook_file, options = {}) + @options = options + @command = [{ 'JSON_INVENTORY_FILE' => inventory_file }] + @command << 'ansible-playbook' + @command = command_options(@command) + @command << playbook_file + end + + private + + def command_options(command) + command.concat(['-i', json_inventory_script]) + command.concat([setup_verbosity]) if verbose? + command.concat(['-T', @options[:timeout]]) unless @options[:timeout].nil? + command + end + + def json_inventory_script + File.expand_path('../../bin/json_inventory.sh', File.dirname(__FILE__)) + end + + def setup_verbosity + verbosity_level = @options[:verbosity_level].to_i + verbosity = '-' + verbosity_level.times do + verbosity += 'v' + end + verbosity + end + + def verbose? + verbosity_level = @options[:verbosity_level] + # rubocop:disable Rails/Present + !verbosity_level.nil? && !verbosity_level.empty? && + verbosity_level.to_i > 0 + end + end +end diff --git a/lib/foreman_ansible_core/playbook_runner.rb b/lib/foreman_ansible_core/playbook_runner.rb index 00218143..803b5cf8 100644 --- a/lib/foreman_ansible_core/playbook_runner.rb +++ b/lib/foreman_ansible_core/playbook_runner.rb @@ -1,4 +1,5 @@ require 'foreman_tasks_core/runner/command_runner' +require_relative 'command_creator' require 'tmpdir' module ForemanAnsibleCore @@ -16,25 +17,16 @@ def initialize(inventory, playbook, options = {}) def start write_inventory write_playbook + command = CommandCreator.new(inventory_file, + playbook_file, + @options).command logger.debug('[foreman_ansible] - Initializing Ansible Runner') Dir.chdir(@ansible_dir) do initialize_command(*command) + logger.debug("[foreman_ansible] - Running command #{command}") end end - def command - command = [{ 'JSON_INVENTORY_FILE' => inventory_file }] - command << 'ansible-playbook' - command.concat(['-i', json_inventory_script]) - if verbose? - command.concat([setup_verbosity]) - end - command.concat(['-T', @options[:timeout]]) unless @options[:timeout].nil? - command << playbook_file - logger.debug("[foreman_ansible] - Running command #{command}") - command - end - def kill publish_data('== TASK ABORTED BY USER ==', 'stdout') publish_exit_status(1) @@ -47,10 +39,6 @@ def close FileUtils.remove_entry(@working_dir) if @tmp_working_dir end - def json_inventory_script - File.expand_path('../../bin/json_inventory.sh', File.dirname(__FILE__)) - end - private def write_inventory @@ -100,29 +88,9 @@ def initialize_working_dir(working_dir) end def initialize_ansible_dir(ansible_dir) - if !ansible_dir.nil? && File.exist?(ansible_dir) - @ansible_dir = ansible_dir - else - raise "Ansible dir #{ansible_dir} does not exist" - end + raise "Ansible dir #{ansible_dir} does not exist" unless + !ansible_dir.nil? && File.exist?(ansible_dir) + @ansible_dir = ansible_dir end - - def setup_verbosity - verbosity_level = @options[:verbosity_level].to_i - logger.debug('[foreman_ansible] - Setting Ansible verbosity level to'\ - "#{verbosity_level}") - verbosity = '-' - verbosity_level.times do - verbosity += 'v' - end - verbosity - end - - def verbose? - verbosity_level = @options[:verbosity_level] - !verbosity_level.nil? && !verbosity_level.empty? && - verbosity_level.to_i > 0 - end - end end From 3abf70de0dc982b6784aa02090e7f947e8e94590 Mon Sep 17 00:00:00 2001 From: Daniel Lobato Garcia Date: Wed, 26 Jul 2017 20:58:17 +0000 Subject: [PATCH 2/4] More rubocop fixes --- .rubocop.yml | 4 +++- test/functional/api/v2/ansible_roles_controller_test.rb | 1 + test/functional/api/v2/hostgroups_controller_test.rb | 1 + test/functional/api/v2/hosts_controller_test.rb | 1 + test/functional/hosts_controller_test.rb | 1 + test/support/fixture_support.rb | 6 +++++- test/support/foreman_tasks/task.rb | 1 + test/unit/lib/foreman_ansible_core/playbook_runner_test.rb | 2 ++ test/unit/lib/foreman_ansible_core/roles_reader_test.rb | 2 ++ test/unit/services/proxy_selector_test.rb | 1 + 10 files changed, 18 insertions(+), 2 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index bcbbd75a..cd3c95f3 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -2,6 +2,8 @@ Rails: Enabled: true AllCops: + TargetRubyVersion: 2.0 + TargetRailsVersion: 4.2 Exclude: - 'extras/**/*' - 'locale/**/*' @@ -11,7 +13,7 @@ AllCops: Style/HashSyntax: EnforcedStyle: hash_rockets -Style/DotPosition: +Layout/DotPosition: EnforcedStyle: 'trailing' Rails/ActionFilter: diff --git a/test/functional/api/v2/ansible_roles_controller_test.rb b/test/functional/api/v2/ansible_roles_controller_test.rb index e7b1dc98..794f50ca 100644 --- a/test/functional/api/v2/ansible_roles_controller_test.rb +++ b/test/functional/api/v2/ansible_roles_controller_test.rb @@ -2,6 +2,7 @@ module Api module V2 + # Tests for the controller to CRUD Ansible Roles class AnsibleRolesControllerTest < ActionController::TestCase setup do @role = FactoryGirl.create(:ansible_role) diff --git a/test/functional/api/v2/hostgroups_controller_test.rb b/test/functional/api/v2/hostgroups_controller_test.rb index b438311f..86dd43c9 100644 --- a/test/functional/api/v2/hostgroups_controller_test.rb +++ b/test/functional/api/v2/hostgroups_controller_test.rb @@ -3,6 +3,7 @@ module Api module V2 + # Tests for the extra methods to play roles on Hostgroup class HostgroupsControllerTest < ActionController::TestCase include ::Dynflow::Testing diff --git a/test/functional/api/v2/hosts_controller_test.rb b/test/functional/api/v2/hosts_controller_test.rb index 481f15e0..254fd067 100644 --- a/test/functional/api/v2/hosts_controller_test.rb +++ b/test/functional/api/v2/hosts_controller_test.rb @@ -2,6 +2,7 @@ module Api module V2 + # Tests for the extra methods to play roles on a Host class HostsControllerTest < ActionController::TestCase include ::Dynflow::Testing diff --git a/test/functional/hosts_controller_test.rb b/test/functional/hosts_controller_test.rb index f23c4a58..28a6875c 100644 --- a/test/functional/hosts_controller_test.rb +++ b/test/functional/hosts_controller_test.rb @@ -9,6 +9,7 @@ class HostsControllerExtensionsTest < ActionController::TestCase tests ::HostsController + # rubocop:disable Metrics/BlockLength context 'role assignment' do setup do @role = FactoryGirl.create(:ansible_role) diff --git a/test/support/fixture_support.rb b/test/support/fixture_support.rb index 9de83ad0..6f376462 100644 --- a/test/support/fixture_support.rb +++ b/test/support/fixture_support.rb @@ -1,11 +1,15 @@ module ForemanAnsible + # Allow to add fixtures to plugins module PluginFixtures FIXTURE_MAPPING = { :ansible_permissions => :permissions }.freeze def self.add_fixtures(new_fixture_path) - FileUtils.cp(Dir.glob("#{Rails.root}/test/fixtures/*"), new_fixture_path) + FileUtils.cp( + Dir.glob(Rails.root.join('test', 'fixtures', '*')), + new_fixture_path + ) copy_plugin_fixtures new_fixture_path end diff --git a/test/support/foreman_tasks/task.rb b/test/support/foreman_tasks/task.rb index f6500ea3..34f204cb 100644 --- a/test/support/foreman_tasks/task.rb +++ b/test/support/foreman_tasks/task.rb @@ -1,5 +1,6 @@ module Support module ForemanTasks + # Stubbing for foreman tasks module Task def stub_tasks! @controller.stubs(:sync_task).returns(build_task_stub) diff --git a/test/unit/lib/foreman_ansible_core/playbook_runner_test.rb b/test/unit/lib/foreman_ansible_core/playbook_runner_test.rb index ef6c440b..bbed2456 100644 --- a/test/unit/lib/foreman_ansible_core/playbook_runner_test.rb +++ b/test/unit/lib/foreman_ansible_core/playbook_runner_test.rb @@ -1,5 +1,7 @@ require 'test_helper' +# Playbook Runner - this class uses foreman_tasks_core +# to run playbooks class PlaybookRunnerTest < ActiveSupport::TestCase context 'roles dir' do test 'reads default when none provided' do diff --git a/test/unit/lib/foreman_ansible_core/roles_reader_test.rb b/test/unit/lib/foreman_ansible_core/roles_reader_test.rb index 0aff1ac8..ffb0b1be 100644 --- a/test/unit/lib/foreman_ansible_core/roles_reader_test.rb +++ b/test/unit/lib/foreman_ansible_core/roles_reader_test.rb @@ -1,5 +1,7 @@ require 'test_plugin_helper' +# Tests for the Roles Reader service of ansible core, +# this class simply reads roles from its path in ansible.cfg class RolesReaderTest < ActiveSupport::TestCase CONFIG_PATH = '/etc/ansible/ansible.cfg'.freeze ROLES_PATH = '/etc/ansible/roles'.freeze diff --git a/test/unit/services/proxy_selector_test.rb b/test/unit/services/proxy_selector_test.rb index a0395880..0afa71db 100644 --- a/test/unit/services/proxy_selector_test.rb +++ b/test/unit/services/proxy_selector_test.rb @@ -1,5 +1,6 @@ require 'test_plugin_helper' +# Tests for the Proxy Selector service class ProxySelectorTest < ActiveSupport::TestCase setup do @host = FactoryGirl.create(:host) From 41542e83bf214dc8adbad9af57731337a6c54769 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guido=20G=C3=BCnther?= Date: Thu, 27 Jul 2017 14:53:06 +0200 Subject: [PATCH 3/4] Fixes #20389 - debian: unbreak adding debian/sid MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Don't report a non numeric major version on debian sid since this otherwise fails like 2017-07-23T15:39:32 07623536 [app] [W] Action failed | ActiveRecord::RecordInvalid: Validation failed: Major version is too long (maximum is 5 characters), Major version is not a number | /usr/share/foreman/vendor/ruby/2.3.0/gems/activerecord-4.2.9/lib/active_record/validations.rb:79:in `raise_record_invalid' | /usr/share/foreman/vendor/ruby/2.3.0/gems/activerecord-4.2.9/lib/active_record/validations.rb:43:in `save!' | /usr/share/foreman/vendor/ruby/2.3.0/gems/activerecord-4.2.9/lib/active_record/attribute_methods/dirty.rb:29:in `save!' | /usr/share/foreman/vendor/ruby/2.3.0/gems/activerecord-4.2.9/lib/active_record/transactions.rb:291:in `block in save!' | /usr/share/foreman/vendor/ruby/2.3.0/gems/activerecord-4.2.9/lib/active_record/transactions.rb:351:in `block in with_transaction_returning_status' | /usr/share/foreman/vendor/ruby/2.3.0/gems/activerecord-4.2.9/lib/active_record/connection_adapters/abstract/database_statements.rb:213:in `block in transaction' | /usr/share/foreman/vendor/ruby/2.3.0/gems/activerecord-4.2.9/lib/active_record/connection_adapters/abstract/transaction.rb:184:in `within_new_transaction' | /usr/share/foreman/vendor/ruby/2.3.0/gems/activerecord-4.2.9/lib/active_record/connection_adapters/abstract/database_statements.rb:213:in `transaction' | /usr/share/foreman/vendor/ruby/2.3.0/gems/activerecord-4.2.9/lib/active_record/transactions.rb:220:in `transaction' | /usr/share/foreman/vendor/ruby/2.3.0/gems/activerecord-4.2.9/lib/active_record/transactions.rb:348:in `with_transaction_returning_status' | /usr/share/foreman/vendor/ruby/2.3.0/gems/activerecord-4.2.9/lib/active_record/transactions.rb:291:in `save!' | /usr/share/foreman/vendor/ruby/2.3.0/gems/activerecord-4.2.9/lib/active_record/persistence.rb:51:in `create!' | /usr/share/foreman/vendor/ruby/2.3.0/gems/foreman_ansible-1.4.5/app/services/foreman_ansible/fact_parser.rb:14:in `operatingsystem' … Debian adds the numeric versions after the release to the /etc/os-release: http://metadata.ftp-master.debian.org/changelogs/main/b/base-files/base-files_10_changelog The puppet fact parser uses s.th. similar. --- app/services/foreman_ansible/fact_parser.rb | 24 ++++++++++++++++++--- test/unit/services/fact_parser_test.rb | 19 ++++++++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/app/services/foreman_ansible/fact_parser.rb b/app/services/foreman_ansible/fact_parser.rb index 863d56d1..6610d732 100644 --- a/app/services/foreman_ansible/fact_parser.rb +++ b/app/services/foreman_ansible/fact_parser.rb @@ -79,10 +79,28 @@ def os_name facts[:ansible_lsb] && facts[:ansible_lsb]['id'] end + def debian_os_major_sid + case facts[:ansible_distribution_major_version] + when /wheezy/i + '7' + when /jessie/i + '8' + when /stretch/i + '9' + when /buster/i + '10' + end + end + def os_major - facts[:ansible_distribution_major_version] || - facts[:ansible_lsb] && facts[:ansible_lsb]['major_release'] || - (facts[:version].split('R')[0] if os_name == 'junos') + if os_name == 'Debian' && + facts[:ansible_distribution_major_version][%r{\/sid}i] + debian_os_major_sid + else + facts[:ansible_distribution_major_version] || + facts[:ansible_lsb] && facts[:ansible_lsb]['major_release'] || + (facts[:version].split('R')[0] if os_name == 'junos') + end end def os_release diff --git a/test/unit/services/fact_parser_test.rb b/test/unit/services/fact_parser_test.rb index f71b9fdb..603316f0 100644 --- a/test/unit/services/fact_parser_test.rb +++ b/test/unit/services/fact_parser_test.rb @@ -50,4 +50,23 @@ def expect_where(model, fact_name) sample_mock.expects(:first_or_create) end end + + class DebianFactParserTest < ActiveSupport::TestCase + setup do + @facts_parser = ForemanAnsible::FactParser.new( + {'_type': 'ansible', + '_timestamp': '2015-10-29 20:01:51 +0100', + 'ansible_facts': { + 'ansible_distribution_major_version': 'buster/sid', + 'ansible_distribution': 'Debian' + } + }) + end + + test 'Parses debian unstable aka sid correctly' do + os = @facts_parser.operatingsystem + assert_equal "10", os.major + assert_equal "Debian", os.name + end + end end From 2fa8a2ed02984902d183341510a1317e61890f83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Lobato=20Garc=C3=ADa?= Date: Tue, 1 Aug 2017 12:00:50 +0200 Subject: [PATCH 4/4] Fixes #20464 - Allow arbitrary connection options host param --- .codeclimate.yml | 39 ++++++++++++++++ .rubocop.yml | 2 +- .../foreman_ansible/inventory_creator.rb | 36 +++++---------- test/unit/services/inventory_creator_test.rb | 46 +++++++++++++++++++ 4 files changed, 98 insertions(+), 25 deletions(-) create mode 100644 .codeclimate.yml create mode 100644 test/unit/services/inventory_creator_test.rb diff --git a/.codeclimate.yml b/.codeclimate.yml new file mode 100644 index 00000000..ef8ace04 --- /dev/null +++ b/.codeclimate.yml @@ -0,0 +1,39 @@ +--- +engines: + brakeman: + enabled: true + duplication: + enabled: true + config: + languages: + - ruby + - javascript + - python + - php + fixme: + enabled: true + radon: + enabled: true + rubocop: + enabled: true + checks: + Rubocop/Style/PercentLiteralDelimiters: + enabled: false # it's outdated vs rubocop +ratings: + paths: + - Gemfile.lock + - "**.erb" + - "**.haml" + - "**.rb" + - "**.rhtml" + - "**.slim" + - "**.inc" + - "**.js" + - "**.jsx" + - "**.module" + - "**.php" + - "**.py" +exclude_paths: +- config/ +- db/ +- test/ diff --git a/.rubocop.yml b/.rubocop.yml index cd3c95f3..391c02d1 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -2,7 +2,7 @@ Rails: Enabled: true AllCops: - TargetRubyVersion: 2.0 + TargetRubyVersion: 2.1 TargetRailsVersion: 4.2 Exclude: - 'extras/**/*' diff --git a/app/services/foreman_ansible/inventory_creator.rb b/app/services/foreman_ansible/inventory_creator.rb index fa2e8a15..caaac83d 100644 --- a/app/services/foreman_ansible/inventory_creator.rb +++ b/app/services/foreman_ansible/inventory_creator.rb @@ -35,28 +35,13 @@ def host_vars(host) end def connection_params(host) - params = { - 'ansible_port' => host_port(host), - 'ansible_user' => host_user(host), - 'ansible_ssh_pass' => host_ssh_pass(host), - 'ansible_connection' => connection_type(host), - 'ansible_winrm_server_cert_validation' => winrm_cert_validation(host) - } + params = ansible_settings.merge ansible_extra_options(host) # Backward compatibility for Ansible 1.x params['ansible_ssh_port'] = params['ansible_port'] params['ansible_ssh_user'] = params['ansible_user'] params end - def winrm_cert_validation(host) - host.host_params['ansible_winrm_server_cert_validation'] || - Setting['ansible_winrm_server_cert_validation'] - end - - def connection_type(host) - host.host_params['ansible_connection'] || Setting['ansible_connection'] - end - def host_roles(host) host.all_ansible_roles.map(&:name) end @@ -69,16 +54,19 @@ def host_params(host) host.host_params end - def host_port(host) - host.host_params['ansible_port'] || Setting[:ansible_port] + def ansible_settings + Hash[ + %w[port user ssh_pass connection + winrm_server_cert_validation].map do |setting| + ["ansible_#{setting}", Setting["ansible_#{setting}"]] + end + ] end - def host_user(host) - host.host_params['ansible_user'] || Setting[:ansible_user] - end - - def host_ssh_pass(host) - host.host_params['ansible_ssh_pass'] || Setting[:ansible_ssh_pass] + def ansible_extra_options(host) + host.host_params.select do |key, _| + /ansible_/.match(key) || Setting[key] + end end private diff --git a/test/unit/services/inventory_creator_test.rb b/test/unit/services/inventory_creator_test.rb new file mode 100644 index 00000000..3a3b655e --- /dev/null +++ b/test/unit/services/inventory_creator_test.rb @@ -0,0 +1,46 @@ +require 'test_plugin_helper' + +module ForemanAnsible + # Test how the inventory creator service transforms host params into + # inventory variables and connection options + class InventoryCreatorTest < ActiveSupport::TestCase + setup do + @host = FactoryGirl.build(:host) + end + + test 'ansible_ parameters get turned into host variables' do + extra_options = { + 'ansible_integer_option' => 1, + 'ansible_string_option' => '1', + 'ansible_boolean_option' => true, + 'ansible_user' => 'someone' + } + @host.expects(:host_params).returns(extra_options).at_least_once + inventory = ForemanAnsible::InventoryCreator.new(@host) + + assert_empty extra_options.to_a - inventory.connection_params(@host).to_a + end + + test 'settings are respected if param cannot be found' do + extra_options = { 'ansible_user' => 'someone', 'ansible_port' => 2000 } + Setting.expects(:[]).with('ansible_port').returns(nil).at_least_once + Setting.expects(:[]).with('ansible_user').returns(nil).at_least_once + Setting.expects(:[]).with('ansible_ssh_pass'). + returns('asafepassword').at_least_once + Setting.expects(:[]).with('ansible_winrm_server_cert_validation'). + returns(true).at_least_once + Setting.expects(:[]).with('ansible_connection'). + returns('ssh').at_least_once + @host.expects(:host_params).returns(extra_options).at_least_once + inventory = ForemanAnsible::InventoryCreator.new(@host) + connection_params = inventory.connection_params(@host) + assert_empty extra_options.to_a - inventory.connection_params(@host).to_a + assert_equal Setting['ansible_connection'], + connection_params['ansible_connection'] + assert_equal Setting['ansible_ssh_pass'], + connection_params['ansible_ssh_pass'] + assert_equal Setting['ansible_winrm_server_cert_validation'], + connection_params['ansible_winrm_server_cert_validation'] + end + end +end