Add compatibility for pulpcore 3.70 #1927
Conversation
gerrod3
force-pushed
the
370-compat
branch
6 times, most recently
from
1e992dc to
e540249
Compare
| @@ -691,7 +691,8 @@ def get(self, request): | |||
| if images: | |||
| results.append({"Name": distribution.base_path, "Images": images}) | |||
|
|
|||
| return Response(data={"Registry": settings.CONTENT_ORIGIN, "Results": results}) | |||
| host = settings.CONTENT_ORIGIN or request.get_host() | |||
There was a problem hiding this comment.
This is the very thing that won't work with badly configured reverse proxy, right?
There was a problem hiding this comment.
Yeah, but it's the best we can do I think.
There was a problem hiding this comment.
request.get_host() does not return the URL scheme/proto, right?
I was worried about flatpak client somehow needing it (or checking the URL format), but it seems like it does not use the /index/dynamic endpoint, so if no other "integrator" depends on it, I think we are fine.
| @@ -1,4 +1,3 @@ | |||
| mock | |||
| pytest-django | |||
| pulp-smash @ git+https://github.com/pulp/pulp-smash.git | |||
There was a problem hiding this comment.
Can we remove it from functest_requirements too?
There was a problem hiding this comment.
I did, the files in git are in alphabetically order.
| return self.distribution.redirect_to_content_app( | ||
| f"{settings.CONTENT_ORIGIN}/pulp/container/{self.path}/{content_type}/{content_id}" | ||
| urljoin( | ||
| settings.CONTENT_ORIGIN, f"/pulp/container/{self.path}/{content_type}/{content_id}" | ||
| ) |
There was a problem hiding this comment.
The redirect here would fail in case CONTENT_ORIGIN=None, right?
What if we do something like
host = settings.CONTENT_ORIGIN or f"{request.scheme}://{request.get_host()}"
urljoin(
host, f"/pulp/container/{self.path}/{content_type}/{content_id}"
)
There was a problem hiding this comment.
The django documentation says it allows for relative urls (https://docs.djangoproject.com/en/4.2/topics/http/shortcuts/#redirect) so if CONTENT_ORIGIN=None this should return a relative path for the redirect. I assume that it works since I'm testing the setting being None in the S3 runner.
There was a problem hiding this comment.
ohh...nice!!
Thank you for the clarification!
Hopefully I got everything...