Merge pull request #7038 from chef/sudo_which

Fix a few bugs in the sudo resource

2 files changed, 17 insertions, 7 deletions

diff --git a/lib/chef/resource/sudo.rb b/lib/chef/resource/sudo.rb
index e2425f811b..f8c4bf5af1 100644
--- a/lib/chef/resource/sudo.rb
+++ b/lib/chef/resource/sudo.rb
@@ -99,6 +99,9 @@ class Chef
                default: lazy { [] }
 
       property :visudo_path, String,
+               description: "Deprecated property. Do not use."
+
+      property :visudo_binary,
                description: "The path to visudo for config verification.",
                default: "/usr/sbin/visudo"
 
@@ -106,6 +109,12 @@ class Chef
                description: "The directory containing the sudoers config file.",
                default: lazy { platform_config_prefix }
 
+      # handle legacy cookbook property
+      def after_created
+        raise "The 'visudo_path' property from the sudo cookbook has been replaced with the 'visudo_binary' property. The path is now more intelligently determined and for most users specifying the path should no longer be necessary. If this resource still cannot determine the path to visudo then provide the full path to the binary with the 'visudo_binary' property." if visudo_path
+      end
+
+      # VERY old legacy properties
       alias_method :user, :users
       alias_method :group, :groups
 
@@ -119,14 +128,15 @@ class Chef
       end
 
       # default config prefix paths based on platform
+      # @return [String]
       def platform_config_prefix
         case node["platform_family"]
         when "smartos"
           "/opt/local/etc"
-        when "freebsd"
-          "/usr/local/etc"
         when "mac_os_x"
           "/private/etc"
+        when "freebsd"
+          "/usr/local/etc"
         else
           "/etc"
         end
@@ -156,7 +166,7 @@ class Chef
             source new_resource.template
             mode "0440"
             variables new_resource.variables
-            verify "#{new_resource.visudo_path} -cf %{path}" if visudo_present?
+            verify "#{new_resource.visudo_binary} -cf %{path}" if visudo_present?
             action :create
           end
         else
@@ -176,7 +186,7 @@ class Chef
                       setenv:             new_resource.setenv,
                       env_keep_add:       new_resource.env_keep_add,
                       env_keep_subtract:  new_resource.env_keep_subtract
-            verify "#{new_resource.visudo_path} -cf %{path}" if visudo_present?
+            verify "#{new_resource.visudo_binary} -cf %{path}" if visudo_present?
             action :create
           end
         end
@@ -212,8 +222,8 @@ class Chef
         end
 
         def visudo_present?
-          return if ::File.exist?(new_resource.visudo_path)
-          Chef::Log.warn("The visudo binary cannot be found at '#{new_resource.visudo_path}'. Skipping sudoer file validation. If visudo is on this system you can specify the path using the 'visudo_path' property.")
+          return true if ::File.exist?(new_resource.visudo_binary)
+          Chef::Log.warn("The visudo binary cannot be found at '#{new_resource.visudo_binary}'. Skipping sudoer file validation. If visudo is on this system you can specify the path using the 'visudo_binary' property.")
         end
       end
     end
diff --git a/lib/chef/resource/support/sudoer.erb b/lib/chef/resource/support/sudoer.erb
index c54507535b..d19540bd33 100644
--- a/lib/chef/resource/support/sudoer.erb
+++ b/lib/chef/resource/support/sudoer.erb
@@ -11,7 +11,7 @@ Defaults    env_keep += "<%= env_keep %>"
 Defaults    env_keep -= "<%= env_keep %>"
 <% end -%>
 <% @commands.each do |command| -%>
-<% if @sudoer %><%= @sudoer %> <%= @host %>=(<%= @runas %>) <%= 'NOEXEC:' if @noexec %><%= 'NOPASSWD:' if @nopasswd.to_s == 'true' %><%= 'SETENV:' if @setenv.to_s == 'true' %><%= command %><% end -%>
+<% unless @sudoer.empty? %><%= @sudoer %> <%= @host %>=(<%= @runas %>) <%= 'NOEXEC:' if @noexec %><%= 'NOPASSWD:' if @nopasswd.to_s == 'true' %><%= 'SETENV:' if @setenv.to_s == 'true' %><%= command %><% end -%>
 <% end -%>
 <% unless @defaults.empty? %>
 Defaults:<%= @sudoer %> <%= @defaults.join(',') %>