Make script resources use properties

15 files changed, 66 insertions, 195 deletions

diff --git a/lib/chef/exceptions.rb b/lib/chef/exceptions.rb
index 5b470f574e..1c03d04638 100644
--- a/lib/chef/exceptions.rb
+++ b/lib/chef/exceptions.rb
@@ -65,6 +65,10 @@ class Chef
       end
     end
 
+    class ValidationFailed < ArgumentError; end
+    # Attempting to access a 64-bit only resource on a 32-bit Windows system
+    class Win32ArchitectureIncorrect < ValidationFailed; end
+
     class User < RuntimeError; end
     class Group < RuntimeError; end
     class Link < RuntimeError; end
@@ -156,8 +160,6 @@ class Chef
     # Attempting to run windows code on a not-windows node
     class Win32NotWindows < RuntimeError; end
     class WindowsNotAdmin < RuntimeError; end
-    # Attempting to access a 64-bit only resource on a 32-bit Windows system
-    class Win32ArchitectureIncorrect < RuntimeError; end
     class ObsoleteDependencySyntax < ArgumentError; end
     class InvalidDataBagPath < ArgumentError; end
     class DuplicateDataBagItem < RuntimeError; end
diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb
index 5436e3ceb3..bfdb39c3c9 100644
--- a/lib/chef/resource.rb
+++ b/lib/chef/resource.rb
@@ -141,13 +141,6 @@ class Chef
       @not_if = []
       @only_if = []
       @source_line = nil
-      # We would like to raise an error when the user gives us a guard
-      # interpreter and a ruby_block to the guard. In order to achieve this
-      # we need to understand when the user overrides the default guard
-      # interpreter. Therefore we store the default separately in a different
-      # attribute.
-      @guard_interpreter = nil
-      @default_guard_interpreter = :default
       @elapsed_time = 0
     end
 
@@ -475,13 +468,7 @@ class Chef
     #   symbol/name.
     # @return [Class, Symbol, String] The Guard interpreter resource.
     #
-    def guard_interpreter(arg = nil)
-      if arg.nil?
-        @guard_interpreter || @default_guard_interpreter
-      else
-        set_or_return(:guard_interpreter, arg, :kind_of => Symbol)
-      end
-    end
+    property :guard_interpreter, Symbol, default: lazy { default_guard_interpreter }, desired_state: false
 
     #
     # Get the value of the state attributes in this resource as a hash.
@@ -699,8 +686,8 @@ class Chef
     def to_hash
       # Grab all current state, then any other ivars (backcompat)
       result = {}
-      self.class.state_properties.each do |p|
-        result[p.name] = p.get(self)
+      self.class.properties.each do |property_name, property|
+        result[property_name] = property.get(self)
       end
       safe_ivars = instance_variables.map { |ivar| ivar.to_sym } - FORBIDDEN_IVARS
       safe_ivars.each do |iv|
@@ -852,7 +839,7 @@ class Chef
     #
     # @return [Class, Symbol, String] the default Guard interpreter resource.
     #
-    attr_reader :default_guard_interpreter
+    property :default_guard_interpreter, Symbol, default: :default, desired_state: false
 
     #
     # The list of actions this Resource is allowed to have.  Setting `action`
diff --git a/lib/chef/resource/bash.rb b/lib/chef/resource/bash.rb
index ebc4a7790b..b421dc2080 100644
--- a/lib/chef/resource/bash.rb
+++ b/lib/chef/resource/bash.rb
@@ -26,12 +26,8 @@ class Chef
     # their nature) not idempotent, as they are typically unique to the environment in which they are run. Use not_if
     # and only_if to guard this resource for idempotence.
     class Bash < Chef::Resource::Script
-
-      def initialize(name, run_context = nil)
-        super
-        @interpreter = "bash"
-      end
-
+      resource_name :bash
+      property :interpreter, default: "bash"
     end
   end
 end
diff --git a/lib/chef/resource/batch.rb b/lib/chef/resource/batch.rb
index b3c7b17cef..89531a0f24 100644
--- a/lib/chef/resource/batch.rb
+++ b/lib/chef/resource/batch.rb
@@ -26,13 +26,11 @@ class Chef
     # typically unique to the environment in which they are run. Use not_if and only_if to guard this resource for
     # idempotence.
     class Batch < Chef::Resource::WindowsScript
-
+      resource_name :batch
       provides :batch, os: "windows"
 
-      def initialize(name, run_context = nil)
-        super(name, run_context, nil, "cmd.exe")
-      end
-
+      property :interpreter, default: "cmd.exe"
+      property :default_guard_interpreter, default: :batch
     end
   end
 end
diff --git a/lib/chef/resource/conditional.rb b/lib/chef/resource/conditional.rb
index 452718cae8..d65d9f515c 100644
--- a/lib/chef/resource/conditional.rb
+++ b/lib/chef/resource/conditional.rb
@@ -47,6 +47,7 @@ class Chef
         @positivity = positivity
         @command, @command_opts = command, command_opts
         @block = block
+        # TODO: Maybe not needed? Certainly not used.
         @block_given = block_given?
         @parent_resource = parent_resource
 
@@ -62,7 +63,7 @@ class Chef
           # We should have a block if we get here
           # Check to see if the user set the guard_interpreter on the parent resource. Note that
           # this error will not be raised when using the default_guard_interpreter
-          if @parent_resource.guard_interpreter != @parent_resource.default_guard_interpreter
+          if @parent_resource.property_is_set?(:guard_interpreter)
             msg = "#{@parent_resource.name} was given a guard_interpreter of #{@parent_resource.guard_interpreter}, "
             msg << "but not given a command as a string. guard_interpreter does not support blocks (because they just contain ruby)."
             raise ArgumentError, msg
diff --git a/lib/chef/resource/csh.rb b/lib/chef/resource/csh.rb
index ef302af6c0..205809c603 100644
--- a/lib/chef/resource/csh.rb
+++ b/lib/chef/resource/csh.rb
@@ -26,12 +26,8 @@ class Chef
     # their nature) not idempotent, as they are typically unique to the environment in which they are run. Use not_if
     # and only_if to guard this resource for idempotence.
     class Csh < Chef::Resource::Script
-
-      def initialize(name, run_context = nil)
-        super
-        @interpreter = "csh"
-      end
-
+      resource_name :csh
+      property :interpreter, default: "csh"
     end
   end
 end
diff --git a/lib/chef/resource/execute.rb b/lib/chef/resource/execute.rb
index de927ec06e..ad3f31433b 100644
--- a/lib/chef/resource/execute.rb
+++ b/lib/chef/resource/execute.rb
@@ -26,116 +26,42 @@ class Chef
     # nature) not idempotent, as they are typically unique to the environment in which they are run. Use not_if and only_if
     # to guard this resource for idempotence.
     class Execute < Chef::Resource
-
-      identity_attr :command
-
-      # The ResourceGuardInterpreter wraps a resource's guards in another resource.  That inner resource
-      # needs to behave differently during (for example) why_run mode, so we flag it here. For why_run mode
-      # we still want to execute the guard resource even if we are not executing the wrapping resource.
-      # Only execute resources (and subclasses) can be guard interpreters.
-      attr_accessor :is_guard_interpreter
+      resource_name :execute
 
       default_action :run
 
       def initialize(name, run_context = nil)
         super
+        # Capture the name before it is coerced to a string
         @command = name
+        # TODO remove. Likely holdover from copy/paste from file
         @backup = 5
-        @creates = nil
-        @cwd = nil
-        @environment = nil
-        @group = nil
-        @returns = 0
-        @timeout = nil
-        @user = nil
-        @umask = nil
-        @default_guard_interpreter = :execute
-        @is_guard_interpreter = false
-        @live_stream = false
-      end
-
-      def umask(arg = nil)
-        set_or_return(
-          :umask,
-          arg,
-          :kind_of => [ String, Integer ]
-        )
-      end
-
-      def command(arg = nil)
-        set_or_return(
-          :command,
-          arg,
-          :kind_of => [ String, Array ]
-        )
-      end
-
-      def creates(arg = nil)
-        set_or_return(
-          :creates,
-          arg,
-          :kind_of => [ String ]
-        )
-      end
-
-      def cwd(arg = nil)
-        set_or_return(
-          :cwd,
-          arg,
-          :kind_of => [ String ]
-        )
-      end
-
-      def environment(arg = nil)
-        set_or_return(
-          :environment,
-          arg,
-          :kind_of => [ Hash ]
-        )
       end
 
+      # The ResourceGuardInterpreter wraps a resource's guards in another resource.  That inner resource
+      # needs to behave differently during (for example) why_run mode, so we flag it here. For why_run mode
+      # we still want to execute the guard resource even if we are not executing the wrapping resource.
+      # Only execute resources (and subclasses) can be guard interpreters.
+      property :is_guard_interpreter, default: false
+      property :umask, [ String, Integer ]
+      property :command, [ String, Array ], identity: true, name_property: true
+      property :creates, String
+      property :cwd, String
+      property :environment, Hash
       alias :env :environment
 
-      def group(arg = nil)
-        set_or_return(
-          :group,
-          arg,
-          :kind_of => [ String, Integer ]
-        )
-      end
-
-      def live_stream(arg = nil)
-        set_or_return(
-          :live_stream,
-          arg,
-          :kind_of => [ TrueClass, FalseClass ])
-      end
-
-      def returns(arg = nil)
-        set_or_return(
-          :returns,
-          arg,
-          :kind_of => [ Integer, Array ]
-        )
-      end
-
-      def timeout(arg = nil)
-        set_or_return(
-          :timeout,
-          arg,
-          :kind_of => [ Integer, Float ]
-        )
-      end
-
       property :user, [ String, Integer ]
-
       property :domain, String
-
       property :password, String, sensitive: true
-
       property :sensitive, [ TrueClass, FalseClass ], default: false, coerce: proc { |x| password ? true : x }
-
       property :elevated, [ TrueClass, FalseClass ], default: false
+      property :group, [ String, Integer ]
+      property :live_stream, [ true, false ], default: false
+      property :path, Array, coerce: proc { |v| Chef::Log.warn "The 'path' attribute of 'execute' is not used by any provider in Chef 11 or Chef 12. Use 'environment' attribute to configure 'PATH'. This attribute will be removed in Chef 13."; v }
+      property :returns, [ Integer, Array ], default: 0
+      property :timeout, [ Integer, Float ]
+      property :user, [ String, Integer ]
+      property :default_guard_interpreter, default: :execute
 
       def self.set_guard_inherited_attributes(*inherited_attributes)
         @class_inherited_attributes = inherited_attributes
diff --git a/lib/chef/resource/ksh.rb b/lib/chef/resource/ksh.rb
index 8024f0d6bd..1009750387 100644
--- a/lib/chef/resource/ksh.rb
+++ b/lib/chef/resource/ksh.rb
@@ -27,12 +27,8 @@ class Chef
     #
     # @since 12.6
     class Ksh < Chef::Resource::Script
-
-      def initialize(name, run_context = nil)
-        super
-        @interpreter = "ksh"
-      end
-
+      resource_name :ksh
+      property :interpreter, default: "ksh"
     end
   end
 end
diff --git a/lib/chef/resource/perl.rb b/lib/chef/resource/perl.rb
index c00246ac6b..743e1696d3 100644
--- a/lib/chef/resource/perl.rb
+++ b/lib/chef/resource/perl.rb
@@ -26,11 +26,8 @@ class Chef
     # their nature) not idempotent, as they are typically unique to the environment in which they are run. Use not_if and
     # only_if to guard this resource for idempotence.
     class Perl < Chef::Resource::Script
-      def initialize(name, run_context = nil)
-        super
-        @interpreter = "perl"
-      end
-
+      resource_name :perl
+      property :interpreter, default: "perl"
     end
   end
 end
diff --git a/lib/chef/resource/powershell_script.rb b/lib/chef/resource/powershell_script.rb
index 8ff58cfeab..6d0c880e0b 100644
--- a/lib/chef/resource/powershell_script.rb
+++ b/lib/chef/resource/powershell_script.rb
@@ -28,20 +28,11 @@ class Chef
     # idempotent, as they are typically unique to the environment in which they are run. Use not_if and only_if to guard
     # this resource for idempotence.
     class PowershellScript < Chef::Resource::WindowsScript
+      resource_name :powershell_script
       provides :powershell_script, os: "windows"
 
-      def initialize(name, run_context = nil)
-        super(name, run_context, :powershell_script, "powershell.exe")
-        @convert_boolean_return = false
-      end
-
-      def convert_boolean_return(arg = nil)
-        set_or_return(
-          :convert_boolean_return,
-          arg,
-          :kind_of => [ FalseClass, TrueClass ]
-        )
-      end
+      property :interpreter, default: "powershell.exe"
+      property :convert_boolean_return, [ true, false ], default: false
 
       # Allow callers evaluating guards to request default
       # attribute values. This is needed to allow
diff --git a/lib/chef/resource/python.rb b/lib/chef/resource/python.rb
index 8c9e74217a..0f6b6795bd 100644
--- a/lib/chef/resource/python.rb
+++ b/lib/chef/resource/python.rb
@@ -25,11 +25,8 @@ class Chef
     # their nature) not idempotent, as they are typically unique to the environment in which they are run. Use not_if and
     # only_if to guard this resource for idempotence.
     class Python < Chef::Resource::Script
-      def initialize(name, run_context = nil)
-        super
-        @interpreter = "python"
-      end
-
+      resource_name :python
+      property :interpreter, default: "python"
     end
   end
 end
diff --git a/lib/chef/resource/ruby.rb b/lib/chef/resource/ruby.rb
index 300405f89e..d7ecf5440d 100644
--- a/lib/chef/resource/ruby.rb
+++ b/lib/chef/resource/ruby.rb
@@ -26,10 +26,8 @@ class Chef
     # their nature) not idempotent, as they are typically unique to the environment in which they are run. Use not_if and
     # only_if to guard this resource for idempotence.
     class Ruby < Chef::Resource::Script
-      def initialize(name, run_context = nil)
-        super
-        @interpreter = "ruby"
-      end
+      resource_name :ruby
+      property :interpreter, default: "ruby"
     end
   end
 end
diff --git a/lib/chef/resource/script.rb b/lib/chef/resource/script.rb
index a7191360ca..5271fa0d96 100644
--- a/lib/chef/resource/script.rb
+++ b/lib/chef/resource/script.rb
@@ -27,26 +27,22 @@ class Chef
     # that are executed with this resource are (by their nature) not idempotent, as they are typically unique to the
     # environment in which they are run. Use not_if and only_if to guard this resource for idempotence.
     class Script < Chef::Resource::Execute
-      identity_attr :name
+      resource_name :script
 
-      def initialize(name, run_context = nil)
-        super
-        @command = nil
-        @default_guard_interpreter = :default
-      end
+      # Chef-13: go back to using :name as the identity attr
+      # Chef-13: the command variable should be initialized to nil
 
-      # FIXME: remove this and use an execute sub-resource instead of inheriting from Execute
-      def command(arg = nil)
-        unless arg.nil?
-          raise Chef::Exceptions::Script, "Do not use the command attribute on a #{resource_name} resource, use the 'code' attribute instead."
-        end
-        super
-      end
+      property :command, coerce: proc { |v|
+        # Chef-13: change this to raise if the user is trying to set a value here
+        Chef::Log.warn "Specifying command attribute on a script resource is a coding error, use the 'code' attribute, or the execute resource"
+        Chef::Log.warn "This attribute is deprecated and must be fixed or this code will fail on Chef 13"
+        v
+      }
 
-      property :code, String, required: true
+      property :code, String
       property :interpreter, String
       property :flags, String
-
+      property :default_guard_interpreter, default: :default
     end
   end
 end
diff --git a/lib/chef/resource/windows_script.rb b/lib/chef/resource/windows_script.rb
index 7c39d9fba0..01dbd7d26f 100644
--- a/lib/chef/resource/windows_script.rb
+++ b/lib/chef/resource/windows_script.rb
@@ -24,33 +24,24 @@ class Chef
   class Resource
     class WindowsScript < Chef::Resource::Script
       # This is an abstract resource meant to be subclasses; thus no 'provides'
+      resource_name :windows_script
 
       set_guard_inherited_attributes(:architecture)
 
+      property :default_guard_interpreter, default: lazy { resource_name }
+      property :architecture, [ :x86_64, :i386 ],
+                 coerce: proc { |v| assert_architecture_compatible!(v) }
+
       protected
 
-      def initialize(name, run_context, resource_name, interpreter_command)
+      def initialize(name, run_context, resource_name = nil, interpreter = nil)
         super(name, run_context)
-        @interpreter = interpreter_command
+        @interpreter = interpreter if interpreter
         @resource_name = resource_name if resource_name
-        @default_guard_interpreter = self.resource_name
       end
 
       include Chef::Mixin::WindowsArchitectureHelper
 
-      public
-
-      def architecture(arg = nil)
-        assert_architecture_compatible!(arg) if ! arg.nil?
-        result = set_or_return(
-          :architecture,
-          arg,
-          :kind_of => Symbol
-        )
-      end
-
-      protected
-
       def assert_architecture_compatible!(desired_architecture)
         if desired_architecture == :i386 && Chef::Platform.windows_nano_server?
           raise Chef::Exceptions::Win32ArchitectureIncorrect,
diff --git a/spec/unit/provider/deploy_spec.rb b/spec/unit/provider/deploy_spec.rb
index b0ede7e260..ee3e5386c0 100644
--- a/spec/unit/provider/deploy_spec.rb
+++ b/spec/unit/provider/deploy_spec.rb
@@ -582,7 +582,6 @@ describe Chef::Provider::Deploy do
       @resource.user("notCoolMan")
       expect(mock_execution).to receive(:user).with("notCoolMan")
       expect(mock_execution).to receive(:cwd).with(no_args()).and_return("/some/value")
-      expect(mock_execution).to receive(:environment).with(no_args()).and_return({})
       @provider.run("iGoToHell4this")
     end