Fix use of legacy facts (#302)

silug · web-flow · commit 09c9644cadf6 · 2024-12-31T12:38:36.000-05:00
* Fix use of legacy facts Fixes #301 * Add missing tests for uid_min fact
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,3 +1,6 @@
+* Mon Dec 30 2024 Steven Pritchard <steve@sicura.us> - 4.14.0
+- Fix use of legacy facts (#301)
+
 * Mon Dec 23 2024 Steven Pritchard <steve@sicura.us> - 4.13.0
 - Refactor and cleanup for rubocop
 
diff --git a/lib/facter/uid_min.rb b/lib/facter/uid_min.rb
@@ -6,23 +6,12 @@
   confine { File.exist?('/etc/login.defs') }
 
   setcode do
-    uid_min = File.open('/etc/login.defs').grep(%r{UID_MIN})
+    uid_min = File.read('/etc/login.defs')
+                  .lines(chomp: true)
+                  .find { |line| line.split.first == 'UID_MIN' }
+                  .to_s.split.last
 
-    # Grep returns an Array
-    uid_min = '' if uid_min.empty?
-
-    unless uid_min.empty?
-      uid_min = uid_min.first.to_s.chomp.split.last
-    end
-
-    unless uid_min.empty?
-      uid_min = if ['RedHat', 'CentOS', 'OracleLinux', 'Scientific'].include?(Facter.value(:operatingsystem)) &&
-                   Facter.value(:operatingsystemmajrelease) < '7'
-                  '500'
-                else
-                  '1000'
-                end
-    end
+    uid_min = '1000' if uid_min.nil? || uid_min.empty?
 
     uid_min
   end
diff --git a/lib/puppet/functions/simplib/filtered.rb b/lib/puppet/functions/simplib/filtered.rb
@@ -13,14 +13,14 @@
 #     data_hash: "yaml_data"  # Use the built-in YAML backend.
 #   hierarchy: # Each hierarchy consists of multiple levels
 #     - name: "OSFamily"
-#       path: "osfamily/%{facts.osfamily}.yaml"
+#       path: "osfamily/%{facts.os.family}.yaml"
 #     - name: "datamodules"
 #       data_hash: simplib::filtered
 #       datadir: "delegated-data"
 #       paths:
-#         - "%{facts.sitename}/osfamily/%{facts.osfamily}.yaml"
-#         - "%{facts.sitename}/os/%{facts.operatingsystem}.yaml"
-#         - "%{facts.sitename}/host/%{facts.fqdn}.yaml"
+#         - "%{facts.sitename}/osfamily/%{facts.os.family}.yaml"
+#         - "%{facts.sitename}/os/%{facts.os.name}.yaml"
+#         - "%{facts.sitename}/host/%{facts.networking.fqdn}.yaml"
 #         - "%{facts.sitename}/common.yaml"
 #       options:
 #         function: yaml_data
diff --git a/lib/puppet/functions/simplib/host_is_me.rb b/lib/puppet/functions/simplib/host_is_me.rb
@@ -28,8 +28,8 @@ def hostlist_contains_me(hosts)
     scope = closure_scope
 
     host_identifiers = [
-      scope['facts']['fqdn'],
-      scope['facts']['hostname'],
+      scope['facts']['networking']['fqdn'],
+      scope['facts']['networking']['hostname'],
       'localhost',
       'localhost.localdomain',
     ]
diff --git a/lib/puppet/functions/simplib/ip_to_cron.rb b/lib/puppet/functions/simplib/ip_to_cron.rb
@@ -29,7 +29,7 @@
   #
   # @param ip
   #   The IP address to use as the basis for the generated values.
-  #   When `nil`, the 'ipaddress' fact (IPv4) is used.
+  #   When `nil`, the 'networking.ip' fact (IPv4) is used.
   #
   # @return [Array[Integer]] Array of integers suitable for use in the
   #   ``minute`` or ``hour`` cron field.
@@ -53,7 +53,7 @@
   def ip_to_cron(occurs = 1, max_value = 59, algorithm = 'ip_mod', ip = nil)
     if ip.nil?
       scope = closure_scope
-      ipaddr = scope['facts']['ipaddress']
+      ipaddr = scope['facts']['networking']['ip']
     else
       ipaddr = ip.dup
     end
diff --git a/lib/puppet/functions/simplib/ipaddresses.rb b/lib/puppet/functions/simplib/ipaddresses.rb
@@ -13,14 +13,10 @@
   def ipaddresses(only_remote = false)
     retval = []
     scope = closure_scope
-    interfaces = scope['facts']['interfaces']
+    interfaces = scope['facts'].dig('networking', 'interfaces')
 
-    if interfaces
-      interfaces.split(',').each do |iface|
-        iface_addr = scope['facts']["ipaddress_#{iface}"]
-
-        retval << iface_addr unless iface_addr.nil? || iface_addr.strip.empty?
-      end
+    if interfaces.is_a?(Hash) && !interfaces.empty?
+      retval = interfaces.select { |_, v| v['ip'].is_a?(String) && !v['ip'].empty? }.map { |_, v| v['ip'] }
 
       retval.delete_if { |x| x =~ %r{^127} } if only_remote
     end
diff --git a/lib/puppet/functions/simplib/join_mount_opts.rb b/lib/puppet/functions/simplib/join_mount_opts.rb
@@ -34,7 +34,7 @@ def join_mount_opts(system_mount_opts, new_mount_opts)
 
     mount_options = {}
     scope = closure_scope
-    selinux_current_mode = scope['facts']['selinux_current_mode']
+    selinux_current_mode = scope['facts']['os'].dig('selinux', 'current_mode')
 
     if !selinux_current_mode || (selinux_current_mode == 'disabled')
       # SELinux is off, get rid of selinux related items in the options
diff --git a/lib/puppet/functions/simplib/validate_sysctl_value.rb b/lib/puppet/functions/simplib/validate_sysctl_value.rb
@@ -44,13 +44,10 @@ def fs__inotify__max_user_watches(key, value)
       validate_sysctl_err("#{key} cannot be #{value}")
     end
 
-    system_ram_mb = closure_scope['facts']['memorysize_mb']
-
-    return unless system_ram_mb
-    system_ram_mb = system_ram_mb.to_i
+    system_ram_mb = closure_scope['facts'].dig('memory', 'system', 'total_bytes').to_i >> 20
 
     size_multiplier = 512
-    size_multiplier = 1024 if closure_scope['facts']['architecture'] == 'x86_64'
+    size_multiplier = 1024 if closure_scope['facts'].dig('os', 'architecture') == 'x86_64'
 
     inode_ram_mb = (value.to_i * size_multiplier) / 1024 / 1024
 
diff --git a/lib/puppet/provider/reboot_notify/notify.rb b/lib/puppet/provider/reboot_notify/notify.rb
@@ -124,7 +124,7 @@ def self.post_resource_eval
       # If the number of seconds between the time that the record was written
       # and the current time is greater than the system uptime then we should
       # remove the record
-      (current_time - v['updated']) > Facter.value(:uptime_seconds)
+      (current_time - v['updated']) > Facter.value(:system_uptime)['seconds']
     end
 
     unless records.empty?
diff --git a/metadata.json b/metadata.json
@@ -1,6 +1,6 @@
 {
   "name": "simp-simplib",
-  "version": "4.13.0",
+  "version": "4.14.0",
   "author": "SIMP Team",
   "summary": "A collection of common SIMP functions, facts, and types",
   "license": "Apache-2.0",
diff --git a/spec/acceptance/suites/default/ipaddresses_spec.rb b/spec/acceptance/suites/default/ipaddresses_spec.rb
@@ -5,19 +5,9 @@
 describe 'simplib::ipaddresses function' do
   hosts.each do |server|
     let(:all_ips) do
-      ifaces = fact_on(server, 'interfaces').split(',').map(&:strip)
-
-      ipaddresses = []
-
-      ifaces.each do |iface|
-        ipaddress = fact_on(server, "ipaddress_#{iface}")
-
-        if ipaddress && !ipaddress.strip.empty?
-          ipaddresses << ipaddress
-        end
-      end
-
-      ipaddresses
+      fact_on(server, 'networking.interfaces')
+        .select { |_, v| v['ip'].is_a?(String) && !v['ip'].empty? }
+        .map { |_, v| v['ip'] }
     end
 
     let(:remote_ips) do
diff --git a/spec/functions/simplib/host_is_me_spec.rb b/spec/functions/simplib/host_is_me_spec.rb
@@ -5,12 +5,15 @@
   context 'FQDN, hostname, and iterface facts exist' do
     let(:facts) do
       {
-        fqdn: 'myhost.example.com',
-        hostname: 'myhost',
-        interfaces: 'eth0,eth1,lo',
-        ipaddress_eth0: '1.2.3.4',
-        ipaddress_eth1: '5.6.7.8',
-        ipaddress_lo: '127.0.0.1',
+        networking: {
+          fqdn: 'myhost.example.com',
+          hostname: 'myhost',
+          interfaces: {
+            eth0: { ip: '1.2.3.4' },
+            eth1: { ip: '5.6.7.8' },
+            lo: { ip: '127.0.0.1' },
+          },
+        },
       }
     end
 
diff --git a/spec/functions/simplib/ip_to_cron_spec.rb b/spec/functions/simplib/ip_to_cron_spec.rb
@@ -2,7 +2,7 @@
 require 'spec_helper'
 
 describe 'simplib::ip_to_cron' do
-  let(:facts) { { ipaddress: '10.0.10.154' } }
+  let(:facts) { { networking: { ip: '10.0.10.154' } } }
 
   context 'with default parameters' do
     it { is_expected.to run.with_params.and_return([54]) }
diff --git a/spec/functions/simplib/ipaddresses_spec.rb b/spec/functions/simplib/ipaddresses_spec.rb
@@ -6,7 +6,8 @@
     let(:result) { subject.execute }
 
     it 'returns an Array' do
-      expect(result.is_a?(Array)).to be true
+      expect(result).to be_a(Array)
+      expect(result).not_to be_empty
     end
 
     it 'returns an Array with no nil values' do
@@ -22,10 +23,13 @@
   context 'All Interfaces Have IP Addresses' do
     let(:facts) do
       {
-        interfaces: 'eth0,eth1,lo',
-        ipaddress_eth0: '1.2.3.4',
-        ipaddress_eth1: '5.6.7.8',
-        ipaddress_lo: '127.0.0.1',
+        networking: {
+          interfaces: {
+            eth0: { ip: '1.2.3.4' },
+            eth1: { ip: '5.6.7.8' },
+            lo: { ip: '127.0.0.1' },
+          },
+        },
       }
     end
 
@@ -35,9 +39,12 @@
   context 'All Interfaces Do Not Have IP Addresses' do
     let(:facts) do
       {
-        interfaces: 'eth0,eth1,lo',
-        ipaddress_eth0: '1.2.3.4',
-        ipaddress_lo: '127.0.0.1',
+        networking: {
+          interfaces: {
+            eth0: { ip: '1.2.3.4' },
+            lo: { ip: '127.0.0.1' },
+          },
+        },
       }
     end
 
@@ -50,5 +57,11 @@
     it 'does not raise an error' do
       expect { subject.execute }.not_to raise_error # rubocop:disable RSpec/NamedSubject
     end
+
+    it 'returns an empty Array' do
+      result = subject.execute # rubocop:disable RSpec/NamedSubject
+      expect(result).to be_a(Array)
+      expect(result).to be_empty
+    end
   end
 end
diff --git a/spec/functions/simplib/join_mount_opts_spec.rb b/spec/functions/simplib/join_mount_opts_spec.rb
@@ -6,7 +6,7 @@
   # "tmp_mount_tmp": "rw,seclabel,nosuid,nodev,noexec,relatime,attr2,inode64,noquota"
   # "tmp_mount_var_tmp": "rw,seclabel,nosuid,nodev,noexec,relatime,attr2,inode64,noquota"
   context 'without selinux mount options specified' do
-    let(:facts) { { selinux_current_mode: 'enforcing' } }
+    let(:facts) { { os: { selinux: { current_mode: 'enforcing' } } } }
 
     context 'with no mount options overlap' do
       it 'concatenates system and new options' do
@@ -58,7 +58,7 @@
   context 'with selinux mount options specified' do
     ['enforcing', 'permissive'].each do |selinux_mode|
       context "with selinux '#{selinux_mode}'" do
-        let(:facts) { { selinux_current_mode: selinux_mode } }
+        let(:facts) { { os: { selinux: { current_mode: selinux_mode } } } }
 
         ['context', 'fscontext', 'defcontext', 'rootcontext'].each do |context_opt|
           it "removes #{context_opt} when seclabel exits in system options" do
@@ -73,7 +73,7 @@
 
     ['disabled', nil].each do |selinux_mode|
       context "with selinux '#{selinux_mode}'" do
-        let(:facts) { { selinux_current_mode: selinux_mode } }
+        let(:facts) { { os: { selinux: { current_mode: selinux_mode } } } }
 
         ['context', 'fscontext', 'defcontext', 'rootcontext'].each do |context_opt|
           it "removes #{context_opt}" do
diff --git a/spec/functions/simplib/validate_sysctl_value_spec.rb b/spec/functions/simplib/validate_sysctl_value_spec.rb
@@ -34,8 +34,10 @@
   context 'fs.inotify.max_user_watches' do
     let(:facts) do
       {
-        architecture: 'x86_64',
-        memorysize_mb: 20,
+        os: { architecture: 'x86_64' },
+        memory: {
+          system: { total_bytes: 20_971_520 },
+        },
       }
     end
 
@@ -72,8 +74,10 @@
       context 'i686' do
         let(:facts) do
           {
-            architecture: 'i686',
-            memorysize_mb: 20,
+            os: { architecture: 'i686' },
+            memory: {
+              system: { total_bytes: 20_971_520 },
+            },
           }
         end
 
diff --git a/spec/unit/facter/login_defs_spec.rb b/spec/unit/facter/login_defs_spec.rb
@@ -5,7 +5,6 @@
     Facter.clear
 
     allow(Facter).to receive(:value).with(any_args).and_call_original
-    allow(Facter).to receive(:value).with(:operatingsystem).and_return('Linux')
     allow(File).to receive(:read).with(any_args).and_call_original
     allow(File).to receive(:readable?).with(any_args).and_call_original
   end
diff --git a/spec/unit/facter/uid_min_spec.rb b/spec/unit/facter/uid_min_spec.rb
@@ -0,0 +1,56 @@
+require 'spec_helper'
+
+describe 'uid_min', type: :fact do
+  subject(:fact) { Facter.fact(:uid_min) }
+
+  before(:each) do
+    Facter.clear
+
+    allow(File).to receive(:exist?).and_call_original
+    allow(File).to receive(:read).and_call_original
+  end
+
+  context 'when /etc/login.defs does not exist' do
+    before(:each) do
+      allow(File).to receive(:exist?).with('/etc/login.defs').and_return(false)
+    end
+
+    it 'returns nil' do
+      expect(fact.value).to be_nil
+    end
+  end
+
+  context 'when /etc/login.defs exists' do
+    before(:each) do
+      allow(File).to receive(:exist?).with('/etc/login.defs').and_return(true)
+    end
+
+    context 'when /etc/login.defs is empty' do
+      before(:each) do
+        allow(File).to receive(:read).with('/etc/login.defs').and_return('')
+      end
+
+      it 'returns default value' do
+        expect(fact.value).to eq('1000')
+      end
+    end
+
+    context 'when /etc/login.defs has matching lines' do
+      let(:login_defs_content) do
+        <<~EOM
+          #UID_MIN ???
+          # UID_MIN 500
+           UID_MIN         10000
+        EOM
+      end
+
+      before(:each) do
+        allow(File).to receive(:read).with('/etc/login.defs').and_return(login_defs_content)
+      end
+
+      it 'returns expected value' do
+        expect(fact.value).to eq('10000')
+      end
+    end
+  end
+end

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"name": "simp-simplib",`
`3`		`- "version": "4.13.0",`
	`3`	`+ "version": "4.14.0",`
`4`	`4`	`"author": "SIMP Team",`
`5`	`5`	`"summary": "A collection of common SIMP functions, facts, and types",`
`6`	`6`	`"license": "Apache-2.0",`