diff options
author | vinay sharma <vinay.sharma@msystechnologies.com> | 2019-05-25 18:32:32 +0530 |
---|---|---|
committer | Bryan McLellan <btm@loftninjas.org> | 2019-05-25 06:02:32 -0700 |
commit | b09187f85157fad6a8f29f08c09a69eac3f0bee5 (patch) | |
tree | 6c1f773504cfd7bd93c44f97425a32e52d538a24 | |
parent | 34d511500a4d5c196417b09abfa83e9f33378052 (diff) | |
download | chef-b09187f85157fad6a8f29f08c09a69eac3f0bee5.tar.gz |
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>
-rw-r--r-- | lib/chef/application.rb | 8 | ||||
-rw-r--r-- | lib/chef/mixin/from_file.rb | 4 | ||||
-rw-r--r-- | spec/functional/mixin/from_file_spec.rb | 11 | ||||
-rw-r--r-- | spec/unit/application/solo_spec.rb | 1 | ||||
-rw-r--r-- | spec/unit/application_spec.rb | 88 | ||||
-rw-r--r-- | spec/unit/environment_spec.rb | 6 | ||||
-rw-r--r-- | spec/unit/role_spec.rb | 9 |
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") |