Fixing #6851 - ResourceType#find/search loads types

Previously we could only find types from site.pp, but
we now automatically load the specified type (for find)
or all types.

This also adds a TypeLoader#import_all capable of importing
all manifests (ruby or puppet) on a given system.

Signed-off-by: Luke Kanies <luke@puppetlabs.com>
Reviewed-by: Daniel Pittman <daniel@puppetlabs.com>

4 files changed, 188 insertions, 17 deletions

diff --git a/lib/puppet/indirector/resource_type/parser.rb b/lib/puppet/indirector/resource_type/parser.rb
index 24b4b065e..fd5b3938a 100644
--- a/lib/puppet/indirector/resource_type/parser.rb
+++ b/lib/puppet/indirector/resource_type/parser.rb
@@ -10,7 +10,10 @@ class Puppet::Indirector::ResourceType::Parser < Puppet::Indirector::Code
 
     # This is a bit ugly.
     [:hostclass, :definition, :node].each do |type|
-      if r = krt.send(type, request.key)
+      # We have to us 'find_<type>' here because it will
+      # load any missing types from disk, whereas the plain
+      # '<type>' method only returns from memory.
+      if r = krt.send("find_#{type}", [""], request.key)
         return r
       end
     end
@@ -18,16 +21,11 @@ class Puppet::Indirector::ResourceType::Parser < Puppet::Indirector::Code
   end
 
   def search(request)
-=begin
-    @modulepath = set_modulepath(options[:modulepath])
-    get_code(@modulepath)[:manifests].collect do |file|
-      Puppet[:manifest]=file
-      get_resources_of_type(:hostclass)
-    end.flatten
-=end
     raise ArgumentError, "Only '*' is acceptable as a search request" unless request.key == "*"
     krt = request.environment.known_resource_types
-    result = [krt.hostclasses.values, krt.definitions.values, krt.nodes.values].flatten
+    # Make sure we've got all of the types loaded.
+    krt.loader.import_all
+    result = [krt.hostclasses.values, krt.definitions.values, krt.nodes.values].flatten.reject { |t| t.name == "" }
     return nil if result.empty?
     result
   end
diff --git a/lib/puppet/parser/type_loader.rb b/lib/puppet/parser/type_loader.rb
index 140c9f2ca..1fba73d0b 100644
--- a/lib/puppet/parser/type_loader.rb
+++ b/lib/puppet/parser/type_loader.rb
@@ -92,6 +92,34 @@ class Puppet::Parser::TypeLoader
     end
   end
 
+  def import_all
+    require 'find'
+
+    module_names = []
+    # Collect the list of all known modules
+    environment.modulepath.each do |path|
+      Dir.chdir(path) do
+        Dir.glob("*").each do |dir|
+          next unless FileTest.directory?(dir)
+          module_names << dir
+        end
+      end
+    end
+
+    module_names.uniq!
+    # And then load all files from each module, but (relying on system
+    # behavior) only load files from the first module of a given name.  E.g.,
+    # given first/foo and second/foo, only files from first/foo will be loaded.
+    module_names.each do |name|
+      mod = Puppet::Module.new(name, environment)
+      Find.find(File.join(mod.path, "manifests")) do |path|
+        if path =~ /\.pp$/ or path =~ /\.rb$/
+          import(path)
+        end
+      end
+    end
+  end
+
   def known_resource_types
     environment.known_resource_types
   end
diff --git a/spec/unit/indirector/resource_type/parser_spec.rb b/spec/unit/indirector/resource_type/parser_spec.rb
index 739e58b35..27e61486c 100755
--- a/spec/unit/indirector/resource_type/parser_spec.rb
+++ b/spec/unit/indirector/resource_type/parser_spec.rb
@@ -3,13 +3,15 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../../spec_helper')
 
 require 'puppet/indirector/resource_type/parser'
+require 'puppet_spec/files'
 
 describe Puppet::Indirector::ResourceType::Parser do
+  include PuppetSpec::Files
+
   before do
     @terminus = Puppet::Indirector::ResourceType::Parser.new
     @request = Puppet::Indirector::Request.new(:resource_type, :find, "foo")
-    @krt = Puppet::Resource::TypeCollection.new(@request.environment)
-    @request.environment.stubs(:known_resource_types).returns @krt
+    @krt = @request.environment.known_resource_types
   end
 
   it "should be registered with the resource_type indirection" do
@@ -17,16 +19,29 @@ describe Puppet::Indirector::ResourceType::Parser do
   end
 
   describe "when finding" do
-    it "should use the request's environment's list of known resource types" do
-      @request.environment.known_resource_types.expects(:hostclass).returns nil
+    it "should return any found type from the request's environment" do
+      type = Puppet::Resource::Type.new(:hostclass, "foo")
+      @request.environment.known_resource_types.add(type)
 
-      @terminus.find(@request)
+      @terminus.find(@request).should == type
     end
 
-    it "should return any found type" do
-      type = @krt.add(Puppet::Resource::Type.new(:hostclass, "foo"))
+    it "should attempt to load the type if none is found in memory" do
+      dir = tmpdir("find_a_type")
+      FileUtils.mkdir_p(dir)
+      Puppet[:modulepath] = dir
 
-      @terminus.find(@request).should == type
+      # Make a new request, since we've reset the env
+      @request = Puppet::Indirector::Request.new(:resource_type, :find, "foo::bar")
+
+      manifest_path = File.join(dir, "foo", "manifests")
+      FileUtils.mkdir_p(manifest_path)
+
+      File.open(File.join(manifest_path, "bar.pp"), "w") { |f| f.puts "class foo::bar {}" }
+
+      result = @terminus.find(@request)
+      result.should be_instance_of(Puppet::Resource::Type)
+      result.name.should == "foo::bar"
     end
 
     it "should return nil if no type can be found" do
@@ -68,8 +83,41 @@ describe Puppet::Indirector::ResourceType::Parser do
       result.should be_include(define)
     end
 
+    it "should not return the 'main' class" do
+      main = @krt.add(Puppet::Resource::Type.new(:hostclass, ""))
+
+      # So there is a return value
+      foo = @krt.add(Puppet::Resource::Type.new(:hostclass, "foo"))
+
+      @terminus.search(@request).should_not be_include(main)
+    end
+
     it "should return nil if no types can be found" do
       @terminus.search(@request).should be_nil
     end
+
+    it "should load all resource types from all search paths" do
+      dir = tmpdir("searching_in_all")
+      first = File.join(dir, "first")
+      second = File.join(dir, "second")
+      FileUtils.mkdir_p(first)
+      FileUtils.mkdir_p(second)
+      Puppet[:modulepath] = "#{first}:#{second}"
+
+      # Make a new request, since we've reset the env
+      @request = Puppet::Indirector::Request.new(:resource_type, :search, "*")
+
+      onepath = File.join(first, "one", "manifests")
+      FileUtils.mkdir_p(onepath)
+      twopath = File.join(first, "two", "manifests")
+      FileUtils.mkdir_p(twopath)
+
+      File.open(File.join(onepath, "oneklass.pp"), "w") { |f| f.puts "class one::oneklass {}" }
+      File.open(File.join(twopath, "twoklass.pp"), "w") { |f| f.puts "class two::twoklass {}" }
+
+      result = @terminus.search(@request)
+      result.find { |t| t.name == "one::oneklass" }.should be_instance_of(Puppet::Resource::Type)
+      result.find { |t| t.name == "two::twoklass" }.should be_instance_of(Puppet::Resource::Type)
+    end
   end
 end
diff --git a/spec/unit/parser/type_loader_spec.rb b/spec/unit/parser/type_loader_spec.rb
index bd41adfb6..12bc1ccd6 100644
--- a/spec/unit/parser/type_loader_spec.rb
+++ b/spec/unit/parser/type_loader_spec.rb
@@ -93,6 +93,103 @@ describe Puppet::Parser::TypeLoader do
     end
   end
 
+  describe "when importing all" do
+    before do
+      @base = tmpdir("base")
+
+      # Create two module path directories
+      @modulebase1 = File.join(@base, "first")
+      FileUtils.mkdir_p(@modulebase1)
+      @modulebase2 = File.join(@base, "second")
+      FileUtils.mkdir_p(@modulebase2)
+
+      Puppet[:modulepath] = "#{@modulebase1}:#{@modulebase2}"
+    end
+
+    def mk_module(basedir, name)
+      module_dir = File.join(basedir, name)
+
+      # Go ahead and make our manifest directory
+      FileUtils.mkdir_p(File.join(module_dir, "manifests"))
+
+      return Puppet::Module.new(name)
+    end
+
+    # We have to pass the base path so that we can
+    # write to modules that are in the second search path
+    def mk_manifests(base, mod, type, files)
+      exts = {"ruby" => ".rb", "puppet" => ".pp"}
+      files.collect do |file|
+        name = mod.name + "::" + file.gsub("/", "::")
+        path = File.join(base, mod.name, "manifests", file + exts[type])
+        FileUtils.mkdir_p(File.split(path)[0])
+
+        # write out the class
+        if type == "ruby"
+          File.open(path, "w") { |f| f.print "hostclass '#{name}' do\nend" }
+        else
+          File.open(path, "w") { |f| f.print "class #{name} {}" }
+        end
+        name
+      end
+    end
+
+    it "should load all puppet manifests from all modules in the specified environment" do
+      @module1 = mk_module(@modulebase1, "one")
+      @module2 = mk_module(@modulebase2, "two")
+
+      mk_manifests(@modulebase1, @module1, "puppet", %w{a b})
+      mk_manifests(@modulebase2, @module2, "puppet", %w{c d})
+
+      @loader.import_all
+
+      @loader.environment.known_resource_types.hostclass("one::a").should be_instance_of(Puppet::Resource::Type)
+      @loader.environment.known_resource_types.hostclass("one::b").should be_instance_of(Puppet::Resource::Type)
+      @loader.environment.known_resource_types.hostclass("two::c").should be_instance_of(Puppet::Resource::Type)
+      @loader.environment.known_resource_types.hostclass("two::d").should be_instance_of(Puppet::Resource::Type)
+    end
+
+    it "should load all ruby manifests from all modules in the specified environment" do
+      @module1 = mk_module(@modulebase1, "one")
+      @module2 = mk_module(@modulebase2, "two")
+
+      mk_manifests(@modulebase1, @module1, "ruby", %w{a b})
+      mk_manifests(@modulebase2, @module2, "ruby", %w{c d})
+
+      @loader.import_all
+
+      @loader.environment.known_resource_types.hostclass("one::a").should be_instance_of(Puppet::Resource::Type)
+      @loader.environment.known_resource_types.hostclass("one::b").should be_instance_of(Puppet::Resource::Type)
+      @loader.environment.known_resource_types.hostclass("two::c").should be_instance_of(Puppet::Resource::Type)
+      @loader.environment.known_resource_types.hostclass("two::d").should be_instance_of(Puppet::Resource::Type)
+    end
+
+    it "should not load manifests from duplicate modules later in the module path" do
+      @module1 = mk_module(@modulebase1, "one")
+
+      # duplicate
+      @module2 = mk_module(@modulebase2, "one")
+
+      mk_manifests(@modulebase1, @module1, "puppet", %w{a})
+      mk_manifests(@modulebase2, @module2, "puppet", %w{c})
+
+      @loader.import_all
+
+      @loader.environment.known_resource_types.hostclass("one::c").should be_nil
+    end
+
+    it "should load manifests from subdirectories" do
+      @module1 = mk_module(@modulebase1, "one")
+
+      mk_manifests(@modulebase1, @module1, "puppet", %w{a a/b a/b/c})
+
+      @loader.import_all
+
+      @loader.environment.known_resource_types.hostclass("one::a::b").should be_instance_of(Puppet::Resource::Type)
+      @loader.environment.known_resource_types.hostclass("one::a::b::c").should be_instance_of(Puppet::Resource::Type)
+    end
+  end
+
   describe "when parsing a file" do
     before do
       @parser = Puppet::Parser::Parser.new(@loader.environment)