Merge pull request #249 from puppetlabs/CAT-1688-Upgrade_rubocop

(CAT-1688) Upgrade rubocop

.rubocop.yml

@@ -5,6 +5,7 @@ require:
 - rubocop-performance
 - rubocop-rspec
 AllCops:
+  NewCops: enable
   DisplayCopNames: true
   TargetRubyVersion: '2.6'
   Include:
@@ -81,3 +82,6 @@ Style/Documentation:
   - spec/**/*
 Style/WordArray:
   EnforcedStyle: brackets
+Style/FileWrite:
+  Description: Enforcing this rule might break the code in unexpected ways.
+  Enabled: false

.rubocop_todo.yml

@@ -1,30 +1,30 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2023-04-28 12:30:24 UTC using RuboCop version 1.48.1.
+# on 2024-01-17 17:28:36 UTC using RuboCop version 1.50.0.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
 # versions of RuboCop, may require this file to be generated again.
 
-# Offense count: 12
+# Offense count: 13
 # Configuration parameters: AllowedMethods, AllowedPatterns, CountRepeatedAttributes.
 Metrics/AbcSize:
   Max: 94
 
-# Offense count: 1
+# Offense count: 2
 # Configuration parameters: CountComments, CountAsOne.
 Metrics/ClassLength:
-  Max: 200
+  Max: 155
 
-# Offense count: 9
+# Offense count: 8
 # Configuration parameters: AllowedMethods, AllowedPatterns.
 Metrics/CyclomaticComplexity:
-  Max: 25
+  Max: 23
 
-# Offense count: 19
+# Offense count: 20
 # Configuration parameters: CountComments, CountAsOne, AllowedMethods, AllowedPatterns.
 Metrics/MethodLength:
-  Max: 69
+  Max: 70
 
 # Offense count: 3
 # Configuration parameters: CountKeywordArgs, MaxOptionalParameters.
@@ -34,7 +34,7 @@ Metrics/ParameterLists:
 # Offense count: 7
 # Configuration parameters: AllowedMethods, AllowedPatterns.
 Metrics/PerceivedComplexity:
-  Max: 30
+  Max: 25
 
 # Offense count: 2
 # Configuration parameters: IgnoredMetadata.
@@ -48,30 +48,29 @@ RSpec/DescribeClass:
     - 'spec/tasks/abs_spec.rb'
     - 'spec/unit/task_helper_spec.rb'
 
-# Offense count: 4
+# Offense count: 12
 # Configuration parameters: CountAsOne.
 RSpec/ExampleLength:
-  Max: 30
+  Max: 27
 
-# Offense count: 6
+# Offense count: 13
 RSpec/MultipleExpectations:
   Max: 13
 
-# Offense count: 2
+# Offense count: 6
 # Configuration parameters: AllowSubject.
 RSpec/MultipleMemoizedHelpers:
   Max: 6
 
-# Offense count: 4
+# Offense count: 6
 RSpec/StubbedMock:
   Exclude:
     - 'spec/tasks/abs_spec.rb'
 
-# Offense count: 8
+# Offense count: 7
 Style/MixinUsage:
   Exclude:
     - 'spec/spec_helper.rb'
     - 'tasks/docker.rb'
     - 'tasks/docker_exp.rb'
-    - 'tasks/provision_service.rb'
     - 'tasks/vagrant.rb'

Gemfile

@@ -31,7 +31,7 @@ group :development do
   gem "pry", '~> 0.10',                          require: false
   gem "simplecov-console", '~> 0.5',             require: false
   gem "puppet-debugger", '~> 1.0',               require: false
-  gem "rubocop", '= 1.48.1',                     require: false
+  gem "rubocop", '~> 1.50.0',                     require: false
   gem "rubocop-performance", '= 1.16.0',         require: false
   gem "rubocop-rspec", '= 2.19.0',               require: false
   gem "rb-readline", '= 0.5.5',                  require: false, platforms: [:mswin, :mingw, :x64_mingw]

tasks/abs.rb

@@ -26,11 +26,11 @@ def abs_host
   def provision(platform, inventory_location, vars)
     uri = URI.parse("https://#{abs_host}/api/v2/request")
     jenkins_build_url = if ENV['CI'] == 'true' && ENV['TRAVIS'] == 'true'
-                          ENV['TRAVIS_JOB_WEB_URL']
+                          ENV.fetch('TRAVIS_JOB_WEB_URL', nil)
                         elsif ENV['CI'] == 'True' && ENV['APPVEYOR'] == 'True'
-                          "https://ci.appveyor.com/project/#{ENV['APPVEYOR_REPO_NAME']}/build/job/#{ENV['APPVEYOR_JOB_ID']}"
+                          "https://ci.appveyor.com/project/#{ENV.fetch('APPVEYOR_REPO_NAME', nil)}/build/job/#{ENV.fetch('APPVEYOR_JOB_ID', nil)}"
                         elsif ENV['GITHUB_ACTIONS'] == 'true'
-                          "https://github.com/#{ENV['GITHUB_REPOSITORY']}/actions/runs/#{ENV['GITHUB_RUN_ID']}"
+                          "https://github.com/#{ENV.fetch('GITHUB_REPOSITORY', nil)}/actions/runs/#{ENV.fetch('GITHUB_RUN_ID', nil)}"
                         else
                           'https://litmus_manual'
                         end
@@ -91,17 +91,18 @@ def provision(platform, inventory_location, vars)
     data.each do |host|
       if platform_uses_ssh(host['type'])
         node = { 'uri' => host['hostname'],
-                 'config' => { 'transport' => 'ssh', 'ssh' => { 'user' => ENV['ABS_USER'], 'host-key-check' => false, 'connect-timeout' => 120 } },
+                 'config' => { 'transport' => 'ssh', 'ssh' => { 'user' => ENV.fetch('ABS_USER', nil), 'host-key-check' => false, 'connect-timeout' => 120 } },
                  'facts' => { 'provisioner' => 'abs', 'platform' => host['type'], 'job_id' => job_id } }
         if !ENV['ABS_SSH_PRIVATE_KEY'].nil? && !ENV['ABS_SSH_PRIVATE_KEY'].empty?
-          node['config']['ssh']['private-key'] = ENV['ABS_SSH_PRIVATE_KEY']
+          node['config']['ssh']['private-key'] = ENV.fetch('ABS_SSH_PRIVATE_KEY', nil)
         else
-          node['config']['ssh']['password'] = ENV['ABS_PASSWORD']
+          node['config']['ssh']['password'] = ENV.fetch('ABS_PASSWORD', nil)
         end
         group_name = 'ssh_nodes'
       else
         node = { 'uri' => host['hostname'],
-                 'config' => { 'transport' => 'winrm', 'winrm' => { 'user' => ENV['ABS_WIN_USER'], 'password' => ENV['ABS_PASSWORD'], 'ssl' => false, 'connect-timeout' => 120 } },
+                 'config' => { 'transport' => 'winrm',
+                               'winrm' => { 'user' => ENV.fetch('ABS_WIN_USER', nil), 'password' => ENV.fetch('ABS_PASSWORD', nil), 'ssl' => false, 'connect-timeout' => 120 } },
                  'facts' => { 'provisioner' => 'abs', 'platform' => host['type'], 'job_id' => job_id } }
         group_name = 'winrm_nodes'
       end

tasks/docker.rb

@@ -23,16 +23,16 @@ def install_ssh_components(distro, version, container)
       # sometimes the redhat 6 variant containers like to eat their rpmdb, leading to
       # issues with "rpmdb: unable to join the environment" errors
       # This "fix" is from https://www.srv24x7.com/criticalyum-main-error-rpmdb-open-failed/
-      run_local_command("docker exec #{container} bash -exc \"rm -f /var/lib/rpm/__db*; "\
-        'db_verify /var/lib/rpm/Packages; '\
-        'rpm --rebuilddb; '\
-        'yum clean all; '\
-        'yum install -y sudo openssh-server openssh-clients"')
+      run_local_command("docker exec #{container} bash -exc \"rm -f /var/lib/rpm/__db*; " \
+                        'db_verify /var/lib/rpm/Packages; ' \
+                        'rpm --rebuilddb; ' \
+                        'yum clean all; ' \
+                        'yum install -y sudo openssh-server openssh-clients"')
     else
       # If systemd is running for init, ensure systemd has finished starting up before proceeding:
-      check_init_cmd = 'if [[ "$(readlink /proc/1/exe)" == "/usr/lib/systemd/systemd" ]]; then '\
-            'count=0 ; while ! [[ "$(systemctl is-system-running)" =~ ^running|degraded$ && $count > 20 ]]; '\
-            'do sleep 0.1 ; count=$((count+1)) ; done ; fi'
+      check_init_cmd = 'if [[ "$(readlink /proc/1/exe)" == "/usr/lib/systemd/systemd" ]]; then ' \
+                       'count=0 ; while ! [[ "$(systemctl is-system-running)" =~ ^running|degraded$ && $count > 20 ]]; ' \
+                       'do sleep 0.1 ; count=$((count+1)) ; done ; fi'
       run_local_command("docker exec #{container} bash -c '#{check_init_cmd}'")
       run_local_command("docker exec #{container} yum install -y sudo openssh-server openssh-clients")
     end
@@ -74,10 +74,10 @@ def fix_ssh(distro, version, container)
     # https://bugzilla.redhat.com/show_bug.cgi?id=1728777
     run_local_command("docker exec #{container} sed 's@session\s*required\s*pam_loginuid.so@session optional pam_loginuid.so@g' -i /etc/pam.d/sshd") if distro =~ %r{redhat|centos} && version =~ %r{^7}
 
-    if !%r{^(7|8|9|2)}.match?(version)
-      run_local_command("docker exec #{container} service sshd restart")
-    else
+    if %r{^(7|8|9|2)}.match?(version)
       run_local_command("docker exec #{container} /usr/sbin/sshd")
+    else
+      run_local_command("docker exec #{container} service sshd restart")
     end
   when %r{sles}
     run_local_command("docker exec #{container} /usr/sbin/sshd")
@@ -161,7 +161,7 @@ def provision(image, inventory_location, vars)
   distro = os_release_facts['ID']
   version = os_release_facts['VERSION_ID']
 
-  hostname = (ENV['DOCKER_HOST'].nil? || ENV['DOCKER_HOST'].empty?) ? 'localhost' : URI.parse(ENV['DOCKER_HOST']).host || ENV['DOCKER_HOST']
+  hostname = (ENV['DOCKER_HOST'].nil? || ENV['DOCKER_HOST'].empty?) ? 'localhost' : URI.parse(ENV.fetch('DOCKER_HOST', nil)).host || ENV.fetch('DOCKER_HOST', nil)
   begin
     # Use the current docker context to determine the docker hostname
     docker_context = JSON.parse(run_local_command('docker context inspect'))[0]

tasks/provision_service.rb

@@ -46,7 +46,7 @@ def invoke_cloud_request(params, uri, job_url, verb, retry_attempts)
       request.body = if job_url
                        { url: job_url, VMs: machines }.to_json
                      else
-                       { github_token: ENV['GITHUB_TOKEN'], VMs: machines }.to_json
+                       { github_token: ENV.fetch('GITHUB_TOKEN', nil), VMs: machines }.to_json
                      end
     when 'delete'
       request = Net::HTTP::Delete.new(uri, headers)
@@ -89,14 +89,14 @@ def provision(platform, inventory_location, vars, retry_attempts)
     # Call the provision service with the information necessary and write the inventory file locally
 
     if ENV['GITHUB_RUN_ID']
-      job_url = ENV['GITHUB_URL'] || "https://api.github.com/repos/#{ENV['GITHUB_REPOSITORY']}/actions/runs/#{ENV['GITHUB_RUN_ID']}"
+      job_url = ENV['GITHUB_URL'] || "https://api.github.com/repos/#{ENV.fetch('GITHUB_REPOSITORY', nil)}/actions/runs/#{ENV['GITHUB_RUN_ID']}"
     else
       puts 'Using GITHUB_TOKEN as no GITHHUB_RUN_ID found'
     end
     uri = URI.parse(ENV['SERVICE_URL'] || default_uri)
-    cloud = ENV['CLOUD']
-    region = ENV['REGION']
-    zone = ENV['ZONE']
+    cloud = ENV.fetch('CLOUD', nil)
+    region = ENV.fetch('REGION', nil)
+    zone = ENV.fetch('ZONE', nil)
     if job_url.nil? && vars
       data = JSON.parse(vars.tr(';', ','))
       job_url = data['job_url']

tasks/update_node_pp.rb

@@ -17,7 +17,7 @@ def update_file(manifest, target_node)
     # remove the last bracket in the manifest
     existing_manifest.pop
     existing_manifest.push("\n  #{manifest}\n}")
-    final_manifest = existing_manifest.join('')
+    final_manifest = existing_manifest.join
   else
     final_manifest = "node '#{target_node}' \n{\n  #{manifest} \n}"
   end

tasks/vagrant.rb

@@ -54,7 +54,7 @@ def generate_vagrantfile(file_path, platform, enable_synced_folder, provider, cp
                      ''
                    end
   vf = <<~VF
-    Vagrant.configure(\"2\") do |config|
+    Vagrant.configure("2") do |config|
       config.vm.box = '#{platform}'
       config.vm.boot_timeout = 600
       config.ssh.insert_key = false
@@ -86,7 +86,7 @@ def configure_remoting(platform, remoting_config_path, password)
       keys: remoting_config['identityfile'],
       password: password,
       verbose: :debug
-    }.reject { |_k, v| v.nil? }
+    }.compact
     Net::SSH.start(
       remoting_config['hostname'],
       remoting_config['user'],
@@ -213,18 +213,16 @@ def tear_down(node_name, inventory_location)
 action = params['action']
 node_name = params['node_name']
 inventory_location = sanitise_inventory_location(params['inventory'])
-enable_synced_folder = params['enable_synced_folder'].nil? ? ENV['VAGRANT_ENABLE_SYNCED_FOLDER'] : params['enable_synced_folder']
-if enable_synced_folder.is_a?(String)
-  enable_synced_folder = enable_synced_folder.casecmp('true').zero? ? true : false
-end
-provider            = params['provider'].nil? ? ENV['VAGRANT_PROVIDER'] : params['provider']
-cpus                = params['cpus'].nil? ? ENV['VAGRANT_CPUS'] : params['cpus']
-memory              = params['memory'].nil? ? ENV['VAGRANT_MEMORY'] : params['memory']
-hyperv_vswitch      = params['hyperv_vswitch'].nil? ? ENV['VAGRANT_HYPERV_VSWITCH'] : params['hyperv_vswitch']
-hyperv_smb_username = params['hyperv_smb_username'].nil? ? ENV['VAGRANT_HYPERV_SMB_USERNAME'] : params['hyperv_smb_username']
-hyperv_smb_password = params['hyperv_smb_password'].nil? ? ENV['VAGRANT_HYPERV_SMB_PASSWORD'] : params['hyperv_smb_password']
-box_url             = params['box_url'].nil? ? ENV['VAGRANT_BOX_URL'] : params['box_url']
-password            = params['password'].nil? ? ENV['VAGRANT_PASSWORD'] : params['password']
+enable_synced_folder = params['enable_synced_folder'].nil? ? ENV.fetch('VAGRANT_ENABLE_SYNCED_FOLDER', nil) : params['enable_synced_folder']
+enable_synced_folder = enable_synced_folder.casecmp('true').zero? if enable_synced_folder.is_a?(String)
+provider            = params['provider'].nil? ? ENV.fetch('VAGRANT_PROVIDER', nil) : params['provider']
+cpus                = params['cpus'].nil? ? ENV.fetch('VAGRANT_CPUS', nil) : params['cpus']
+memory              = params['memory'].nil? ? ENV.fetch('VAGRANT_MEMORY', nil) : params['memory']
+hyperv_vswitch      = params['hyperv_vswitch'].nil? ? ENV.fetch('VAGRANT_HYPERV_VSWITCH', nil) : params['hyperv_vswitch']
+hyperv_smb_username = params['hyperv_smb_username'].nil? ? ENV.fetch('VAGRANT_HYPERV_SMB_USERNAME', nil) : params['hyperv_smb_username']
+hyperv_smb_password = params['hyperv_smb_password'].nil? ? ENV.fetch('VAGRANT_HYPERV_SMB_PASSWORD', nil) : params['hyperv_smb_password']
+box_url             = params['box_url'].nil? ? ENV.fetch('VAGRANT_BOX_URL', nil) : params['box_url']
+password            = params['password'].nil? ? ENV.fetch('VAGRANT_PASSWORD', nil) : params['password']
 raise 'specify a node_name when tearing down' if action == 'tear_down' && node_name.nil?
 raise 'specify a platform when provisioning' if action == 'provision' && platform.nil?