Improve errors around loading YAML recipes

YAML is a bit terse. The wrong : or - makes it all fall apart. Be a bit
more helpful.

Signed-off-by: Bryan McLellan <btm@loftninjas.org>

2 files changed, 79 insertions, 13 deletions

diff --git a/lib/chef/recipe.rb b/lib/chef/recipe.rb
index ddb45de8e3..936b1ac9d7 100644
--- a/lib/chef/recipe.rb
+++ b/lib/chef/recipe.rb
@@ -1,7 +1,7 @@
 #--
 # Author:: Adam Jacob (<adam@chef.io>)
 # Author:: Christopher Walters (<cw@chef.io>)
-# Copyright:: Copyright 2008-2018, Chef Software Inc.
+# Copyright:: Copyright 2008-2020, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -88,26 +88,24 @@ class Chef
     def from_yaml_file(filename)
       self.source_file = filename
       if File.file?(filename) && File.readable?(filename)
-        from_yaml(IO.read(filename))
+        yaml_contents = IO.read(filename)
+        if YAML.load_stream(yaml_contents).length > 1
+          raise ArgumentError, "YAML recipe '#{filename}' contains multiple documents, only one is supported"
+        end
+
+        from_yaml(yaml_contents)
       else
-        raise IOError, "Cannot open or read #{filename}!"
+        raise IOError, "Cannot open or read file '#{filename}'!"
       end
     end
 
     def from_yaml(string)
       res = ::YAML.safe_load(string)
-      if res.is_a?(Hash)
-        from_hash(res)
-      elsif res.is_a?(Array)
-        from_array(res)
-      else
-        raise "boom"
+      unless res.is_a?(Hash) && res.key?("resources")
+        raise ArgumentError, "YAML recipe '#{source_file}' must contain a top-level 'resources' hash (YAML sequence), i.e. 'resources:'"
       end
-    end
 
-    def from_array(array)
-      Chef::Log.warn "array yaml files are super duper experimental behavior"
-      array.each { |e| from_hash(e) }
+      from_hash(res)
     end
 
     def from_hash(hash)
diff --git a/spec/unit/recipe_spec.rb b/spec/unit/recipe_spec.rb
index 099e6750f6..ef8fb78f90 100644
--- a/spec/unit/recipe_spec.rb
+++ b/spec/unit/recipe_spec.rb
@@ -576,6 +576,74 @@ describe Chef::Recipe do
     end
   end
 
+  describe "from_yaml_file" do
+    it "raises ArgumentError if the YAML file contains multiple documents" do
+      filename = "multiple_docs.yaml"
+      yaml = "---\n- resources:\n  - type: false\n---\n-resources:\n  - type: false\n"
+      allow(File).to receive(:file?).and_call_original
+      allow(File).to receive(:readable?).and_call_original
+      allow(IO).to receive(:read).and_call_original
+      allow(File).to receive(:file?).with(filename).and_return(true)
+      allow(File).to receive(:readable?).with(filename).and_return(true)
+      allow(IO).to receive(:read).with(filename).and_return(yaml)
+      expect { recipe.from_yaml_file(filename) }.to raise_error(ArgumentError, /contains multiple documents/)
+    end
+
+    it "raises IOError if the file does not exist" do
+      filename = "/nonexistent"
+      allow(File).to receive(:file?).and_call_original
+      allow(File).to receive(:file?).with(filename).and_return(false)
+      expect { recipe.from_yaml_file(filename) }.to raise_error(IOError, /Cannot open or read/)
+    end
+  end
+
+  describe "from_yaml" do
+    it "raises ArgumentError if the YAML is not a top-level hash" do
+      yaml = <<~YAML
+        ---
+        - one
+        - resources
+        - three
+      YAML
+      expect { recipe.from_yaml(yaml) }.to raise_error(ArgumentError, /must contain a top-level 'resources' hash/)
+    end
+
+    it "raises ArgumentError if the YAML does not contain a resources hash" do
+      yaml = <<~YAML
+        ---
+        - airplanes:
+          - type: "execute"
+            command: "whoami"
+      YAML
+      expect { recipe.from_yaml(yaml) }.to raise_error(ArgumentError, /must contain a top-level 'resources' hash/)
+    end
+
+    it "does not raise if the YAML contains a resources hash" do
+      yaml = <<~YAML
+        ---
+        resources:
+        - type: "execute"
+          command: "whoami"
+      YAML
+      expect(recipe).to receive(:from_hash).with({ "resources" => [{ "command" => "whoami", "type" => "execute" }] })
+      recipe.from_yaml(yaml)
+    end
+  end
+
+  describe "from_hash" do
+    it "declares resources from a hash" do
+      resources = { "resources" => [
+        { "name" => "running some commands", "type" => "execute", "command" => "whoami" },
+        { "name" => "preparing the bits", "type" => "service", "action" => "start", "service_name" => "bit_launcher" },
+      ] }
+
+      recipe.from_hash(resources)
+      expect(recipe.resources(execute: "running some commands").command).to eql("whoami")
+      expect(recipe.resources(service: "preparing the bits").service_name).to eql("bit_launcher")
+      expect(recipe.resources(service: "preparing the bits").action).to eql([:start])
+    end
+  end
+
   describe "included DSL" do
     it "should respond to :ps_credential from Chef::DSL::Powershell" do
       expect(recipe.respond_to?(:ps_credential)).to be true