From cc0758401940c1655dd75585462ea3b543cf71e1 Mon Sep 17 00:00:00 2001 From: Natnael Getahun Date: Tue, 2 Jan 2024 12:47:58 +0100 Subject: [PATCH 1/3] Refactoring: Moved suma product tree mirroring into a separate class --- lib/rmt/mirror/suma_product_tree.rb | 37 ++++++++++++++ spec/lib/rmt/mirror/suma_product_tree_spec.rb | 51 +++++++++++++++++++ 2 files changed, 88 insertions(+) create mode 100644 lib/rmt/mirror/suma_product_tree.rb create mode 100644 spec/lib/rmt/mirror/suma_product_tree_spec.rb diff --git a/lib/rmt/mirror/suma_product_tree.rb b/lib/rmt/mirror/suma_product_tree.rb new file mode 100644 index 000000000..cc0f45a9f --- /dev/null +++ b/lib/rmt/mirror/suma_product_tree.rb @@ -0,0 +1,37 @@ +class RMT::Mirror::SumaProductTree + FILE_URL = 'https://scc.suse.com/suma/'.freeze + + attr_reader :mirroring_base_dir, :url, :logger + + def initialize(logger:, mirroring_base_dir:, url: FILE_URL) + @mirroring_base_dir = mirroring_base_dir + @url = url + @logger = logger + end + + def downloader + @downloader ||= RMT::Downloader.new(logger: logger, track_files: false) + end + + def mirror + # NOTE: Incase we detect the default mirror path which ends with `public/repo`, + # we remove the `repo` sub-path since we expect the file to be stored in `public/` + # FIXME: refactor this into cli/mirror.rb + base_dir = mirroring_base_dir + base_dir = File.expand_path(File.join(mirroring_base_dir, '/../')) if mirroring_base_dir == RMT::DEFAULT_MIRROR_DIR + + dest = File.join(base_dir, '/suma/') + ref = RMT::Mirror::FileReference.new( + relative_path: 'product_tree.json', + base_url: url, + base_dir: dest, + cache_dir: nil + ) + + logger.info _('Mirroring SUSE Manager product tree to %{dir}') % { dir: dest } + downloader.download_multi([ref]) + rescue RMT::Downloader::Exception => e + raise RMT::Mirror::Exception.new(_('Could not mirror SUSE Manager product tree with error: %{error}') % { error: e.message }) + end + +end diff --git a/spec/lib/rmt/mirror/suma_product_tree_spec.rb b/spec/lib/rmt/mirror/suma_product_tree_spec.rb new file mode 100644 index 000000000..96627603c --- /dev/null +++ b/spec/lib/rmt/mirror/suma_product_tree_spec.rb @@ -0,0 +1,51 @@ +require 'rails_helper' + +RSpec.describe RMT::Mirror::SumaProductTree do + subject(:suma) { described_class.new(**configuration) } + + let(:configuration) do + { + logger: RMT::Logger.new('/dev/null'), + mirroring_base_dir: base_dir + } + end + let(:ref_configuration) do + { + relative_path: 'product_tree.json', + base_url: described_class::FILE_URL, + cache_dir: nil, + base_dir: File.join(base_dir, '/suma/') + } + end + + let(:base_dir) { '/tmp' } + let(:downloader) { instance_double RMT::Downloader } + + describe '#mirror' do + before do + allow(suma).to receive(:downloader).and_return downloader + end + + it 'mirrors the product_tree file' do + expect(RMT::Mirror::FileReference).to receive(:new).with(**ref_configuration) + expect(downloader).to receive(:download_multi) + suma.mirror + end + + it 'fails to mirror the product file' do + expect(downloader).to receive(:download_multi).and_raise(RMT::Downloader::Exception, 'Network issues') + expect { suma.mirror }.to raise_error(RMT::Mirror::Exception, /Could not mirror SUSE Manager/) + end + + context 'with default mirror dir' do + let(:updated_base_dir) { File.expand_path(File.join(RMT::DEFAULT_MIRROR_DIR, '/../')) } + let(:base_dir) { File.join(updated_base_dir, '/suma/') } + + it 'alters the default mirroring path' do + expect(RMT::Mirror::FileReference).to receive(:new).with(**ref_configuration) + expect(downloader).to receive(:download_multi) + suma.mirror + end + end + end +end From cde14b266db09b6198411a0e2e30de0a3da77702 Mon Sep 17 00:00:00 2001 From: Natnael Getahun Date: Tue, 2 Jan 2024 15:16:17 +0100 Subject: [PATCH 2/3] Rerfactor suma product tree mirroring in repomd and mirror classes --- lib/rmt/cli/mirror.rb | 6 ++++- lib/rmt/mirror/repomd.rb | 23 ----------------- spec/lib/rmt/cli/mirror_spec.rb | 18 ++++++------- spec/lib/rmt/mirror/repomd_spec.rb | 41 ------------------------------ 4 files changed, 14 insertions(+), 74 deletions(-) diff --git a/lib/rmt/cli/mirror.rb b/lib/rmt/cli/mirror.rb index aa3643b3a..dd1df9cec 100644 --- a/lib/rmt/cli/mirror.rb +++ b/lib/rmt/cli/mirror.rb @@ -5,7 +5,7 @@ class RMT::CLI::Mirror < RMT::CLI::Base def all RMT::Lockfile.lock('mirror') do begin - mirror.mirror_suma_product_tree(repository_url: 'https://scc.suse.com/suma/') + suma_product_tree.mirror rescue RMT::Mirror::Exception => e errors << _('Mirroring SUMA product tree failed: %{error_message}') % { error_message: e.message } end @@ -78,6 +78,10 @@ def product(*targets) protected + def suma_product_tree + RMT::Mirror::SumaProductTree.new(logger: logger, mirroring_base_dir: RMT::DEFAULT_MIRROR_DIR) + end + def mirror config = { logger: logger, diff --git a/lib/rmt/mirror/repomd.rb b/lib/rmt/mirror/repomd.rb index 199511de4..7ca2c0d04 100644 --- a/lib/rmt/mirror/repomd.rb +++ b/lib/rmt/mirror/repomd.rb @@ -18,29 +18,6 @@ def initialize(logger:, mirroring_base_dir: RMT::DEFAULT_MIRROR_DIR, mirror_src: @downloader = RMT::Downloader.new(logger: logger, track_files: !airgap_mode) end - def mirror_suma_product_tree(repository_url:) - # we have an inconsistency in how we mirror in offline mode - # in normal mode we mirror in the following way: - # base_dir/repo/... - # however, in offline mode we mirror in the following way - # base_dir/... - # we need this extra step to ensure that we write to the public directory - base_dir = mirroring_base_dir - base_dir = File.expand_path(File.join(mirroring_base_dir, '/../')) if mirroring_base_dir == RMT::DEFAULT_MIRROR_DIR - - repository_dir = File.join(base_dir, '/suma/') - mirroring_paths = { - base_url: URI.join(repository_url), - base_dir: repository_dir, - cache_dir: repository_dir - } - - logger.info _('Mirroring SUSE Manager product tree to %{dir}') % { dir: repository_dir } - downloader.download_multi([RMT::Mirror::FileReference.new(relative_path: 'product_tree.json', **mirroring_paths)]) - rescue RMT::Downloader::Exception => e - raise RMT::Mirror::Exception.new(_('Could not mirror SUSE Manager product tree with error: %{error}') % { error: e.message }) - end - def mirror(repository_url:, local_path:, auth_token: nil, repo_name: nil) repository_dir = File.join(mirroring_base_dir, local_path) diff --git a/spec/lib/rmt/cli/mirror_spec.rb b/spec/lib/rmt/cli/mirror_spec.rb index a49882ae6..ebcdadde0 100644 --- a/spec/lib/rmt/cli/mirror_spec.rb +++ b/spec/lib/rmt/cli/mirror_spec.rb @@ -26,8 +26,8 @@ let(:error_messages) { "Mirroring SUMA product tree failed: #{suma_error}." } it 'handles the exception and raises an error after mirroring all repos' do - expect_any_instance_of(RMT::Mirror::Repomd) - .to receive(:mirror_suma_product_tree) + expect_any_instance_of(RMT::Mirror::SumaProductTree) + .to receive(:mirror) .and_raise(RMT::Mirror::Exception, suma_error) expect_any_instance_of(RMT::Mirror::Repomd).to receive(:mirror) @@ -44,7 +44,7 @@ end it 'raises an error' do - expect_any_instance_of(RMT::Mirror::Repomd).to receive(:mirror_suma_product_tree) + expect_any_instance_of(RMT::Mirror::SumaProductTree).to receive(:mirror) expect_any_instance_of(RMT::Mirror::Repomd).not_to receive(:mirror) expect { command } @@ -58,7 +58,7 @@ let!(:repository) { create :repository, :with_products, mirroring_enabled: true } it 'updates repository mirroring timestamp' do - expect_any_instance_of(RMT::Mirror::Repomd).to receive(:mirror_suma_product_tree) + expect_any_instance_of(RMT::Mirror::SumaProductTree).to receive(:mirror) expect_any_instance_of(RMT::Mirror::Repomd).to receive(:mirror) Timecop.freeze(Time.utc(2018)) do @@ -79,7 +79,7 @@ let(:error_messages) { /Repository '#{repository.name}' \(#{repository.friendly_id}\): #{mirroring_error}\./ } it 'raises an error' do - expect_any_instance_of(RMT::Mirror::Repomd).to receive(:mirror_suma_product_tree) + expect_any_instance_of(RMT::Mirror::SumaProductTree).to receive(:mirror) expect { command } .to raise_error(SystemExit) { |e| expect(e.status).to eq(1) } @@ -115,7 +115,7 @@ let(:argv) { ['all', '--do-not-raise-unpublished'] } it 'log the warning and does not raise an error' do - allow_any_instance_of(RMT::Mirror::Repomd).to receive(:mirror_suma_product_tree) + allow_any_instance_of(RMT::Mirror::SumaProductTree).to receive(:mirror) expect { command }.to output(error_log).to_stdout end end @@ -126,7 +126,7 @@ let!(:additional_repository) { create :repository, :with_products, mirroring_enabled: false } it 'mirrors additional repositories' do - expect_any_instance_of(RMT::Mirror::Repomd).to receive(:mirror_suma_product_tree) + expect_any_instance_of(RMT::Mirror::SumaProductTree).to receive(:mirror) expect_any_instance_of(RMT::Mirror::Repomd).to receive(:mirror).with( repository_url: repository.external_url, local_path: anything, @@ -156,7 +156,7 @@ let(:error_messages) { /Repository '#{repository.name}' \(#{repository.friendly_id}\): #{mirroring_error}\./ } it 'handles exceptions and mirrors additional repositories' do - expect_any_instance_of(RMT::Mirror::Repomd).to receive(:mirror_suma_product_tree) + expect_any_instance_of(RMT::Mirror::SumaProductTree).to receive(:mirror) expect_any_instance_of(RMT::Mirror::Repomd).to receive(:mirror).with( repository_url: repository.external_url, local_path: anything, @@ -381,7 +381,7 @@ context 'mirror product using --do-not-raise-unpublished flag' do it 'log the warning and does not raise an error' do - allow_any_instance_of(RMT::Mirror::Repomd).to receive(:mirror_suma_product_tree) + allow_any_instance_of(RMT::Mirror::SumaProductTree).to receive(:mirror) expect { command }.to output(error_log).to_stdout end end diff --git a/spec/lib/rmt/mirror/repomd_spec.rb b/spec/lib/rmt/mirror/repomd_spec.rb index 2def94af4..bed3239b5 100644 --- a/spec/lib/rmt/mirror/repomd_spec.rb +++ b/spec/lib/rmt/mirror/repomd_spec.rb @@ -13,47 +13,6 @@ let(:logger) { RMT::Logger.new('/dev/null') } - describe '#mirror_suma_product_tree' do - subject(:command) { rmt_mirror.mirror_suma_product_tree(repository_url: 'https://scc.suse.com/suma/') } - - let(:rmt_mirror) do - described_class.new( - mirroring_base_dir: @tmp_dir, - logger: logger, - mirror_src: false - ) - end - - around do |example| - @tmp_dir = Dir.mktmpdir('rmt') - example.run - FileUtils.remove_entry(@tmp_dir) - end - - context 'all is well', vcr: { cassette_name: 'mirroring_suma_product_tree' } do - before do - expect(logger).to receive(:info).with(/Mirroring SUSE Manager product tree to/).once - expect(logger).to receive(:info).with(/↓ product_tree.json/).once - end - - it 'downloads the suma product tree' do - command - content = File.read(File.join(@tmp_dir, 'suma/product_tree.json')) - expect(Digest::SHA256.hexdigest(content)).to eq('7486026e9c1181affae5b21c9aa64637aa682fcdeacb099e213f0e8c7e86d85d') - end - end - - context 'with download exception' do - before do - expect_any_instance_of(RMT::Downloader).to receive(:download_multi).and_raise(RMT::Downloader::Exception, "418 - I'm a teapot") - end - - it 'raises mirroring exception' do - expect { command }.to raise_error(RMT::Mirror::Exception, "Could not mirror SUSE Manager product tree with error: 418 - I'm a teapot") - end - end - end - describe '#mirror' do around do |example| @tmp_dir = Dir.mktmpdir('rmt') From 41344af815b2d96ec946fd039628d90a3e363348 Mon Sep 17 00:00:00 2001 From: Natnael Getahun Date: Wed, 3 Jan 2024 13:53:53 +0100 Subject: [PATCH 3/3] Fix broken specs --- lib/rmt/cli/export.rb | 4 ++-- lib/rmt/cli/import.rb | 3 ++- spec/lib/rmt/cli/export_spec.rb | 12 +++++++----- spec/lib/rmt/cli/import_spec.rb | 10 +++++----- 4 files changed, 16 insertions(+), 13 deletions(-) diff --git a/lib/rmt/cli/export.rb b/lib/rmt/cli/export.rb index fc515cd28..836120bba 100644 --- a/lib/rmt/cli/export.rb +++ b/lib/rmt/cli/export.rb @@ -31,9 +31,9 @@ def repos(path) path = needs_path(path, writable: true) mirror = RMT::Mirror::Repomd.new(mirroring_base_dir: path, logger: logger, airgap_mode: true) - + suma_product_tree = RMT::Mirror::SumaProductTree.new(mirroring_base_dir: path, logger: logger) begin - mirror.mirror_suma_product_tree(repository_url: 'https://scc.suse.com/suma/') + suma_product_tree.mirror rescue RMT::Mirror::Exception => e logger.warn(e.message) end diff --git a/lib/rmt/cli/import.rb b/lib/rmt/cli/import.rb index 99cd09d73..b1884295e 100644 --- a/lib/rmt/cli/import.rb +++ b/lib/rmt/cli/import.rb @@ -21,7 +21,8 @@ def repos(path) begin exported_suma_path = File.join(path, '/suma/') suma_repo_url = URI.join('file://', exported_suma_path).to_s - mirror.mirror_suma_product_tree(repository_url: suma_repo_url) + suma_product_tree = RMT::Mirror::SumaProductTree.new(mirroring_base_dir: exported_suma_path, logger: logger, url: suma_repo_url) + suma_product_tree.mirror rescue RMT::Mirror::Exception => e logger.warn(e.message) end diff --git a/spec/lib/rmt/cli/export_spec.rb b/spec/lib/rmt/cli/export_spec.rb index 95a7520ea..a35e07c54 100644 --- a/spec/lib/rmt/cli/export_spec.rb +++ b/spec/lib/rmt/cli/export_spec.rb @@ -68,6 +68,7 @@ let(:command) { described_class.start(['repos', path]) } let(:mirror_double) { instance_double('RMT::Mirror::Repomd') } + let(:suma_product_tree_double) { instance_double('RMT::Mirror::SumaProductTree') } let(:repo_settings) do [ { url: 'http://foo.bar/repo1', auth_token: 'foobar' }, @@ -77,7 +78,7 @@ context 'suma product tree mirror with exception' do it 'outputs exception message' do - expect_any_instance_of(RMT::Mirror::Repomd).to receive(:mirror_suma_product_tree).and_raise(RMT::Mirror::Exception, 'black mirror') + expect_any_instance_of(RMT::Mirror::SumaProductTree).to receive(:mirror).and_raise(RMT::Mirror::Exception, 'black mirror') expect_any_instance_of(RMT::Mirror::Repomd).to receive(:mirror).twice FileUtils.mkdir_p path @@ -92,7 +93,7 @@ it 'outputs a warning' do FileUtils.mkdir_p path - expect_any_instance_of(RMT::Mirror::Repomd).to receive(:mirror_suma_product_tree) + expect_any_instance_of(RMT::Mirror::SumaProductTree).to receive(:mirror) expect { command }.to raise_error(SystemExit).and(output("#{File.join(path, 'repos.json')} does not exist.\n").to_stderr) end end @@ -112,7 +113,7 @@ FileUtils.mkdir_p path File.write('/mnt/usb/repos.json', repo_settings.to_json) - expect(mirror_double).to receive(:mirror_suma_product_tree) + expect_any_instance_of(RMT::Mirror::SumaProductTree).to receive(:mirror) expect(mirror_double).to receive(:mirror).with( repository_url: 'http://foo.bar/repo1', auth_token: 'foobar', @@ -142,7 +143,7 @@ FileUtils.mkdir_p path File.write("#{path}/repos.json", repo_settings.to_json) - expect(mirror_double).to receive(:mirror_suma_product_tree) + expect_any_instance_of(RMT::Mirror::SumaProductTree).to receive(:mirror) expect(mirror_double).to receive(:mirror).with( repository_url: 'http://foo.bar/repo1', auth_token: 'foobar', @@ -163,7 +164,8 @@ FileUtils.mkdir_p path File.write("#{path}/repos.json", repo_settings.to_json) - expect(mirror_double).to receive(:mirror_suma_product_tree).with({ repository_url: 'https://scc.suse.com/suma/' }) + + expect_any_instance_of(RMT::Mirror::SumaProductTree).to receive(:mirror) expect(mirror_double).to receive(:mirror).with( repository_url: 'http://foo.bar/repo1', auth_token: 'foobar', diff --git a/spec/lib/rmt/cli/import_spec.rb b/spec/lib/rmt/cli/import_spec.rb index e3fe88f00..2ec1f0ce7 100644 --- a/spec/lib/rmt/cli/import_spec.rb +++ b/spec/lib/rmt/cli/import_spec.rb @@ -64,7 +64,7 @@ airgap_mode: true ).and_return(mirror_double) - expect(mirror_double).to receive(:mirror_suma_product_tree) + expect_any_instance_of(RMT::Mirror::SumaProductTree).to receive(:mirror) expect(mirror_double).to receive(:mirror).with( repository_url: repo1_local_path, local_path: Repository.make_local_path(repo1.external_url), @@ -96,7 +96,7 @@ airgap_mode: true ).and_return(mirror_double) - expect(mirror_double).to receive(:mirror_suma_product_tree) + expect_any_instance_of(RMT::Mirror::SumaProductTree).to receive(:mirror) expect(mirror_double).to receive(:mirror).with( repository_url: repo1_local_path, local_path: Repository.make_local_path(repo1.external_url), @@ -124,7 +124,7 @@ context 'suma product tree mirror with exception' do it 'outputs exception message' do - expect_any_instance_of(RMT::Mirror::Repomd).to receive(:mirror_suma_product_tree).and_raise(RMT::Mirror::Exception, 'black mirror') + expect_any_instance_of(RMT::Mirror::SumaProductTree).to receive(:mirror).and_raise(RMT::Mirror::Exception, 'black mirror') expect_any_instance_of(RMT::Mirror::Repomd).to receive(:mirror).twice FileUtils.mkdir_p path @@ -148,7 +148,7 @@ FileUtils.mkdir_p path File.write("#{path}/repos.json", repo_settings.to_json) - expect_any_instance_of(RMT::Mirror::Repomd).to receive(:mirror_suma_product_tree) + expect_any_instance_of(RMT::Mirror::SumaProductTree).to receive(:mirror) expect { command }.to output(/repository by URL #{missing_repo_url} does not exist in database/).to_stderr.and output('').to_stdout end end @@ -170,7 +170,7 @@ airgap_mode: true ).and_return(mirror_double) - expect(mirror_double).to receive(:mirror_suma_product_tree) + expect_any_instance_of(RMT::Mirror::SumaProductTree).to receive(:mirror) expect(mirror_double).to receive(:mirror).with( repository_url: repo1_local_path, local_path: Repository.make_local_path(repo1.external_url),