Please review pull request #718: (#8235) Add plug-in system for tools like Hiera opened by (kelseyhightower)
Description:
A new data_binding indirection is being added for interfacing with
tools like Hiera; and provides an "in-dsl" solution for data separation.
Data exposed by tools like Hiera will be made available during parse
time.
For example, if we had the follow class definition:
class nginx($port=80) { ... }
And it was declared without specifying the port attribute:
class {'nginx': }The above would result in a data lookup to the data_binding backend for
the following namespaced key:nginx::portNamespaced keys are being used to prevent the pollution of the global
namespace. The convention for namespaced keys is a combination of the
class name and the class parameter being resolved.Example:
class ssh::server($port) { ... } # Lookup ssh::server::portThe data_binding backend would need to provide a value for the
namespaced key or return nil. When a nil value is returned by the
data_binding backend the class parameter default, if available, will be
used instead.It should be noted that class parameters are looked up one at a time
with no assumptions in regards to the performance of data lookups. It's
the responsibility of the data_binding backend to handle any performance
requirements including caching.This patch also adds two new terminii: hiera and none. The "none"
terminus implements a find method that always returns nil; it's being
set as the default data_binding terminus to allow end-users the ability
to opt-in and maintain backwards compatibility with older versions of
Puppet.The "hiera" terminus implements a find method that delegates class
parameter lookups to Hiera using the Hiera Ruby API.Two new configuration settings are being added:
[main] hiera_config = "$confdir/hiera.yaml" data_binding_terminus = "none"Updated specs are included in this patch
- Opened: Thu Apr 26 12:20:49 UTC 2012
- Based on: puppetlabs:master (df8db7c6b298fc02637f936b8b0609ee8ca2bb0a)
- Requested merge: kelseyhightower:ticket/master/8235_plugin_system_for_tools_like_hiera (3ac5b50ce66948d9488bd394c169022d55ecf0d1)
Diff follows:
diff --git a/lib/puppet.rb b/lib/puppet.rb
index d1643f6..aa8301d 100644
--- a/lib/puppet.rb
+++ b/lib/puppet.rb
@@ -135,6 +135,7 @@ def self.newtype(name, options = {}, &block)
require 'puppet/network'
require 'puppet/ssl'
require 'puppet/module'
+require 'puppet/data_binding'
require 'puppet/util/storage'
require 'puppet/status'
require 'puppet/file_bucket/file'
diff --git a/lib/puppet/data_binding.rb b/lib/puppet/data_binding.rb
new file mode 100644
index 0000000..009e75e
--- /dev/null
+++ b/lib/puppet/data_binding.rb
@@ -0,0 +1,11 @@
+require 'puppet/indirector'
+
+# A class for managing data lookups
+class Puppet::DataBinding
+
+ # Set up indirection, so that data can be looked for in the complier
+ extend Puppet::Indirector
+
+ indirects(:data_binding, :terminus_setting => :data_binding_terminus,
+ :doc => "Where to find external data bindings.")
+end
diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb
index 5f01c94..808b1cd 100644
--- a/lib/puppet/defaults.rb
+++ b/lib/puppet/defaults.rb
@@ -228,6 +228,15 @@ module Puppet
:default => "plain",
:desc => "Where to find information about nodes.",
},
+ :data_binding_terminus => {
+ :default => "none",
+ :desc => "Where to retrive information about data.",
+ },
+ :hiera_config => {
+ :default => "$confdir/hiera.yaml",
+ :desc => "The hiera configuration file",
+ :type => :file,
+ },
:catalog_terminus => {
:default => "compiler",
:desc => "Where to get node catalogs. This is useful to change if, for instance,
diff --git a/lib/puppet/feature/base.rb b/lib/puppet/feature/base.rb
index cb05675..fc2cdb3 100644
--- a/lib/puppet/feature/base.rb
+++ b/lib/puppet/feature/base.rb
@@ -67,3 +67,6 @@
# We have sqlite
Puppet.features.add(:sqlite, :libs => ["sqlite3"])
+
+# We have Hiera
+Puppet.features.add(:hiera, :libs => ["hiera"])
diff --git a/lib/puppet/indirector/data_binding/hiera.rb b/lib/puppet/indirector/data_binding/hiera.rb
new file mode 100644
index 0000000..5271b99
--- /dev/null
+++ b/lib/puppet/indirector/data_binding/hiera.rb
@@ -0,0 +1,6 @@
+require 'puppet/indirector/hiera'
+
+class Puppet::DataBinding::Hiera < Puppet::Indirector::Hiera
+ desc "Retrieve data using Hiera."
+end
+
diff --git a/lib/puppet/indirector/data_binding/none.rb b/lib/puppet/indirector/data_binding/none.rb
new file mode 100644
index 0000000..f0a2fcc
--- /dev/null
+++ b/lib/puppet/indirector/data_binding/none.rb
@@ -0,0 +1,5 @@
+require 'puppet/indirector/none'
+
+class Puppet::DataBinding::None < Puppet::Indirector::None
+ desc "A Dummy terminus that always returns nil for data lookups."
+end
diff --git a/lib/puppet/indirector/hiera.rb b/lib/puppet/indirector/hiera.rb
new file mode 100644
index 0000000..b36be06
--- /dev/null
+++ b/lib/puppet/indirector/hiera.rb
@@ -0,0 +1,26 @@
+require 'puppet/indirector/terminus'
+
+class Puppet::Indirector::Hiera < Puppet::Indirector::Terminus
+ def initialize(*args)
+ if ! Puppet.features.hiera?
+ raise "Hiera terminus not supported without hiera gem"
+ end
+ super
+ end
+
+ def find(request)
+ facts = Puppet::Node::Facts.indirection.find(request.options[:host]).values
+ hiera.lookup(request.key, nil, facts, nil, nil)
+ end
+
+ private
+
+ def self.hiera
+ @hiera || Hiera.new(:config => Puppet.settings[:hiera_config])
+ end
+
+ def hiera
+ self.class.hiera
+ end
+end
+
diff --git a/lib/puppet/indirector/none.rb b/lib/puppet/indirector/none.rb
new file mode 100644
index 0000000..69c888f
--- /dev/null
+++ b/lib/puppet/indirector/none.rb
@@ -0,0 +1,9 @@
+require 'puppet/indirector/terminus'
+
+# A none terminus type, meant to always return nil
+class Puppet::Indirector::None < Puppet::Indirector::Terminus
+ def find(request)
+ return nil
+ end
+end
+
diff --git a/lib/puppet/resource.rb b/lib/puppet/resource.rb
index 5fc088a..a1057da 100644
--- a/lib/puppet/resource.rb
+++ b/lib/puppet/resource.rb
@@ -304,9 +304,30 @@ def set_default_parameters(scope)
fail Puppet::DevError, "Cannot evaluate default parameters for #{self} - not a parser resource"
end
- next if default.nil?
+ # Consult external data bindings for class parameter values which must be
+ # namespaced in the backend.
+ #
+ # Example:
+ #
+ # class foo($port){ ... }
+ #
+ # We make a request to the backend for the key 'foo::port' not 'foo'
+ #
+ external_value = nil
+ if resource_type.type == :hostclass
+ namespaced_param = "#{resource_type.name}::#{param}"
+ external_value = Puppet::DataBinding.indirection.find(
+ namespaced_param, :host => scope.host)
+ end
- self[param] = default.safeevaluate(scope)
+ if external_value.nil?
+ next if default.nil?
+ value = default.safeevaluate(scope)
+ else
+ value = external_value
+ end
+
+ self[param] = value
result << param
end
result
diff --git a/spec/integration/defaults_spec.rb b/spec/integration/defaults_spec.rb
index 27fefe8..0737fb6 100755
--- a/spec/integration/defaults_spec.rb
+++ b/spec/integration/defaults_spec.rb
@@ -310,4 +310,20 @@
Puppet.settings[:diff].should == ''
end
end
+
+ describe "when configuring hiera" do
+ it "should have a hiera_config setting" do
+ Puppet.settings[:hiera_config].should_not be_nil
+ end
+ end
+
+ describe "when configuring the data_binding terminus" do
+ it "should have a data_binding_terminus setting" do
+ Puppet.settings[:data_binding_terminus].should_not be_nil
+ end
+
+ it "should be set to none by default" do
+ Puppet.settings[:data_binding_terminus].should == 'none'
+ end
+ end
end
diff --git a/spec/unit/data_binding_spec.rb b/spec/unit/data_binding_spec.rb
new file mode 100644
index 0000000..df181a1
--- /dev/null
+++ b/spec/unit/data_binding_spec.rb
@@ -0,0 +1,11 @@
+require 'spec_helper'
+require 'puppet/data_binding'
+
+describe Puppet::DataBinding do
+ describe "when indirecting" do
+ it "should default to the 'none' data_binding terminus" do
+ Puppet::DataBinding.indirection.reset_terminus_class
+ Puppet::DataBinding.indirection.terminus_class.should == :none
+ end
+ end
+end
diff --git a/spec/unit/indirector/data_binding/hiera_spec.rb b/spec/unit/indirector/data_binding/hiera_spec.rb
new file mode 100644
index 0000000..98883bb
--- /dev/null
+++ b/spec/unit/indirector/data_binding/hiera_spec.rb
@@ -0,0 +1,21 @@
+require 'spec_helper'
+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
+
+ it "should be registered with the data_binding indirection" do
+ indirection = Puppet::Indirector::Indirection.instance(:data_binding)
+ Puppet::DataBinding::Hiera.indirection.should equal(indirection)
+ end
+
+ it "should have its name set to :hiera" do
+ Puppet::DataBinding::Hiera.name.should == :hiera
+ end
+end
diff --git a/spec/unit/indirector/data_binding/none_spec.rb b/spec/unit/indirector/data_binding/none_spec.rb
new file mode 100644
index 0000000..ebd7d4b
--- /dev/null
+++ b/spec/unit/indirector/data_binding/none_spec.rb
@@ -0,0 +1,28 @@
+require 'spec_helper'
+require 'puppet/indirector/data_binding/none'
+
+describe Puppet::DataBinding::None do
+ it "should be a subclass of the None terminus" do
+ Puppet::DataBinding::None.superclass.should equal(Puppet::Indirector::None)
+ end
+
+ it "should have documentation" do
+ Puppet::DataBinding::None.doc.should_not be_nil
+ end
+
+ it "should be registered with the data_binding indirection" do
+ indirection = Puppet::Indirector::Indirection.instance(:data_binding)
+ Puppet::DataBinding::None.indirection.should equal(indirection)
+ end
+
+ it "should have its name set to :none" do
+ Puppet::DataBinding::None.name.should == :none
+ end
+
+ describe "the behavior of the find method" do
+ it "should just return nil" do
+ data_binding = Puppet::DataBinding::None.new
+ data_binding.find('fake_request').should be_nil
+ end
+ end
+end
diff --git a/spec/unit/indirector/hiera_spec.rb b/spec/unit/indirector/hiera_spec.rb
new file mode 100644
index 0000000..7cb0b0d
--- /dev/null
+++ b/spec/unit/indirector/hiera_spec.rb
@@ -0,0 +1,48 @@
+require 'spec_helper'
+require 'puppet/indirector/hiera'
+
+describe Puppet::Indirector::Hiera do
+ before do
+ Puppet.settings[:hiera_config] = {}
+ Puppet::Indirector::Terminus.stubs(:register_terminus_class)
+ Puppet::Indirector::Indirection.stubs(:instance).returns(indirection)
+
+ module Testing; end
+ @hiera_class = class Testing::Hiera < Puppet::Indirector::Hiera
+ self
+ end
+ end
+
+ let(:model) { mock('model') }
+ let(:options) { {:host => 'foo' } }
+ let(:request) { stub('request', :key => "port", :options => options) }
+ let(:indirection) do
+ stub('indirection', :name => :none, :register_terminus_type => nil,
+ :model => model)
+ end
+
+ let(:facts) do
+ { 'fqdn' => 'agent.testing.com' }
+ end
+ let(:facter_obj) { stub(:values => facts) }
+
+ it "should not be the default data_binding terminus" do
+ Puppet.settings[:data_binding_terminus].should_not == 'hiera'
+ end
+
+ it "should raise an error if we don't have the hiera feature" do
+ Puppet.features.expects(:hiera?).returns(false)
+ lambda { @hiera_class.new }.should raise_error RuntimeError,
+ "Hiera terminus not supported without hiera gem"
+ end
+
+ describe "the behavior of the find method" do
+ it "should lookup the requested key in hiera", :if => Puppet.features.hiera? do
+ Hiera.any_instance.expects(:lookup).with("port", nil, facts, nil, nil).returns('3000')
+ Puppet::Node::Facts.indirection.expects(:find).with('foo').returns(facter_obj)
+
+ data_binder = @hiera_class.new
+ data_binder.find(request).should == '3000'
+ end
+ end
+end
diff --git a/spec/unit/indirector/none_spec.rb b/spec/unit/indirector/none_spec.rb
new file mode 100644
index 0000000..997e69e
--- /dev/null
+++ b/spec/unit/indirector/none_spec.rb
@@ -0,0 +1,33 @@
+require 'spec_helper'
+require 'puppet/indirector/none'
+
+describe Puppet::Indirector::None do
+ before do
+ Puppet::Indirector::Terminus.stubs(:register_terminus_class)
+ Puppet::Indirector::Indirection.stubs(:instance).returns(indirection)
+
+ module Testing; end
+ @none_class = class Testing::None < Puppet::Indirector::None
+ self
+ end
+
+ @data_binder = @none_class.new
+ end
+
+ let(:model) { mock('model') }
+ let(:request) { stub('request', :key => "port") }
+ let(:indirection) do
+ stub('indirection', :name => :none, :register_terminus_type => nil,
+ :model => model)
+ end
+
+ it "should be the default data_binding_terminus" do
+ Puppet.settings[:data_binding_terminus].should == 'none'
+ end
+
+ describe "the behavior of the find method" do
+ it "should just return nil" do
+ @data_binder.find(request).should be_nil
+ end
+ end
+end
diff --git a/spec/unit/resource_spec.rb b/spec/unit/resource_spec.rb
index 548322f..f67cced 100755
--- a/spec/unit/resource_spec.rb
+++ b/spec/unit/resource_spec.rb
@@ -303,6 +303,74 @@
resource = Puppet::Parser::Resource.new("default_param", "name", :scope => Puppet::Parser::Scope.new)
resource.set_default_parameters(@scope).should == [:a]
end
+
+ describe "when the resource type is :hostclass" do
+ before do
+ @scope.stubs(:host).returns('foo')
+ Puppet::Node::Environment.new.known_resource_types.add(apache)
+ end
+
+ let(:port) { Puppet::Parser::AST::String.new(:value => '80') }
+ let(:apache) do
+ Puppet::Resource::Type.new(:hostclass, 'apache', :arguments => { 'port' => port })
+ end
+
+ context "when no value is provided" do
+ let(:resource) do
+ Puppet::Parser::Resource.new("class", "apache", :scope => @scope)
+ end
+
+ it "should query the data_binding terminus using a namespaced key" do
+ Puppet::DataBinding.indirection.expects(:find).with(
+ 'apache::port', :host => 'foo')
+ resource.set_default_parameters(@scope)
+ end
+
+ it "should query the data_binding terminus using the host attribute from the scope" do
+ Puppet::DataBinding.indirection.expects(:find).with(
+ 'apache::port', :host => 'foo')
+ resource.set_default_parameters(@scope)
+ end
+
+ it "should use the value from the data_binding terminus" do
+ Puppet::DataBinding.indirection.expects(:find).with(
+ 'apache::port', :host => 'foo').returns('443')
+ resource.set_default_parameters(@scope).should == [:port]
+ resource[:port].should == '443'
+ end
+
+ it "should use the default value if the data_binding terminus returns nil" do
+ Puppet::DataBinding.indirection.expects(:find).with(
+ 'apache::port', :host => 'foo').returns(nil)
+ resource.set_default_parameters(@scope).should == [:port]
+ resource[:port].should == '80'
+ end
+ end
+
+ context "when a value is provided" do
+ let(:port_parameter) do
+ Puppet::Parser::Resource::Param.new(
+ { :name => 'port', :value => '8080' }
+ )
+ end
+
+ let(:resource) do
+ Puppet::Parser::Resource.new("class", "apache", :scope => @scope,
+ :parameters => [port_parameter])
+ end
+
+ it "should not query the data_binding terminus" do
+ Puppet::DataBinding.indirection.expects(:find).never
+ resource.set_default_parameters(@scope)
+ end
+
+ it "should use the value provided" do
+ Puppet::DataBinding.indirection.expects(:find).never
+ resource.set_default_parameters(@scope).should == []
+ resource[:port].should == '8080'
+ end
+ end
+ end
end
describe "when validating all required parameters are present" do
-- You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to [email protected].
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
