From 0098b704ea0269e36aa9edba52cafe420b362398 Mon Sep 17 00:00:00 2001 From: Brenton Leanhardt Date: Fri, 12 Apr 2013 14:01:42 -0400 Subject: [PATCH 1/5] tito releasers update --- rel-eng/releasers.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rel-eng/releasers.conf b/rel-eng/releasers.conf index b258db73d..83a418707 100644 --- a/rel-eng/releasers.conf +++ b/rel-eng/releasers.conf @@ -2,6 +2,6 @@ releaser = tito.release.DistGitReleaser branches = libra-rhel-6 -[onprem] +[ose-1.2] releaser = tito.release.DistGitReleaser branches = devops-1-rhel-6 From 4218c8d2b20caa722871d6f3fec4a306080472c1 Mon Sep 17 00:00:00 2001 From: fabianofranz Date: Fri, 12 Apr 2013 12:26:02 -0300 Subject: [PATCH 2/5] Bug 951369 --- lib/rhc/commands/alias.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/rhc/commands/alias.rb b/lib/rhc/commands/alias.rb index f7ccbb350..1eacbc591 100644 --- a/lib/rhc/commands/alias.rb +++ b/lib/rhc/commands/alias.rb @@ -55,12 +55,12 @@ def remove(app, app_alias) Pass phrase for the certificate private key is required if the provided private key is encrypted. DESC - syntax " --certificate FILE --private-key FILE [--passphrase passphrase]" + syntax " --certificate FILE --private-key FILE [--passphrase PASSPHRASE]" argument :app, "Application name (required)", ["-a", "--app name"], :context => :app_context, :required => true argument :app_alias, "Custom domain name for the application (required)", [] option ["--certificate FILE"], "SSL certificate filepath (file in .crt or .pem format)", :required => true option ["--private-key FILE"], "Private key filepath for the given SSL certificate", :required => true - option ["--passphrase passphrase"], "Private key pass phrase, required if the private key is encripted", :required => false + option ["--passphrase PASSPHRASE"], "Private key pass phrase, required if the private key is encrypted", :required => false option ["-n", "--namespace NAME"], "Namespace of your application", :context => :namespace_context, :required => true def update_cert(app, app_alias) certificate_file_path = options.certificate From faf9cabaaa1c544d4ac86e2deda41383c860fa55 Mon Sep 17 00:00:00 2001 From: fabianofranz Date: Fri, 12 Apr 2013 17:43:39 -0300 Subject: [PATCH 3/5] Bug 951436 - handling Windows platform on paging --- lib/rhc/highline_extensions.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/rhc/highline_extensions.rb b/lib/rhc/highline_extensions.rb index d5af6022c..a29dc8ae2 100644 --- a/lib/rhc/highline_extensions.rb +++ b/lib/rhc/highline_extensions.rb @@ -170,7 +170,7 @@ def paragraph(&block) def pager #:nocov: - return if RUBY_PLATFORM =~ /win32/ + return if RHC::Helpers.windows? return unless @output.tty? read, write = IO.pipe @@ -405,4 +405,4 @@ def rows end $terminal = HighLineExtension.new -$terminal.indent_size = 2 if $terminal.respond_to? :indent_size \ No newline at end of file +$terminal.indent_size = 2 if $terminal.respond_to? :indent_size From 5fdaef648e2baa80a668037bf3fdb4abe056e53c Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Fri, 12 Apr 2013 17:45:21 -0400 Subject: [PATCH 4/5] Bug 951352 - Retry the key name during the wizard flow --- lib/rhc/highline_extensions.rb | 6 +- lib/rhc/rest/mock.rb | 9 ++- lib/rhc/ssh_helpers.rb | 4 +- lib/rhc/wizard.rb | 139 ++++++++++++++++----------------- spec/rhc/wizard_spec.rb | 87 ++++++++++++++------- spec/wizard_spec_helper.rb | 4 +- 6 files changed, 140 insertions(+), 109 deletions(-) diff --git a/lib/rhc/highline_extensions.rb b/lib/rhc/highline_extensions.rb index d5af6022c..0b4e77ad0 100644 --- a/lib/rhc/highline_extensions.rb +++ b/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 diff --git a/lib/rhc/rest/mock.rb b/lib/rhc/rest/mock.rb index 8cd2a8a3d..f979c1fbb 100644 --- a/lib/rhc/rest/mock.rb +++ b/lib/rhc/rest/mock.rb @@ -68,9 +68,9 @@ 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) @@ -78,6 +78,11 @@ def stub_update_key(name) 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})). diff --git a/lib/rhc/ssh_helpers.rb b/lib/rhc/ssh_helpers.rb index 290937b50..cc18b9af9 100644 --- a/lib/rhc/ssh_helpers.rb +++ b/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 diff --git a/lib/rhc/wizard.rb b/lib/rhc/wizard.rb index d65eaf875..965a70426 100644 --- a/lib/rhc/wizard.rb +++ b/lib/rhc/wizard.rb @@ -219,7 +219,7 @@ 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 @@ -227,111 +227,106 @@ def existing_keys_info 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 diff --git a/spec/rhc/wizard_spec.rb b/spec/rhc/wizard_spec.rb index 1a51bfab1..e5efc7e3d 100644 --- a/spec/rhc/wizard_spec.rb +++ b/spec/rhc/wizard_spec.rb @@ -282,7 +282,7 @@ def expect_raise_from_api(error) context "when the user enters a domain and uploads a key" do before do - stub_create_default_key + stub_add_key stub_api_request(:post, 'broker/rest/domains', user_auth). with(:body => /(thisnamespaceistoobig|invalidnamespace)/). to_return({ @@ -344,6 +344,33 @@ def expect_raise_from_api(error) end end + context "when a multiple keys exist but is not the same" do + before{ setup_mock_ssh(true) } + before do + stub_one_key('a_key') + stub_add_key_error('invalid```--', 'Invalid key name') + stub_add_key('another_key') + end + it "should give the user a name the key" do + should_greet_user + should_challenge_for(username, password) + should_write_config + should_not_create_an_ssh_keypair + + input_line 'yes' + input_line 'invalid```--' + input_line 'another_key' + next_stage + + last_output do |s| + s.should match(/a_key \(type: ssh-rsa\)/) + s.should match("Fingerprint: #{rsa_key_fingerprint_public}") + s.should match(" name |a_key|") + s.should match("Invalid key name") + s.should match("Uploading key 'another_key'") + end + end + end context "when the default key already exists on the server" do before{ setup_mock_ssh(true) } before{ stub_mock_ssh_keys } @@ -366,7 +393,7 @@ def expect_raise_from_api(error) stub_api(:user => username) stub_user stub_no_keys - stub_create_default_key + stub_add_key stub_api_request(:post, 'broker/rest/domains', user_auth). with(:body => /(thisnamespaceistoobig|invalidnamespace)/). to_return({ @@ -418,7 +445,7 @@ def expect_raise_from_api(error) context "with no server keys" do before{ stub_no_keys } - before{ stub_create_default_key } + before{ stub_add_key } it "should generate and upload keys since the user does not have them" do input_line "yes" @@ -455,44 +482,45 @@ def expect_raise_from_api(error) let(:wizard){ RerunWizardDriver.new } it "should cause ssh_key_upload? to catch NoMethodError and call the fallback to get the fingerprint" do - Net::SSH::KeyFactory.stub(:load_public_key) { raise NoMethodError } - @fallback_run = false - wizard.stub(:ssh_keygen_fallback) { @fallback_run = true } - key_data = wizard.get_mock_key_data - @rest_client.stub(:sshkeys) { key_data } + Net::SSH::KeyFactory.should_receive(:load_public_key).exactly(4).times.and_raise(NoMethodError) + wizard.should_receive(:ssh_keygen_fallback).exactly(4).times + wizard.should_receive(:ssh_keys).at_least(1).times.and_return(wizard.get_mock_key_data) wizard.send(:ssh_key_uploaded?) - - @fallback_run.should be_true end it "should cause upload_ssh_key to catch NoMethodError and call the fallback to get the fingerprint" do - wizard.ssh_keys = wizard.get_mock_key_data - @fallback_run = false - wizard.stub(:ssh_keygen_fallback) do - @fallback_run = true - [OpenStruct.new( :name => 'default', :fingerprint => 'AA:BB:CC:DD:EE:FF', :type => 'ssh-rsa' )] - end - $?.stub(:exitstatus) { 255 } - Net::SSH::KeyFactory.stub(:load_public_key) { raise NoMethodError } + Net::SSH::KeyFactory.should_receive(:load_public_key).exactly(5).times.and_raise(NoMethodError) + wizard.stub(:ssh_keys).at_least(1).times.and_return(wizard.get_mock_key_data) + wizard.should_receive(:ssh_keygen_fallback).exactly(5).times.and_return(stub(:name => 'default', :fingerprint => 'AA:BB:CC:DD:EE:FF', :type => 'ssh-rsa' )) - wizard.send(:upload_ssh_key).should be_false + input_line 'y' - output = last_output - output.should match("Your ssh public key at .* is invalid or unreadable\.") - @fallback_run.should be_true + wizard.send(:upload_ssh_key_stage).should be_false + + last_output.should match("Your ssh public key at .* is invalid or unreadable\.") end it "should cause upload_ssh_key to catch NotImplementedError and return false" do - wizard.ssh_keys = wizard.get_mock_key_data - Net::SSH::KeyFactory.stub(:load_public_key) { raise NotImplementedError } + Net::SSH::KeyFactory.should_receive(:load_public_key).exactly(5).times.and_raise(NoMethodError) + wizard.should_receive(:ssh_keys).at_least(1).times.and_return(wizard.get_mock_key_data) + + input_line 'y' - wizard.send(:upload_ssh_key).should be_false + wizard.send(:upload_ssh_key_stage).should be_false output = last_output output.should match("Your ssh public key at .* is invalid or unreadable\.") end + it "should find a unique name" do + wizard.should_receive(:ssh_keys).at_least(1).times.and_return(wizard.get_mock_key_data) + + wizard.send(:find_unique_key_name, 'cb490595').should == 'cb4905951' + wizard.send(:find_unique_key_name, 'default').should == 'default1' + wizard.send(:find_unique_key_name, 'abc').should == 'abc' + end + it "should match ssh key fallback fingerprint to net::ssh fingerprint" do # we need to write to a live file system so ssh-keygen can find it FakeFS.deactivate! @@ -525,12 +553,11 @@ def expect_raise_from_api(error) key_name = 'default' key_data = wizard.get_mock_key_data wizard.ssh_keys = key_data - wizard.stub(:get_preferred_key_name) { key_name } wizard.stub(:ssh_key_triple_for_default_key) { pub_key.chomp.split } wizard.stub(:fingerprint_for_default_key) { "" } # this value is irrelevant - wizard.rest_client.stub(:find_key) { key_data.detect { |k| k.name == key_name } } + wizard.rest_client = stub('RestClient').tap{ |o| o.stub(:find_key) { key_data.detect { |k| k.name == key_name } } } - wizard.send(:upload_ssh_key) + wizard.send(:upload_ssh_key, key_name) output = last_output output.should match 'Updating' end @@ -544,9 +571,9 @@ def expect_raise_from_api(error) wizard.ssh_keys = key_data wizard.stub(:ssh_key_triple_for_default_key) { pub_key.chomp.split } wizard.stub(:fingerprint_for_default_key) { "" } # this value is irrelevant - wizard.rest_client.stub(:add_key) { true } + wizard.rest_client = stub('RestClient').tap{ |o| o.stub(:add_key) { true } } - wizard.send(:upload_ssh_key) + wizard.send(:upload_ssh_key, "other") output = last_output # since the clashing key name is short, we expect to present # a key name with "1" attached to it. diff --git a/spec/wizard_spec_helper.rb b/spec/wizard_spec_helper.rb index d143ed127..4b688282b 100644 --- a/spec/wizard_spec_helper.rb +++ b/spec/wizard_spec_helper.rb @@ -64,8 +64,8 @@ def should_upload_default_key last_output do |s| s.should match('Since you do not have any keys associated') - s.should match(/Fingerprint\: (?:[a-f0-9]{2}\:){15}/) - s.should match("Uploading key 'default' from #{current_ssh_dir}/id_rsa.pub ... ") + #s.should match(/Fingerprint\: (?:[a-f0-9]{2}\:){15}/) + s.should match("Uploading key 'default' ... ") end end From d7698fe3676e1657eeaf877b7ad272cdf70c98ac Mon Sep 17 00:00:00 2001 From: Krishna Raman Date: Sat, 13 Apr 2013 15:42:48 -0700 Subject: [PATCH 5/5] Automatic commit of package [rhc] release [1.7.5-1]. --- client.spec | 12 +++++++++++- rel-eng/packages/rhc | 2 +- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/client.spec b/client.spec index fd9a394a5..87b9bb93e 100644 --- a/client.spec +++ b/client.spec @@ -3,7 +3,7 @@ Summary: OpenShift client management tools Name: rhc -Version: 1.7.4 +Version: 1.7.5 Release: 1%{?dist} Group: Network/Daemons License: ASL 2.0 @@ -99,6 +99,16 @@ rm -rf $RPM_BUILD_ROOT %attr(0644,-,-) /etc/bash_completion.d/rhc %changelog +* Sat Apr 13 2013 Krishna Raman 1.7.5-1 +- Merge pull request #358 from + smarterclayton/bug_951352_should_retry_key_on_bad_name + (dmcphers+openshiftbot@redhat.com) +- Bug 951352 - Retry the key name during the wizard flow (ccoleman@redhat.com) +- Merge pull request #355 from fabianofranz/master (dmcphers@redhat.com) +- Bug 951436 - handling Windows platform on paging (ffranz@redhat.com) +- Bug 951369 (ffranz@redhat.com) +- tito releasers update (bleanhar@redhat.com) + * Thu Apr 11 2013 Adam Miller 1.7.4-1 - Merge pull request #352 from smarterclayton/origin_ui_13_autocomplete_and_wrapping diff --git a/rel-eng/packages/rhc b/rel-eng/packages/rhc index b4b963d19..c6dc83b94 100644 --- a/rel-eng/packages/rhc +++ b/rel-eng/packages/rhc @@ -1 +1 @@ -1.7.4-1 / +1.7.5-1 /