Regression caused by the fix for #9464. IRC discussion:
mimmus
hi, I'm taking up syncing an Amazon Linux repo again, after ggainey_ kindly produced a patch to overcome a bug (8875)
mimmus
patch seemed decisive but now I'm getting again:
$ pulp --no-verify-ssl --username admin rpm repository sync --name cerved-amzn2-core --remote cerved-amzn2-core
password:
Started background task /pulp/api/v3/tasks/b86fb6f5-8e36-4c73-9d3f-a8852099e2e3/
..................Error: Task /pulp/api/v3/tasks/b86fb6f5-8e36-4c73-9d3f-a8852099e2e3/ failed: '403, message='Forbidden', url=URL('http://amazonlinux.us-east-1.amazonaws.com/blobstore/c29be7cc532fd89e7565ec6e981c8f441f1184259db1eb492aad42e7a1a1c2af/uuid-c++-1.6.2-26.amzn2.0.1.x86_64.rpm')'
mimmus
regression?
ggainey
mimmus: hullo - folk are off today, but I happened to notice this - yes, that feels like a regression. Sigh. Open an issue, refer to 8875, add in the remote's URL, I'll look at Monday
ggainey
(esp since I know which bug we fixed, that likely introduced the regression, and I'm the one that 'fixed' it...)
mimmus
:-) thank you!
ggainey
altho how the fix for 9464 could have regressed this, I am not sure
ggainey
sigh
mimmus
just before using Pulp in real world 😰
mimmus
https://pulp.plan.io/issues/9529
ggainey
thank you
ggainey
I can see what's going on - urlunparse undoes the quote() work, because It Knows Better
ggainey
argh
ggainey
mimmus: if I give you a patch in the next 5 min, are you game to test it?
mimmus
yes!
ggainey
what version of pulp_rpm are you on?
mimmus
how can I check?
mimmus
it's not a pkg, I installed by pulp_installer
mimmus
3.16.0 ?
ggainey
http :/pulp/api/v3/status/ will tell you the versions of all installed plugins
ggainey
(or "pulp status" if you have pulp-cli installed)
mimmus
"versions": [
{
"component": "core",
"version": "3.16.0"
},
{
"component": "rpm",
"version": "3.16.0"
}
],
ggainey
yup, cool
ggainey
ok, let me test this before I inflict it on you
mimmus
"inflict it on you" 🤣
ggainey
only fair, I inflicted the regression on you
ggainey
and it works - here's a diff : https://paste.centos.org/view/97b1259d
ggainey
unparse(), it turns out, "helpfully" undoes the quote() output back into ++ . When fixing the bnasic-auth problem, I didn't run the amazon test, and it isn't part of the auto-tests because syncing amazon-core takes too much time/space for CI runs. (not an excuse, btw, just explaining to myself how we/I missed this regression)
ggainey
fun!
ggainey
I'll add to that issue, one sec
This diff fixes the problem:
(pulp3) (3.16) ~/github/Pulp3/pulp_rpm $ git diff
diff --git a/pulp_rpm/app/downloaders.py b/pulp_rpm/app/downloaders.py
index 67bcb948..bd5a102b 100644
--- a/pulp_rpm/app/downloaders.py
+++ b/pulp_rpm/app/downloaders.py
@@ -69,8 +69,8 @@ class RpmDownloader(HttpDownloader):
# Let's make them happy - while not urlencoding **anything else**
parsed = urlparse(self.url)
new_path = quote(unquote(parsed.path), safe=":/")
- parsed._replace(path=new_path)
- new_url = urlunparse(parsed)
+ from urllib.parse import urljoin
+ new_url = urljoin(self.url, new_path)
if self.sles_auth_token:
auth_param = f"?{self.sles_auth_token}"
(pulp3) (3.16) ~/github/Pulp3/pulp_rpm $
Fixed regression in Amazon-repo-downloading encoding issue.
Note RE testing - currently only Amazon's httpd causes this behavior, and only on content-download. We'd have to have a fixture running behind a specifically-configured webserver to reproduce the behavior.
fixes #9529 [nocoverage]