Skip to content

Commit 34fb058

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
1 parent 5b321da commit 34fb058

File tree

4 files changed

+136
-30
lines changed

4 files changed

+136
-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: 103 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+
A JSON document describing sources, destinations, and content to be copied.
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,83 @@ 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+
href_or_prn = entry[name]
627+
resource_pk, ref_class, is_prn = parse_reference(href_or_prn)
628+
try:
629+
if ref_class is RepositoryVersion and is_prn:
630+
resource_domain_pk = (
631+
RepositoryVersion.objects.select_related("repository")
632+
.values("repository__pulp_domain")
633+
.get(pk=resource_pk)["repository__pulp_domain"]
634+
)
635+
else:
636+
resource_domain_pk = RpmRepository.objects.values("pulp_domain").get(
637+
pk=resource_pk
638+
)["pulp_domain"]
639+
except RepositoryVersion.DoesNotExit as e:
640+
raise serializers.ValidationError from e
641+
except RpmRepository.DoesNotExit as e:
642+
raise serializers.ValidationError from e
643+
644+
if resource_domain_pk != curr_domain.pk:
645+
raise_validation(name, curr_domain.name, resource_domain_pk)
569646

570647
def check_cross_domain_config(cfg):
571648
"""Check that all config-elts are in 'our' domain."""
572649
# copy-cfg is a list of dictionaries.
573650
# source_repo_version and dest_repo are required fields.
574651
# Insure curr-domain exists in src/dest/dest_base_version/content-list hrefs
575-
curr_domain_name = get_domain().name
652+
curr_domain = get_domain()
576653
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")
654+
# Check required fields individually
655+
check_domain(entry, "source_repo_version", curr_domain)
656+
check_domain(entry, "dest_repo", curr_domain)
657+
658+
# Check content generically to avoid timeout of multiple calls
659+
content_list = entry.get("content", None)
660+
if content_list:
661+
content_list = [parse_reference(v)[0] for v in content_list]
662+
distinct = (
663+
Content.objects.filter(pk__in=content_list).values("pulp_domain").distinct()
664+
)
665+
domain_ok = (
666+
len(distinct) == 1 and distinct.first()["pulp_domain"] == curr_domain.pk
667+
)
668+
if not domain_ok:
669+
raise_validation("content", curr_domain.name)
584670

585671
super().validate(data)
586672
if "config" in data:

pulp_rpm/tests/functional/api/test_copy.py

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,26 @@
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+
TODO: This is a Copy-paste from pulpcore. Make it public there.
31+
"""
32+
commands = f"from pulpcore.app.util import get_prn; print(get_prn(uri='{uri}'));"
33+
process = subprocess.run(["pulpcore-manager", "shell", "-c", commands], capture_output=True)
34+
35+
assert process.returncode == 0
36+
prn = process.stdout.decode().strip()
37+
return prn
38+
39+
40+
@pytest.mark.parametrize("get_id", [noop, get_prn], ids=["without-prn", "with-prn"])
2341
@pytest.mark.parallel
2442
def test_modular_static_context_copy(
2543
init_and_sync,
@@ -28,13 +46,19 @@ def test_modular_static_context_copy(
2846
rpm_modulemd_api,
2947
rpm_repository_factory,
3048
rpm_repository_api,
49+
get_id,
3150
):
3251
"""Test copying a static_context-using repo to an empty destination."""
3352
src, _ = init_and_sync(url=RPM_MODULES_STATIC_CONTEXT_FIXTURE_URL)
3453
dest = rpm_repository_factory()
3554

3655
data = Copy(
37-
config=[{"source_repo_version": src.latest_version_href, "dest_repo": dest.pulp_href}],
56+
config=[
57+
{
58+
"source_repo_version": get_id(src.latest_version_href),
59+
"dest_repo": get_id(dest.pulp_href),
60+
}
61+
],
3862
dependency_solving=False,
3963
)
4064
monitor_task(rpm_copy_api.copy_content(data).task)
@@ -44,7 +68,7 @@ def test_modular_static_context_copy(
4468
assert get_content_summary(dest.to_dict()) == RPM_MODULAR_STATIC_FIXTURE_SUMMARY
4569
assert get_added_content_summary(dest.to_dict()) == RPM_MODULAR_STATIC_FIXTURE_SUMMARY
4670

47-
modules = rpm_modulemd_api.list(repository_version=dest.latest_version_href).results
71+
modules = rpm_modulemd_api.list(repository_version=get_id(dest.latest_version_href)).results
4872
module_static_contexts = [
4973
(module.name, module.version) for module in modules if module.static_context
5074
]
@@ -141,6 +165,7 @@ def test_invalid_config(
141165
)
142166
rpm_copy_api.copy_content(data)
143167

168+
@pytest.mark.parametrize("get_id", [noop, get_prn], ids=["without-prn", "with-prn"])
144169
def test_content(
145170
self,
146171
monitor_task,
@@ -149,20 +174,21 @@ def test_content(
149174
rpm_repository_api,
150175
rpm_repository_factory,
151176
rpm_unsigned_repo_immediate,
177+
get_id,
152178
):
153179
"""Test the content parameter."""
154180
src = rpm_unsigned_repo_immediate
155181

156182
content = rpm_advisory_api.list(repository_version=src.latest_version_href).results
157-
content_to_copy = (content[0].pulp_href, content[1].pulp_href)
183+
content_to_copy = (get_id(content[0].pulp_href), get_id(content[1].pulp_href))
158184

159185
dest = rpm_repository_factory()
160186

161187
data = Copy(
162188
config=[
163189
{
164-
"source_repo_version": src.latest_version_href,
165-
"dest_repo": dest.pulp_href,
190+
"source_repo_version": get_id(src.latest_version_href),
191+
"dest_repo": get_id(dest.pulp_href),
166192
"content": content_to_copy,
167193
}
168194
],
@@ -172,7 +198,7 @@ def test_content(
172198

173199
dest = rpm_repository_api.read(dest.pulp_href)
174200
dc = rpm_advisory_api.list(repository_version=dest.latest_version_href)
175-
dest_content = [c.pulp_href for c in dc.results]
201+
dest_content = [get_id(c.pulp_href) for c in dc.results]
176202

177203
assert sorted(content_to_copy) == sorted(dest_content)
178204

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)