Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add plugin API #341

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: Add plugin API #341

wants to merge 1 commit into from

Conversation

trygve-lie
Copy link
Contributor

Add plugin API

This is a first draft on a plugin API for the Eik server and is intended at this stage to be the base for discussion.

Why a plugin API?

There are currently a couple of ideas floating around where we would like to be able to tap into the routes of the REST API and run operations against third party systems. This might best be described by an example:

The Eik server should always have a http cache in front of it. We do not want to bind Eik to a specific http cache so what http cache one put in front of the Eik server can be anything. Having a http cache in front of the Eik server adds complexity in terms of files which have been read do in theory live two places; in the storage of the Eik server and in the http cache.

Currently the Eik server does not provide an http endpoint to delete files and this is mostly due to the fact that we do not really want to delete files on Eik, but there are corner cases where we have too delete a file. So, we do want to add an http endpoint for deleting files. Though, the problem with adding a delete endpoint today is that we're only able to delete the files in the storage of the Eik server. Today we have no way to also purge (delete) the same files from the http cache which live in front of the Eik server since that http cache can be anything.

With a pluging API we can implement a purge plugin suited to purge files in the http cache server one have chosen to put in front of the Eik server (if the http cache has such a feature) and plug that into the server.

Some Eik core concepts

In the Eik server there are a couple of key components and concepts we use in all http endpoints which is handy to know about:

  • When a requests come into a http endpoint, we sanitize and parse out a set of values about the request. These values together with the raw request object is set on a HttpIncoming object.
  • The HttpIncoming object is then handed over to a parser. Parser's are the part which does the unique work for each route. Depending on the route they perform stuff like extracting uploaded files and saving them in the storage, load a file from the storage and serve it, etc, etc,
  • When a parser is done a HttpOutgoing object is created and filled with values from the operations the parser did. The HttpOutgoing object is then used to form a http response of the http endpoint.

Initial plugin thoughts

These are some of the initial thought of the features and limitations of a plugin:

  • Plugins must be bound to http endpoints and not the underlying implementation. The reason for this is because we do ex use the same underlying implementation for endpoints under both the npm and pkg namespace. In other words, a plugin should be able to run one set of logic on a endpoint under the npm namespace and a different set of logic on a endpoint under the pkg namespace despite the fact that the Eik server uses the same code for both namespaces.
  • Plugins should be able to hook into multiple steps of the life cycle of a request.
  • Its not the role of a plugin to alter the input or output of a request.
  • Plugins should be self contained and not exchange data between each other.
  • Plugins should be executed async and in paralell for each step they are hooked into.

Please feel free to question these thought!

How does this work?

The current implementation is so that there is a registry of registered plugins. A plugin is a object which has a set of methods where each method is intended to be on different locations in the request cycle plus some other utility methods. In this POC one can hook into the start and end of the request cycle but its also possible to make hooks run on ex when a file is extracted form a uploaded tar.

A plugin looks something like this:

import Plugin from './plugin.js';

const PluginCustom = class PluginCustom extends Plugin {
    constructor() {
        super();
    }

    onRequestStart(incoming) {
        if (incoming.handle !== 'put:pkg:version') {
            return;
        }

        return new Promise((resolve, reject) => {
            // do some stuff
            resolve();
        });
    }

    onRequestEnd(incoming, outgoing) {
        return new Promise((resolve, reject) => {
            // do something else
            resolve();
        });
    }

    get [Symbol.toStringTag]() {
        return 'PluginCustom';
    }
}

Each hook in the life cycle of a request will execute the associated method in all the plugins in the registry in parallel. Each method should always be handed HttpIncoming as the first argument and possibly other more specific arguments depending on where one are in the request life cycle. Ex; if there is a hook for each extracted file in a tar, this hook should probably provide the meta data object for that file as an argument to this hook.

Inside each method its then possible to check the .handle property of HttpIncoming to filter on what API and HTTP method one run an operation.

This does also follow the same approach as we do in the sinks where there is a interface class the plugin must extend upon. This is there to secure that all methods in a plugin is present (but just returns nothing) making it possible to just implement the hooks in the request life cycle one want to do some logic in.

In other words; a plugin which only does something on the start of a request looks like so:

import Plugin from './plugin.js';

const PluginCustom = class PluginCustom extends Plugin {
    constructor() {
        super();
    }

    onRequestStart(incoming) {
        if (incoming.handle !== 'put:pkg:version') {
            return;
        }

        return new Promise((resolve, reject) => {
            // do some stuff
            resolve();
        });
    }

    get [Symbol.toStringTag]() {
        return 'PluginCustom';
    }
}

The plugin interface should be published as an independent module in the same way as the interface for the sink.

Copy link
Member

@digitalsadhu digitalsadhu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. What’s the reason for not allowing plugins to manipulate the request in any way? I could imagine a case where you hook in and then attach additional metadata on the outgoing.

Eg.

  1. Add a description field to each route.
  2. reformat the api output to support a schema such as jsonapi
  3. Change cors headers?

@trygve-lie
Copy link
Contributor Author

What’s the reason for not allowing plugins to manipulate the request in any way? I could imagine a case where you hook in and then attach additional metadata on the outgoing.

I am not 100% sure what we want and need here. But I'm thinking this can be a rabbit hole.

Its fully possible to make this so a plugin can change everything on the output (and even the upload). One can ex make it so that one can plug in a transform on the output stream of a file and one can go crazy. But is that really what we want to do? I am not 100% sure if we want to open up so much that one can plug in ex a full transform step (ex applying minifying) in the output of the request. I am kinda thinking that if we want Eik to ex transform something its something we build into Eik.

One could let plugins alter stuff like the http headers being set etc but is that again something we want? Today there is very simple config for configuring some http headers so if we want to let plugins do so, we kinda need to make a stand if configuration of http headers should happen by a config or by writing a plugin. Personally since Eik is a set of http endpoints I find it most naturally that this is something one tune through a config and not by writing a plugin if one need to tune http headers.

One reason to not let a plugin alter output is that plugins currently are by nature async and if there are more than one plugin registered the operation is run in parallel. The benefit here is that if one have two plugins which is ex communicating with third party services (ex one plugin for purging stuff in Fastly and ex one plugin sending some stats to a metric system) they can be run in parallel if they do not alter the HttpIncoming or HttpOutgoing. If we allow them to alter HttpIncoming or HttpOutgoing we need to run them in sync to avoid race conditions.

There is though use cases for letting one plugin set a value in one hook and then use that in a later hook. If one ex want to do a metric plugin that is something one would like to do to ex time the length of a request. Doing so will not interfere with running plugins in parallel. So, I think this should be possible.

@benja
Copy link

benja commented Oct 28, 2022

Nice write-up, @trygve-lie!

There seem to be a few things to tackle here.

First I think we have to understand the goal of an Eik plugin: is it to alter underlying Eik functionality and make it behave differently from how we intended it to be, or is it only supposed to add functionality on top of an opinionated library?

Plugins must be bound to http endpoints and not the underlying implementation. The reason for this is because we do ex use the same underlying implementation for endpoints under both the npm and pkg namespace. In other words, a plugin should be able to run one set of logic on a endpoint under the npm namespace and a different set of logic on a endpoint under the pkg namespace despite the fact that the Eik server uses the same code for both namespaces.

This is more of a question for clarity on my end. How would this look if one requires adding the same plugin to multiple endpoints? I guess defining and configuring a plugin instance, then sending it into each endpoint that requires it? Or would the plugin by default be applied to every endpoint, but you have to ensure your plugin is or isn't called for every endpoint with the if-check you are doing above?

if (incoming.handle !== 'put:pkg:version') {
  return;
}

If one doesn't specify this, will it run on every request, regardless of the endpoint being called?


I agree with @digitalsadhu that I believe we are introducing somewhat of a limitation by not allowing the ability of a plugin to alter the request. Even if it may not be a common use case, it may incur at some point and I think it could scare someone off from using Eik. I believe allowing it will give Eik extensibility, which could obtain a bigger community of people interested in adding all kinds of awesome plugins to Eik which we at some point could discuss whether actually should live in the core of Eik so everyone gets the benefit of what it's achieving.

However, this comes at the potential loss of asynchronously running plugins which I'm not the biggest fan of either. Perhaps if plugins have a way to run sync and/or async that problem could be solved. I see benefits for both use cases, e.g. if someone wants to track how long requests take and asynchronously report back to their tracking software.

I am kinda thinking that if we want Eik to ex transform something its something we build into Eik.

I see the value in your point here and agree to a degree (nice rhyme heh). I think by disallowing the community to build these tools when they need it, instead of waiting for us to review, discuss and eventually build it into Eik, might potentially scare off some users. I believe the freedom to let the wider community build what they want, then we can have a discussion about whether it should belong in the core of Eik instead feels more right, but I'm not opposed to being more opinionated as I see the benefits of having Eik be Eik and letting plugins act as additional async logic.

Personally since Eik is a set of http endpoints I find it most naturally that this is something one tune through a config and not by writing a plugin if one need to tune http headers.

I agree with this. If we provide a wide set of configuration options for this, there is no need for plugins to handle this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants