[Pkg-puppet-devel] [SCM] Puppet packaging for Debian branch, experimental, updated. debian/2.6.8-1-844-g7ec39d5

Paul Berry paul at puppetlabs.com
Tue May 10 07:59:01 UTC 2011


The following commit has been merged in the experimental branch:
commit 6b278503021c4404904f56ced6995d0fbfa5b8fe
Author: Paul Berry <paul at puppetlabs.com>
Date:   Thu Sep 2 18:10:37 2010 -0700

    [#4657] Customer-supplied .rb files are not compatible with multiple environments or staleness check
    
    Changed the resource type API to create AST objects rather than
    directly instantiating resource types.  This allows the same code
    paths to be used to handle the results of parsing both .pp and .rb
    files.  This makes .rb files work properly in multiple environments,
    because the types are now instantiated by code that is aware of which
    environment the compilation is happening in.  It also reduces the risk
    of future changes breaking .rb file support.
    
    Also, switched to using "instance_eval" rather than "require" to
    evaluate the contents of the .rb file.  This ensures that if the file
    has to be recompiled (because it became stale), it will actually get
    re-evaluated.  As a side benefit, ResourceTypeAPI is now a class
    rather than a mixin to Object, so its methods do not pollute the
    global namespace.
    
    To reduce the risk of customers coming to rely on implementation
    details of the resource type API, changed its methods to return nil,
    and removed methods from it that were misleadingly labeled as
    "private".

diff --git a/lib/puppet/dsl.rb b/lib/puppet/dsl.rb
index abdb78f..97a3104 100644
--- a/lib/puppet/dsl.rb
+++ b/lib/puppet/dsl.rb
@@ -5,7 +5,3 @@ end
 
 require 'puppet/dsl/resource_type_api'
 require 'puppet/dsl/resource_api'
-
-class Object
-  include Puppet::DSL::ResourceTypeAPI
-end
diff --git a/lib/puppet/dsl/resource_type_api.rb b/lib/puppet/dsl/resource_type_api.rb
index 487aab9..8810d53 100644
--- a/lib/puppet/dsl/resource_type_api.rb
+++ b/lib/puppet/dsl/resource_type_api.rb
@@ -1,34 +1,15 @@
 require 'puppet/resource/type'
 
-module Puppet::DSL::ResourceTypeAPI
-  def define(name, *args, &block)
-    result = mk_resource_type(:definition, name, Hash.new, block)
-    result.set_arguments(munge_type_arguments(args))
-    result
-  end
-
-  def hostclass(name, options = {}, &block)
-    mk_resource_type(:hostclass, name, options, block)
-  end
-
-  def node(name, options = {}, &block)
-    mk_resource_type(:node, name, options, block)
-  end
-
-  private
-
-  def mk_resource_type(type, name, options, code)
-    klass = Puppet::Resource::Type.new(type, name, options)
-
-    klass.ruby_code = code if code
-
-    Puppet::Node::Environment.new.known_resource_types.add klass
-
-    klass
+# Type of the objects inside of which pure ruby manifest files are
+# executed.  Provides methods for creating defines, hostclasses, and
+# nodes.
+class Puppet::DSL::ResourceTypeAPI
+  def initialize
+    @__created_ast_objects__ = []
   end
 
-  def munge_type_arguments(args)
-    args.inject([]) do |result, item|
+  def define(name, *args, &block)
+    args = args.inject([]) do |result, item|
       if item.is_a?(Hash)
         item.each { |p, v| result << [p, v] }
       else
@@ -36,5 +17,18 @@ module Puppet::DSL::ResourceTypeAPI
       end
       result
     end
+    @__created_ast_objects__.push Puppet::Parser::AST::Definition.new(name, {:arguments => args}, &block)
+    nil
+  end
+
+  def hostclass(name, options = {}, &block)
+    @__created_ast_objects__.push Puppet::Parser::AST::Hostclass.new(name, options, &block)
+    nil
+  end
+
+  def node(name, options = {}, &block)
+    name = [name] unless name.is_a?(Array)
+    @__created_ast_objects__.push Puppet::Parser::AST::Node.new(name, options, &block)
+    nil
   end
 end
diff --git a/lib/puppet/parser/ast/definition.rb b/lib/puppet/parser/ast/definition.rb
index 09f52b5..985f8f2 100644
--- a/lib/puppet/parser/ast/definition.rb
+++ b/lib/puppet/parser/ast/definition.rb
@@ -1,12 +1,15 @@
 require 'puppet/parser/ast/top_level_construct'
 
 class Puppet::Parser::AST::Definition < Puppet::Parser::AST::TopLevelConstruct
-  def initialize(name, context = {})
+  def initialize(name, context = {}, &ruby_code)
     @name = name
     @context = context
+    @ruby_code = ruby_code
   end
 
   def instantiate(modname)
-    return [Puppet::Resource::Type.new(:definition, @name, @context.merge(:module_name => modname))]
+    new_definition = Puppet::Resource::Type.new(:definition, @name, @context.merge(:module_name => modname))
+    new_definition.ruby_code = @ruby_code if @ruby_code
+    [new_definition]
   end
 end
diff --git a/lib/puppet/parser/ast/hostclass.rb b/lib/puppet/parser/ast/hostclass.rb
index d539e4d..cab5e4a 100644
--- a/lib/puppet/parser/ast/hostclass.rb
+++ b/lib/puppet/parser/ast/hostclass.rb
@@ -3,13 +3,16 @@ require 'puppet/parser/ast/top_level_construct'
 class Puppet::Parser::AST::Hostclass < Puppet::Parser::AST::TopLevelConstruct
   attr_accessor :name, :context
 
-  def initialize(name, context = {})
+  def initialize(name, context = {}, &ruby_code)
     @context = context
     @name = name
+    @ruby_code = ruby_code
   end
 
   def instantiate(modname)
-    all_types = [Puppet::Resource::Type.new(:hostclass, @name, @context.merge(:module_name => modname))]
+    new_class = Puppet::Resource::Type.new(:hostclass, @name, @context.merge(:module_name => modname))
+    new_class.ruby_code = @ruby_code if @ruby_code
+    all_types = [new_class]
     if code
       code.each do |nested_ast_node|
         if nested_ast_node.respond_to? :instantiate
diff --git a/lib/puppet/parser/ast/node.rb b/lib/puppet/parser/ast/node.rb
index c19a24c..9767399 100644
--- a/lib/puppet/parser/ast/node.rb
+++ b/lib/puppet/parser/ast/node.rb
@@ -3,15 +3,18 @@ require 'puppet/parser/ast/top_level_construct'
 class Puppet::Parser::AST::Node < Puppet::Parser::AST::TopLevelConstruct
   attr_accessor :names
 
-  def initialize(names, context = {})
+  def initialize(names, context = {}, &ruby_code)
     raise ArgumentError, "names should be an array" unless names.is_a? Array
     @names = names
     @context = context
+    @ruby_code = ruby_code
   end
 
   def instantiate(modname)
     @names.collect do |name|
-      Puppet::Resource::Type.new(:node, name, @context.merge(:module_name => modname))
+      new_node = Puppet::Resource::Type.new(:node, name, @context.merge(:module_name => modname))
+      new_node.ruby_code = @ruby_code if @ruby_code
+      new_node
     end
   end
 end
diff --git a/lib/puppet/parser/parser_support.rb b/lib/puppet/parser/parser_support.rb
index 859897a..a9df33f 100644
--- a/lib/puppet/parser/parser_support.rb
+++ b/lib/puppet/parser/parser_support.rb
@@ -155,8 +155,7 @@ class Puppet::Parser::Parser
   # how should I do error handling here?
   def parse(string = nil)
     if self.file =~ /\.rb$/
-      parse_ruby_file
-      main = nil
+      main = parse_ruby_file
     else
       self.string = string if string
       begin
@@ -196,7 +195,13 @@ class Puppet::Parser::Parser
   end
 
   def parse_ruby_file
-    require self.file
+    # Execute the contents of the file inside its own "main" object so
+    # that it can call methods in the resource type API.
+    main_object = Puppet::DSL::ResourceTypeAPI.new
+    main_object.instance_eval(File.read(self.file))
+
+    # Then extract any types that were created.
+    Puppet::Parser::AST::ASTArray.new :children => main_object.instance_eval { @__created_ast_objects__ }
   end
 
   def string=(string)
diff --git a/spec/integration/parser/ruby_manifest_spec.rb b/spec/integration/parser/ruby_manifest_spec.rb
new file mode 100644
index 0000000..6627a24
--- /dev/null
+++ b/spec/integration/parser/ruby_manifest_spec.rb
@@ -0,0 +1,126 @@
+#!/usr/bin/env ruby
+
+require File.dirname(__FILE__) + '/../../spec_helper'
+
+require 'puppet_spec/files'
+
+describe "Pure ruby manifests" do
+  include PuppetSpec::Files
+
+  before do
+    @node = Puppet::Node.new "testnode"
+
+    @scope_resource = stub 'scope_resource', :builtin? => true, :finish => nil, :ref => 'Class[main]'
+    @scope = stub 'scope', :resource => @scope_resource, :source => mock("source")
+    @test_dir = tmpdir('ruby_manifest_test')
+  end
+
+  after do
+    Puppet.settings.clear
+  end
+
+  def write_file(name, contents)
+    path = File.join(@test_dir, name)
+    File.open(path, "w") { |f| f.write(contents) }
+    path
+  end
+
+  def compile(contents)
+    Puppet[:code] = contents
+    Dir.chdir(@test_dir) do
+      Puppet::Parser::Compiler.compile(Puppet::Node.new("mynode"))
+    end
+  end
+
+  it "should allow classes" do
+    write_file('foo.rb', ["hostclass 'one' do notify('one_notify') end",
+                          "hostclass 'two' do notify('two_notify') end"].join("\n"))
+    catalog = compile("import 'foo'\ninclude one")
+    catalog.resource("Notify[one_notify]").should_not be_nil
+    catalog.resource("Notify[two_notify]").should be_nil
+  end
+
+  it "should allow defines" do
+    write_file('foo.rb', 'define "foo", :arg do notify("foo_#{@name}_#{@arg}") end')
+    catalog = compile("import 'foo'\nfoo { instance: arg => 'xyz' }")
+    catalog.resource("Notify[foo_instance_xyz]").should_not be_nil
+    catalog.resource("Foo[instance]").should_not be_nil
+  end
+
+  it "should allow node declarations" do
+    write_file('foo.rb', ["node 'mynode' do notify('mynode') end",
+                          "node 'theirnode' do notify('theirnode') end"].join("\n"))
+    catalog = compile("import 'foo'")
+    catalog.resource("Notify[mynode]").should_not be_nil
+    catalog.resource("Notify[theirnode]").should be_nil
+  end
+
+  it "should allow access to the environment" do
+    write_file('foo.rb', ["hostclass 'foo' do",
+                          "  if environment.is_a? Puppet::Node::Environment",
+                          "    notify('success')",
+                          "  end",
+                          "end"].join("\n"))
+    compile("import 'foo'\ninclude foo").resource("Notify[success]").should_not be_nil
+  end
+
+  it "should allow creation of built-in types" do
+    write_file('foo.rb', "hostclass 'foo' do file 'test_file', :owner => 'root', :mode => '644' end")
+    catalog = compile("import 'foo'\ninclude foo")
+    file = catalog.resource("File[test_file]")
+    file.should be_a Puppet::Resource
+    file.type.should == 'File'
+    file.title.should == 'test_file'
+    file.exported.should_not be
+    file.virtual.should_not be
+    file[:owner].should == 'root'
+    file[:mode].should == '644'
+    file[:stage].should be_nil # TODO: is this correct behavior?
+  end
+
+  it "should allow calling user-defined functions" do
+    write_file('foo.rb', "hostclass 'foo' do user_func :arg => 'xyz' end")
+    catalog = compile(['define user_func($arg) { notify {"n_$arg": } }',
+                       'user_func { "one": arg => "two" }'].join("\n"))
+    catalog.resource("Notify[n_two]").should_not be_nil
+    catalog.resource("User_func[one]").should_not be_nil
+  end
+
+  it "should be properly cached for multiple compiles" do
+    # Note: we can't test this by calling compile() twice, because
+    # that sets Puppet[:code], which clears out all cached
+    # environments.
+    Puppet[:filetimeout] = 1000
+    write_file('foo.rb', "hostclass 'foo' do notify('success') end")
+    Puppet[:code] = "import 'foo'\ninclude foo"
+
+    # Compile the catalog and check it
+    catalog = Dir.chdir(@test_dir) do
+      Puppet::Parser::Compiler.compile(Puppet::Node.new("mynode"))
+    end
+    catalog.resource("Notify[success]").should_not be_nil
+
+    # Secretly change the file to make it invalid.  This change
+    # shouldn't be noticed because the we've set a high
+    # Puppet[:filetimeout].
+    write_file('foo.rb', "raise 'should not be executed'")
+
+    # Compile the catalog a second time and make sure it's still ok.
+    catalog = Dir.chdir(@test_dir) do
+      Puppet::Parser::Compiler.compile(Puppet::Node.new("mynode"))
+    end
+    catalog.resource("Notify[success]").should_not be_nil
+  end
+
+  it "should be properly reloaded when stale" do
+    Puppet[:filetimeout] = -1 # force stale check to happen all the time
+    write_file('foo.rb', "hostclass 'foo' do notify('version1') end")
+    catalog = compile("import 'foo'\ninclude foo")
+    catalog.resource("Notify[version1]").should_not be_nil
+    sleep 1 # so that timestamp will change forcing file reload
+    write_file('foo.rb', "hostclass 'foo' do notify('version2') end")
+    catalog = compile("import 'foo'\ninclude foo")
+    catalog.resource("Notify[version1]").should be_nil
+    catalog.resource("Notify[version2]").should_not be_nil
+  end
+end
diff --git a/spec/unit/dsl/resource_type_api_spec.rb b/spec/unit/dsl/resource_type_api_spec.rb
index 4b8ccf5..c9a5d27 100755
--- a/spec/unit/dsl/resource_type_api_spec.rb
+++ b/spec/unit/dsl/resource_type_api_spec.rb
@@ -4,41 +4,51 @@ require File.dirname(__FILE__) + '/../../spec_helper'
 
 require 'puppet/dsl/resource_type_api'
 
-class DSLAPITester
-  include Puppet::DSL::ResourceTypeAPI
-end
-
 describe Puppet::DSL::ResourceTypeAPI do
-  before do
-    @api = DSLAPITester.new
+  # Verify that the block creates a single AST node through the API,
+  # instantiate that AST node into a types, and return that type.
+  def test_api_call(&block)
+    main_object = Puppet::DSL::ResourceTypeAPI.new
+    main_object.instance_eval(&block)
+    created_ast_objects = main_object.instance_eval { @__created_ast_objects__ }
+    created_ast_objects.length.should == 1
+    new_types = created_ast_objects[0].instantiate('')
+    new_types.length.should == 1
+    new_types[0]
+  ensure
+    Thread.current[:ruby_file_parse_result] = nil
   end
 
   [:definition, :node, :hostclass].each do |type|
     method = type == :definition ? "define" : type
     it "should be able to create a #{type}" do
-      newtype = @api.send(method, "myname")
+      newtype = test_api_call { send(method, "myname").should == nil }
       newtype.should be_a(Puppet::Resource::Type)
       newtype.type.should == type
     end
 
     it "should use the provided name when creating a #{type}" do
-      newtype = @api.send(method, "myname")
+      newtype = test_api_call { send(method, "myname") }
       newtype.name.should == "myname"
     end
 
     unless type == :definition
       it "should pass in any provided options when creating a #{type}" do
-        newtype = @api.send(method, "myname", :line => 200)
+        newtype = test_api_call { send(method, "myname", :line => 200) }
         newtype.line.should == 200
       end
     end
 
-    it "should set any provided block as the type's ruby code"
-
-    it "should add the type to the current environment's known resource types"
+    it "should set any provided block as the type's ruby code" do
+      newtype = test_api_call { send(method, "myname") { 'method_result' } }
+      newtype.ruby_code.call.should == 'method_result'
+    end
   end
 
   describe "when creating a definition" do
-    it "should use the provided options to define valid arguments for the resource type"
+    it "should use the provided options to define valid arguments for the resource type" do
+      newtype = test_api_call { define("myname", :arg1, :arg2) }
+      newtype.arguments.should == { 'arg1' => nil, 'arg2' => nil }
+    end
   end
 end
diff --git a/spec/unit/parser/parser_spec.rb b/spec/unit/parser/parser_spec.rb
index 0a61e73..a964315 100755
--- a/spec/unit/parser/parser_spec.rb
+++ b/spec/unit/parser/parser_spec.rb
@@ -52,15 +52,6 @@ describe Puppet::Parser do
       @parser.file = "/my/file.rb"
       @parser.parse
     end
-
-    describe "in ruby" do
-      it "should use the ruby interpreter to load the file" do
-        @parser.file = "/my/file.rb"
-        @parser.expects(:require).with "/my/file.rb"
-
-        @parser.parse_ruby_file
-      end
-    end
   end
 
   describe "when parsing append operator" do

-- 
Puppet packaging for Debian



More information about the Pkg-puppet-devel mailing list