From: Tom Hughes <tom@compton.nu>
Date: Thu, 5 Feb 2015 22:52:55 +0000 (+0000)
Subject: Rubocop cleanups
X-Git-Url: https://git.openstreetmap.org/chef.git/commitdiff_plain/fb1f1d56e8d8a6a86b79c71546e6c114fec2640c

Rubocop cleanups
---

diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
index 4d8a94292..155b7d135 100644
--- a/.rubocop_todo.yml
+++ b/.rubocop_todo.yml
@@ -68,11 +68,6 @@ Style/IfUnlessModifier:
 Style/MultilineOperationIndentation:
   Enabled: false
 
-# Offense count: 19
-# Configuration parameters: EnforcedStyle, MinBodyLength, SupportedStyles.
-Style/Next:
-  Enabled: false
-
 # Offense count: 29
 # Cop supports --auto-correct.
 Style/NumericLiterals:
diff --git a/cookbooks/chef/recipes/default.rb b/cookbooks/chef/recipes/default.rb
index a9fa67b58..b8e81a3ed 100644
--- a/cookbooks/chef/recipes/default.rb
+++ b/cookbooks/chef/recipes/default.rb
@@ -34,11 +34,11 @@ directory "/var/cache/chef" do
 end
 
 Dir.glob("/var/cache/chef/chef_*.deb").each do |deb|
-  if deb != "/var/cache/chef/#{chef_package}"
-    file deb do
-      action :delete
-      backup false
-    end
+  next if deb == "/var/cache/chef/#{chef_package}"
+
+  file deb do
+    action :delete
+    backup false
   end
 end
 
diff --git a/cookbooks/chef/recipes/server.rb b/cookbooks/chef/recipes/server.rb
index f314515c4..37c84e78f 100644
--- a/cookbooks/chef/recipes/server.rb
+++ b/cookbooks/chef/recipes/server.rb
@@ -34,11 +34,11 @@ directory "/var/cache/chef" do
 end
 
 Dir.glob("/var/cache/chef/chef-server_*.deb").each do |deb|
-  if deb != "/var/cache/chef/#{chef_package}"
-    file deb do
-      action :delete
-      backup false
-    end
+  next if deb == "/var/cache/chef/#{chef_package}"
+
+  file deb do
+    action :delete
+    backup false
   end
 end
 
diff --git a/cookbooks/dev/recipes/default.rb b/cookbooks/dev/recipes/default.rb
index 75dcefc8b..039f5b204 100644
--- a/cookbooks/dev/recipes/default.rb
+++ b/cookbooks/dev/recipes/default.rb
@@ -103,27 +103,28 @@ end
 search(:accounts, "*:*").each do |account|
   name = account["id"]
   details = node[:accounts][:users][name] || {}
-  port = 7000 + account["uid"].to_i
 
-  if %w(user administrator).include?(details[:status])
-    user_home = details[:home] || account["home"] || "#{node[:accounts][:home]}/#{name}"
+  next unless %w(user administrator).include?(details[:status])
 
-    if File.directory?("#{user_home}/public_html")
-      template "/etc/php5/fpm/pool.d/#{name}.conf" do
-        source "fpm.conf.erb"
-        owner "root"
-        group "root"
-        mode 0644
-        variables :user => name, :port => port
-        notifies :reload, "service[php5-fpm]"
-      end
+  user_home = details[:home] || account["home"] || "#{node[:accounts][:home]}/#{name}"
 
-      apache_site "#{name}.dev.openstreetmap.org" do
-        template "apache.user.erb"
-        directory "#{user_home}/public_html"
-        variables :user => name, :port => port
-      end
-    end
+  next unless File.directory?("#{user_home}/public_html")
+
+  port = 7000 + account["uid"].to_i
+
+  template "/etc/php5/fpm/pool.d/#{name}.conf" do
+    source "fpm.conf.erb"
+    owner "root"
+    group "root"
+    mode 0644
+    variables :user => name, :port => port
+    notifies :reload, "service[php5-fpm]"
+  end
+
+  apache_site "#{name}.dev.openstreetmap.org" do
+    template "apache.user.erb"
+    directory "#{user_home}/public_html"
+    variables :user => name, :port => port
   end
 end
 
diff --git a/cookbooks/git/recipes/server.rb b/cookbooks/git/recipes/server.rb
index 7361624c4..52d6304b0 100644
--- a/cookbooks/git/recipes/server.rb
+++ b/cookbooks/git/recipes/server.rb
@@ -61,14 +61,14 @@ Dir.new(git_directory).select { |name| name =~ /\.git$/ }.each do |repository|
     mode 0755
   end
 
-  if node[:recipes].include?("trace") && repository != "dns.git"
-    template "#{git_directory}/#{repository}/hooks/post-receive" do
-      source "post-receive.erb"
-      owner "root"
-      group node[:git][:group]
-      mode 0755
-      variables :repository => "#{git_directory}/#{repository}"
-    end
+  next unless node[:recipes].include?("trac") && repository != "dns.git"
+
+  template "#{git_directory}/#{repository}/hooks/post-receive" do
+    source "post-receive.erb"
+    owner "root"
+    group node[:git][:group]
+    mode 0755
+    variables :repository => "#{git_directory}/#{repository}"
   end
 end
 
diff --git a/cookbooks/hardware/recipes/default.rb b/cookbooks/hardware/recipes/default.rb
index a0ee58485..dd2d80157 100644
--- a/cookbooks/hardware/recipes/default.rb
+++ b/cookbooks/hardware/recipes/default.rb
@@ -204,13 +204,13 @@ node[:kernel][:modules].each_key do |modname|
 end
 
 node[:block_device].each do |name, attributes|
-  if attributes[:vendor] == "HP" && attributes[:model] == "LOGICAL VOLUME"
-    if name =~ /^cciss!(c[0-9]+)d[0-9]+$/
-      status_packages["cciss-vol-status"] |= ["cciss/#{Regexp.last_match[1]}d0"]
-    else
-      Dir.glob("/sys/block/#{name}/device/scsi_generic/*").each do |sg|
-        status_packages["cciss-vol-status"] |= [File.basename(sg)]
-      end
+  next unless attributes[:vendor] == "HP" && attributes[:model] == "LOGICAL VOLUME"
+
+  if name =~ /^cciss!(c[0-9]+)d[0-9]+$/
+    status_packages["cciss-vol-status"] |= ["cciss/#{Regexp.last_match[1]}d0"]
+  else
+    Dir.glob("/sys/block/#{name}/device/scsi_generic/*").each do |sg|
+      status_packages["cciss-vol-status"] |= [File.basename(sg)]
     end
   end
 end
diff --git a/cookbooks/mysql/libraries/mysql.rb b/cookbooks/mysql/libraries/mysql.rb
index 2b2bca67d..769df807e 100644
--- a/cookbooks/mysql/libraries/mysql.rb
+++ b/cookbooks/mysql/libraries/mysql.rb
@@ -104,12 +104,12 @@ class Chef
       end
 
       query("SELECT * FROM db").each do |record|
-        if database = @databases[record[:db]]
-          user = "'#{record[:user]}'@'#{record[:host]}'"
+        next unless database = @databases[record[:db]]
 
-          database[:permissions][user] = DATABASE_PRIVILEGES.each_with_object([]) do |privilege, privileges|
-            privileges << privilege if record["#{privilege}_priv".to_sym] == "Y"
-          end
+        user = "'#{record[:user]}'@'#{record[:host]}'"
+
+        database[:permissions][user] = DATABASE_PRIVILEGES.each_with_object([]) do |privilege, privileges|
+          privileges << privilege if record["#{privilege}_priv".to_sym] == "Y"
         end
       end
 
diff --git a/cookbooks/mysql/providers/database.rb b/cookbooks/mysql/providers/database.rb
index ab735cd7e..647925f26 100644
--- a/cookbooks/mysql/providers/database.rb
+++ b/cookbooks/mysql/providers/database.rb
@@ -41,11 +41,11 @@ action :create do
   end]
 
   @current_resource.permissions.each_key do |user|
-    unless new_permissions[user]
-      converge_by("revoke all for #{user} on #{new_resource}") do
-        Chef::Log.info("Revoking all for #{user} on #{new_resource}")
-        @mysql.execute(:command => "REVOKE ALL ON `#{new_resource.database}`.* FROM #{user}")
-      end
+    next if new_permissions[user]
+
+    converge_by("revoke all for #{user} on #{new_resource}") do
+      Chef::Log.info("Revoking all for #{user} on #{new_resource}")
+      @mysql.execute(:command => "REVOKE ALL ON `#{new_resource.database}`.* FROM #{user}")
     end
   end
 
diff --git a/cookbooks/mysql/providers/user.rb b/cookbooks/mysql/providers/user.rb
index e027b4f5c..05fd01eb8 100644
--- a/cookbooks/mysql/providers/user.rb
+++ b/cookbooks/mysql/providers/user.rb
@@ -42,17 +42,17 @@ action :create do
   end
 
   Chef::MySQL::USER_PRIVILEGES.each do |privilege|
-    if new_resource.send(privilege) != @current_resource.send(privilege)
-      if new_resource.send(privilege)
-        converge_by("grant #{privilege} for #{new_resource}") do
-          Chef::Log.info("Granting #{privilege} for #{new_resource}")
-          @mysql.execute(:command => "GRANT #{@mysql.privilege_name(privilege)} ON *.* TO #{user}")
-        end
-      else
-        converge_by("revoke #{privilege} for #{new_resource}") do
-          Chef::Log.info("Revoking #{privilege} for #{new_resource}")
-          @mysql.execute(:command => "REVOKE #{@mysql.privilege_name(privilege)} ON *.* FROM #{user}")
-        end
+    next if new_resource.send(privilege) == @current_resource.send(privilege)
+
+    if new_resource.send(privilege)
+      converge_by("grant #{privilege} for #{new_resource}") do
+        Chef::Log.info("Granting #{privilege} for #{new_resource}")
+        @mysql.execute(:command => "GRANT #{@mysql.privilege_name(privilege)} ON *.* TO #{user}")
+      end
+    else
+      converge_by("revoke #{privilege} for #{new_resource}") do
+        Chef::Log.info("Revoking #{privilege} for #{new_resource}")
+        @mysql.execute(:command => "REVOKE #{@mysql.privilege_name(privilege)} ON *.* FROM #{user}")
       end
     end
   end
diff --git a/cookbooks/networking/libraries/interfaces.rb b/cookbooks/networking/libraries/interfaces.rb
index fa0a93c7d..cd9116dfa 100644
--- a/cookbooks/networking/libraries/interfaces.rb
+++ b/cookbooks/networking/libraries/interfaces.rb
@@ -7,14 +7,13 @@ class Chef
       networking_interfaces = networking[:interfaces] || []
 
       networking_interfaces.each_value  do |interface|
-        if options[:role].nil? || interface[:role].to_s == options[:role].to_s
-          if options[:family].nil? || interface[:family].to_s == options[:family].to_s
-            if block.nil?
-              interfaces << interface
-            else
-              block.call(interface)
-            end
-          end
+        next unless options[:role].nil? || interface[:role].to_s == options[:role].to_s
+        next unless options[:family].nil? || interface[:family].to_s == options[:family].to_s
+
+        if block.nil?
+          interfaces << interface
+        else
+          block.call(interface)
         end
       end
 
diff --git a/cookbooks/networking/recipes/default.rb b/cookbooks/networking/recipes/default.rb
index dc5c69227..9eebca8c7 100644
--- a/cookbooks/networking/recipes/default.rb
+++ b/cookbooks/networking/recipes/default.rb
@@ -83,24 +83,24 @@ end
 node.interfaces(:role => :internal) do |interface|
   if interface[:gateway] && interface[:gateway] != interface[:address]
     search(:node, "networking_interfaces*address:#{interface[:gateway]}") do |gateway|
-      if gateway[:openvpn]
-        gateway[:openvpn][:tunnels].each_value do |tunnel|
-          if tunnel[:peer][:address]
-            route tunnel[:peer][:address] do
-              netmask "255.255.255.255"
-              gateway interface[:gateway]
-              device interface[:interface]
-            end
+      next unless gateway[:openvpn]
+
+      gateway[:openvpn][:tunnels].each_value do |tunnel|
+        if tunnel[:peer][:address]
+          route tunnel[:peer][:address] do
+            netmask "255.255.255.255"
+            gateway interface[:gateway]
+            device interface[:interface]
           end
+        end
+
+        next unless tunnel[:peer][:networks]
 
-          if tunnel[:peer][:networks]
-            tunnel[:peer][:networks].each do |network|
-              route network[:address] do
-                netmask network[:netmask]
-                gateway interface[:gateway]
-                device interface[:interface]
-              end
-            end
+        tunnel[:peer][:networks].each do |network|
+          route network[:address] do
+            netmask network[:netmask]
+            gateway interface[:gateway]
+            device interface[:interface]
           end
         end
       end
@@ -111,14 +111,14 @@ end
 zones = {}
 
 search(:node, "networking:interfaces").collect do |n|
-  if n[:fqdn] != node[:fqdn]
-    n.interfaces.each do |interface|
-      if interface[:role] == "external" && interface[:zone]
-        zones[interface[:zone]] ||= Hash.new
-        zones[interface[:zone]][interface[:family]] ||= Array.new
-        zones[interface[:zone]][interface[:family]] << interface[:address]
-      end
-    end
+  next if n[:fqdn] == node[:fqdn]
+
+  n.interfaces.each do |interface|
+    next unless interface[:role] == "external" && interface[:zone]
+
+    zones[interface[:zone]] ||= Hash.new
+    zones[interface[:zone]][interface[:family]] ||= Array.new
+    zones[interface[:zone]][interface[:family]] << interface[:address]
   end
 end
 
diff --git a/cookbooks/nfs/recipes/server.rb b/cookbooks/nfs/recipes/server.rb
index 1142adb3f..f75e44e88 100644
--- a/cookbooks/nfs/recipes/server.rb
+++ b/cookbooks/nfs/recipes/server.rb
@@ -33,18 +33,18 @@ end
 exports = {}
 
 search(:node, "*:*") do |client|
-  if client[:nfs]
-    client[:nfs].each_value do |mount|
-      if mount[:host] == node[:hostname]
-        client.ipaddresses do |address|
-          exports[mount[:path]] ||= {}
+  next unless client[:nfs]
 
-          if mount[:readonly]
-            exports[mount[:path]][address] = "ro"
-          else
-            exports[mount[:path]][address] = "rw"
-          end
-        end
+  client[:nfs].each_value do |mount|
+    next unless mount[:host] == node[:hostname]
+
+    client.ipaddresses do |address|
+      exports[mount[:path]] ||= {}
+
+      if mount[:readonly]
+        exports[mount[:path]][address] = "ro"
+      else
+        exports[mount[:path]][address] = "rw"
       end
     end
   end
diff --git a/cookbooks/ntp/recipes/default.rb b/cookbooks/ntp/recipes/default.rb
index c2aba251d..96e6c2caf 100644
--- a/cookbooks/ntp/recipes/default.rb
+++ b/cookbooks/ntp/recipes/default.rb
@@ -60,10 +60,10 @@ munin_plugin "ntp_offset"
 
 if File.directory?("/etc/munin/plugins")
   Dir.new("/etc/munin/plugins").each do |plugin|
-    if plugin.match(/^ntp_/) && !munin_plugins.include?(plugin)
-      munin_plugin plugin do
-        action :delete
-      end
+    next unless plugin.match(/^ntp_/) && !munin_plugins.include?(plugin)
+
+    munin_plugin plugin do
+      action :delete
     end
   end
 end
diff --git a/cookbooks/osmosis/recipes/default.rb b/cookbooks/osmosis/recipes/default.rb
index 7532cd7ac..2c55a64aa 100644
--- a/cookbooks/osmosis/recipes/default.rb
+++ b/cookbooks/osmosis/recipes/default.rb
@@ -26,11 +26,11 @@ osmosis_package = "osmosis-#{node[:osmosis][:version]}.zip"
 osmosis_directory = "/opt/osmosis-#{node[:osmosis][:version]}"
 
 Dir.glob("/var/cache/chef/osmosis-*.zip").each do |zip|
-  if zip != "/var/cache/chef/#{osmosis_package}"
-    file zip do
-      action :delete
-      backup false
-    end
+  next if zip == "/var/cache/chef/#{osmosis_package}"
+
+  file zip do
+    action :delete
+    backup false
   end
 end
 
diff --git a/cookbooks/postgresql/providers/table.rb b/cookbooks/postgresql/providers/table.rb
index 7bc8cfb02..b1dc85f7b 100644
--- a/cookbooks/postgresql/providers/table.rb
+++ b/cookbooks/postgresql/providers/table.rb
@@ -43,11 +43,11 @@ action :create do
     end
 
     @current_resource.permissions.each_key do |user|
-      unless new_resource.permissions[user]
-        converge_by("revoke all for #{user} on #{new_resource}") do
-          Chef::Log.info("Revoking all for #{user} on #{new_resource}")
-          @pg.execute(:command => "REVOKE ALL ON #{@name} FROM \"#{user}\"", :database => new_resource.database)
-        end
+      next if new_resource.permissions[user]
+
+      converge_by("revoke all for #{user} on #{new_resource}") do
+        Chef::Log.info("Revoking all for #{user} on #{new_resource}")
+        @pg.execute(:command => "REVOKE ALL ON #{@name} FROM \"#{user}\"", :database => new_resource.database)
       end
     end
 
diff --git a/cookbooks/postgresql/recipes/default.rb b/cookbooks/postgresql/recipes/default.rb
index 9fe64344a..ea043096c 100644
--- a/cookbooks/postgresql/recipes/default.rb
+++ b/cookbooks/postgresql/recipes/default.rb
@@ -131,11 +131,11 @@ clusters.each do |name, details|
     conf_variables :port => details[:port]
   end
 
-  if File.exist?("/var/lib/postgresql/#{details[:version]}/main/recovery.conf")
-    munin_plugin "postgres_replication_#{suffix}" do
-      target "postgres_replication"
-      conf "munin.erb"
-      conf_variables :port => details[:port]
-    end
+  next unless File.exist?("/var/lib/postgresql/#{details[:version]}/main/recovery.conf")
+
+  munin_plugin "postgres_replication_#{suffix}" do
+    target "postgres_replication"
+    conf "munin.erb"
+    conf_variables :port => details[:port]
   end
 end
diff --git a/cookbooks/rsyncd/recipes/default.rb b/cookbooks/rsyncd/recipes/default.rb
index 6d4d6b763..18eb23082 100644
--- a/cookbooks/rsyncd/recipes/default.rb
+++ b/cookbooks/rsyncd/recipes/default.rb
@@ -25,19 +25,15 @@ hosts_deny = {}
 node[:rsyncd][:modules].each do |name, details|
   hosts_allow[name] = details[:hosts_allow] || []
 
-  if details[:nodes_allow]
-    hosts_allow[name] |= search(:node, details[:nodes_allow]).collect do |n|
-      n.ipaddresses(:role => :external)
-    end.flatten
-  end
+  hosts_allow[name] |= search(:node, details[:nodes_allow]).collect do |n|
+    n.ipaddresses(:role => :external)
+  end.flatten if details[:nodes_allow]
 
   hosts_deny[name] = details[:hosts_deny] || []
 
-  if details[:nodes_deny]
-    hosts_deny[name] |= search(:node, details[:nodes_deny]).collect do |n|
-      n.ipaddresses(:role => :external)
-    end.flatten
-  end
+  hosts_deny[name] |= search(:node, details[:nodes_deny]).collect do |n|
+    n.ipaddresses(:role => :external)
+  end.flatten if details[:nodes_deny]
 end
 
 package "rsync"
diff --git a/cookbooks/tile/files/default/ruby/expire.rb b/cookbooks/tile/files/default/ruby/expire.rb
index c6916943a..7757386c5 100755
--- a/cookbooks/tile/files/default/ruby/expire.rb
+++ b/cookbooks/tile/files/default/ruby/expire.rb
@@ -126,13 +126,14 @@ module Expire
     node_cache = NodeCache.new(NODE_CACHE_FILE)
     doc.find('//way/nd').each do |node|
       node_id = node['ref'].to_i
-      unless nodes.include? node_id
-        # this is a node referenced but not added, modified or deleted, so it should
-        # still be in the node cache.
-        if entry = node_cache[node_id]
-          point = Proj4::Point.new(entry.lon, entry.lat)
-          nodes[node_id] = tile_from_merc(point, max_zoom)
-        end
+
+      next if nodes.include? node_id
+
+      # this is a node referenced but not added, modified or deleted, so it should
+      # still be in the node cache.
+      if entry = node_cache[node_id]
+        point = Proj4::Point.new(entry.lon, entry.lat)
+        nodes[node_id] = tile_from_merc(point, max_zoom)
       end
     end