copacetic
copacetic copied to clipboard
feat: add push flag to support pushing to registry directly
This PR enables direct push to registry from buildkit:
copa patch -i docker.io/library/alpine:3.16.4 -r patch.json -p ttl.sh/alpinepatched:1h
One side-effect of this would be that copa wouldn't rely on Docker anymore if there is a remote buildkit setup as I mentioned here. https://github.com/project-copacetic/copacetic/issues/285#issuecomment-1708239927
Closes #293
Codecov Report
Attention: 63 lines in your changes are missing coverage. Please review.
Comparison is base (
2b9f177) 33.02% compared to head (82060be) 33.49%.
Additional details and impacted files
@@ Coverage Diff @@
## main #294 +/- ##
==========================================
+ Coverage 33.02% 33.49% +0.46%
==========================================
Files 17 17
Lines 1626 1663 +37
==========================================
+ Hits 537 557 +20
- Misses 1060 1072 +12
- Partials 29 34 +5
| Files | Coverage Δ | |
|---|---|---|
| pkg/patch/cmd.go | 47.91% <25.00%> (-0.98%) |
:arrow_down: |
| pkg/patch/patch.go | 20.00% <46.34%> (+13.96%) |
:arrow_up: |
| pkg/buildkit/buildkit.go | 0.00% <0.00%> (ø) |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
I am wondering if we should align with buildx CLI here. Ideally, we don't want to break existing users so what I was thinking was to use --tag to accept registy/repo:tag and --push can be a bool. to remain backwards compat, we can parse if --tag includes : and /. If it includes both : and /, retag the entire image. If it doesn't include both : and /, only replace the tag.
-i foo.io/repo/image:tag -t -patched -> foo.io/repo/image:tag-patched (what we have today)
-i foo.io/repo/image:tag -t bar.io/repo/image:tag -> bar.io/repo/image:tag
-i foo.io/repo/image:tag -t bar.io/repo/image -> invalid (must provide tag)
-i foo.io/repo/image:tag -t repo/image:tag -> invalid (must provide registry)
@jeremyrickard @cpuguy83 wdyt?
I am wondering if we should align with buildx CLI here. Ideally, we don't want to break existing users so what I was thinking was to use
--tagto acceptregisty/repo:tagand--pushcan be a bool. to remain backwards compat, we can parse if--tagincludes:and/. If it includes both:and/, retag the entire image. If it doesn't include both:and/, only replace the tag.
-i foo.io/repo/image:tag -t -patched-> foo.io/repo/image:tag-patched (what we have today)-i foo.io/repo/image:tag -t bar.io/repo/image:tag-> bar.io/repo/image:tag-i foo.io/repo/image:tag -t bar.io/repo/image-> invalid (must provide tag)-i foo.io/repo/image:tag -t repo/image:tag-> invalid (must provide registry)@jeremyrickard @cpuguy83 wdyt?
I like the -i foo.io/repo/image:tag -t bar.io/repo/image:tag syntax because it's also very explicit.
I also think that adding a --push flag would be useful, especially if it didn't require docker locally. Not the same scenario, but the experience withko and pushing images w/o docker is super useful.
cool, happy to go with what you folks suggested.
Although, I am not sure about this bit:
-i foo.io/repo/image:tag -t repo/image:tag -> invalid (must provide registry)
This might still be what the user wants, partially due to how popular dockerhub is: username/image:tag is valid over there. Also, other such registries like ttl.sh also don't mind that format.
Even this bit, might be specific to how registries should manage it. Many registries would just accept it and tag it as latest(although, not encouraged):
-i foo.io/repo/image:tag -t bar.io/repo/image -> invalid (must provide tag)
I've pushed a commit to get most of what you folks suggested. Added couple test cases that shows we'll be backwards-compatible and a new case for specifying a different registry. I've tried to stay unopinionated about destination registry conventions as copa will throw an error during push if buildkit fails to push anyway. Happy to take your suggestions tho
I think setting up the mock buildkit API in unit tests failed, is it acting flaky?
This might still be what the user wants, partially due to how popular dockerhub is: username/image:tag is valid over there. Also, other such registries like ttl.sh also don't mind that format.
this is being consistent with -i since you must provide docker.io there
this is being consistent with -i since you must provide docker.io there
cool, I've added the registry check @sozercan
@sozercan @jeremyrickard don't mean to badger, is there anything else you'd like me to change on this PR?
@RealHarshThakur sorry for the delay - @sozercan has been on vacation for the past couple days. I'll see if I can get you a review asap :)
ping @RealHarshThakur. we are getting close to cutting v0.5.0 release if you want this to be included
I'll try but most likely, I'll be able to pick it up by end of the month
@sozercan sorry for the delay, I've made the changes you've requested.
@RealHarshThakur thanks! can you please rebase when you get a chance? looks there are a few conflicts now.
Closing due to inactivity. Please feel free to re-open if there's ongoing work. Thanks!