Allow to use repo middleware wrapper during cross-repo mounts

[miminar]

In OpenShift, there's a pullthroughBlobStore middleware wrapper that acts like a proxy. It allows to pull blobs from remote registries based on per-repository configuration stored in etcd.

We definitely want to utilize the cross-repository mount functionality where the blob existing in other repository (in this case a remote registry) is mounted/linked into the target one if the client knows about it.

With this mount functionality working for the remote blobs, the registry won't store them on local storage. Which is what we aim for.

This PR allows to pass repository middleware wrapper to the Create() method of linkedBlobsStore() as an option. The method then calls it in order to Stat() the blob in the source repository.

Maybe there's a less intrusive way to achieve the same, please let me know if you have better idea.

/cc @stevvooe, @RichardScothern

[RichardScothern]

hi @miminar is your change to linkedBlobStore (changing the embedded type to an interface) enough to allow you to get your behavior purely though the middleware system?

i.e. you can use linkedBlob store with your own registry / repository types created via RegistryMiddlware?

[stevvooe]

@RichardScothern Might be interesting to look for a solution that integrates the existing proxy support, as that is very similar to the request. Although, I don't know that code well enough to provide a suggestion.

[RichardScothern]

I don't see any other way of controlling blob mounting functionality without a change like this. Blob mounting and the linked blob store are closely coupled. Using the term middleware to describe an override function like this is a bit confusing. Perhaps allowing a Create method to be passed into the linkedBlob store would be cleaner?

@miminar are you able to use any of the proxy functionality for your needs?

[miminar]

Thanks for your suggestions!

is your change to linkedBlobStore (changing the embedded type to an interface) enough to allow you to get your behavior purely though the middleware system?

Yes, I've already tested it.

@miminar are you able to use any of the proxy functionality for your needs?

We don't use distribution's proxy code. I believe we could use it in our blob store middleware for remote queries, but it doesn't address our problem. We can stat() remote blobs using the client library or proxy just fine. And it's not a propagation Create call to remote registry we're interested in. Just an ability to react to the mount request.

Perhaps allowing a Create method to be passed into the linkedBlob store would be cleaner?

I don't understand your suggestion. Our pullthroughBlobStore.Create() method wraps linkedBlobStore.Create(). The create request already goes through our code. If we could read options passed to the linkedBlobStore, we could solve the problem easily. Unfortunately createOptions is private, therefor we don't know whether the mount is requested nor the source registry.

If we pass our Create() to the linked blob store, it wouldn't understand the options either.

So the smallest change that would allow us to handle the mountFrom is just exporting createOptions but I'm not sure if it's less intrusive to the API than the current approach.

Using the term middleware to describe an override function like this is a bit confusing.

Would WithRepositoryFactory be acceptible name instead of WithRepositoryMiddlewareWrapper?

[mfojtik]

@RichardScothern bump :)

[RichardScothern]

hi sorry @miminar and @mfojtik, I was on holiday and forgot about this one.

Reading back through the comments I think perhaps it would be easier to just export createOptions into the top-level package and introspect the mount parameters from your blob store. This struct is already duplicated in two packages

[miminar]

Reading back through the comments I think perhaps it would be easier to just export createOptions into the top-level package and introspect the mount parameters from your blob store.

@RichardScothern awesome, I agree. I'll update this in a few.
Updated.

[codecov-io]

Current coverage is 52.53%

Merging #1757 into master will decrease coverage by 6.83%

@@             master      #1757   diff @@
==========================================
  Files           120        120          
  Lines         10477      10467    -10   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits           6220       5499   -721   
- Misses         3425       4230   +805   
+ Partials        832        738    -94   

Powered by Codecov. Last updated by 4e17ab5...3f14345

[legionus]

@RichardScothern What do you think about this now ?

[RichardScothern]

LGTM.

[miminar]

@RichardScothern thanks!

[miminar]

The CreateOption exposed is really useful to us. Nevertheless, we still need to be able to override the repository used to Stat() the blob in the source repository. I'll re-submit my original patch a bit modified.