Skip to content

Commit d738251

Browse files
committed
Assert PRN support to advanced copy API
* Add cross_domain validation for PRNs on copy API serializer. The new validation doesn't capture the first href/prn that failed anymore, as it relies on querying distinct domains in the bulk of references. Fixes: pulp#3853 fixup: review changes fixup: review changes on class type validation
1 parent 5b321da commit d738251

File tree

4 files changed

+141
-30
lines changed

4 files changed

+141
-30
lines changed

CHANGES/3853.bugfix

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Extended PRN support to Advanced Copy API and DistributionTree.

pulp_rpm/app/serializers/repository.py

Lines changed: 111 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,13 @@
33

44
from django.conf import settings
55
from jsonschema import Draft7Validator
6-
from pulpcore.plugin.models import AsciiArmoredDetachedSigningService, Publication, Remote
6+
from pulpcore.plugin.models import (
7+
AsciiArmoredDetachedSigningService,
8+
Publication,
9+
Remote,
10+
Content,
11+
RepositoryVersion,
12+
)
713
from pulpcore.plugin.serializers import (
814
DetailRelatedField,
915
DistributionSerializer,
@@ -14,7 +20,7 @@
1420
RepositorySyncURLSerializer,
1521
ValidateFieldsMixin,
1622
)
17-
from pulpcore.plugin.util import get_domain
23+
from pulpcore.plugin.util import get_domain, resolve_prn
1824
from rest_framework import serializers
1925
from pulp_rpm.app.fields import CustomJSONField
2026

@@ -37,6 +43,7 @@
3743
)
3844
from pulp_rpm.app.schema import COPY_CONFIG_SCHEMA
3945
from urllib.parse import urlparse
46+
from textwrap import dedent
4047

4148

4249
class RpmRepositorySerializer(RepositorySerializer):
@@ -543,7 +550,32 @@ class CopySerializer(ValidateFieldsMixin, serializers.Serializer):
543550
"""
544551

545552
config = CustomJSONField(
546-
help_text=_("A JSON document describing sources, destinations, and content to be copied"),
553+
help_text=_(
554+
dedent(
555+
"""\
556+
Content to be copied into the given destinations from the given sources.
557+
558+
Its a list of dictionaries with the following available fields:
559+
560+
```json
561+
[
562+
{
563+
"source_repo_version": <RepositoryVersion [pulp_href|prn]>,
564+
"dest_repo": <RpmRepository [pulp_href|prn]>,
565+
"dest_base_version": <int>,
566+
"content": [<Content [pulp_href|prn]>, ...]
567+
},
568+
...
569+
]
570+
```
571+
572+
If domains are enabled, the refered pulp objects must be part of the current domain.
573+
574+
For usage examples, refer to the advanced copy guide:
575+
<https://pulpproject.org/pulp_rpm/docs/user/guides/modify/#advanced-copy-workflow>
576+
"""
577+
)
578+
),
547579
)
548580

549581
dependency_solving = serializers.BooleanField(
@@ -558,29 +590,91 @@ def validate(self, data):
558590
Check for cross-domain references (if domain-enabled).
559591
"""
560592

561-
def check_domain(domain, href, name):
562-
# We're doing just a string-check here rather than checking objects
563-
# because there can be A LOT of objects, and this is happening in the view-layer
564-
# where we have strictly-limited timescales to work with
565-
if href and domain not in href:
593+
def raise_validation(field, domain, id=""):
594+
id = f"\n{id}" if id else ""
595+
raise serializers.ValidationError(
596+
_("The field {} contains object(s) not in {} domain.{}".format(field, domain, id))
597+
)
598+
599+
def parse_reference(ref) -> tuple[str, str, bool]:
600+
"""Extract info from prn/href to enable checking domains.
601+
602+
This is used for:
603+
1. In case of HREFS, avoid expensive extract_pk(href) to get pks.
604+
2. HREF and PRNs have different information hardcoded available.
605+
E.g: RepositoryVerseion HREF has its Repository pk; PRNs have the RepoVer pk.
606+
607+
Returns a tuple with (pk, class_name, is_prn)
608+
"""
609+
if ref.startswith("prn:"):
610+
ref_class, pk = resolve_prn(ref)
611+
return (pk, ref_class, True)
612+
# content: ${BASE}/content/rpm/packages/${UUID}/
613+
# repository: ${BASE}/repositories/rpm/rpm/${UUID}/
614+
# repover: ${BASE}/repositories/rpm/rpm/${UUID}/versions/0/
615+
url = urlparse(ref).path.strip("/").split("/")
616+
ref_class = RpmRepository if "/repositories/" in ref else Content
617+
is_repover_href = url[-1].isdigit() and url[-2] == "versions"
618+
uuid = url[-3] if is_repover_href else url[-1]
619+
if len(uuid) < 32:
566620
raise serializers.ValidationError(
567-
_("{} must be a part of the {} domain.").format(name, domain)
621+
_("The href path should end with a uuid pk: {}".format(ref))
568622
)
623+
return (uuid, ref_class, False)
624+
625+
def check_domain(entry, name, curr_domain):
626+
"""Check domain for RpmRepository and RepositoryVersion objects."""
627+
href_or_prn = entry[name]
628+
resource_pk, ref_class, is_prn = parse_reference(href_or_prn)
629+
try:
630+
if ref_class is RepositoryVersion and is_prn:
631+
resource_domain_pk = (
632+
RepositoryVersion.objects.select_related("repository")
633+
.values("repository__pulp_domain")
634+
.get(pk=resource_pk)["repository__pulp_domain"]
635+
)
636+
elif ref_class is RpmRepository:
637+
resource_domain_pk = RpmRepository.objects.values("pulp_domain").get(
638+
pk=resource_pk
639+
)["pulp_domain"]
640+
else:
641+
raise serializers.ValidationError(
642+
_(
643+
"Expected RpmRepository or RepositoryVersion ref_class. "
644+
"Got {} from {}.".format(ref_class, href_or_prn)
645+
)
646+
)
647+
except RepositoryVersion.DoesNotExit as e:
648+
raise serializers.ValidationError from e
649+
except RpmRepository.DoesNotExit as e:
650+
raise serializers.ValidationError from e
651+
652+
if resource_domain_pk != curr_domain.pk:
653+
raise_validation(name, curr_domain.name, resource_domain_pk)
569654

570655
def check_cross_domain_config(cfg):
571656
"""Check that all config-elts are in 'our' domain."""
572657
# copy-cfg is a list of dictionaries.
573658
# source_repo_version and dest_repo are required fields.
574659
# Insure curr-domain exists in src/dest/dest_base_version/content-list hrefs
575-
curr_domain_name = get_domain().name
660+
curr_domain = get_domain()
576661
for entry in cfg:
577-
check_domain(curr_domain_name, entry["source_repo_version"], "dest_repo")
578-
check_domain(curr_domain_name, entry["dest_repo"], "dest_repo")
579-
check_domain(
580-
curr_domain_name, entry.get("dest_base_version", None), "dest_base_version"
581-
)
582-
for content_href in entry.get("content", []):
583-
check_domain(curr_domain_name, content_href, "content")
662+
# Check required fields individually
663+
check_domain(entry, "source_repo_version", curr_domain)
664+
check_domain(entry, "dest_repo", curr_domain)
665+
666+
# Check content generically to avoid timeout of multiple calls
667+
content_list = entry.get("content", None)
668+
if content_list:
669+
content_list = [parse_reference(v)[0] for v in content_list]
670+
distinct = (
671+
Content.objects.filter(pk__in=content_list).values("pulp_domain").distinct()
672+
)
673+
domain_ok = (
674+
len(distinct) == 1 and distinct.first()["pulp_domain"] == curr_domain.pk
675+
)
676+
if not domain_ok:
677+
raise_validation("content", curr_domain.name)
584678

585679
super().validate(data)
586680
if "config" in data:

pulp_rpm/tests/functional/api/test_copy.py

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,23 @@
1818

1919
from pulpcore.client.pulp_rpm import Copy
2020
from pulpcore.client.pulp_rpm.exceptions import ApiException
21+
import subprocess
2122

2223

24+
def noop(uri):
25+
return uri
26+
27+
28+
def get_prn(uri):
29+
"""Utility to get prn without having to setup django."""
30+
commands = f"from pulpcore.app.util import get_prn; print(get_prn(uri='{uri}'));"
31+
process = subprocess.run(["pulpcore-manager", "shell", "-c", commands], capture_output=True)
32+
assert process.returncode == 0
33+
prn = process.stdout.decode().strip()
34+
return prn
35+
36+
37+
@pytest.mark.parametrize("get_id", [noop, get_prn], ids=["without-prn", "with-prn"])
2338
@pytest.mark.parallel
2439
def test_modular_static_context_copy(
2540
init_and_sync,
@@ -28,13 +43,19 @@ def test_modular_static_context_copy(
2843
rpm_modulemd_api,
2944
rpm_repository_factory,
3045
rpm_repository_api,
46+
get_id,
3147
):
3248
"""Test copying a static_context-using repo to an empty destination."""
3349
src, _ = init_and_sync(url=RPM_MODULES_STATIC_CONTEXT_FIXTURE_URL)
3450
dest = rpm_repository_factory()
3551

3652
data = Copy(
37-
config=[{"source_repo_version": src.latest_version_href, "dest_repo": dest.pulp_href}],
53+
config=[
54+
{
55+
"source_repo_version": get_id(src.latest_version_href),
56+
"dest_repo": get_id(dest.pulp_href),
57+
}
58+
],
3859
dependency_solving=False,
3960
)
4061
monitor_task(rpm_copy_api.copy_content(data).task)
@@ -44,7 +65,7 @@ def test_modular_static_context_copy(
4465
assert get_content_summary(dest.to_dict()) == RPM_MODULAR_STATIC_FIXTURE_SUMMARY
4566
assert get_added_content_summary(dest.to_dict()) == RPM_MODULAR_STATIC_FIXTURE_SUMMARY
4667

47-
modules = rpm_modulemd_api.list(repository_version=dest.latest_version_href).results
68+
modules = rpm_modulemd_api.list(repository_version=get_id(dest.latest_version_href)).results
4869
module_static_contexts = [
4970
(module.name, module.version) for module in modules if module.static_context
5071
]
@@ -141,6 +162,7 @@ def test_invalid_config(
141162
)
142163
rpm_copy_api.copy_content(data)
143164

165+
@pytest.mark.parametrize("get_id", [noop, get_prn], ids=["without-prn", "with-prn"])
144166
def test_content(
145167
self,
146168
monitor_task,
@@ -149,20 +171,21 @@ def test_content(
149171
rpm_repository_api,
150172
rpm_repository_factory,
151173
rpm_unsigned_repo_immediate,
174+
get_id,
152175
):
153176
"""Test the content parameter."""
154177
src = rpm_unsigned_repo_immediate
155178

156179
content = rpm_advisory_api.list(repository_version=src.latest_version_href).results
157-
content_to_copy = (content[0].pulp_href, content[1].pulp_href)
180+
content_to_copy = (get_id(content[0].pulp_href), get_id(content[1].pulp_href))
158181

159182
dest = rpm_repository_factory()
160183

161184
data = Copy(
162185
config=[
163186
{
164-
"source_repo_version": src.latest_version_href,
165-
"dest_repo": dest.pulp_href,
187+
"source_repo_version": get_id(src.latest_version_href),
188+
"dest_repo": get_id(dest.pulp_href),
166189
"content": content_to_copy,
167190
}
168191
],
@@ -172,7 +195,7 @@ def test_content(
172195

173196
dest = rpm_repository_api.read(dest.pulp_href)
174197
dc = rpm_advisory_api.list(repository_version=dest.latest_version_href)
175-
dest_content = [c.pulp_href for c in dc.results]
198+
dest_content = [get_id(c.pulp_href) for c in dc.results]
176199

177200
assert sorted(content_to_copy) == sorted(dest_content)
178201

pulp_rpm/tests/functional/api/test_prn.py

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,6 @@
11
import pytest
2-
import requests
32

43

5-
6-
# @pytest.fixture(scope="session")
7-
# def pulp_openapi_schema_rpm(pulp_api_v3_url):
8-
# COMPONENT="rpm"
9-
# return requests.get(f"{pulp_api_v3_url}docs/api.json?bindings&component={COMPONENT}").json()
10-
114
@pytest.mark.parallel
125
def test_prn_schema(pulp_openapi_schema):
136
"""Test that PRN is a part of every serializer with a pulp_href."""

0 commit comments

Comments
 (0)