From e63401ce5764413399837f267a577da7e9317b45 Mon Sep 17 00:00:00 2001
From: Tomer Brisker <tbrisker@gmail.com>
Date: Tue, 13 Sep 2016 11:04:43 +0300
Subject: [PATCH] Fixes #14545 - Correctly parse y.z minor OS version from
 facts

Some OSes use y.z minor version, for example CentOS uses versions such
as 7.2.1511. Currently, the puppet fact parser will only take the 'y'
part of the version, 2 in this case, which can lead to conflicts if the
OS has been defined manually with the 2.1511 minor version (as the
description will still contain the entire version).
---
 app/services/fact_parser.rb          |  6 ------
 app/services/puppet_fact_parser.rb   | 14 +++++++-------
 test/unit/puppet_fact_parser_test.rb | 11 +++++++++++
 3 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/app/services/fact_parser.rb b/app/services/fact_parser.rb
index df5955d7e415..dd54863b1ba4 100644
--- a/app/services/fact_parser.rb
+++ b/app/services/fact_parser.rb
@@ -179,10 +179,4 @@ def normalize_interfaces(interfaces)
   def not_implemented_error(method)
     "#{method} fact parsing not implemented in #{self.class}"
   end
-
-  def is_numeric?(string)
-    !!Integer(string)
-  rescue ArgumentError, TypeError
-    false
-  end
 end
diff --git a/app/services/puppet_fact_parser.rb b/app/services/puppet_fact_parser.rb
index d6d24281aeb4..db4e26f068c1 100644
--- a/app/services/puppet_fact_parser.rb
+++ b/app/services/puppet_fact_parser.rb
@@ -7,7 +7,7 @@ def operatingsystem
     if os_name == "Archlinux"
       # Archlinux is rolling release, so it has no release. We use 1.0 always
       args = {:name => os_name, :major => "1", :minor => "0"}
-      os = Operatingsystem.where(args).first || Operatingsystem.new(args)
+      os = Operatingsystem.find_or_initialize_by(args)
     elsif orel.present?
       if os_name == "Debian" && orel[/testing|unstable/i]
         case facts[:lsbdistcodename]
@@ -27,14 +27,14 @@ def operatingsystem
       elsif os_name[/FreeBSD/i]
         orel.gsub!(/\-RELEASE\-p[0-9]+/, '')
       end
-      major, minor = orel.split(".")
-      major.to_s.gsub!(/\D/, '') unless is_numeric? major
-      minor.to_s.gsub!(/\D/, '') unless is_numeric? minor
-      args = {:name => os_name, :major => major.to_s, :minor => minor.to_s}
-      os = Operatingsystem.where(args).first || Operatingsystem.create!(args)
+      major, minor = orel.split('.', 2)
+      major.to_s.gsub!(/\D/, '')
+      minor.to_s.gsub!(/[^\d\.]/, '')
+      args = {:name => os_name, :major => major, :minor => minor}
+      os = Operatingsystem.find_or_initialize_by(args)
       os.release_name = facts[:lsbdistcodename] if facts[:lsbdistcodename] && (os_name[/debian|ubuntu/i] || os.family == 'Debian')
     else
-      os = Operatingsystem.find_by_name(os_name) || Operatingsystem.create!(:name => os_name)
+      os = Operatingsystem.find_or_initialize_by(:name => os_name)
     end
     if os.description.blank?
       if os_name == 'SLES'
diff --git a/test/unit/puppet_fact_parser_test.rb b/test/unit/puppet_fact_parser_test.rb
index 4e4fd0e08070..75728db91347 100644
--- a/test/unit/puppet_fact_parser_test.rb
+++ b/test/unit/puppet_fact_parser_test.rb
@@ -99,6 +99,17 @@ def setup
     refute @importer.operatingsystem.description
   end
 
+  test 'should accept y.z minor version' do
+    FactoryGirl.create(:operatingsystem, name: "CentOS",
+                                         major: "7",
+                                         minor: "2.1511",
+                                         description: "CentOS Linux 7.2.1511")
+    assert_valid PuppetFactParser.new({ "operatingsystem" => "CentOS",
+                                        "lsbdistdescription" => "CentOS Linux release 7.2.1511 (Core) ",
+                                        "operatingsystemrelease" => "7.2.1511"
+    }).operatingsystem
+  end
+
   test "should set os.major and minor correctly from AIX facts" do
     @importer = PuppetFactParser.new(aix_facts)
     assert_equal 'AIX', @importer.operatingsystem.family