Enable fetching plugins from github repository #1970

jimil749 · 2021-08-07T06:35:16Z

This PR adds following changes:

Feature: Enables fetching reva-plugins from github, Users can provide following configuration:

[grpc.services.userprovider]
driver=https://github.com/jimil749/json

This configuration downloads the plugin source code into /var/reva/tmp/ext/<pluginType>/<driver>, compiles the code and loads it onto reva at runtime. Repo used for testing: https://github.com/jimil749/json

Fix: Use Struct based approach to encode and decode context kv pairs. Instead of sending map[interface{}]interface{} over RPC, we extract kv pairs into following struct:

type Ctx struct {
    User   []*userpb.User
    Token  []string
}

and send this struct over RPC.

PS: @ishank011 I did not test having github repo w/o go.mod because the traditional go get would be deprecated in the further go versions. If we still want to support plugins w/o go modules, we'd have to download the files into GOPATH for successful compilation.

update-docs · 2021-08-07T06:35:21Z

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

jimil749 · 2021-08-07T06:37:05Z

pkg/plugin/loader.go

@@ -82,14 +87,59 @@ func checkDirAndCompile(pluginType, driver string) (string, error) {
 	return bin, nil
 }

+// downloadPlugin downloads the plugin and stores it into local filesystem
+func downloadAndCompilePlugin(pluginType, driver string) (string, error) {
+	driverURL := strings.TrimPrefix(driver, "https://")


This is needed because of go-getter. It errors when url is of the type https://github.com/jimil749/..., it should be of the type: github.com/jimil749/...

This isn't really good. We need to support other protocols such as git as well. It even says there in the README.

Protocols are used to download files/directories using a specific mechanism. Example protocols are Git and HTTP.

We can pass the getters to the client https://github.com/hashicorp/go-getter/blob/main/client.go#L144 https://github.com/hashicorp/go-getter/blob/main/get.go#L67-L75

With the current mechanism, we are supporting Git protocol. The URL of the type github.com/jimil749/json would be turned into Git URL by go-getter. The go-getter uses Git protocol to fetch the repo.

The concept of a detector automatically turns invalid URLs into proper URLs. For example: "github.com/hashicorp/go-getter" would turn into a Git URL. Or "./foo" would turn into a file URL

Reason to trim https:// was because github.com/jimil/... is not a valid URL (w/o the https:// part), hence the url checker would fail if one entered URL of that kind.

We could get rid of this by changing the way we check the URL.

The user can pass paths beginning with other protocols, eg. http, git, file. We'll have to trim all these separately. Let's try the approach in the above comment

Ahhh, so we're supporting protocols other than git! User can just add a prefix as per their choices.

We don't actually need to pass the in-built getter because by default go-getter already uses those if not specified in the client (https://github.com/hashicorp/go-getter/blob/main/client_option.go#L29). So if we need to support multiple protocols and also not use trimming, we could ask the users to specify urls like: git::https://github.com/repo/plugin which would force the protocol desired by the user.

Doesn't look like a good option to me. We ask the use to specify the protocol, and then specify another to override it. I'd still prefer passing the getters at init.

Sure, just want to know that how would the user configure the plugins then. Could you provide an example of diff types of URLs/Protocol configuration?

Sure so

https://github.com/repo/plugin http://github.com/repo/plugin s3://my-s3-deployment.com/bucket/repo/plugin file:///root/repo/plugin.git

Ok, so the only ambiguity is b/w git and http protocol. Even if I pass the getters as mentioned here (which I think we don't need to because those are used by default if no getter is passed while instantiating the client), there would be ambiguity b/w git and http protocol. The correct way to use git protocol is by passing url like github.com/jimil749/json and not http://github.com/jimil749/json (wherein the getter would use http protocol). Sorry, but I am still confused as in what would passing the getter to Client achieve?

pkg/plugin/context.go

pkg/auth/rpc_auth.go

pkg/plugin/context.go

ishank011 · 2021-08-09T11:29:03Z

go.sum

@@ -1,5 +1,6 @@
 bou.ke/monkey v1.0.2 h1:kWcnsrCNUatbxncxR/ThdYqbytgOIArtYWqcQLQzKLI=


Please run go mod tidy once

Hmm. I ran go mod tidy after resolving the merge conflict! 🤔

pkg/plugin/loader.go

ishank011 · 2021-08-09T11:51:33Z

pkg/plugin/loader.go

@@ -82,14 +87,59 @@ func checkDirAndCompile(pluginType, driver string) (string, error) {
 	return bin, nil
 }

+// downloadPlugin downloads the plugin and stores it into local filesystem
+func downloadAndCompilePlugin(pluginType, driver string) (string, error) {
+	driverURL := strings.TrimPrefix(driver, "https://")


This isn't really good. We need to support other protocols such as git as well. It even says there in the README.

Protocols are used to download files/directories using a specific mechanism. Example protocols are Git and HTTP.

We can pass the getters to the client https://github.com/hashicorp/go-getter/blob/main/client.go#L144 https://github.com/hashicorp/go-getter/blob/main/get.go#L67-L75

ishank011 · 2021-08-09T14:12:50Z

examples/plugin/plugin.toml

@@ -9,7 +9,7 @@ address = "0.0.0.0:19000"
 userprovidersvc = "localhost:19000"

 [grpc.services.userprovider]
-driver = "./json"
+driver = "https://github.com/jimil749/json"


Also, just to demo the multiple ways of adding plugins, let's have one local and one hosted remotely

Sure, I'll do that.

lgtm-com · 2021-08-11T17:45:20Z

This pull request introduces 3 alerts when merging 160af2b into ec4099d - view on LGTM.com

new alerts:

3 for Useless assignment to local variable

jimil749 · 2021-08-11T17:47:02Z

I've pushed a couple of changes, let me know what do you think about them:

Go-getter: Now, instead of trimming, we ask user to specify the protocol that they want to use. They just need to append the url with the required protocol. So for git protocol, correct configuration would be git::https://github.com/jimil749/json.
Context: Use SetContext and GetContextStruct from plugin pkg to set and get contexts.
Compilation: Add cmd.Dir field to run the build command in the directory where the source code lies. This is because building a go project with go.mod from the current Reva directory is not possible.

Due to (3) the user needs to provide absolute path to the source in order to compile local plugins.

1. Feature: Enables fetching reva-plugins from github. 2. Fix: Use struct based approach to encode and decode context kv pairs Signed-off-by: Jimil Desai <jimildesai42@gmail.com>

Signed-off-by: Jimil Desai <jimildesai42@gmail.com>

1. Use ctx package to set and get contexts Signed-off-by: Jimil Desai <jimildesai42@gmail.com>

Signed-off-by: Jimil Desai <jimildesai42@gmail.com>

jimil749 · 2021-08-25T17:20:09Z

pkg/plugin/loader.go

@@ -39,24 +41,26 @@ type RevaPlugin struct {

 const dirname = "/var/tmp/reva"

-var isAlphaNum = regexp.MustCompile(`^[A-Za-z0-9]+$`).MatchString
+var isAlphaNum = regexp.MustCompile(`^[A-Za-z0-9_-]+$`).MatchString
+var isURL = regexp.MustCompile(`^(http:\/\/www\.|https:\/\/www\.|http:\/\/|https:\/\/)?[a-z0-9]+([\-\.]{1}[a-z0-9]+)*\.[a-z]{2,5}(:[0-9]{1,5})?(\/.*)?$`).MatchString


I have added this check, works for any URL:

github.com

https://github.com

www.github.com

http://github.com

Signed-off-by: Jimil Desai <jimildesai42@gmail.com>

jimil749 · 2021-08-25T17:26:00Z

examples/plugin/plugin.toml


 [grpc.services.userprovider.drivers.json]
 users = "users.demo.json"
+
+[grpc.services.authprovider]
+auth_manager = "github.com/cs3org/reva-auth-json-plugin"


I've added this example, but it would only work once cs3org/reva-auth-json-plugin#1 is merged.

jimil749 commented Aug 7, 2021

View reviewed changes

pkg/plugin/context.go Outdated Show resolved Hide resolved

jimil749 linked an issue Aug 7, 2021 that may be closed by this pull request

Enable fetching plugin from a github repository #1961

Open

ishank011 reviewed Aug 7, 2021

View reviewed changes

pkg/auth/rpc_auth.go Outdated Show resolved Hide resolved

pkg/plugin/context.go Outdated Show resolved Hide resolved

jimil749 mentioned this pull request Aug 9, 2021

Documentation for runtime-plugin #1971

Merged

ishank011 reviewed Aug 9, 2021

View reviewed changes

jimil749 force-pushed the github-plugins branch from f0b68f3 to aa9e8d3 Compare August 11, 2021 17:29

jimil749 marked this pull request as ready for review August 12, 2021 05:18

jimil749 requested a review from labkode as a code owner August 12, 2021 05:18

glpatcern added the GSoC label Aug 20, 2021

jimil749 added 9 commits August 25, 2021 22:34

This PR adds the following changes:

3f2c50d

1. Feature: Enables fetching reva-plugins from github. 2. Fix: Use struct based approach to encode and decode context kv pairs Signed-off-by: Jimil Desai <jimildesai42@gmail.com>

fix: update url check

f67c8cf

Signed-off-by: Jimil Desai <jimildesai42@gmail.com>

add comments

dc107e0

Signed-off-by: Jimil Desai <jimildesai42@gmail.com>

Updates:

2978c97

1. Use ctx package to set and get contexts Signed-off-by: Jimil Desai <jimildesai42@gmail.com>

add comment

48f3310

Signed-off-by: Jimil Desai <jimildesai42@gmail.com>

add err check in rpc_user

f575cbe

Signed-off-by: Jimil Desai <jimildesai42@gmail.com>

add license

28e9a1d

Signed-off-by: Jimil Desai <jimildesai42@gmail.com>

change cookie value to reva

b0bd1e2

Signed-off-by: Jimil Desai <jimildesai42@gmail.com>

update URL check and make handshake-config global

13ac310

Signed-off-by: Jimil Desai <jimildesai42@gmail.com>

jimil749 force-pushed the github-plugins branch from c33afb1 to 13ac310 Compare August 25, 2021 17:07

jimil749 commented Aug 25, 2021

View reviewed changes

jimil749 added 2 commits August 25, 2021 22:52

add comments

a784864

Signed-off-by: Jimil Desai <jimildesai42@gmail.com>

added github repo example

3bd3d6f

Signed-off-by: Jimil Desai <jimildesai42@gmail.com>

jimil749 commented Aug 25, 2021

View reviewed changes

jimil749 requested review from glpatcern and a team as code owners June 16, 2022 11:48

gmgigi96 force-pushed the master branch from af911b7 to a11f753 Compare November 21, 2022 12:19

gmgigi96 closed this Jul 19, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable fetching plugins from github repository #1970

Enable fetching plugins from github repository #1970

jimil749 commented Aug 7, 2021

update-docs bot commented Aug 7, 2021

jimil749 Aug 7, 2021

ishank011 Aug 9, 2021

jimil749 Aug 9, 2021

ishank011 Aug 9, 2021

jimil749 Aug 9, 2021

jimil749 Aug 11, 2021

ishank011 Aug 12, 2021

jimil749 Aug 12, 2021

ishank011 Aug 12, 2021

jimil749 Aug 12, 2021

ishank011 Aug 9, 2021

jimil749 Aug 25, 2021

ishank011 Aug 9, 2021

ishank011 Aug 9, 2021

jimil749 Aug 9, 2021

lgtm-com bot commented Aug 11, 2021

jimil749 commented Aug 11, 2021

jimil749 Aug 25, 2021

jimil749 Aug 25, 2021

		@@ -1,5 +1,6 @@
		bou.ke/monkey v1.0.2 h1:kWcnsrCNUatbxncxR/ThdYqbytgOIArtYWqcQLQzKLI=

Enable fetching plugins from github repository #1970

Enable fetching plugins from github repository #1970

Conversation

jimil749 commented Aug 7, 2021

update-docs bot commented Aug 7, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lgtm-com bot commented Aug 11, 2021

jimil749 commented Aug 11, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment