From d118d658de20708eac1a09b58471ba7f7c2e77bc Mon Sep 17 00:00:00 2001 From: Andrei Shaidurov Date: Wed, 12 Apr 2017 12:34:11 +0200 Subject: [PATCH] More product deactivation implementation --- lib/suse/connect/cli.rb | 5 ++ lib/suse/connect/client.rb | 27 +++++----- lib/suse/connect/errors.rb | 1 + lib/suse/connect/zypper.rb | 4 ++ spec/connect/cli_spec.rb | 100 ++++++++++++++++++++---------------- spec/connect/client_spec.rb | 57 ++++++++------------ spec/connect/status_spec.rb | 6 ++- 7 files changed, 104 insertions(+), 96 deletions(-) diff --git a/lib/suse/connect/cli.rb b/lib/suse/connect/cli.rb index 95faa871..41b08a32 100644 --- a/lib/suse/connect/cli.rb +++ b/lib/suse/connect/cli.rb @@ -75,6 +75,11 @@ def execute! # rubocop:disable MethodLength, CyclomaticComplexity # Zypper errors are in the range 1-7 and 100-105 (which connect will not cause) log.fatal "Error: zypper returned (#{e.exitstatus}) with '#{e.output}'" exit e.exitstatus + rescue SystemNotRegisteredError => e + log.fatal 'Deregistration failed. Check if the system has been '\ + 'registered using the --status-text option or use the '\ + '--regcode parameter to register it.' + exit 69 end private diff --git a/lib/suse/connect/client.rb b/lib/suse/connect/client.rb index 1ddbde13..6592d355 100644 --- a/lib/suse/connect/client.rb +++ b/lib/suse/connect/client.rb @@ -31,23 +31,20 @@ def register! print_success_message product end + # Deregisters a whole system or a single product + # # @returns: Empty body and 204 status code def deregister! - if registered? - if @config.product - service = deactivate_product @config.product - System.remove_service service - Zypper.remove_release_package product.identifier - print_success_message product, action: 'Deregistered' - else - @api.deregister(system_auth) - System.cleanup! - log.info 'Successfully deregistered system.' - end + raise SystemNotRegisteredError unless registered? + if @config.product + service = deactivate_product @config.product + System.remove_service service + Zypper.remove_release_package @config.product.identifier + print_success_message @config.product, action: 'Deregistered' else - log.fatal 'Deregistration failed. Check if the system has been '\ - 'registered using the -s option or use the --regcode parameter to '\ - 'register it.' + @api.deregister(system_auth) + System.cleanup! + log.info 'Successfully deregistered system.' end end @@ -168,7 +165,7 @@ def list_installer_updates(product) # Announces the system to the server, receiving and storing its credentials. # When already announced, sends the current hardware details to the server def announce_or_update - if System.credentials? + if registered? update_system else login, password = announce_system(nil, @config.instance_data_file) diff --git a/lib/suse/connect/errors.rb b/lib/suse/connect/errors.rb index 0be623b2..f2fa6fbb 100644 --- a/lib/suse/connect/errors.rb +++ b/lib/suse/connect/errors.rb @@ -10,6 +10,7 @@ class CannotDetectBaseProduct < StandardError; end class SystemCallError < StandardError; end class UnsupportedStatusFormat < StandardError; end class NetworkError < StandardError; end + class SystemNotRegisteredError < StandardError; end # Basic error for API interactions. Collects HTTP response (which includes # status code and response body) for future showing to user via {Cli} diff --git a/lib/suse/connect/zypper.rb b/lib/suse/connect/zypper.rb index bc22b2d9..97ba0afc 100644 --- a/lib/suse/connect/zypper.rb +++ b/lib/suse/connect/zypper.rb @@ -151,6 +151,10 @@ def install_release_package(identifier) call("--no-refresh --non-interactive install --no-recommends -t product #{identifier}") if identifier end + def remove_release_package(identifier) + call("--no-refresh --non-interactive remove --no-recommends -t product #{identifier}") if identifier + end + # rubocop:disable AccessorMethodName def set_release_version(version) call("--non-interactive --releasever #{version} ref -f") diff --git a/spec/connect/cli_spec.rb b/spec/connect/cli_spec.rb index 6c04f9b5..8b7a9278 100644 --- a/spec/connect/cli_spec.rb +++ b/spec/connect/cli_spec.rb @@ -2,17 +2,16 @@ require 'suse/connect/cli' describe SUSE::Connect::Cli do - subject { SUSE::Connect::Cli } - let(:default_logger) { SUSE::Connect::GlobalLogger.instance.log } let(:string_logger) { ::Logger.new(StringIO.new) } - let(:cli) { subject.new({}) } + let(:opts) { {} } + let(:cli) { described_class.new opts } let(:config_file) { File.expand_path File.join(File.dirname(__FILE__), '../fixtures/SUSEConnect') } before do allow(Zypper).to receive_messages(base_product: {}) - allow_any_instance_of(subject).to receive(:exit) - allow_any_instance_of(subject).to receive_messages(puts: true) + allow_any_instance_of(described_class).to receive(:exit) + allow_any_instance_of(described_class).to receive_messages(puts: true) SUSE::Connect::GlobalLogger.instance.log = string_logger allow_any_instance_of(Status).to receive(:activated_base_product?).and_return(true) end @@ -22,8 +21,10 @@ end describe '#execute!' do + subject { cli.execute! } + context 'server errors' do - let(:cli) { subject.new(%w{-r 123}) } + let(:opts) { %w{-r 123} } it 'should produce log output if ApiError encountered' do expect(string_logger).to receive(:fatal).with("Error: SCC returned 'test' (222)") @@ -95,7 +96,7 @@ end context 'zypper error' do - let(:cli) { subject.new(%w{-r 456}) } + let(:opts) { %w{-r 456} } it 'should produce log output if zypper errors' do expect(string_logger).to receive(:fatal).with('Error: zypper returned (666) with \'zypper down\'') @@ -106,13 +107,13 @@ context 'parameter dependencies' do it 'requires no other parameters on --status' do - cli = subject.new(%w{--status}) + cli = described_class.new(%w{--status}) expect_any_instance_of(Status).to receive(:print_product_statuses) cli.execute! end it 'does not require --regcode or --url when specifying a product (eg. an extension)' do - cli = subject.new(%w{-p sle-module-web-scripting/12/x86_64}) + cli = described_class.new(%w{-p sle-module-web-scripting/12/x86_64}) expect_any_instance_of(Client).to receive(:register!) cli.execute! end @@ -127,13 +128,13 @@ end it 'registers the system if --regcode was provided' do - cli = subject.new(%w{-r 456}) + cli = described_class.new(%w{-r 456}) expect_any_instance_of(Client).to receive(:register!) cli.execute! end it 'registers the system if --url was provided' do - cli = subject.new(%w{--url http://somewhere.com}) + cli = described_class.new(%w{--url http://somewhere.com}) expect_any_instance_of(Client).to receive(:register!) cli.execute! end @@ -153,21 +154,21 @@ end it '--instance-data requires --url' do - cli = subject.new(%w{--instance-data /tmp/test}) + cli = described_class.new(%w{--instance-data /tmp/test}) expect(string_logger).to receive(:error) .with('Please use --instance-data only in combination with --url pointing to your SMT server') cli.execute! end it '--instance-data is mutually exclusive with --regcode' do - cli = subject.new(%w{-r 123 --instance-data /tmp/test --url test}) + cli = described_class.new(%w{-r 123 --instance-data /tmp/test --url test}) expect(string_logger).to receive(:error) .with('Please use either --regcode or --instance-data') cli.execute! end it '--url implies --write-config' do - cli = subject.new(%w{-r 123 --url http://foo.test.com}) + cli = described_class.new(%w{-r 123 --url http://foo.test.com}) expect(cli.config.write_config).to eq true allow_any_instance_of(SUSE::Connect::Client).to receive(:register!) expect_any_instance_of(SUSE::Connect::Config).to receive(:write!) @@ -176,16 +177,26 @@ end context 'de-register command' do + let(:opts) { %w{--de-register} } + it '--de-register calls deregister! method' do - cli = subject.new(%w{--de-register}) expect_any_instance_of(Client).to receive(:deregister!) - cli.execute! + subject + end + + context 'on unregistered system' do + before { allow(SUSE::Connect::System).to receive(:credentials).and_return(nil) } + + it 'dies with error' do + expect(string_logger).to receive(:fatal).with(/Deregistration failed. Check if the system has been registered/) + subject + end end end context 'cleanup command' do it '--cleanup calls Systems cleanup! method' do - cli = subject.new(%w{--cleanup}) + cli = described_class.new(%w{--cleanup}) expect(System).to receive(:cleanup!) cli.execute! end @@ -194,20 +205,20 @@ context 'namespace option' do |_variables| it '--namespace requires namespace' do expect(string_logger).to receive(:error).with('Please provide a namespace') - subject.new('--namespace') + described_class.new('--namespace') end end context 'status subcommand' do it '--status calls json_product_status' do - cli = subject.new(%w{--status}) + cli = described_class.new(%w{--status}) expect_any_instance_of(Client).to_not receive(:register!) expect_any_instance_of(Status).to receive(:json_product_status) cli.execute! end it '--status-text calls text_product_status' do - cli = subject.new(%w{--status-text}) + cli = described_class.new(%w{--status-text}) expect_any_instance_of(Client).to_not receive(:register!) expect_any_instance_of(Status).to receive(:text_product_status) cli.execute! @@ -217,7 +228,7 @@ describe 'list extensions subcommand' do context 'on system with registered base product' do it '--list-extensions lists all available extensions on the system' do - cli = subject.new(%w{--list-extensions}) + cli = described_class.new(%w{--list-extensions}) expect_any_instance_of(Client).not_to receive(:register!) expect_any_instance_of(Status).to receive(:print_extensions_list) cli.execute! @@ -227,12 +238,12 @@ context 'on system with no registered base product' do it '--list-extensions exits with an error explaining that a base product has to be registered first' do allow_any_instance_of(Status).to receive(:activated_base_product?).and_return(false) - cli = subject.new(%w{--list-extensions}) + cli = described_class.new(%w{--list-extensions}) expect_any_instance_of(Client).not_to receive(:register!) expect_any_instance_of(Status).not_to receive(:print_extensions_list) expect(string_logger).to receive(:error) .with(/To list extensions, you must first register the base product, using: SUSEConnect -r /) - expect_any_instance_of(subject).to receive(:exit) + expect_any_instance_of(described_class).to receive(:exit) cli.execute! end end @@ -242,13 +253,13 @@ it '--rollback calls SUSE::Connect::Migration.rollback' do expect_any_instance_of(Client).not_to receive(:register!) expect(SUSE::Connect::Migration).to receive(:rollback) - subject.new(%w{--rollback}) + described_class.new(%w{--rollback}) end end describe 'config write' do it 'writes config if appropriate cli param been passed' do - cli = subject.new(%w{--write-config --status}) + cli = described_class.new(%w{--write-config --status}) expect_any_instance_of(SUSE::Connect::Config).to receive(:write!) allow_any_instance_of(Status).to receive(:print_product_statuses) cli.execute! @@ -259,75 +270,75 @@ describe '?extract_options' do it 'sets token options' do argv = %w{-r matoken} - cli = subject.new(argv) + cli = described_class.new(argv) expect(cli.options[:token]).to eq 'matoken' end it 'sets product options' do argv = %w{--product sles/12/i386} - cli = subject.new(argv) + cli = described_class.new(argv) expect(cli.options[:product]).to eq Remote::Product.new(identifier: 'sles', version: '12', arch: 'i386') end it 'sets token options' do argv = %w{--regcode matoken} - cli = subject.new(argv) + cli = described_class.new(argv) expect(cli.options[:token]).to eq 'matoken' end it 'sets email options' do argv = %w{--email me@hotmail.com} - cli = subject.new(argv) + cli = described_class.new(argv) expect(cli.options[:email]).to eq 'me@hotmail.com' end it 'sets url options' do argv = %w{--url test} - cli = subject.new(argv) + cli = described_class.new(argv) expect(cli.options[:url]).to eq 'test' end it 'puts version on version flag' do argv = %w{--version} - expect_any_instance_of(subject).to receive(:puts).with(VERSION) - subject.new(argv) + expect_any_instance_of(described_class).to receive(:puts).with(VERSION) + described_class.new(argv) end it 'outputs help on help flag with no line longer than 80 characters' do argv = %w{--help} - expect_any_instance_of(subject).to receive(:puts) do |option_parser| + expect_any_instance_of(described_class).to receive(:puts) do |option_parser| expect(option_parser.instance_variable_get(:@opts).to_s.split("\n").map(&:length).max).to be <= 80 end - subject.new(argv) + described_class.new(argv) end it 'sets verbose options' do argv = %w{--debug} - cli = subject.new(argv) + cli = described_class.new(argv) expect(cli.options[:debug]).to be true end it 'sets deregister option' do argv = %w{--de-register} - cli = subject.new(argv) + cli = described_class.new(argv) expect(cli.options[:deregister]).to be true end it 'sets root option' do argv = %w{--root /path/to/root} - subject.new(argv) + described_class.new(argv) expect(SUSE::Connect::System.filesystem_root).to eq '/path/to/root' SUSE::Connect::System.filesystem_root = nil end it 'requests status sub-command' do argv = %w{--status} - expect(subject.new(argv).options[:status]).to be true + expect(described_class.new(argv).options[:status]).to be true end it 'sets write_config option' do argv = %w{--write-config} - cli = subject.new(argv) + cli = described_class.new(argv) expect(cli.options[:write_config]).to be true end end @@ -336,23 +347,22 @@ it 'error on invalid product options format with hint where to find correct product identifiers' do expect(string_logger).to receive(:error).with(/Please provide the product identifier in this format.*SUSEConnect --list-extensions/) argv = %w{--product sles} - subject.new(argv) + described_class.new(argv) end end describe '?check_if_param' do it 'will exit with message if opt is nil' do - expect_any_instance_of(subject).to receive(:exit) + expect_any_instance_of(described_class).to receive(:exit) expect(string_logger).to receive(:error).with('Kaboom') - subject.new({}).send(:check_if_param, nil, 'Kaboom') + cli.send(:check_if_param, nil, 'Kaboom') end end describe 'reads environment variables' do + before { ENV['LANG'] = 'de' } + it 'sets language header based on LANG' do - # is ENV global? - ENV['LANG'] = 'de' - cli = subject.new([]) expect(cli.options[:language]).to eq 'de' end end diff --git a/spec/connect/client_spec.rb b/spec/connect/client_spec.rb index 9e3f2300..cc934bc2 100644 --- a/spec/connect/client_spec.rb +++ b/spec/connect/client_spec.rb @@ -1,23 +1,21 @@ require 'spec_helper' describe SUSE::Connect::Client do - let :config do - SUSE::Connect::Config.new - end - - subject { SUSE::Connect::Client.new(config) } - + let(:config) { SUSE::Connect::Config.new } let(:default_logger) { SUSE::Connect::GlobalLogger.instance.log } let(:string_logger) { ::Logger.new(StringIO.new) } + let(:client_instance) { described_class.new config } + + subject { client_instance } describe '.new' do - context :empty_opts do + context 'empty opts' do it 'should set url to default_url' do expect(subject.config.url).to eq SUSE::Connect::Config::DEFAULT_URL end end - context :passed_opts do + context 'passed opts' do it 'should set insecure flag from options if it was passed via constructor' do config.insecure = true client = Client.new(config) @@ -37,7 +35,7 @@ end end - context :from_config do + context 'from config' do subject do SUSE::Connect::Client.new(SUSE::Connect::Config.new) end @@ -65,7 +63,7 @@ end describe '#announce_system' do - context :direct_connection do + context 'direct connection' do subject do config.token = 'blabla' SUSE::Connect::Client.new(config) @@ -109,7 +107,7 @@ end describe '#update_system' do - context :direct_connection do + context 'direct connection' do subject { SUSE::Connect::Client.new(config) } before do @@ -142,7 +140,7 @@ end end - context :registration_proxy_connection do + context 'registration proxy connection' do subject do config.url = 'http://smt.local' SUSE::Connect::Client.new(config) @@ -396,54 +394,43 @@ end describe '#deregister!' do - let(:stubbed_response) do - OpenStruct.new( - code: 204, - body: nil, - success: true - ) - end + let(:stubbed_response) { OpenStruct.new(code: 204, body: nil, success: true) } + subject { client_instance.deregister! } before { SUSE::Connect::GlobalLogger.instance.log = string_logger } context 'when system is registered' do before do - allow(subject).to receive_messages(system_auth: 'Basic: encodedstring') - allow(subject).to receive(:registered?).and_return true - allow(subject.api).to receive(:deregister).with('Basic: encodedstring').and_return stubbed_response + allow(client_instance).to receive_messages(system_auth: 'Basic: encodedstring') + allow(client_instance).to receive(:registered?).and_return true + allow(client_instance.api).to receive(:deregister).with('Basic: encodedstring').and_return stubbed_response allow(System).to receive(:cleanup!).and_return(true) end it 'calls underlying api and removes credentials file' do - expect(subject.api).to receive(:deregister).with('Basic: encodedstring').and_return stubbed_response - subject.deregister! + expect(client_instance.api).to receive(:deregister).with('Basic: encodedstring').and_return stubbed_response + subject end it 'cleans up system' do expect(System).to receive(:cleanup!).and_return(true) - subject.deregister! + subject end context 'when system is cleaned up' do - before do - allow(System).to receive(:cleanup!).and_return(true) - end + before { allow(System).to receive(:cleanup!).and_return(true) } it 'prints confirmation message' do expect(string_logger).to receive(:info).with('Successfully deregistered system.') - subject.deregister! + subject end end end context 'when system is not registered' do - before { allow(subject).to receive(:registered?).and_return false } + before { allow(::SUSE::Connect::System).to receive(:credentials).and_return(nil) } - it 'prints a warning' do - expect(string_logger).to receive(:fatal).with('Deregistration failed. Check if the system has been '\ - 'registered using the -s option or use the --regcode parameter to register it.') - subject.deregister! - end + it { expect { subject }.to raise_error(::SUSE::Connect::SystemNotRegisteredError) } end end diff --git a/spec/connect/status_spec.rb b/spec/connect/status_spec.rb index 0a3b867c..f7574b3a 100644 --- a/spec/connect/status_spec.rb +++ b/spec/connect/status_spec.rb @@ -128,6 +128,7 @@ describe '#print_extensions_list' do subject { -> { status_instance.print_extensions_list } } before do + allow(Zypper).to receive(:installed_products).and_return [] allow(Zypper).to receive(:base_product).and_return Zypper::Product.new(name: 'SLES', version: '12', arch: 'x86_64') allow(client_double).to receive(:show_product).with(Zypper.base_product).and_return(Remote::Product.new(dummy_product_data)) end @@ -149,7 +150,9 @@ context 'with activated module' do before do - allow(status_instance.client).to receive_message_chain(:system_activations, :body).and_return [{ 'service' => { 'product' => { identifier: 'sle-sdk', version: '12', arch: 'ppc64le' }} }] + allow(status_instance.client).to receive_message_chain(:system_activations, :body).and_return [{ 'service' => { + 'product' => { identifier: 'sle-sdk', version: '12', arch: 'ppc64le' } + } }] allow(SUSE::Connect::System).to receive(:credentials?).and_return true end @@ -159,6 +162,7 @@ describe '#available_system_extensions' do it 'returns a list of all available extensions on this system' do + allow(Zypper).to receive(:installed_products).and_return [] allow(Zypper).to receive(:base_product).and_return Zypper::Product.new(:name => 'SLES', :version => '12', :arch => 'x86_64') allow(client_double).to receive(:show_product).with(Zypper.base_product).and_return(Remote::Product.new(dummy_product_data)) expect(subject.available_system_extensions).to match_array([