clean up AIX provider

Signed-off-by: Lamont Granquist <lamont@scriptkiddie.org>

3 files changed, 37 insertions, 55 deletions

diff --git a/lib/chef/provider/user/aix.rb b/lib/chef/provider/user/aix.rb
index aac21bc87a..351d603c93 100644
--- a/lib/chef/provider/user/aix.rb
+++ b/lib/chef/provider/user/aix.rb
@@ -23,42 +23,48 @@ class Chef
         provides :user, os: "aix"
         provides :aix_user
 
-        UNIVERSAL_OPTIONS = [[:comment, "-c"], [:gid, "-g"], [:shell, "-s"], [:uid, "-u"]].freeze
-
         def create_user
-          command = compile_command("useradd") do |useradd|
-            useradd.concat(universal_options)
-            useradd.concat(useradd_options)
-          end
-          shell_out_compact!(command)
+          shell_out_compact!("useradd", universal_options, useradd_options, new_resource.username)
           add_password
         end
 
         def manage_user
           add_password
           manage_home
-          return if universal_options.empty?
-          command = compile_command("usermod") do |u|
-            u.concat(universal_options)
-          end
-          shell_out_compact!(command)
+          return if universal_options.empty? && usermod_options.empty?
+          shell_out_compact!("usermod", universal_options, usermod_options, new_resource.username)
         end
 
         def remove_user
-          command = [ "userdel" ]
-          command << "-r" if new_resource.manage_home
-          command << "-f" if new_resource.force
-          command << new_resource.username
-          shell_out_compact!(command)
+          shell_out_compact!("userdel", userdel_options, new_resource.username)
         end
 
         # Aix does not support -r like other unix, sytem account is created by adding to 'system' group
         def useradd_options
           opts = []
           opts << "-g" << "system" if new_resource.system
+          if updating_home?
+            if new_resource.manage_home
+              logger.trace("#{new_resource} managing the users home directory")
+              opts << "-m"
+            else
+              logger.trace("#{new_resource} setting home to #{new_resource.home}")
+            end
+          end
           opts
         end
 
+        def userdel_options
+          opts = []
+          opts << "-r" if new_resource.manage_home
+          opts << "-f" if new_resource.force
+          opts
+        end
+
+        def usermod_options
+          []
+        end
+
         def check_lock
           lock_info = shell_out_compact!("lsuser", "-a", "account_locked", new_resource.username)
           if whyrun_mode? && passwd_s.stdout.empty? && lock_info.stderr.match(/does not exist/)
@@ -86,47 +92,23 @@ class Chef
           shell_out_compact!("chuser", "account_locked=false", new_resource.username)
         end
 
-        def compile_command(base_command)
-          base_command = Array(base_command)
-          yield base_command
-          base_command << new_resource.username
-          base_command
-        end
-
         def universal_options
-          @universal_options ||=
-            begin
-              opts = []
-              # magic allows UNIVERSAL_OPTIONS to be overridden in a subclass
-              self.class::UNIVERSAL_OPTIONS.each do |field, option|
-                update_options(field, option, opts)
-              end
-              if updating_home?
-                opts << "-d" << new_resource.home
-                if new_resource.manage_home
-                  logger.trace("#{new_resource} managing the users home directory")
-                  opts << "-m"
-                else
-                  logger.trace("#{new_resource} setting home to #{new_resource.home}")
-                end
-              end
-              opts << "-o" if new_resource.non_unique
-              opts
-            end
+          opts = []
+          opts << "-c" << new_resource.comment if should_set?(:comment)
+          opts << "-g" << new_resource.gid if should_set?(:gid)
+          opts << "-s" << new_resource.shell if should_set?(:shell)
+          opts << "-u" << new_resource.uid if should_set?(:uid)
+          opts << "-d" << new_resource.home if updating_home?
+          opts << "-o" if new_resource.non_unique
+          opts
         end
 
-        def update_options(field, option, opts)
-          return unless current_resource.send(field).to_s != new_resource.send(field).to_s
-          return unless new_resource.send(field)
-          logger.trace("#{new_resource} setting #{field} to #{new_resource.send(field)}")
-          opts << option << new_resource.send(field).to_s
+        def should_set?(sym)
+          current_resource.send(sym).to_s != new_resource.send(sym).to_s && new_resource.send(sym)
         end
 
+        # FIXME: move to superclass, and one of these implements must be buggy?
         def updating_home?
-          # will return false if paths are equivalent
-          # Pathname#cleanpath does a better job than ::File::expand_path (on both unix and windows)
-          # ::File.expand_path("///tmp") == ::File.expand_path("/tmp") => false
-          # ::File.expand_path("\\tmp") => "C:/tmp"
           return true if current_resource.home.nil? && new_resource.home
           new_resource.home && Pathname.new(current_resource.home).cleanpath != Pathname.new(new_resource.home).cleanpath
         end
@@ -144,7 +126,6 @@ class Chef
         def manage_home
           return unless updating_home? && new_resource.manage_home
           # -m option does not work on aix, so move dir.
-          universal_options.delete("-m")
           if ::File.directory?(current_resource.home)
             logger.trace("Changing users home directory from #{current_resource.home} to #{new_resource.home}")
             FileUtils.mv current_resource.home, new_resource.home
diff --git a/lib/chef/provider/user/linux.rb b/lib/chef/provider/user/linux.rb
index 2db6c218bd..07df198ae2 100644
--- a/lib/chef/provider/user/linux.rb
+++ b/lib/chef/provider/user/linux.rb
@@ -1,5 +1,5 @@
 #
-# Copyright:: Copyright 2016-2017, Chef Software Inc.
+# Copyright:: Copyright 2016-2018, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -89,6 +89,7 @@ class Chef
           current_resource.send(sym).to_s != new_resource.send(sym).to_s && new_resource.send(sym)
         end
 
+        # FIXME: move to superclass, and one of these implementations must be buggy?
         def updating_home?
           return false unless new_resource.home
           return true unless current_resource.home
diff --git a/lib/chef/provider/user/solaris.rb b/lib/chef/provider/user/solaris.rb
index 87a2ee953f..494b10237a 100644
--- a/lib/chef/provider/user/solaris.rb
+++ b/lib/chef/provider/user/solaris.rb
@@ -71,9 +71,9 @@ class Chef
           opts << "-g" << new_resource.gid if should_set?(:gid)
           opts << "-s" << new_resource.shell if should_set?(:shell)
           opts << "-u" << new_resource.uid if should_set?(:uid)
+          opts << "-d" << new_resource.home if updating_home?
           opts << "-o" if new_resource.non_unique
           if updating_home?
-            opts << "-d" << new_resource.home
             if new_resource.manage_home
               logger.trace("#{new_resource} managing the users home directory")
               opts << "-m"