Merge pull request #11224 from MsysTechnologiesllc/smriti/10948_knife_user_create_update_posistional_arguments

Signed-off-by: Tim Smith <tsmith@chef.io>

4 files changed, 103 insertions, 27 deletions

diff --git a/knife/lib/chef/knife/user_create.rb b/knife/lib/chef/knife/user_create.rb
index ae1f81628c..bbba882102 100644
--- a/knife/lib/chef/knife/user_create.rb
+++ b/knife/lib/chef/knife/user_create.rb
@@ -54,7 +54,23 @@ class Chef
         short: "-p",
         description: "Prompt for user password"
 
-      banner "knife user create USERNAME DISPLAY_NAME FIRST_NAME LAST_NAME EMAIL PASSWORD (options)"
+      option :first_name,
+        long: "--first-name FIRSTNAME",
+        description: "First name for the user"
+
+      option :last_name,
+        long: "--last-name LASTNAME",
+        description: "Last name for the user"
+
+      option :email,
+        long: "--email EMAIL",
+        description: "Email for the user"
+
+      option :password,
+        long: "--password PASSWORD",
+        description: "Password for the user"
+
+      banner "knife user create USERNAME --email EMAIL --password PASSWORD (options)"
 
       def user
         @user_field ||= Chef::UserV1.new
@@ -64,19 +80,27 @@ class Chef
         test_mandatory_field(@name_args[0], "username")
         user.username @name_args[0]
 
-        test_mandatory_field(@name_args[1], "display name")
-        user.display_name @name_args[1]
-
-        test_mandatory_field(@name_args[2], "first name")
-        user.first_name @name_args[2]
-
-        test_mandatory_field(@name_args[3], "last name")
-        user.last_name @name_args[3]
-
-        test_mandatory_field(@name_args[4], "email")
-        user.email @name_args[4]
+        if @name_args.size > 1
+          ui.warn "[DEPRECATED] DISPLAY_NAME FIRST_NAME LAST_NAME EMAIL PASSWORD options are deprecated and will be removed in future release. Use USERNAME --email --password TAGS option instead."
+          test_mandatory_field(@name_args[1], "display name")
+          user.display_name @name_args[1]
+          test_mandatory_field(@name_args[2], "first name")
+          user.first_name @name_args[2]
+          test_mandatory_field(@name_args[3], "last name")
+          user.last_name @name_args[3]
+          test_mandatory_field(@name_args[4], "email")
+          user.email @name_args[4]
+          password = config[:passwordprompt] ? prompt_for_password : @name_args[5]
+        else
+          test_mandatory_field(config[:email], "email")
+          test_mandatory_field(config[:password], "password") unless config[:passwordprompt]
+          user.display_name user.username
+          user.first_name config[:first_name] || ""
+          user.last_name config[:last_name] || ""
+          user.email config[:email]
+          password = config[:passwordprompt] ? prompt_for_password : config[:password]
+        end
 
-        password = config[:passwordprompt] ? prompt_for_password : @name_args[5]
         unless password
           ui.fatal "You must either provide a password or use the --prompt-for-password (-p) option"
           exit 1
@@ -96,14 +120,25 @@ class Chef
           user.public_key File.read(File.expand_path(config[:user_key]))
         end
 
-        user_hash = {
-          username: user.username,
-          first_name: user.first_name,
-          last_name: user.last_name,
-          display_name: "#{user.first_name} #{user.last_name}",
-          email: user.email,
-          password: password,
-        }
+        if @name_args.size > 1
+          user_hash = {
+            username: user.username,
+            first_name: user.first_name,
+            last_name: user.last_name,
+            display_name: "#{user.first_name} #{user.last_name}",
+            email: user.email,
+            password: password,
+          }
+        else
+          user_hash = {
+            username: user.username,
+            first_name: user.first_name,
+            last_name: user.last_name,
+            display_name: user.display_name,
+            email: user.email,
+            password: password,
+          }
+        end
 
         # Check the file before creating the user so the api is more transactional.
         if config[:file]
diff --git a/lib/chef/user_v1.rb b/lib/chef/user_v1.rb
index 945f0197df..3720f5efe7 100644
--- a/lib/chef/user_v1.rb
+++ b/lib/chef/user_v1.rb
@@ -145,11 +145,11 @@ class Chef
         payload = {
           username: @username,
           display_name: @display_name,
-          first_name: @first_name,
-          last_name: @last_name,
           email: @email,
-          password: @password,
         }
+        payload[:first_name] = @first_name unless @first_name.nil?
+        payload[:last_name] = @last_name unless @last_name.nil?
+        payload[:password] = @password unless @password.nil?
         payload[:public_key] = @public_key unless @public_key.nil?
         payload[:create_key] = @create_key unless @create_key.nil?
         payload[:middle_name] = @middle_name unless @middle_name.nil?
@@ -258,7 +258,6 @@ class Chef
     end
 
     # Class Methods
-
     def self.from_hash(user_hash)
       user = Chef::UserV1.new
       user.username user_hash["username"]
diff --git a/spec/unit/knife/user_create_spec.rb b/spec/unit/knife/user_create_spec.rb
index fb6a2c3ba0..1257d92ee9 100644
--- a/spec/unit/knife/user_create_spec.rb
+++ b/spec/unit/knife/user_create_spec.rb
@@ -212,5 +212,45 @@ describe Chef::Knife::UserCreate do
       end
     end
 
+    describe "user user_name, --email, --password" do
+      let(:name_args) { %w{some_user} }
+
+      before :each do
+        @user = double("Chef::User")
+        expect(Chef::ServerAPI).to receive(:new).with(Chef::Config[:chef_server_root]).and_return(root_rest)
+        expect(root_rest).to receive(:post).and_return(@user)
+        @key = "You don't come into cooking to get rich - Ramsay"
+        allow(@user).to receive(:[]).with("private_key").and_return(@key)
+        knife.name_args = name_args
+        knife.config[:email] = "test@email.com"
+        knife.config[:password] = "some_password"
+      end
+
+      it "creates an user" do
+        expect(knife.ui).to receive(:msg).with(@key)
+        knife.run
+      end
+
+      context "with --orgname" do
+        before :each do
+          knife.config[:orgname] = "ramsay"
+          @uri = "http://www.example.com/1"
+          allow(@user).to receive(:[]).with("uri").and_return(@uri)
+        end
+
+        let(:request_body) {
+          { user: "some_user" }
+        }
+
+        it "creates an user, associates a user, and adds it to the admins group" do
+
+          expect(root_rest).to receive(:post).with("organizations/ramsay/association_requests", request_body).and_return(@user)
+          expect(root_rest).to receive(:put).with("users/some_user/association_requests/1", { response: "accept" })
+          knife.run
+        end
+      end
+
+    end
+
   end # when all mandatory fields are validly specified
 end
diff --git a/spec/unit/user_v1_spec.rb b/spec/unit/user_v1_spec.rb
index 17bd4f3094..3984fb4dec 100644
--- a/spec/unit/user_v1_spec.rb
+++ b/spec/unit/user_v1_spec.rb
@@ -176,8 +176,10 @@ describe Chef::UserV1 do
       expect(@user.to_json).to include(%{"display_name":"get_displayed"})
     end
 
-    it "does not include the display name if not present" do
-      expect(@json).not_to include("display_name")
+    it "does not include the display name if user name not present" do
+      unless @user.username
+        expect(@json).not_to include("display_name")
+      end
     end
 
     it "includes the first name when present" do