diff options
| -rw-r--r-- | lib/puppet/parser/ast.rb | 60 | ||||
| -rw-r--r-- | lib/puppet/parser/scope.rb | 124 | ||||
| -rw-r--r-- | lib/puppet/transportable.rb | 11 | ||||
| -rwxr-xr-x | test/language/ast.rb | 134 | ||||
| -rwxr-xr-x | test/language/scope.rb | 113 | ||||
| -rw-r--r-- | test/puppettest.rb | 106 |
6 files changed, 386 insertions, 162 deletions
diff --git a/lib/puppet/parser/ast.rb b/lib/puppet/parser/ast.rb index fa21f8a9e..dd56c109a 100644 --- a/lib/puppet/parser/ast.rb +++ b/lib/puppet/parser/ast.rb @@ -1228,7 +1228,8 @@ module Puppet end names.each { |name| - Puppet.debug("defining host '%s'" % name) + Puppet.debug("defining host '%s' in scope %s" % + [name, scope.object_id]) arghash = { :name => name, :code => @code @@ -1285,9 +1286,10 @@ module Puppet # The class name @name = :component - attr_accessor :name, :args, :code + attr_accessor :name, :args, :code, :scope def evaluate(scope,hash,objtype,objname) + scope = scope.newscope # The type is the component or class name @@ -1297,9 +1299,18 @@ module Puppet # been dynamically generated. This is almost never used scope.name = objname + #if self.is_a?(Node) + # scope.isnodescope + #end + # Additionally, add a tag for whatever kind of class # we are - scope.base = self.class.name + scope.tag(objtype) + + unless objname =~ /-\d+/ # it was generated + scope.tag(objname) + end + #scope.base = self.class.name # define all of the arguments in our local scope @@ -1354,6 +1365,10 @@ module Puppet # Now just evaluate the code with our new bindings. self.code.safeevaluate(scope) + + # We return the scope, so that our children can make their scopes + # under ours. This allows them to find our definitions. + return scope end end @@ -1370,7 +1385,13 @@ module Puppet return nil end - self.evalparent(scope, hash, objname) + if tmp = self.evalparent(scope, hash, objname) + # Override our scope binding with the parent scope + # binding. This is quite hackish, but I can't think + # of another way to make sure our scopes end up under + # our parent scopes. + scope = tmp + end # just use the Component evaluate method, but change the type # to our own type @@ -1382,7 +1403,9 @@ module Puppet return retval end - # Evaluate our parent class. + # Evaluate our parent class. Parent classes are evaluated in the + # exact same scope as the children. This is maybe not a good idea + # but, eh. def evalparent(scope, args, name) if @parentclass parentobj = nil @@ -1411,14 +1434,14 @@ module Puppet # Verify that the parent and child are of the same type unless parentobj.class == self.class error = Puppet::ParseError.new( - "Class %s has incompatible parent type" % - [@name] + "Class %s has incompatible parent type, %s vs %s" % + [@name, parentobj.class, self.class] ) error.file = self.file error.line = self.line raise error end - parentobj.safeevaluate(scope,args,@parentclass,name) + return parentobj.safeevaluate(scope,args,@parentclass,name) end end @@ -1429,8 +1452,9 @@ module Puppet end - # The specific code associated with a host. - class Node < AST::Component + # The specific code associated with a host. Nodes are annoyingly unlike + # other objects. That's just the way it is, at least for now. + class Node < AST::HostClass @name = :node attr_accessor :name, :args, :code, :parentclass @@ -1448,14 +1472,24 @@ module Puppet # Mark this scope as a nodescope, so that classes will be # singletons within it - scope.nodescope = true + scope.isnodescope # Now set all of the facts inside this scope facts.each { |var, value| scope.setvar(var, value) } - self.evalparent(scope) + if tmp = self.evalparent(scope) + # Again, override our scope with the parent scope, if + # there is one. + scope = tmp + end + + #scope.tag(@name) + + # We never pass the facts to the parent class, because they've + # already been defined at this top-level scope. + #super(scope, facts, @name, @name) # And then evaluate our code. @code.safeevaluate(scope) @@ -1489,7 +1523,7 @@ module Puppet node = hash[:node] # Tag the scope with the parent's name/type. name = node.name - Puppet.info "Tagging with parent node %s" % name + #Puppet.info "Tagging with parent node %s" % name scope.tag(name) begin diff --git a/lib/puppet/parser/scope.rb b/lib/puppet/parser/scope.rb index 89e6ab75d..6e0854206 100644 --- a/lib/puppet/parser/scope.rb +++ b/lib/puppet/parser/scope.rb @@ -11,8 +11,8 @@ module Puppet attr_accessor :name, :type, :topscope, :base # This is probably not all that good of an idea, but... - # This way a parent can share its node table with all of its children. - attr_writer :nodetable + # This way a parent can share its tables with all of its children. + attr_writer :nodetable, :classtable, :definedtable # Whether we behave declaratively. Note that it's a class variable, # so all scopes behave the same. @@ -70,6 +70,18 @@ module Puppet else raise Puppet::DevError, "No nodetable has been defined" end + + if defined? @classtable + scope.classtable = @classtable + else + raise Puppet::DevError, "No classtable has been defined" + end + + if defined? @definedtable + scope.definedtable = @definedtable + else + raise Puppet::DevError, "No definedtable has been defined" + end end # Test whether a given scope is declarative. Even though it's @@ -108,8 +120,28 @@ module Puppet @nodescope end - def nodescope=(bool) - @nodescope = bool + # Mark that we are a nodescope. + def isnodescope + @nodescope = true + + # Also, create the extra tables associated with being a node + # scope. + # The table for storing class singletons. + @classtable = Hash.new(nil) + + # Also, create the object checking map + @definedtable = Hash.new { |types, type| + types[type] = {} + } + end + + # Has a given object been defined anywhere in the scope tree? + def objectdefined?(name, type) + unless defined? @definedtable + raise Puppet::DevError, "Scope did not receive definedtable" + end + + return @definedtable[type][name] end # Are we the top scope? @@ -197,7 +229,10 @@ module Puppet end end - # Evaluate normally, with no node definitions + # Evaluate normally, with no node definitions. This is a bit of a + # silly method, in that it just calls evaluate on the passed-in + # objects, and then calls to_trans on itself. It just conceals + # a paltry amount of info from whomever's using the scope object. def evaluate(objects, facts = {}) facts.each { |var, value| self.setvar(var, value) @@ -222,6 +257,16 @@ module Puppet @@declarative = declarative + # The table for storing class singletons. This will only actually + # be used by top scopes and node scopes. + @classtable = Hash.new(nil) + + # The table for all defined objects. This will only be + # used in the top scope if we don't have any nodescopes. + @definedtable = Hash.new { |types, type| + types[type] = {} + } + # A table for storing nodes. @nodetable = Hash.new(nil) @@ -256,10 +301,6 @@ module Puppet # The type table for this scope @typetable = Hash.new(nil) - # The table for storing class singletons. This will only actually - # be used by top scopes and node scopes. - @classtable = Hash.new(nil) - # All of the defaults set for types. It's a hash of hashes, # with the first key being the type, then the second key being # the parameter. @@ -321,14 +362,10 @@ module Puppet # Look up a given class. This enables us to make sure classes are # singletons def lookupclass(klass) - if self.nodescope? or self.topscope? - return @classtable[klass] - else - unless @parent - raise Puppet::DevError, "Not top scope but not parent defined" - end - return @parent.lookupclass(klass) + unless defined? @classtable + raise Puppet::DevError, "Scope did not receive class table" end + return @classtable[klass] end # Collect all of the defaults set at any higher scopes. @@ -492,14 +529,34 @@ module Puppet return newstring end - # This is kind of quirky, because it doesn't differentiate between - # creating a new object and adding params to an existing object. - # It doesn't solve the real problem, though: cases like file recursion, - # where one statement explicitly modifies an object, and another - # statement modifies it because of recursion. + # This method will fail if the named object is already defined anywhere + # in the scope tree, which is what provides some minimal closure-like + # behaviour. def setobject(type, name, params, file, line) + # First see if we can look the object up using normal scope + # rules, i.e., one of our parent classes has defined the + # object or something obj = self.lookupobject(name,type) + + # If we can't find it... if obj == :undefined or obj.nil? + # Make sure it's not defined elsewhere in the configuration + if tmp = self.objectdefined?(name, type) + msg = "Duplicate definition: %s[%s] is already defined" % + [type, name] + if tmp.line + msg += " at line %s" % tmp.line + end + if tmp.file + msg += " in file %s" % tmp.file + end + error = Puppet::ParseError.new(msg) + error.file = file + error.line = line + raise error + end + + # And if it's not, then create it anew obj = @objectable[type][name] # only set these if we've created the object, which is the @@ -508,13 +565,16 @@ module Puppet obj.line = line end - # now add the params to whatever object we've found, whether - # it was in a higher scope or we just created it - # it will not be obvious where these parameters are from, that is, - # which file they're in or whatever + # Now add our parameters. This has the function of overriding + # existing values, which might have been defined in a higher + # scope. params.each { |var,value| obj[var] = value } + + # And finally store the fact that we've defined this object. + @definedtable[type][name] = obj + return obj end @@ -538,14 +598,15 @@ module Puppet end end - # Add a tag to our current list. This is only currently used - # when nodes evaluate their parents. + # Add a tag to our current list. These tags will be added to all + # of the objects contained in this scope. def tag(*ary) ary.each { |tag| if tag.nil? or tag == "" Puppet.warning "got told to tag with %s" % tag.inspect end unless @tags.include?(tag) + #Puppet.info "Tagging scope %s with %s" % [self.object_id, tag] @tags << tag.to_s end } @@ -593,8 +654,10 @@ module Puppet results.push(result) } else - # Otherwise, just add it to our list of results. - results.push(cresult) + unless cresult.empty? + # Otherwise, just add it to our list of results. + results.push(cresult) + end end # Nodescopes are one-time; once they've been evaluated @@ -604,6 +667,9 @@ module Puppet @children.delete(child) end elsif child.is_a?(TransObject) + if child.empty? + next + end # Wait until the last minute to set tags, although this # probably should not matter child.tags = self.tags diff --git a/lib/puppet/transportable.rb b/lib/puppet/transportable.rb index a9dea65ec..5ea00713e 100644 --- a/lib/puppet/transportable.rb +++ b/lib/puppet/transportable.rb @@ -1,11 +1,8 @@ -#!/usr/local/bin/ruby -w - -# $Id$ - require 'puppet' module Puppet - #------------------------------------------------------------ + # The transportable objects themselves. Basically just a hash with some + # metadata and a few extra methods. class TransObject < Hash attr_accessor :type, :name, :file, :line @@ -23,7 +20,7 @@ module Puppet end def tags - return @tags + [@type, @name] + return @tags end def to_s @@ -167,3 +164,5 @@ module Puppet end #------------------------------------------------------------ end + +# $Id$ diff --git a/test/language/ast.rb b/test/language/ast.rb index d78ce382c..6aa89c225 100755 --- a/test/language/ast.rb +++ b/test/language/ast.rb @@ -14,109 +14,7 @@ require 'test/unit' require 'puppettest' class TestAST < Test::Unit::TestCase - include TestPuppet - AST = Puppet::Parser::AST - - def astarray - AST::ASTArray.new( - :children => [] - ) - end - - def classobj(name, args = {}) - unless args.include?(:name) - args[:name] = nameobj(name) - end - unless args.include?(:code) - args[:code] = AST::ASTArray.new( - :children => [ - varobj("%svar" % name, "%svalue" % name), - fileobj("/%s" % name) - ] - ) - end - assert_nothing_raised("Could not create class %s" % name) { - return AST::ClassDef.new(args) - } - end - - def compobj(name, args = {}) - args[:name] = nameobj(name) - args[:code] = AST::ASTArray.new( - :children => [ - varobj("%svar" % name, "%svalue" % name), - fileobj("/%s" % name) - ] - ) - assert_nothing_raised("Could not create compdef %s" % name) { - return AST::CompDef.new(args) - } - end - - def fileobj(path, hash = {"owner" => "root"}) - assert_nothing_raised("Could not create file %s" % path) { - return AST::ObjectDef.new( - :name => stringobj(path), - :type => nameobj("file"), - :params => objectinst(hash) - ) - } - end - - def nameobj(name) - assert_nothing_raised("Could not create name %s" % name) { - return AST::Name.new( - :value => name - ) - } - end - - def nodeobj(name) - assert_nothing_raised("Could not create node %s" % name) { - return AST::NodeDef.new( - :names => nameobj(name), - :code => AST::ASTArray.new( - :children => [ - varobj("%svar" % name, "%svalue" % name), - fileobj("/%s" % name) - ] - ) - ) - } - end - - def objectinst(hash) - assert_nothing_raised("Could not create object instance") { - params = hash.collect { |param, value| - objectparam(param, value) - } - return AST::ObjectInst.new( - :children => params - ) - } - end - - def objectparam(param, value) - assert_nothing_raised("Could not create param %s" % param) { - return AST::ObjectParam.new( - :param => nameobj(param), - :value => stringobj(value) - ) - } - end - - def stringobj(value) - AST::String.new(:value => value) - end - - def varobj(name, value) - assert_nothing_raised("Could not create %s code" % name) { - return AST::VarDef.new( - :name => nameobj(name), - :value => stringobj(value) - ) - } - end + include ParserTesting # Test that classes behave like singletons def test_classsingleton @@ -441,7 +339,7 @@ class TestAST < Test::Unit::TestCase end # Test that node inheritance works correctly - def test_nodeinheritance + def test_znodeinheritance children = [] # create the base node @@ -487,14 +385,28 @@ class TestAST < Test::Unit::TestCase } assert(objects, "Could not retrieve node definition") - # And now verify that we got the subnode file - assert_nothing_raised("Could not find basenode file") { - assert_equal("/basenode", objects[0][0][:name]) - } + assert_nothing_raised { + inner = objects[0] + + # And now verify that we got the subnode file + assert_nothing_raised("Could not find basenode file") { + base = inner[0] + assert_equal("/basenode", base[:name]) + } - # and the parent node file - assert_nothing_raised("Could not find subnode file") { - assert_equal("/subnode", objects[0][1][:name]) + # and the parent node file + assert_nothing_raised("Could not find subnode file") { + sub = inner[1] + assert_equal("/subnode", sub[:name]) + } + + inner.each { |obj| + %w{basenode subnode}.each { |tag| + assert(obj.tags.include?(tag), + "%s did not include %s tag" % [obj[:name], tag] + ) + } + } } end end diff --git a/test/language/scope.rb b/test/language/scope.rb index 38bcdce5a..3937ad361 100755 --- a/test/language/scope.rb +++ b/test/language/scope.rb @@ -23,8 +23,8 @@ require 'puppettest' # and test whether we've got things in the right scopes class TestScope < Test::Unit::TestCase - include TestPuppet - AST = Puppet::Parser::AST + include ParserTesting + def to_ary(hash) hash.collect { |key,value| [key,value] @@ -260,7 +260,7 @@ class TestScope < Test::Unit::TestCase end # Test some of the host manipulations - def test_zhostlookup + def test_hostlookup top = Puppet::Parser::Scope.new(nil) # Create a deep scope tree, so that we know we're doing a deeply recursive @@ -302,4 +302,111 @@ class TestScope < Test::Unit::TestCase assert(host, "Could not find host") assert(host.code == :notused, "Host is not what we stored") end + + # Verify that two statements about a file within the same scope tree + # will cause a conflict. + def test_noconflicts + filename = tempfile() + children = [] + + # create the parent class + children << classobj("one", :code => AST::ASTArray.new( + :children => [ + fileobj(filename, "owner" => "root") + ] + )) + + # now create a child class with differ values + children << classobj("two", + :code => AST::ASTArray.new( + :children => [ + fileobj(filename, "owner" => "bin") + ] + )) + + # Now call the child class + assert_nothing_raised("Could not add AST nodes for calling") { + children << AST::ObjectDef.new( + :type => nameobj("two"), + :name => nameobj("yayness"), + :params => astarray() + ) << AST::ObjectDef.new( + :type => nameobj("one"), + :name => nameobj("yayness"), + :params => astarray() + ) + } + + top = nil + assert_nothing_raised("Could not create top object") { + top = AST::ASTArray.new( + :children => children + ) + } + + objects = nil + scope = nil + + # Here's where we should encounter the failure. It should find that + # it has already created an object with that name, and this should result + # in some pukey-pukeyness. + assert_raise(Puppet::ParseError) { + scope = Puppet::Parser::Scope.new() + objects = scope.evaluate(top) + } + end + + # Verify that we override statements that we find within our scope + def test_zsuboverrides + filename = tempfile() + children = [] + + # create the parent class + children << classobj("parent", :code => AST::ASTArray.new( + :children => [ + fileobj(filename, "owner" => "root") + ] + )) + + # now create a child class with differ values + children << classobj("child", :parentclass => nameobj("parent"), + :code => AST::ASTArray.new( + :children => [ + fileobj(filename, "owner" => "bin") + ] + )) + + # Now call the child class + assert_nothing_raised("Could not add AST nodes for calling") { + children << AST::ObjectDef.new( + :type => nameobj("child"), + :name => nameobj("yayness"), + :params => astarray() + ) + } + + top = nil + assert_nothing_raised("Could not create top object") { + top = AST::ASTArray.new( + :children => children + ) + } + + objects = nil + scope = nil + assert_nothing_raised("Could not evaluate") { + scope = Puppet::Parser::Scope.new() + objects = scope.evaluate(top) + } + + assert_equal(1, objects.length, "Returned too many objects: %s" % + objects.inspect) + assert_equal(1, objects[0].length, "Returned too many objects: %s" % + objects[0].inspect) + assert_nothing_raised { + file = objects[0][0] + + assert_equal("bin", file["owner"], "Value did not override correctly") + } + end end diff --git a/test/puppettest.rb b/test/puppettest.rb index 6f6b961f8..ccaa027cd 100644 --- a/test/puppettest.rb +++ b/test/puppettest.rb @@ -561,6 +561,112 @@ module FileTesting end end +module ParserTesting + include TestPuppet + AST = Puppet::Parser::AST + + def astarray + AST::ASTArray.new( + :children => [] + ) + end + + def classobj(name, args = {}) + unless args.include?(:name) + args[:name] = nameobj(name) + end + unless args.include?(:code) + args[:code] = AST::ASTArray.new( + :children => [ + varobj("%svar" % name, "%svalue" % name), + fileobj("/%s" % name) + ] + ) + end + assert_nothing_raised("Could not create class %s" % name) { + return AST::ClassDef.new(args) + } + end + + def compobj(name, args = {}) + args[:name] = nameobj(name) + args[:code] = AST::ASTArray.new( + :children => [ + varobj("%svar" % name, "%svalue" % name), + fileobj("/%s" % name) + ] + ) + assert_nothing_raised("Could not create compdef %s" % name) { + return AST::CompDef.new(args) + } + end + + def fileobj(path, hash = {"owner" => "root"}) + assert_nothing_raised("Could not create file %s" % path) { + return AST::ObjectDef.new( + :name => stringobj(path), + :type => nameobj("file"), + :params => objectinst(hash) + ) + } + end + + def nameobj(name) + assert_nothing_raised("Could not create name %s" % name) { + return AST::Name.new( + :value => name + ) + } + end + + def nodeobj(name) + assert_nothing_raised("Could not create node %s" % name) { + return AST::NodeDef.new( + :names => nameobj(name), + :code => AST::ASTArray.new( + :children => [ + varobj("%svar" % name, "%svalue" % name), + fileobj("/%s" % name) + ] + ) + ) + } + end + + def objectinst(hash) + assert_nothing_raised("Could not create object instance") { + params = hash.collect { |param, value| + objectparam(param, value) + } + return AST::ObjectInst.new( + :children => params + ) + } + end + + def objectparam(param, value) + assert_nothing_raised("Could not create param %s" % param) { + return AST::ObjectParam.new( + :param => nameobj(param), + :value => stringobj(value) + ) + } + end + + def stringobj(value) + AST::String.new(:value => value) + end + + def varobj(name, value) + assert_nothing_raised("Could not create %s code" % name) { + return AST::VarDef.new( + :name => nameobj(name), + :value => stringobj(value) + ) + } + end +end + class PuppetTestSuite attr_accessor :subdir |