Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Makes CONTENT_ORIGIN not a required setting PULP-194 #6175

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
1 change: 1 addition & 0 deletions CHANGES/5931.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Allow CONTENT_ORIGIN to be None. When None, the base_url for Distributions is a relative path.
2 changes: 2 additions & 0 deletions CHANGES/plugin_api/relative-path-base-url.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Modified the `artifact_url` method from `ArtifactDistribution` model to return a relative URL
(no protocol, fqdn, and port) in case `CONTENT_ORIGIN` is not defined.
12 changes: 7 additions & 5 deletions docs/admin/reference/settings.md
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
# Settings

There are only two 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>`
- `CONTENT_ORIGIN <content-origin-setting>`

!!! note
For more information on how to specify settings see the [`Applying Settings`](#).
Expand Down Expand Up @@ -197,10 +196,13 @@ this setting only applies to uploads created after the change.

### CONTENT_ORIGIN

A required 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: `https://pulp.example.com:24816`.
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,
distribution_base_url,
):
cert_data, status_code = parameterized_cert

distribution_base_url = 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
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,
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(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,
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 = 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,
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"{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,
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 = 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,
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 = 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,
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 = 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
15 changes: 0 additions & 15 deletions pulpcore/app/checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,6 @@
from django.core.checks import Error as CheckError, Warning as CheckWarning, register


@register(deploy=True)
def content_origin_check(app_configs, **kwargs):
messages = []
if getattr(settings, "CONTENT_ORIGIN", "UNREACHABLE") == "UNREACHABLE":
messages.append(
CheckError(
"CONTENT_ORIGIN is a required setting but it was not configured. This may be "
"caused by invalid read permissions of the settings file. Note that "
"CONTENT_ORIGIN is set by the installation automatically.",
id="pulpcore.E001",
)
)
return messages


@register(deploy=True)
def secret_key_check(app_configs, **kwargs):
messages = []
Expand Down
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):
git-hyagi marked this conversation as resolved.
Show resolved Hide resolved
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
4 changes: 4 additions & 0 deletions pulpcore/app/replica.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from django.db.models import Model
from django.utils.dateparse import parse_datetime
from urllib.parse import urljoin

from pulp_glue.common.context import PulpContext
from pulpcore.tasking.tasks import dispatch
Expand Down Expand Up @@ -103,6 +104,9 @@ def create_or_update_remote(self, upstream_distribution):
):
return None
url = self.url(upstream_distribution)
if url.startswith("/"):
url = urljoin(self.server.base_url, url)

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("/")
git-hyagi marked this conversation as resolved.
Show resolved Hide resolved
prefix = settings.CONTENT_PATH_PREFIX.strip("/")
base_path = value.base_path.strip("/")
url = urljoin(origin, prefix + "/")
Expand Down
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=False,
allow_null=True,
required=False,
)
content_path_prefix = serializers.CharField(
help_text=_("The CONTENT_PATH_PREFIX setting for this Pulp instance"),
Expand Down
3 changes: 1 addition & 2 deletions pulpcore/app/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -262,8 +262,7 @@

DRF_ACCESS_POLICY = {"reusable_conditions": ["pulpcore.app.global_access_conditions"]}

# This is a sentinal value for deploy checks, since we don't want to set a default one.
CONTENT_ORIGIN = "UNREACHABLE"
CONTENT_ORIGIN = None
CONTENT_PATH_PREFIX = "/pulp/content/"

API_APP_TTL = 120 # The heartbeat is called from gunicorn notify (defaulting to 45 sec).
Expand Down
17 changes: 16 additions & 1 deletion pulpcore/pytest_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -773,8 +773,11 @@ def pulp_api_v3_url(bindings_cfg, pulp_api_v3_path):


@pytest.fixture(scope="session")
def pulp_content_url(pulp_settings, pulp_domain_enabled):
def pulp_content_url(bindings_cfg, pulp_settings, pulp_domain_enabled):
url = f"{pulp_settings.CONTENT_ORIGIN}{pulp_settings.CONTENT_PATH_PREFIX}"
if not pulp_settings.CONTENT_ORIGIN:
url = f"{bindings_cfg.host}{pulp_settings.CONTENT_PATH_PREFIX}"

if pulp_domain_enabled:
url += "default/"
return url
Expand Down Expand Up @@ -1212,3 +1215,15 @@ def _openpgp_keyring_factory(**kwargs):
)

return _openpgp_keyring_factory


# 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 distribution_base_url(bindings_cfg):
def _distribution_base_url(base_url):
if base_url.startswith("http"):
return base_url
return bindings_cfg.host + base_url

return _distribution_base_url
Loading
Loading