Skip to content

Commit

Permalink
Fixes #14545 - Correctly parse y.z minor OS version from facts
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
tbrisker committed Sep 13, 2016
1 parent 659dc6f commit c1504ca
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 13 deletions.
6 changes: 0 additions & 6 deletions app/services/fact_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
14 changes: 7 additions & 7 deletions app/services/puppet_fact_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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'
Expand Down
12 changes: 12 additions & 0 deletions test/unit/puppet_fact_parser_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,18 @@ 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
Expand Down

0 comments on commit c1504ca

Please sign in to comment.