From d2e718e4496419a53572224fd3ffbdc21944f142 Mon Sep 17 00:00:00 2001
From: Tim Smith <tsmith@chef.io>
Date: Tue, 19 Dec 2017 14:36:03 -0800
Subject: Modernize the execute command

Signed-off-by: Tim Smith <tsmith@chef.io>
---
 lib/chef/resource/execute.rb                 | 103 ++++---------------------
 spec/support/shared/unit/execute_resource.rb | 109 +++++++++++++--------------
 spec/unit/resource/execute_spec.rb           |  25 +++---
 3 files changed, 76 insertions(+), 161 deletions(-)

diff --git a/lib/chef/resource/execute.rb b/lib/chef/resource/execute.rb
index de927ec06e..b654969cb0 100644
--- a/lib/chef/resource/execute.rb
+++ b/lib/chef/resource/execute.rb
@@ -26,10 +26,9 @@ 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
+      resource_name :execute
 
-      identity_attr :command
-
-      # The ResourceGuardInterpreter wraps a resource's guards in another resource.  That inner resource
+      # 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.
@@ -39,104 +38,28 @@ class Chef
 
       def initialize(name, run_context = nil)
         super
-        @command = name
         @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
-
-      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 :umask, [ String, Integer ]
+      property :command, [ String, Array ], name_property: true, identity: true
+      property :creates, String
+      property :cwd, String
+      property :environment, Hash
+      property :group, [ String, Integer ]
+      property :live_stream, [ TrueClass, FalseClass ], default: false
+      property :returns, [ Integer, Array ], default: 0
+      property :timeout, [ Integer, Float ]
       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
 
+      alias :env :environment
+
       def self.set_guard_inherited_attributes(*inherited_attributes)
         @class_inherited_attributes = inherited_attributes
       end
diff --git a/spec/support/shared/unit/execute_resource.rb b/spec/support/shared/unit/execute_resource.rb
index ae56a9697d..2708eccade 100644
--- a/spec/support/shared/unit/execute_resource.rb
+++ b/spec/support/shared/unit/execute_resource.rb
@@ -20,142 +20,135 @@
 require "spec_helper"
 
 shared_examples_for "an execute resource" do
-
-  before(:each) do
-    @resource = execute_resource
-  end
-
-  it "should create a new Chef::Resource::Execute" do
-    expect(@resource).to be_a_kind_of(Chef::Resource)
-    expect(@resource).to be_a_kind_of(Chef::Resource::Execute)
+  it "resource_name should equal :execute" do
+    expect(resource.resource_name).to eq(:execute)
   end
 
-  it "should set the command to the first argument to new" do
-    expect(@resource.command).to eql(resource_instance_name)
+  it "should set the command to the resource name" do
+    expect(resource.command).to eql("some command")
   end
 
   it "should accept an array on instantiation, too" do
     resource = Chef::Resource::Execute.new(%w{something else})
-    expect(resource).to be_a_kind_of(Chef::Resource)
-    expect(resource).to be_a_kind_of(Chef::Resource::Execute)
+    expect(resource.resource_name).to eq(:execute)
     expect(resource.command).to eql(%w{something else})
   end
 
   it "should accept a string for the command to run" do
-    @resource.command "something"
-    expect(@resource.command).to eql("something")
+    resource.command "something"
+    expect(resource.command).to eql("something")
   end
 
   it "should accept an array for the command to run" do
-    @resource.command %w{something else}
-    expect(@resource.command).to eql(%w{something else})
+    resource.command %w{something else}
+    expect(resource.command).to eql(%w{something else})
   end
 
   it "should accept a string for the cwd" do
-    @resource.cwd "something"
-    expect(@resource.cwd).to eql("something")
+    resource.cwd "something"
+    expect(resource.cwd).to eql("something")
   end
 
   it "should accept a hash for the environment" do
     test_hash = { :one => :two }
-    @resource.environment(test_hash)
-    expect(@resource.environment).to eql(test_hash)
+    resource.environment(test_hash)
+    expect(resource.environment).to eql(test_hash)
   end
 
   it "allows the environment to be specified with #env" do
-    expect(@resource).to respond_to(:env)
+    expect(resource).to respond_to(:env)
   end
 
   it "should accept a string for the group" do
-    @resource.group "something"
-    expect(@resource.group).to eql("something")
+    resource.group "something"
+    expect(resource.group).to eql("something")
   end
 
   it "should accept an integer for the group" do
-    @resource.group 1
-    expect(@resource.group).to eql(1)
+    resource.group 1
+    expect(resource.group).to eql(1)
   end
 
   it "the old path property (that never worked) is not supported in chef >= 13" do
-    expect { @resource.path [ "woot" ] }.to raise_error
+    expect { resource.path [ "woot" ] }.to raise_error
   end
 
   it "should accept an integer for the return code" do
-    @resource.returns 1
-    expect(@resource.returns).to eql(1)
+    resource.returns 1
+    expect(resource.returns).to eql(1)
   end
 
   it "should accept an integer for the timeout" do
-    @resource.timeout 1
-    expect(@resource.timeout).to eql(1)
+    resource.timeout 1
+    expect(resource.timeout).to eql(1)
   end
 
   it "should accept a string for the user" do
-    @resource.user "something"
-    expect(@resource.user).to eql("something")
+    resource.user "something"
+    expect(resource.user).to eql("something")
   end
 
   it "should accept an integer for the user" do
-    @resource.user 1
-    expect(@resource.user).to eql(1)
+    resource.user 1
+    expect(resource.user).to eql(1)
   end
 
   it "should accept a string for the domain" do
-    @resource.domain "mothership"
-    expect(@resource.domain).to eql("mothership")
+    resource.domain "mothership"
+    expect(resource.domain).to eql("mothership")
   end
 
   it "should accept a string for the password" do
-    @resource.password "we.funk!"
-    expect(@resource.password).to eql("we.funk!")
+    resource.password "we.funk!"
+    expect(resource.password).to eql("we.funk!")
   end
 
   it "should accept a string for creates" do
-    @resource.creates "something"
-    expect(@resource.creates).to eql("something")
+    resource.creates "something"
+    expect(resource.creates).to eql("something")
   end
 
   it "should accept a boolean for live streaming" do
-    @resource.live_stream true
-    expect(@resource.live_stream).to be true
+    resource.live_stream true
+    expect(resource.live_stream).to be true
   end
 
   describe "the resource's sensitive attribute" do
     it "should be false by default" do
-      expect(@resource.sensitive).to eq(false)
+      expect(resource.sensitive).to eq(false)
     end
 
     it "should be true if set to true" do
-      expect(@resource.sensitive).to eq(false)
-      @resource.sensitive true
-      expect(@resource.sensitive).to eq(true)
+      expect(resource.sensitive).to eq(false)
+      resource.sensitive true
+      expect(resource.sensitive).to eq(true)
     end
 
     it "should be true if the password is non-nil" do
-      @resource.password("we.funk!")
-      expect(@resource.sensitive).to eq(true)
+      resource.password("we.funk!")
+      expect(resource.sensitive).to eq(true)
     end
 
     it "should be true if the password is non-nil but the value is explicitly set to false" do
-      @resource.password("we.funk!")
-      @resource.sensitive false
-      expect(@resource.sensitive).to eq(true)
+      resource.password("we.funk!")
+      resource.sensitive false
+      expect(resource.sensitive).to eq(true)
     end
 
   end
 
   describe "when it has cwd, environment, group, path, return value, and a user" do
     before do
-      @resource.command("grep")
-      @resource.cwd("/tmp/")
-      @resource.environment({ :one => :two })
-      @resource.group("legos")
-      @resource.returns(1)
-      @resource.user("root")
+      resource.command("grep")
+      resource.cwd("/tmp/")
+      resource.environment({ :one => :two })
+      resource.group("legos")
+      resource.returns(1)
+      resource.user("root")
     end
 
     it "returns the command as its identity" do
-      expect(@resource.identity).to eq("grep")
+      expect(resource.identity).to eq("grep")
     end
   end
 end
diff --git a/spec/unit/resource/execute_spec.rb b/spec/unit/resource/execute_spec.rb
index c99e87b351..469d4105c4 100644
--- a/spec/unit/resource/execute_spec.rb
+++ b/spec/unit/resource/execute_spec.rb
@@ -20,16 +20,15 @@
 require "spec_helper"
 
 describe Chef::Resource::Execute do
-  let(:resource_instance_name) { "some command" }
-  let(:execute_resource) { Chef::Resource::Execute.new(resource_instance_name) }
+  let(:resource) { Chef::Resource::Execute.new("some command") }
   it_behaves_like "an execute resource"
 
   it "default guard interpreter is :execute interpreter" do
-    expect(execute_resource.guard_interpreter).to be(:execute)
+    expect(resource.guard_interpreter).to be(:execute)
   end
 
   it "defaults to not being a guard interpreter" do
-    expect(execute_resource.is_guard_interpreter).to eq(false)
+    expect(resource.is_guard_interpreter).to eq(false)
   end
 
   describe "#qualify_user" do
@@ -40,7 +39,7 @@ describe Chef::Resource::Execute do
       let(:username) { "user@domain" }
 
       it "correctly parses the user and domain" do
-        identity = execute_resource.qualify_user(username, password, domain)
+        identity = resource.qualify_user(username, password, domain)
         expect(identity[:domain]).to eq("domain")
         expect(identity[:user]).to eq("user")
       end
@@ -50,7 +49,7 @@ describe Chef::Resource::Execute do
       let(:username) { "domain\\user" }
 
       it "correctly parses the user and domain" do
-        identity = execute_resource.qualify_user(username, password, domain)
+        identity = resource.qualify_user(username, password, domain)
         expect(identity[:domain]).to eq("domain")
         expect(identity[:user]).to eq("user")
       end
@@ -60,14 +59,14 @@ describe Chef::Resource::Execute do
   shared_examples_for "it received valid credentials" do
     describe "the validation method" do
       it "does not raise an error" do
-        expect { execute_resource.validate_identity_platform(username, password, domain) }.not_to raise_error
+        expect { resource.validate_identity_platform(username, password, domain) }.not_to raise_error
       end
     end
 
     describe "the name qualification method" do
       it "correctly translates the user and domain" do
         identity = nil
-        expect { identity = execute_resource.qualify_user(username, password, domain) }.not_to raise_error
+        expect { identity = resource.qualify_user(username, password, domain) }.not_to raise_error
         expect(identity[:domain]).to eq(domain)
         expect(identity[:user]).to eq(username)
       end
@@ -77,7 +76,7 @@ describe Chef::Resource::Execute do
   shared_examples_for "it received invalid credentials" do
     describe "the validation method" do
       it "raises an error" do
-        expect { execute_resource.validate_identity_platform(username, password, domain, elevated) }.to raise_error(ArgumentError)
+        expect { resource.validate_identity_platform(username, password, domain, elevated) }.to raise_error(ArgumentError)
       end
     end
   end
@@ -85,7 +84,7 @@ describe Chef::Resource::Execute do
   shared_examples_for "it received invalid username and domain" do
     describe "the validation method" do
       it "raises an error" do
-        expect { execute_resource.qualify_user(username, password, domain) }.to raise_error(ArgumentError)
+        expect { resource.qualify_user(username, password, domain) }.to raise_error(ArgumentError)
       end
     end
   end
@@ -93,7 +92,7 @@ describe Chef::Resource::Execute do
   shared_examples_for "it received credentials that are not valid on the platform" do
     describe "the validation method" do
       it "raises an error" do
-        expect { execute_resource.validate_identity_platform(username, password, domain) }.to raise_error(Chef::Exceptions::UnsupportedPlatform)
+        expect { resource.validate_identity_platform(username, password, domain) }.to raise_error(Chef::Exceptions::UnsupportedPlatform)
       end
     end
   end
@@ -101,7 +100,7 @@ describe Chef::Resource::Execute do
   shared_examples_for "a consumer of the Execute resource" do
     context "when running on Windows" do
       before do
-        allow(execute_resource).to receive(:node).and_return({ :platform_family => "windows" })
+        allow(resource).to receive(:node).and_return({ :platform_family => "windows" })
       end
 
       context "when no user, domain, or password is specified" do
@@ -203,7 +202,7 @@ describe Chef::Resource::Execute do
 
     context "when not running on Windows" do
       before do
-        allow(execute_resource).to receive(:node).and_return({ :platform_family => "ubuntu" })
+        allow(resource).to receive(:node).and_return({ :platform_family => "ubuntu" })
       end
 
       context "when no user, domain, or password is specified" do
-- 
cgit v1.2.1