Fixing #432 - you can now manage home dirs with users. You cannot yet purge home directories, because there is still controversy over how that should be done. Also, allowdupe is now handled like a feature, which is, um, better.

git-svn-id: https://reductivelabs.com/svn/puppet/trunk@2328 980ebf18-57e1-0310-9a29-db15c13687c0

10 files changed, 300 insertions, 48 deletions

diff --git a/CHANGELOG b/CHANGELOG
index e8b7b810c..13fd07384 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,4 +1,8 @@
 0.22.2 (grover)
+    Users can now manage their home directories, using the managehome
+    parameter, partially using patches provided by Tim Stoop and
+    Matt Palmer. (#432)
+
     Added 'ralsh' (formerly x2puppet) to the svn tree.  When possible it
     should be added to the packages.
 
diff --git a/lib/puppet/parameter.rb b/lib/puppet/parameter.rb
index d34bc7a9e..91949e5e3 100644
--- a/lib/puppet/parameter.rb
+++ b/lib/puppet/parameter.rb
@@ -445,6 +445,12 @@ class Puppet::Parameter < Puppet::Element
         end
     end
 
+    # Retrieve the parent's provider.  Some types don't have providers, in which
+    # case we return the parent object itself.
+    def provider
+        @parent.provider || @parent
+    end
+
     # If there's a shadowing metaparam, instantiate it now.
     # This allows us to create a property or parameter with the
     # same name as a metaparameter, and the metaparam will only be
diff --git a/lib/puppet/provider/nameservice.rb b/lib/puppet/provider/nameservice.rb
index 377393973..3af5bfe35 100644
--- a/lib/puppet/provider/nameservice.rb
+++ b/lib/puppet/provider/nameservice.rb
@@ -84,6 +84,19 @@ class Puppet::Provider::NameService < Puppet::Provider
             return names
         end
 
+        def model=(model)
+            super
+            @model.validproperties.each do |prop|
+                next if prop == :ensure
+                unless public_method_defined?(prop)
+                    define_method(prop) { get(prop) || :absent}
+                end
+                unless public_method_defined?(prop.to_s + "+")
+                    define_method(prop.to_s + "=") { |*vals| set(prop, *vals) }
+                end
+            end
+        end
+
         # This is annoying, but there really aren't that many options,
         # and this *is* built into Ruby.
         def section
@@ -309,25 +322,6 @@ class Puppet::Provider::NameService < Puppet::Provider
         @objectinfo = nil
     end
 
-    # 
-    def method_missing(name, *args)
-        name = name.to_s
-
-        # Make sure it's a valid property.  We go up our class structure instead of
-        # our model's because the model is fake during testing.
-        unless self.class.model.validproperty?(name.sub("=",''))
-            raise Puppet::DevError, "%s is not a valid %s property" %
-                [name, @model.class.name]
-        end
-
-        # Each class has to override this manually
-        if name =~ /=/
-            set(name.to_s.sub("=", ''), *args)
-        else
-            return get(name.intern) || :absent
-        end
-    end
-
     def set(param, value)
         #self.class.validate(param, value)
         cmd = modifycmd(param, value)
diff --git a/lib/puppet/provider/user/useradd.rb b/lib/puppet/provider/user/useradd.rb
index 55cc2d5dd..8ed0a9bd1 100644
--- a/lib/puppet/provider/user/useradd.rb
+++ b/lib/puppet/provider/user/useradd.rb
@@ -17,6 +17,8 @@ Puppet::Type.type(:user).provide :useradd, :parent => Puppet::Provider::NameServ
         value !~ /\s/
     end
 
+    has_features :manages_homedir, :allows_duplicates
+
     def addcmd
         cmd = [command(:add)]
         @model.class.validproperties.each do |property|
@@ -33,10 +35,17 @@ Puppet::Type.type(:user).provide :useradd, :parent => Puppet::Provider::NameServ
             cmd << "-M"
         else
         end
-        if @model[:allowdupe]  == :true
+
+        if @model.allowdupe?
             cmd << "-o"
         end
 
+        if @model.managehome?
+            cmd << "-m"
+        elsif %w{Fedora RedHat}.include?(Facter.value("operatingsystem"))
+            cmd << "-M"
+        end
+
         cmd << @model[:name]
 
         cmd
diff --git a/lib/puppet/type/property.rb b/lib/puppet/type/property.rb
index 4d772191d..5c4babf38 100644
--- a/lib/puppet/type/property.rb
+++ b/lib/puppet/type/property.rb
@@ -313,12 +313,6 @@ class Property < Puppet::Parameter
         return tmp
     end
 
-    # Retrieve the parent's provider.  Some types don't have providers, in which
-    # case we return the parent object itself.
-    def provider
-        @parent.provider || @parent
-    end
-
     # By default, call the method associated with the property name on our
     # provider.  In other words, if the property name is 'gid', we'll call
     # 'provider.gid' to retrieve the current value.
diff --git a/lib/puppet/type/user.rb b/lib/puppet/type/user.rb
index ae62e577d..c4ca0d18a 100755
--- a/lib/puppet/type/user.rb
+++ b/lib/puppet/type/user.rb
@@ -4,6 +4,28 @@ require 'puppet/type/property'
 
 module Puppet
     newtype(:user) do
+        @doc = "Manage users.  This type is mostly built to manage system
+            users, so it is lacking some features useful for managing normal
+            users.
+            
+            This element type uses the prescribed native tools for creating
+            groups and generally uses POSIX APIs for retrieving information
+            about them.  It does not directly modify /etc/passwd or anything.
+            
+            For most platforms, the tools used are ``useradd`` and its ilk;
+            for Mac OS X, NetInfo is used.  This is currently unconfigurable,
+            but if you desperately need it to be so, please contact us."
+
+        feature :allows_duplicates,
+            "The provider supports duplicate users with the same UID."
+
+        feature :manages_homedir,
+            "The provider can create and remove home directories."
+
+        feature :manages_passwords,
+            "The provider can modify user passwords, by accepting a password
+            hash."
+
         newproperty(:ensure) do
             newvalue(:present, :event => :user_created) do
                 provider.create
@@ -77,9 +99,10 @@ module Puppet
                 being created, if no user ID is specified then one will be
                 chosen automatically, which will likely result in the same user
                 having different IDs on different systems, which is not
-                recommended.  This is especially noteworthy if you use Puppet to manage
-                the same user on both Darwin and other platforms, since Puppet does the
-                ID generation for you on Darwin, but the tools do so on other platforms."
+                recommended.  This is especially noteworthy if you use Puppet
+                to manage the same user on both Darwin and other platforms,
+                since Puppet does the ID generation for you on Darwin, but the
+                tools do so on other platforms."
 
             munge do |value|
                 case value
@@ -262,7 +285,7 @@ module Puppet
             defaultto :minimum
         end
 
-        newparam(:allowdupe) do
+        newparam(:allowdupe, :boolean => true) do
             desc "Whether to allow duplicate UIDs."
                 
             newvalues(:true, :false)
@@ -270,19 +293,21 @@ module Puppet
             defaultto false
         end
 
-        @doc = "Manage users.  Currently can create and modify users, but
-            cannot delete them.  Theoretically all of the parameters are
-            optional, but if no parameters are specified the comment will
-            be set to the user name in order to make the internals work out
-            correctly.
-            
-            This element type uses the prescribed native tools for creating
-            groups and generally uses POSIX APIs for retrieving information
-            about them.  It does not directly modify /etc/passwd or anything.
-            
-            For most platforms, the tools used are ``useradd`` and its ilk;
-            for Mac OS X, NetInfo is used.  This is currently unconfigurable,
-            but if you desperately need it to be so, please contact us."
+        newparam(:managehome, :boolean => true) do
+            desc "Whether to manage the home directory when managing the user."
+
+            newvalues(:true, :false)
+
+            defaultto false
+
+            validate do |val|
+                if val.to_s == "true"
+                    unless provider.class.manages_homedir?
+                        raise ArgumentError, "User provider %s can not manage home directories" % provider.class.name
+                    end
+                end
+            end
+        end
 
         # Autorequire the group, if it's around
         autorequire(:group) do
diff --git a/lib/puppet/util/provider_features.rb b/lib/puppet/util/provider_features.rb
index fbf2260dd..773f0ec98 100644
--- a/lib/puppet/util/provider_features.rb
+++ b/lib/puppet/util/provider_features.rb
@@ -12,11 +12,18 @@ module Puppet::Util::ProviderFeatures
 
         # Are all of the requirements met?
         def available?(obj)
-            if self.methods and ! methods_available?(obj)
+            if self.methods 
+                if methods_available?(obj)
+                    return true
+                else
+                    return false
+                end
+            else
+                # In this case, the provider has to declare support for this
+                # feature, and that's been checked before we ever get to the
+                # method checks.
                 return false
             end
-
-            true
         end
 
         def initialize(name, docs, hash)
@@ -44,7 +51,7 @@ module Puppet::Util::ProviderFeatures
     # Define one or more features.  At a minimum, features require a name
     # and docs, and at this point they should also specify a list of methods
     # required to determine if the feature is present.
-    def feature(name, docs, hash)
+    def feature(name, docs, hash = {})
         @features ||= {}
         if @features.include?(name)
             raise Puppet::DevError, "Feature %s is already defined" % name
diff --git a/test/ral/manager/provider.rb b/test/ral/manager/provider.rb
index a314506c5..3b727a4c5 100755
--- a/test/ral/manager/provider.rb
+++ b/test/ral/manager/provider.rb
@@ -138,6 +138,9 @@ class TestProviderFeatures < Test::Unit::TestCase
     end
 
     def test_has_feature
+        # Define a new feature that has no methods
+        @type.feature(:nomeths, "desc")
+
         # Define a provider with nothing
         provider = @type.provide(:nothing) {}
 
@@ -163,8 +166,14 @@ class TestProviderFeatures < Test::Unit::TestCase
             has_features :alpha
         end
 
+        # And a provider that declares it has our methodless feature.
+        @type.provide(:none) do
+            has_features :nomeths
+        end
+
         should = {:nothing => [], :both => [:numeric, :alpha],
-            :letters => [:alpha], :numbers => [:numeric]}
+            :letters => [:alpha], :numbers => [:numeric],
+            :none => [:nomeths]}
 
         should.each do |name, features|
             provider = @type.provider(name)
diff --git a/test/ral/providers/user/useradd.rb b/test/ral/providers/user/useradd.rb
new file mode 100755
index 000000000..1ababecb7
--- /dev/null
+++ b/test/ral/providers/user/useradd.rb
@@ -0,0 +1,179 @@
+#!/usr/bin/env ruby
+
+require File.dirname(__FILE__) + '/../../../lib/puppettest'
+
+require 'mocha'
+
+class UserAddProviderTest < PuppetTest::TestCase
+    confine "useradd user provider missing" =>
+        Puppet::Type.type(:user).provider(:useradd).suitable?
+
+	def setup
+        super
+		@type = Puppet::Type.type(:user)
+		@provider = Puppet::Type.type(:user).provider(:useradd)
+        @home = tempfile
+		@vals = {:name => 'faff',
+            :provider => :useradd,
+            :ensure => :present,
+            :uid => 5000,
+            :gid => 5000,
+            :home => @home,
+            :comment => "yayness",
+            :groups => %w{one two}
+        }
+	end
+
+    def setup_user
+		@user = @type.create(@vals)
+
+        @vals.each do |name, val|
+            next unless @user.class.validproperty?(name)
+            @user.is = [name, :absent]
+        end
+        @user
+    end
+
+    def test_features
+        [:manages_homedir].each do |feature|
+            assert(@provider.feature?(feature),
+                "useradd provider is missing %s" % feature)
+        end
+    end
+	
+	def test_create
+		user = setup_user
+
+        @vals.each do |name, val|
+            next unless user.class.validproperty?(name)
+            user.is = [name, :absent]
+        end
+
+        user.expects(:allowdupe?).returns(false)
+        user.expects(:managehome?).returns(false)
+
+		user.provider.expects(:execute).with do |params|
+            command = params.shift
+            assert_equal(@provider.command(:add), command,
+                "Got incorrect command")
+
+            if %w{Fedora RedHat}.include?(Facter.value(:operatingsystem))
+                assert(params.include?("-M"),
+                    "Did not disable homedir creation on red hat")
+                params.delete("-M")
+            end
+
+            options = {}
+            while params.length > 0
+                options[params.shift] = params.shift
+            end
+
+            @vals[:groups] = @vals[:groups].join(",")
+
+            flags = {:home => "-d", :groups => "-G", :gid => "-g",
+                :uid => "-u", :comment => "-c"}
+
+            flags.each do |param, flag|
+                assert_equal(@vals[param], options[flag],
+                    "Got incorrect value for %s" % param)
+            end
+
+            true
+        end
+		
+        user.provider.create
+	end
+
+    # Make sure we add the right flags when managing home
+    def test_managehome
+        @vals[:managehome] = true
+        setup_user
+
+        assert(@user.provider.respond_to?(:manages_homedir?),
+            "provider did not get managehome test set")
+
+        assert(@user.managehome?, "provider did not get managehome")
+
+        # First run
+        @user.expects(:managehome?).returns(true)
+
+		@user.provider.expects(:execute).with do |params|
+            assert_equal(params[0], @provider.command(:add),
+                "useradd was not called")
+            assert(params.include?("-m"),
+                "Did not add -m when managehome was in affect")
+
+            true
+        end
+
+        @user.provider.create
+        @user.class.clear
+
+        # Start again, this time with manages_home off
+        @vals[:managehome] = false
+        setup_user
+
+        # First run
+        @user.expects(:managehome?).returns(false)
+
+		@user.provider.expects(:execute).with do |params|
+            assert_equal(params[0], @provider.command(:add),
+                "useradd was not called")
+            if %w{Fedora RedHat}.include?(Facter.value(:operatingsystem))
+                assert(params.include?("-M"),
+                    "Did not add -M on Red Hat")
+            end
+                assert(! params.include?("-m"),
+                    "Added -m when managehome was disabled")
+
+            true
+        end
+
+        @user.provider.create
+    end
+
+    def test_allowdupe
+        @vals[:allowdupe] = true
+        setup_user
+
+        assert(@user.provider.respond_to?(:manages_homedir?),
+            "provider did not get allowdupe test set")
+
+        assert(@user.allowdupe?, "provider did not get allowdupe")
+
+        # First run
+        @user.expects(:allowdupe?).returns(true)
+
+		@user.provider.expects(:execute).with do |params|
+            assert_equal(params[0], @provider.command(:add),
+                "useradd was not called")
+            assert(params.include?("-o"),
+                "Did not add -o when allowdupe was in affect")
+
+            true
+        end
+
+        @user.provider.create
+        @user.class.clear
+
+        # Start again, this time with manages_home off
+        @vals[:allowdupe] = false
+        setup_user
+
+        # First run
+        @user.expects(:allowdupe?).returns(false)
+
+		@user.provider.expects(:execute).with do |params|
+            assert_equal(params[0], @provider.command(:add),
+                "useradd was not called")
+            assert(! params.include?("-o"),
+                "Added -o when allowdupe was disabled")
+
+            true
+        end
+
+        @user.provider.create
+    end
+end
+
+# $Id$
diff --git a/test/ral/types/user.rb b/test/ral/types/user.rb
index 8af6b5d48..be1dd6f78 100755
--- a/test/ral/types/user.rb
+++ b/test/ral/types/user.rb
@@ -463,6 +463,31 @@ class TestUser < Test::Unit::TestCase
         assert(user.send(:property, :groups).insync?,
             "Groups state considered out of sync with no :should value")
     end
+
+    # Make sure the 'managehome' param can only be set when the provider
+    # has that feature.  Uses a patch from #432.
+    def test_managehome
+        user = Puppet::Type.type(:user).create(:name => "yaytest", :check => :all)
+
+        prov = user.provider
+
+        home = false
+        prov.class.meta_def(:manages_homedir?) { home }
+
+        assert_nothing_raised("failed on false managehome") do
+            user[:managehome] = false
+        end
+
+        assert_raise(ArgumentError, "did not fail when managehome? is false") do
+            user[:managehome] = true
+        end
+
+        home = true
+        assert(prov.class.manages_homedir?, "provider did not enable homedir")
+        assert_nothing_raised("failed when managehome is true") do
+            user[:managehome] = true
+        end
+    end
 end
 
 # $Id$