Config validation updates

Signed-off-by: Vivek Singh <vivek.singh@msystechnologies.com>

2 files changed, 28 insertions, 29 deletions

diff --git a/lib/chef/data_collector/config_validation.rb b/lib/chef/data_collector/config_validation.rb
index 06b0c84c28..a58472a82b 100644
--- a/lib/chef/data_collector/config_validation.rb
+++ b/lib/chef/data_collector/config_validation.rb
@@ -110,16 +110,10 @@ class Chef
 
         # validate an output_location file
         def validate_file!(file)
-          File.open(File.expand_path(file), "a") {}
-        rescue Errno::ENOENT
+          return true if Chef::Config.path_accessible?(File.expand_path(file))
+
           raise Chef::Exceptions::ConfigurationError,
             "Chef::Config[:data_collector][:output_locations][:files] contains the location #{file}, which is a non existent file path."
-        rescue Errno::EACCES
-          raise Chef::Exceptions::ConfigurationError,
-            "Chef::Config[:data_collector][:output_locations][:files] contains the location #{file}, which cannot be written to by Chef."
-        rescue Exception => e
-          raise Chef::Exceptions::ConfigurationError,
-            "Chef::Config[:data_collector][:output_locations][:files] contains the location #{file}, which is invalid: #{e.message}."
         end
 
         # validate an output_location url
@@ -130,16 +124,14 @@ class Chef
             "Chef::Config[:data_collector][:output_locations][:urls] contains the url #{url} which is not valid."
         end
 
-        # Validate a non-empty hash that includes either of keys of both.
+        # Validate the hash contains at least one of the given keys.
         #
-        # @param hash [Hash] the hash contains data collector output_locations.
-        # @param keys [Array] the multiple keys as arguments array.
-        # @return [Boolean] true if the hash contains either of keys or both.
+        # @param hash [Hash] the hash to be validated.
+        # @param keys [Array] an array of keys to check existence of in the hash.
+        # @return [Boolean] true if the hash contains any of the given keys.
         #
         def valid_hash_with_keys?(hash, *keys)
-          return false if hash.empty? || !hash.is_a?(Hash)
-
-          keys.any? { |k| hash.key?(k) }
+          hash.is_a?(Hash) && keys.any? { |k| hash.key?(k) }
         end
       end
     end
diff --git a/spec/unit/data_collector/config_validation_spec.rb b/spec/unit/data_collector/config_validation_spec.rb
index 38f7dac554..d00811f3ef 100644
--- a/spec/unit/data_collector/config_validation_spec.rb
+++ b/spec/unit/data_collector/config_validation_spec.rb
@@ -179,22 +179,29 @@ describe Chef::DataCollector::ConfigValidation do
       expect { Chef::DataCollector::ConfigValidation.validate_output_locations! }.not_to raise_error
     end
 
-    it "with valid files options" do
-      allow(File).to receive(:open).and_return(true)
-      Chef::Config[:data_collector][:output_locations] = { files: ["/tmp/client-runs.txt"] }
-      expect { Chef::DataCollector::ConfigValidation.validate_output_locations! }.not_to raise_error
-    end
+    context "output_locations contains files" do
+      let(:file_path) { "/tmp/client-runs.txt" }
 
-    it "with valid files & URLs options" do
-      allow(File).to receive(:open).and_return(true)
-      Chef::Config[:data_collector][:output_locations] = { urls: ["https://www.esa.local/ariane5/data-collector"], files: ["/tmp/client-runs.txt"] }
-      expect { Chef::DataCollector::ConfigValidation.validate_output_locations! }.not_to raise_error
-    end
+      before(:each) do
+        allow(File).to receive(:exist?).with(file_path).and_return(true)
+        allow(File).to receive(:readable?).with(file_path).and_return(true)
+        allow(File).to receive(:writable?).with(file_path).and_return(true)
+      end
 
-    it "with valid files options & String location value" do
-      allow(File).to receive(:open).and_return(true)
-      Chef::Config[:data_collector][:output_locations] = { files: "/tmp/client-runs.txt" }
-      expect { Chef::DataCollector::ConfigValidation.validate_output_locations! }.not_to raise_error
+      it "with valid files options" do
+        Chef::Config[:data_collector][:output_locations] = { files: [file_path] }
+        expect { Chef::DataCollector::ConfigValidation.validate_output_locations! }.not_to raise_error
+      end
+
+      it "with valid files & URLs options" do
+        Chef::Config[:data_collector][:output_locations] = { urls: ["https://www.esa.local/ariane5/data-collector"], files: [file_path] }
+        expect { Chef::DataCollector::ConfigValidation.validate_output_locations! }.not_to raise_error
+      end
+
+      it "with valid files options & String location value" do
+        Chef::Config[:data_collector][:output_locations] = { files: file_path }
+        expect { Chef::DataCollector::ConfigValidation.validate_output_locations! }.not_to raise_error
+      end
     end
   end
 end