Merge pull request #403 from legal90/cookstyle

Fix cookstyle and foodcritic offences

.foodcritic

@@ -1 +1,2 @@
--t ~FC001 -t ~FC054
+~FC019
+~FC044

.rubocop.yml

@@ -1,48 +1,5 @@
 ---
-AllCops:
-  Exclude:
-    - 'Rakefile'
-    - 'Vagrantfile'
-    - 'Policyfile.rb'
-    - 'Berksfile'
-    - 'Gemfile'
-    - 'metadata.rb'
-    - 'test/**/*'
-    - 'bin/**'
-    - 'vendor/**/*'
-AlignParameters:
-  Enabled: false
-ClassLength:
-  Enabled: false
-CyclomaticComplexity:
-  Enabled: false
-Documentation:
-  Enabled: false
-Encoding:
-  Enabled: false
-Style/FileName:
-  Enabled: false
-LineLength:
-  Enabled: false
-MethodLength:
-  Enabled: false
-Metrics/AbcSize:
-  Enabled: false
-PerceivedComplexity:
-  Enabled: false
-Style/SpaceBeforeFirstArg:
-  Enabled: false
-Style/ClassAndModuleChildren:
-  Enabled: false
-Style/FileName:
-  Enabled: false
 Style/GuardClause:
   Enabled: false
-Style/PercentLiteralDelimiters:
-  Enabled: false
 Style/ModuleFunction:
   Enabled: false
-Style/AlignHash:
-  Enabled: false
-Style/SpaceInsideHashLiteralBraces:
-  Enabled: false

Rakefile

@@ -19,7 +19,7 @@ namespace :style do
     FoodCritic::Rake::LintTask.new(:chef) do |t|
       t.options = {
         fail_tags: ['any'],
-        progress: true
+        progress: true,
       }
     end
   rescue LoadError

libraries/consul_acl.rb

@@ -33,7 +33,7 @@ class ConsulAcl < Chef::Resource
 
       # @!attribute type
       # @return [String]
-      attribute(:type, equal_to: %w{client management}, default: 'client')
+      attribute(:type, equal_to: %w(client management), default: 'client')
 
       # @!attribute rules
       # @return [String]

libraries/consul_config.rb

@@ -176,8 +176,8 @@ def to_json
           watches
         )
 
-        for_keeps << %i{bootstrap bootstrap_expect} if server
-        for_keeps << %i{ca_file cert_file key_file} if tls?
+        for_keeps << %i(bootstrap bootstrap_expect) if server
+        for_keeps << %i(ca_file cert_file key_file) if tls?
         for_keeps = for_keeps.flatten
 
         config = to_hash.keep_if do |k, _|
@@ -195,7 +195,7 @@ def tls?
           [::File.dirname(new_resource.path), new_resource.config_dir].each do |dir|
             directory dir do
               recursive true
-              unless node.platform?('windows')
+              unless platform?('windows')
                 owner new_resource.owner
                 group new_resource.group
                 mode '0755'
@@ -205,7 +205,7 @@ def tls?
           end
 
           file new_resource.path do
-            unless node.platform?('windows')
+            unless platform?('windows')
               owner new_resource.owner
               group new_resource.group
               mode '0640'

libraries/consul_definition.rb

@@ -29,7 +29,7 @@ class ConsulDefinition < Chef::Resource
 
       # @!attribute type
       # @return [String]
-      attribute(:type, equal_to: %w{check service checks services})
+      attribute(:type, equal_to: %w(check service checks services))
 
       # @!attribute parameters
       # @return [Hash]
@@ -45,7 +45,7 @@ def to_json
         notifying_block do
           directory ::File.dirname(new_resource.path) do
             recursive true
-            unless node.platform?('windows')
+            unless platform?('windows')
               owner new_resource.user
               group new_resource.group
               mode '0755'
@@ -57,7 +57,7 @@ def to_json
 
           file new_resource.path do
             content new_resource.to_json
-            unless node.platform?('windows')
+            unless platform?('windows')
               owner new_resource.user
               group new_resource.group
               mode '0640'

libraries/consul_installation_binary.rb

@@ -32,7 +32,7 @@ def self.provides_auto?(_node, _resource)
       # @return [Hash]
       # @api private
       def self.default_inversion_options(node, resource)
-        extract_path = node.windows? ? node.config_prefix_path : '/opt/consul'
+        extract_path = node.platform_family?('windows') ? node.config_prefix_path : '/opt/consul'
         super.merge(extract_to: extract_path,
                     version: resource.version,
                     archive_url: 'https://releases.hashicorp.com/consul/%{version}/%{basename}',
@@ -47,7 +47,7 @@ def action_create
             recursive true
           end
 
-          url = options[:archive_url] % {version: options[:version], basename: options[:archive_basename]}
+          url = options[:archive_url] % { version: options[:version], basename: options[:archive_basename] }
           poise_archive url do
             destination join_path(options[:extract_to], new_resource.version)
             source_properties checksum: options[:archive_checksum]

libraries/consul_service.rb

@@ -90,7 +90,7 @@ def service_options(service)
         service.options(:sysvinit, template: 'consul:sysvinit.service.erb')
         service.options(:upstart, template: 'consul:upstart.service.erb', executable: new_resource.program)
 
-        if node.platform_family?('rhel') && node.platform_version.to_i == 6
+        if platform_family?('rhel') && node['platform_version'].to_i == 6
           service.provider(:sysvinit)
         end
       end

libraries/consul_service_windows.rb

@@ -14,15 +14,15 @@ module Provider
     class ConsulServiceWindows < Chef::Provider
       include Poise
       include Chef::Mixin::ShellOut
-      provides(:consul_service, os: %w{windows})
+      provides(:consul_service, os: %w(windows))
       include ConsulCookbook::Helpers
 
       def action_enable
         notifying_block do
-          directories = %W{#{new_resource.data_dir}
+          directories = %W(#{new_resource.data_dir}
                            #{new_resource.config_dir}
                            #{::File.dirname(new_resource.nssm_params['AppStdout'])}
-                           #{::File.dirname(new_resource.nssm_params['AppStderr'])}}.uniq.compact
+                           #{::File.dirname(new_resource.nssm_params['AppStderr'])}).uniq.compact
           directories.delete_if { |i| i.eql? '.' }.each do |dirname|
             directory dirname do
               recursive true
@@ -54,10 +54,9 @@ def action_enable
             end
             # Check if the service is running, but don't bother if we're already
             # changing some nssm parameters
-            unless nssm_service_status?(%w{SERVICE_RUNNING}) && mismatch_params.empty?
-              powershell_script 'Trigger consul restart' do
-                code 'restart-service consul'
-              end
+            powershell_script 'Trigger consul restart' do
+              code 'restart-service consul'
+              not_if { nssm_service_status?(%w(SERVICE_RUNNING)) && mismatch_params.empty? }
             end
           end
         end
@@ -87,7 +86,7 @@ def action_disable
           powershell_script 'Stop consul' do
             action :run
             code 'stop-service consul'
-            only_if { nssm_service_installed? && nssm_service_status?(%w{SERVICE_RUNNING SERVICE_PAUSED}) }
+            only_if { nssm_service_installed? && nssm_service_status?(%w(SERVICE_RUNNING SERVICE_PAUSED)) }
           end
 
           nssm 'consul' do

libraries/consul_watch.rb

@@ -29,7 +29,7 @@ class ConsulWatch < Chef::Resource
 
       # @!attribute type
       # @return [String]
-      attribute(:type, equal_to: %w{checks event key keyprefix nodes service services})
+      attribute(:type, equal_to: %w(checks event key keyprefix nodes service services))
 
       # @!attribute parameters
       # @return [Hash]
@@ -43,7 +43,7 @@ def to_json
         notifying_block do
           directory ::File.dirname(new_resource.path) do
             recursive true
-            unless node.platform?('windows')
+            unless platform?('windows')
               owner new_resource.user
               group new_resource.group
               mode '0755'
@@ -52,7 +52,7 @@ def to_json
 
           file new_resource.path do
             content new_resource.to_json
-            unless node.platform?('windows')
+            unless platform?('windows')
               owner new_resource.user
               group new_resource.group
               mode '0640'

libraries/helpers.rb

@@ -46,7 +46,7 @@ def data_path
 
     def command(config_file, config_dir)
       if windows?
-        %{agent -config-file="""#{config_file}""" -config-dir="""#{config_dir}"""}
+        %(agent -config-file="""#{config_file}""" -config-dir="""#{config_dir}""")
       else
         "/usr/local/bin/consul agent -config-file=#{config_file} -config-dir=#{config_dir}"
       end
@@ -57,7 +57,7 @@ def nssm_exe
     end
 
     def nssm_params
-      %w{Application
+      %w(Application
          AppParameters
          AppDirectory
          AppExit
@@ -97,18 +97,18 @@ def nssm_params
          ObjectName
          Name
          Start
-         Type}
+         Type)
     end
 
     def nssm_service_installed?
       # 1 is command not found
       # 3 is service not found
-      exit_code = shell_out!(%{"#{nssm_exe}" status consul}, returns: [0, 1, 3]).exitstatus
+      exit_code = shell_out!(%("#{nssm_exe}" status consul), returns: [0, 1, 3]).exitstatus
       exit_code.zero?
     end
 
     def nssm_service_status?(expected_status)
-      expected_status.include? shell_out!(%{"#{nssm_exe}" status consul}, returns: [0]).stdout.delete("\0").strip
+      expected_status.include? shell_out!(%("#{nssm_exe}" status consul), returns: [0]).stdout.delete("\0").strip
     end
 
     # Returns a hash of mismatched params
@@ -117,7 +117,7 @@ def check_nssm_params
       params = node['consul']['service']['nssm_params'].select { |k, _v| nssm_params.include? k.to_s }
       params.each.each_with_object({}) do |(k, v), mismatch|
         # shell_out! returns values with null bytes, need to delete them before we evaluate
-        unless shell_out!(%{"#{nssm_exe}" get consul #{k}}, returns: [0]).stdout.delete("\0").strip.eql? v.to_s
+        unless shell_out!(%("#{nssm_exe}" get consul #{k}), returns: [0]).stdout.delete("\0").strip.eql? v.to_s
           mismatch[k] = v
         end
       end

test/fixtures/cookbooks/consul_spec/recipes/consul_definition.rb

@@ -1,7 +1,7 @@
 
 # The ruby interpreter is guaranteed to exist since it's currently running.
-file "/consul_definition_check.rb" do
-  content (<<-EOF).gsub(/^ */, '')
+file '/consul_definition_check.rb' do
+  content <<-EOF.gsub(/^ */, '')
     /bin/sh -c 'echo "Consul check script invoked"'
   EOF
   unless node.platform?('windows')
@@ -13,7 +13,7 @@
 consul_definition 'consul_definition_check' do
   type 'check'
   user 'root'
-  parameters(id: "consul_definition_check",
+  parameters(id: 'consul_definition_check',
              script: '/consul_definition_check.rb',
              interval: '10s',
              timeout: '10s')

test/fixtures/cookbooks/consul_spec/recipes/consul_watch.rb

@@ -1,7 +1,7 @@
 
 # The ruby interpreter is guaranteed to exist since it's currently running.
-file "/consul_watch_handler.rb" do
-  content (<<-EOF).gsub(/^ */, '')
+file '/consul_watch_handler.rb' do
+  content <<-EOF.gsub(/^ */, '')
     /bin/sh -c 'echo "Consul watch handler invoked"'
   EOF
   unless node.platform?('windows')
@@ -13,6 +13,6 @@
 consul_watch 'consul_watch_check' do
   type 'event'
   user 'root'
-  parameters(handler: "/consul_watch_handler.rb")
+  parameters(handler: '/consul_watch_handler.rb')
   notifies :reload, 'consul_service[consul]', :delayed
 end

test/integration/client/default_spec.rb

@@ -19,7 +19,7 @@
 end
 
 describe command("su - consul -c 'echo successfully logged in'") do
-  its(:stdout)      { should_not match /successfully logged in/ }
+  its(:stdout)      { should_not include 'successfully logged in' }
   its(:exit_status) { should_not eq 0 }
 end
 
@@ -36,8 +36,8 @@
 
 describe command("#{consul_executable} members -detailed") do
   its(:exit_status) { should eq 0 }
-  its(:stdout) { should match %r{\balive\b} }
-  its(:stdout) { should match %r{\brole=node\b} }
+  its(:stdout) { should include 'alive' }
+  its(:stdout) { should include 'role=node' }
 end
 
 describe file('/usr/local/bin/consul') do

test/integration/default/default_spec.rb

@@ -21,7 +21,7 @@
 end
 
 describe command("su - consul -c 'echo successfully logged in'") do
-  its(:stdout)      { should_not match /successfully logged in/ }
+  its(:stdout)      { should_not include 'successfully logged in' }
   its(:exit_status) { should_not eq 0 }
 end
 
@@ -38,10 +38,10 @@
 
 describe command("#{consul_executable} members -detailed") do
   its(:exit_status) { should eq 0 }
-  its(:stdout) { should match %r{\balive\b} }
-  its(:stdout) { should match %r{\brole=consul\b} }
-  its(:stdout) { should match %r{\bbootstrap=1\b} }
-  its(:stdout) { should match %r{\bdc=fortmeade\b} }
+  its(:stdout) { should include 'alive' }
+  its(:stdout) { should include 'role=consul' }
+  its(:stdout) { should include 'bootstrap=1' }
+  its(:stdout) { should include 'dc=fortmeade' }
 end
 
 describe file('/usr/local/bin/consul') do

test/integration/windows/windows_spec.rb

@@ -24,10 +24,10 @@
 
 describe command("#{consul_command} members -detailed") do
   its(:exit_status) { should eq 0 }
-  its(:stdout) { should match %r{\balive\b} }
-  its(:stdout) { should match %r{\brole=consul\b} }
-  its(:stdout) { should match %r{\bbootstrap=1\b} }
-  its(:stdout) { should match %r{\bdc=fortmeade\b} }
+  its(:stdout) { should include 'alive' }
+  its(:stdout) { should include 'role=consul' }
+  its(:stdout) { should include 'bootstrap=1' }
+  its(:stdout) { should include 'dc=fortmeade' }
 end
 
 describe file(config_file) do