Severity of missing plugins

[fxedel]

Version of KrakenD you are using

KrakenD Version: 2.4.3
Go Version: 1.20.6
Glibc Version: MUSL-1.2.4_(alpine-3.18.2)

Is your feature request related to a problem? Please describe.
More an anecdote than a problem: I recently tried to deploy krakend + my custom Go plugin on a machine. Krakend was running successfully and I could query my endpoints, but the response was not as expected. As it turned out, my plugin was not loaded due to some differences in the compilation environment (I set a custom GOMODCACHE, which is not possible when developing krakend plugins). However, it took me some time to find that out, since most relevant log messages were on the debug level and I'm using the info level on production.

Describe the solution you'd like
I don't know by which extent the current behavior is desired, which would make sense if plugins were expected to be optional and it should not matter that much if they are loaded or not. In my understanding, though, missing plugins can lead to severe bugs, as the functionality of the API gateway is not fully guaranteed. When using security-relevant plugins, you want to be 100% sure that this plugin is loaded and running. I would argue that warning or failing on missing plugins adheres to krakend's architectural design principle "Failing fast is better than succeeding slow".

In particular, there are several steps about loading plugins that could be changed:

• The LoadPlugins function:

  krakend-ce/plugin.go

  Line 11 in ad15eb3

  func LoadPlugins(folder, pattern string, logger logging.Logger) {

  • Loads plugin files matching the pattern specified in the configuration file
  • Currently, errors are logged on the debug level and the number of loaded plugins is logged on the error level
  • I would propose logging errors at least on the warning level. Or, to be even stricter, let krakend completely fail at this point.
  • Exception: Failures like symbol HandlerRegisterer not found in plugin, since it is completely valid for a plugin to not implement all three plugin types.
  • Why? If a plugin cannot be loaded, this means that the file is either not a plugin binary at all (in which case the pattern should be updated), the binary's dependencies mismatch, or that the plugin is wrongly implement (i.e. wrong type of HandlerRegisterer). All of these cases should be fixed, and not (almost) silently ignored.
• The RunServerFactory or in particular the handler plugin's New function: https://github.com/luraproject/lura/blob/v2.3.0/transport/http/server/plugin/server.go#L18:
  • Adds an HTTP handler for every plugin in the "plugin/http-server"."name" config
  • Most errors are logged on the debug level, some on the warning level. If a plugin name is not registered, only a debug message is emitted. If no plugins are registered at all, it is not even checked if plugin names are configured (and can for sure not be loaded).
  • I would assume that if you configure a plugin, you usually also want it to be loaded. Therefore, again, the corresponding log messages should have at least warning level or may even lead to krakend failing.
• Similarly for loading request/response modifiers and client plugins

Describe alternatives you've considered

• Set log level to debug on production: This is somewhat counterintuitive, since we don't want to debug on production, and it leads to annoying noise in the log, e.g. when using the InfluxDB metrics exporter
• Consider plugins as optional: This is not an option if plugins provide critical functionality, especially security features. Furthermore, even if krakend could operate meaningfully without plugins, loading errors are probably mistakes that should be fixed.

[alombarte]

Hi @fxedel ,

Our philosophy is that you should test everything locally before pushing it to production, and plugin testing should be part of your CI/CD pipeline (pre-running it). Nevertheless, your input is valuable, and we will definitely take action.

We are thinking of facilitating the offline testing of plugins (not in runtime - which must be superfast) so you can know before deploying a new KrakenD configuration that the plugins are loadable. This will probably be implemented as a new Krakend subcommand that you will add to your pipeline (as you do with krakend check,krakend audit,` etc.) and will ensure the plugins work. This is still pending to define, but I wanted to notify you beforehand.

In addition we will change the severity and messaging of the logs, so problems are easier to find. Some work has started: luraproject/lura#703

Thank you

[fxedel]

Hi @alombarte, thanks for your reply! Having a dedicated command to check plugin loading sounds like a good idea (although I don't really understood why KrakenD is even allowed to start if a plugin cannot be loaded, instead of failing fast) and the increased log levels definitely help very much at debugging. Thanks!