[new-backend] Discussion: Global Plugins

[atmaxinger]

Quotes taken from doc/decisions/global_plugins.md

Notification does not happen once after final commit, but for every plugin

What exactly does that mean? How should it work?

Plugin interface should be the same. Many plugins, e.g. dbus, should work as global plugins w/o any change in code (i.e. only changes in contract)

How exactly would this work? How can we have a different contract with hooks specific for each type of plugin when there should not be any change in the code of the plugin?

Global plugins might depend on specific applications or specific mount points (it should be possible to enforce global plugins for specific applications).

So we need some filter where we can say "execute this plugin only for these parent keys"

Also, should it be possible that there are multiple active plugins of the same type?

• This might be important if we utilize the logging type for the recording plugin - otherwise you can only either have logs or session recording.

How do we configure which plugins are active if there are more than one of the same type?

• IIRC, there was some talk about symlinks, not sure whether I like this approach.

[markus2330]

What exactly does that mean? How should it work?

For global plugins: It should work in a way that only if all backends successfully did their job, logging&notification happens.

How exactly would this work?

As the global plugins have the same interface as other plugins, plugins suitable to be global plugins can be mounted in both ways. Only global plugins, however, can fulfill additional global properties (like doing something only if all plugins succeeded).

So we need some filter where we can say "execute this plugin only for these parent keys"

This "filter" is exactly what happens with mounted plugins.

Also, should it be possible that there are multiple active plugins of the same type?

I don't think it is needed. There were longer discussions about it. I do not think it is useful to start the same discussions again here. Difficult decisions should have a documented decision process. So if you (or someone else) think such a feature is useful, please open a PR with a decision template.

This might be important if we utilize the logging type for the recording plugin - otherwise you can only either have logs or session recording.

This could be simply fixed by having "recording" (collecting ours, base and assert keys) as a different kind of global plugin than "logging". Btw. please think hard about terminology&names. It is very hard to change terminology later, as it goes across the whole docu.

IIRC, there was some talk about symlinks, not sure whether I like this approach.

If we change it, we should consistently change it also for default storage and resolver. See change-resolver-symlink and change-storage-symlink for how it works in these cases.

The alternative would be double-bootstrapping: a minimal config that only gives the values for default plugins (resolver, storage and global plugins), which then reads the mountpoint definitions, which then reads the real configs. This would also have the advantage that the mountpoint definitions (which may contain passwords etc.) could be encrypted (or use any other feature Elektra provides). Imho it would be great to have it but it is probably not a trivial feature.

[kodebach]

Notification does not happen once after final commit, but for every plugin

I don't think this sentence is correct anymore. According to the git blame it was written 7 years ago.

How should it work?

As the global plugins have the same interface as other plugins, plugins suitable to be global plugins can be mounted in both ways. Only global plugins, however, can fulfill additional global properties (like doing something only if all plugins succeeded).

Actually, I think most global plugins should never be mounted as normal plugins.

The way I see it, there should be fixed places within kdbGet/kdbSet where a specific global plugin is called (if it is enabled). (*1)

To illustrate let's look at gopts. There is a specific point in kdbGet where command-line options and environment variables should be parsed. This is the gopts hook point. Here libelektra-kdb will look for libelektra-gopts.so (maybe libelektra-hook-gopts.so would be a better name). If this .so is found (*2) the get function of the plugin in libelektra-gopts.so will be called.

So "global plugins" export the same interface as actual plugins, but are not actually normal plugins and should probably not be treated as such. Maybe dbus is special and would work as a mountpoint-specific plugin too, but for most global plugins this isn't the case AFAIK.

See also #4407 ("specific functions for hooks/global plugins")

Global plugins might depend on specific applications or specific mount points (it should be possible to enforce global plugins for specific applications).

So we need some filter where we can say "execute this plugin only for these parent keys"

This "filter" is exactly what happens with mounted plugins.

Actually, while this specific sentence can't refer to gopts (the sentence was written before gopts was a thing), with gopts we need parent key, but we cannot make it mountpoint specific, because it acts across multiple namespaces.

However, both the need to pass specific config (like a parent key) and the need to enforce a plugin for an application was already solved with the introduction of the contract for kdbOpen.

Also, should it be possible that there are multiple active plugins of the same type?

No not in general. But this is also a reason why moving to specific hooks is a good idea. If we decide that e.g. the logging hook, could support multiple active implementations, then we can just write it that way. Instead of calling one specific plugin, there would be a list somewhere in system:/elektra (e.g. system:/elektra/hook/logging/#) with all plugins that will be called.

This could be simply fixed by having "recording" (collecting ours, base and assert keys) as a different kind of global plugin than "logging"

IMO the kinds of hooks should only be determined by the contract between libelektra-kdb and the plugin on the other side. Specifically, it should be about when and with what arguments the plugin expects to be called and what libelektra-kdb expects as a result. For example spec in kdbGet: The plugin is called by libelektra-kdb when spec:/ backends have been processed and will receive the entire loaded KeySet across all namespaces and backends. The plugin must apply the specification from spec:/ keys to the other namespaces and return the result.

With logging/recording the contract would be the same. It would just be libelektra-kdb calls this plugin at the very end of kdbGet/kdbSet and passes the exact KeySet that will be returned to the caller of kdbGet/kdbSet. The plugin must not modify the KeySet, no returned result is expected.

How do we configure which plugins are active if there are more than one of the same type?

• IIRC, there was some talk about symlinks, not sure whether I like this approach.

If we change it, we should consistently change it also for default storage and resolver. [...]

I agree using symlinks seems a bit odd. The reason the default storage/resolver plugins are defined by symlinks is that we need them to load system:/elektra, which then defines all the other config for Elektra.

That's actually (part of) why I would separate default storage/resolver from bootstrap storage/resolver (see also #4407 ("bootstrap resolver")). The bootstrap storage/resolver would be defined by symlinks and would only be used to load system:/elektra. The default plugins (which are used for all the default mountpoints system:/, user:/, etc.) are defined by keys in system:/elektra.

The plugins that are used for the various hooks could then also be defined by keys in system:/elektra.

mountpoint definitions (which may contain passwords etc.) could be encrypted

Wouldn't you still need to store the encryption key in the bootstrap config? If there is a way around storing the key, why can't this be applied to the passwords in the mountpoint definitions?

---

(*1) IMO we should get rid of the confusing terminology, and find a better name for these things. Maybe "hook plugin" would be better. It shows that the plugin is special (it is for a specific hook), instead of just being global, but otherwise the same. (*2) gopts specifically, also needs to be enabled via a contract passed to kdbOpen

---

@atmaxinger For you this means:

1. Find where your plugin needs to be called in kdbGet/kdbSet (AFAICT at the very end both times)
2. Add code to call your plugin, if it is enabled

To begin with, it should be good enough to hardcode the name of the plugin. A mechanism to check a key in system:/elektra for the plugin name, can always be added later.

For the other hooks/plugins that exist:

• cache: Already described in doc/dev/kdb-operations.md. @mpranj needs to check, if the description is correct.
• gopts/spec: Described in doc/dev/kdb-operations.md and code is already mostly there, but commented out in kdb.c. gopts must be enabled via contract, spec is always active. spec also needs some changes, before it can be fully enabled.
• notifications (internalnotification, dbus, dbusrecv, zeromqsend, zeromqrecv): Not described IIRC. All of these must be enabled via contract. The *recv plugins are never called, they are only opened/closed and then call defined callbacks. The others (internalnotification, dbus, zeromqsend) must be called at the end of kdbGet and kdbSet (after poststorage and postcommit). From a contract POV, this is actually quite similar to your logging/recording case, so maybe the same hook point can be used. In theory you could even build your tool/plugin on top of internalnotification (with elektraNotificationRegisterCallbackSameOrBelow).

[markus2330]

To illustrate let's look at gopts

gopts is a bad example. For logger&notification plugins (and this is what we talk about here) it makes perfectly sense to mount them both as global and local plugin.

Wouldn't you still need to store the encryption key in the bootstrap config? If there is a way around storing the key, why can't this be applied to the passwords in the mountpoint definitions?

Yes, the first bootstrap definition would have the IDs of these keys. But I think this is off-topic here. What is on-topic: the first bootstrap could define which logger, recording and notification plugins should be used, including for accessing the mountpoint definitions (which definitely should be a part of logging, recording and notifications).

IMO we should get rid of the confusing terminology, and find a better name for these things

:+1:

To begin with

As @atmaxinger work should also bring unification in terms, behavior and implementation, it makes a lot of sense that first he thinks through how it should be before starting implementations.

A mechanism to check a key in system:/elektra for the plugin name, can always be added later.

I agree that this is independent, this is also why I suggested to use symlinks (which means to not have any code concerning the choice what the default plugin should be).

@mpranj needs to check, if the description is correct.

IIRC he checked. Unfortunately, the document is a bit hard to read. So maybe @atmaxinger can make it a bit easier to read. Then we can recheck if he understands it in the same way as @kodebach intended it to be (and hopefully uncover problems that we couldn't see because of difficulties in the text).

From a contract POV, this is actually quite similar to your logging/recording case

I even think it is identical. This is also why I asked him to unify these parts: so that we do not get artificial differences between these.

[kodebach]

gopts is a bad example

I think that's exactly the problem the term "global plugin" is used far to broadly. They can do basically anything. But actually the notification plugins are the only current example that could work on a per-mountpoint level. spec and cache definitely only work globally. With internalnotification I'm not sure, but it would certainly be inefficient to have multiple instances, since the plugin keeps a list of keys to watch anyway.

accessing the mountpoint definitions (which definitely should be a part of logging, recording and notifications)

Good point. But if the active session is stored in user:/elektra/record/session, we need to extend the bootstrap process to also load this user:/ mountpoint. (IMHO this is another point in favour of external storage for the active session)

it makes a lot of sense that first he thinks through how it should be before starting implementations

I don't know about his process, but for me it's always easier to start coding somewhere to get feel for what works and what doesn't. Especially, if I don't have that much experience with a project.

But of course before a final implementation, we need to decide on the general setup. My suggestion was more for a first PoC, and to get started with the actual recording part. Since you need to call the recording plugin where to test that it actually works the way you think.

Unfortunately, the document is a bit hard to read.

Sorry about that. The document wasn't yet intended for public consumption. It was mostly for my own reference, but now that other people work on new-backend it has become more important.

So maybe atmaxinger can make it a bit easier to read. Then we can recheck if he understands it in the same way as kodebach intended it to be

Probably a good idea.

I even think it is identical.

I see a very slight difference. As we know the notification plugins also make sense when configured at a specific mountpoint. It simply means you'll just get notifications for that mountpoint. For logging it is slightly different. Adding logging to a specific mountpoint could make sense, but not when you want to use logging for audit purposes. Let's look at an example to see why.

Alice adds logging to the system:/foo mountpoint, because she want to audit the changes there. But Mallory wants to evade that audit log. She creates a new mountpoint system:/foo/bar. By the design of Elektra applications don't care about this change. But since logging is only added to the system:/foo mountpoint, any changes below system:/foo/bar (now a different mountpoint) won't be logged.

What you may want for logging is the ability to limit a logging session to changes below a certain /parent/key, but limiting to a mountpoint is probably not what you want. Note that when you limiting a session to changes below a certain key, it would probably also be useful to have multiple active session. I think all of that could be achieved, by globally mounting internalnotification and using elektraNotificationRegisterCallbackSameOrBelow to register a callback for every active session to trigger the logging code.

[markus2330]

Adding logging to a specific mountpoint could make sense, but not when you want to use logging for audit purposes.

This is not describing a difference: For both logging&notification for some purposes the global plugin and for other purposes a mountpoint is needed. For both the global plugin is, afaik, the more common case.

@atmaxinger Is any question open here? Otherwise, please close the issue and create PRs with glossary, decisions, etc. of the outcome so that we can have a clear picture where we are at the moment.

[kodebach]

For both logging&notification for some purposes the global plugin and for other purposes a mountpoint is needed. For both the global plugin is, afaik, the more common case

Actually I'm not really sure the mountpoint option makes much sense in any case. I can't think of any case, where you'd want the behaviour described above. (additional sub-mountpoints being excluded)

That said, the important part from my last comment is

I think all of that could be achieved, by globally mounting internalnotification and using elektraNotificationRegisterCallbackSameOrBelow to register a callback for every active session to trigger the logging code.

I think @atmaxinger should look into this. If this approach is good enough for the logging use case, it might be the easiest solution to just use the internalnotification "system" for logging, notifications and all similar systems based on a callback for changed keys.

If we cannot reuse internalnotification directly, we should think about whether we can create a more general system that can be used by internalnotification and record. Both cases are based on code being invoked, when a key changed, so we should really not have to separate systems for that, if at all possible.

[markus2330]

Yes, I would very much appreciate if internalnotification, loggers and also 3-way merge can be revitalized by @atmaxinger and can be made in a way that they smoothly work together.

[atmaxinger]

@kodebach where do I actually need to call the gopts plugin? I have some rudimentary hooks system working right now. There are a lot of // HACK: for gopts, and in doc/dev/kdb-operations.md it says somewhere around step 13.

If I understand this correctly, I should be able to kdb get proc:/elektra/gopts/help and the gopts plugin will have parsed the command line and put 0 or 1 in there, depending on wheter --help was specified or not.

However, if I run this command, kdbGet already exits at step 6. Right above step 6, there is another // HACK: for gopts and I suppose this is for the goptsActive part in the if conditional. ksGetSize(backends) is 1 btw.

// Step 6: return if no backends left
// HACK: for gopts
if (ksGetSize (backends) == 0 || (goptsActive && keyGetNamespace (ksAtCursor (backends, 0)) == KEY_NS_PROC &&
				  keyGetNamespace (ksAtCursor (backends, ksGetSize (backends) - 1)) == KEY_NS_PROC))
{
	// TODO (kodebach): name not needed, once lock is in place
	keyCopy (parentKey, initialParent, KEY_CP_NAME | KEY_CP_VALUE);
	keyDel (initialParent);

	keyCopy (parentKey, keyGetMeta (backendsFindParent (allBackends, parentKey), "meta:/internal/kdbmountpoint"),
		 KEY_CP_STRING);

	ksDel (backends);
	ksDel (allBackends);
	errno = errnosave;
	return 0;
}

What am I supposed to be doing there?

[kodebach]

If I understand this correctly, I should be able to kdb get proc:/elektra/gopts/help and the gopts plugin will have parsed the command line and put 0 or 1 in there, depending on wheter --help was specified or not.

That's not quite correct. The current version of kdb doesn't use gopts so that won't work.

There is however a basic test case for gopts in tests/kdb/testkdb_contracts.cpp so you can use this test case to check your changes.

---

I'll explain the current hacky solution first and then what is actually supposed to happen.

Hacky solution

At the beginning of kdbGet we check, whether the gopts plugin was mounted. For gopts this will have happened via a contract that was passed to kdbOpen.

If gopts is actually there, we also add a fake backend to collect the proc:/ keys. Without this, we'd get errors about keys outside of backends.

https://github.com/ElektraInitiative/libelektra/blob/3e9f5ed10725627405fb36c7a1ea33fa7eb67a1e/src/libs/elektra/kdb.c#L1739-L1746

As the TODO suggests this fake backend is also not ideal. What should probably happen instead is that during kdbOpen a backend with the correct parent key is created for the gopts keys. However, for the first version we can keep this hack and just add the fake backend.

The next hack for gopts is the one you already mentioned above.

https://github.com/ElektraInitiative/libelektra/blob/3e9f5ed10725627405fb36c7a1ea33fa7eb67a1e/src/libs/elektra/kdb.c#L1778-L1782

Here we check, if there are any backends where we need to load data. During the resolver phase a backend can indicate that there is nothing to load, either because there is no data (e.g. no config file) or because the data hasn't change since the last kdbGet.

This check should just be (ksGetSize (backends) == 0. But since we added the fake backend above we also check, if gopts is active and if so, whether only the fake proc:/ backend is left. We also return early, if there is only the fake backend, because gopts can't produce any meaningful data without a spec.

Finally, we call gopts via the old procgetstorage global plugins mechanism (which actually calls the list plugin which then calls gopts).

https://github.com/ElektraInitiative/libelektra/blob/3e9f5ed10725627405fb36c7a1ea33fa7eb67a1e/src/libs/elektra/kdb.c#L1844-L1848

What should happen

As stated above, the backend for proc:/ keys from gopts should ideally be created during kdbOpen. But for the first version it is fine to keep the fake backend hack at the start of kdbGet IMO. (*)

The hack in Step 6 is also mostly fine. The check could be a bit more strict to make sure it really is the fake backend:

if (ksGetSize (backends) == 0 || (goptsActive && ksGetSize (backends) == 1 && ksAtCursor (backends, 0) == ksLookupByName (handle->backends, "proc:/", 0)))

Overall though I think there isn't really a way around this, unless we make a call to gopts to let it decide whether or not we need to do a full update, like it happens for other backends in resolveBackendsForGet.

Also it might be possible to move the adding of the fake backend to after Step 6, to avoid the extra check. However, that wouldn't help, when we add the backend in kdbOpen.

Finally, Step 13 needs to be replaced completely. Instead something similar to this should happen

https://github.com/ElektraInitiative/libelektra/blob/3e9f5ed10725627405fb36c7a1ea33fa7eb67a1e/src/libs/elektra/kdb.c#L1857-L1866

Here goptsGet would be a function that calls the kdbGetPtr kdbGet function from the Plugin * for gopts. (Note: goptsGet definitely needs access to the KDB * handle for that) This is what I was referring to as the "hook point" in recent discussions.

[atmaxinger]

@kodebach Thanks for this fantastic write-up! I got it mostly working now. However, there is one thing in the test cases you mentioned that isn't quite clear to me (all other asserts work though):

https://github.com/ElektraInitiative/libelektra/blob/3e9f5ed10725627405fb36c7a1ea33fa7eb67a1e/tests/kdb/testkdb_contracts.cpp#L98-L102

Why is the key k, which is /tests/kdb/contracts supposed to have the value "getter"? Currently it is empty.

Does it have something to do with this?:

https://github.com/ElektraInitiative/libelektra/blob/3e9f5ed10725627405fb36c7a1ea33fa7eb67a1e/src/libs/elektra/kdb.c#L1858-L1861

I currently have not implemented this because cascadingParent is undeclared.

[kodebach]

If this EXPECT_TRUE fails, but the next two succeed then that would be really weird, because both should be set by elektraGetOpts.

https://github.com/ElektraInitiative/libelektra/blob/3e9f5ed10725627405fb36c7a1ea33fa7eb67a1e/tests/kdb/testkdb_contracts.cpp#L103-L107

The value "getter" comes from the fact, that that is the (relative) name of the key that contains the spec for the command that was executed:

https://github.com/ElektraInitiative/libelektra/blob/3e9f5ed10725627405fb36c7a1ea33fa7eb67a1e/tests/kdb/testkdb_contracts.cpp#L41-L42

Does it have something to do with this?

You can mostly ignore that. The important part here is that gopts must be called with a parent key with cascading namespace. So cascadingParent is just a copy of parentKey but with keySetNamespace (cascadingParent, KEY_NS_CASCADING) called.

For the test case this shouldn't matter however, since in the test case the parentKey is already a cascading key. So if you just used the parentKey directly it should work too.

EDIT: just checked, the parent key for gopts doesn't actually matter, if it was configured via a kdbOpen contract, which is the case here.

[atmaxinger]

If this EXPECT_TRUE fails, but the next two succeed then that would be really weird, because both should be set by elektraGetOpts.

Yes but that is what happens. Could this be because the spec plugin not yet working?

[kodebach]

No the spec plugin isn't required for this to work. The value should be set in this line

https://github.com/ElektraInitiative/libelektra/blob/3e9f5ed10725627405fb36c7a1ea33fa7eb67a1e/src/libs/opts/opts.c#L250

Could you check with a debugger what happens there?

[atmaxinger]

So I fixed the bug...

Turns out my code didn't quite work yet, because I didn't initialize the loaded plugins with global and the config from the contract. The other asserts did work because the old global plugin code did the heavy lifting.

I fixed that now and made sure that the old global plugins do no longer run.

[github-actions[bot]]

I mark this stale as it did not have any activity for one year. I'll close it in two weeks if no further activity occurs. If you want it to be alive again, ping by writing a message here or create a new issue with the remainder of this issue. Thank you for your contributions :sparkling_heart:

[github-actions[bot]]

I closed this now because it has been inactive for more than one year. If I closed it by mistake, please do not hesitate to reopen it or create a new issue with the remainder of this issue. Thank you for your contributions :sparkling_heart: