Skip to content
This repository has been archived by the owner on May 3, 2022. It is now read-only.

Extract bundle structures in a different project #680

Closed
simonferquel opened this issue Mar 27, 2019 · 11 comments
Closed

Extract bundle structures in a different project #680

simonferquel opened this issue Mar 27, 2019 · 11 comments

Comments

@simonferquel
Copy link
Contributor

Yesterday @silvin-lubecki @michelleN and @radu-matei got stuck in a vendoring hell when trying to integrate cnab-to-oci into duffle. The main reason was that cnab-to-oci depends on duffle, just for using the same bundle structure (and makes it easier to work with code-bases that already leverage duffle such as docker-app).

The problem there is that there is a dependency loop when trying to integrate back cnab-to-oci into duffle to support OCI registries push/pull.

If we want to enrich Duffle in such ways (vendoring external projects in the cnab ecosystem), we need to break this dependency-loop, thus the need for having a different repository for holding common types.

What I suggest then, is to extract pkg/bundle to its own repository (something like deislabs/cnab-go), and vendor it from duffle, cnab-to-oci etc. Some projects might still need to vendor duffle (docker-app for example, leverages duffle implementation of the cnab runtime), but most won't need that.

Another bonus-side here, is that we will be able to iterate quicker on this repository to align those structures with the CNAB spec, and duffle will be able to update its support asynchronously.

@radu-matei
Copy link
Member

radu-matei commented Mar 27, 2019

See https://github.com/radu-matei/cnab-go, which is used in my fork of cnab-to-oci from #681

@simonferquel
Copy link
Contributor Author

I think we just need PKG/bundle (if it is not the case we should refactor so that the package is self-contained)

@radu-matei
Copy link
Member

Well, for cnab-to-oci we might only need the bundle package, but in other places (Porter, for example), we might need more than that -- and since we have been discussing about carving out a Go client for CNAB, this might be right time to look closely at it.

That being said, my repo is just an attempt to make things work, and it should not be necessarily used as a reference for a Go client (yet, at least).

@simonferquel
Copy link
Contributor Author

Hmm I guess we need to have a broader package slicing strategy then. I see different aspects and different slicing strategies. Basically, I identify those dependency rings:

  • cnab object model: basically structures implementing the cnab json schema
  • cnab validation logic: implementing validation rules specified in the cnab spec (immutable references, etc.)
  • claim system and parameter value / credential validation
  • CNAB runtime and drivers
  • top level tools

I see the immediate need of extracting the object model so that top level tools like duffle can vendor code from projects manipulating those objects (like cnab-to-oci)
We might want to evaluate if further slicing should be made (e.g.: do we want a common claim system, or a common CNAB runtime) etc.

@radu-matei
Copy link
Member

I do have a couple of thoughts about this as well:

  • I think we could treat the Go client like any other CNAB client we're working on
  • sure, I think we should have separate packages for the object models, validation, claims, runtimes (totally agree with @simonferquel here), but they are already in Duffle today as separate Go packages.
  • the easiest approach would be to have something along the lines of https://github.com/radu-matei/cnab-go -- which simply moves all of Duffle/pkg under a separate repo. Projects that only need the object model can only import that, and prune the rest of the unused packages.

My feeling is that if we separate them even more (each point @simonferquel made above into its separate repo), we would end up in a situation where we need to constantly lookup (and try) which version of the object model is used in the claim system, for example.

On the other hand, I'd also like to make sure that if we prune the unused deps correctly, pulling in the object model package (which should be self-contained, or at least with a minimal number of dependencies) does not bring constraints for other unused dependencies.

@simonferquel -- does something like this make sense?

@silvin-lubecki
Copy link
Contributor

On the other hand, I'd also like to make sure that if we prune the unused deps correctly, pulling in the object model package (which should be self-contained, or at least with a minimal number of dependencies) does not bring constraints for other unused dependencies.

I think we can eventually create a test which will run into a container, simulating a project vendoring the go client with only the bundle package, and checking the resulting GoPkg.lock. It will assure there is no regression on this side.

@carolynvs
Copy link
Contributor

carolynvs commented Mar 28, 2019

I would hate to see us split up duffle into too many repositories. It certainly increases the difficulty when making a change when it touches more than one repository.

Right now we have a clear need for a repository that helps other people who are writing code in Go to work with CNAB. That includes both the structs, and reusable logic. I don't see the need to break them up into separate repos where in other language repos they aren't. It adds overhead and at this point, I disagree that it is helpful to us right now.

That said, I do see things in Radu's fork that are quite specific to duffle that should move back into duffle. But that's something we can work out as we go. 😀

@silvin-lubecki 's suggestion for a consumer dependency regression test is great. I've used it in the past on dep and it works well.

@radu-matei
Copy link
Member

Oh, absolutely, my repo simply copied whatever was in duffle/pkg just to make one specific thing work, and it should not be used as a reference 😅

@michelleN
Copy link
Contributor

I think we should hold off on this for a bit just until the duffle repo is in sync with the current CNAB spec and has all of the latest and greatest changes. Right now, I am still refactoring pretty heavily in the background every time I touch pick up a new ticket/implement a feature.

@radu-matei
Copy link
Member

Ok, I mostly agree with that, @michelleN -- that being said, I tend to get back to @simonferquel's initial suggestion -- to start the package only with the contents of github.com/deislabs/duffe/pkg/bundle, which should be mostly stable and self-contained by now, and as we go on with refactoring and we have an actual need on packages for other projects, to move them over.

How does this sound for a strategy in the next couple of weeks?

@radu-matei
Copy link
Member

Implemented in https://github.com/deislabs/cnab-go

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants