From 04fd726c2974706aa1812f2ebb0530222f8868b4 Mon Sep 17 00:00:00 2001 From: thrasher-redhat Date: Fri, 16 Oct 2015 17:56:17 -0400 Subject: [PATCH] server.rb: fixes incorrect error output and additional input validation Bug 1155003 Bugzilla link https://bugzilla.redhat.com/show_bug.cgi?id=1155003 Changes usage messages to include --ssl-client-key-file option, which was previously missing. The rhc server add command also requires all three of --ssl-ca-file, --ssl-client-cert-file, and ssl-client-key-file and will now raise an error if only 1 or 2 of the 3 are present. Adds corresponding spec test. --- lib/rhc/commands/server.rb | 21 +++++++------ spec/rhc/commands/server_spec.rb | 54 +++++++++++++++++++------------- 2 files changed, 45 insertions(+), 30 deletions(-) diff --git a/lib/rhc/commands/server.rb b/lib/rhc/commands/server.rb index 71fe5871d..9b0e32aca 100644 --- a/lib/rhc/commands/server.rb +++ b/lib/rhc/commands/server.rb @@ -10,7 +10,7 @@ class Server < Base servers to interact with the rhc commands and easily switch between them. - For example, if an user's company has installations of OpenShift Origin + For example, if an user's company has installations of OpenShift Origin (development) and Enterprise (production) and the user also has a personal OpenShift Online account: @@ -70,12 +70,12 @@ def status(server=nil) summary "Add a new server" description <<-DESC - Add and configure a new OpenShift server that will be available to + Add and configure a new OpenShift server that will be available to use through rhc commands. When adding a new server users can optionally provide a 'nickname' - that will allow to easily switch between servers. + that will allow to easily switch between servers. DESC - syntax " [] [--rhlogin LOGIN] [--[no-]use-authorization-tokens] [--[no-]insecure] [--use] [--skip-wizard] [--timeout SECONDS] [--ssl-ca-file FILE] [--ssl-client-cert-file FILE] [--ssl-version VERSION]" + syntax " [] [--rhlogin LOGIN] [--[no-]use-authorization-tokens] [--[no-]insecure] [--use] [--skip-wizard] [--timeout SECONDS] [--ssl-ca-file FILE] [--ssl-client-cert-file FILE] [--ssl-client-key-file FILE] [--ssl-version VERSION]" argument :hostname, "Hostname of the server you are adding", ["--server HOSTNAME"] argument :nickname, "Optionally provide a nickname to the server you are adding (e.g. 'development', 'production', 'online')", ["--nickname NICKNAME"], :optional => true option ["-l", "--rhlogin LOGIN"], "Change the default OpenShift login used on this server" @@ -90,10 +90,13 @@ def status(server=nil) option ["--ssl-version VERSION"], "The version of SSL to use to be used on this server", :type => SSLVersion, :optional => true def add(hostname, nickname) raise ArgumentError, "The --use and --skip-wizard options cannot be used together." if options.use && options.skip_wizard + unless !options.ssl_ca_file == !options.ssl_client_cert_file && !options.ssl_client_cert_file == !options.ssl_client_key_file + raise ArgumentError, "You must use the --ssl-ca-file, --ssl-client-cert-file, and --ssl-client-key-file commands together." + end attrs = [:login, :use_authorization_tokens, :insecure, :timeout, :ssl_version, :ssl_client_cert_file, :ssl_client_key_file, :ssl_ca_file] - server = server_configs.add(hostname, + server = server_configs.add(hostname, attrs.inject({:nickname => nickname}){ |h, (k, v)| h[k] = options[k == :login ? :rhlogin : k]; h }) unless options.skip_wizard @@ -117,7 +120,7 @@ def list say display_server(server) end - paragraph do + paragraph do case servers.length when 0 warn "You don't have any servers configured. Use 'rhc setup' to configure your OpenShift server." @@ -166,7 +169,7 @@ def remove(server) end summary "Update server attributes" - syntax " [--hostname HOSTNAME] [--nickname NICKNAME] [--rhlogin LOGIN] [--[no-]use-authorization-tokens] [--[no-]insecure] [--use] [--skip-wizard] [--timeout SECONDS] [--ssl-ca-file FILE] [--ssl-client-cert-file FILE] [--ssl-version VERSION]" + syntax " [--hostname HOSTNAME] [--nickname NICKNAME] [--rhlogin LOGIN] [--[no-]use-authorization-tokens] [--[no-]insecure] [--use] [--skip-wizard] [--timeout SECONDS] [--ssl-ca-file FILE] [--ssl-client-cert-file FILE] [--ssl-client-key-file FILE] [--ssl-version VERSION]" argument :server, "Server hostname or nickname to be configured", ["--server SERVER"] option ["--hostname HOSTNAME"], "Change the hostname of this server" option ["--nickname NICKNAME"], "Change the nickname of this server" @@ -187,8 +190,8 @@ def configure(server) attrs = [:hostname, :nickname, :login, :use_authorization_tokens, :insecure, :timeout, :ssl_version, :ssl_client_cert_file, :ssl_client_key_file, :ssl_ca_file].inject({}){ |h, (k, v)| v = options[k == :login ? :rhlogin : k]; h[k] = (v.nil? ? server.send(k) : v); h } - raise RHC::ServerNicknameExistsException.new(options.nickname) if options.nickname && - server_configs.nickname_exists?(options.nickname) && + raise RHC::ServerNicknameExistsException.new(options.nickname) if options.nickname && + server_configs.nickname_exists?(options.nickname) && server_configs.find(options.nickname).hostname != server.hostname server = server_configs.update(server.hostname, attrs) diff --git a/spec/rhc/commands/server_spec.rb b/spec/rhc/commands/server_spec.rb index b343f367a..ef70c0c19 100644 --- a/spec/rhc/commands/server_spec.rb +++ b/spec/rhc/commands/server_spec.rb @@ -85,7 +85,7 @@ describe "server list" do context "without express.conf or servers.yml" do let(:arguments) { ['servers'] } - it 'should output correctly' do + it 'should output correctly' do run_output.should =~ /You don't have any servers configured\. Use 'rhc setup' to configure your OpenShift server/ end it { expect { run }.to exit_with_code(0) } @@ -96,7 +96,7 @@ stub_servers_yml end let(:arguments) { ['servers'] } - it 'should output correctly' do + it 'should output correctly' do run_output.should =~ /Server 'server1'/ run_output.should =~ /Hostname:\s+openshift1.server.com/ run_output.should =~ /Login:\s+user1/ @@ -114,7 +114,7 @@ stub_servers_yml end let(:arguments) { ['servers'] } - it 'should output correctly' do + it 'should output correctly' do run_output.should =~ /Server 'online' \(not configured, run 'rhc setup'\)/ run_output.should =~ /Hostname:\s+#{local_config_server}/ run_output.should =~ /Server 'server1'/ @@ -135,7 +135,7 @@ stub_servers_yml(entries) end let(:arguments) { ['servers'] } - it 'should output correctly' do + it 'should output correctly' do run_output.should =~ /Server 'online' \(not configured, run 'rhc setup'\)/ run_output.should =~ /Hostname:\s+#{local_config_server}/ Array(1..entries).each do |i| @@ -156,7 +156,7 @@ stub_servers_yml(entries) end let(:arguments) { ['servers'] } - it 'should output correctly' do + it 'should output correctly' do run_output.should =~ /Server 'server1' \(in use\)/ run_output.should =~ /Hostname:\s+#{local_config_server}/ run_output.should =~ /Login:\s+#{local_config_username}/ @@ -176,7 +176,7 @@ stub_servers_yml(entries) end let(:arguments) { ['servers'] } - it 'should output correctly' do + it 'should output correctly' do run_output.should =~ /Server '#{local_config_server}' \(not configured, run 'rhc setup'\)/ run_output.should =~ /Hostname:\s+#{local_config_server}/ Array(1..entries).each do |i| @@ -193,11 +193,11 @@ let(:local_config_username){ 'local_config_user' } let(:local_config_password){ 'password' } let(:local_config_server){ 'openshift.redhat.com' } - before do + before do local_config end let(:arguments) { ['server', 'show', 'online'] } - it 'should output correctly' do + it 'should output correctly' do run_output.should =~ /Server 'online' \(in use\)/ run_output.should =~ /Hostname:\s+openshift.redhat.com/ run_output.should =~ /Login:\s+local_config_user/ @@ -217,7 +217,7 @@ end end end - it 'should output correctly' do + it 'should output correctly' do run_output.should =~ /Server 'openshift1.server.com'/ run_output.should =~ /Hostname:\s+openshift1.server.com/ run_output.should =~ /Login:\s+user1/ @@ -235,7 +235,7 @@ local_config end let(:arguments) { ['server', 'show', 'zee'] } - it 'should output correctly' do + it 'should output correctly' do run_output.should =~ /You don't have any server configured with the hostname or nickname 'zee'/ end it { expect { run }.to exit_with_code(166) } @@ -248,7 +248,7 @@ let(:username){ 'user1' } let(:token){ 'an_existing_token' } let(:arguments) { ['server', 'add', new_server, '-l', username, '--use-authorization-tokens', '--no-insecure', '--token', token, '--use'] } - before(:each) { + before(:each) { stub_wizard } it 'should create servers.yml' do @@ -277,7 +277,7 @@ stub_wizard local_config end - it 'should output correctly' do + it 'should output correctly' do run_output.should =~ /Using an existing token for #{username} to login to #{server}/ run_output.should =~ /Saving configuration to.*express\.conf.*done/ run_output.should =~ /Saving server configuration to.*servers\.yml.*done/ @@ -292,7 +292,7 @@ before(:each) do local_config end - it 'should output correctly' do + it 'should output correctly' do run_output.should =~ /You already have a server configured with the hostname '#{local_config_server}'/ end it { expect { run }.to exit_with_code(165) } @@ -364,12 +364,24 @@ stub_servers_yml(2) end let(:arguments) { ['server', 'add', 'foo.com', 'server1', '-l', local_config_username, '--use-authorization-tokens', '--no-insecure'] } - it 'should output correctly' do + it 'should output correctly' do run_output.should =~ /You already have a server configured with the nickname 'server1'/ end it { expect { run }.to exit_with_code(164) } end + context "when providing ssl information and missing a required option" do + let(:arguments) { ['server', 'add', 'testing.server.com', '--ssl-ca-file', '~/ca.crt', '--ssl-client-cert-file', '~/server.crt'] } + before do + RHC::HelpFormatter.any_instance.stub(:render_command_syntax).and_return('foo') + RHC::Helpers.stub(:certificate_file).and_return('foo') + end + it 'should output correctly' do + run_output.should =~ /You must use the --ssl-ca-file, --ssl-client-cert-file, and --ssl-client-key-file commands together./ + end + it{ expect{run}.to exit_with_code(1) } + end + context "with wizard failure" do let(:token){ 'an_existing_token' } let(:arguments) { ['server', 'add', 'failure.server.com', 'failed', '-l', 'failer'] } @@ -390,7 +402,7 @@ local_config end let(:arguments) { ['server', 'remove', local_config_server] } - it 'should output correctly' do + it 'should output correctly' do run_output.should =~ /The '#{local_config_server}' server is in use/ end it { expect { run }.to exit_with_code(167) } @@ -405,7 +417,7 @@ local_config end let(:arguments) { ['server', 'remove', server] } - it 'should output correctly' do + it 'should output correctly' do run_output.should =~ /Removing '#{server}'.*done/ end it { expect { run }.to exit_with_code(0) } @@ -419,7 +431,7 @@ local_config end let(:arguments) { ['server', 'remove', 'zee'] } - it 'should output correctly' do + it 'should output correctly' do run_output.should =~ /You don't have any server configured with the hostname or nickname 'zee'/ end it { expect { run }.to exit_with_code(166) } @@ -464,7 +476,7 @@ let(:local_config_server_new_username){ 'new_username' } let(:local_config_server_new_name){ 'new_name' } - let(:arguments) { ['server', 'configure', local_config_server, '--nickname', local_config_server_new_name, '-l', local_config_server_new_username, '--insecure', + let(:arguments) { ['server', 'configure', local_config_server, '--nickname', local_config_server_new_name, '-l', local_config_server_new_username, '--insecure', '--skip-wizard'] } before do local_config @@ -487,7 +499,7 @@ local_config end let(:arguments) { ['server', 'configure', 'zee', '--insecure'] } - it 'should output correctly' do + it 'should output correctly' do run_output.should =~ /You don't have any server configured with the hostname or nickname 'zee'/ end it { expect { run }.to exit_with_code(166) } @@ -529,13 +541,13 @@ RHC::ServerWizard.any_instance.stub(:run).and_return(false) local_config stub_servers_yml - end + end let(:arguments) { ['server', 'use', 'local.server.com'] } it { expect { run }.to exit_with_code(1) } end end - protected + protected def stub_servers_yml(entries=1, &block) RHC::Servers.any_instance.stub(:present?).and_return(true) RHC::Servers.any_instance.stub(:load).and_return(