From 45786569a67bbc2848f769cd02d7ada8159441a9 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, #13104 - Correctly parse y.z minor OS versions

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 df5955d7e41..dd54863b1ba 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 d6d24281aeb..db4e26f068c 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 4e4fd0e0807..75728db9134 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