Merge branch '2.6.next' of github.com:puppetlabs/puppet into 2.6.next

* '2.6.next' of github.com:puppetlabs/puppet:
  (#5392) Give a better error when realizing a non-existant resource
  (#2645) Adding a less-stubby test to verify the "system" attribute's behavior
  maint: Remove serialization of InventoryFact values
  maint: Rename InventoryHost to InventoryNode
  Fixed #2645 - Added support for creating system users
  maint: Remove spec run noise
  (#6338) Support searching on metadata in InventoryActiveRecord terminus
  (#6338) Implement search for InventoryActiveRecord facts terminus
  (#6338) Add an InventoryActiveRecord terminus for Facts

9 files changed, 237 insertions, 19 deletions

diff --git a/spec/unit/indirector/facts/inventory_active_record_spec.rb b/spec/unit/indirector/facts/inventory_active_record_spec.rb
new file mode 100644
index 000000000..c29e58400
--- /dev/null
+++ b/spec/unit/indirector/facts/inventory_active_record_spec.rb
@@ -0,0 +1,166 @@
+#!/usr/bin/env ruby
+
+require File.dirname(__FILE__) + '/../../../spec_helper'
+require 'sqlite3' rescue nil
+require 'tempfile'
+require 'puppet/rails'
+
+describe "Puppet::Node::Facts::InventoryActiveRecord", :if => (Puppet.features.rails? and defined? SQLite3) do
+  let(:terminus) { Puppet::Node::Facts::InventoryActiveRecord.new }
+
+  before :all do
+    require 'puppet/indirector/facts/inventory_active_record'
+    @dbfile = Tempfile.new("testdb")
+    @dbfile.close
+  end
+
+  after :all do
+    Puppet::Node::Facts.indirection.reset_terminus_class
+    @dbfile.unlink
+  end
+
+  before :each do
+    Puppet::Node::Facts.terminus_class = :inventory_active_record
+    Puppet[:dbadapter]  = 'sqlite3'
+    Puppet[:dblocation] = @dbfile.path
+    Puppet[:railslog] = "/dev/null"
+    Puppet::Rails.init
+  end
+
+  after :each do
+    Puppet::Rails.teardown
+  end
+
+  describe "#save" do
+    it "should use an existing node if possible" do
+      node = Puppet::Rails::InventoryNode.new(:name => "foo", :timestamp => Time.now)
+      node.save
+      Puppet::Node::Facts.new("foo", "uptime_days" => "60", "kernel" => "Darwin").save
+
+      Puppet::Rails::InventoryNode.count.should == 1
+      Puppet::Rails::InventoryNode.first.should == node
+    end
+
+    it "should create a new node if one can't be found" do
+      # This test isn't valid if there are nodes to begin with
+      Puppet::Rails::InventoryNode.count.should == 0
+
+      Puppet::Node::Facts.new("foo", "uptime_days" => "60", "kernel" => "Darwin").save
+
+      Puppet::Rails::InventoryNode.count.should == 1
+      Puppet::Rails::InventoryNode.first.name.should == "foo"
+    end
+
+    it "should save the facts" do
+      Puppet::Node::Facts.new("foo", "uptime_days" => "60", "kernel" => "Darwin").save
+
+      Puppet::Rails::InventoryFact.all.map{|f| [f.name,f.value]}.should =~ [["uptime_days","60"],["kernel","Darwin"]]
+    end
+
+    it "should remove the previous facts for an existing node" do
+      Puppet::Node::Facts.new("foo", "uptime_days" => "30", "kernel" => "Darwin").save
+      bar_facts = Puppet::Node::Facts.new("bar", "uptime_days" => "35", "kernel" => "Linux")
+      foo_facts = Puppet::Node::Facts.new("foo", "uptime_days" => "60", "is_virtual" => "false")
+      bar_facts.save
+      foo_facts.save
+
+      Puppet::Node::Facts.find("bar").should == bar_facts
+      Puppet::Node::Facts.find("foo").should == foo_facts
+      Puppet::Rails::InventoryFact.all.map{|f| [f.name,f.value]}.should_not include(["uptime_days", "30"], ["kernel", "Darwin"])
+    end
+
+    it "should not replace the node's facts if something goes wrong" do
+    end
+  end
+
+  describe "#find" do
+    before do
+      @foo_facts = Puppet::Node::Facts.new("foo", "uptime_days" => "60", "kernel" => "Darwin")
+      @bar_facts = Puppet::Node::Facts.new("bar", "uptime_days" => "30", "kernel" => "Linux")
+      @foo_facts.save
+      @bar_facts.save
+    end
+
+    it "should identify facts by node name" do
+      Puppet::Node::Facts.find("foo").should == @foo_facts
+    end
+
+    it "should return nil if no node instance can be found" do
+      Puppet::Node::Facts.find("non-existent node").should == nil
+    end
+  end
+
+  describe "#search" do
+    def search_request(conditions)
+      Puppet::Indirector::Request.new(:facts, :search, nil, conditions)
+    end
+
+    before :each do
+      @now = Time.now
+      @foo = Puppet::Node::Facts.new("foo", "fact1" => "value1", "fact2" => "value2", "uptime_days" => "30")
+      @bar = Puppet::Node::Facts.new("bar", "fact1" => "value1", "uptime_days" => "60")
+      @baz = Puppet::Node::Facts.new("baz", "fact1" => "value2", "fact2" => "value1", "uptime_days" => "90")
+      @bat = Puppet::Node::Facts.new("bat")
+      @foo.timestamp = @now - 3600*1
+      @bar.timestamp = @now - 3600*3
+      @baz.timestamp = @now - 3600*5
+      @bat.timestamp = @now - 3600*7
+      @foo.save
+      @bar.save
+      @baz.save
+      @bat.save
+    end
+
+    it "should return node names that match 'equal' constraints" do
+      request = search_request('facts.fact1.eq' => 'value1',
+                               'facts.fact2.eq' => 'value2')
+      terminus.search(request).should == ["foo"]
+    end
+
+    it "should return node names that match 'not equal' constraints" do
+      request = search_request('facts.fact1.ne' => 'value2')
+      terminus.search(request).should == ["bar","foo"]
+    end
+
+    it "should return node names that match strict inequality constraints" do
+      request = search_request('facts.uptime_days.gt' => '20',
+                               'facts.uptime_days.lt' => '70')
+      terminus.search(request).should == ["bar","foo"]
+    end
+
+    it "should return node names that match non-strict inequality constraints" do
+      request = search_request('facts.uptime_days.ge' => '30',
+                               'facts.uptime_days.le' => '60')
+      terminus.search(request).should == ["bar","foo"]
+    end
+
+    it "should return node names whose facts are within a given timeframe" do
+      request = search_request('meta.timestamp.ge' => @now - 3600*5,
+                               'meta.timestamp.le' => @now - 3600*1)
+      terminus.search(request).should == ["bar","baz","foo"]
+    end
+
+    it "should return node names whose facts are from a specific time" do
+      request = search_request('meta.timestamp.eq' => @now - 3600*3)
+      terminus.search(request).should == ["bar"]
+    end
+
+    it "should return node names whose facts are not from a specific time" do
+      request = search_request('meta.timestamp.ne' => @now - 3600*1)
+      terminus.search(request).should == ["bar","bat","baz"]
+    end
+
+    it "should perform strict searches on nodes by timestamp" do
+      request = search_request('meta.timestamp.gt' => @now - 3600*5,
+                               'meta.timestamp.lt' => @now - 3600*1)
+      terminus.search(request).should == ["bar"]
+    end
+
+    it "should search nodes based on both facts and timestamp values" do
+      request = search_request('facts.uptime_days.gt' => '45',
+                               'meta.timestamp.lt'    => @now - 3600*4)
+      terminus.search(request).should == ["baz"]
+    end
+  end
+end
+
diff --git a/spec/unit/indirector/queue_spec.rb b/spec/unit/indirector/queue_spec.rb
index 00463ee0f..bbe00c75f 100755
--- a/spec/unit/indirector/queue_spec.rb
+++ b/spec/unit/indirector/queue_spec.rb
@@ -114,6 +114,9 @@ describe Puppet::Indirector::Queue, :if => Puppet.features.pson? do
       @store_class.client.expects(:subscribe).yields("foo")
       @store_class.expects(:intern).raises ArgumentError
       Puppet.expects(:err)
+
+      @store_class.expects(:puts)
+
       @store_class.subscribe {|o| o }
     end
   end
diff --git a/spec/unit/module_spec.rb b/spec/unit/module_spec.rb
index 37dad7e25..0b4873f5f 100755
--- a/spec/unit/module_spec.rb
+++ b/spec/unit/module_spec.rb
@@ -367,9 +367,9 @@ describe Puppet::Module do
     mod.stubs(:path).returns "/a/foo"
     FileTest.expects(:exist?).with("/a/foo/plugins").returns true
 
-    mod.expects(:warning)
-
     mod.plugin_directory.should == "/a/foo/plugins"
+    @logs.first.message.should == "using the deprecated 'plugins' directory for ruby extensions; please move to 'lib'"
+    @logs.first.level.should == :warning
   end
 
   it "should default to 'lib' for the plugins directory" do
diff --git a/spec/unit/network/handler/fileserver_spec.rb b/spec/unit/network/handler/fileserver_spec.rb
index b37d4f551..9d34e9cdd 100644
--- a/spec/unit/network/handler/fileserver_spec.rb
+++ b/spec/unit/network/handler/fileserver_spec.rb
@@ -158,7 +158,7 @@ describe Puppet::Network::Handler::FileServer do
     end
 
     it "should not fail for inexistant plugins type" do
-      lambda { @mount.list("puppet/parser",true,false) }.should_not raise_error
+      @mount.list("puppet/parser",true,false)
     end
 
   end
diff --git a/spec/unit/parser/ast/collection_spec.rb b/spec/unit/parser/ast/collection_spec.rb
index 392a2c0f0..cc33075b7 100755
--- a/spec/unit/parser/ast/collection_spec.rb
+++ b/spec/unit/parser/ast/collection_spec.rb
@@ -4,20 +4,21 @@ require File.dirname(__FILE__) + '/../../../spec_helper'
 
 describe Puppet::Parser::AST::Collection do
   before :each do
-    @scope = stub_everything 'scope'
-    @mytype = stub_everything('mytype')
-    @scope.stubs(:find_resource_type).returns @mytype
-    @compiler = stub_everything 'compile'
-    @scope.stubs(:compiler).returns(@compiler)
+    @mytype = Puppet::Resource::Type.new(:definition, "mytype")
+    @environment = Puppet::Node::Environment.new
+    @environment.known_resource_types.add @mytype
+
+    @compiler = Puppet::Parser::Compiler.new(Puppet::Node.new("foonode", :environment => @environment))
+    @scope = Puppet::Parser::Scope.new(:compiler => @compiler)
 
     @overrides = stub_everything 'overrides'
     @overrides.stubs(:is_a?).with(Puppet::Parser::AST).returns(true)
-
   end
 
   it "should evaluate its query" do
     query = mock 'query'
     collection = Puppet::Parser::AST::Collection.new :query => query, :form => :virtual
+    collection.type = 'mytype'
 
     query.expects(:safeevaluate).with(@scope)
 
@@ -26,8 +27,8 @@ describe Puppet::Parser::AST::Collection do
 
   it "should instantiate a Collector for this type" do
     collection = Puppet::Parser::AST::Collection.new :form => :virtual, :type => "test"
-    @test_type = stub 'type', :name => 'test'
-    @scope.expects(:find_resource_type).with('test').returns @test_type
+    @test_type = Puppet::Resource::Type.new(:definition, "test")
+    @environment.known_resource_types.add @test_type
 
     Puppet::Parser::Collector.expects(:new).with(@scope, "test", nil, nil, :virtual)
 
@@ -35,7 +36,7 @@ describe Puppet::Parser::AST::Collection do
   end
 
   it "should tell the compiler about this collector" do
-    collection = Puppet::Parser::AST::Collection.new :form => :virtual, :type => "test"
+    collection = Puppet::Parser::AST::Collection.new :form => :virtual, :type => "mytype"
     Puppet::Parser::Collector.stubs(:new).returns("whatever")
 
     @compiler.expects(:add_collection).with("whatever")
@@ -45,7 +46,7 @@ describe Puppet::Parser::AST::Collection do
 
   it "should evaluate overriden paramaters" do
     collector = stub_everything 'collector'
-    collection = Puppet::Parser::AST::Collection.new :form => :virtual, :type => "test", :override => @overrides
+    collection = Puppet::Parser::AST::Collection.new :form => :virtual, :type => "mytype", :override => @overrides
     Puppet::Parser::Collector.stubs(:new).returns(collector)
 
     @overrides.expects(:safeevaluate).with(@scope)
@@ -55,7 +56,7 @@ describe Puppet::Parser::AST::Collection do
 
   it "should tell the collector about overrides" do
     collector = mock 'collector'
-    collection = Puppet::Parser::AST::Collection.new :form => :virtual, :type => "test", :override => @overrides
+    collection = Puppet::Parser::AST::Collection.new :form => :virtual, :type => "mytype", :override => @overrides
     Puppet::Parser::Collector.stubs(:new).returns(collector)
 
     collector.expects(:add_override)
@@ -63,5 +64,8 @@ describe Puppet::Parser::AST::Collection do
     collection.evaluate(@scope)
   end
 
-
+  it "should fail when evaluating undefined resource types" do
+    collection = Puppet::Parser::AST::Collection.new :form => :virtual, :type => "bogus"
+    lambda { collection.evaluate(@scope) }.should raise_error "Resource type bogus doesn't exist"
+  end
 end
diff --git a/spec/unit/parser/parser_spec.rb b/spec/unit/parser/parser_spec.rb
index 6cc393d91..2ed279fd9 100755
--- a/spec/unit/parser/parser_spec.rb
+++ b/spec/unit/parser/parser_spec.rb
@@ -259,7 +259,7 @@ describe Puppet::Parser do
     before do
       @lexer = stub 'lexer', :line => 50, :file => "/foo/bar", :getcomment => "whev"
       @parser.stubs(:lexer).returns @lexer
-      @class = stub 'class', :use_docs => false
+      @class = Puppet::Resource::Type.new(:hostclass, "myclass", :use_docs => false)
     end
 
     it "should return a new instance of the provided class created with the provided options" do
diff --git a/spec/unit/provider/user/user_role_add_spec.rb b/spec/unit/provider/user/user_role_add_spec.rb
index 9cf649267..f73942389 100644
--- a/spec/unit/provider/user/user_role_add_spec.rb
+++ b/spec/unit/provider/user/user_role_add_spec.rb
@@ -115,6 +115,7 @@ describe provider_class do
   describe "when allow duplicate is enabled" do
     before do
       @resource.expects(:allowdupe?).returns true
+      @resource.stubs(:system?)
       @provider.stubs(:is_role?).returns(false)
       @provider.stubs(:execute)
       @provider.expects(:execute).with { |args| args.include?("-o") }
diff --git a/spec/unit/provider/user/useradd_spec.rb b/spec/unit/provider/user/useradd_spec.rb
index 9ebba596c..fd75c43f3 100755
--- a/spec/unit/provider/user/useradd_spec.rb
+++ b/spec/unit/provider/user/useradd_spec.rb
@@ -15,6 +15,7 @@ describe provider_class do
   # #1360
   it "should add -o when allowdupe is enabled and the user is being created" do
     @resource.expects(:allowdupe?).returns true
+    @resource.expects(:system?).returns true
     @provider.stubs(:execute)
     @provider.expects(:execute).with { |args| args.include?("-o") }
     @provider.create
@@ -27,6 +28,14 @@ describe provider_class do
     @provider.uid = 150
   end
 
+  it "should add -r when system is enabled" do
+    @resource.expects(:allowdupe?).returns true
+    @resource.expects(:system?).returns true
+    @provider.stubs(:execute)
+    @provider.expects(:execute).with { |args| args.include?("-r") }
+    @provider.create
+  end
+
   it "should set password age rules" do
     provider_class.has_feature :manages_password_age
     @resource = Puppet::Type.type(:user).new :name => "myuser", :password_min_age => 5, :password_max_age => 10, :provider => :useradd
@@ -53,6 +62,23 @@ describe provider_class do
     end
   end
 
+  describe "when checking to add system users" do
+    it "should check system users" do
+      @resource.expects(:system?)
+      @provider.check_system_users
+    end
+
+    it "should return an array with a flag if it's a system user" do
+      @resource.stubs(:system?).returns true
+      @provider.check_system_users.must == ["-r"]
+    end
+
+    it "should return an empty array if it's not a system user" do
+      @resource.stubs(:system?).returns false
+      @provider.check_system_users.must == []
+    end
+  end
+
   describe "when checking manage home" do
     it "should check manage home" do
       @resource.expects(:managehome?)
@@ -88,6 +114,7 @@ describe provider_class do
     before do
       @resource.stubs(:allowdupe?).returns true
       @resource.stubs(:managehome?).returns true
+      @resource.stubs(:system?).returns true
     end
 
     it "should call command with :add" do
@@ -115,20 +142,32 @@ describe provider_class do
       @provider.addcmd
     end
 
+    it "should return an array with -r if system? is true" do
+      resource = Puppet::Type.type(:user).new( :name => "bob", :system => true)
+
+      provider_class.new( resource ).addcmd.should include("-r")
+    end
+
+    it "should return an array without -r if system? is false" do
+      resource = Puppet::Type.type(:user).new( :name => "bob", :system => false)
+
+      provider_class.new( resource ).addcmd.should_not include("-r")
+    end
+
     it "should return an array with full command" do
       @provider.stubs(:command).with(:add).returns("useradd")
       @provider.stubs(:add_properties).returns(["-G", "somegroup"])
       @resource.stubs(:[]).with(:name).returns("someuser")
       @resource.stubs(:[]).with(:expiry).returns("somedate")
-      @provider.addcmd.must == ["useradd", "-G", "somegroup", "-o", "-m", '-e somedate', "someuser"]
+      @provider.addcmd.must == ["useradd", "-G", "somegroup", "-o", "-m", '-e somedate', "-r", "someuser"]
     end
 
-    it "should return an array without -e if expery is undefined full command" do
+    it "should return an array without -e if expiry is undefined full command" do
       @provider.stubs(:command).with(:add).returns("useradd")
       @provider.stubs(:add_properties).returns(["-G", "somegroup"])
       @resource.stubs(:[]).with(:name).returns("someuser")
       @resource.stubs(:[]).with(:expiry).returns nil
-      @provider.addcmd.must == ["useradd", "-G", "somegroup", "-o", "-m", "someuser"]
+      @provider.addcmd.must == ["useradd", "-G", "somegroup", "-o", "-m", "-r", "someuser"]
     end
   end
 
@@ -136,6 +175,7 @@ describe provider_class do
     before do
       @resource.stubs(:allowdupe?).returns true
       @resource.stubs(:managehome?).returns true
+      @resource.stubs(:system?).returns true
     end
 
     it "should call command with :pass" do
diff --git a/spec/unit/type/user_spec.rb b/spec/unit/type/user_spec.rb
index 297134446..5a84af443 100755
--- a/spec/unit/type/user_spec.rb
+++ b/spec/unit/type/user_spec.rb
@@ -43,6 +43,10 @@ describe user do
     user.provider_feature(:manages_password_age).should_not be_nil
   end
 
+  it "should have a system_users feature" do
+    user.provider_feature(:system_users).should_not be_nil
+  end
+
   describe "instances" do
     it "should have a valid provider" do
       user.new(:name => "foo").provider.class.ancestors.should be_include(Puppet::Provider)