Refactor rabbitmq_user provider (mpolenchuk)

lib/puppet/provider/rabbitmq_user/rabbitmqctl.rb

@@ -1,78 +1,92 @@
-require 'puppet'
-require 'set'
 require File.expand_path(File.join(File.dirname(__FILE__), '..', 'rabbitmqctl'))
-Puppet::Type.type(:rabbitmq_user).provide(:rabbitmqctl, parent: Puppet::Provider::Rabbitmqctl) do
-  if Puppet::PUPPETVERSION.to_f < 3
-    commands rabbitmqctl: 'rabbitmqctl'
-  else
-    has_command(:rabbitmqctl, 'rabbitmqctl') do
-      environment HOME: '/tmp'
-    end
+Puppet::Type.type(:rabbitmq_user).provide(
+  :rabbitmqctl,
+  parent: Puppet::Provider::Rabbitmqctl
+) do
+  has_command(:rabbitmqctl, 'rabbitmqctl') do
+    environment HOME: '/tmp'
   end
 
   defaultfor feature: :posix
 
+  def initialize(value = {})
+    super(value)
+    @property_flush = {}
+  end
+
   def self.instances
     user_list = run_with_retries do
       rabbitmqctl('-q', 'list_users')
     end
 
     user_list.split(%r{\n}).map do |line|
-      raise Puppet::Error, "Cannot parse invalid user line: #{line}" unless line =~ %r{^(\S+)(\s+\[.*?\]|)$}
-      new(name: Regexp.last_match(1))
+      raise Puppet::Error, "Cannot parse invalid user line: #{line}" unless line =~ %r{^(\S+)\s+\[(.*?)\]$}
+      user = Regexp.last_match(1)
+      tags = Regexp.last_match(2).split(%r{,\s*})
+      new(
+        ensure: :present,
+        name: user,
+        tags: tags
+      )
     end
   end
 
+  def self.prefetch(resources)
+    users = instances
+    resources.each_key do |user|
+      if (provider = users.find { |u| u.name == user })
+        resources[user].provider = provider
+      end
+    end
+  end
+
+  def exists?
+    @property_hash[:ensure] == :present
+  end
+
   def create
+    # Fail here (rather than a validate block in the type) if password is not
+    # set, so that "puppet resource" still works.
     raise Puppet::Error, "Password is a required parameter for rabbitmq_user (user: #{name})" if @resource[:password].nil?
 
-    rabbitmqctl('add_user', resource[:name], resource[:password])
-    make_user_admin if resource[:admin] == :true
-    set_user_tags(resource[:tags]) unless resource[:tags].empty?
-  end
+    rabbitmqctl('add_user', @resource[:name], @resource[:password])
 
-  def change_password
-    rabbitmqctl('change_password', resource[:name], resource[:password])
-  end
+    tags = @resource[:tags]
+    tags << admin_tag if @resource[:admin] == :true
+    rabbitmqctl('set_user_tags', @resource[:name], tags) unless tags.empty?
 
-  def password
-    nil
+    @property_hash[:ensure] = :present
   end
 
-  def check_password
-    response = self.class.run_with_retries do
-      rabbitmqctl('eval', 'rabbit_access_control:check_user_pass_login(list_to_binary("' + resource[:name] + '"), list_to_binary("' + resource[:password] + '")).')
-    end
-    if response.include? 'refused'
-      false
-    else
-      true
-    end
+  def destroy
+    rabbitmqctl('delete_user', @resource[:name])
+    @property_hash[:ensure] = :absent
   end
 
-  def destroy
-    rabbitmqctl('delete_user', resource[:name])
+  def password=(password)
+    rabbitmqctl('change_password', @resource[:name], password)
   end
 
-  def exists?
-    user_list = self.class.run_with_retries do
-      rabbitmqctl('-q', 'list_users')
-    end
+  def password; end
 
-    user_list.split(%r{\n}).find do |line|
-      line.match(%r{^#{Regexp.escape(resource[:name])}(\s+(\[.*?\]|\S+)|)$})
-    end
+  def check_password(password)
+    check_access_control = [
+      'rabbit_access_control:check_user_pass_login(',
+      %[list_to_binary("#{@resource[:name]}"), ],
+      %[list_to_binary("#{password}")).]
+    ]
+
+    response = rabbitmqctl('eval', check_access_control.join)
+    !response.include? 'refused'
   end
 
   def tags
-    tags = get_user_tags
     # do not expose the administrator tag for admins
-    tags.delete('administrator') if resource[:admin] == :true
-    tags.entries.sort
+    @property_hash[:tags].reject { |tag| tag == admin_tag }
   end
 
   def tags=(tags)
-    set_user_tags(tags) unless tags.nil?
+    @property_flush[:tags] = tags
   end
 
   def admin
@@ -91,26 +105,25 @@ def admin=(state)
     end
   end
 
-  def set_user_tags(tags) # rubocop:disable Style/AccessorMethodName
-    is_admin = get_user_tags.member?('administrator') \
-               || resource[:admin] == :true
-    usertags = Set.new(tags)
-    usertags.add('administrator') if is_admin
-    rabbitmqctl('set_user_tags', resource[:name], usertags.entries.sort)
+  def admin
+    @property_hash[:tags].include?(admin_tag) ? :true : :false
   end
 
-  def make_user_admin
-    usertags = get_user_tags
-    usertags.add('administrator')
-    rabbitmqctl('set_user_tags', resource[:name], usertags.entries.sort)
+  def admin=(state)
+    @property_flush[:admin] = state
+  end
+
+  def flush
+    return if @property_flush.empty?
+    tags = @property_flush[:tags] || @resource[:tags]
+    tags << admin_tag if @resource[:admin] == :true
+    rabbitmqctl('set_user_tags', @resource[:name], tags)
+    @property_flush.clear
   end
 
   private
 
-  def get_user_tags # rubocop:disable Style/AccessorMethodName
-    match = rabbitmqctl('-q', 'list_users').split(%r{\n}).map do |line|
-      line.match(%r{^#{Regexp.escape(resource[:name])}\s+\[(.*?)\]})
-    end.compact.first
-    Set.new(match[1].split(' ').map { |x| x.gsub(%r{,$}, '') }) if match
+  def admin_tag
+    'administrator'
   end
 end

lib/puppet/type/rabbitmq_user.rb

@@ -40,20 +40,12 @@
   newproperty(:password) do
     desc 'User password to be set *on creation* and validated each run'
     def insync?(_is)
-      provider.check_password
-    end
-
-    def set(_value)
-      provider.change_password
+      provider.check_password(should)
     end
 
     def change_to_s(_current, _desired)
       'password has been changed'
     end
-
-    def should_to_s(_newvalue = @should)
-      '<new password>'
-    end
   end
 
   newproperty(:admin) do
@@ -80,23 +72,11 @@ def should_to_s(_newvalue = @should)
     defaultto []
 
     def insync?(is)
-      is_to_s(is) == should_to_s
+      is.sort == should.sort
     end
 
-    def is_to_s(currentvalue = @is) # rubocop:disable Style/PredicateName
-      if currentvalue
-        "[#{currentvalue.sort.join(', ')}]"
-      else
-        '[]'
-      end
-    end
-
-    def should_to_s(newvalue = @should)
-      if newvalue
-        "[#{newvalue.sort.join(', ')}]"
-      else
-        '[]'
-      end
+    def should_to_s(value)
+      Array(value)
     end
   end
 end

spec/acceptance/user_spec.rb

@@ -27,11 +27,30 @@ class { '::rabbitmq':
 
     # rubocop:disable RSpec/MultipleExpectations
     it 'has the user' do
-      shell('rabbitmqctl list_users') do |r|
+      shell('rabbitmqctl list_users -q') do |r|
         expect(r.stdout).to match(%r{dan.*administrator})
         expect(r.exit_code).to be_zero
       end
     end
     # rubocop:enable RSpec/MultipleExpectations
   end
+
+  context 'destroy user resource' do
+    it 'runs successfully' do
+      pp = <<-EOS
+      rabbitmq_user { 'dan':
+        ensure => absent,
+      }
+      EOS
+
+      apply_manifest(pp, catch_failures: true)
+      apply_manifest(pp, catch_changes: true)
+    end
+
+    it 'does not have the user' do
+      shell('rabbitmqctl list_users -q') do |r|
+        expect(r.stdout).not_to match(%r{dan\s+})
+      end
+    end
+  end
 end