osbuild / images

Image builder image definition library
Apache License 2.0
23 stars 52 forks source link

ostree: fix proxy parsing (HMS-5018) #1050

Closed lzap closed 2 hours ago

lzap commented 6 hours ago

Finally found the ostree resolving problem - it was in proxy parsing. The configuration value is not in URL form, it is in hostname:port form. And url.Parse silently parses such URI setting a nonsense scheme to hostname. TIL that URL in Go actually is technically URI: https://pkg.go.dev/net/url

lzap commented 5 hours ago

I did split the OTK hunks into a separate PR: https://github.com/osbuild/images/pull/1051

mvo5 commented 3 hours ago

Would it make sense to add a regression test here? Also in the "test-as-documentation" sense this might be useful(?)

ezr-ondrej commented 3 hours ago

It would make sense to add test for sure... I'd prefer a follow-up tho, I wouldn't like to block this, unless someone disagrees.

lzap commented 2 hours ago

Would it make sense to add a regression test here?

I can do a followup, I actually tried by running a tiny HTTP proxy via library but it was not great test to be honest.

mvo5 commented 2 hours ago

Would it make sense to add a regression test here?

I can do a followup, I actually tried by running a tiny HTTP proxy via library but it was not great test to be honest.

Cool, we don't need to block on this, I had a quick look and opened https://github.com/osbuild/images/pull/1052 - it's not ideal as it's not a full integration test but OTOH I think the extraction of the helper is beneficial and it does check that the proxy is correctly set (but it does know a bit too much about the implementation details for my taste).