Fixed issue for chef-client run was throwing error when provided empty string with it (#8200)

* fixed issue for chef-client run was throwing error when provided empty string with it

Signed-off-by: vinay033 <vsharma@chef.io>

7 files changed, 119 insertions, 8 deletions

diff --git a/lib/chef/application.rb b/lib/chef/application.rb
index 3093d358ea..f9f1056f44 100644
--- a/lib/chef/application.rb
+++ b/lib/chef/application.rb
@@ -155,10 +155,16 @@ class Chef
       Chef::Application.fatal!(e.message)
     end
 
+    # Set the specific recipes to Chef::Config if the recipes are valid
+    # otherwise log a fatal error message and exit the application.
     def set_specific_recipes
-      if cli_arguments.respond_to?(:map)
+      if cli_arguments.is_a?(Array) &&
+          (cli_arguments.empty? || cli_arguments.all? { |file| File.file?(file) } )
         chef_config[:specific_recipes] =
           cli_arguments.map { |file| File.expand_path(file) }
+      else
+        Chef::Application.fatal!("Invalid arguments are not supported by the chef-client: \"" +
+          cli_arguments.select { |file| !File.file?(file) }.join('", "') + '"')
       end
     end
 
diff --git a/lib/chef/mixin/from_file.rb b/lib/chef/mixin/from_file.rb
index e19597dde8..b8300340c7 100644
--- a/lib/chef/mixin/from_file.rb
+++ b/lib/chef/mixin/from_file.rb
@@ -30,7 +30,7 @@ class Chef
       # Raises an IOError if the file cannot be found, or is not readable.
       def from_file(filename)
         self.source_file = filename
-        if File.exists?(filename) && File.readable?(filename)
+        if File.file?(filename) && File.readable?(filename)
           instance_eval(IO.read(filename), filename, 1)
         else
           raise IOError, "Cannot open or read #{filename}!"
@@ -43,7 +43,7 @@ class Chef
       # Raises an IOError if the file cannot be found, or is not readable.
       def class_from_file(filename)
         self.source_file = filename
-        if File.exists?(filename) && File.readable?(filename)
+        if File.file?(filename) && File.readable?(filename)
           class_eval(IO.read(filename), filename, 1)
         else
           raise IOError, "Cannot open or read #{filename}!"
diff --git a/spec/functional/mixin/from_file_spec.rb b/spec/functional/mixin/from_file_spec.rb
index a279f48790..d8519b4185 100644
--- a/spec/functional/mixin/from_file_spec.rb
+++ b/spec/functional/mixin/from_file_spec.rb
@@ -21,6 +21,7 @@ describe Chef::Mixin::FromFile do
   REAL_DATA = File.join(CHEF_SPEC_DATA, "mixin", "real_data.rb")
   INVALID_DATA = File.join(CHEF_SPEC_DATA, "mixin", "invalid_data.rb")
   NO_DATA = File.join(CHEF_SPEC_DATA, "mixin", "non_existant_data.rb")
+  DIRECTORY = File.expand_path("")
 
   class TestData
     include Chef::Mixin::FromFile
@@ -78,5 +79,15 @@ describe Chef::Mixin::FromFile do
       datum = TestData.new
       expect { datum.from_file(NO_DATA) }.to raise_error(IOError)
     end
+
+    it "should fail if it's a directory not a file" do
+      datum = TestData.new
+      expect { datum.from_file(DIRECTORY) }.to raise_error(IOError)
+    end
+
+    it "should fail class if it's a directory not a file" do
+      datum = ClassTestData
+      expect { datum.from_file(DIRECTORY) }.to raise_error(IOError)
+    end
   end
 end
diff --git a/spec/unit/application/solo_spec.rb b/spec/unit/application/solo_spec.rb
index 74f71a9115..3f540d24e2 100644
--- a/spec/unit/application/solo_spec.rb
+++ b/spec/unit/application/solo_spec.rb
@@ -27,6 +27,7 @@ describe Chef::Application::Solo do
     allow(app).to receive(:configure_chef).and_return(true)
     allow(app).to receive(:configure_logging).and_return(true)
     allow(app).to receive(:trap)
+    allow(app).to receive(:cli_arguments).and_return([])
 
     Chef::Config[:json_attribs] = false
     Chef::Config[:solo] = true
diff --git a/spec/unit/application_spec.rb b/spec/unit/application_spec.rb
index 10dff0e250..e94f9b74ae 100644
--- a/spec/unit/application_spec.rb
+++ b/spec/unit/application_spec.rb
@@ -393,6 +393,94 @@ describe Chef::Application do
 
   end
 
+  describe "#set_specific_recipes" do
+    let(:app) { Chef::Application.new }
+    context "when cli arguments does not contain any values" do
+      before do
+        allow(app).to receive(:cli_arguments).and_return([])
+      end
+
+      it "returns an empty array" do
+        app.set_specific_recipes
+        expect(Chef::Config[:specific_recipes]).to eq([])
+      end
+    end
+
+    context "when cli arguments contain valid recipe file path" do
+      let(:tempfile) { Tempfile.new("default.rb").path }
+      before do
+        allow(app).to receive(:cli_arguments).and_return([tempfile])
+      end
+
+      it "sets the specific recipes to config" do
+        app.set_specific_recipes
+        expect(Chef::Config[:specific_recipes]).to eq([tempfile])
+      end
+    end
+
+    context "when cli arguments contain invalid recipe file path" do
+      let(:fatal) { false }
+      before do
+        tempfile = "/root/default.rb"
+        allow(app).to receive(:cli_arguments).and_return([tempfile])
+        allow(Chef::Application).to receive(:fatal!).and_return(fatal)
+      end
+
+      it "raises an error with application exit" do
+        expect(app.set_specific_recipes).to eq(fatal)
+      end
+    end
+
+    context "when cli arguments contain empty string" do
+      let(:fatal) { false }
+      before do
+        allow(app).to receive(:cli_arguments).and_return([""])
+        allow(Chef::Application).to receive(:fatal!).and_return(fatal)
+      end
+
+      it "raises an arguments error" do
+        expect(app.set_specific_recipes).to eq(fatal)
+      end
+    end
+
+    context "when cli arguments contain any string" do
+      let(:fatal) { false }
+      before do
+        allow(app).to receive(:cli_arguments).and_return(["test"])
+        allow(Chef::Application).to receive(:fatal!).and_return(fatal)
+      end
+
+      it "raises an arguments error" do
+        expect(app.set_specific_recipes).to eq(fatal)
+      end
+    end
+
+    context "when cli arguments contain multiple invalid strings" do
+      let(:fatal) { false }
+      before do
+        allow(app).to receive(:cli_arguments).and_return(["", "test"])
+        allow(Chef::Application).to receive(:fatal!).and_return(fatal)
+      end
+
+      it "raises an arguments error" do
+        expect(app.set_specific_recipes).to eq(fatal)
+      end
+    end
+
+    context "when cli arguments contain valid recipe file path and invalid string" do
+      let(:fatal) { false }
+      before do
+        tempfile = Tempfile.new("default.rb").path
+        allow(app).to receive(:cli_arguments).and_return([tempfile, "test"])
+        allow(Chef::Application).to receive(:fatal!).and_return(fatal)
+      end
+
+      it "raises an arguments error" do
+        expect(app.set_specific_recipes).to eq(fatal)
+      end
+    end
+  end
+
   describe "configuration errors" do
     before do
       expect(Process).to receive(:exit)
diff --git a/spec/unit/environment_spec.rb b/spec/unit/environment_spec.rb
index 1841707c2d..8103adc8fa 100644
--- a/spec/unit/environment_spec.rb
+++ b/spec/unit/environment_spec.rb
@@ -403,8 +403,9 @@ describe Chef::Environment do
       it "should get the environment from the environment_path" do
         expect(File).to receive(:directory?).with(Chef::Config[:environment_path]).and_return(true)
         expect(File).to receive(:exists?).with(File.join(Chef::Config[:environment_path], "foo.json")).and_return(false)
-        expect(File).to receive(:exists?).with(File.join(Chef::Config[:environment_path], "foo.rb")).exactly(2).times.and_return(true)
+        expect(File).to receive(:exists?).with(File.join(Chef::Config[:environment_path], "foo.rb")).exactly(1).times.and_return(true)
         expect(File).to receive(:readable?).with(File.join(Chef::Config[:environment_path], "foo.rb")).and_return(true)
+        expect(File).to receive(:file?).with(File.join(Chef::Config[:environment_path], "foo.rb")).and_return(true)
         role_dsl = "name \"foo\"\ndescription \"desc\"\n"
         expect(IO).to receive(:read).with(File.join(Chef::Config[:environment_path], "foo.rb")).and_return(role_dsl)
         Chef::Environment.load("foo")
@@ -436,8 +437,9 @@ describe Chef::Environment do
       it "should return a Chef::Environment object from Ruby DSL" do
         expect(File).to receive(:directory?).with(Chef::Config[:environment_path]).and_return(true)
         expect(File).to receive(:exists?).with(File.join(Chef::Config[:environment_path], "foo.json")).and_return(false)
-        expect(File).to receive(:exists?).with(File.join(Chef::Config[:environment_path], "foo.rb")).exactly(2).times.and_return(true)
+        expect(File).to receive(:exists?).with(File.join(Chef::Config[:environment_path], "foo.rb")).exactly(1).times.and_return(true)
         expect(File).to receive(:readable?).with(File.join(Chef::Config[:environment_path], "foo.rb")).and_return(true)
+        expect(File).to receive(:file?).with(File.join(Chef::Config[:environment_path], "foo.rb")).and_return(true)
         role_dsl = "name \"foo\"\ndescription \"desc\"\n"
         expect(IO).to receive(:read).with(File.join(Chef::Config[:environment_path], "foo.rb")).and_return(role_dsl)
         environment = Chef::Environment.load("foo")
diff --git a/spec/unit/role_spec.rb b/spec/unit/role_spec.rb
index 0e12f65e58..46ee71bb1c 100644
--- a/spec/unit/role_spec.rb
+++ b/spec/unit/role_spec.rb
@@ -268,8 +268,9 @@ describe Chef::Role do
     it "should return a Chef::Role object from a Ruby DSL" do
       expect(Dir).to receive(:glob).and_return(["#{Chef::Config[:role_path]}/memes", "#{Chef::Config[:role_path]}/memes/lolcat.rb"])
       rb_path = File.join(Chef::Config[:role_path], "memes/lolcat.rb")
-      expect(File).to receive(:exists?).with(rb_path).exactly(2).times.and_return(true)
+      expect(File).to receive(:exists?).with(rb_path).exactly(1).times.and_return(true)
       expect(File).to receive(:readable?).with(rb_path).exactly(1).times.and_return(true)
+      expect(File).to receive(:file?).with(rb_path).exactly(1).times.and_return(true)
       expect(IO).to receive(:read).with(rb_path).and_return(ROLE_DSL)
       expect(@role).to be_a_kind_of(Chef::Role)
       @role.class.from_disk("lolcat")
@@ -331,8 +332,9 @@ describe Chef::Role do
 
     it "should return a Chef::Role object from a Ruby DSL" do
       expect(Dir).to receive(:glob).with(File.join("/path1", "**", "**")).exactly(1).times.and_return(["/path1/lolcat.rb"])
-      expect(File).to receive(:exists?).with("/path1/lolcat.rb").exactly(2).times.and_return(true)
+      expect(File).to receive(:exists?).with("/path1/lolcat.rb").exactly(1).times.and_return(true)
       expect(File).to receive(:readable?).with("/path1/lolcat.rb").and_return(true)
+      expect(File).to receive(:file?).with("/path1/lolcat.rb").and_return(true)
       expect(IO).to receive(:read).with("/path1/lolcat.rb").exactly(1).times.and_return(ROLE_DSL)
       expect(@role).to be_a_kind_of(Chef::Role)
       @role.class.from_disk("lolcat")
@@ -341,8 +343,9 @@ describe Chef::Role do
     it "should return a Chef::Role object from a Ruby DSL when role is in the second path" do
       expect(Dir).to receive(:glob).with(File.join("/path1", "**", "**")).exactly(1).times.and_return([])
       expect(Dir).to receive(:glob).with(File.join("/path/path2", "**", "**")).exactly(1).times.and_return(["/path/path2/lolcat.rb"])
-      expect(File).to receive(:exists?).with("/path/path2/lolcat.rb").exactly(2).times.and_return(true)
+      expect(File).to receive(:exists?).with("/path/path2/lolcat.rb").exactly(1).times.and_return(true)
       expect(File).to receive(:readable?).with("/path/path2/lolcat.rb").and_return(true)
+      expect(File).to receive(:file?).with("/path/path2/lolcat.rb").and_return(true)
       expect(IO).to receive(:read).with("/path/path2/lolcat.rb").exactly(1).times.and_return(ROLE_DSL)
       expect(@role).to be_a_kind_of(Chef::Role)
       @role.class.from_disk("lolcat")