Runtime Plugins: Design & integration

[jimil749]

As discussed, this issues is opened to have discussions on how to implement the plugins pkg into the reva codebase. The primary candidate for plugin system is the hashicorp go-plugin, which supports plugins over RPC and gRPC.

[jimil749]

Just to initiate the discussion, here are my initial thoughts:

Plugin Package

Firstly, in order to facilitate the plugins, we would require an extra plugins package. This package would be primarily responsible for:

1. Plugin Discorvery: For the case, where we have plugin code, hosted as version control repo, this package should be responsible for downloading the source code into a pre-defined folder.
2. Loading and Compiling Plugin: This package should have a Load method responsible for loading in the plugins. This method would return an interface{}, which can be then asserted appropriately by the services to fit their use-case.

The plugin package gives the services, "ready to use" implementation of the driver. Each service should use this package to get their respective drivers.

The plugins package should be called by the services. (authprovider, userprovider, storageprovider etc). Unlike earlier, wherein the services fetch the drivers from the in-memory registry, the services should now fetch the drivers (or rather, the implementation) from the plugins package.

Drivers

Each driver needs to expose an interface for the plugins to implement. I think we already have interfaces for each drivers in their respective packages. Now, along with the exposed interfaces, each driver package should also have hashicorp's plugin.Plugin implementation. This implementation has following methods:

func GRPCServer() // returns grpc server
func GRPCClient() // returns  grpc client 

These methods are responsible for starting the gRPC/RPC (based on the protocol) server/client. The host i.e reva core acts as an rpc client, whereas the plugin acs as an rpc server. The rpc server needs to be started by the plugin code whereas the rpc client, is started by the plugins package (in reva codebase).

Thoughts?
/cc @ishank011 @refs @labkode

[jimil749]

In order to get more clearer understanding, I tried to write a small "POC" for the plugins package. I've added the code in my local branch here: https://github.com/cs3org/reva/compare/master...jimil749:runtime-plugins?expand=1 And I have the plugin code as a github gist setup here: https://gist.github.com/jimil749/e439e8ebdbb5c8b82d122311b42b92f3. I've added an extra UserManager interface with an extra New method to initialize the plugin struct with relevant configurations. Also for the purpose of showcasing, I've made a couple of changes to the configurations:

type pluginConfig struct{
   driver string
   location string // location of the plugin binary
   users string // plugin configuration
}

Also, just a question: do we want to parse the plugin configurations at the service level and pass them to the plugins or have plugins parse the configurations (just pass map[string]interface{})?

[labkode]

@jimil749 I would like this, to ensure backwards compatiblity:

[grpc.services.userprovider]
# 3 ways to load the plugin
driver = "json"
driver = "/path/to/userprovider.bin"
driver = "github.com/jimil740/userprovider"

The plugin configuration doesn't change
[grpc.services.userprovider.json]
users = /tmp/users.json

The plugin creation should also take the map[string]interface{} to ensure backwards compatibility.

[jimil749]

@labkode sure, that makes sense! Plugins should take in the map[string]interface{} and decode the configs themselves (I tried testing this, and seems to work fine 👍 ). And yes, in order to ensure backward compatibility we can have configurations as you suggested, but my only concern is that then we'd need some mechanism to identify what way is the user trying to load the plugin. May we can have something like this:

[grpc.services.userprovider]
driver = "json"   // load from in-memory registry (like we do now)
driverPath = "/path/to/userprovider.bin"  //  load external plugin from the give path 
driverURL = "github.com/jimil749/userprovider"  //   get the plugin from an external version control system

PS: Do we want to support loading plugins from the in-memory registry (which is initialized at the init stage), after addition of runtime plugins?

[labkode]

@jimil749 I think the logic could be:

if driver starts with / points to local plugin
if driver contains only a single word points to in-memory registry
if driver contains an url it points to a remote repository
else abort

Yes, we still need to be able to load plugins from the in-memory registry as these are core plugins that come out of the box with Reva

[jimil749]

Sure, that'll work. Thanks for the clarification! So, i think for the most part the configuration (toml file) would not involve any major changes. The inputs can be processed as suggested.

The next part that I wanted to discuss was the protocol that we'll be using. We have a choice b/w rpc and grpc. The only issues with grpc is that we need to generate new protobuf binding, with the added New method, this is to initialize the plugin with the configuration data, other than that, we'd be fine with either of the protocol. @ishank011, you have any inputs here?

[ishank011]

Hi Jimil. Sorry for the delay.

The plugin creation should also take the map[string]interface{} to ensure backwards compatibility.

Totally agreed

driverPath = "/path/to/userprovider.bin" // load external plugin from the give path

I'm not sure 100% sure about this. We can support this as well but this shouldn't be the primary method in which we access runtime drivers. The whole binary management should be taken care of by the plugins pkg. You specify something like driver = "json" (preferred way) or driver = "pkg/user/manager/json" and the rest should be automatically handled. We can have a global flag as discussed during the proposal stage which can indicate whether we want to use the registry or generate the binaries.

The next part that I wanted to discuss was the protocol that we'll be using. We have a choice b/w rpc and grpc.

I think rpc should be good enough as discussed. I want to avoid having two versions of almost similar bindings. Defining the request and response types for the individual calls like you did should work.

I'll also look into yaegi today to see if we can figure out the protobuf issue. Also, please create a PR once you start with the implementation and then we can continue on that; makes reviewing easier.

[jimil749]

driverPath = "/path/to/userprovider.bin" // load external plugin from the give path

I'm not sure 100% sure about this. We can support this as well but this shouldn't be the primary method in which we access runtime drivers. The whole binary management should be taken care of by the plugins pkg. You specify something like driver = "json" (preferred way) or driver = "pkg/user/manager/json" and the rest should be automatically handled. We can have a global flag as discussed during the proposal stage which can indicate whether we want to use the registry or generate the binaries.

Agreed. So we'd just need to point towards the source code. The plugins pkg should be responsible for "compiling" and then loading the binary. And yes, we could add a flag to choose b/w in-memory registry or the other way. That makes sense! 👍

The next part that I wanted to discuss was the protocol that we'll be using. We have a choice b/w rpc and grpc.

I think rpc should be good enough as discussed. I want to avoid having two versions of almost similar bindings. Defining the request and response types for the individual calls like you did should work.

Sure! Sounds good.

I'll also look into yaegi today to see if we can figure out the protobuf issue. Also, please create a PR once you start with the implementation and then we can continue on that; makes reviewing easier.

Will do that. I'll start with implementing the plugins package, and try to move the userprovider json plugin out as runtime driver.

[ishank011]

Perfect!

[jimil749]

Also, @ishank011, yaegi does not support cgo, (traefik/yaegi#780), this can cause issues with some drivers.

[ishank011]

Ah! Let's not waste time on it then, there might be even more surprises

[labkode]

I'm not sure 100% sure about this. We can support this as well but this shouldn't be the primary method in which we access runtime drivers. The whole binary management should be taken care of by the plugins pkg. You specify something like driver = "json" (preferred way) or driver = "pkg/user/manager/json" and the rest should be automatically handled. We can have a global flag as discussed during the proposal stage which can indicate whether we want to use the registry or generate the binaries.

There is another requirement, we need to be able to load a compiled plugin already (we won't have access to source code).
The loading by path is still needed.

[jimil749]

We can have a global flag as discussed during the proposal stage which can indicate whether we want to use the registry or generate the binaries.

Just a query here @ishank011, if we add a global "plugin" flag to indicate whether we want to use the registry or external plugins, wouldn't that enforce all the services to use runtime drivers?

[ishank011]

There is another requirement, we need to be able to load a compiled plugin already (we won't have access to source code).
The loading by path is still needed.

Ah okay, we should support that as well then

if we add a global "plugin" flag to indicate whether we want to use the registry or external plugins, wouldn't that enforce all the services to use runtime drivers?

Yep, that's the plan for the long term. For the time being, you can use workarounds for testing

[labkode]

There is another requirement, we need to be able to load a compiled plugin already (we won't have access to source code).
The loading by path is still needed.

Ah okay, we should support that as well then

The reason for this one comes from ownCloud.
Some plugins will be closed-source, usually done for enterprise subscriptions or third-party applications.
So you get a black box basically.

[jimil749]

Continuing the discussion with compilation of plugins:
I have a question regarding this, so we'd want to compile the plugin whenever the user provides an external github url, right? We'd need to incorporate that first. For that, maybe we could use: https://github.com/go-git/go-git package to clone the code locally and then compile it?

[ishank011]

Yep that should work but first, let's take care of local drivers. For such drivers, the plugins should be generated at runtime (once we reach the hot-reloading stage) or compile time for now.

[jimil749]

Okay, so as for the local drivers, we need to have a pre-defined directory (maybe we could have reva/plugins/userprovider/<driver_type>) to store the binaries. Or maybe we can even store them in some temp directory and clean it once we are done.

As for the compilation part:

1. We could simply use the exec package to run go build -o json json.go to compile the plugin via go source code. (And then we could load that binary just like we do it now)
2. And then we could add a watcher like fsnotify on top of that, which could watch for write events and we fire the exec command again to get the new version of the binary. (We can talk more about this in the hot-reload stage)

Also @ishank011,

I'm not sure 100% sure about this. We can support this as well but this shouldn't be the primary method in which we access runtime drivers. The whole binary management should be taken care of by the plugins pkg. You specify something like driver = "json" (preferred way) or driver = "pkg/user/manager/json" and the rest should be automatically handled. We can have a global flag as discussed during the proposal stage which can indicate whether we want to use the registry or generate the binaries.

here you mentioned that when we specify driver = "json", the reva core should compile the plugin and load it, do we have some sort of pre-defined directory where the plugin source is situated? I am a bit confused as in how would this work?

[ishank011]

Okay, so as for the local drivers, we need to have a pre-defined directory

Yep, sounds good. We can use reva/bin/...

Regarding just specifying the plugin driver name, the motivation is to have backward compatibility with the way we currently configure it. If we were sure that all the locally present userprovider drivers were in the same parent pkg, we could have added this part to where we register the userprovider plugin, but this is not the case. So yeah, we can specify the full path, but switching between the plugin and prod mode will be a pain. Try to think of a way around this, I will as well.

[jimil749]

@ishank011, by switching b/w plugin and prod mode, do you refer to switching b/w compiling and loading the plugins, and loading the provided binary? Or are you referring to switching between the registry mode (like it happens now) and plugin mode?
The latter isn't an issue, as the user can easily provide the relevant configuration.

For the former, one solution can be (this may sound trivial) to have another sharedconf configuration. We could have a compile configuration/flag which when set true, will compile the plugins. This would only be the case for local drivers, as when the user provides a github url, compilation is inevitable.

[ishank011]

So when plugin = false,

• driver = "json" -> in-memory registry
• driver = "github.com/reva-pkgs/user/custom" -> fetch, compile in a temp dir, use over rpc

and when plugin = true,

• driver = "pkg/user/json" -> stat this path, check if it's binary file, if yes, use directly, else compile + fire up watcher
• driver = "github.com/reva-pkgs/user/custom" -> fetch, compile in a temp dir, use over rpc

My point is that when switching the plugin flag, we'd have to change the path even if we want to use the same package (from json to pkg/user/json) and this will have to be done for all packages, which is not the most ideal thing. Makes sense?

[ishank011]

driver = "pkg/user/json" -> stat this path, check if it's binary file, if yes, use directly, else compile + fire up watcher

We can consider starting a watcher over existing binaries as well

[jimil749]

So when plugin = false,

* driver = "json" -> in-memory registry

* driver = "github.com/reva-pkgs/user/custom" -> fetch, compile in a temp dir, use over rpc

and when plugin = true,

* driver = "pkg/user/json" -> stat this path, check if it's binary file, if yes, use directly, else compile + fire up watcher

* driver = "github.com/reva-pkgs/user/custom" -> fetch, compile in a temp dir, use over rpc

My point is that when switching the plugin flag, we'd have to change the path even if we want to use the same package (from json to pkg/user/json) and this will have to be done for all packages, which is not the most ideal thing. Makes sense?

Yep, this makes sense. Thanks for the elaborate description. So whenever we have the plugin = true, the configuration for driver needs to have the entire path as opposed to just having the driver name (because we cannot just assume that the driver source would be located under pkg/user/ directory, neither can we enforce the user to have source under a particular directory). This creates a bit of discrepancy between the plugin mode and the registry mode.

[jimil749]

So @ishank011, in an ideal world we'd want to have the same configuration i.e driver = json, for both the plugin mode and the registry mode to ensure backward compatibility and consistency. Right?

[ishank011]

Yep

[jimil749]

@ishank011, this is a tricky situation. If we think from the user perspective, shouldn't it be a bit convenient to provide the path to source code instead of just having the name of the driver?

[jimil749]

@ishank011 talking about the "fetching" stage (getting plugins from github) I tested the working of hashicorp's go-getter package. It seems to suitable for our use case. I had a small question about the same. Do we download the source code into some /tmp directory or inside the reva sub-directory? Also when where should the binaries be stored after compilation?

[jimil749]

So I was trying to add the go-getter library in the plugin pkg and it seems to be working. I used the go-getter library to download the plugins from github (https://github.com/jimil749/json) into ./remote-plugins/userprovider/json/. I then used the compile func to compile the package into an exec (stored at ./bin/userprovider/json) and loaded the executable. The only constraint that we have is that the name of the project/repo should be the name of the driver!