ElektraInitiative / libelektra

Elektra serves as a universal and secure framework to access configuration settings in a global, hierarchical key database.
https://www.libelektra.org
BSD 3-Clause "New" or "Revised" License
208 stars 123 forks source link

kdb mount plugin config is not per-plugin #146

Closed markus2330 closed 9 years ago

markus2330 commented 9 years ago

When using the mount commando, e.g.

kdb mount -c classpath=.:/usr/share/java/jna.jar:/usr/share/java/elektra.jar,print= file.properties /jni jni classname=elektra/plugin/PropertiesStorage

one would expect that the backend config -c (the classpath) is available for the whole backend and the jni config (the classname) is just available for the plugin jni.

Currently, however, the classname is in the backend config, but not in the plugin config.

So for above example we get mounted:

system/elektra/mountpoints/_jni
system/elektra/mountpoints/_jni/config
system/elektra/mountpoints/_jni/config/classname
system/elektra/mountpoints/_jni/config/path
system/elektra/mountpoints/_jni/config/print
system/elektra/mountpoints/_jni/errorplugins
system/elektra/mountpoints/_jni/errorplugins/#5#resolver#resolver#
system/elektra/mountpoints/_jni/errorplugins/#6#syslog#syslog#
system/elektra/mountpoints/_jni/getplugins
system/elektra/mountpoints/_jni/getplugins/#0#resolver
system/elektra/mountpoints/_jni/getplugins/#5#jni#jni#
system/elektra/mountpoints/_jni/mountpoint
system/elektra/mountpoints/_jni/setplugins
system/elektra/mountpoints/_jni/setplugins/#0#resolver
system/elektra/mountpoints/_jni/setplugins/#5#jni
system/elektra/mountpoints/_jni/setplugins/#6#sync#sync#
system/elektra/mountpoints/_jni/setplugins/#7#resolver
system/elektra/mountpoints/_jni/setplugins/#8#syslog

So we have following problems:

system/elektra/mountpoints/_jni/config/classpath

is missing, because -c is just passed to plugin validation, but not to permanent writing.

system/elektra/mountpoints/_jni/getplugins/#5#jni#jni#/config/classname

is missing, because the classname is not treated as per-plugin config as it should be.

The per-plugin config, on the other hand, is not passed to plugin validation, see TODO.

Workaround: if you want the same config written as being validated, just pass the same config string to -c and to plugin config.

This workaround obviously won't work anymore if plugin1 needs a different classname than plugin2.

Maybe its also possible to refactor, so that the plugin-config parsing code only exists once. Now a duplicate in the class Cmdline exists.

fberlakovich commented 9 years ago

I skimmed over the issue and the mount code. This shoiuld be straight straight forward. However, am I right that there is no API ready to write plugin specific configurations (especially not in the cpp api)? This would drastically simplify the implementation.

markus2330 commented 9 years ago

As a matter of fact ideally most of it should be implemented within libelektra-tools. Given these methods in the class Backend:

addPlugin(name, pluginConfig);
addConfig(backendConfig);

(The first already exists, but currently does not contribute to the serialization) basically everything is within libtools.

fberlakovich commented 9 years ago

As a matter of fact ideally most of it should be implemented within libelektra-tools. Given these methods in the class Backend:

Now I am confused. As a matter of fact these methods reside in libtools.

Why should most of this be implemented in libelektra-tools? Wouldn't it be better to implement / change these methods in libtools and simply use them in libelektra-tools?

Should addPlugin(name, pluginConfig) contribute to the serialization (I was actually surprisded that it does not do it)?

markus2330 commented 9 years ago

Hello,

you wrote:

Now I am confused. As a matter of fact these methods reside in libtools.

Why should most of this be implemented in libelektra-tools? Wouldn't it be better to implement / change these methods in libtools and simply use them in libelektra-tools?

I am confused, too. libtools and libelektra-tools are the same thing (one is the name of the source code's folder, the other is the name of the .so). Could you please rephrase the question?

Should addPlugin(name, pluginConfig) contribute to the serialization (I was actually surprisded that it does not do it)?

Yeah it definitely should. It is simply not implemented.

best regards

fberlakovich commented 9 years ago

I am confused, too. libtools and libelektra-tools are the same thing (one is the name of the source code's folder, the other is the name of the .so). Could you please rephrase the question?

I thought you were speaking of the code in the tools folder (and not libtools). It makes sense now :)

markus2330 commented 9 years ago

Additional, passing meta data currently is not supported.

fberlakovich commented 9 years ago

Why does the backend configuration below system/elektra/mountpoints/<backend name>/getplugins and system/elektra/mountpoints/<backend name>/setplugins have such awkward key names? For example:

system/elektra/mountpoints/system_hosts_augeas/getplugins
system/elektra/mountpoints/system_hosts_augeas/getplugins/#0#resolver
system/elektra/mountpoints/system_hosts_augeas/getplugins/#5#augeas#augeas#
system/elektra/mountpoints/system_hosts_augeas/getplugins/#5#augeas#augeas#/config/lens = Hosts.lns

I stumbled upon this because I investigated how to implement the per plugin configuration serialisation and realised that it is not as simple as I initially thought. Or is there any API I am missing that I can use to find the base path for per plugin configuration (such as Backends::getConfigBasePath (name) for the Backend configuration)? Wouldn't it be much easier for humans to read as well as for developers to implement to have clean arrays. E.g. the above backend configuration could look like this

system/elektra/mountpoints/system_hosts_augeas/getplugins
system/elektra/mountpoints/system_hosts_augeas/getplugins/#0#/name = resolver
system/elektra/mountpoints/system_hosts_augeas/getplugins/#5#name = augeas
system/elektra/mountpoints/system_hosts_augeas/getplugins/#5#/config/lens = Hosts.lns

We could further build upon and improve the array API instead of having yet another schema.

I could simply hack the lookup of the right plugin configuration base path into the addPlugin method, but I would rather like to discuss if this the current schema is meaningful.

markus2330 commented 9 years ago

Hello,

you wrote:

Why does the backend configuration below system/elektra/mountpoints/<backend name>/getplugins and system/elektra/mountpoints/<backend name>/setplugins have such awkward key names? For example: system/elektra/mountpoints/system_hosts_augeas/getplugins system/elektra/mountpoints/system_hosts_augeas/getplugins/#0#resolver system/elektra/mountpoints/system_hosts_augeas/getplugins/#5#augeas#augeas# system/elektra/mountpoints/system_hosts_augeas/getplugins/#5#augeas#augeas# /config/lens = Hosts.lns

The reason is that the same plugin may occur more than once in an backend. The syntax is:

#<number>#<name>#<label>

where

#n#<ref>

This is needed for stateful plugins like the resolver plugin (conflict+update detection).

For more detailed explanation see here and the code that does this serialization can be found in src/libtools/src/plugins.cpp (serialise).

I stumbled upon this because I investigated how to implement the per plugin configuration serialisation and realised that it is not as simple as I initially thought. Or is there any API I am missing that I can use to find the base path for per plugin configuration (such as Backends::getConfigBasePath (name) for the Backend configuration)?

Yeah, you missed that the serialise of Backend/Plugin already exists in libtools. Just add a KeySet per plugin that gets serialized properly. At point of serialization the name, label and ref are generated for you. You would just have to add another loop that adds the per-plugin and per-backend configuration.

Wouldn't it be much easier for humans to read as well as for developers to implement to have clean arrays. E.g. the above backend configuration could look like this system/elektra/mountpoints/system_hosts_augeas/getplugins system/elektra/mountpoints/system_hosts_augeas/getplugins/#0#/name = resolver system/elektra/mountpoints/system_hosts_augeas/getplugins/#5#name = augeas system/elektra/mountpoints/system_hosts_augeas/getplugins/#5#/config/lens = Hosts.lns We could further build upon and improve the array API instead of having yet another schema.

The array syntax was mimiced after this label+ref syntax.

I am all for introducing new kinds of Backends that do not have the limitations the current file-based-backends have (backends without resolver). One can also build more simple Backends like you propose once different kinds are possible (when referencing back=stateful plugins are not needed). But its a rather large topic (diploma thesis-like) with a lot of initial effort. To get rid of any limitations, backends should be able to include (any) backends, and not only plugins.

Nevertheless, its useful that mount fully implements the current file-based backends. I think its no secret that the current Elektra implementation has its focus to supporting configuration files well. And for that particual usecase the resolver+referencing features are ideally suitable with very nice properties, e.g. import/export with any storage plugin for free. So the file- based backends will always have their niche, even when other backend-mechanism exist (e.g. with journaling, git, registry, network-based,...).

I could simply hack the lookup of the right plugin configuration base path into the addPlugin method,

The implementation should have a KeySet per plugin + a proper serialization. But ask again, if you still think its a hack ;)

but I would rather like to discuss if this the current schema is meaningful.

Of course we can further discuss this. But be assured that a lot of thought has already be put into it ;)

best regards,

Markus Raab http://www.libelektra.org Technische Universität Wien elektra@markus-raab.org Institut für Computersprachen Phone: (+431) 58801/185185 Argentinierstr. 8, 1040 Wien, Austria FAX: (+431) 58801/18598

DVR 0005886