-
Notifications
You must be signed in to change notification settings - Fork 297
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
Add registry mirror support for pack build #1210
Conversation
40ef1f0
to
10397ec
Compare
Codecov Report
@@ Coverage Diff @@
## main #1210 +/- ##
==========================================
+ Coverage 80.96% 81.00% +0.05%
==========================================
Files 136 138 +2
Lines 8316 8435 +119
==========================================
+ Hits 6732 6832 +100
- Misses 1158 1172 +14
- Partials 426 431 +5
Flags with carried forward coverage won't be shown. Click here to find out more. |
bccf755
to
7d38c24
Compare
internal/image/fetcher.go
Outdated
name, err := pname.TranslateRegistry(name, f.registryMirrors) | ||
if err != nil { | ||
return nil, err | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just acknowledging that this line isn't tested. 😕
The implemented proxy per host or base url is interesting. [registry-mirrors]
"index.docker.io" = "nexus.mycompany.local/mirror"
"gcr.io/some/path" = "nexus.mycompany.local/mirror" I'm not sure where that requirement came from (I see it in the example config comment here) but mirrors are more commonly applied globally to any and all dependencies. I'd want to validate that this feature request does need a per host/base url type configuration.
Reference:
EDIT: Based on my comment and additional research I've been doing I think it's worth having the per host/base url mirrors BUT we should implement the wildcard option. |
@jromero This flexibility for per host/base url came from my experience using harbor as a proxy cache. At the very least, clients using the proxy cache feature of Harbor cannot set a global mirror. |
7d38c24
to
c039c75
Compare
TL;DR: we want both per base urls and wildcard (any). That makes sense although it does appear to be a harbor specific limitation. FWIW, the global mirror I was referring to is declared at the client side (pack in this case). It basically says, "for any image, I want you to go look it up at this registry" (ie. Harbor or Artifactory), then it's up to configuration in the registry to look up that image in configured remote (delegated) registries. In Artifactory's case you can setup multiple registries it can delegate/proxy so as a client you want "any" support so that it's up to the Artifactory to do the look up without having to manually map all base urls at the client side. |
c039c75
to
3f61659
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really super for my current use case, where I want to ensure all of our requests are proxied to Artifactory. Really happy to see this work @aemengo; superb as always.
One very confusing aspect of it for me, in UA, was the fetcher doesn't immediately say that it's pulling from the mirror. I cancelled it a number of times, because I didn't think I had set my mirror appropriately, until I got to the Status
line and saw that it did work appropriately. If you could make the image mirror explicit from the beginning, that would make the UX for me significantly better.
$ out/pack build test-go -B paketobuildpacks/builder:tiny -p ../paketo-samples/go/mod
tiny: Pulling from paketobuildpacks/builder
b734f5039f3d: Pull complete
...
Digest: sha256:e6e5fcd7c7094ed28f45b98e1528303f0f3304696e6bd737537c838cdd8f463f
Status: Downloaded newer image for <Artifactory-mirror>/paketobuildpacks/builder:tiny
tiny-cnb: Pulling from paketobuildpacks/run
4fb75c639a6b: Pull complete
...
Digest: sha256:b643257f93f44aa4e77c1c746fea14f419e29a2123829c7a228bc7f1b633fe2a
Status: Downloaded newer image for <Artifactory Mirror>/paketobuildpacks/run:tiny-cnb
...
If we could change the Pulling from
line to also reflect the registries mirror, that would be a slam-dunk from my end
Also adds a new config subcommand for registry-mirrors Signed-off-by: Anthony Emengo <[email protected]>
3f61659
to
9ed36d7
Compare
* Support wildcard registry mirror * Add registry-mirror alias Signed-off-by: Anthony Emengo <[email protected]>
9ed36d7
to
cfdac4a
Compare
It looks like we're not writing that line, it's being feed to us by the docker API. I don't know how you still feel about replacing it. To me, it's a bit unsavory. Maybe a big yellow text EDIT: see 95ed9769 $ pack build
Using mirror <MY-MIRROR>/dockerhub-mirror/paketobuildpacks/builder:full for index.docker.io/paketobuildpacks/builder:full
full: Pulling from dockerhub-mirror/paketobuildpacks/builder
Digest: sha256:7190dbb8177139fbfcdf9445f03420aaaa94832d8b3084e1a34870b11cf0d102
Status: Image is up to date for <MY-MIRROR>/dockerhub-mirror/paketobuildpacks/builder:full
Using mirror <MY-MIRROR>/dockerhub-mirror/paketobuildpacks/run:full-cnb for index.docker.io/paketobuildpacks/run:full-cnb
full-cnb: Pulling from dockerhub-mirror/paketobuildpacks/run
Digest: sha256:65972587451d7a0b6a375f9e39573c4272add20ce1c6a82c7d195e9e915e2796
Status: Image is up to date for <MY-MIRROR>/dockerhub-mirror/paketobuildpacks/run:full-cnb
Using mirror <MY-MIRROR>/dockerhub-mirror/paketobuildpacks/run:full-cnb for index.docker.io/paketobuildpacks/run:full-cnb
===> DETECTING
3 of 6 buildpacks participating
paketo-buildpacks/ca-certificates 2.3.2
paketo-buildpacks/go-dist 0.5.1
paketo-buildpacks/go-build 0.3.4
... |
Signed-off-by: Anthony Emengo <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary
Add support for a registry mirroring during pack build command. The use-case being enterprise environments where public registry access is denied.
If pack's
config.toml
is configured like:Then requests from registries
paketobuildpacks/builder
will be turned intonexus.mycompany.local/mirror/paketobuildpacks/builder
This PR also adds the config subcommands to manipulate these entries to config.toml.
Output
Before
$ pack build aemengo/hello full: Pulling from paketobuildpacks/builder Digest: sha256:2917564c2de52debd1df70d11c014b2708c8974e6e853804958825be84cfb859 Status: Image is up to date for paketobuildpacks/builder:full ...
After
$ pack build aemengo/hello full: Pulling from mirror/paketobuildpacks/builder Digest: sha256:2917564c2de52debd1df70d11c014b2708c8974e6e853804958825be84cfb859 Status: Image is up to date for nexus.mycompany.local/mirror/dockerhub-mirror/paketobuildpacks/builder:full ...
Caveats
image-tag
is not modified. Not sure if this is desired behavior or not. 🤷🏾♂️Documentation
Related
Resolves #821