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

Support null CONTENT_ORIGIN #3856

Open
dralley opened this issue Jan 16, 2025 · 4 comments · May be fixed by #3865
Open

Support null CONTENT_ORIGIN #3856

dralley opened this issue Jan 16, 2025 · 4 comments · May be fixed by #3865
Assignees
Labels
Milestone

Comments

@dralley
Copy link
Contributor

dralley commented Jan 16, 2025

As per pulp/pulpcore#5931, Pulp plugins need to support the possibility of the value provided for CONTENT_ORIGIN being None

@dralley
Copy link
Contributor Author

dralley commented Jan 16, 2025

@dralley dralley added this to the 3.28 milestone Jan 16, 2025
@ggainey
Copy link
Contributor

ggainey commented Jan 20, 2025

The place where we're affected is when we try to auto-generate the config.repo file - baseurl wants to be a full URL, not relative.

We can do one of a few things here :

  1. just return relative-urls in config.repo and let the user Figure It Out
  2. refuse to generate config.repo if you don't set CONTENT_ORIGIN - user has to "supply their own" rather than rely on us to set it. DOCUMENT THIS.
  3. build the URL based on the incoming request (assuming we even have that?) ("path" is the filename, not the URL)

See https://github.com/pulp/pulp_rpm/blob/main/pulp_rpm/app/models/repository.py#L526 for the code that's affected

@dralley
Copy link
Contributor Author

dralley commented Jan 20, 2025

re: 3, It would likely be an API breaking change to the content_handler() function, but it could be done.

I think I'm partial to option 2 personally

@mdellweg
Copy link
Member

mdellweg commented Jan 21, 2025

IMHO the feature is described as if content origin is not set, the base url is the same as from the incoming request. So ideally pulpcore would come with a helper function to extract that from the request so we can do a simple search and replace for settings.CONTENT_ORIGIN to content_origin(request). That should enable option 3 (least surprising one?).

edit: AFTER some discussion (e.g. that the request may not know the original URL in all deployments) I agree with 2: Explicitly telling the user it's impossible and maybe bring more scenarios back later when we know it's possible. Existing installations will have the setting anyway, so nothing changes for them.

@ggainey ggainey self-assigned this Jan 21, 2025
ggainey added a commit to ggainey/pulp_rpm that referenced this issue Jan 22, 2025
ggainey added a commit to ggainey/pulp_rpm that referenced this issue Jan 22, 2025
@ggainey ggainey linked a pull request Jan 22, 2025 that will close this issue
ggainey added a commit to ggainey/pulp_rpm that referenced this issue Jan 24, 2025
ggainey added a commit to ggainey/pulp_rpm that referenced this issue Jan 24, 2025
Removed tests/upgrade directory - these tests don't test anything
current, and have not actually been run in years.

fixes pulp#3856.
ggainey added a commit to ggainey/pulp_rpm that referenced this issue Jan 24, 2025
Removed tests/upgrade directory - these tests don't test anything
current, and have not actually been run in years.

fixes pulp#3856.
ggainey added a commit to ggainey/pulp_rpm that referenced this issue Jan 24, 2025
Removed tests/upgrade directory - these tests don't test anything
current, and have not actually been run in years.

fixes pulp#3856.
ggainey added a commit to ggainey/pulp_rpm that referenced this issue Jan 24, 2025
Removed tests/upgrade directory - these tests don't test anything
current, and have not actually been run in years.

fixes pulp#3856.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants