Fix the latest Chefstyle warnings

Signed-off-by: Tim Smith <[email protected]>
chef · Feb 20, 2020 · 86598fc · 86598fc
1 parent 108ca08
commit 86598fc
Show file tree

Hide file tree

Showing 30 changed files with 128 additions and 95 deletions.
diff --git a/.rubocop.yml b/.rubocop.yml
@@ -0,0 +1,2 @@
+AllCops:
+  TargetRubyVersion: 2.6
diff --git a/Rakefile b/Rakefile
@@ -39,4 +39,4 @@ task :console do
   IRB.start
 end
 
-task default: [:style, :spec]
+task default: %i{style spec}
diff --git a/lib/chef/resource/chef_acl.rb b/lib/chef/resource/chef_acl.rb
@@ -99,6 +99,7 @@ def create_acl(path)
                     # NOTE: if superusers exist, this should turn into a warning.
                     raise "chef_acl attempted to remove all actors from GRANT!  I'm sorry Dave, I can't let you remove access to an object with no hope of recovery."
                   end
+
                   modify[differences] ||= {}
                   modify[differences][permission] = desired_json
                 end
@@ -107,7 +108,7 @@ def create_acl(path)
               if modify.size > 0
                 changed = true
                 description = [ "update acl #{path} at #{rest_url(path)}" ] + modify.flat_map do |diffs, permissions|
-                  diffs.map { |diff| "  #{permissions.keys.join(', ')}:#{diff}" }
+                  diffs.map { |diff| "  #{permissions.keys.join(", ")}:#{diff}" }
                 end
                 converge_by description do
                   modify.values.each do |permissions|
@@ -127,11 +128,13 @@ def create_acl(path)
             children, _error = list(path, "*")
             Chef::ChefFS::Parallelizer.parallel_do(children) do |child|
               next if child.split("/")[-1] == "containers"
+
               create_acl(child)
             end
             # containers mess up our descent, so we do them last
             Chef::ChefFS::Parallelizer.parallel_do(children) do |child|
               next if child.split("/")[-1] != "containers"
+
               create_acl(child)
             end
 
@@ -141,7 +144,7 @@ def create_acl(path)
         # Get the current ACL for the given path
         def current_acl(acl_path)
           @current_acls ||= {}
-          if !@current_acls.key?(acl_path)
+          unless @current_acls.key?(acl_path)
             @current_acls[acl_path] = begin
               rest.get(rest_url(acl_path))
                                       rescue Net::HTTPServerException => e
@@ -252,8 +255,7 @@ def remove_rights(json)
           end
         end
 
-        def load_current_resource
-        end
+        def load_current_resource; end
 
         #
         # Matches chef_acl paths like nodes, nodes/*.
@@ -270,7 +272,7 @@ def load_current_resource
         def match_paths(path)
           # Turn multiple slashes into one
           # nodes//x -> nodes/x
-          path = path.gsub(/[\/]+/, "/")
+          path = path.gsub(%r{[/]+}, "/")
           # If it's absolute, start the matching with /.  If it's relative, start with '' (relative root).
           if path[0] == "/"
             matches = [ "/" ]
@@ -305,6 +307,7 @@ def match_paths(path)
                 if parts[0..index - 1].all? { |p| p != "*" }
                   raise error
                 end
+
                 []
               else
                 found
@@ -421,21 +424,21 @@ def list(path, child)
             when 1
               # /organizations/*, /users/*, roles/*, nodes/*, etc.
               results, error = rest_list(path)
-              if !error
+              unless error
                 results = results.map { |result| ::File.join(path, result) }
               end
 
             when 2
               # /organizations/NAME/*
               results, error = rest_list(::File.join(path, "containers"))
-              if !error
+              unless error
                 results = results.map { |result| ::File.join(path, result) }
               end
 
             when 3
               # /organizations/NAME/TYPE/*
               results, error = rest_list(path)
-              if !error
+              unless error
                 results = results.map { |result| ::File.join(path, result) }
               end
             end

diff --git a/lib/chef/resource/chef_client.rb b/lib/chef/resource/chef_client.rb
@@ -18,7 +18,7 @@ class ChefClient < Cheffish::ChefActorBase
 
       # Output public key (if so desired)
       property :output_key_path, String
-      property :output_key_format, Symbol, default: :openssh, equal_to: [ :pem, :der, :openssh ]
+      property :output_key_format, Symbol, default: :openssh, equal_to: %i{pem der openssh}
 
       # Proc that runs just before the resource executes.  Called with (resource)
       def before(&block)

diff --git a/lib/chef/resource/chef_container.rb b/lib/chef/resource/chef_container.rb
@@ -10,7 +10,7 @@ class ChefContainer < Cheffish::BaseResource
       property :chef_container_name, Cheffish::NAME_REGEX, name_property: true
 
       action :create do
-        if !@current_exists
+        unless @current_exists
           converge_by "create container #{new_resource.chef_container_name} at #{rest.url}" do
             rest.post("containers", normalize_for_post(new_json))
           end

diff --git a/lib/chef/resource/chef_data_bag.rb b/lib/chef/resource/chef_data_bag.rb
@@ -9,7 +9,7 @@ class ChefDataBag < Cheffish::BaseResource
       property :data_bag_name, Cheffish::NAME_REGEX, name_property: true
 
       action :create do
-        if !current_resource_exists?
+        unless current_resource_exists?
           converge_by "create data bag #{new_resource.data_bag_name} at #{rest.url}" do
             rest.post("data", { "name" => new_resource.data_bag_name })
           end

diff --git a/lib/chef/resource/chef_data_bag_item.rb b/lib/chef/resource/chef_data_bag_item.rb
@@ -137,7 +137,7 @@ def load_current_resource
 
               # If the current secret doesn't work, look through the specified old secrets
 
-              if !current_resource.secret
+              unless current_resource.secret
                 old_secrets = []
                 if new_resource.old_secret
                   old_secrets += Array(new_resource.old_secret)
@@ -148,17 +148,17 @@ def load_current_resource
                   end
                 end
                 old_secrets.each do |secret|
-                  begin
-                    Chef::EncryptedDataBagItem::Decryptor.for(first_real_value, secret).for_decrypted_item
-                    current_resource.secret secret
-                  rescue Chef::EncryptedDataBagItem::DecryptionFailure
-                    decrypt_error = $!
-                  end
+
+                  Chef::EncryptedDataBagItem::Decryptor.for(first_real_value, secret).for_decrypted_item
+                  current_resource.secret secret
+                rescue Chef::EncryptedDataBagItem::DecryptionFailure
+                  decrypt_error = $!
+
                 end
 
                 # If we couldn't figure out the secret, emit a warning (this isn't a fatal flaw unless we
                 # need to reuse one of the values from the data bag)
-                if !current_resource.secret
+                unless current_resource.secret
                   if decrypt_error
                     Chef::Log.warn "Existing data bag is encrypted, but could not decrypt: #{decrypt_error.message}."
                   else
@@ -280,9 +280,10 @@ def calculate_differences
             elsif current_resource.encrypt
               # Encryption is different and we can't read the old values.  Only allow the change
               # if we're overwriting the data bag item
-              if !new_resource.complete
+              unless new_resource.complete
                 raise "Cannot encrypt #{new_resource.name} due to failure to decrypt existing resource.  Set 'complete true' to overwrite or add the old secret as old_secret / old_secret_path."
               end
+
               _differences = [ "overwrite data bag item (cannot decrypt old data bag item)"]
               differences = (new_resource.raw_data.keys & current_resource.raw_data.keys).map { |key| "overwrite #{key}" }
               differences += (new_resource.raw_data.keys - current_resource.raw_data.keys).map { |key| "add #{key}" }

diff --git a/lib/chef/resource/chef_mirror.rb b/lib/chef/resource/chef_mirror.rb
@@ -95,6 +95,7 @@ def copy_to(src_root, dest_root)
           if new_resource.concurrency <= 0
             raise "chef_mirror.concurrency must be above 0!  Was set to #{new_resource.concurrency}"
           end
+
           # Honor concurrency
           Chef::ChefFS::Parallelizer.threads = new_resource.concurrency - 1
 
@@ -103,13 +104,14 @@ def copy_to(src_root, dest_root)
           if new_resource.path[0] == "/"
             raise "Absolute paths in chef_mirror not yet supported."
           end
+
           # Copy!
           path = Chef::ChefFS::FilePattern.new("/#{new_resource.path}")
           ui = CopyListener.new(self)
           error = Chef::ChefFS::FileSystem.copy_to(path, src_root, dest_root, nil, options, ui, proc { |p| p.path })
 
           if error
-            raise "Errors while copying:#{ui.errors.map { |e| "#{e}\n" }.join('')}"
+            raise "Errors while copying:#{ui.errors.map { |e| "#{e}\n" }.join("")}"
           end
         end
 
@@ -163,7 +165,7 @@ def remote_fs
         end
 
         def repo_mode
-          new_resource.chef_server[:chef_server_url] =~ /\/organizations\// ? "hosted_everything" : "everything"
+          new_resource.chef_server[:chef_server_url] =~ %r{/organizations/} ? "hosted_everything" : "everything"
         end
 
         def options
@@ -179,8 +181,7 @@ def options
           result
         end
 
-        def load_current_resource
-        end
+        def load_current_resource; end
 
         class CopyListener
           def initialize(mirror)

diff --git a/lib/chef/resource/chef_organization.rb b/lib/chef/resource/chef_organization.rb
@@ -68,7 +68,7 @@ class ChefOrganization < Cheffish::BaseResource
           end
         end
         new_resource.members.each do |user|
-          if !existing_members.include?(user)
+          unless existing_members.include?(user)
             converge_by "Add #{user} to organization #{new_resource.organization_name}" do
               rest.post("#{rest.root_url}/organizations/#{new_resource.organization_name}/users/", { "username" => user })
             end

diff --git a/lib/chef/resource/chef_resolved_cookbooks.rb b/lib/chef/resource/chef_resolved_cookbooks.rb
@@ -43,7 +43,8 @@ def cookbooks_from(path = nil)
           new_resource.berksfile.upload(
             server_url: new_resource.chef_server[:chef_server_url],
             client_name: new_resource.chef_server[:options][:client_name],
-            client_key: new_resource.chef_server[:options][:signing_key_filename])
+            client_key: new_resource.chef_server[:options][:signing_key_filename]
+          )
         else
           file = Tempfile.new("privatekey")
           begin
@@ -53,7 +54,8 @@ def cookbooks_from(path = nil)
             new_resource.berksfile.upload(
               server_url: new_resource.chef_server[:chef_server_url],
               client_name: new_resource.chef_server[:options][:client_name] || "me",
-              client_key: file.path)
+              client_key: file.path
+            )
 
           ensure
             file.close

diff --git a/lib/chef/resource/chef_role.rb b/lib/chef/resource/chef_role.rb
@@ -62,6 +62,7 @@ def recipe(*recipes)
         if recipes.size == 0
           raise ArgumentError, "At least one recipe must be specified"
         end
+
         @run_list_modifiers ||= []
         @run_list_modifiers += recipes.map { |recipe| Chef::RunList::RunListItem.new("recipe[#{recipe}]") }
       end
@@ -70,6 +71,7 @@ def role(*roles)
         if roles.size == 0
           raise ArgumentError, "At least one role must be specified"
         end
+
         @run_list_modifiers ||= []
         @run_list_modifiers += roles.map { |role| Chef::RunList::RunListItem.new("role[#{role}]") }
       end
@@ -78,6 +80,7 @@ def remove_recipe(*recipes)
         if recipes.size == 0
           raise ArgumentError, "At least one recipe must be specified"
         end
+
         @run_list_removers ||= []
         @run_list_removers += recipes.map { |recipe| Chef::RunList::RunListItem.new("recipe[#{recipe}]") }
       end
@@ -86,6 +89,7 @@ def remove_role(*roles)
         if roles.size == 0
           raise ArgumentError, "At least one role must be specified"
         end
+
         @run_list_removers ||= []
         @run_list_removers += roles.map { |recipe| Chef::RunList::RunListItem.new("role[#{role}]") }
       end

diff --git a/lib/chef/resource/chef_user.rb b/lib/chef/resource/chef_user.rb
@@ -25,7 +25,7 @@ class ChefUser < Cheffish::ChefActorBase
 
       # Output public key (if so desired)
       property :output_key_path, String
-      property :output_key_format, [ :pem, :der, :openssh ], default: :openssh
+      property :output_key_format, %i{pem der openssh}, default: :openssh
 
       # Proc that runs just before the resource executes.  Called with (resource)
       def before(&block)

diff --git a/lib/chef/resource/private_key.rb b/lib/chef/resource/private_key.rb
@@ -13,11 +13,11 @@ class PrivateKey < Cheffish::BaseResource
 
       # Path to private key.  Set to :none to create the key in memory and not on disk.
       property :path, [ String, :none ], name_property: true
-      property :format, [ :pem, :der ], default: :pem
-      property :type, [ :rsa, :dsa ], default: :rsa # TODO support :ec
+      property :format, %i{pem der}, default: :pem
+      property :type, %i{rsa dsa}, default: :rsa # TODO support :ec
       # These specify an optional public_key you can spit out if you want.
       property :public_key_path, String
-      property :public_key_format, [ :openssh, :pem, :der ], default: :openssh
+      property :public_key_format, %i{openssh pem der}, default: :openssh
       # Specify this if you want to copy another private key but give it a different format / password
       property :source_key
       property :source_key_path, String
@@ -31,7 +31,7 @@ class PrivateKey < Cheffish::BaseResource
 
       # PEM-only
       property :pass_phrase, String
-      property :cipher, String, equal_to: OpenSSL::Cipher.ciphers.map { |x| x.downcase }, default: "des-ede3-cbc", coerce: proc { |x| x.downcase }
+      property :cipher, String, equal_to: OpenSSL::Cipher.ciphers.map(&:downcase), default: "des-ede3-cbc", coerce: proc { |x| x.downcase }
 
       # Set this to regenerate the key if it does not have the desired characteristics (like size, type, etc.)
       property :regenerate_if_different, [TrueClass, FalseClass]

diff --git a/lib/chef/resource/public_key.rb b/lib/chef/resource/public_key.rb
@@ -12,7 +12,7 @@ class PublicKey < Cheffish::BaseResource
       default_action :create
 
       property :path, String, name_property: true
-      property :format, [ :pem, :der, :openssh ], default: :openssh
+      property :format, %i{pem der openssh}, default: :openssh
 
       property :source_key
       property :source_key_path, String
@@ -24,9 +24,10 @@ def load_prior_resource(*args)
       end
 
       action :create do
-        if !new_source_key
+        unless new_source_key
           raise "No source key specified"
         end
+
         desired_output = encode_public_key(new_source_key)
         if Array(current_resource.action) == [ :delete ] || desired_output != IO.read(new_resource.path)
           converge_by "write #{new_resource.format} public key #{new_resource.path} from #{new_source_key_publicity} key #{new_resource.source_key_path}" do

diff --git a/lib/cheffish.rb b/lib/cheffish.rb
@@ -98,6 +98,7 @@ def self.get_private_key_with_path(name, config = profiled_config)
     elsif config[:private_key_paths]
       config[:private_key_paths].each do |private_key_path|
         next unless File.exist?(private_key_path)
+
         Dir.entries(private_key_path).sort.each do |key|
           ext = File.extname(key)
           if key == name || ext == "" || ext == ".pem"