Merge pull request puppetlabs#1964 from zaphod42/hlindberg-issues/iss…

…ue-19955-data-binding-rescue-and-log

(#19955) Give better error msg when Hiera databindings fail.

lib/puppet/data_binding.rb

@@ -2,6 +2,7 @@
 
 # A class for managing data lookups
 class Puppet::DataBinding
+  class LookupError < Puppet::Error; end
 
   # Set up indirection, so that data can be looked for in the compiler
   extend Puppet::Indirector

lib/puppet/indirector/data_binding/hiera.rb

@@ -1,6 +1,50 @@
-require 'puppet/indirector/hiera'
+require 'puppet/indirector/code'
+require 'hiera/scope'
 
-class Puppet::DataBinding::Hiera < Puppet::Indirector::Hiera
+class Puppet::DataBinding::Hiera < Puppet::Indirector::Code
   desc "Retrieve data using Hiera."
+
+  def initialize(*args)
+    if ! Puppet.features.hiera?
+      raise "Hiera terminus not supported without hiera library"
+    end
+    super
+  end
+
+  if defined?(::Psych::SyntaxError)
+    DataBindingExceptions = [::StandardError, ::Psych::SyntaxError]
+  else
+    DataBindingExceptions = [::StandardError]
+  end
+
+  def find(request)
+    hiera.lookup(request.key, nil, Hiera::Scope.new(request.options[:variables]), nil, nil)
+  rescue *DataBindingExceptions => detail
+    raise Puppet::DataBinding::LookupError.new(detail.message, detail)
+  end
+
+  private
+
+  def self.hiera_config
+    hiera_config = Puppet.settings[:hiera_config]
+    config = {}
+
+    if File.exist?(hiera_config)
+      config = Hiera::Config.load(hiera_config)
+    else
+      Puppet.warning "Config file #{hiera_config} not found, using Hiera defaults"
+    end
+
+    config[:logger] = 'puppet'
+    config
+  end
+
+  def self.hiera
+    @hiera ||= Hiera.new(:config => hiera_config)
+  end
+
+  def hiera
+    self.class.hiera
+  end
 end
 

lib/puppet/resource.rb

@@ -328,16 +328,26 @@ def lookup_external_default_for(param, scope)
       result = scope.compiler.injector.lookup(scope, name)
     end
     if result.nil?
+      lookup_with_databinding(name, scope)
+    else
+      result
+    end
+  end
+
+  private :lookup_external_default_for
+
+  def lookup_with_databinding(name, scope)
+    begin
       Puppet::DataBinding.indirection.find(
         name,
         :environment => scope.environment.to_s,
         :variables => scope)
-    else
-      result
+    rescue Puppet::DataBinding::LookupError => e
+      raise Puppet::Error.new("Error from DataBinding '#{Puppet[:data_binding_terminus]}' while looking up '#{name}': #{e.message}", e)
     end
   end
 
-  private :lookup_external_default_for
+  private :lookup_with_databinding
 
   def set_default_parameters(scope)
     return [] unless resource_type and resource_type.respond_to?(:arguments)

spec/fixtures/unit/indirector/data_binding/hiera/invalid.yaml

@@ -0,0 +1 @@
+{ invalid:

spec/unit/indirector/data_binding/hiera_spec.rb

@@ -2,10 +2,6 @@
 require 'puppet/indirector/data_binding/hiera'
 
 describe Puppet::DataBinding::Hiera do
-  it "should be a subclass of the Hiera terminus" do
-    Puppet::DataBinding::Hiera.superclass.should equal(Puppet::Indirector::Hiera)
-  end
-
   it "should have documentation" do
     Puppet::DataBinding::Hiera.doc.should_not be_nil
   end
@@ -18,4 +14,97 @@
   it "should have its name set to :hiera" do
     Puppet::DataBinding::Hiera.name.should == :hiera
   end
+  include PuppetSpec::Files
+
+  def write_hiera_config(config_file, datadir)
+    File.open(config_file, 'w') do |f|
+      f.write("---
+        :yaml:
+          :datadir: #{datadir}
+        :hierarchy: ['global', 'invalid']
+        :logger: 'noop'
+        :backends: ['yaml']
+      ")
+    end
+  end
+
+  before do
+    hiera_config_file = tmpfile("hiera.yaml")
+    Puppet.settings[:hiera_config] = hiera_config_file
+    write_hiera_config(hiera_config_file, my_fixture_dir)
+  end
+
+  it "should be the default data_binding terminus" do
+    Puppet.settings[:data_binding_terminus].should == :hiera
+  end
+
+  it "should raise an error if we don't have the hiera feature" do
+    Puppet.features.expects(:hiera?).returns(false)
+    lambda { Puppet::DataBinding::Hiera.new }.should raise_error RuntimeError,
+      "Hiera terminus not supported without hiera library"
+  end
+
+  describe "the behavior of the hiera_config method", :if => Puppet.features.hiera? do
+    it "should override the logger and set it to puppet" do
+      Puppet::DataBinding::Hiera.hiera_config[:logger].should == "puppet"
+    end
+
+    context "when the Hiera configuration file does not exist" do
+      let(:path) { File.expand_path('/doesnotexist') }
+
+      before do
+        Puppet.settings[:hiera_config] = path
+      end
+
+      it "should log a warning" do
+        Puppet.expects(:warning).with(
+         "Config file #{path} not found, using Hiera defaults")
+        Puppet::DataBinding::Hiera.hiera_config
+      end
+
+      it "should only configure the logger and set it to puppet" do
+        Puppet.expects(:warning).with(
+         "Config file #{path} not found, using Hiera defaults")
+        Puppet::DataBinding::Hiera.hiera_config.should == { :logger => 'puppet' }
+      end
+    end
+  end
+
+  describe "the behavior of the find method", :if => Puppet.features.hiera? do
+
+    let(:data_binder) { Puppet::DataBinding::Hiera.new }
+
+    it "should support looking up an integer" do
+      data_binder.find(request("integer")).should == 3000
+    end
+
+    it "should support looking up a string" do
+      data_binder.find(request("string")).should == 'apache'
+    end
+
+    it "should support looking up an array" do
+      data_binder.find(request("array")).should == [
+        '0.ntp.puppetlabs.com',
+        '1.ntp.puppetlabs.com',
+      ]
+    end
+
+    it "should support looking up a hash" do
+      data_binder.find(request("hash")).should == {
+        'user'  => 'Hightower',
+        'group' => 'admin',
+        'mode'  => '0644'
+      }
+    end
+
+    it "raises a data binding error if hiera cannot parse the yaml data" do
+      expect do
+        data_binder.find(request('invalid'))
+      end.to raise_error(Puppet::DataBinding::LookupError)
+    end
+  end
+
+  def request(key)
+    Puppet::Indirector::Request.new(:hiera, :find, key, nil)
+  end
 end