Skip to content

Commit

Permalink
Write Config Even During Failure Conditions
Browse files Browse the repository at this point in the history
We noticed a problem in RMT that came from a failure logic in
SUSEConnect.

* A server is registered against RMT with SUSEConnect, but that product
  isn't mirrored yet in RMT.
* As the Connect API announce endpoint does not verify that a product is
  mirrored, RMT has no idea that a subsequent call will try to activate
  an unmirrored product.
* The server is announced to RMT, so RMT gave it credentials which are
  saved on the server.
* During the activation phase, RMT tells the server it can't activate,
  so the registration process in SUSEConenct errors and the config isn't
  written.

The problem is that SUSEConnect already got the credentials from the
RMT server, but because it errored, it didn't save /etc/SUSEConnect to
know where those credentials are valid. Instead, we should save
/etc/SUSEConnect, as the system is valid in RMT, it's activation just
isn't and can be fixed without a deregistration.
  • Loading branch information
tmuntaner committed Aug 28, 2020
1 parent 1c8c9c6 commit fff4488
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 2 deletions.
4 changes: 2 additions & 2 deletions lib/suse/connect/cli.rb
Expand Up @@ -49,8 +49,6 @@ def execute! # rubocop:disable MethodLength, CyclomaticComplexity, PerceivedComp
Client.new(@config).register!
end
end

@config.write! if @config.write_config
rescue Errno::ECONNREFUSED
log.fatal "Error: Connection refused by server #{@config.url}"
exit 64
Expand Down Expand Up @@ -84,6 +82,8 @@ def execute! # rubocop:disable MethodLength, CyclomaticComplexity, PerceivedComp
rescue BaseProductDeactivationError
log.fatal 'Can not deregister base product. Use SUSEConnect -d to deactivate the whole system.'
exit 70
ensure
@config.write! if @config.write_config
end

private
Expand Down
1 change: 1 addition & 0 deletions lib/suse/connect/client.rb
Expand Up @@ -104,6 +104,7 @@ def announce_system(distro_target = nil, instance_data_file = nil)
params.push(@config.namespace) if @config.namespace

response = @api.announce_system(*params)

[response.body['login'], response.body['password']]
end

Expand Down
13 changes: 13 additions & 0 deletions spec/connect/cli_spec.rb
Expand Up @@ -169,6 +169,7 @@
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')
expect_any_instance_of(SUSE::Connect::Config).to receive(:write!)
expect { cli.execute! }.to raise_error(SystemExit)
end

Expand All @@ -179,6 +180,18 @@
expect_any_instance_of(SUSE::Connect::Config).to receive(:write!)
cli.execute!
end

it 'writes config even when exceptions are raised' do
cli = described_class.new(%w[--url http://foo.test.com])
expect(cli.config.write_config).to eq true

response = double(code: 401, body: { 'localized_error' => 'Invalid foo' })
allow(System).to receive(:credentials?).and_return true
allow_any_instance_of(SUSE::Connect::Client).to receive(:register!).and_raise(ApiError.new(response))

expect_any_instance_of(SUSE::Connect::Config).to receive(:write!)
expect { cli.execute! }.to raise_error(SystemExit)
end
end

describe 'de-register command' do
Expand Down

0 comments on commit fff4488

Please sign in to comment.