Skip to content

Commit

Permalink
fixes #11436 - update repo destroy action to check if repo is deletab…
Browse files Browse the repository at this point in the history
…le, in plan phase

This small commit is to ensure that if a user attempts to delete a repository
(e.g. as a result of 'hammer repository-set disable') that the request
is immediately denied in the action plan phase; otherwise, other backend
actions (e.g. pulp/candlepin) will get executed on a repo that is not
deletable.
  • Loading branch information
bbuckingham committed Aug 21, 2015
1 parent 617448e commit 501e428
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 13 deletions.
7 changes: 7 additions & 0 deletions app/lib/actions/katello/repository/destroy.rb
Expand Up @@ -12,6 +12,13 @@ def plan(repository, options = {})

skip_environment_update = options.fetch(:organization_destroy, false)
action_subject(repository)

if !planned_destroy && !repository.assert_deletable
# The repository is going to be deleted in finalize, but it cannot be deleted.
# Stop now and inform the user.
fail repository.errors.messages.values.join("\n")
end

plan_action(ContentViewPuppetModule::Destroy, repository) if repository.puppet?
plan_action(Pulp::Repository::Destroy, pulp_id: repository.pulp_id)
plan_action(Product::ContentDestroy, repository)
Expand Down
24 changes: 12 additions & 12 deletions app/models/katello/repository.rb
Expand Up @@ -490,18 +490,6 @@ def remove_db_units(units)
end
end

protected

def removable_unit_association
if yum?
self.rpms
elsif docker?
self.docker_images
else
fail "Content type not supported for removal"
end
end

def assert_deletable
if self.environment.try(:library?) && self.content_view.default?
if self.environment.organization.being_deleted?
Expand All @@ -519,6 +507,18 @@ def assert_deletable
end
end

protected

def removable_unit_association
if yum?
self.rpms
elsif docker?
self.docker_images
else
fail "Content type not supported for removal"
end
end

def downcase_pulp_id
# Docker doesn't support uppercase letters in repository names. Since the pulp_id
# is currently being used for the name, it will be downcased for this content type.
Expand Down
11 changes: 10 additions & 1 deletion test/actions/katello/repository_test.rb
Expand Up @@ -49,15 +49,24 @@ class DestroyTest < TestBase
let(:action_class) { ::Actions::Katello::Repository::Destroy }
let(:pulp_action_class) { ::Actions::Pulp::Repository::Destroy }
let(:unpublished_repository) { katello_repositories(:fedora_17_unpublished) }
let(:published_repository) { katello_repositories(:rhel_6_x86_64) }

it 'plans' do
action = create_action action_class
action = create_action action_class
action.stubs(:action_subject).with(unpublished_repository)
action.expects(:plan_self)
plan_action action, unpublished_repository
assert_action_planed_with action, pulp_action_class, pulp_id: unpublished_repository.pulp_id
assert_action_planed_with action, ::Actions::Katello::Product::ContentDestroy, unpublished_repository
end

it "plan fails if repository is not deletable" do
action = create_action action_class
action.stubs(:action_subject).with(published_repository)
assert_raises(RuntimeError) do
plan_action action, published_repository
end
end
end

class DyscoverTest < TestBase
Expand Down

0 comments on commit 501e428

Please sign in to comment.