From 6c0389f267577613f4ab6f0a55405d6926df061a Mon Sep 17 00:00:00 2001 From: Matthias Dellweg Date: Wed, 30 Aug 2023 23:45:59 +0200 Subject: [PATCH] Add labels api fixes #3332 --- CHANGES/3332.feature | 2 + CHANGES/plugin_api/3332.feature | 3 + .../concepts/rbac/access_policy.rst | 4 +- pulpcore/app/serializers/__init__.py | 4 +- pulpcore/app/serializers/base.py | 30 +++++++++ pulpcore/app/viewsets/__init__.py | 1 + pulpcore/app/viewsets/base.py | 45 ++++++++++++- pulpcore/app/viewsets/publication.py | 2 + pulpcore/app/viewsets/repository.py | 4 +- pulpcore/plugin/viewsets/__init__.py | 1 + .../api/using_plugin/test_labels.py | 63 +++++++++++++++++++ 11 files changed, 154 insertions(+), 5 deletions(-) create mode 100644 CHANGES/3332.feature create mode 100644 CHANGES/plugin_api/3332.feature create mode 100644 pulpcore/tests/functional/api/using_plugin/test_labels.py diff --git a/CHANGES/3332.feature b/CHANGES/3332.feature new file mode 100644 index 0000000000..5b07ade74c --- /dev/null +++ b/CHANGES/3332.feature @@ -0,0 +1,2 @@ +Added ``set_label`` and ``unset_label`` endpoints to allow manipulating individual labels +synchronously. diff --git a/CHANGES/plugin_api/3332.feature b/CHANGES/plugin_api/3332.feature new file mode 100644 index 0000000000..524de9d883 --- /dev/null +++ b/CHANGES/plugin_api/3332.feature @@ -0,0 +1,3 @@ +Added a ``LabelsMixin`` for views to allow syncronous manipulation of labels on existing objects. +Repository, remote and distribution views inherit this from pulpcore, but default access policies +need to be adjusted. diff --git a/docs/plugin_dev/plugin-writer/concepts/rbac/access_policy.rst b/docs/plugin_dev/plugin-writer/concepts/rbac/access_policy.rst index bf070f8fc7..77d4d4fc08 100644 --- a/docs/plugin_dev/plugin-writer/concepts/rbac/access_policy.rst +++ b/docs/plugin_dev/plugin-writer/concepts/rbac/access_policy.rst @@ -32,7 +32,7 @@ Below is an example policy used by ``FileRemote``, with an explanation of its ef "condition": "has_model_or_domain_or_obj_perms:file.view_fileremote", }, { - "action": ["update", "partial_update"], + "action": ["update", "partial_update", "set_label", "unset_label"], "principal": "authenticated", "effect": "allow", "condition": "has_model_or_domain_or_obj_perms:file.change_fileremote", @@ -167,7 +167,7 @@ Here's an example of code to define a default policy: "condition": "has_model_or_domain_or_obj_perms:file.view_fileremote", }, { - "action": ["update", "partial_update"], + "action": ["update", "partial_update", "set_label", "unset_label"], "principal": "authenticated", "effect": "allow", "condition": "has_model_or_domain_or_obj_perms:file.change_fileremote", diff --git a/pulpcore/app/serializers/__init__.py b/pulpcore/app/serializers/__init__.py index 81884f2df6..cab545ef48 100644 --- a/pulpcore/app/serializers/__init__.py +++ b/pulpcore/app/serializers/__init__.py @@ -8,16 +8,18 @@ DetailRelatedField, DomainUniqueValidator, GetOrCreateSerializerMixin, + HiddenFieldsMixin, IdentityField, ModelSerializer, NestedIdentityField, NestedRelatedField, RelatedField, RelatedResourceField, + SetLabelSerializer, TaskGroupOperationResponseSerializer, + UnsetLabelSerializer, ValidateFieldsMixin, validate_unknown_fields, - HiddenFieldsMixin, ) from .fields import ( BaseURLField, diff --git a/pulpcore/app/serializers/base.py b/pulpcore/app/serializers/base.py index f842328f12..25eeaf09f8 100644 --- a/pulpcore/app/serializers/base.py +++ b/pulpcore/app/serializers/base.py @@ -504,3 +504,33 @@ class TaskGroupOperationResponseSerializer(serializers.Serializer): view_name="task-groups-detail", allow_null=False, ) + + +class SetLabelSerializer(serializers.Serializer): + """ + Serializer for synchronously setting a label. + """ + + key = serializers.SlugField(required=True) + value = serializers.CharField(required=True, allow_null=True, allow_blank=True) + + +class UnsetLabelSerializer(serializers.Serializer): + """ + Serializer for synchronously setting a label. + """ + + key = serializers.SlugField(required=True) + value = serializers.CharField(read_only=True) + + def validate_key(self, value): + if value not in self.context["content_object"].pulp_labels: + raise serializers.ValidationError( + _("Label '{key}' is not set on the object.").format(key=value) + ) + return value + + def validate(self, data): + data = super().validate(data) + data["value"] = self.context["content_object"].pulp_labels[data["key"]] + return data diff --git a/pulpcore/app/viewsets/__init__.py b/pulpcore/app/viewsets/__init__.py index 6d17c93fc8..6f017e6a03 100644 --- a/pulpcore/app/viewsets/__init__.py +++ b/pulpcore/app/viewsets/__init__.py @@ -2,6 +2,7 @@ AsyncCreateMixin, AsyncRemoveMixin, AsyncUpdateMixin, + LabelsMixin, NamedModelViewSet, RolesMixin, NAME_FILTER_OPTIONS, diff --git a/pulpcore/app/viewsets/base.py b/pulpcore/app/viewsets/base.py index 017cdcd419..9dd3aa40a4 100644 --- a/pulpcore/app/viewsets/base.py +++ b/pulpcore/app/viewsets/base.py @@ -4,6 +4,7 @@ from django.conf import settings from django.db import transaction +from django.db.models.expressions import RawSQL from django.core.exceptions import FieldError, ValidationError from django.urls import Resolver404, resolve from django.contrib.contenttypes.models import ContentType @@ -20,7 +21,12 @@ from pulpcore.app.models.role import GroupRole, UserRole from pulpcore.app.response import OperationPostponedResponse from pulpcore.app.role_util import get_objects_for_user -from pulpcore.app.serializers import AsyncOperationResponseSerializer, NestedRoleSerializer +from pulpcore.app.serializers import ( + AsyncOperationResponseSerializer, + NestedRoleSerializer, + SetLabelSerializer, + UnsetLabelSerializer, +) from pulpcore.app.util import get_viewset_for_model from pulpcore.tasking.tasks import dispatch @@ -626,3 +632,40 @@ def my_permissions(self, request, pk=None): ".".join((app_label, codename)) for codename in request.user.get_all_permissions(obj) ] return Response({"permissions": permissions}) + + +class LabelsMixin: + @extend_schema( + summary="Set a label", + description="Set a single pulp_label on the object to a specific value or null.", + ) + @action(detail=True, methods=["post"], serializer_class=SetLabelSerializer) + def set_label(self, request, pk=None): + obj = self.get_object() + serializer = SetLabelSerializer( + data=request.data, context={"request": request, "content_object": obj} + ) + serializer.is_valid(raise_exception=True) + obj._meta.model.objects.filter(pk=obj.pk).update( + pulp_labels=RawSQL( + "pulp_labels || hstore(%s, %s)", + [serializer.validated_data["key"], serializer.validated_data["value"]], + ) + ) + return Response(serializer.data, status=201) + + @extend_schema( + summary="Unset a label", + description="Unset a single pulp_label on the object.", + ) + @action(detail=True, methods=["post"], serializer_class=UnsetLabelSerializer) + def unset_label(self, request, pk=None): + obj = self.get_object() + serializer = UnsetLabelSerializer( + data=request.data, context={"request": request, "content_object": obj} + ) + serializer.is_valid(raise_exception=True) + obj._meta.model.objects.filter(pk=obj.pk).update( + pulp_labels=RawSQL("pulp_labels - %s::text", [serializer.validated_data["key"]]) + ) + return Response(serializer.data, status=201) diff --git a/pulpcore/app/viewsets/publication.py b/pulpcore/app/viewsets/publication.py index 2431e5396c..087f73df4d 100644 --- a/pulpcore/app/viewsets/publication.py +++ b/pulpcore/app/viewsets/publication.py @@ -27,6 +27,7 @@ AsyncCreateMixin, AsyncRemoveMixin, AsyncUpdateMixin, + LabelsMixin, NamedModelViewSet, RolesMixin, ) @@ -434,6 +435,7 @@ class DistributionViewSet( AsyncCreateMixin, AsyncRemoveMixin, AsyncUpdateMixin, + LabelsMixin, ): """ Provides read and list methods and also provides asynchronous CUD methods to dispatch tasks diff --git a/pulpcore/app/viewsets/repository.py b/pulpcore/app/viewsets/repository.py index 75af664274..c28a5492b7 100644 --- a/pulpcore/app/viewsets/repository.py +++ b/pulpcore/app/viewsets/repository.py @@ -30,6 +30,7 @@ from pulpcore.app.viewsets import ( AsyncRemoveMixin, AsyncUpdateMixin, + LabelsMixin, NamedModelViewSet, ) from pulpcore.app.viewsets.base import ( @@ -168,7 +169,7 @@ class ImmutableRepositoryViewSet( """ -class RepositoryViewSet(ImmutableRepositoryViewSet, AsyncUpdateMixin): +class RepositoryViewSet(ImmutableRepositoryViewSet, AsyncUpdateMixin, LabelsMixin): """ A ViewSet for an ordinary repository. """ @@ -369,6 +370,7 @@ class RemoteViewSet( mixins.ListModelMixin, AsyncUpdateMixin, AsyncRemoveMixin, + LabelsMixin, ): endpoint_name = "remotes" serializer_class = RemoteSerializer diff --git a/pulpcore/plugin/viewsets/__init__.py b/pulpcore/plugin/viewsets/__init__.py index 6f00b938d9..0a4c9af9e9 100644 --- a/pulpcore/plugin/viewsets/__init__.py +++ b/pulpcore/plugin/viewsets/__init__.py @@ -18,6 +18,7 @@ ImmutableRepositoryViewSet, ImporterViewSet, ImportViewSet, + LabelsMixin, NamedModelViewSet, NAME_FILTER_OPTIONS, NULLABLE_NUMERIC_FILTER_OPTIONS, diff --git a/pulpcore/tests/functional/api/using_plugin/test_labels.py b/pulpcore/tests/functional/api/using_plugin/test_labels.py new file mode 100644 index 0000000000..598628b7d8 --- /dev/null +++ b/pulpcore/tests/functional/api/using_plugin/test_labels.py @@ -0,0 +1,63 @@ +import pytest + + +@pytest.fixture +def label_access_policy(access_policies_api_client): + orig_access_policy = access_policies_api_client.list( + viewset_name="repositories/file/file" + ).results[0] + new_statements = orig_access_policy.statements.copy() + new_statements.append( + { + "action": ["set_label", "unset_label"], + "effect": "allow", + "condition": [ + "has_model_or_domain_or_obj_perms:file.modify_filerepository", + "has_model_or_domain_or_obj_perms:file.view_filerepository", + ], + "principal": "authenticated", + } + ) + access_policies_api_client.partial_update( + orig_access_policy.pulp_href, {"statements": new_statements} + ) + yield + if orig_access_policy.customized: + access_policies_api_client.partial_update( + orig_access_policy.pulp_href, {"statements": orig_access_policy.statements} + ) + else: + access_policies_api_client.reset(orig_access_policy.pulp_href) + + +@pytest.mark.parallel +def test_set_label(label_access_policy, file_repository_api_client, file_repository_factory): + repository = file_repository_factory() + assert repository.pulp_labels == {} + + file_repository_api_client.set_label(repository.pulp_href, {"key": "a", "value": None}) + file_repository_api_client.set_label(repository.pulp_href, {"key": "b", "value": ""}) + file_repository_api_client.set_label(repository.pulp_href, {"key": "c", "value": "val1"}) + file_repository_api_client.set_label(repository.pulp_href, {"key": "d", "value": "val2"}) + file_repository_api_client.set_label(repository.pulp_href, {"key": "e", "value": "val3"}) + file_repository_api_client.set_label(repository.pulp_href, {"key": "c", "value": "val4"}) + file_repository_api_client.set_label(repository.pulp_href, {"key": "d", "value": None}) + + repository = file_repository_api_client.read(repository.pulp_href) + assert repository.pulp_labels == { + "a": None, + "b": "", + "c": "val4", + "d": None, + "e": "val3", + } + + file_repository_api_client.unset_label(repository.pulp_href, {"key": "e"}) + + repository = file_repository_api_client.read(repository.pulp_href) + assert repository.pulp_labels == { + "a": None, + "b": "", + "c": "val4", + "d": None, + }