From 06452b5f349194d7aad319f40f04b85f9af57541 Mon Sep 17 00:00:00 2001 From: Samir Jha Date: Mon, 27 Jan 2025 15:24:39 -0500 Subject: [PATCH] Fixes #38040 - Clear retain_package_version_count when mirroring policy is not additive (#11285) --- .../katello/api/v2/repositories_controller.rb | 7 +++++++ app/models/katello/root_repository.rb | 3 +++ test/actions/katello/repository_test.rb | 13 +++++++++++-- .../api/v2/repositories_controller_test.rb | 8 +++++++- test/models/root_repository_test.rb | 7 +++++++ 5 files changed, 35 insertions(+), 3 deletions(-) diff --git a/app/controllers/katello/api/v2/repositories_controller.rb b/app/controllers/katello/api/v2/repositories_controller.rb index 47b5350afb2..b2ab2e08c6d 100644 --- a/app/controllers/katello/api/v2/repositories_controller.rb +++ b/app/controllers/katello/api/v2/repositories_controller.rb @@ -403,6 +403,13 @@ def update end end + additive_policy = ::Katello::RootRepository::MIRRORING_POLICY_ADDITIVE + is_repo_param_additive = repo_params[:mirroring_policy] == additive_policy + is_root_additive = @repository.root.mirroring_policy == additive_policy + + if @repository.yum? && !(is_repo_param_additive || is_root_additive) + repo_params[:retain_package_versions_count] = nil + end sync_task(::Actions::Katello::Repository::Update, @repository.root, repo_params) respond_for_show(:resource => @repository) end diff --git a/app/models/katello/root_repository.rb b/app/models/katello/root_repository.rb index 15906f3554b..afe9b042e2f 100644 --- a/app/models/katello/root_repository.rb +++ b/app/models/katello/root_repository.rb @@ -338,6 +338,9 @@ def ensure_valid_retain_package_versions_count unless yum? errors.add(:retain_package_versions_count, N_("is only allowed for Yum repositories.")) end + if yum? && self.mirroring_policy != MIRRORING_POLICY_ADDITIVE + errors.add(:retain_package_versions_count, N_("cannot be set for repositories without 'Additive' mirroring policy.")) + end if self.retain_package_versions_count.to_i < 0 errors.add(:retain_package_versions_count, N_("must not be a negative value.")) end diff --git a/test/actions/katello/repository_test.rb b/test/actions/katello/repository_test.rb index 5de71b4ae8e..0d1685780d1 100644 --- a/test/actions/katello/repository_test.rb +++ b/test/actions/katello/repository_test.rb @@ -141,11 +141,20 @@ def setup assert_action_planed action, candlepin_action_class end - it 'plans pulp3 update when retain_package_version is updated' do + it 'throws error on pulp3 update when retain_package_version is updated without additive' do action = create_action action_class action.stubs(:action_subject).with(repository) + error = assert_raises(ActiveRecord::RecordInvalid) do + plan_action action, repository.root, :retain_package_versions_count => 17 + end + assert_match(/Retain package versions count cannot be set for repositories without 'Additive' mirroring policy/, error.message) + end - plan_action action, repository.root, :retain_package_versions_count => 17 + it 'plans pulp3 update when retain_package_version is updated with additive' do + action = create_action action_class + action.stubs(:action_subject).with(repository) + repo_params = { retain_package_versions_count: 17, mirroring_policy: ::Katello::RootRepository::MIRRORING_POLICY_ADDITIVE } + plan_action action, repository.root, repo_params assert_action_planed action, pulp3_action_class end diff --git a/test/controllers/api/v2/repositories_controller_test.rb b/test/controllers/api/v2/repositories_controller_test.rb index 7c704194b59..60c8c6875cd 100644 --- a/test/controllers/api/v2/repositories_controller_test.rb +++ b/test/controllers/api/v2/repositories_controller_test.rb @@ -545,6 +545,7 @@ def test_show_protected_specific_instance end def test_update_with_gpg_key + @repository.root.update!(mirroring_policy: Katello::RootRepository::MIRRORING_POLICY_ADDITIVE) key = ContentCredential.find(katello_gpg_keys('fedora_gpg_key').id) assert_sync_task(::Actions::Katello::Repository::Update) do |root, attributes| assert_equal root, @repository.root @@ -557,6 +558,7 @@ def test_update_with_gpg_key end def test_update_with_cert + @repository.root.update!(mirroring_policy: Katello::RootRepository::MIRRORING_POLICY_ADDITIVE) cert = ContentCredential.find(katello_gpg_keys('fedora_cert').id) assert_sync_task(::Actions::Katello::Repository::Update) do |root, attributes| assert_equal root, @repository.root @@ -657,8 +659,11 @@ def test_update_with_retain_package_versions_count repo = katello_repositories(:fedora_17_unpublished) assert_sync_task(::Actions::Katello::Repository::Update) do |_, attributes| assert_equal attributes[:retain_package_versions_count], retain_package_versions_count + assert_equal attributes[:mirroring_policy], ::Katello::RootRepository::MIRRORING_POLICY_ADDITIVE end - put :update, params: { :id => repo.id, :retain_package_versions_count => retain_package_versions_count } + put :update, params: { :id => repo.id, + :retain_package_versions_count => retain_package_versions_count, + :mirroring_policy => ::Katello::RootRepository::MIRRORING_POLICY_ADDITIVE } end def test_update_with_limit_tags @@ -675,6 +680,7 @@ def test_update_with_limit_tags end def test_update_non_docker_repo_with_limit_tags + @repository.root.update!(mirroring_policy: Katello::RootRepository::MIRRORING_POLICY_ADDITIVE) assert_sync_task(::Actions::Katello::Repository::Update) do |root, attributes| assert_equal root, @repository.root expected = { 'name' => 'new name' } diff --git a/test/models/root_repository_test.rb b/test/models/root_repository_test.rb index c89fea349f4..686d0f84e51 100644 --- a/test/models/root_repository_test.rb +++ b/test/models/root_repository_test.rb @@ -59,6 +59,7 @@ def test_invalid_retain_package_version_counts @root.content_type = 'yum' @root.url = 'http://inecas.fedorapeople.org/fakerepos/zoo2/' @root.retain_package_versions_count = -2 + @root.mirroring_policy = 'additive' assert_not_valid @root assert_equal @root.errors[:retain_package_versions_count], ["must not be a negative value."] end @@ -67,6 +68,7 @@ def test_valid_retain_package_version_counts @root.content_type = 'yum' @root.url = 'http://inecas.fedorapeople.org/fakerepos/zoo2/' @root.retain_package_versions_count = 2 + @root.mirroring_policy = 'additive' assert_valid @root end @@ -582,7 +584,12 @@ def test_yum_retain_package_versions_count assert @root.valid? @root.retain_package_versions_count = 2 + @root.mirroring_policy = 'additive' assert @root.valid? + + @root.mirroring_policy = 'mirror_content_only' + refute @root.valid? + assert_equal @root.errors[:retain_package_versions_count], ["cannot be set for repositories without 'Additive' mirroring policy."] end def test_sha1_checksum_removed