Made the following modifications to ResourceTypeAPI: (1) returned nil from “define”, “hostclass”, and “node”.
(2) renamed “mk_resource_type” and “munge_type_arguments” to “__mk_resource_type__” and “__munge_type_arguments__” to discourage customers from calling them. (3) Made ResourceTypeAPI a class rather than a module, and changed the parser to evaluate the contents of pure ruby manifests using a instances of this class. (4) Changed ResourceTypeAPI to insert newly instantiated types into Thread.current[:known_resource_types] rather than the default environment's known_resource_types. This effectively backports the fix for issue #4657 to 2.6.x. Also backported the new spec tests from #4657. Signed-off-by: Paul Berry <[email protected]> --- lib/puppet/dsl.rb | 4 - lib/puppet/dsl/resource_type_api.rb | 22 ++-- lib/puppet/parser/parser_support.rb | 4 +- spec/integration/parser/ruby_manifest_spec.rb | 127 +++++++++++++++++++++++++ spec/unit/dsl/resource_type_api_spec.rb | 44 ++++++--- spec/unit/parser/parser_spec.rb | 9 -- 6 files changed, 173 insertions(+), 37 deletions(-) create mode 100644 spec/integration/parser/ruby_manifest_spec.rb 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..90dd5b2 100644 --- a/lib/puppet/dsl/resource_type_api.rb +++ b/lib/puppet/dsl/resource_type_api.rb @@ -1,33 +1,33 @@ require 'puppet/resource/type' -module Puppet::DSL::ResourceTypeAPI +class 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 + result = __mk_resource_type__(:definition, name, Hash.new, block) + result.set_arguments(__munge_type_arguments__(args)) + nil end def hostclass(name, options = {}, &block) - mk_resource_type(:hostclass, name, options, block) + __mk_resource_type__(:hostclass, name, options, block) + nil end def node(name, options = {}, &block) - mk_resource_type(:node, name, options, block) + __mk_resource_type__(:node, name, options, block) + nil end - private - - def mk_resource_type(type, name, options, code) + 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 + Thread.current[:known_resource_types].add klass klass end - def munge_type_arguments(args) + def __munge_type_arguments__(args) args.inject([]) do |result, item| if item.is_a?(Hash) item.each { |p, v| result << [p, v] } diff --git a/lib/puppet/parser/parser_support.rb b/lib/puppet/parser/parser_support.rb index c0fd371..25bf25f 100644 --- a/lib/puppet/parser/parser_support.rb +++ b/lib/puppet/parser/parser_support.rb @@ -215,7 +215,9 @@ 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. + Puppet::DSL::ResourceTypeAPI.new.instance_eval(File.read(self.file)) 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..9ff7805 --- /dev/null +++ b/spec/integration/parser/ruby_manifest_spec.rb @@ -0,0 +1,127 @@ +#!/usr/bin/env ruby + +require File.dirname(__FILE__) + '/../../spec_helper' + +require 'tempfile' +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 5abe79e..ba396ac 100755 --- a/spec/unit/dsl/resource_type_api_spec.rb +++ b/spec/unit/dsl/resource_type_api_spec.rb @@ -4,13 +4,14 @@ 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 + # Run the given block in the context of a new ResourceTypeAPI + # object. + def test_api_call(&block) + Thread.current[:known_resource_types] = Puppet::Resource::TypeCollection.new(:env) + Puppet::DSL::ResourceTypeAPI.new.instance_eval(&block) + ensure + Thread.current[:known_resource_types] = nil end [:definition, :node, :hostclass].each do |type| @@ -18,29 +19,48 @@ describe Puppet::DSL::ResourceTypeAPI do it "should be able to create a #{type}" do newtype = Puppet::Resource::Type.new(:hostclass, "foo") Puppet::Resource::Type.expects(:new).with { |t, n, args| t == type }.returns newtype - @api.send(method, "myname") + test_api_call { send(method, "myname") } end it "should use the provided name when creating a #{type}" do type = Puppet::Resource::Type.new(:hostclass, "foo") Puppet::Resource::Type.expects(:new).with { |t, n, args| n == "myname" }.returns type - @api.send(method, "myname") + test_api_call { send(method, "myname") } end unless type == :definition it "should pass in any provided options" do type = Puppet::Resource::Type.new(:hostclass, "foo") Puppet::Resource::Type.expects(:new).with { |t, n, args| args == {:myarg => :myvalue} }.returns type - @api.send(method, "myname", :myarg => :myvalue) + test_api_call { send(method, "myname", :myarg => :myvalue) } end end - it "should set any provided block as the type's ruby code" + it "should set any provided block as the type's ruby code" do + Puppet::Resource::Type.any_instance.expects(:ruby_code=) + test_api_call { send(method, "myname") { 'method_result' } } + end - it "should add the type to the current environment's known resource types" + it "should add the type to the current environment's known resource types" do + begin + newtype = Puppet::Resource::Type.new(:hostclass, "foo") + Puppet::Resource::Type.expects(:new).returns newtype + known_resource_types = Puppet::Resource::TypeCollection.new(:env) + Thread.current[:known_resource_types] = known_resource_types + known_resource_types.expects(:add).with(newtype) + Puppet::DSL::ResourceTypeAPI.new.instance_eval { hostclass "myname" } + ensure + Thread.current[:known_resource_types] = nil + end + 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 = Puppet::Resource::Type.new(:definition, "foo") + Puppet::Resource::Type.expects(:new).returns newtype + test_api_call { define("myname", :arg1, :arg2) } + newtype.instance_eval { @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 0657ab3..f73e07a 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 -- 1.7.2 -- 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.
