Merge pull request openshift#358 from smarterclayton/bug_951352_shoul…

…d_retry_key_on_bad_name

Merged by openshift-bot

lib/rhc/highline_extensions.rb

@@ -7,7 +7,9 @@ class HighLineExtension < HighLine
   [:ask, :agree].each do |sym|
     define_method(sym) do |*args, &block|
       separate_blocks
-      super(*args, &block)
+      r = super(*args, &block)
+      @last_line_open = false
+      r
     end
   end
 
@@ -31,8 +33,10 @@ def say(msg)
         if statement[-1, 1] == " " or statement[-1, 1] == "\t"
           @output.print(statement)
           @output.flush
+          true
         else
           @output.puts(statement)
+          false
         end
 
     elsif msg.respond_to? :each

lib/rhc/rest/mock.rb

@@ -68,16 +68,21 @@ def stub_api_v12(auth=false)
     def stub_user(auth=mock_user_auth)
       stub_api_request(:get, 'broker/rest/user', auth).to_return(simple_user(username))
     end
-    def stub_create_default_key
+    def stub_add_key(name='default')
       stub_api_request(:post, 'broker/rest/user/keys', mock_user_auth).
-        with(:body => hash_including({:name => 'default', :type => 'ssh-rsa'})).
+        with(:body => hash_including({:name => name, :type => 'ssh-rsa'})).
         to_return({:status => 201, :body => {}.to_json})
     end
     def stub_update_key(name)
       stub_api_request(:put, "broker/rest/user/keys/#{name}", mock_user_auth).
         with(:body => hash_including({:type => 'ssh-rsa'})).
         to_return({:status => 200, :body => {}.to_json})
     end
+    def stub_add_key_error(name, message, code=422)
+      stub_api_request(:post, "broker/rest/user/keys", mock_user_auth).
+        with(:body => hash_including({:type => 'ssh-rsa'})).
+        to_return({:status => code, :body => {:messages => [{:text => message, :field => 'name', :severity => 'error'}]}.to_json})
+    end
     def stub_create_domain(name)
       stub_api_request(:post, 'broker/rest/domains', mock_user_auth).
         with(:body => hash_including({:id => name})).

lib/rhc/ssh_helpers.rb

@@ -127,7 +127,7 @@ def fingerprint_for_local_key(key)
     end
 
     def fingerprint_for_default_key
-      fingerprint_for_local_key RHC::Config.ssh_pub_key_file_path
+      fingerprint_for_local_key(RHC::Config.ssh_pub_key_file_path)
     end
 
     # for an SSH public key specified by 'key', return a triple
@@ -144,7 +144,7 @@ def ssh_key_triple_for(key)
     end
 
     def ssh_key_triple_for_default_key
-      ssh_key_triple_for RHC::Config.ssh_pub_key_file_path
+      ssh_key_triple_for(RHC::Config.ssh_pub_key_file_path)
     end
 
     private

lib/rhc/wizard.rb

@@ -219,119 +219,114 @@ def config_ssh_key_stage
     # return true if the account has the public key defined by
     # RHC::Config::ssh_pub_key_file_path
     def ssh_key_uploaded?
-      ssh_keys.any? { |k| k.fingerprint == fingerprint_for_default_key }
+      ssh_keys.present? && ssh_keys.any? { |k| k.fingerprint.present? && k.fingerprint == fingerprint_for_default_key }
     end
 
     def existing_keys_info
       return unless ssh_keys
       indent{ ssh_keys.each{ |key| paragraph{ display_key(key) } } }
     end
 
-    def get_preferred_key_name
-      key_name = 'default'
+    def upload_ssh_key_stage
+      return true if ssh_key_uploaded?
 
-      if ssh_keys.empty?
-        paragraph do
-          info "Since you do not have any keys associated with your OpenShift account, "\
-              "your new key will be uploaded as the 'default' key."
-        end
-      else
-        paragraph do
-          say "You can enter a name for your key, or leave it blank to use the default name. " \
-              "Using the same name as an existing key will overwrite the old key."
-        end
+      upload = paragraph do
+        agree "Your public SSH key must be uploaded to the OpenShift server to access code.  Upload now? (yes|no) "
+      end
 
-        paragraph { existing_keys_info }
+      if upload
+        if ssh_keys.empty?
+          paragraph do
+            info "Since you do not have any keys associated with your OpenShift account, "\
+                "your new key will be uploaded as the 'default' key."
+            upload_ssh_key('default')
+          end
+        else
+          paragraph { existing_keys_info }
+
+          key_fingerprint = fingerprint_for_default_key
+          unless key_fingerprint
+            paragraph do
+              warn "Your ssh public key at #{system_path(RHC::Config.ssh_pub_key_file_path)} is invalid or unreadable. "\
+                  "Setup can not continue until you manually remove or fix your "\
+                  "public and private keys id_rsa keys."
+            end
+            return false
+          end
 
-        key_fingerprint = fingerprint_for_default_key
-        unless key_fingerprint
           paragraph do
-            warn "Your ssh public key at #{system_path(RHC::Config.ssh_pub_key_file_path)} is invalid or unreadable. "\
-                "Setup can not continue until you manually remove or fix your "\
-                "public and private keys id_rsa keys."
+            say "You can enter a name for your key, or leave it blank to use the default name. " \
+                "Using the same name as an existing key will overwrite the old key."
           end
-          return nil
+          ask_for_key_name
+        end
+      else
+        paragraph do
+          info "You can upload your SSH key at a later time using the 'rhc sshkey' command"
         end
+      end
 
-        userkey = username ? username.gsub(/@.*/, '') : ''
-        pubkey_base_name = "#{userkey}#{hostname.gsub(/\..*\z/,'')}".gsub(/[^A-Za-z0-9]/,'').slice(0,16)
-        default_name = find_unique_key_name(
-          :keys => ssh_keys,
-          :base => pubkey_base_name,
-          :max_length => DEFAULT_MAX_LENGTH
-        )
+      true
+    end
 
-        paragraph do
-          key_name = ask("Provide a name for this key: ") do |q|
+    def ask_for_key_name(default_name=get_preferred_key_name)
+      key_name = nil
+      paragraph do
+        begin
+          key_name = ask "Provide a name for this key: " do |q|
             q.default = default_name
-            q.validate = /^[0-9a-zA-Z]*$/
-            q.responses[:not_valid]    = 'Your key name must be letters and numbers only.'
+            q.responses[:ask_on_error] = ''
           end
-        end
+        end while !upload_ssh_key(key_name)
       end
+    end
 
-      key_name
+    def get_preferred_key_name
+      userkey = username ? username.gsub(/@.*/, '') : ''
+      pubkey_base_name = "#{userkey}#{hostname.gsub(/\..*\z/,'')}".gsub(/[^A-Za-z0-9]/,'').slice(0,16)
+      find_unique_key_name(pubkey_base_name)
     end
-    
+
     # given the base name and the maximum length,
     # find a name that does not clash with what is in opts[:keys]
-    def find_unique_key_name(opts)
-      keys = opts[:keys] || ssh_keys
-      base = opts[:base] || 'default'
-      max  = opts[:max_length] || DEFAULT_MAX_LENGTH
+    def find_unique_key_name(base='default')
+      max = DEFAULT_MAX_LENGTH
       key_name_suffix = 1
       candidate = base
       while ssh_keys.detect { |k| k.name == candidate }
-        candidate = base.slice(0, max - key_name_suffix.to_s.length) +
-          key_name_suffix.to_s
+        candidate = base.slice(0, max - key_name_suffix.to_s.length) + key_name_suffix.to_s
         key_name_suffix += 1
       end
       candidate
     end
 
-    def upload_ssh_key
-      key_name = get_preferred_key_name
-      return false unless key_name
+    def upload_ssh_key(key_name)
+      return false unless key_name.present?
 
       type, content, comment = ssh_key_triple_for_default_key
-      indent do
-        say table([['Type:', type], ['Fingerprint:', fingerprint_for_default_key]])
-      end
 
-      paragraph do
-        if !ssh_keys.empty? && ssh_keys.any? { |k| k.name == key_name }
-          clear_ssh_keys_cache
-          say "Key with the name #{key_name} already exists. Updating ... "
+      if !ssh_keys.empty? && ssh_keys.any? { |k| k.name == key_name }
+        clear_ssh_keys_cache
+        paragraph do
+          say "Key with the name '#{key_name}' already exists. Updating ... "
           key = rest_client.find_key(key_name)
           key.update(type, content)
-        else
-          clear_ssh_keys_cache
-          say "Uploading key '#{key_name}' from #{system_path(RHC::Config::ssh_pub_key_file_path)} ... "
-          rest_client.add_key key_name, content, type
+          success "done"
         end
-        success "done"
-      end
-
-      true
-    end
-
-    def upload_ssh_key_stage
-      return true if ssh_key_uploaded?
-
-      upload = paragraph do
-        agree "Your public SSH key must be uploaded to the OpenShift server to access code.  Upload now? (yes|no) "
-      end
-
-      if upload
-        upload_ssh_key
       else
-        paragraph do
-          info "You can upload your SSH key at a later time using the 'rhc sshkey' command"
+        clear_ssh_keys_cache
+        begin
+          rest_client.add_key(key_name, content, type)
+          paragraph{ say "Uploading key '#{key_name}' ... #{color('done', :green)}" }
+        rescue RHC::Rest::ValidationException => e
+          error e.message || "Unknown error during key upload."
+          return false
         end
       end
 
       true
-    end
+    end    
+
 
     ##
     # Alert the user that they should manually install tools if they are not