docker / app

Make your Docker Compose applications reusable, and share them on Docker Hub
Apache License 2.0
1.57k stars 177 forks source link

Push/pull with cnab-to-oci #470

Closed simonferquel closed 5 years ago

simonferquel commented 5 years ago

Co-authored with @ijc

- What I did

- How I did it

Leveraged cnab-to-oci to store the built app-packages into an OCI index

- How to verify it

Unit and integration tests part of the PR Push/Pull subcommands for manual testing, as well as using other commands on registry refs

- Description for the changelog

Added push/pull of the CNAB bundles created by docker-app to any registry

doanac commented 5 years ago

I might be misunderstanding the nature of this, but I've hit a problem. I've done a helloworld.dockerapp with "name: doanac/hellow-world". Push and pull worked fine with:

$ bin/docker-app-linux push hello-world.dockerapp
$ bin/docker-app-linux pull doanac/hello-world:0.1.0

However, install fails with:

$ bin/docker-app-linux install doanac/hello-world:0.1.0
Error: invalid name "doanac/hello-world". Names must be [a-zA-Z0-9-_]+

Am I using the wrong name here?

codecov[bot] commented 5 years ago

Codecov Report

Merging #470 into master will increase coverage by 5.94%. The diff coverage is 50.75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #470      +/-   ##
==========================================
+ Coverage   63.43%   69.38%   +5.94%     
==========================================
  Files          71       50      -21     
  Lines        3733     2515    -1218     
==========================================
- Hits         2368     1745     -623     
+ Misses       1044      540     -504     
+ Partials      321      230      -91
Impacted Files Coverage Δ
cmd/docker-app/main.go 44.44% <0%> (-35.56%) :arrow_down:
internal/packager/extract.go 62.5% <0%> (-2.81%) :arrow_down:
cmd/docker-app/root.go 100% <100%> (+5.47%) :arrow_up:
cmd/docker-app/bundle.go 66.07% <100%> (-2.68%) :arrow_down:
cmd/docker-app/install.go 65.07% <100%> (-6.01%) :arrow_down:
cmd/docker-app/cnab.go 64.04% <27.27%> (-11.57%) :arrow_down:
cmd/docker-app/push.go 38.6% <35.33%> (-0.13%) :arrow_down:
cmd/docker-app/upgrade.go 65.3% <66.66%> (-4.54%) :arrow_down:
internal/store/store.go 71.42% <71.42%> (ø) :arrow_up:
cmd/docker-app/pull.go 80% <73.33%> (+16%) :arrow_up:
... and 27 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 c33123d...57ba688. Read the comment docs.

simonferquel commented 5 years ago

@doanac in your case, docker-app is complaining about the name of the installation (not the name of the package itself), which by default is the same as the app-package. We need to think about how we want to change this default name, as it is indeed a bad experience when the package name does not match the constraints of imposed on installation names. In the mean time, you can use docker-app install --name <installation-name> doanac/hello-world:0.1.0.

caervs commented 5 years ago

This is really cool!

Any chance we can change the behavior of --insecure-registries to not do cert validation instead of switching to http? That would be consistent with what insecure-registries means in the engine config so less confusing to users IMO

simonferquel commented 5 years ago

@caervs, by looking at the documentation (https://docs.docker.com/registry/insecure/) I think insecure-registries is there for using Plain HTTP in dockerd as well.

caervs commented 5 years ago

@simonferquel ah! You are correct. Https without cert validation comes first and then falling back to http.