Fixes #36547 - Fix parsing of Ubuntu version in fact parsers

app/models/operatingsystem.rb

@@ -169,14 +169,26 @@ def to_s
   def self.find_by_to_label(str)
     os = find_by_description(str.to_s)
     return os if os
-    a = str.split(" ")
-    b = a[1].split('.') if a[1]
-    cond = {:name => a[0]}
-    cond[:major] = b[0] if b && b[0]
-    cond[:minor] = b[1] if b && b[1]
+    name, version = str.split(" ")
+    cond = {:name => name}
+    if version
+      (major, minor) = os_major_minor_from_version_str(name, version)
+      cond[:major] = major if major
+      cond[:minor] = minor if minor
+    end
     find_by(cond)
   end
 
+  def self.os_major_minor_from_version_str(os_name, version_str)
+    if os_name == 'Ubuntu'
+      x, y, minor = version_str.split('.', 3)
+      major = "#{x}.#{y}"
+    else
+      major, minor = version_str.split('.')
+    end
+    [major, minor]
+  end
+
   # Implemented only in the OSs subclasses where it makes sense
   def available_loaders
     ["None", "PXELinux BIOS"]

app/services/foreman_ansible/operating_system_parser.rb

@@ -68,8 +68,12 @@ def os_release
     end
 
     def os_minor
-      _, minor = os_release&.split('.', 2) ||
+      if os_name == 'Ubuntu'
+        _, _, minor = os_release&.split('.', 3)
+      else
+        _, minor = os_release&.split('.', 2) ||
           (facts[:version].split('R') if os_name == 'junos')
+      end
       # Until Foreman supports os.minor as something that's not a number,
       # we should remove the extra dots in the version. E.g:
       # '6.1.7601.65536' becomes '6.1.760165536'

app/services/foreman_chef/fact_parser.rb

@@ -9,6 +9,8 @@ def operatingsystem
       # if we have no release information we can't assign OS properly (e.g. missing redhat-lsb)
       if release.nil?
         major, minor = 1, nil
+      elsif os_name == 'Ubuntu'
+        major, minor = release, nil
       else
         major, minor = release.split('.')
       end

app/services/katello/rhsm_fact_parser.rb

@@ -52,7 +52,7 @@ def operatingsystem
       return nil if name.nil? || version.nil?
 
       os_name = distribution_to_puppet_os(name)
-      major, minor = version.split('.')
+      (major, minor) = ::Operatingsystem.os_major_minor_from_version_str(os_name, version)
       unless facts['ignore_os']
         os_attributes = {:major => major, :minor => minor || '', :name => os_name}
 

app/services/puppet_fact_parser.rb

@@ -5,9 +5,14 @@ def operatingsystem
     orel = os_release.dup
 
     if orel.present?
-      major, minor = orel.split('.', 2)
-      major = major.to_s.gsub(/\D/, '')
-      minor = minor.to_s.gsub(/[^\d\.]/, '')
+      if os_name =~ /ubuntu/i
+        major = os_major_version
+        minor = os_minor_version
+      else
+        major, minor = orel.split('.', 2)
+        major = major.to_s.gsub(/\D/, '')
+        minor = minor.to_s.gsub(/[^\d\.]/, '')
+      end
       args = {:name => os_name, :major => major, :minor => minor}
       os = Operatingsystem.find_or_initialize_by(args)
       if os_name[/debian|ubuntu/i] || os.family == 'Debian'
@@ -227,6 +232,14 @@ def os_name
     raise(::Foreman::Exception.new("invalid facts, missing operating system value"))
   end
 
+  def os_major_version
+    facts.dig(:os, :release, :major)
+  end
+
+  def os_minor_version
+    facts.dig(:os, :release, :minor)
+  end
+
   def os_release
     case os_name
     when /(windows)/i

db/migrate/20230719080900_fix_ubuntu_versions.rb

@@ -0,0 +1,19 @@
+class FixUbuntuVersions < ActiveRecord::Migration[6.0]
+  def up
+    # For every Ubuntu operating system with a major version and a minor version in the format "<digits>(.<digits>)",
+    #   * We check if there is already an operating system present with a major version of
+    #     "<major>.<first_place_of_minor>" and a minor version of "<second_place_of_minor_or_empty>".
+    #   * If not, we update the major version of the operating system to "<major>.<first_place_of_minor>" and the minor
+    #     version to "<second_place_of_minor_or_empty>".
+    Operatingsystem.where(name: 'Ubuntu')
+      .where('operatingsystems.major ~* ?', '^\d*$')
+      .where('operatingsystems.minor ~* ?', '^\d\d(\.\d)?$')
+      .where('o2.id IS NULL')
+      .joins("left outer join operatingsystems o2 " \
+              "on o2.name = operatingsystems.name " \
+              "and o2.major = concat(operatingsystems.major, '.', split_part(operatingsystems.minor, '.', 1)) " \
+              "and o2.minor = split_part(operatingsystems.minor, '.', 2)")
+      .update_all("major = concat(operatingsystems.major, '.', split_part(operatingsystems.minor, '.', 1)), " \
+                  "minor = split_part(operatingsystems.minor, '.', 2)")
+  end
+end

test/factories/operatingsystem.rb

@@ -108,6 +108,24 @@
       title { 'Ubuntu Utopic' }
     end
 
+    factory :ubuntu22_04, class: Debian do
+      sequence(:name) { 'Ubuntu' }
+      major { '22.04' }
+      minor { '' }
+      type { 'Debian' }
+      release_name { 'jammy' }
+      title { 'Ubuntu Jammy' }
+    end
+
+    factory :ubuntu22_04_3, class: Debian do
+      sequence(:name) { 'Ubuntu' }
+      major { '22.04' }
+      minor { '3' }
+      type { 'Debian' }
+      release_name { 'jammy' }
+      title { 'Ubuntu Jammy' }
+    end
+
     factory :debian7_0, class: Debian do
       sequence(:name) { 'Debian' }
       major { '7' }

test/fixtures/operatingsystems.yml

@@ -20,8 +20,7 @@ redhat:
 
 ubuntu1010:
   name: Ubuntu
-  major: 10
-  minor: 10
+  major: '10.10'
   release_name: rn10
   type: Debian
   media: ubuntu

test/models/operatingsystem_test.rb

@@ -56,6 +56,20 @@ class OperatingsystemTest < ActiveSupport::TestCase
     assert operating_system.to_s == operating_system.to_label
   end
 
+  test "should find Ubuntu 22.04 by fullname string" do
+    FactoryBot.create(:ubuntu22_04)
+    str = "Ubuntu 22.04"
+    os = Operatingsystem.find_by_to_label(str)
+    assert_equal str, os.fullname
+  end
+
+  test "should find Ubuntu 22.04.3 by fullname string" do
+    FactoryBot.create(:ubuntu22_04_3)
+    str = "Ubuntu 22.04.3"
+    os = Operatingsystem.find_by_to_label(str)
+    assert_equal str, os.fullname
+  end
+
   test "should find by fullname string" do
     str = "Redhat 6.1"
     os = Operatingsystem.find_by_to_label(str)

test/unit/katello/rhsm_fact_parser_test.rb

@@ -127,14 +127,26 @@ def test_operatingsystem_debian
 
     def test_operatingsystem_ubuntu
       @facts['distribution.name'] = 'Ubuntu GNU/Linux'
-      @facts['distribution.version'] = '19.04'
-      @facts['distribution.id'] = 'Disco Dingo'
+      @facts['distribution.version'] = '22.04'
+      @facts['distribution.id'] = 'Jammy Jellyfish'
 
-      assert_equal parser.operatingsystem.release_name, 'disco'
+      assert_equal parser.operatingsystem.release_name, 'jammy'
       assert_equal parser.operatingsystem.name, 'Ubuntu'
       assert_equal parser.operatingsystem.type, 'Debian'
-      assert_equal parser.operatingsystem.major, '19'
-      assert_equal parser.operatingsystem.minor, '04'
+      assert_equal parser.operatingsystem.major, '22.04'
+      assert_equal parser.operatingsystem.minor, ''
+    end
+
+    def test_operatingsystem_ubuntu_with_minor
+      @facts['distribution.name'] = 'Ubuntu GNU/Linux'
+      @facts['distribution.version'] = '22.04.3'
+      @facts['distribution.id'] = 'Jammy Jellyfish'
+
+      assert_equal parser.operatingsystem.release_name, 'jammy'
+      assert_equal parser.operatingsystem.name, 'Ubuntu'
+      assert_equal parser.operatingsystem.type, 'Debian'
+      assert_equal parser.operatingsystem.major, '22.04'
+      assert_equal parser.operatingsystem.minor, '3'
     end
 
     def test_operatingsystem_release

test/unit/puppet_fact_parser_test.rb

@@ -772,6 +772,23 @@ def setup
       end
     end
 
+    describe 'Ubuntu 18.04' do
+      let(:os_name) { 'Ubuntu' }
+      let(:os_major) { '18.04' }
+
+      ['2.5', '4.1'].each do |facterversion|
+        describe "Facter #{facterversion}" do
+          let(:facterversion) { facterversion }
+
+          test "should correctly identify Ubuntu 18.04" do
+            assert_equal 'Ubuntu', subject.send(:os_name)
+            assert_equal '18.04', subject.send(:os_major_version)
+            assert_nil subject.send(:os_minor_version)
+          end
+        end
+      end
+    end
+
     describe 'Windows 2012' do
       let(:os_name) { 'windows' }
       let(:os_major) { '2012' }