[Enhancement/Proposal] Update Plugin System

[someone1]

Continuing discussion started in Issue #248

The *sqlx.DB reference for plugins is required as when its loaded an immediate call to db.Ping() is made. I can open a separate issue to drop the reference altogether as the plugin should be able to manage the connection on its own and doesn't need the reference it returns to be passed into its own exposed functions as a parameter.

Additionally, it would be great to extend plugins for not only database backends but for signing mechanisms as well (e.g. for HashiCorp Vault or GCP IAM API).

Finally, I also propose to enhance the system in general and export minor parts of the bootstrap process for those of us that want to customize hydra directly vs loading a plugin.

[aeneasr]

I think all of them are very sensible requests. Happy to talk proposals and PRs (please discuss first)!

[someone1]

I actually have a PR for the first part done (database plugin) - I think it simplifies a lot of the database config process around hydra. I'll submit a PR shortly for this and maybe we can discuss the changes I made there (though no hard feelings if you reject it!)

I'd like to discuss the plugin extension around external JWT signing mechanisms - I think hydra needs to decouple the jwk bits by adding more interfaces like there is around the other parts of the code first.

[someone1]

Actually, just read the contributing document. Here is my work in a single commit.

The gist of the changes are:

• Introduce a backend interface for databases
• Create both memory and sql implementations of the new interface
• Update plugin load sequence to leverage new backed interface
• Update config/context loading for the new interface
• Update cmd/server bootstrap for the changes to config (greatly simplified)
• Enhancement to cmd/server: create an exported function that takes in a hydra.Config and spits out a http.Handler

It's a relatively small change architecture-wise (IMHO) - and it removes much more code than it adds while increasing flexibility of backends supported.

Let me know if you're open to a PR for this, I've run tests (with -short) and can follow the remaining steps in the contribution guide. Open to feedback as well!

[aeneasr]

This looks pretty solid. The dependency checks have been removed (they check if the dependency has been initialized and if not fatalf) which should definitely be re-added. I can't promise right now that this will stay 100% as is in the future as there might still be issues which we'll yet have to surface but from a high level perspective this looks good. Feel free to PR.

[aeneasr]

One thing to note is that I'm currently working on #904 which will refactor the way we initialize the router. I'm not sure if I should wait for your PR (which cleans up some stuff, I really like it) or ask you to rebase your changes once mine is done. Might come down to who is faster :D

[someone1]

How about I take that out of my PR and you put something in #904 to the same effect (for both the Admin and Public facing handlers)? It's really an oddball in the PR anyway, the entire handler.go file doesn't need to be touched for the changes to the Plugin system/new database backend interface.

I really just cut/paste out the handler code to an exported function NewHandler and moved out anything that was still changing the configuration object.

Glad to hear you like the changes, let me know which way you're leaning!

[aeneasr]

Ok sounds good, I'll keep refactoring #904 because my train is also delayed 2 hours so I have more than enough time to work on this ;) Then you can rebase your changes on top of that without touch handler.go. I'll incorporate some of your ideas around NewHandler into my PR.

[someone1]

Added a PR - reintroduced dependency checks as requested and added a test script to compile and load a database backend (just the memory backend using a different name).

Can wait/rebase if you want to finish your PR first.