[Puppet-dev] (puppetlabs/puppet) New pull request: (#9459) Fix problems with Windows 'user' and 'group' providers.

[wearetherobots]

Greetings!

Please review the pull request #116: (#9459) Fix problems with Windows 'user' and 'group' providers. opened by (ChaseMotorman)

Some more information about the pull request:

• Opened: Wed Sep 14 22:41:15 UTC 2011
• Based on: puppetlabs:2.7.x (d54410446efc8cf1d436886b161083a534f705b2)
• Requested merge: ChaseMotorman:ticket/2.7.x/9459-user-group-failures (a282cc788bb1b73fa284a903dedbb7ad3026fc02)

Description:

This commit corrects several problems with the Windows 'user' and
'group' providers, and Puppet::Util::ADSI helper class.

The 'user' provider failed to add the username to the set of groups
specified in the 'groups' property when creating a new user, due to
the provider trying to enumerate a user's group membership before the
underlying ADSI user object was saved. Any group referenced in the
property must exist prior to creating the resource.

The 'group' provider failed to save a newly-created resource,
due to a missing 'flush' method, which in turn calls the
'Puppet::Util::ADSI.commit' (save) method.

Windows does not allow user and groups to share the same name,
and when attempted the ADSI connection would throw a misleading
exception referring to an 'invalid moniker'. The 'user' provider will
now raise an error if it attempts to create a resource when a like-named
group already exists, and vice-versa.

Thanks!
The Pull Request Bot

Diff follows:

diff --git a/lib/puppet/provider/group/windows_adsi.rb b/lib/puppet/provider/group/windows_adsi.rb
index 4468d00..2ca8522 100644
--- a/lib/puppet/provider/group/windows_adsi.rb
+++ b/lib/puppet/provider/group/windows_adsi.rb
@@ -34,6 +34,11 @@ Puppet::Type.type(:group).provide :windows_adsi do
     Puppet::Util::ADSI::Group.delete(@resource[:name])
   end
 
+  # Only flush if we created or modified a group, not deleted
+  def flush
+    @group.commit if @group
+  end
+
   def gid
     nil
   end
diff --git a/lib/puppet/provider/user/windows_adsi.rb b/lib/puppet/provider/user/windows_adsi.rb
index 9250def..fa6f0b5 100644
--- a/lib/puppet/provider/user/windows_adsi.rb
+++ b/lib/puppet/provider/user/windows_adsi.rb
@@ -23,6 +23,7 @@ Puppet::Type.type(:user).provide :windows_adsi do
 
   def create
     @user = Puppet::Util::ADSI::User.create(@resource[:name])
+    @user.commit if @user #9459: ensure the state is set before enumerating groups
     [:comment, :home, :groups].each do |prop|
       send("#{prop}=", @resource[prop]) if @resource[prop]
     end
diff --git a/lib/puppet/util/adsi.rb b/lib/puppet/util/adsi.rb
index f865743..dfa8a04 100644
--- a/lib/puppet/util/adsi.rb
+++ b/lib/puppet/util/adsi.rb
@@ -127,7 +127,7 @@ module Puppet::Util::ADSI
     def groups
       # WIN32OLE objects aren't enumerable, so no map
       groups = []
-      native_user.Groups.each {|g| groups << g.Name}
+      native_user.Groups.each {|g| groups << g.Name} rescue nil
       groups
     end
 
@@ -163,6 +163,8 @@ module Puppet::Util::ADSI
     end
 
     def self.create(name)
+      # Windows error 1379: The specified local group already exists.
+      raise Puppet::Error.new( "Cannot create user if group '#{name}' exists." ) if Puppet::Util::ADSI::Group.exists? name
       new(name, Puppet::Util::ADSI.create(name, 'user'))
     end
 
@@ -253,6 +255,8 @@ module Puppet::Util::ADSI
     end
 
     def self.create(name)
+      # Windows error 2224: The account already exists.
+      raise Puppet::Error.new( "Cannot create group if user '#{name}' exists." ) if Puppet::Util::ADSI::User.exists? name
       new(name, Puppet::Util::ADSI.create(name, 'group'))
     end
 

    

--
You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to puppet-dev@googlegroups.com.
To unsubscribe from this group, send email to puppet-dev+unsubscr...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.