feat: experimental plugin setup #269

coopernetes · 2023-08-27T16:45:14Z

This is a working PR for adding experimental plugin support to git-proxy.

ability to define custom behaviour as part of the proxied requests. This can be both runtime (user specified) as well as pre-defined or hard-coded plugins
define a minimum viable interface or method for defining this custom behaviour without breaking existing code too much
experiment to see if we can use this same method to integrate a call to an Open Policy Agent as described in Open Policy Agent checks #99

Related #47

Misc:

added a Dockerfile (fixes feat: Add Dockerfile for container-based deploys #275)

maoo · 2023-08-27T19:46:40Z

Quickly reviewed the code, I like the solution .

I'd be curious to see the implementation of a simple hello world plugin, and it seems that you just need one line to invoke a certain function (with a standard signature) to close the loop, if I'm not mistaken.

At that point we should already have a working solution for modularity at application level.

coopernetes · 2023-08-28T15:19:58Z

I'd be curious to see the implementation of a simple hello world plugin, and it seems that you just need one line to invoke a certain function (with a standard signature) to close the loop, if I'm not mistaken.

As part of this PR, I intend to create a trivial example plugin which the PluginManager can detect, load and present for git-proxy to execute against.

In terms of how plugins should expose their functionality, defining it via a function is fine as a first draft. However, I anticipate this can much more complex if we want to start modularizing specific layers such as the database/persistence, audit logs or events and different server types (HTTPS, SSH, email via #61). A single callable function can work for trivial use cases (particularly around checking commit information against a list of "good values" such as repo names, authors, etc).

For the purposes of externalizing the static configuration-based checks (resources/config.json) and providing an OPA option via #99 , we can stick with a simple request/response function that a plugin author can pass in. I do think we want to provide much more facilities for custom behaviour within git-proxy but we can iterate on that.

src/plugin.js

coopernetes · 2023-09-04T17:08:48Z

@JamieSlome I had to bump the nodejs workflow to Node 16.x as an FYI - adding this new dependency requires >= 14

@maoo working "hello world" example plugin as of c381bc5 :

App startup

$ npm run server

> @finos/git-proxy@1.0.0 server
> node index.js

Initializing plugin manager...
Found 1 plugin modules
Loaded 1 plugins
Service Listening on 8080
Listening on 8000

on push, plugin executes

/coopernetes/test-repo.git/info/refs?service=git-receive-pack
recieved
action processed
/coopernetes/test-repo.git/info/refs?service=git-receive-pack
recieved
action processed
/coopernetes/test-repo.git/git-receive-pack
recieved
Found 1, inserting into proxy chain
Inserting plugin [object Object] into chain
This is a message from the helloPlugin!

the remaining steps (`checkRepoInAuthorisedList`)

[
  {
    project: 'finos',
    name: 'git-proxy',
    url: 'https://github.com/finos/git-proxy.git',
    users: { canPush: [Array], canAuthorise: [Array] },
    _id: 'CoEwvG0trXnXkEfP'
  }
]
coopernetes/test-repo = finos/git-proxy
undefined
not found
checkRepoInAuthorisedList - repo coopernetes/test-repo.git is not in the authorisedList, ending
setting error
checkRepoInAuthorisedList - Rejecting repo coopernetes/test-repo.git not in the authorisedList
action processed
Rejecting repo coopernetes/test-repo.git not in the authorisedList

JamieSlome · 2023-09-05T08:38:49Z

@coopernetes - great PR 🎉 The Node bump should be fine, checks and tests are passing 👍

I have reviewed the code and as a first estimation, this is great. As you mention in your code, we can re-think the entry/loading point of the functions into the "chain".

I still need to play around with the code by running this locally, but from your proof of concept above this looks easy enough.

I don't think we need to strive for perfection on how the plugin manager works, so long as it gives us enough room for extensibility and modularity, i.e. expose custom (user-defined) and pre-defined (official/recommended) plugins to the "chain". We can always look to improve the sophistication later on.

package.json

coopernetes · 2023-09-06T18:41:55Z

src/plugin.js

+          localPlugins = localPlugins.concat(p);
+        } else {
+          packagePlugins = packagePlugins.concat(p);


TODO: simply push a new item into the plugin list instead of concat.

Suggested change

localPlugins = localPlugins.concat(p);

} else {

packagePlugins = packagePlugins.concat(p);

localPlugins.push(p);

} else {

packagePlugins.push(p);

* add Dockerfile & entrypoint script for container deployments

* on proxy startup, read plugin names via environment variable `GITPROXY_LOAD_PLUGINS` as a comma-separated list * on startup, initialize PluginManager which sets a list of plugins and loads them via `load-plugin`. Note, this is a no-op atm since the actual proxy processors are not calling into plugin functions yet * add lerna to support multiple npm packages in a single workspace. This is necessary to start breaking apart the monolithic git-proxy package code into separate packages. * add HttpPlugin class which takes in a request & response function. This class should implement those methods. Note, this is better suited to TypeScript which has built-in interfaces. This doesn't "do" anything yet. feat: plugin system re-ordered hardcoded values work in progress

- fixed bug with async execution of PluginManager initialization. Plugins by name or package are first resolved & loaded then we do the ProxyPlugin object casting once those run to completion via a Promise.all()

- remove list of Node module objects (this.pluginModules) in PluginManager. It's never used outside of loadPlugins() so its fine to convert to a local var

- pluginManager.plugins was capturing a nested Array ([][]ProxyPlugin) so fixed that up. - working through a bug in parsePackFile action

- upgrade nodejs workflow to 16.x. New dependencies won't build on <14 - pass in the ActionPlugin.exec function as part of the proxy chain - update JSDoc on ActionPlugin - add test for creating the PluginLoader

coopernetes · 2023-09-15T07:52:32Z

Test coverage isn't complete and some of the bits in the PluginLoader (renamed from PluginManager) would be pretty difficult to test since we're dynamically bringing in Node modules directly. Some additional JSDoc notes added and I've removed the hard-coded plugin that would be loaded on startup so that plugins are "opt-in" via the GITPROXY_PLUGIN_* environment variables. Confirmed this is working as intended behaviourally:

PluginLoader isn't invoked

$ npm run server

> @finos/git-proxy@1.0.0 server
> node index.js

Service Listening on 8080
Listening on 8000

PluginLoader running with "hello world" example

$ GITPROXY_PLUGIN_FILES='packages/git-proxy-notify-hello/index.js' npm run server

> @finos/git-proxy@1.0.0 server
> node index.js

Found 1 plugin modules
Loaded 1 plugins
Service Listening on 8080
Listening on 8000
/coopernetes/test-repo.git/info/refs?service=git-receive-pack
recieved
action processed
/coopernetes/test-repo.git/info/refs?service=git-receive-pack
recieved
action processed
/coopernetes/test-repo.git/git-receive-pack
recieved
Found 1, inserting into proxy chain
Inserting plugin [object Object] into chain
This is a message from the HelloPlugin!
[
  {
    project: 'finos',
    name: 'git-proxy',
    url: 'https://github.com/finos/git-proxy.git',
    users: { canPush: [Array], canAuthorise: [Array] },
    _id: 'CoEwvG0trXnXkEfP'
  },
  {
    project: 'finos',
    name: 'test-repo',
    url: 'https://github.com/finos/test-repo.git',
    users: { canPush: [Array], canAuthorise: [Array] },
    _id: 'zTmvyqXUda83ivbA'
  }
]
coopernetes/test-repo = finos/git-proxy
coopernetes/test-repo = finos/test-repo
undefined
not found
checkRepoInAuthorisedList - repo coopernetes/test-repo.git is not in the authorisedList, ending
setting error
checkRepoInAuthorisedList - Rejecting repo coopernetes/test-repo.git not in the authorisedList
action processed
Rejecting repo coopernetes/test-repo.git not in the authorisedList

Ready to merge as a prototype.

- closing in favour of finos#282

- unused lerna config deleted. This would be re-added only if Lerna is the tool of choice and when refactoring the existing src into discreet packages & workspaces

JamieSlome · 2023-09-15T08:30:19Z

@coopernetes - fabulous work! 🎉

Thanks for including some tests. I'll have a play around with the code at some point today but generally looks good to merge.

JamieSlome

LGTM! 🍰

maoo · 2023-09-19T10:48:21Z

This is huge! Thanks @coopernetes !

feat: experimental plugin setup

coopernetes force-pushed the experiment/plugin-opa branch from 100e337 to 33b96cd Compare August 27, 2023 16:48

coopernetes force-pushed the experiment/plugin-opa branch from 33b96cd to f252a6d Compare August 31, 2023 04:48

maoo reviewed Aug 31, 2023

View reviewed changes

src/plugin.js Outdated Show resolved Hide resolved

This comment was marked as outdated.

Sign in to view

coopernetes force-pushed the experiment/plugin-opa branch from 6024ac6 to cc4321c Compare September 4, 2023 15:11

coopernetes commented Sep 6, 2023

View reviewed changes

package.json Outdated Show resolved Hide resolved

coopernetes commented Sep 6, 2023

View reviewed changes

coopernetes force-pushed the experiment/plugin-opa branch from c381bc5 to 4c0d0ba Compare September 15, 2023 05:05

coopernetes added 6 commits September 15, 2023 03:46

feat: docker setup

7c2d7ca

* add Dockerfile & entrypoint script for container deployments

feat: setup pluginmanager state

42e14d7

- fixed bug with async execution of PluginManager initialization. Plugins by name or package are first resolved & loaded then we do the ProxyPlugin object casting once those run to completion via a Promise.all()

chore: remove extra state

fcf5709

- remove list of Node module objects (this.pluginModules) in PluginManager. It's never used outside of loadPlugins() so its fine to convert to a local var

fix: use ProxyPlugin objs correctly

e8559b8

- pluginManager.plugins was capturing a nested Array ([][]ProxyPlugin) so fixed that up. - working through a bug in parsePackFile action

feat: actionplugin exec function + node 16

157a24a

- upgrade nodejs workflow to 16.x. New dependencies won't build on <14 - pass in the ActionPlugin.exec function as part of the proxy chain - update JSDoc on ActionPlugin - add test for creating the PluginLoader

coopernetes force-pushed the experiment/plugin-opa branch from 4c0d0ba to 157a24a Compare September 15, 2023 07:46

coopernetes marked this pull request as ready for review September 15, 2023 07:46

coopernetes added 2 commits September 15, 2023 04:11

chore: remove docker

c5cd565

- closing in favour of finos#282

chore: remove lerna config

f7ca203

- unused lerna config deleted. This would be re-added only if Lerna is the tool of choice and when refactoring the existing src into discreet packages & workspaces

coopernetes mentioned this pull request Sep 15, 2023

feat: Add Dockerfile for container-based deploys #275 (solved) #282

Closed

Merge branch 'main' into experiment/plugin-opa

8f4b9e5

JamieSlome approved these changes Sep 19, 2023

View reviewed changes

JamieSlome merged commit ea14ce8 into finos:main Sep 19, 2023

JamieSlome mentioned this pull request Sep 19, 2023

feat: Make git-proxy executable, custom config file arg & validation, bump to 1.1.0 #287

Merged

coopernetes pushed a commit to coopernetes/git-proxy that referenced this pull request Oct 13, 2023

Merge pull request finos#269 from coopernetes/experiment/plugin-opa

520db94

feat: experimental plugin setup

coopernetes deleted the experiment/plugin-opa branch January 24, 2024 05:20

Psingle20 pushed a commit to Psingle20/git-proxy that referenced this pull request Nov 27, 2024

Merge pull request finos#269 from coopernetes/experiment/plugin-opa

70512da

feat: experimental plugin setup

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: experimental plugin setup #269

feat: experimental plugin setup #269

coopernetes commented Aug 27, 2023 •

edited

Loading

maoo commented Aug 27, 2023 •

edited

Loading

coopernetes commented Aug 28, 2023

This comment was marked as outdated.

coopernetes commented Sep 4, 2023

JamieSlome commented Sep 5, 2023

coopernetes Sep 6, 2023

coopernetes commented Sep 15, 2023

JamieSlome commented Sep 15, 2023

JamieSlome left a comment

maoo commented Sep 19, 2023

feat: experimental plugin setup #269

feat: experimental plugin setup #269

Conversation

coopernetes commented Aug 27, 2023 • edited Loading

maoo commented Aug 27, 2023 • edited Loading

coopernetes commented Aug 28, 2023

This comment was marked as outdated.

coopernetes commented Sep 4, 2023

App startup

on push, plugin executes

the remaining steps (checkRepoInAuthorisedList)

JamieSlome commented Sep 5, 2023

coopernetes Sep 6, 2023

Choose a reason for hiding this comment

coopernetes commented Sep 15, 2023

PluginLoader isn't invoked

PluginLoader running with "hello world" example

JamieSlome commented Sep 15, 2023

JamieSlome left a comment

Choose a reason for hiding this comment

maoo commented Sep 19, 2023

coopernetes commented Aug 27, 2023 •

edited

Loading

maoo commented Aug 27, 2023 •

edited

Loading

the remaining steps (`checkRepoInAuthorisedList`)