administrator/admin and tag related fixes

While working with this puppet module, I observed a few glitches:

- setting admin to true and tags to an empty list, a puppet run would
  either attempt to set the user tags to an empty list, or adding the
  adminstrator tag again. Therefore, do not expose the administrator
  tag through tags if admin is true. Further, do not allow setting
  the administrator explicitly in tags.

- providing tags that are not sorted to rabbitmq::user would make
  puppet try to apply the non sorted list over and over again. Also,
  default to the empty array for tags. This will remove existing tags
  from users if they weren't set through puppet.

- provide nicer output when changing tags using tags.join(' ,')

lib/puppet/provider/rabbitmq_user/rabbitmqctl.rb

@@ -30,7 +30,7 @@ def create
     if resource[:admin] == :true
       make_user_admin()
     end
-    if !resource[:tags].nil?
+    if ! resource[:tags].empty?
       set_user_tags(resource[:tags])
     end
   end
@@ -67,7 +67,12 @@ def exists?
 
 
   def tags
-    get_user_tags.entries.sort
+    tags = get_user_tags
+    # do not expose the administrator tag for admins
+    if resource[:admin] == :true
+      tags.delete('administrator')
+    end
+    tags.entries.sort
   end
 
 

lib/puppet/type/rabbitmq_user.rb

@@ -32,17 +32,48 @@ def change_to_s(current, desired)
   end
 
   newproperty(:admin) do
-    desc 'rather or not user should be an admin'
+    desc 'whether or not user should be an admin'
     newvalues(/true|false/)
     munge do |value|
-      # converting to_s incase its a boolean
+      # converting to_s in case its a boolean
       value.to_s.to_sym
     end
     defaultto :false
   end
 
   newproperty(:tags, :array_matching => :all) do
     desc 'additional tags for the user'
+    validate do |value|
+      unless value =~ /^\S+$/
+        raise ArgumentError, "Invalid tag: #{value.inspect}"
+      end
+
+      if value == "administrator"
+        raise ArgumentError, "must use admin property instead of administrator tag"
+      end
+    end
+    defaultto []
+
+    def insync?(is)
+      self.is_to_s(is) == self.should_to_s
+    end
+
+    def is_to_s(currentvalue = @is)
+      if currentvalue
+        "[#{currentvalue.sort.join(', ')}]"
+      else
+        '[]'
+      end
+    end
+
+    def should_to_s(newvalue = @should)
+      if newvalue
+        "[#{newvalue.sort.join(', ')}]"
+      else
+        '[]'
+      end
+    end
+
   end
 
   validate do

spec/unit/puppet/provider/rabbitmq_user/rabbitmqctl_spec.rb

@@ -194,4 +194,22 @@
     @provider.create
   end
 
+  it 'should not return the administrator tag in tags for admins' do
+    @resource[:tags] = []
+    @resource[:admin]  = true
+    @provider.expects(:rabbitmqctl).with('-q', 'list_users').returns <<-EOT
+foo [administrator]
+EOT
+    @provider.tags.should == []
+  end
+
+  it 'should return the administrator tag for non-admins' do
+    # this should not happen though.
+    @resource[:tags] = []
+    @resource[:admin]  = :false
+    @provider.expects(:rabbitmqctl).with('-q', 'list_users').returns <<-EOT
+foo [administrator]
+EOT
+    @provider.tags.should == ["administrator"]
+  end
 end

spec/unit/puppet/type/rabbitmq_user_spec.rb

@@ -39,4 +39,14 @@
       @user[:admin] = 'yes'
     }.to raise_error(Puppet::Error, /Invalid value/)
   end
+  it 'should not accept tags with spaces' do
+    expect {
+      @user[:tags] = ['policy maker']
+    }.to raise_error(Puppet::Error, /Invalid tag/)
+  end
+  it 'should not accept the administrator tag' do
+    expect {
+      @user[:tags] = ['administrator']
+    }.to raise_error(Puppet::Error, /must use admin property/)
+  end
 end