[Pkg-puppet-devel] [SCM] Puppet packaging for Debian branch, master, updated. debian/0.24.6-1-356-g5718585

James Turnbull james at lovedthanlost.net
Fri Jan 23 14:21:42 UTC 2009


The following commit has been merged in the master branch:
commit e9f858a1f9108c523efb3856e3ce46e3f9615646
Author: Luke Kanies <luke at madstop.com>
Date:   Thu Nov 20 12:51:42 2008 -0600

    Refactoring the file/owner property to be simpler and cleaner.
    
    It now looks almost exactly like the group property, and has a
    much more current data flow (e.g., sync is never no-op,
    and the sync method doesn't check whether the file is present).
    
    Signed-off-by: Luke Kanies <luke at madstop.com>

diff --git a/lib/puppet/type/file/owner.rb b/lib/puppet/type/file/owner.rb
index 6f9bbd6..6bc40ec 100755
--- a/lib/puppet/type/file/owner.rb
+++ b/lib/puppet/type/file/owner.rb
@@ -1,5 +1,8 @@
 module Puppet
     Puppet.type(:file).newproperty(:owner) do
+        include Puppet::Util::POSIX
+        include Puppet::Util::Warnings
+
         require 'etc'
         desc "To whom the file should belong.  Argument can be user name or
             user ID."
@@ -24,40 +27,32 @@ module Puppet
             end
         end
 
-        def name2id(value)
-            if value.is_a?(Symbol)
-                return value.to_s
+        def insync?(current)
+            unless Puppet::Util::SUIDManager.uid == 0
+                warning "Cannot manage ownership unless running as root"
+                return true
             end
-            begin
-                user = Etc.getpwnam(value)
-                if user.uid == ""
-                    return nil
+
+            @should.each do |value|
+                if value =~ /^\d+$/
+                    uid = Integer(value)
+                elsif value.is_a?(String)
+                    fail "Could not find user %s" % value unless uid = uid(value)
+                else
+                    uid = value
                 end
-                return user.uid
-            rescue ArgumentError => detail
-                return nil
+
+                return true if uid == current
             end
+            return false
         end
 
         # Determine if the user is valid, and if so, return the UID
         def validuser?(value)
-            if value =~ /^\d+$/
-                value = value.to_i
-            end
-
-            if value.is_a?(Integer)
-                # verify the user is a valid user
-                if tmp = id2name(value)
-                    return value
-                else
-                    return false
-                end
+            if number = uid(value)
+                return number
             else
-                if tmp = name2id(value)
-                    return tmp
-                else
-                    return false
-                end
+                return false
             end
         end
 
@@ -99,13 +94,6 @@ module Puppet
                 return :absent
             end
 
-            # Set our method appropriately, depending on links.
-            if stat.ftype == "link" and @resource[:links] != :follow
-                @method = :lchown
-            else
-                @method = :chown
-            end
-
             currentvalue = stat.uid
             
             # On OS X, files that are owned by -2 get returned as really
@@ -120,44 +108,24 @@ module Puppet
         end
 
         def sync
-            unless Puppet::Util::SUIDManager.uid == 0
-                unless defined? @@notifieduid
-                    self.notice "Cannot manage ownership unless running as root"
-                    #@resource.delete(self.name)
-                    @@notifieduid = true
-                end
-                return nil
+            # Set our method appropriately, depending on links.
+            if resource[:links] == :manage
+                method = :lchown
+            else
+                method = :chown
             end
 
-            user = nil
-            unless user = self.validuser?(self.should)
-                tmp = self.should
-                unless defined? @@usermissing
-                    @@usermissing = {}
-                end
-
-                if @@usermissing.include?(tmp)
-                    @@usermissing[tmp] += 1
-                else
-                    self.notice "user %s does not exist" % tmp
-                    @@usermissing[tmp] = 1
-                end
-                return nil
+            uid = nil
+            @should.each do |user|
+                break if uid = validuser?(user)
             end
 
-            unless @resource.stat(false)
-                unless @resource.stat(true)
-                    self.debug "File does not exist; cannot set owner"
-                    return nil
-                end
-                #self.debug "%s: after refresh, is '%s'" % [self.class.name, at is]
-            end
+            raise Puppet::Error, "Could not find user(s) %s" % @should.join(",") unless uid
 
             begin
-                File.send(@method, user, nil, @resource[:path])
+                File.send(method, uid, nil, @resource[:path])
             rescue => detail
-                raise Puppet::Error, "Failed to set owner to '%s': %s" %
-                    [user, detail]
+                raise Puppet::Error, "Failed to set owner to '%s': %s" % [uid, detail]
             end
 
             return :file_changed
diff --git a/spec/unit/type/file/owner.rb b/spec/unit/type/file/owner.rb
new file mode 100755
index 0000000..743e640
--- /dev/null
+++ b/spec/unit/type/file/owner.rb
@@ -0,0 +1,132 @@
+#!/usr/bin/env ruby
+
+Dir.chdir(File.dirname(__FILE__)) { (s = lambda { |f| File.exist?(f) ? require(f) : Dir.chdir("..") { s.call(f) } }).call("spec/spec_helper.rb") }
+
+property = Puppet::Type.type(:file).attrclass(:owner)
+
+describe property do
+    before do
+        @resource = stub 'resource', :line => "foo", :file => "bar"
+        @resource.stubs(:[]).returns "foo"
+        @resource.stubs(:[]).with(:path).returns "/my/file"
+        @owner = property.new :resource => @resource
+    end
+
+    it "should have a method for testing whether an owner is valid" do
+        @owner.must respond_to(:validuser?)
+    end
+
+    it "should return the found uid if an owner is valid" do
+        @owner.expects(:uid).with("foo").returns 500
+        @owner.validuser?("foo").should == 500
+    end
+
+    it "should return false if an owner is not valid" do
+        @owner.expects(:uid).with("foo").returns nil
+        @owner.validuser?("foo").should be_false
+    end
+
+    describe "when retrieving the current value" do
+        it "should return :absent if the file cannot stat" do
+            @resource.expects(:stat).returns nil
+
+            @owner.retrieve.should == :absent
+        end
+
+        it "should get the uid from the stat instance from the file" do
+            stat = stub 'stat', :ftype => "foo"
+            @resource.expects(:stat).returns stat
+            stat.expects(:uid).returns 500
+
+            @owner.retrieve.should == 500
+        end
+
+        it "should warn and return :silly if the found value is higher than the maximum uid value" do
+            Puppet.settings.expects(:value).with(:maximum_uid).returns 500
+
+            stat = stub 'stat', :ftype => "foo"
+            @resource.expects(:stat).returns stat
+            stat.expects(:uid).returns 1000
+
+            @owner.expects(:warning)
+            @owner.retrieve.should == :silly
+        end
+    end
+
+    describe "when determining if the file is in sync" do
+        describe "and not running as root" do
+            it "should warn and return true" do
+                @owner.should = 10
+                Puppet::Util::SUIDManager.expects(:uid).returns 1
+                @owner.expects(:warning)
+                @owner.must be_insync("whatever")
+            end
+        end
+
+        before do
+            Puppet::Util::SUIDManager.stubs(:uid).returns 0
+        end
+
+        it "should directly compare the owner values if the desired owner is an integer" do
+            @owner.should = [10]
+            @owner.must be_insync(10)
+        end
+
+        it "should treat numeric strings as integers" do
+            @owner.should = ["10"]
+            @owner.must be_insync(10)
+        end
+
+        it "should convert the owner name to an integer if the desired owner is a string" do
+            @owner.expects(:uid).with("foo").returns 10
+            @owner.should = %w{foo}
+
+            @owner.must be_insync(10)
+        end
+
+        it "should fail if it cannot convert an owner name to an integer" do
+            @owner.expects(:uid).with("foo").returns nil
+            @owner.should = %w{foo}
+
+            lambda { @owner.insync?(10) }.should raise_error(Puppet::Error)
+        end
+
+        it "should return false if the owners are not equal" do
+            @owner.should = [10]
+            @owner.should_not be_insync(20)
+        end
+    end
+
+    describe "when changing the owner" do
+        before do
+            @owner.should = %w{one}
+            @owner.stubs(:path).returns "path"
+            @owner.stubs(:uid).returns 500
+        end
+
+        it "should chown the file if :links is set to :follow" do
+            @resource.expects(:[]).with(:links).returns :follow
+            File.expects(:chown)
+
+            @owner.sync
+        end
+
+        it "should lchown the file if :links is set to :manage" do
+            @resource.expects(:[]).with(:links).returns :manage
+            File.expects(:lchown)
+
+            @owner.sync
+        end
+
+        it "should use the first valid owner in its 'should' list" do
+            @owner.should = %w{one two three}
+            @owner.expects(:validuser?).with("one").returns nil
+            @owner.expects(:validuser?).with("two").returns 500
+            @owner.expects(:validuser?).with("three").never
+
+            File.expects(:chown).with(500, nil, "/my/file")
+
+            @owner.sync
+        end
+    end
+end

-- 
Puppet packaging for Debian



More information about the Pkg-puppet-devel mailing list