cloudfoundry · cf-bosh · Mar 29, 2013 · Mar 29, 2013 · Mar 29, 2013
diff --git a/bosh_cli/lib/cli/core_ext.rb b/bosh_cli/lib/cli/core_ext.rb
@@ -67,17 +67,25 @@ def format_time(time)
 
   def load_yaml_file(path, expected_type = Hash)
     err("Cannot find file `#{path}'".red) unless File.exist?(path)
-    yaml = Psych::load(ERB.new(File.read(path)).result)
 
-    if expected_type && !yaml.is_a?(expected_type)
-      err("Incorrect file format in `#{path}', #{expected_type} expected")
+    begin
+      yaml_str = ERB.new(File.read(path)).result
+    rescue SystemCallError => e
+      err("Cannot load YAML file at `#{path}': #{e}".red)
+    end
+
+    begin
+      Bosh::Cli::YamlHelper.check_duplicate_keys(yaml_str)
+    rescue => e
+      err("Incorrect YAML structure in `#{path}': #{e}".red)
     end
 
-    Bosh::Cli::YamlHelper.check_duplicate_keys(path)
+    yaml = Psych::load(yaml_str)
+    if expected_type && !yaml.is_a?(expected_type)
+      err("Incorrect YAML structure in `#{path}': expected #{expected_type} at the root".red)
+    end
 
     yaml
-  rescue SystemCallError => e
-    err("Cannot load YAML file at `#{path}': #{e}".red)
   end
 
   def write_yaml(manifest, path)

diff --git a/bosh_cli/lib/cli/yaml_helper.rb b/bosh_cli/lib/cli/yaml_helper.rb
@@ -11,6 +11,9 @@ def process_object(o)
           when Psych::Nodes::Sequence
             process_sequence(o)
           when Psych::Nodes::Scalar
+            # noop
+          when Psych::Nodes::Alias
+            # noop
           else
             err("Unhandled class #{o.class}, fix yaml duplicate check")
         end
@@ -29,7 +32,7 @@ def process_mapping(m)
         m.children.each_with_index do |key_or_value, index|
           next if index.odd? # skip the values
           key = key_or_value.value # Sorry this is confusing, Psych mappings don't behave nicely like maps
-          raise "Found duplicate key #{key}" if s.include?(key)
+          raise "Found duplicate key '#{key}'" if s.include?(key)
           s.add(key)
         end
 
@@ -40,20 +43,11 @@ def process_mapping(m)
       end
 
       public
-      def check_duplicate_keys(path)
-        begin
-          string_with_erb = File.read(path)
-          yaml = ERB.new(string_with_erb).result
-          document = Psych.parse(yaml)
-          process_mapping(document.root) if document
-        rescue => e
-          if e.message.include? "Found duplicate key"
-            raise "Bad YAML file when parsing file: #{path}\n#{e.message}"
-          else
-            raise e
-          end
-        end
+
+      def check_duplicate_keys(yaml_str)
+        document = Psych.parse(yaml_str)
+        process_mapping(document.root) if document
       end
     end
   end
-end
+end
diff --git a/bosh_cli/spec/unit/blob_manager_spec.rb b/bosh_cli/spec/unit/blob_manager_spec.rb
@@ -49,9 +49,10 @@ def make_manager(release)
       File.open(File.join(@config_dir, "blobs.yml"), "w") do |f|
         Psych.dump("string", f)
       end
+
       expect {
         make_manager(@release)
-      }.to raise_error(/Incorrect file format/)
+      }.to raise_error(/Incorrect YAML structure/)
     end
 
     it "migrates legacy index file" do

diff --git a/bosh_cli/spec/unit/core_ext_spec.rb b/bosh_cli/spec/unit/core_ext_spec.rb
@@ -84,21 +84,27 @@
     o.should_not be_blank
   end
 
-  describe "loading YAML files" do
+  describe "#load_yaml_file" do
     it "can load YAML files with ERB" do
       load_yaml_file(spec_asset("dummy.yml.erb")).should == {"four" => 4}
     end
 
     it "gives a nice error when the file cannot be found" do
-      expect { load_yaml_file("non-existent.yml") }.to raise_error(Bosh::Cli::CliError, "Cannot find file `non-existent.yml'")
+      expect {
+        load_yaml_file("non-existent.yml")
+      }.to raise_error(Bosh::Cli::CliError, "Cannot find file `non-existent.yml'")
     end
 
     it "gives a nice error when the parsed YAML is not a Hash" do
-      expect { load_yaml_file(spec_asset("not_a_hash.yml")) }.to raise_error(Bosh::Cli::CliError, /Incorrect file format in/)
+      expect {
+        load_yaml_file(spec_asset("not_a_hash.yml"))
+      }.to raise_error(Bosh::Cli::CliError, /Incorrect YAML structure .* expected Hash/)
     end
 
     it "gives a nice error when the parsed YAML produces a hash with repeated keys" do
-      expect { load_yaml_file(spec_asset("duplicate_keys.yml")) }.to raise_error(RuntimeError, /Bad YAML file/)
+      expect {
+        load_yaml_file(spec_asset("duplicate_keys.yml"))
+      }.to raise_error(Bosh::Cli::CliError, /Incorrect YAML structure .* duplicate key 'unique_key'/)
     end
   end
 end
diff --git a/bosh_cli/spec/unit/yaml_helper_spec.rb b/bosh_cli/spec/unit/yaml_helper_spec.rb
@@ -0,0 +1,21 @@
+# Copyright (c) 2009-2012 VMware, Inc.
+
+require "spec_helper"
+
+describe Bosh::Cli::YamlHelper do
+  subject { Bosh::Cli::YamlHelper }
+
+  describe "#check_duplicate_keys" do
+    context "when yaml contains anchors" do
+      it "does not raise an error" do
+        subject.check_duplicate_keys("key1: &key1")
+      end
+    end
+
+    context "when yaml contains aliases" do
+      it "does not raise an error" do
+        subject.check_duplicate_keys("key1: *key1")
+      end
+    end
+  end
+end