Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

As a user, I can remove all container content with ["*"] #11

Merged
merged 1 commit into from
Nov 20, 2019
Merged

As a user, I can remove all container content with ["*"] #11

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES/5756.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
As a user, I can remove all repository container content with ["*"]
2 changes: 1 addition & 1 deletion docs/_static/api.json
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 4 additions & 0 deletions docs/workflows/manage-content.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,10 @@ New Repository Version::
"number": 2
}

.. note::

Users can remove all content from the repo by specifying '*' in the content_units

Reference: `Container Recursive Remove Usage <../restapi.html#tag/container:-recursive-remove>`_

.. _tag-copy:
Expand Down
13 changes: 13 additions & 0 deletions pulp_container/app/serializers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,19 @@ class RecursiveManageSerializer(serializers.Serializer):
required=False
)

def validate(self, data):
"""
Validate data passed through a request call.
"""
content_units = data.get('content_units', None)
if content_units:
if '*' in content_units and len(content_units) > 1:
raise serializers.ValidationError(
_("'*' should be the only item present in the {}"
.format(content_units))
)
return data


class CopySerializer(serializers.Serializer):
"""
Expand Down
131 changes: 68 additions & 63 deletions pulp_container/app/tasks/recursive_remove.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,66 +32,71 @@ def recursive_remove_content(repository_pk, content_units):
repository = ContainerRepository.objects.get(pk=repository_pk)
latest_version = repository.latest_version()
latest_content = latest_version.content.all() if latest_version else Content.objects.none()

tags_in_repo = Q(pk__in=latest_content.filter(pulp_type='container.tag'))
manifests_in_repo = Q(pk__in=latest_content.filter(pulp_type='container.manifest'))
user_provided_content = Q(pk__in=content_units)
type_manifest_list = Q(media_type=MEDIA_TYPE.MANIFEST_LIST)
type_manifest = Q(media_type__in=[MEDIA_TYPE.MANIFEST_V1, MEDIA_TYPE.MANIFEST_V2])
blobs_in_repo = Q(pk__in=latest_content.filter(pulp_type='container.blob'))

# Tags do not have must_remain because they are the highest level content.
tags_to_remove = Tag.objects.filter(user_provided_content & tags_in_repo)
tags_to_remain = Tag.objects.filter(tags_in_repo).exclude(pk__in=tags_to_remove)
tagged_manifests_must_remain = Q(
pk__in=tags_to_remain.values_list("tagged_manifest", flat=True)
)
tagged_manifests_to_remove = Q(pk__in=tags_to_remove.values_list("tagged_manifest", flat=True))

manifest_lists_must_remain = Manifest.objects.filter(
manifests_in_repo & tagged_manifests_must_remain & type_manifest_list
)
manifest_lists_to_remove = Manifest.objects.filter(
user_provided_content | tagged_manifests_to_remove
).filter(
type_manifest_list & manifests_in_repo
).exclude(pk__in=manifest_lists_must_remain)

manifest_lists_to_remain = Manifest.objects.filter(
manifests_in_repo & type_manifest_list
).exclude(pk__in=manifest_lists_to_remove)

listed_manifests_must_remain = Q(
pk__in=manifest_lists_to_remain.values_list('listed_manifests', flat=True)
)
manifests_must_remain = Manifest.objects.filter(
tagged_manifests_must_remain | listed_manifests_must_remain
).filter(type_manifest & manifests_in_repo)

listed_manifests_to_remove = Q(
pk__in=manifest_lists_to_remove.values_list('listed_manifests', flat=True)
)
manifests_to_remove = Manifest.objects.filter(
user_provided_content | listed_manifests_to_remove | tagged_manifests_to_remove
).filter(type_manifest & manifests_in_repo).exclude(pk__in=manifests_must_remain)

manifests_to_remain = Manifest.objects.filter(
manifests_in_repo & type_manifest
).exclude(pk__in=manifests_to_remove)

listed_blobs_must_remain = Q(
pk__in=manifests_to_remain.values_list('blobs', flat=True)
) | Q(pk__in=manifests_to_remain.values_list('config_blob', flat=True))
listed_blobs_to_remove = Q(
pk__in=manifests_to_remove.values_list('blobs', flat=True)
) | Q(pk__in=manifests_to_remove.values_list('config_blob', flat=True))

blobs_to_remove = Blob.objects.filter(
user_provided_content | listed_blobs_to_remove
).filter(blobs_in_repo).exclude(listed_blobs_must_remain)

with repository.new_version() as new_version:
new_version.remove_content(tags_to_remove)
new_version.remove_content(manifest_lists_to_remove)
new_version.remove_content(manifests_to_remove)
new_version.remove_content(blobs_to_remove)
if '*' in content_units:
with repository.new_version() as new_version:
new_version.remove_content(latest_content)
else:
tags_in_repo = Q(pk__in=latest_content.filter(pulp_type='container.tag'))
manifests_in_repo = Q(pk__in=latest_content.filter(pulp_type='container.manifest'))
user_provided_content = Q(pk__in=content_units)
type_manifest_list = Q(media_type=MEDIA_TYPE.MANIFEST_LIST)
type_manifest = Q(media_type__in=[MEDIA_TYPE.MANIFEST_V1, MEDIA_TYPE.MANIFEST_V2])
blobs_in_repo = Q(pk__in=latest_content.filter(pulp_type='container.blob'))

# Tags do not have must_remain because they are the highest level content.
tags_to_remove = Tag.objects.filter(user_provided_content & tags_in_repo)
tags_to_remain = Tag.objects.filter(tags_in_repo).exclude(pk__in=tags_to_remove)
tagged_manifests_must_remain = Q(
pk__in=tags_to_remain.values_list("tagged_manifest", flat=True)
)
tagged_manifests_to_remove = Q(
pk__in=tags_to_remove.values_list("tagged_manifest", flat=True)
)

manifest_lists_must_remain = Manifest.objects.filter(
manifests_in_repo & tagged_manifests_must_remain & type_manifest_list
)
manifest_lists_to_remove = Manifest.objects.filter(
user_provided_content | tagged_manifests_to_remove
).filter(
type_manifest_list & manifests_in_repo
).exclude(pk__in=manifest_lists_must_remain)

manifest_lists_to_remain = Manifest.objects.filter(
manifests_in_repo & type_manifest_list
).exclude(pk__in=manifest_lists_to_remove)

listed_manifests_must_remain = Q(
pk__in=manifest_lists_to_remain.values_list('listed_manifests', flat=True)
)
manifests_must_remain = Manifest.objects.filter(
tagged_manifests_must_remain | listed_manifests_must_remain
).filter(type_manifest & manifests_in_repo)

listed_manifests_to_remove = Q(
pk__in=manifest_lists_to_remove.values_list('listed_manifests', flat=True)
)
manifests_to_remove = Manifest.objects.filter(
user_provided_content | listed_manifests_to_remove | tagged_manifests_to_remove
).filter(type_manifest & manifests_in_repo).exclude(pk__in=manifests_must_remain)

manifests_to_remain = Manifest.objects.filter(
manifests_in_repo & type_manifest
).exclude(pk__in=manifests_to_remove)

listed_blobs_must_remain = Q(
pk__in=manifests_to_remain.values_list('blobs', flat=True)
) | Q(pk__in=manifests_to_remain.values_list('config_blob', flat=True))
listed_blobs_to_remove = Q(
pk__in=manifests_to_remove.values_list('blobs', flat=True)
) | Q(pk__in=manifests_to_remove.values_list('config_blob', flat=True))

blobs_to_remove = Blob.objects.filter(
user_provided_content | listed_blobs_to_remove
).filter(blobs_in_repo).exclude(listed_blobs_must_remain)

with repository.new_version() as new_version:
new_version.remove_content(tags_to_remove)
new_version.remove_content(manifest_lists_to_remove)
new_version.remove_content(manifests_to_remove)
new_version.remove_content(blobs_to_remove)
4 changes: 4 additions & 0 deletions pulp_container/app/viewsets.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,10 @@ def remove(self, request, pk):

if 'content_units' in request.data:
for url in request.data['content_units']:
if url == '*':
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better if we required '*' to be the only URL and not just one of the URLs. What do you think?

Copy link
Member Author

@ipanova ipanova Nov 20, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

can we probably do this, on the other hand if we see [ 'href, 'href', '*', 'href'] i guess we can still treat this as 'remove all'
shall we raise a validation error in case '*' is not the only provided data in the list?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, let's raise a validation error if '*' is not the only item in the list.

remove_content_units = [url]
break

content = NamedModelViewSet.get_resource(url, Content)
remove_content_units.append(content.pk)

Expand Down
37 changes: 37 additions & 0 deletions pulp_container/tests/functional/api/test_recursive_remove.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

from pulp_smash import api, config
from pulp_smash.pulp3.utils import gen_repo, sync
from requests.exceptions import HTTPError

from pulp_container.tests.functional.constants import (
CONTAINER_TAG_PATH,
Expand Down Expand Up @@ -53,6 +54,42 @@ def test_repository_only_no_latest_version(self):
latest_version_href = self.client.get(self.to_repo['pulp_href'])['latest_version_href']
self.assertIsNone(latest_version_href)

def test_remove_everything(self):
"""Add a manifest and its related blobs."""
manifest_a = self.client.get("{unit_path}?{filters}".format(
unit_path=CONTAINER_TAG_PATH,
filters="name=manifest_a&{v_filter}".format(v_filter=self.latest_from_version),
))['results'][0]['tagged_manifest']
self.client.post(
self.CONTAINER_RECURSIVE_ADD_PATH,
{'content_units': [manifest_a]})
latest_version_href = self.client.get(self.to_repo['pulp_href'])['latest_version_href']
latest = self.client.get(latest_version_href)

# Ensure test begins in the correct state
self.assertFalse('container.tag' in latest['content_summary']['added'])
self.assertEqual(latest['content_summary']['added']['container.manifest']['count'], 1)
self.assertEqual(latest['content_summary']['added']['container.blob']['count'], 2)

# Actual test
self.client.post(
self.CONTAINER_RECURSIVE_REMOVE_PATH,
{'content_units': ["*"]})
latest_version_href = self.client.get(self.to_repo['pulp_href'])['latest_version_href']
latest = self.client.get(latest_version_href)
self.assertEqual(latest['content_summary']['present'], {})
self.assertEqual(latest['content_summary']['removed']['container.blob']['count'], 2)
self.assertEqual(latest['content_summary']['removed']['container.manifest']['count'], 1)

def test_remove_invalid_content_units(self):
"""Ensure exception is raised when '*' is not the only item in the content_units."""
with self.assertRaises(HTTPError) as context:
self.client.post(
self.CONTAINER_RECURSIVE_REMOVE_PATH,
{'content_units': ["*", "some_href"]}
)
self.assertEqual(context.exception.response.status_code, 400)

def test_manifest_recursion(self):
"""Add a manifest and its related blobs."""
manifest_a = self.client.get("{unit_path}?{filters}".format(
Expand Down