Skip to content

Commit 8a7fc2a

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 8a7fc2a

File tree

4 files changed

+98
-30
lines changed

4 files changed

+98
-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: 65 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
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 AsciiArmoredDetachedSigningService, Publication, Remote, Content
77
from pulpcore.plugin.serializers import (
88
DetailRelatedField,
99
DistributionSerializer,
@@ -14,7 +14,7 @@
1414
RepositorySyncURLSerializer,
1515
ValidateFieldsMixin,
1616
)
17-
from pulpcore.plugin.util import get_domain
17+
from pulpcore.plugin.util import get_domain, extract_pk
1818
from rest_framework import serializers
1919
from pulp_rpm.app.fields import CustomJSONField
2020

@@ -37,6 +37,7 @@
3737
)
3838
from pulp_rpm.app.schema import COPY_CONFIG_SCHEMA
3939
from urllib.parse import urlparse
40+
from textwrap import dedent
4041

4142

4243
class RpmRepositorySerializer(RepositorySerializer):
@@ -543,7 +544,32 @@ class CopySerializer(ValidateFieldsMixin, serializers.Serializer):
543544
"""
544545

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

549575
dependency_solving = serializers.BooleanField(
@@ -558,14 +584,24 @@ def validate(self, data):
558584
Check for cross-domain references (if domain-enabled).
559585
"""
560586

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
587+
def _extract_pk(ref):
588+
"""Workaround for expensive extract_pk(href) calls."""
589+
if ref.starswith("prn:"):
590+
return extract_pk(ref)
591+
else:
592+
# This assumes ref is a non-nested href. E.g:
593+
# /pulp/api/v3/content/rpm/packagegroups/0194a901-a9a6-705a-9436-ba6e19c98f40/
594+
uuid = urlparse(ref).path.strip("/").split("/")[-1]
595+
if len(uuid) < 32:
596+
raise serializers.ValidationError(
597+
_("The href path should end with a uuid pk: {}".format(ref))
598+
)
599+
return uuid
600+
601+
def check_domain_ok(domain, href) -> bool:
565602
if href and domain not in href:
566-
raise serializers.ValidationError(
567-
_("{} must be a part of the {} domain.").format(name, domain)
568-
)
603+
return False
604+
return True
569605

570606
def check_cross_domain_config(cfg):
571607
"""Check that all config-elts are in 'our' domain."""
@@ -574,13 +610,25 @@ def check_cross_domain_config(cfg):
574610
# Insure curr-domain exists in src/dest/dest_base_version/content-list hrefs
575611
curr_domain_name = get_domain().name
576612
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")
613+
domain_ok = True
614+
ref_list = [entry["dest_version"]]
615+
616+
# Special case for source_repo_version, because the general method using extract_pk
617+
# can't be used for nested hrefs (the pk is not in the href).
618+
if (source_rv := entry["source_repo_version"]).startswith("prn:"):
619+
ref_list.append(source_rv)
620+
else:
621+
domain_ok = domain_ok and check_domain_ok(curr_domain_name, source_rv)
622+
623+
# Check if all references belong to the correct domain without individual calls
624+
ref_list.extend(entry.get("content", []))
625+
pks_list = (_extract_pk(v) for v in ref_list)
626+
distinct = Content.objects.filter(pk__in=pks_list).values("pulp_domain").distinct()
627+
domain_ok = domain_ok and (len(distinct) == 1 and distinct.name == curr_domain_name)
628+
if not domain_ok:
629+
raise serializers.ValidationError(
630+
_("All href/prns must be a part of the {} domain.").format(curr_domain_name)
631+
)
584632

585633
super().validate(data)
586634
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)