diff --git a/app/models/operatingsystem.rb b/app/models/operatingsystem.rb index b22a3619e4d..6766892c880 100644 --- a/app/models/operatingsystem.rb +++ b/app/models/operatingsystem.rb @@ -46,6 +46,18 @@ class Operatingsystem < ApplicationRecord default_scope -> { order(:title) } + # Find all operatingsystems that are duplicates of the given operating system according to all unique constraints + def self.find_by_attributes(name: nil, major: nil, minor: nil, description: nil) + where_attributes = { + name: name, + major: major, + minor: minor, + }.compact + scope = where(where_attributes) + scope = scope.or(where(description: description)) if description.present? + scope.or(where(title: generate_title(**where_attributes.merge(description: description)))) + end + scoped_search :on => :id, :complete_enabled => false, :only_explicit => true, :validator => ScopedSearch::Validators::INTEGER scoped_search :on => :name, :complete_value => :true scoped_search :on => :major, :complete_value => :true @@ -139,10 +151,26 @@ def self.families_as_collection end end + def self.generate_title(description:, name:, major:, minor:) + to_label(description: description, name: name, major: major, minor: minor).to_s[0..254] + end + + def self.to_label(description:, name:, major:, minor:) + return description if description.present? + fullname(name: name, major: major, minor: minor) + end + + def self.fullname(name:, major:, minor:) + "#{name} #{release(major: major, minor: minor)}" + end + + def self.release(major:, minor:) + "#{major}#{('.' + minor.to_s) if minor.present?}" + end + # The OS is usually represented as the concatenation of the OS and the revision def to_label - return description if description.present? - fullname + self.class.to_label(description: description, name: name, major: major, minor: minor) end # to_label setter updates description and does not try to parse and update major, minor attributes @@ -155,11 +183,11 @@ def to_param end def release - "#{major}#{('.' + minor.to_s) if minor.present?}" + self.class.release(major: major, minor: minor) end def fullname - "#{name} #{release}" + self.class.fullname(name: name, major: major, minor: minor) end def to_s @@ -384,7 +412,7 @@ def set_family end def set_title - self.title = to_label.to_s[0..254] + self.title = self.class.generate_title(description: description, name: name, major: major, minor: minor) end def stringify_major_and_minor diff --git a/app/services/foreman_ansible/operating_system_parser.rb b/app/services/foreman_ansible/operating_system_parser.rb index 3b55445b17e..b4d968dcdd3 100644 --- a/app/services/foreman_ansible/operating_system_parser.rb +++ b/app/services/foreman_ansible/operating_system_parser.rb @@ -8,6 +8,7 @@ def operatingsystem args[:release_name] = os_release_name if os_name == 'Debian' || os_name == 'Ubuntu' # for Ansible, the CentOS Stream can be identified by missing minor version only args[:name] = "CentOS_Stream" if os_name == 'CentOS' && os_minor.blank? + args[:description] = os_description return @local_os if local_os(args).present? return @new_os if new_os(args).present? logger.debug do @@ -20,15 +21,19 @@ def operatingsystem end def local_os(args) - @local_os = Operatingsystem.where(args).first + @local_os = Operatingsystem.find_by_attributes(**args.slice(:name, :major, :minor, :description)).first end def new_os(args) return @new_os if @new_os.present? - @new_os = Operatingsystem.new(args.merge(:description => os_description)) + @new_os = generate_new_os(args) @new_os if @new_os.valid? && @new_os.save end + def generate_new_os(args) + Operatingsystem.new(args.merge(:description => os_description)) + end + def debian_os_major_sid release = if facts[:ansible_distribution_major_version] == 'n/a' facts[:ansible_distribution_release] diff --git a/app/services/foreman_chef/fact_parser.rb b/app/services/foreman_chef/fact_parser.rb index d370c86dc03..da87b2844e9 100644 --- a/app/services/foreman_chef/fact_parser.rb +++ b/app/services/foreman_chef/fact_parser.rb @@ -51,7 +51,7 @@ def operatingsystem end args = { :name => os_name, :major => major.to_s, :minor => minor.to_s } - klass.where(args).first || klass.new(args.merge(:release_name => release_name)).tap(&:save!) + Operatingsystem.find_by_attributes(**args).first || klass.new(args.merge(:release_name => release_name)).tap(&:save!) end # we don't want to parse puppet environment, foreman_chef already has its own environment diff --git a/app/services/foreman_salt/fact_parser.rb b/app/services/foreman_salt/fact_parser.rb index 4e7d5b1c165..654ef89a333 100644 --- a/app/services/foreman_salt/fact_parser.rb +++ b/app/services/foreman_salt/fact_parser.rb @@ -3,7 +3,7 @@ class FactParser < ::FactParser attr_reader :facts def operatingsystem - os = Operatingsystem.where(os_hash).first_or_initialize + os = Operatingsystem.find_by_attributes(**os_hash).first || Operatingsystem.new(os_hash) if os.new_record? os.deduce_family os.release_name = facts[:oscodename] || facts[:lsb_distrib_codename] diff --git a/app/services/katello/rhsm_fact_parser.rb b/app/services/katello/rhsm_fact_parser.rb index bb2fe1b1738..d7840814238 100644 --- a/app/services/katello/rhsm_fact_parser.rb +++ b/app/services/katello/rhsm_fact_parser.rb @@ -73,9 +73,9 @@ def operatingsystem os_attributes[:name] = "CentOS" end - os = ::Operatingsystem.find_by(os_attributes) + os = ::Operatingsystem.find_by_attributes(**os_attributes.slice(:name, :major, :minor, :description)).first if os.blank? - created = ::Operatingsystem.create_or_find_by(os_attributes) + created = ::Operatingsystem.create(os_attributes) created.errors.any? ? ::Operatingsystem.find_by(os_attributes) : created else os diff --git a/app/services/puppet_fact_parser.rb b/app/services/puppet_fact_parser.rb index df04a64cae5..0c5afaf7357 100644 --- a/app/services/puppet_fact_parser.rb +++ b/app/services/puppet_fact_parser.rb @@ -4,6 +4,8 @@ class PuppetFactParser < FactParser def operatingsystem orel = os_release.dup + args = { :name => os_name } + if orel.present? if os_name =~ /ubuntu/i major = os_major_version.to_s @@ -13,29 +15,24 @@ def operatingsystem 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' - if distro_codename - os.release_name = distro_codename - elsif os.release_name.blank? - os.release_name = 'unknown' - end + args[:major] = major + args[:minor] = minor + if os_name[/debian|ubuntu/i] + args[:release_name] = distro_codename if distro_codename + args[:release_name] = 'unknown' if args[:release_name].blank? end - else - os = Operatingsystem.find_or_initialize_by(:name => os_name) end - if os.description.blank? - if os_name == 'SLES' - os.description = os_name + ' ' + orel.gsub('.', ' SP') - elsif distro_description - family = os.deduce_family || 'Operatingsystem' - os = os.becomes(family.constantize) - os.description = os.shorten_description(distro_description) - end + if os_name == 'SLES' + args[:description] = "#{os_name} #{orel.gsub('.', ' SP')}" + elsif distro_description + family = Operatingsystem.deduce_family(os_name) || 'Operatingsystem' + os_class = family.constantize + args[:description] = os_class.new(args).shorten_description(distro_description) end + os = Operatingsystem.find_by_attributes(**args.slice(:name, :major, :minor, :description)).first || Operatingsystem.new(args) + if os.new_record? os.save! Operatingsystem.find_by_id(os.id) # complete reload to be an instance of the STI subclass diff --git a/test/static_fixtures/facts/puppet_rhel_9_facter_4.1.json b/test/static_fixtures/facts/puppet_rhel_9_facter_4.1.json new file mode 100644 index 00000000000..597b02c23dc --- /dev/null +++ b/test/static_fixtures/facts/puppet_rhel_9_facter_4.1.json @@ -0,0 +1,247 @@ +{ + "disks": { + "vda": { + "size": "7.00 GiB", + "size_bytes": 7516192768, + "type": "hdd", + "vendor": "0x1af4" + } + }, + "dmi": { + "bios": { + "release_date": "04/01/2014", + "vendor": "SeaBIOS", + "version": "1.14.0-1.fc33" + }, + "chassis": { + "type": "Other" + }, + "manufacturer": "QEMU", + "product": { + "name": "Standard PC (Q35 + ICH9, 2009)", + "uuid": "1ddbeb7e-aea3-4ae3-8a57-61867d346372" + } + }, + "facterversion": "4.1.0", + "filesystems": "ext2,ext3,ext4,xfs", + "fips_enabled": false, + "hypervisors": { + "kvm": { + } + }, + "identity": { + "gid": 0, + "group": "root", + "privileged": true, + "uid": 0, + "user": "root" + }, + "is_virtual": true, + "kernel": "Linux", + "kernelmajversion": "4.18", + "kernelrelease": "4.18.0-193.6.3.el8_2.x86_64", + "kernelversion": "4.18.0", + "load_averages": { + "15m": 0.31, + "1m": 0.2, + "5m": 0.36 + }, + "memory": { + "swap": { + "available": "609.23 MiB", + "available_bytes": 638828544, + "capacity": "0.94%", + "total": "615.00 MiB", + "total_bytes": 644870144, + "used": "5.76 MiB", + "used_bytes": 6041600 + }, + "system": { + "available": "1.33 GiB", + "available_bytes": 1428824064, + "capacity": "25.41%", + "total": "1.78 GiB", + "total_bytes": 1915502592, + "used": "464.13 MiB", + "used_bytes": 486678528 + } + }, + "networking": { + "fqdn": "stream", + "hostname": "stream", + "interfaces": { + "enp1s0": { + "bindings": [ + { + "address": "192.168.122.29", + "netmask": "255.255.255.0", + "network": "192.168.122.0" + } + ], + "bindings6": [ + { + "address": "fe80::5054:ff:fe85:821", + "netmask": "ffff:ffff:ffff:ffff::", + "network": "fe80::", + "scope6": "link" + } + ], + "ip": "192.168.122.29", + "ip6": "fe80::5054:ff:fe85:821", + "mac": "52:54:00:85:08:21", + "mtu": 1500, + "netmask": "255.255.255.0", + "netmask6": "ffff:ffff:ffff:ffff::", + "network": "192.168.122.0", + "network6": "fe80::", + "scope6": "link" + }, + "lo": { + "bindings": [ + { + "address": "127.0.0.1", + "netmask": "255.0.0.0", + "network": "127.0.0.0" + } + ], + "bindings6": [ + { + "address": "::1", + "netmask": "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff", + "network": "::1", + "scope6": "host" + } + ], + "ip": "127.0.0.1", + "ip6": "::1", + "mtu": 65536, + "netmask": "255.0.0.0", + "netmask6": "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff", + "network": "127.0.0.0", + "network6": "::1", + "scope6": "host" + } + }, + "ip": "192.168.122.29", + "ip6": "fe80::5054:ff:fe85:821", + "mac": "52:54:00:85:08:21", + "mtu": 1500, + "netmask": "255.255.255.0", + "netmask6": "ffff:ffff:ffff:ffff::", + "network": "192.168.122.0", + "network6": "fe80::", + "primary": "enp1s0", + "scope6": "link" + }, + "os": { + "architecture": "x86_64", + "distro": { + "codename": "Plow", + "description": "Red Hat Enterprise Linux release 9.6 (Plow)", + "id": "RedHatEnterprise", + "release": { + "full": "9.6", + "major": "9", + "minor": "6" + } + }, + "family": "RedHat", + "hardware": "x86_64", + "name": "RedHat", + "release": { + "full": "9.6", + "major": "9", + "minor": "6" + }, + "selinux": { + "config_mode": "enforcing", + "config_policy": "targeted", + "current_mode": "enforcing", + "enabled": true, + "enforced": true, + "policy_version": "33" + } + }, + "partitions": { + "/dev/vda1": { + "partlabel": "primary", + "partuuid": "d4ceee7d-f2a8-41cc-9a1f-941b15f6d988", + "size": "1.00 MiB", + "size_bytes": 1048576 + }, + "/dev/vda2": { + "filesystem": "ext4", + "partlabel": "primary", + "partuuid": "268627c0-d337-45e0-8b78-3b4516743e37", + "size": "1.00 GiB", + "size_bytes": 1073741824, + "uuid": "0b3df967-65cc-4a48-8b98-f7f0badc0d91" + }, + "/dev/vda3": { + "filesystem": "swap", + "partlabel": "primary", + "partuuid": "c30f23cb-3504-4020-81b6-1da9d9413a5b", + "size": "615.00 MiB", + "size_bytes": 644874240, + "uuid": "40f14688-2619-4046-a9eb-b7333fff1b84" + }, + "/dev/vda4": { + "filesystem": "xfs", + "partlabel": "primary", + "partuuid": "81a2926c-2b51-4bc8-8ba9-f0af2a91fe85", + "size": "5.40 GiB", + "size_bytes": 5794103296, + "uuid": "4fd120e4-1f6d-46b3-a404-5569ef6af1f9" + } + }, + "path": "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/root/bin", + "processors": { + "count": 2, + "isa": "x86_64", + "models": [ + "Intel Core Processor (Skylake, IBRS)", + "Intel Core Processor (Skylake, IBRS)" + ], + "physicalcount": 2, + "speed": "3.00 GHz" + }, + "ruby": { + "platform": "x86_64-linux", + "sitedir": "/usr/local/share/ruby/site_ruby", + "version": "2.7.1" + }, + "ssh": { + "ecdsa": { + "fingerprints": { + "sha1": "SSHFP 3 1 1896eda9c97def529c64f97ebcdb5aee32e25285", + "sha256": "SSHFP 3 2 6932a390ad20961350b2ab36861b18ec2ce2644d433bc54f432600421f58bcd1" + }, + "key": "AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBEBFrxBiebSB+0M1LGyX3H3ZHd676wnGGfYjexjSvopznvfkb0PdNSLUvHpc1gU/yPBx7HP/pdGvdDWnwvoNkLY=", + "type": "ecdsa-sha2-nistp256" + }, + "ed25519": { + "fingerprints": { + "sha1": "SSHFP 4 1 43113e7fca870374e8a5f9981e4bf6f97a7bc59a", + "sha256": "SSHFP 4 2 d17bfb375bc87e76dcc937708974cd3f7a361843625f67c485426ad4c9bf441f" + }, + "key": "AAAAC3NzaC1lZDI1NTE5AAAAIFu6kGHnNmCA56gkfRAyAxZZEdwF5mCD6aRb0PznE++s", + "type": "ssh-ed25519" + }, + "rsa": { + "fingerprints": { + "sha1": "SSHFP 1 1 0f2ab3941f9c7a54211076bc27dee370e10698b0", + "sha256": "SSHFP 1 2 c45674d9bc1091ac4cc9150fd517b0b895650fa590e41e288cc07f2699dfb9e1" + }, + "key": "AAAAB3NzaC1yc2EAAAADAQABAAABgQDSOlxRR5iy0VM5BVl2JiFvUvv5UMAUMPkNxKuo89U/jG1X3N9pqxSpgwy4VnzAJTMXTUObCnSTueiVjj0HHz6UMuRxIe4MYb2XFNrdQUV3BPKvbP3NG02bBGRhH35mr0g9Yd9Rc1Ayf5gTlfGkbOh4JbjPygQ9ub/5gUnEbFOS4x39U3wnZV4z/1s0RwWtzdLe8kj36/gvoYaBdrsDNtFQjEGWSHhjiwXf0wOvOVt+OKY77Z1+pM6CcqQVR75VcT52P/y5UEIREo7CP0eHxjD5JAcm7fWJrxeHFH3Cv1dh9I/J9b5k3WGFciqsv5zebx4MMq64poKbmyFyR+R1ixKM9laoaUiTC5N3ajM54kSb5wREIiMMFpPo7ArfXnIbjOsHidnmXvMfOwTLwwymBoKsWNR2LIf3u/mBOdBC2PtrYz9nL6pi5Lb2OOpa0S+LQFxWulFPWvTN83Mqlcr8qScJqPfgwlM6pIco6blJwe078N5ldXdeaj54I0RQ4XYChhE=", + "type": "ssh-rsa" + } + }, + "system_uptime": { + "days": 0, + "hours": 0, + "seconds": 679, + "uptime": "0:11 hours" + }, + "timezone": "EDT", + "virtual": "kvm" +} diff --git a/test/unit/foreman_ansible/fact_parser_test.rb b/test/unit/foreman_ansible/fact_parser_test.rb index 6d90064f901..d3ef1eca284 100644 --- a/test/unit/foreman_ansible/fact_parser_test.rb +++ b/test/unit/foreman_ansible/fact_parser_test.rb @@ -47,15 +47,13 @@ class FactParserTest < ActiveSupport::TestCase end test 'creates operatingsystem from operating system options' do - sample_mock = mock major_fact = @facts_parser.facts['ansible_distribution_major_version'] _, minor_fact = @facts_parser. facts['ansible_distribution_version'].split('.') - Operatingsystem.expects(:where). - with({ :name => @facts_parser.facts['ansible_distribution'], - :major => major_fact, :minor => minor_fact || '' }). - returns(sample_mock) - sample_mock.expects(:first) + description = @facts_parser.facts[:ansible_lsb]['description'] + Operatingsystem.expects(:find_by_attributes) + .with(name: @facts_parser.facts['ansible_distribution'], major: major_fact, minor: minor_fact.to_s, description: description) + .returns([]) @facts_parser.operatingsystem end @@ -68,6 +66,22 @@ class FactParserTest < ActiveSupport::TestCase assert_nil @facts_parser.operatingsystem end + test 'should pick up an OS by description' do + # Create OS with the same description as the one in the facts, but different name + expected = FactoryBot.create(:operatingsystem, name: 'FedFed', major: '22', description: 'Fedora release 22 (Twenty Two)') + os = @facts_parser.operatingsystem + assert_equal expected.id, os.id + end + + test 'should pick up an OS by title' do + # Create OS with the same description as the one in the facts, but different name + expected = FactoryBot.create(:operatingsystem, name: 'Fedora', major: '22', description: 'Fedora release 22 (Twenty Two)') + # now we update the description in the DB to make sure we're testing the title comparison + Operatingsystem.where(id: expected.id).update_all(description: 'ZZZ') + os = @facts_parser.operatingsystem + assert_equal expected.id, os.id + end + test 'RHEL 7 OS is correctly mapped' do rhel7_facts = HashWithIndifferentAccess.new(read_json_fixture('facts/ansible_rhel_7_server.json')) fact_parser = AnsibleFactParser.new(rhel7_facts) diff --git a/test/unit/katello/rhsm_fact_parser_test.rb b/test/unit/katello/rhsm_fact_parser_test.rb index a5cb78e1117..ba3dba1e659 100644 --- a/test/unit/katello/rhsm_fact_parser_test.rb +++ b/test/unit/katello/rhsm_fact_parser_test.rb @@ -226,7 +226,8 @@ def test_uname_architecture def test_operatingsystem_race_condition_handling existing_os = ::Operatingsystem.create(name: 'RedHat', major: '9', minor: '') - ::Operatingsystem.expects(:find_by).twice.returns(nil) + ::Operatingsystem.expects(:find_by).returns(nil) + ::Operatingsystem.expects(:find_by_attributes).returns([]) @facts['distribution.name'] = 'Red Hat Enterprise Linux' @facts['distribution.version'] = '9' @facts['distribution.id'] = 'Nine' diff --git a/test/unit/puppet_fact_parser_test.rb b/test/unit/puppet_fact_parser_test.rb index 03cb2715912..5a919a3c2a5 100644 --- a/test/unit/puppet_fact_parser_test.rb +++ b/test/unit/puppet_fact_parser_test.rb @@ -236,6 +236,24 @@ def setup assert_equal '8', os.major assert_equal '3.2011', os.minor end + + test 'should pick up an OS by description' do + # Create OS with the same description as the one in the facts, but different name + expected = FactoryBot.create(:operatingsystem, name: 'RHEL', major: '9', minor: '6', description: 'RHEL 9.6') + parser = PuppetFactParser.new(rhel_9_6_facts) + os = parser.operatingsystem + assert_equal expected.id, os.id + end + + test 'should pick up an OS by title' do + # Create OS with the same description as the one in the facts, but different name + expected = FactoryBot.create(:operatingsystem, name: 'RHEL', major: '9', minor: '6', description: 'RHEL 9.6') + # now we update the description in the DB to make sure we're testing the title comparison + Operatingsystem.where(id: expected.id).update_all(description: 'ZZZ 9.6') + parser = PuppetFactParser.new(rhel_9_6_facts) + os = parser.operatingsystem + assert_equal expected.id, os.id + end end describe "#facterversion" do @@ -894,6 +912,10 @@ def centos_8_facts_facter_4 read_json_fixture('facts/puppet_centos_8_facter_4.1.json') end + def rhel_9_6_facts + read_json_fixture('facts/puppet_rhel_9_facter_4.1.json') + end + def debian_facts read_json_fixture('facts/facts_debian.json')['facts'] end