Ruby 3 support (#288)

silug · web-flow · commit 36291dbb83bc · 2023-08-29T10:27:50.000-05:00
* Support stdlib 9
* Update gem dependencies
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,3 +1,9 @@
+* Mon Aug 28 2023 Steven Pritchard <steve@sicura.us> - 4.12.0
+- Fixed
+  - Ruby 3 support
+  - puppetlabs-stdlib 9 support
+  - Updated gem dependencies
+
 * Mon Jun 12 2023 Chris Tessmer <chris.tessmer@onyxpoint.com> - 4.11.0
 - Add RockyLinux 8 support
 
diff --git a/Gemfile b/Gemfile
@@ -11,7 +11,7 @@ ENV['PDK_DISABLE_ANALYTICS'] ||= 'true'
 gem_sources.each { |gem_source| source gem_source }
 
 group :test do
-  puppet_version = ENV['PUPPET_VERSION'] || '~> 7'
+  puppet_version = ENV['PUPPET_VERSION'] || '~> 8'
   major_puppet_version = puppet_version.scan(/(\d+)(?:\.|\Z)/).flatten.first.to_i
   gem 'rake'
   gem 'puppet', puppet_version
@@ -24,8 +24,8 @@ group :test do
   gem 'puppet-lint-trailing_comma-check', :require => false
   gem 'simp-rspec-puppet-facts', ENV['SIMP_RSPEC_PUPPET_FACTS_VERSION'] || '~> 3.1'
   gem 'simp-rake-helpers', ENV['SIMP_RAKE_HELPERS_VERSION'] || ['>= 5.12.1', '< 6']
-  gem( 'pdk', ENV['PDK_VERSION'] || '~> 2.0', :require => false) if major_puppet_version > 5
-  gem 'pathspec', '~> 0.2' if Gem::Requirement.create('< 2.6').satisfied_by?(Gem::Version.new(RUBY_VERSION.dup))
+  gem( 'pdk', ENV['PDK_VERSION'] || ['>= 2.0', '<= 4.0'], :require => false) if major_puppet_version > 5
+  gem 'pathspec', '~> 1.0' if Gem::Requirement.create('< 2.6').satisfied_by?(Gem::Version.new(RUBY_VERSION.dup))
 end
 
 group :development do
diff --git a/lib/facter/ipa.rb b/lib/facter/ipa.rb
@@ -69,8 +69,8 @@
     # We won't know if we are connected to a server until later
     defaults['connected'] = false
 
-    Facter::Core::Execution.execute("#{klist} -s")
-    unless $?.success?
+    klist_retval = Puppet::Util::Execution.execute("#{klist} -s")
+    unless klist_retval.exitstatus.zero?
       # Obtain host Kerberos token so we can use IPA API
       kinit_msg = Facter::Core::Execution.execute("#{kinit} -k 2>&1", options = {:timeout => kinit_timeout})
     end
@@ -101,5 +101,7 @@
     end
 
     defaults.merge(ipa_response)
+  rescue => e
+    Facter.warn(e)
   end
 end
diff --git a/lib/facter/simplib__networkmanager.rb b/lib/facter/simplib__networkmanager.rb
@@ -9,9 +9,9 @@
 
     nmcli_cmd = @nmcli_cmd + ' -t'
 
-    general_status = Facter::Core::Execution.execute(%(#{nmcli_cmd} -m multiline general status))
+    general_status = Puppet::Util::Execution.execute(%(#{nmcli_cmd} -m multiline general status))
 
-    if $?.success?
+    if general_status.exitstatus.zero?
       general_status = general_status.lines.map{|line| line.strip.split(':') }
 
       info['enabled'] = true
@@ -22,17 +22,17 @@
       }
     end
 
-    general_hostname = Facter::Core::Execution.execute(%{#{nmcli_cmd} general hostname})
+    general_hostname = Puppet::Util::Execution.execute(%{#{nmcli_cmd} general hostname})
 
-    if $?.success?
+    if general_hostname.exitstatus.zero?
       info['enabled'] = true
       info['general'] ||= {}
       info['general']['hostname'] = general_hostname.strip
     end
 
-    connections = Facter::Core::Execution.execute(%(#{nmcli_cmd} connection show))
+    connections = Puppet::Util::Execution.execute(%(#{nmcli_cmd} connection show))
 
-    if $?.success?
+    if connections.exitstatus.zero?
       info['enabled'] = true
       info['connection'] = {}
 
@@ -48,5 +48,7 @@
     end
 
     info
+  rescue => e
+    Facter.warn(e)
   end
 end
diff --git a/lib/puppet/functions/simplib/debug/inspect.rb b/lib/puppet/functions/simplib/debug/inspect.rb
@@ -21,7 +21,7 @@
   def inspect(scope, to_inspect, print=true)
     data = {
       :type     => to_inspect.class,
-      :content  => to_inspect.to_pson
+      :content  => to_inspect.to_json
     }
 
     if scope
diff --git a/lib/puppet/functions/simplib/passgen/legacy/passgen.rb b/lib/puppet/functions/simplib/passgen/legacy/passgen.rb
@@ -107,7 +107,7 @@ def passgen(identifier, modifier_hash={})
 
     unless File.directory?(settings['keydir'])
       begin
-        FileUtils.mkdir_p(settings['keydir'],{:mode => 0750})
+        FileUtils.mkdir_p(settings['keydir'], mode: 0750)
         # This chown is applicable as long as it is applied
         # by puppet, not puppetserver.
         FileUtils.chown(settings['user'],
@@ -294,15 +294,15 @@ def get_current_password(identifier, options, settings)
   # 'password' in options or had to generate a password.
   def get_last_password(identifier, options, settings)
     toread = nil
-    if File.exists?("#{settings['keydir']}/#{identifier}.last")
+    if File.exist?("#{settings['keydir']}/#{identifier}.last")
       toread = "#{settings['keydir']}/#{identifier}.last"
     else
       toread = "#{settings['keydir']}/#{identifier}"
     end
 
     passwd = ''
     salt = ''
-    if File.exists?(toread)
+    if File.exist?(toread)
       passwd = IO.readlines(toread)[0].to_s.chomp
       sf = "#{File.dirname(toread)}/#{File.basename(toread,'.last')}.salt.last"
       saltfile = File.open(sf,'a+',0640)
diff --git a/lib/puppet/functions/simplib/passgen/legacy/set.rb b/lib/puppet/functions/simplib/passgen/legacy/set.rb
@@ -85,7 +85,7 @@ def move_files(files, source_prefix, dest_prefix)
     FileUtils.mv(files["#{source_prefix}password"], files["#{dest_prefix}password"],
       :force => true)
 
-    if File.exists?(files["#{source_prefix}salt"])
+    if File.exist?(files["#{source_prefix}salt"])
       FileUtils.mv(files["#{source_prefix}salt"], files["#{dest_prefix}salt"],
         :force => true)
     else
@@ -104,7 +104,7 @@ def set_password_info(settings, identifier, password, salt)
   # @raise RuntimeError if fails to create or set permissions on keydir
   def set_up_keydir(settings)
     begin
-      FileUtils.mkdir_p(settings['keydir'], {:mode => settings['dir_mode']})
+      FileUtils.mkdir_p(settings['keydir'], mode: settings['dir_mode'])
       FileUtils.chown(settings['user'], settings['group'], settings['keydir'])
     rescue SystemCallError => e
       err_msg = "simplib::passgen::legacy::set: Could not make directory" +
diff --git a/metadata.json b/metadata.json
@@ -1,6 +1,6 @@
 {
   "name": "simp-simplib",
-  "version": "4.11.0",
+  "version": "4.12.0",
   "author": "SIMP Team",
   "summary": "A collection of common SIMP functions, facts, and types",
   "license": "Apache-2.0",
@@ -18,7 +18,7 @@
   "dependencies": [
     {
       "name": "puppetlabs/stdlib",
-      "version_requirement": ">= 8.0.0 < 9.0.0"
+      "version_requirement": ">= 8.0.0 < 10.0.0"
     },
     {
       "name": "simp/simpkv",
@@ -96,7 +96,7 @@
   "requirements": [
     {
       "name": "puppet",
-      "version_requirement": ">= 6.22.1 < 8.0.0"
+      "version_requirement": ">= 7.0.0 < 9.0.0"
     }
   ],
   "package_release_version": "0"
diff --git a/spec/functions/simplib/gen_random_password_spec.rb b/spec/functions/simplib/gen_random_password_spec.rb
@@ -64,7 +64,7 @@
     # This test is timing sensitive.  It may not fail if the test machine
     # processes faster than the dev machine.
     it 'raises timeout error if password cannot be generated in specified interval' do
-      is_expected.to run.with_params(10**20, 2, false, 0.01).and_raise_error(TimeoutError)
+      is_expected.to run.with_params(10**20, 2, false, 0.01).and_raise_error(Timeout::Error)
     end
 
     it 'fails if length is outside of specified range' do
diff --git a/spec/functions/simplib/validate_net_list_spec.rb b/spec/functions/simplib/validate_net_list_spec.rb
@@ -26,7 +26,7 @@
       end
 
       it 'rejects CIDR missing network mask' do
-        pending "IPAddr allows empty network mask '1.2.3.4/'"
+        pending "IPAddr allows empty network mask '1.2.3.4/'" if Gem::Version.new(Puppet.version) < Gem::Version.new('8')
         is_expected.to run.with_params('1.2.3.4/').and_raise_error(RuntimeError)
       end
 
@@ -73,7 +73,7 @@
       end
 
       it 'rejects CIDR missing network mask' do
-        pending "IPAddr allows empty network mask '2001:db8:a::/'"
+        pending "IPAddr allows empty network mask '2001:db8:a::/'" if Gem::Version.new(Puppet.version) < Gem::Version.new('8')
         is_expected.to run.with_params('2001:db8:a::/').and_raise_error(RuntimeError)
       end
 
diff --git a/spec/unit/facter/ipa_spec.rb b/spec/unit/facter/ipa_spec.rb
@@ -57,7 +57,6 @@
   before(:each) do
     Facter.clear
 
-
     # mock out Facter method called when evaluating confine for :kernel
     # Facter 4
     if defined?(Facter::Resolvers::Uname)
@@ -70,23 +69,24 @@
   context 'host is joined to IPA domain' do
 
     before(:each) do
-      expect(ENV).to receive(:fetch).with('LANG', 'en_US.UTF-8').and_return('en_US.UTF-8')
-      expect(Facter::Core::Execution).to receive(:which).with('kinit').and_return('/usr/bin/kinit')
-      expect(Facter::Core::Execution).to receive(:which).with('klist').and_return('/usr/bin/klist')
-      expect(Facter::Core::Execution).to receive(:which).with('ipa').and_return('/usr/bin/ipa')
-      expect(Facter::Core::Execution).to receive(:which).with('true').and_return('/bin/true')
+      allow(ENV).to receive(:fetch).with('LANG', 'en_US.UTF-8').and_return('en_US.UTF-8')
+      allow(Facter::Core::Execution).to receive(:which).with('kinit').and_return('/usr/bin/kinit')
+      allow(Facter::Core::Execution).to receive(:which).with('klist').and_return('/usr/bin/klist')
+      allow(Facter::Core::Execution).to receive(:which).with('ipa').and_return('/usr/bin/ipa')
+      allow(Facter::Core::Execution).to receive(:which).with('true').and_return('/bin/true')
     end
 
     context 'IPA server is available' do
       context 'kinit is not required' do
-        it 'should execute only ipa commands and report local env + connected status' do
-          expect(File).to receive(:exist?).with('/etc/ipa/default.conf').and_return(true)
-          expect(File).to receive(:read).with('/etc/ipa/default.conf').and_return(default_conf)
-          expect(Facter::Core::Execution).to receive(:execute).with('/usr/bin/klist -s')
-          allow_any_instance_of(Process::Status).to receive(:success?).and_return(true)
-          expect(Facter::Core::Execution).to receive(:execute).with(ipa_env_query, ipa_query_options).and_return(ipa_env)
-          expect(Facter::Core::Execution).to receive(:execute).with(ipa_env_server_query, ipa_query_options).and_return(ipa_server_env)
+        before(:each) do
+          allow(File).to receive(:exist?).with('/etc/ipa/default.conf').and_return(true)
+          allow(File).to receive(:read).with('/etc/ipa/default.conf').and_return(default_conf.dup)
+          allow(Puppet::Util::Execution).to receive(:execute).with('/usr/bin/klist -s').and_return(Puppet::Util::Execution::ProcessOutput.new('', 0))
+          allow(Facter::Core::Execution).to receive(:execute).with(ipa_env_query, ipa_query_options).and_return(ipa_env)
+          allow(Facter::Core::Execution).to receive(:execute).with(ipa_env_server_query, ipa_query_options).and_return(ipa_server_env)
+        end
 
+        it 'should execute only ipa commands and report local env + connected status' do
           # Have to set $? for Facter 4
           %x{( exit 0 )}
           expect(Facter.fact('ipa').value).to eq({
@@ -100,14 +100,16 @@
       end
 
       context 'kinit is required' do
+        before(:each) do
+          allow(File).to receive(:exist?).with('/etc/ipa/default.conf').and_return(true)
+          allow(File).to receive(:read).with('/etc/ipa/default.conf').and_return(default_conf)
+          allow(Puppet::Util::Execution).to receive(:execute).with('/usr/bin/klist -s').and_return(Puppet::Util::Execution::ProcessOutput.new('', 1))
+          allow(Facter::Core::Execution).to receive(:execute).with('/usr/bin/kinit -k 2>&1', kinit_query_options).and_return('')
+          allow(Facter::Core::Execution).to receive(:execute).with(ipa_env_query, ipa_query_options).and_return(ipa_env)
+          allow(Facter::Core::Execution).to receive(:execute).with(ipa_env_server_query, ipa_query_options).and_return(ipa_server_env)
+        end
+
         it 'should execute kinit + ipa commands and return local env + connected status' do
-          expect(File).to receive(:exist?).with('/etc/ipa/default.conf').and_return(true)
-          expect(File).to receive(:read).with('/etc/ipa/default.conf').and_return(default_conf)
-          expect(Facter::Core::Execution).to receive(:execute).with('/usr/bin/klist -s')
-          allow_any_instance_of(Process::Status).to receive(:success?).and_return(false)
-          expect(Facter::Core::Execution).to receive(:execute).with('/usr/bin/kinit -k 2>&1', kinit_query_options).and_return('')
-          expect(Facter::Core::Execution).to receive(:execute).with( ipa_env_query, ipa_query_options).and_return(ipa_env)
-          expect(Facter::Core::Execution).to receive(:execute).with(ipa_env_server_query, ipa_query_options).and_return(ipa_server_env)
           expect(Facter.fact('ipa').value).to eq({
             'connected' => true,
             'domain'    => 'example.com',
@@ -120,13 +122,15 @@
     end
 
     context 'IPA server is not available' do
+      before(:each) do
+        allow(File).to receive(:exist?).with('/etc/ipa/default.conf').and_return(true)
+        allow(File).to receive(:read).with('/etc/ipa/default.conf').and_return(default_conf)
+        allow(Puppet::Util::Execution).to receive(:execute).with('/usr/bin/klist -s').and_return(Puppet::Util::Execution::ProcessOutput.new('', 1))
+        allow(Facter::Core::Execution).to receive(:execute).with('/usr/bin/kinit -k 2>&1', kinit_query_options).and_return('some error message')
+        allow(Facter::Core::Execution).to receive(:execute).with(ipa_env_query, ipa_query_options).and_return('')
+      end
+
       it 'should return defaults from /etc/ipa/default.conf and disconnected status' do
-        expect(File).to receive(:exist?).with('/etc/ipa/default.conf').and_return(true)
-        expect(File).to receive(:read).with('/etc/ipa/default.conf').and_return(default_conf)
-        expect(Facter::Core::Execution).to receive(:execute).with('/usr/bin/klist -s')
-        allow_any_instance_of(Process::Status).to receive(:success?).and_return(false)
-        expect(Facter::Core::Execution).to receive(:execute).with('/usr/bin/kinit -k 2>&1', kinit_query_options).and_return('some error message')
-        expect(Facter::Core::Execution).to receive(:execute).with(ipa_env_query, ipa_query_options).and_return('')
         expect(Facter.fact('ipa').value).to eq({
           'connected' => false,
           'domain'    => 'example.com',
diff --git a/spec/unit/facter/simplib__networkmanager_spec.rb b/spec/unit/facter/simplib__networkmanager_spec.rb
@@ -12,44 +12,47 @@
       allow(Facter::Core::Execution).to receive(:exec).with('uname -s').and_return('Linux')
     end
 
-    expect(Facter::Util::Resolution).to receive(:which).with('nmcli').and_return('/usr/sbin/nmcli')
+    allow(Facter::Util::Resolution).to receive(:which).and_call_original
+    allow(Facter::Util::Resolution).to receive(:which).with('nmcli').and_return('/usr/sbin/nmcli')
 
-    expect(Facter::Core::Execution).to receive(:execute).with('/usr/sbin/nmcli -t -m multiline general status').and_return(general_status)
-    expect(Facter::Core::Execution).to receive(:execute).with('/usr/sbin/nmcli -t general hostname').and_return(general_hostname)
-    expect(Facter::Core::Execution).to receive(:execute).with('/usr/sbin/nmcli -t connection show').and_return(connections)
+    allow(Puppet::Util::Execution).to receive(:execute).with('/usr/sbin/nmcli -t -m multiline general status').and_return(general_status)
+    allow(Puppet::Util::Execution).to receive(:execute).with('/usr/sbin/nmcli -t general hostname').and_return(general_hostname)
+    allow(Puppet::Util::Execution).to receive(:execute).with('/usr/sbin/nmcli -t connection show').and_return(connections)
   end
 
   context 'nmcli fails' do
-    let(:general_status){ '' }
-    let(:general_hostname){ '' }
-    let(:connections){ '' }
+    let(:general_status){ Puppet::Util::Execution::ProcessOutput.new('', 1) }
+    let(:general_hostname){ Puppet::Util::Execution::ProcessOutput.new('', 1) }
+    let(:connections){ Puppet::Util::Execution::ProcessOutput.new('', 1) }
 
     it 'returns "enabled" = false' do
-      allow_any_instance_of(Process::Status).to receive(:success?).and_return(false)
+      # allow_any_instance_of(Process::Status).to receive(:success?).and_return(false)
 
       expect(Facter.fact('simplib__networkmanager').value).to eq({'enabled' => false})
     end
   end
 
   context 'nmcli succeeds' do
     let(:general_status){
-      <<~EOM
+      output = <<~EOM
         STATE:connected
         CONNECTIVITY:full
         WIFI-HW:enabled
         WIFI:enabled
         WWAN-HW:enabled
         WWAN:enabled
         EOM
+      Puppet::Util::Execution::ProcessOutput.new(output, 0)
     }
 
-    let(:general_hostname){ "foo.bar.baz\n" }
+    let(:general_hostname){ Puppet::Util::Execution::ProcessOutput.new("foo.bar.baz\n", 0) }
 
     let(:connections){
-      <<~EOM
+      output = <<~EOM
         Eth Dev:b961cb37-ae05-4c67-98b0-432465fe03c2:802-3-ethernet:eth0
         Bridge Dev:0c190f3f-262b-4585-a7de-2a146896ea86:bridge:virbr0
         EOM
+      Puppet::Util::Execution::ProcessOutput.new(output, 0)
     }
 
     let(:expected){{

Original file line number	Diff line number	Diff line change
`@@ -21,7 +21,7 @@`
`21`	`21`	`def inspect(scope, to_inspect, print=true)`
`22`	`22`	`data = {`
`23`	`23`	`:type => to_inspect.class,`
`24`		`- :content => to_inspect.to_pson`
	`24`	`+ :content => to_inspect.to_json`
`25`	`25`	`}`
`26`	`26`
`27`	`27`	`if scope`
Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"name": "simp-simplib",`
`3`		`- "version": "4.11.0",`
	`3`	`+ "version": "4.12.0",`
`4`	`4`	`"author": "SIMP Team",`
`5`	`5`	`"summary": "A collection of common SIMP functions, facts, and types",`
`6`	`6`	`"license": "Apache-2.0",`
`@@ -18,7 +18,7 @@`
`18`	`18`	`"dependencies": [`
`19`	`19`	`{`
`20`	`20`	`"name": "puppetlabs/stdlib",`
`21`		`- "version_requirement": ">= 8.0.0 < 9.0.0"`
	`21`	`+ "version_requirement": ">= 8.0.0 < 10.0.0"`
`22`	`22`	`},`
`23`	`23`	`{`
`24`	`24`	`"name": "simp/simpkv",`
`@@ -96,7 +96,7 @@`
`96`	`96`	`"requirements": [`
`97`	`97`	`{`
`98`	`98`	`"name": "puppet",`
`99`		`- "version_requirement": ">= 6.22.1 < 8.0.0"`
	`99`	`+ "version_requirement": ">= 7.0.0 < 9.0.0"`
`100`	`100`	`}`
`101`	`101`	`],`
`102`	`102`	`"package_release_version": "0"`