Add proxy support for ImageRepository API#627
Conversation
matheuscscp
force-pushed
the
imagerepo-proxy
branch
from
cca60b8 to
57aa68a
Compare
stefanprodan
added
enhancement
stefanprodan
left a comment
stefanprodan
left a comment
There was a problem hiding this comment.
LGTM
Thanks @matheuscscp 🏅
matheuscscp
force-pushed
the
imagerepo-proxy
branch
from
2b3e8b5 to
14e9fa0
Compare
Signed-off-by: Matheus Pimenta <matheuscscp@gmail.com>
matheuscscp
force-pushed
the
imagerepo-proxy
branch
from
14e9fa0 to
b283a55
Compare
| auth, authErr = login.NewManager().Login(ctx, obj.Spec.Image, ref, opts) | ||
| var managerOpts []login.Option | ||
| if proxyURL != nil { | ||
| managerOpts = append(managerOpts, login.WithProxyURL(proxyURL)) | ||
| } |
There was a problem hiding this comment.
Why not login.NewManager(login.WithProxyURL(proxyURL)).Login(...), the nil check is done by the manager. This is how I did it in source-controller here: fluxcd/source-controller#1607
There was a problem hiding this comment.
I think login.NewManager(login.WithProxyURL(proxyURL)).Login(...) kinda suggests to the reader that there is always a proxy URL set in this case even though it may not be true (and this expression is also very long, and would read nicer if broken down, but that's unrelated to your specific suggestion, we could just break it in managerOpts := ...; manager := ...; manager.Login(...) and still do it as you say). This is how I did it for most of these optional parameters for Bucket: https://github.com/fluxcd/source-controller/blob/main/internal/controller/bucket_controller.go#L449-L537
3 participants
Fixes #612