genuinetools / img

Standalone, daemon-less, unprivileged Dockerfile and OCI compatible container image builder.
https://blog.jessfraz.com/post/building-container-images-securely-on-kubernetes/
MIT License
3.89k stars 230 forks source link

Replace CLI with same library docker CLI uses for maximum compatibility. #230

Closed kekoav closed 5 years ago

kekoav commented 5 years ago

What I did

I replaced the existing CLI interface with the same CLI library that is in use in docker/cli: spf13/cobra & spf13/pflag.

Changes

Why did I do that?

Img is touted as a drop-in replacement for docker commands. The easiest way for a user to adopt img is if the only modification to their code is s/docker/img/g. I'd like to improve compatibility and UX as much as possible to help drive adoption with no gotchas.

Detailed reasons

  1. CLI incompatibility generates a lot of issues, and is a headache for users wanting to adopt: #149, #203, #220, and maybe #201.
  2. The Go flag module is notoriously unforgiving with argument order, and doesn't provide a clean POSIX interface. This is a problem spf13/pflag addresses, while not throwing out the way Go flag works. I tried to be as light-handed as possible with changing the way the app is structured.
  3. By using the same library that docker/cli uses, we can more easily keep pace with CLI compatibility issues over the long term.
  4. Cobra is an awesome library, it is well known and used by many major projects, so it's a solid choice on it's own merits.

Other Feedback

I found it tricky to create a clean development environment to get the build working (all make commands). I added a Dockerfile.dev file which helped me along (based it off what I saw in travis config). Streamlining the process for a contributor would be helpful.

codecov-io commented 5 years ago

Codecov Report

Merging #230 into master will not change coverage. The diff coverage is 0%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #230    +/-   ##
======================================
  Coverage       0%     0%            
======================================
  Files          14     16     +2     
  Lines         782    945   +163     
======================================
- Misses        782    945   +163
Impacted Files Coverage Δ
push.go 0% <0%> (ø) :arrow_up:
unpack.go 0% <0%> (ø) :arrow_up:
pull.go 0% <0%> (ø) :arrow_up:
login.go 0% <0%> (ø) :arrow_up:
tag.go 0% <0%> (ø) :arrow_up:
main.go 0% <0%> (ø) :arrow_up:
diskusage.go 0% <0%> (ø) :arrow_up:
prune.go 0% <0%> (ø) :arrow_up:
build.go 0% <0%> (ø) :arrow_up:
list.go 0% <0%> (ø) :arrow_up:
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 158ee8b...876495f. Read the comment docs.

jessfraz commented 5 years ago

so sorry needs a rebase

kekoav commented 5 years ago

@jessfraz rebased, please check again.

AkihiroSuda commented 5 years ago

merged via #232