Fix all remaining rubocop & foodcritic errors

Fix all remaining rubocop & foodcritic errors, fixes sous-chefs#365.

Author: martinb3

.rubocop.yml

@@ -16,6 +16,10 @@ Encoding:
 LineLength:
   Max: 180
 
+# Longer classes aren't _so_ bad
+Metrics/ClassLength:
+  Max: 125
+
 # Increase allowed lines in a method.  Short methods are good, but 10 lines
 #   is a bit too low.
 MethodLength:
@@ -30,9 +34,12 @@ RedundantReturn:
 IfUnlessModifier:
   Enabled: false
 
-# Raise allowed CyclomaticComplexity to 10.
+# Raise allowed CyclomaticComplexity & Perceivedto 10.
 CyclomaticComplexity:
-  Max: 10
+  Max: 15
+
+Metrics/PerceivedComplexity:
+  Max: 15
 
 # Disable Single Space before first arg
 SingleSpaceBeforeFirstArg:
@@ -46,6 +53,10 @@ WordArray:
 UnusedBlockArgument:
   Enabled: false
 
+# allow both hash syntaxes
+Style/HashSyntax:
+  Enabled: false
+
 # There are too many non-ruby files that run up against rubocop rules in a cookbook
 AllCops:
   Include:

Rakefile

@@ -66,6 +66,6 @@ rescue Exception => e
   if e.class.to_s =~ /^Timeout::|^Kitchen::|LoadError/
     STDERR.puts "[!] Omitting Kitchen tasks [#{e.class}: #{e.message} at #{e.backtrace.first}]\n\n"
   else
-    raise e
+    fail e
   end
 end

attributes/default.rb

@@ -3,9 +3,9 @@
 default['elasticsearch']['install_type'] = 'tarball'
 
 # platform_family keyed download URLs
-default['elasticsearch']['download_urls']['debian'] = "https://download.elasticsearch.org/elasticsearch/elasticsearch/elasticsearch-%s.deb"
-default['elasticsearch']['download_urls']['rhel'] = "https://download.elasticsearch.org/elasticsearch/elasticsearch/elasticsearch-%s.noarch.rpm"
-default['elasticsearch']['download_urls']['tar'] = "https://download.elasticsearch.org/elasticsearch/elasticsearch/elasticsearch-%s.tar.gz"
+default['elasticsearch']['download_urls']['debian'] = 'https://download.elasticsearch.org/elasticsearch/elasticsearch/elasticsearch-%s.deb'
+default['elasticsearch']['download_urls']['rhel'] = 'https://download.elasticsearch.org/elasticsearch/elasticsearch/elasticsearch-%s.noarch.rpm'
+default['elasticsearch']['download_urls']['tar'] = 'https://download.elasticsearch.org/elasticsearch/elasticsearch/elasticsearch-%s.tar.gz'
 
 # platform_family keyed download checksums
 default['elasticsearch']['checksums']['1.4.5']['debian'] = '68dce951181e9802e94fd83b894f4b628394fc44bb01c77eb61fdbd1940d94b5'

libraries/helpers.rb

@@ -1,13 +1,13 @@
 module ElasticsearchCookbook
+  # Helper methods included by various providers and passed to the template engine
   module Helpers
-
     def determine_version(new_resource, node)
       if new_resource.version
         new_resource.version.to_s
       elsif node['elasticsearch'] && node['elasticsearch']['version']
         node['elasticsearch']['version'].to_s
       else
-        raise 'could not determine version of elasticsearch to install'
+        fail 'could not determine version of elasticsearch to install'
       end
     end
 
@@ -17,7 +17,7 @@ def determine_install_type(new_resource, node)
       elsif node['elasticsearch'] && node['elasticsearch']['install_type']
         node['elasticsearch']['install_type'].to_s
       else
-        raise 'could not determine how to install elasticsearch (package? tarball?)'
+        fail 'could not determine how to install elasticsearch (package? tarball?)'
       end
     end
 
@@ -57,15 +57,15 @@ def determine_download_checksum(new_resource, node)
     end
 
     def get_tarball_home_dir(new_resource, node)
-      new_resource.dir || node.ark[:prefix_home]
+      new_resource.dir || node['ark']['prefix_home']
     end
 
     def get_tarball_root_dir(new_resource, node)
-      new_resource.dir || node.ark[:prefix_root]
+      new_resource.dir || node['ark']['prefix_root']
     end
 
     # This method takes a hash, but will convert to mash
-    def print_value(data, key, options={})
+    def print_value(data, key, options = {})
       separator = options[:separator] || ': '
 
       final_value = format_value(find_value(data, key))
@@ -86,16 +86,15 @@ def find_value(data, key)
     end
 
     def format_value(value)
-      unless value.nil?
-        if value.is_a?(Array)
-          value.join(',').to_s
-        elsif value.respond_to?(:empty?) && value.empty?
-          nil # anything that answers to empty? should be nil again
-        else
-          value.to_s
-        end
+      if value.nil?
+        nil # just pass through nil
+      elsif value.is_a?(Array)
+        value.join(',').to_s
+      elsif value.respond_to?(:empty?) && value.empty?
+        nil # anything that answers to empty? should be nil again
+      else
+        value.to_s
       end
     end
-
   end
 end

libraries/matchers.rb

@@ -1,33 +1,42 @@
 # ChefSpec is a tool to unit test cookbooks in conjunction with rspec
-# Learn more on the README or at https://github.com/sethvargo/chefspec. 
+# Learn more on the README or at https://github.com/sethvargo/chefspec.
 if defined?(ChefSpec)
   def create_elasticsearch_user(resource_name)
     ChefSpec::Matchers::ResourceMatcher.new(:elasticsearch_user, :create, resource_name)
   end
+
   def remove_elasticsearch_user(resource_name)
     ChefSpec::Matchers::ResourceMatcher.new(:elasticsearch_user, :remove, resource_name)
   end
+
   def install_elasticsearch(resource_name)
     ChefSpec::Matchers::ResourceMatcher.new(:elasticsearch_install, :install, resource_name)
   end
+
   def remove_elasticsearch(resource_name)
     ChefSpec::Matchers::ResourceMatcher.new(:elasticsearch_install, :remove, resource_name)
   end
+
   def manage_elasticsearch_configure(resource_name)
     ChefSpec::Matchers::ResourceMatcher.new(:elasticsearch_configure, :manage, resource_name)
   end
+
   def remove_elasticsearch_configure(resource_name)
     ChefSpec::Matchers::ResourceMatcher.new(:elasticsearch_configure, :remove, resource_name)
   end
+
   def configure_elasticsearch_service(resource_name)
     ChefSpec::Matchers::ResourceMatcher.new(:elasticsearch_service, :configure, resource_name)
   end
+
   def remove_elasticsearch_service(resource_name)
     ChefSpec::Matchers::ResourceMatcher.new(:elasticsearch_service, :remove, resource_name)
   end
+
   def install_elasticsearch_plugin(resource_name)
     ChefSpec::Matchers::ResourceMatcher.new(:elasticsearch_plugin, :install, resource_name)
   end
+
   def remove_elasticsearch_plugin(resource_name)
     ChefSpec::Matchers::ResourceMatcher.new(:elasticsearch_plugin, :remove, resource_name)
   end

libraries/provider_configure.rb

@@ -5,8 +5,6 @@ class Provider::ElasticsearchConfigure < Chef::Provider::LWRPBase
 
     action :manage do
       converge_by('configure elasticsearch instance') do
-        touched_resources = []
-
         # if a subdir parameter is missing but dir is set, infer the subdir name
         # then go and be sure it's also set in the YML hash if it wasn't given there
         if new_resource.dir && !new_resource.path_conf
@@ -32,13 +30,14 @@ class Provider::ElasticsearchConfigure < Chef::Provider::LWRPBase
 
         # calculation for memory allocation; 50% or 31g, whatever is smaller
         unless new_resource.allocated_memory
-          half = ((node.memory.total.to_i * 0.5 ).floor / 1024)
-          new_resource.allocated_memory (half > 31000 ? "31g" : "#{half}m")
+          half = ((node['memory']['total'].to_i * 0.5).floor / 1024)
+          malloc_str = (half > 31_000 ? '31g' : "#{half}m")
+          new_resource.allocated_memory malloc_str
         end
 
         # Create ES directories
         #
-        [ new_resource.path_conf, new_resource.path_logs ].each do |path|
+        [new_resource.path_conf, new_resource.path_logs].each do |path|
           d = directory path do
             owner new_resource.user
             group new_resource.group
@@ -66,39 +65,37 @@ class Provider::ElasticsearchConfigure < Chef::Provider::LWRPBase
           new_resource.updated_by_last_action(true) if d.updated_by_last_action?
         end
 
-        shell_template = template "elasticsearch.in.sh" do
+        shell_template = template 'elasticsearch.in.sh' do
           path "#{new_resource.path_conf}/elasticsearch.in.sh"
           source new_resource.template_elasticsearch_env
           cookbook 'elasticsearch'
           owner new_resource.user
           group new_resource.group
           mode 0755
-          variables({
-            java_home: new_resource.java_home,
-            es_home: new_resource.es_home,
-            es_config: new_resource.path_conf,
-            allocated_memory: new_resource.allocated_memory,
-            Xms: new_resource.allocated_memory,
-            Xmx: new_resource.allocated_memory,
-            Xss: new_resource.thread_stack_size,
-            gc_settings: new_resource.gc_settings,
-            env_options: new_resource.env_options
-            })
+          variables(java_home: new_resource.java_home,
+                    es_home: new_resource.es_home,
+                    es_config: new_resource.path_conf,
+                    allocated_memory: new_resource.allocated_memory,
+                    Xms: new_resource.allocated_memory,
+                    Xmx: new_resource.allocated_memory,
+                    Xss: new_resource.thread_stack_size,
+                    gc_settings: new_resource.gc_settings,
+                    env_options: new_resource.env_options)
           action :nothing
         end
         shell_template.run_action(:create)
         new_resource.updated_by_last_action(true) if shell_template.updated_by_last_action?
 
         # Create ES logging file
         #
-        logging_template = template "logging.yml" do
+        logging_template = template 'logging.yml' do
           path   "#{new_resource.path_conf}/logging.yml"
           source new_resource.template_logging_yml
           cookbook 'elasticsearch'
           owner new_resource.user
           group new_resource.group
           mode 0755
-          variables({logging: new_resource.logging})
+          variables(logging: new_resource.logging)
           action :nothing
         end
         logging_template.run_action(:create)
@@ -113,22 +110,20 @@ class Provider::ElasticsearchConfigure < Chef::Provider::LWRPBase
           Chef::Log.warn("Please change the following to strings in order to work with this Elasticsearch cookbook: #{found_symbols.join(',')}")
         end
 
-        yml_template = template "elasticsearch.yml" do
+        yml_template = template 'elasticsearch.yml' do
           path "#{new_resource.path_conf}/elasticsearch.yml"
           source new_resource.template_elasticsearch_yml
           cookbook 'elasticsearch'
           owner new_resource.user
           group new_resource.group
           mode 0755
           helpers(ElasticsearchCookbook::Helpers)
-          variables({
-            config: merged_configuration
-            })
+          variables(config: merged_configuration)
           action :nothing
         end
         yml_template.run_action(:create)
         new_resource.updated_by_last_action(true) if yml_template.updated_by_last_action?
       end
     end
-   end
- end
+  end
+end

libraries/provider_install.rb

@@ -15,7 +15,7 @@ class Provider::ElasticsearchInstall < Chef::Provider::LWRPBase
         elsif install_type == 'package'
           install_package_wrapper_action
         else
-          raise "#{install_type} is not a valid install type"
+          fail "#{install_type} is not a valid install type"
         end
       end
     end
@@ -28,7 +28,7 @@ class Provider::ElasticsearchInstall < Chef::Provider::LWRPBase
         elsif install_type == 'package'
           remove_package_wrapper_action
         else
-          raise "#{install_type} is not a valid install type"
+          fail "#{install_type} is not a valid install type"
         end
       end
     end
@@ -90,7 +90,7 @@ def install_tarball_wrapper_action
       include_recipe 'ark'
       include_recipe 'curl'
 
-      ark_r = ark "elasticsearch" do
+      ark_r = ark 'elasticsearch' do
         url   determine_download_url(new_resource, node)
         owner new_resource.owner
         group new_resource.group
@@ -105,7 +105,7 @@ def install_tarball_wrapper_action
           target = "#{new_resource.dir}/elasticsearch-#{determine_version(new_resource, node)}"
           binary = "#{target}/bin/elasticsearch"
 
-          ::File.directory?(link) && ::File.symlink?(link) && ::File.readlink(link) == target && ::File.exists?(binary)
+          ::File.directory?(link) && ::File.symlink?(link) && ::File.readlink(link) == target && ::File.exist?(binary)
         end
         action :nothing
       end

libraries/provider_plugin.rb

@@ -1,21 +1,25 @@
 
 class Chef
+  # Chef Provider for installing an elasticsearch plugin
   class Provider::ElasticsearchPlugin < Chef::Provider::LWRPBase
     include ElasticsearchCookbook::Helpers
     include Chef::Mixin::ShellOut
 
     action :install do
       name    = new_resource.plugin_name
       version = new_resource.version ? "/#{new_resource.version}" : nil
-      url     = new_resource.url     ? " -url #{new_resource.url}" : nil
+      url     = new_resource.url ? " -url #{new_resource.url}" : nil
 
-      raise 'Could not determine the plugin directory. Please set plugin_dir on this resource.' unless new_resource.plugin_dir
+      fail 'Could not determine the plugin directory. Please set plugin_dir on this resource.' unless new_resource.plugin_dir
       converge_by("install plugin #{name}") do
-
-        plugin_exists = Dir.entries(new_resource.plugin_dir).any? do |plugin|
-          next if plugin =~ /^\./
-          name.include? plugin
-        end rescue false
+        plugin_exists = begin
+                          Dir.entries(new_resource.plugin_dir).any? do |plugin|
+                            next if plugin =~ /^\./
+                            name.include? plugin
+                          end
+                        rescue
+                          false
+                        end
 
         unless plugin_exists
           # automatically raises on error, logs command output