Skip to content

Commit

Permalink
Update the tests for CONTENT_ORIGIN being optional
Browse files Browse the repository at this point in the history
  • Loading branch information
git-hyagi committed Jan 21, 2025
1 parent 6e35d29 commit e81499a
Show file tree
Hide file tree
Showing 30 changed files with 194 additions and 70 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/scripts/install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ if [ "$TEST" = "azure" ]; then
- ./azurite:/etc/pulp\
command: "azurite-blob --blobHost 0.0.0.0"' vars/main.yaml
sed -i -e '$a azure_test: true\
pulp_scenario_settings: {"api_root_rewrite_header": "X-API-Root", "domain_enabled": true, "rest_framework__default_permission_classes": ["pulpcore.plugin.access_policy.DefaultAccessPolicy"]}\
pulp_scenario_settings: {"api_root_rewrite_header": "X-API-Root", "content_origin": null, "domain_enabled": true, "rest_framework__default_permission_classes": ["pulpcore.plugin.access_policy.DefaultAccessPolicy"]}\
pulp_scenario_env: {}\
' vars/main.yaml
fi
Expand Down
12 changes: 7 additions & 5 deletions docs/admin/reference/settings.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Settings

There is one required settings, although specific plugins may have additional required
There is one required setting, although specific plugins may have additional required
settings.

- `SECRET_KEY <secret-key-setting>`
Expand Down Expand Up @@ -196,11 +196,13 @@ this setting only applies to uploads created after the change.

### CONTENT_ORIGIN

A string containing the protocol, fqdn, and port where the content app is reachable by
users. This is used by `pulpcore` and various plugins when referring users to the content app.
A string containing the protocol, fqdn, and port where the content app is reachable by users.
This is used by `pulpcore` and various plugins when referring users to the content app.
For example if the API should refer users to content at using http to pulp.example.com on port
24816, (the content default port), you would set: `http://pulp.example.com:24816`. The default is
`None`. When set to `None`, the API returns relative URLs (without the protocol, fqdn, and port).
24816, (the content default port), you would set: `https://pulp.example.com:24816`. The default is `None`.
When set to `None`, the `base_url` for Distributions is a relative path.
This means the API returns relative URLs without the protocol, fqdn, and port.


### HIDE_GUARDED_DISTRIBUTIONS

Expand Down
11 changes: 9 additions & 2 deletions pulp_certguard/tests/functional/api/test_x509_certguard.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,18 @@ def parameterized_cert(request):
class TestX509CertGuard:
"""A test class to share the costly distribution setup."""

def test_download(self, x509_guarded_distribution, parameterized_cert, content_path):
def test_download(
self,
x509_guarded_distribution,
parameterized_cert,
content_path,
file_distribution_base_url,
):
cert_data, status_code = parameterized_cert

distribution_base_url = file_distribution_base_url(x509_guarded_distribution.base_url)
response = requests.get(
urljoin(x509_guarded_distribution.base_url, content_path),
urljoin(distribution_base_url, content_path),
headers=cert_data and {"X-CLIENT-CERT": cert_data},
)
assert response.status_code == status_code
12 changes: 12 additions & 0 deletions pulp_file/pytest_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -380,3 +380,15 @@ def _generate_server_and_remote(*, manifest_path, policy):
return server, remote

yield _generate_server_and_remote


# if content_origin == None, base_url will return the relative path and
# we need to add the hostname to run the tests
@pytest.fixture
def file_distribution_base_url(bindings_cfg):
def _file_distribution_base_url(base_url):
if base_url.startswith("http"):
return base_url
return bindings_cfg.host + base_url

return _file_distribution_base_url
3 changes: 2 additions & 1 deletion pulp_file/tests/functional/api/test_acs.py
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ def test_acs_sync_with_paths(
def test_serving_acs_content(
file_bindings,
file_repo,
file_distribution_base_url,
file_distribution_factory,
basic_manifest_path,
gen_object_with_cleanup,
Expand Down Expand Up @@ -245,7 +246,7 @@ def test_serving_acs_content(
# Download one of the files and assert that it has the right checksum and that it is downloaded
# from the ACS server.
content_unit = list(expected_files)[0]
content_unit_url = urljoin(distribution.base_url, content_unit[0])
content_unit_url = urljoin(file_distribution_base_url(distribution.base_url), content_unit[0])
downloaded_file = download_file(content_unit_url)
actual_checksum = hashlib.sha256(downloaded_file.body).hexdigest()
expected_checksum = content_unit[1]
Expand Down
6 changes: 4 additions & 2 deletions pulp_file/tests/functional/api/test_auto_publish.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ def file_repo_with_auto_publish(file_repository_factory):
@pytest.mark.parallel
def test_auto_publish_and_distribution(
file_bindings,
file_distribution_base_url,
file_repo_with_auto_publish,
file_remote_ssl_factory,
basic_manifest_path,
Expand All @@ -32,6 +33,7 @@ def test_auto_publish_and_distribution(
file_bindings.DistributionsFileApi,
{"name": "foo", "base_path": "bar/foo", "repository": repo.pulp_href},
)
distribution_base_url = file_distribution_base_url(distribution.base_url)

# Assert that the repository is at version 0 and that there are no publications associated with
# this Repository and that the distribution doesn't have a publication associated with it.
Expand Down Expand Up @@ -67,7 +69,7 @@ def test_auto_publish_and_distribution(

# Download the custom manifest
files_in_first_publication = get_files_in_manifest(
"{}{}".format(distribution.base_url, publication.manifest)
"{}{}".format(distribution_base_url, publication.manifest)
)
assert files_in_first_publication == expected_files

Expand All @@ -80,7 +82,7 @@ def test_auto_publish_and_distribution(
)
repo = file_bindings.RepositoriesFileApi.read(repo.pulp_href)
files_in_second_publication = get_files_in_manifest(
"{}{}".format(distribution.base_url, publication.manifest)
"{}{}".format(distribution_base_url, publication.manifest)
)
files_added = files_in_second_publication - files_in_first_publication
assert repo.latest_version_href.endswith("/versions/2/")
Expand Down
3 changes: 2 additions & 1 deletion pulp_file/tests/functional/api/test_crud_content_unit.py
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,7 @@ def test_create_file_from_url(
file_bindings,
file_repository_factory,
file_remote_factory,
file_distribution_base_url,
file_distribution_factory,
basic_manifest_path,
monitor_task,
Expand All @@ -283,7 +284,7 @@ def test_create_file_from_url(

# Test create w/ url for already existing content
response = file_bindings.ContentFilesApi.create(
file_url=f"{distro.base_url}1.iso",
file_url=f"{file_distribution_base_url(distro.base_url)}1.iso",
relative_path="1.iso",
)
task = monitor_task(response.task)
Expand Down
6 changes: 4 additions & 2 deletions pulp_file/tests/functional/api/test_domains.py
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ def test_content_promotion(
file_bindings,
basic_manifest_path,
file_remote_factory,
file_distribution_base_url,
file_distribution_factory,
gen_object_with_cleanup,
monitor_task,
Expand Down Expand Up @@ -217,14 +218,15 @@ def test_content_promotion(

# Distribute Task
distro = file_distribution_factory(publication=pub.pulp_href, pulp_domain=domain.name)
distro_base_url = file_distribution_base_url(distro.base_url)

assert distro.publication == pub.pulp_href
# Url structure should be host/CONTENT_ORIGIN/DOMAIN_PATH/BASE_PATH
assert domain.name == distro.base_url.rstrip("/").split("/")[-2]
assert domain.name == distro_base_url.rstrip("/").split("/")[-2]

# Check that content can be downloaded from base_url
for path in ("1.iso", "2.iso", "3.iso"):
download = download_file(f"{distro.base_url}{path}")
download = download_file(f"{distro_base_url}{path}")
assert download.response_obj.status == 200
assert len(download.body) == 1024

Expand Down
22 changes: 12 additions & 10 deletions pulp_file/tests/functional/api/test_download_policies.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ def test_download_policy(
pulpcore_bindings,
file_bindings,
file_repo,
file_distribution_base_url,
file_remote_ssl_factory,
range_header_manifest_path,
gen_object_with_cleanup,
Expand Down Expand Up @@ -100,11 +101,12 @@ def test_download_policy(
"repository": file_repo.pulp_href,
},
)
distribution_base_url = file_distribution_base_url(distribution.base_url)

# Assert that un-published content is not available
for expected_file in expected_files:
with pytest.raises(ClientResponseError) as exc:
content_unit_url = urljoin(distribution.base_url, expected_file[0])
content_unit_url = urljoin(distribution_base_url, expected_file[0])
download_file(content_unit_url)
assert exc.value.status == 404

Expand All @@ -127,7 +129,7 @@ def test_download_policy(
assert process.returncode == 0
content_artifact_created_date = process.stdout.decode().strip()
# Download the listing page for the 'foo' directory
distribution_html_page = download_file(f"{distribution.base_url}foo")
distribution_html_page = download_file(f"{distribution_base_url}foo")
# Assert that requesting a path inside a distribution without a trailing / returns a 301
assert distribution_html_page.response_obj.history[0].status == 301
soup = BeautifulSoup(distribution_html_page.body, "html.parser")
Expand All @@ -141,7 +143,7 @@ def test_download_policy(
# Download one of the files and assert that it has the right checksum
expected_files_list = list(expected_files)
content_unit = expected_files_list[0]
content_unit_url = urljoin(distribution.base_url, content_unit[0])
content_unit_url = urljoin(distribution_base_url, content_unit[0])
downloaded_file = download_file(content_unit_url)
actual_checksum = hashlib.sha256(downloaded_file.body).hexdigest()
expected_checksum = content_unit[1]
Expand All @@ -160,32 +162,32 @@ def test_download_policy(
range_header = {"Range": "bytes=1048586-1049586"}
num_bytes = 1001
content_unit = expected_files_list[1]
content_unit_url = urljoin(distribution.base_url, content_unit[0])
content_unit_url = urljoin(distribution_base_url, content_unit[0])
_do_range_request_download_and_assert(content_unit_url, range_header, num_bytes)

# Assert proper download with range requests spanning multiple chunks of downloader
range_header = {"Range": "bytes=1048176-2248576"}
num_bytes = 1200401
content_unit = expected_files_list[2]
content_unit_url = urljoin(distribution.base_url, content_unit[0])
content_unit_url = urljoin(distribution_base_url, content_unit[0])
_do_range_request_download_and_assert(content_unit_url, range_header, num_bytes)

# Assert that multiple requests with different Range header values work as expected
range_header = {"Range": "bytes=1048176-2248576"}
num_bytes = 1200401
content_unit = expected_files_list[3]
content_unit_url = urljoin(distribution.base_url, content_unit[0])
content_unit_url = urljoin(distribution_base_url, content_unit[0])
_do_range_request_download_and_assert(content_unit_url, range_header, num_bytes)

range_header = {"Range": "bytes=2042176-3248576"}
num_bytes = 1206401
content_unit = expected_files_list[3]
content_unit_url = urljoin(distribution.base_url, content_unit[0])
content_unit_url = urljoin(distribution_base_url, content_unit[0])
_do_range_request_download_and_assert(content_unit_url, range_header, num_bytes)

# Assert that range requests with a negative start value errors as expected
content_unit = expected_files_list[4]
content_unit_url = urljoin(distribution.base_url, content_unit[0])
content_unit_url = urljoin(distribution_base_url, content_unit[0])
# The S3 test API project doesn't handle invalid Range values correctly
if settings.DEFAULT_FILE_STORAGE == "pulpcore.app.models.storage.FileSystem":
with pytest.raises(ClientResponseError) as exc:
Expand All @@ -195,7 +197,7 @@ def test_download_policy(

# Assert that a range request with a start value larger than the content errors
content_unit = expected_files_list[5]
content_unit_url = urljoin(distribution.base_url, content_unit[0])
content_unit_url = urljoin(distribution_base_url, content_unit[0])
with pytest.raises(ClientResponseError) as exc:
range_header = {"Range": "bytes=10485860-10485870"}
download_file(content_unit_url, headers=range_header)
Expand All @@ -205,7 +207,7 @@ def test_download_policy(
range_header = {"Range": "bytes=4193804-4294304"}
num_bytes = 500
content_unit = expected_files_list[6]
content_unit_url = urljoin(distribution.base_url, content_unit[0])
content_unit_url = urljoin(distribution_base_url, content_unit[0])
_do_range_request_download_and_assert(content_unit_url, range_header, num_bytes)

# Assert that artifacts were not downloaded if policy is not immediate
Expand Down
4 changes: 3 additions & 1 deletion pulp_file/tests/functional/api/test_mime_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
@pytest.mark.parallel
def test_content_types(
file_bindings,
file_distribution_base_url,
file_repo_with_auto_publish,
file_content_unit_with_name_factory,
gen_object_with_cleanup,
Expand Down Expand Up @@ -46,13 +47,14 @@ def test_content_types(
repository=file_repo_with_auto_publish.pulp_href,
)
distribution = gen_object_with_cleanup(file_bindings.DistributionsFileApi, data)
distribution_base_url = file_distribution_base_url(distribution.base_url)

received_mimetypes = {}
for extension, content_unit in files.items():

async def get_content_type():
async with aiohttp.ClientSession() as session:
url = urljoin(distribution.base_url, content_unit.relative_path)
url = urljoin(distribution_base_url, content_unit.relative_path)
async with session.get(url) as response:
return response.headers.get("Content-Type")

Expand Down
18 changes: 18 additions & 0 deletions pulpcore/app/migrations/0127_add_content_origin_to_upstreampulp.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Generated by Django 4.2.16 on 2025-01-17 15:37

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('core', '0126_remoteartifact_failed_at'),
]

operations = [
migrations.AddField(
model_name='upstreampulp',
name='content_origin',
field=models.TextField(null=True),
),
]
7 changes: 6 additions & 1 deletion pulpcore/app/models/publication.py
Original file line number Diff line number Diff line change
Expand Up @@ -728,7 +728,12 @@ def ensure_singleton(self):
raise RuntimeError(f"This system already has a {cls.__name__}")

def artifact_url(self, artifact):
origin = settings.CONTENT_ORIGIN.strip("/")

# When CONTENT_ORIGIN == None we need to set origin as "/" so that the base_url will
# have the relative path like "/some/file/path", instead of "some/file/path"
origin = "/"
if settings.CONTENT_ORIGIN:
origin = settings.CONTENT_ORIGIN.strip("/")
prefix = settings.CONTENT_PATH_PREFIX.strip("/")
base_path = self.base_path.strip("/")
if settings.DOMAIN_ENABLED:
Expand Down
2 changes: 2 additions & 0 deletions pulpcore/app/models/replica.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ class UpstreamPulp(BaseModel, AutoAddObjPermsMixin):

last_replication = models.DateTimeField(null=True)

content_origin = models.TextField(null=True)

class Meta:
unique_together = ("name", "pulp_domain")
permissions = [
Expand Down
10 changes: 10 additions & 0 deletions pulpcore/app/replica.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,16 @@ def create_or_update_remote(self, upstream_distribution):
):
return None
url = self.url(upstream_distribution)
if url.startswith("/"):
if self.server.content_origin:
url = self.server.content_origin + url
else:
raise Exception(
"UpstreamPulp needs to have content_origin defined because the downstream Pulp"
"does not know upstream's hostname from the distribution base_url (upstream"
"Pulp does not have the content_origin defined)."
)

remote_fields_dict = {"url": url}
remote_fields_dict.update(self.tls_settings)
remote_fields_dict.update(self.remote_extra_fields(upstream_distribution))
Expand Down
7 changes: 6 additions & 1 deletion pulpcore/app/serializers/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,12 @@ class BaseURLField(serializers.CharField):
"""

def to_representation(self, value):
origin = settings.CONTENT_ORIGIN.strip("/")

# When CONTENT_ORIGIN == None we need to set origin as "/" so that the base_url will
# have the relative path like "/some/file/path", instead of "some/file/path"
origin = "/"
if settings.CONTENT_ORIGIN:
origin = settings.CONTENT_ORIGIN.strip("/")
prefix = settings.CONTENT_PATH_PREFIX.strip("/")
base_path = value.base_path.strip("/")
url = urljoin(origin, prefix + "/")
Expand Down
11 changes: 11 additions & 0 deletions pulpcore/app/serializers/replica.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,16 @@ class UpstreamPulpSerializer(ModelSerializer, HiddenFieldsMixin):
read_only=True,
)

content_origin = serializers.CharField(
help_text=_(
"Upstream server address. This field is used by downstream Pulp in case upstream Pulp"
"does not have content_origin set."
),
allow_null=True,
allow_blank=True,
required=False,
)

def validate_q_select(self, value):
"""Ensure we have a valid q_select expression."""
from pulpcore.app.viewsets import DistributionFilter
Expand All @@ -116,4 +126,5 @@ class Meta:
"hidden_fields",
"q_select",
"last_replication",
"content_origin",
)
3 changes: 3 additions & 0 deletions pulpcore/app/serializers/status.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@ class ContentSettingsSerializer(serializers.Serializer):

content_origin = serializers.CharField(
help_text=_("The CONTENT_ORIGIN setting for this Pulp instance"),
allow_blank=True,
allow_null=True,
required=False,
)
content_path_prefix = serializers.CharField(
help_text=_("The CONTENT_PATH_PREFIX setting for this Pulp instance"),
Expand Down
Loading

0 comments on commit e81499a

Please sign in to comment.