[Pkg-puppet-devel] [SCM] Puppet packaging for Debian branch, upstream, updated. 0.25.5-639-g8f94f35
test branch
puppet-dev at googlegroups.com
Wed Jul 14 10:31:55 UTC 2010
The following commit has been merged in the upstream branch:
commit 2fa0a489e26fc2512783c67b1b4579a03f8a20a6
Author: Luke Kanies <luke at reductivelabs.com>
Date: Fri Jan 22 00:48:37 2010 -0800
Adding parameter validation to Puppet::Resource
This will allow us to remove all of the parameter
validation from the other Resource classes.
This is possible because resource types defined
in the language are visible outside of the parser,
via the environment.
This will enable lots of code removal and simplication.
Signed-off-by: Luke Kanies <luke at reductivelabs.com>
diff --git a/lib/puppet/parser/resource.rb b/lib/puppet/parser/resource.rb
index 63d028c..428b9df 100644
--- a/lib/puppet/parser/resource.rb
+++ b/lib/puppet/parser/resource.rb
@@ -396,7 +396,7 @@ class Puppet::Parser::Resource
# Now make sure it's a valid argument to our class. These checks
# are organized in order of commonhood -- most types, it's a valid
# argument and paramcheck is enabled.
- if @ref.typeclass.validattr?(param)
+ if @ref.typeclass.valid_parameter?(param)
true
elsif %w{name title}.include?(param) # always allow these
true
diff --git a/lib/puppet/provider/ldap.rb b/lib/puppet/provider/ldap.rb
index be66838..38668e5 100644
--- a/lib/puppet/provider/ldap.rb
+++ b/lib/puppet/provider/ldap.rb
@@ -78,7 +78,7 @@ class Puppet::Provider::Ldap < Puppet::Provider
param, values = ary
# Skip any attributes we don't manage.
- next result unless self.class.resource_type.validattr?(param)
+ next result unless self.class.resource_type.valid_parameter?(param)
paramclass = self.class.resource_type.attrclass(param)
diff --git a/lib/puppet/provider/nameservice.rb b/lib/puppet/provider/nameservice.rb
index cc517ee..57441dd 100644
--- a/lib/puppet/provider/nameservice.rb
+++ b/lib/puppet/provider/nameservice.rb
@@ -44,7 +44,7 @@ class Puppet::Provider::NameService < Puppet::Provider
end
def options(name, hash)
- unless resource_type.validattr?(name)
+ unless resource_type.valid_parameter?(name)
raise Puppet::DevError, "%s is not a valid attribute for %s" %
[name, resource_type.name]
end
diff --git a/lib/puppet/resource.rb b/lib/puppet/resource.rb
index e475017..010cd95 100644
--- a/lib/puppet/resource.rb
+++ b/lib/puppet/resource.rb
@@ -1,16 +1,20 @@
require 'puppet'
require 'puppet/util/tagging'
-#require 'puppet/resource/reference'
require 'puppet/util/pson'
# The simplest resource class. Eventually it will function as the
# base class for all resource-like behaviour.
class Puppet::Resource
+ require 'puppet/resource/reference'
include Puppet::Util::Tagging
+
+ require 'puppet/resource/type_collection_helper'
+ include Puppet::Resource::TypeCollectionHelper
+
extend Puppet::Util::Pson
include Enumerable
- attr_accessor :file, :line, :catalog, :exported, :virtual
- attr_writer :type, :title
+ attr_accessor :file, :line, :catalog, :exported, :virtual, :namespace, :validate_parameters
+ attr_writer :type, :title, :environment
require 'puppet/indirector'
extend Puppet::Indirector
@@ -81,6 +85,7 @@ class Puppet::Resource
# Set a given parameter. Converts all passed names
# to lower-case symbols.
def []=(param, value)
+ validate_parameter(param) if validate_parameters
@parameters[parameter_name(param)] = value
end
@@ -117,7 +122,13 @@ class Puppet::Resource
# Create our resource.
def initialize(type, title, attributes = {})
+ # Doing this, instead of including it in the class,
+ # is the only way I could get the load order to work
+ # here.
+ extend Puppet::Node::Environment::Helper
+
@parameters = {}
+ @namespace = ""
(attributes[:parameters] || {}).each do |param, value|
self[param] = value
@@ -139,6 +150,15 @@ class Puppet::Resource
@reference.to_s
end
+ def resource_type
+ case type.to_s.downcase
+ when "class"; find_hostclass
+ when "node"; find_node
+ else
+ find_builtin_resource_type || find_defined_resource_type
+ end
+ end
+
# Get our title information from the reference, since it will canonize it for us.
def title
@reference.title
@@ -246,8 +266,33 @@ class Puppet::Resource
self
end
+ def valid_parameter?(name)
+ resource_type.valid_parameter?(name)
+ end
+
+ def validate_parameter(name)
+ raise ArgumentError, "Invalid parameter #{name}" unless valid_parameter?(name)
+ end
+
private
+ def find_node
+ known_resource_types.node(title)
+ end
+
+ def find_hostclass
+ name = title == :main ? "" : title
+ known_resource_types.find_hostclass(namespace, name)
+ end
+
+ def find_builtin_resource_type
+ Puppet::Type.type(type.to_s.downcase.to_sym)
+ end
+
+ def find_defined_resource_type
+ known_resource_types.find_definition(namespace, type.to_s.downcase)
+ end
+
# Produce a canonical method name.
def parameter_name(param)
param = param.to_s.downcase.to_sym
@@ -267,11 +312,6 @@ class Puppet::Resource
end
end
- # Retrieve the resource type.
- def resource_type
- Puppet::Type.type(type)
- end
-
# Create an old-style TransBucket instance, for non-builtin resource types.
def to_transbucket
bucket = Puppet::TransBucket.new([])
diff --git a/lib/puppet/resource/type.rb b/lib/puppet/resource/type.rb
index 9baf198..d476582 100644
--- a/lib/puppet/resource/type.rb
+++ b/lib/puppet/resource/type.rb
@@ -144,7 +144,7 @@ class Puppet::Resource::Type
set = {}
resource.to_hash.each do |param, value|
param = param.to_sym
- fail Puppet::ParseError, "#{resource.ref} does not accept attribute #{param}" unless validattr?(param)
+ fail Puppet::ParseError, "#{resource.ref} does not accept attribute #{param}" unless valid_parameter?(param)
exceptwrap { scope.setvar(param.to_s, value) }
@@ -174,7 +174,7 @@ class Puppet::Resource::Type
end
# Check whether a given argument is valid.
- def validattr?(param)
+ def valid_parameter?(param)
param = param.to_s
return true if param == "name"
diff --git a/lib/puppet/transportable.rb b/lib/puppet/transportable.rb
index 68977dc..1970d9f 100644
--- a/lib/puppet/transportable.rb
+++ b/lib/puppet/transportable.rb
@@ -49,7 +49,7 @@ module Puppet
def to_component
trans = TransObject.new(ref, :component)
@params.each { |param,value|
- next unless Puppet::Type::Component.validattr?(param)
+ next unless Puppet::Type::Component.valid_parameter?(param)
Puppet.debug "Defining %s on %s" % [param, ref]
trans[param] = value
}
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index 2fb4abc..31728c3 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -388,6 +388,11 @@ class Type
end
end
+ # This is a forward-compatibility method - it's the validity interface we'll use in Puppet::Resource.
+ def self.valid_parameter?(name)
+ validattr?(name)
+ end
+
# Return either the attribute alias or the attribute.
def attr_alias(name)
name = symbolize(name)
diff --git a/lib/puppet/type/component.rb b/lib/puppet/type/component.rb
index 5fed176..bf9007a 100644
--- a/lib/puppet/type/component.rb
+++ b/lib/puppet/type/component.rb
@@ -14,14 +14,14 @@ Puppet::Type.newtype(:component) do
# Override how parameters are handled so that we support the extra
# parameters that are used with defined resource types.
def [](param)
- return super if self.class.validattr?(param)
+ return super if self.class.valid_parameter?(param)
@extra_parameters[param.to_sym]
end
# Override how parameters are handled so that we support the extra
# parameters that are used with defined resource types.
def []=(param, value)
- return super if self.class.validattr?(param)
+ return super if self.class.valid_parameter?(param)
@extra_parameters[param.to_sym] = value
end
diff --git a/spec/unit/provider/ldap.rb b/spec/unit/provider/ldap.rb
index fd5d1bd..6c58208 100755
--- a/spec/unit/provider/ldap.rb
+++ b/spec/unit/provider/ldap.rb
@@ -131,7 +131,7 @@ describe Puppet::Provider::Ldap do
@property_class = stub 'property_class', :array_matching => :all, :superclass => Puppet::Property
@resource_class.stubs(:attrclass).with(:one).returns(@property_class)
- @resource_class.stubs(:validattr?).returns true
+ @resource_class.stubs(:valid_parameter?).returns true
end
it "should store a copy of the hash as its ldap_properties" do
@@ -161,7 +161,7 @@ describe Puppet::Provider::Ldap do
end
it "should discard any properties not valid in the resource class" do
- @resource_class.expects(:validattr?).with(:a).returns false
+ @resource_class.expects(:valid_parameter?).with(:a).returns false
@property_class.stubs(:array_matching).returns :all
instance = @class.new(:one => %w{two three}, :a => %w{b})
@@ -177,7 +177,7 @@ describe Puppet::Provider::Ldap do
@instance = @class.new
@property_class = stub 'property_class', :array_matching => :all, :superclass => Puppet::Property
- @resource_class = stub 'resource_class', :attrclass => @property_class, :validattr? => true, :validproperties => [:one, :two]
+ @resource_class = stub 'resource_class', :attrclass => @property_class, :valid_parameter? => true, :validproperties => [:one, :two]
@class.stubs(:resource_type).returns @resource_class
end
diff --git a/spec/unit/resource.rb b/spec/unit/resource.rb
index bc47fa7..73bbfd8 100755
--- a/spec/unit/resource.rb
+++ b/spec/unit/resource.rb
@@ -90,20 +90,111 @@ describe Puppet::Resource do
resource.should be_exported
end
- it "should support an environment attribute"
+ it "should support an environment attribute" do
+ Puppet::Resource.new("file", "/my/file", :environment => :foo).environment.name.should == :foo
+ end
+
+ it "should support a namespace attribute" do
+ Puppet::Resource.new("file", "/my/file", :namespace => :foo).namespace.should == :foo
+ end
+
+ it "should default to a namespace of an empty string" do
+ Puppet::Resource.new("file", "/my/file").namespace.should == ""
+ end
+
+ it "should be able to look up its resource type when the type is a builtin resource" do
+ Puppet::Resource.new("file", "/my/file").resource_type.should equal(Puppet::Type.type(:file))
+ end
+
+ it "should be able to look up its resource type via its environment when the type is a defined resource type" do
+ resource = Puppet::Resource.new("foobar", "/my/file")
+ type = Puppet::Resource::Type.new(:definition, "foobar")
+ resource.environment.known_resource_types.add type
+
+ resource.resource_type.should equal(type)
+ end
+
+ it "should be able to look up its resource type via its environment when the type is a node" do
+ resource = Puppet::Resource.new("node", "foobar")
+ node = Puppet::Resource::Type.new(:node, "foobar")
+ resource.environment.known_resource_types.add node
+
+ resource.resource_type.should equal(node)
+ end
+
+ it "should be able to look up its resource type via its environment when the type is a class" do
+ resource = Puppet::Resource.new("class", "foobar")
+ klass = Puppet::Resource::Type.new(:hostclass, "foobar")
+ resource.environment.known_resource_types.add klass
- it "should convert its environment into an environment instance if one is provided"
+ resource.resource_type.should equal(klass)
+ end
+
+ it "should use its namespace when looking up defined resource types" do
+ resource = Puppet::Resource.new("bar", "/my/file", :namespace => "foo")
+ type = Puppet::Resource::Type.new(:definition, "foo::bar")
+ resource.environment.known_resource_types.add type
+
+ resource.resource_type.should equal(type)
+ end
- it "should support a namespace attribute"
+ it "should use its namespace when looking up host classes" do
+ resource = Puppet::Resource.new("class", "bar", :namespace => "foo")
+ type = Puppet::Resource::Type.new(:hostclass, "foo::bar")
+ resource.environment.known_resource_types.add type
+
+ resource.resource_type.should equal(type)
+ end
+
+ it "should return nil when looking up resource types that don't exist" do
+ Puppet::Resource.new("foobar", "bar").resource_type.should be_nil
+ end
+
+ it "should fail when an invalid parameter is used and parameter validation is enabled" do
+ type = Puppet::Resource::Type.new(:definition, "foobar")
+ Puppet::Node::Environment.new.known_resource_types.add type
+ resource = Puppet::Resource.new("foobar", "/my/file", :validate_parameters => true)
+ lambda { resource[:yay] = true }.should raise_error(ArgumentError)
+ end
+
+ it "should not fail when an invalid parameter is used and parameter validation is disabled" do
+ type = Puppet::Resource::Type.new(:definition, "foobar")
+ Puppet::Node::Environment.new.known_resource_types.add type
+ resource = Puppet::Resource.new("foobar", "/my/file")
+ resource[:yay] = true
+ end
+
+ it "should not fail when a valid parameter is used and parameter validation is enabled" do
+ type = Puppet::Resource::Type.new(:definition, "foobar", :arguments => {"yay" => nil})
+ Puppet::Node::Environment.new.known_resource_types.add type
+ resource = Puppet::Resource.new("foobar", "/my/file", :validate_parameters => true)
+ resource[:yay] = true
+ end
describe "when managing parameters" do
before do
@resource = Puppet::Resource.new("file", "/my/file")
end
- it "should be able to check whether parameters are valid when the resource models builtin resources"
+ it "should correctly detect when provided parameters are not valid for builtin types" do
+ Puppet::Resource.new("file", "/my/file").should_not be_valid_parameter("foobar")
+ end
+
+ it "should correctly detect when provided parameters are valid for builtin types" do
+ Puppet::Resource.new("file", "/my/file").should be_valid_parameter("mode")
+ end
+
+ it "should correctly detect when provided parameters are not valid for defined resource types" do
+ type = Puppet::Resource::Type.new(:definition, "foobar")
+ Puppet::Node::Environment.new.known_resource_types.add type
+ Puppet::Resource.new("foobar", "/my/file").should_not be_valid_parameter("myparam")
+ end
- it "should be able to check whether parameters are valid when the resource models defined resources"
+ it "should correctly detect when provided parameters are valid for defined resource types" do
+ type = Puppet::Resource::Type.new(:definition, "foobar", :arguments => {"myparam" => nil})
+ Puppet::Node::Environment.new.known_resource_types.add type
+ Puppet::Resource.new("foobar", "/my/file").should be_valid_parameter("myparam")
+ end
it "should allow setting and retrieving of parameters" do
@resource[:foo] = "bar"
@@ -138,6 +229,7 @@ describe Puppet::Resource do
it "should be able to set the name for non-builtin types" do
resource = Puppet::Resource.new(:foo, "bar")
+ resource[:name] = "eh"
lambda { resource[:name] = "eh" }.should_not raise_error
end
diff --git a/spec/unit/resource/type.rb b/spec/unit/resource/type.rb
index 98b38c9..0a2f447 100755
--- a/spec/unit/resource/type.rb
+++ b/spec/unit/resource/type.rb
@@ -138,37 +138,37 @@ describe Puppet::Resource::Type do
it "should set any provided arguments with the keys as symbols" do
type = Puppet::Resource::Type.new(:hostclass, "foo", :arguments => {:foo => "bar", :baz => "biz"})
- type.should be_validattr("foo")
- type.should be_validattr("baz")
+ type.should be_valid_parameter("foo")
+ type.should be_valid_parameter("baz")
end
it "should set any provided arguments with they keys as strings" do
type = Puppet::Resource::Type.new(:hostclass, "foo", :arguments => {"foo" => "bar", "baz" => "biz"})
- type.should be_validattr(:foo)
- type.should be_validattr(:baz)
+ type.should be_valid_parameter(:foo)
+ type.should be_valid_parameter(:baz)
end
it "should function if provided no arguments" do
type = Puppet::Resource::Type.new(:hostclass, "foo")
- type.should_not be_validattr(:foo)
+ type.should_not be_valid_parameter(:foo)
end
end
describe "when testing the validity of an attribute" do
it "should return true if the parameter was typed at initialization" do
- Puppet::Resource::Type.new(:hostclass, "foo", :arguments => {"foo" => "bar"}).should be_validattr("foo")
+ Puppet::Resource::Type.new(:hostclass, "foo", :arguments => {"foo" => "bar"}).should be_valid_parameter("foo")
end
it "should return true if it is a metaparam" do
- Puppet::Resource::Type.new(:hostclass, "foo").should be_validattr("require")
+ Puppet::Resource::Type.new(:hostclass, "foo").should be_valid_parameter("require")
end
it "should return true if the parameter is named 'name'" do
- Puppet::Resource::Type.new(:hostclass, "foo").should be_validattr("name")
+ Puppet::Resource::Type.new(:hostclass, "foo").should be_valid_parameter("name")
end
it "should return false if it is not a metaparam and was not provided at initialization" do
- Puppet::Resource::Type.new(:hostclass, "foo").should_not be_validattr("yayness")
+ Puppet::Resource::Type.new(:hostclass, "foo").should_not be_valid_parameter("yayness")
end
end
diff --git a/spec/unit/type.rb b/spec/unit/type.rb
index 97dc113..73f249f 100755
--- a/spec/unit/type.rb
+++ b/spec/unit/type.rb
@@ -7,6 +7,18 @@ describe Puppet::Type do
Puppet::Type.ancestors.should be_include(Puppet::Util::Cacher)
end
+ it "should consider a parameter to be valid if it is a valid parameter" do
+ Puppet::Type.type(:mount).should be_valid_parameter(:path)
+ end
+
+ it "should consider a parameter to be valid if it is a valid property" do
+ Puppet::Type.type(:mount).should be_valid_parameter(:fstype)
+ end
+
+ it "should consider a parameter to be valid if it is a valid metaparam" do
+ Puppet::Type.type(:mount).should be_valid_parameter(:noop)
+ end
+
it "should use its catalog as its expirer" do
catalog = Puppet::Resource::Catalog.new
resource = Puppet::Type.type(:mount).new(:name => "foo", :fstype => "bar", :pass => 1, :ensure => :present)
diff --git a/test/language/resource.rb b/test/language/resource.rb
index 69e82fd..c1d116f 100755
--- a/test/language/resource.rb
+++ b/test/language/resource.rb
@@ -67,17 +67,17 @@ class TestResource < PuppetTest::TestCase
res.instance_variable_set("@ref", ref)
klass = mock("class")
ref.expects(:typeclass).returns(klass).times(4)
- klass.expects(:validattr?).with("good").returns(true)
+ klass.expects(:valid_parameter?).with("good").returns(true)
assert(res.send(:paramcheck, :good), "Did not allow valid param")
# It's name or title
- klass.expects(:validattr?).with("name").returns(false)
+ klass.expects(:valid_parameter?).with("name").returns(false)
assert(res.send(:paramcheck, :name), "Did not allow name")
- klass.expects(:validattr?).with("title").returns(false)
+ klass.expects(:valid_parameter?).with("title").returns(false)
assert(res.send(:paramcheck, :title), "Did not allow title")
# It's not actually allowed
- klass.expects(:validattr?).with("other").returns(false)
+ klass.expects(:valid_parameter?).with("other").returns(false)
res.expects(:fail)
ref.expects(:type)
res.send(:paramcheck, :other)
diff --git a/test/lib/puppettest/fakes.rb b/test/lib/puppettest/fakes.rb
index db6ca4d..0aedb59 100644
--- a/test/lib/puppettest/fakes.rb
+++ b/test/lib/puppettest/fakes.rb
@@ -35,7 +35,7 @@ module PuppetTest
def []=(param, value)
param = symbolize(param)
- unless @realresource.validattr?(param)
+ unless @realresource.valid_parameter?(param)
raise Puppet::DevError, "Invalid attribute %s for %s" %
[param, @realresource.name]
end
--
Puppet packaging for Debian
More information about the Pkg-puppet-devel
mailing list