Help message not printed when high level API with code generation is used #3998

qwepoizt · 2021-08-19T16:10:41Z

@kodebach I'd like to fix this myself as part of my thesis, but beforehand I'd appreciate your assessment:
Can you take a look at this? Have I configured something wrong or is this a bug/regression?

Steps to Reproduce the Problem

Execute the steps below which follow the tutorial High-level API (with code-generation).
Note: The steps below use a different signature for loadConfiguration() than the tutorial, because the tutorial is outdated (see https://github.com/ElektraInitiative/libelektra/pull/3997/files).
Create file test.ini with content:

[]
mountpoint = test.ecf

[temp/day]
type = unsigned_short
description = The color temperature the screen should have during daytime.
default = 6500
example = 6500
opt/long = temp-day
opt/arg = required

Execute kdb gen -F ni=test.ini highlevel "/sw/example/myapp/#0/current" conf
Create file myapp.c with content:

#include <stdlib.h> 
#include <stdio.h> 
#include <elektra.h> 
#include "conf.h" 
 
int main(int argc, const char * const * argv, const char * const * envp) { 
        exitForSpecload (argc, argv); 
        ElektraError * error = NULL; 
        Elektra * elektra = NULL; 
        int rc = loadConfiguration (&elektra, argc, argv, envp, &error); 
 
        if (rc == -1) 
        { 
            fprintf (stderr, "An error occurred while opening Elektra: %s", elektraErrorDescription (error)); 
            elektraErrorReset (&error); 
            exit (EXIT_FAILURE); 
        } 
 
        if (rc == 1) 
        { 
            // help mode - application was called with '-h' or '--help' 
            // for more information see "Command line options" below 
            printHelpMessage (elektra, NULL, NULL); 
            elektraClose (elektra); 
            exit (EXIT_SUCCESS); 
        } 
}

Link and compile

cc myapp.c conf.c `pkg-config --cflags --libs elektra-codegen` -I. -o myapp -Wl,-rpath `pkg-config --variable=libdir elektra-codegen`

Run executable with --help

./myapp --help

Run executable with -h

./myapp -h

Expected Result

The help message generated by code generator should be printed including information about the supported command line argument --temp-day.

Actual Result

Nothing is printed.

System Information

Elektra Version: 0.9.7
Operating System: Ubuntu 20.04

The text was updated successfully, but these errors were encountered:

kodebach · 2021-08-19T17:45:57Z

First of all, only --help triggers help mode, so the behaviour for -h is expected.

You didn't mount the specification (there should be conf.mount.sh script to do that in your example). However, you should still at the very least get the fallback help message that was generated by the code-generator. This fallback message is created via the same elektraOpts calls that should normally happen at runtime:

libelektra/src/tools/kdb/gen/highlevel/highlevel.cpp

Lines 213 to 230 in 71ecdaf

    
           static std::string generateHelpMessage (const std::string & appName, const std::string & specParent, kdb::KeySet spec) 
        
           { 
        
           	std::vector<const char *> argv = { appName.c_str (), "--help" }; 
        
           	kdb::Key parentKey (specParent.c_str (), KEY_END); 
        
           	int ret = ckdb::elektraGetOpts (spec.getKeySet (), argv.size (), argv.data (), NULL, parentKey.getKey ()); 
        
           	if (ret != 1) 
        
           	{ 
        
           		throw CommandAbortException ("could not generate fallback help message"); 
        
           	} 
        
           	char * help = ckdb::elektraGetOptsHelpMessage (parentKey.getKey (), NULL, NULL); 
        
           	std::string result (help); 
        
           	ckdb::elektraFree (help); 
        
           	return result; 
        
           }

I investigated this a bit and it turns out this is a bug in the opts library. Specifically that generateOptionsList doesn't recognise the temp/day key as an option (because of keyIsDirectlyBelow). I don't think it is a simple as using keyIsBelow, because then it probably shows to many options, when commands are used in the spec. Probably keyWithOpt needs to be generated differently in the various process* options, so that the keyIsDirectlyBelow check works.

qwepoizt · 2021-08-20T09:17:25Z

Thanks for your comments! I'll investigate further.

qwepoizt · 2021-08-20T12:08:55Z

I created #3999 for the issue of missing command line options in the help message.

@kodebach: I'd appreciate your opinion on these 2 topics:

Mounting before executing application

An elektrified application using the high-level API needs SFM=yes and SME=yes, right? (acronyms explained below). I believe so, because without the both: default values are missing, application doesn't know which values are valid for the keys, and so on and so forth.
Could the mounting be done with C code within ./myapp?
Do you have other suggestions on how to streamline the mounting?
How did you do it for lcdproc?

Behavior of help message depends on mount status

There are 2 mount conditions to consider:

SFM: Was Specification file itself mounted?
SME: Was kdb spec-mount executed?

That leads to 3 possible combinations:

./myapp --help with SFM=no and SME=no:
Outcome: Neither fallback help message nor help message from key proc:/elektra/gopts/help/message is printed.
Reason: call to elektraHelpKey(e) within conf.c returns a non-null pointer, but keyString (elektraHelpKey(e)) returns NULL. Therefore loadConfiguration returns 0, and the if (rc == 1) branch is not entered (see code from step 3 above)

@kodebach Do you agree that this case should never happen, because the application should simply fail, if SFM=no?

./myapp --help SFM=yes and SME=no:
Outcome: Help message from key proc:/elektra/gopts/help/message is printed.
Reason: keyString (elektraHelpKey(e)) now returns '1', the if (rc == 1) branch is entered.

@kodebach Do you agree that this case should never happen, because the application should simply fail, if SFM=no?

./myapp --help SFM=yes and SME=yes:
Outcome: Help message from key proc:/elektra/gopts/help/message is printed.
Reason: analogous to 2.

@kodebach Do you agree that this would be the only legal case?

kodebach · 2021-08-20T15:56:12Z

Could the mounting be done with C code within ./myapp?

In theory yes, however, currently the code for creating mountpoints is written in C++, so calling it from C is not as easy. Some of the mounting code needs to change for #3693. Probably we should rewrite it in pure C.

Do you have other suggestions on how to streamline the mounting?
How did you do it for lcdproc?

The *.mount.sh shell script that is generated by the code-generator already was my attempt at streamlining things. That's also what I used for LCDproc.

One thing you have to keep in mind is that the mounting only has to be done during installation. So it would probably not make that much sense to build it directly into the main executable. However, a basic check whether the mounting has happend could be put into the executable. Then we could at least show an error message, if the specification wasn't mounted.

That leads to 3 possible combinations:

In general, the idea was that ./myapp --help always leads to some kind of help message being printed, even if there was some other error. That's why the fallback message exists.

In the case of a missing specification (SFM=no or SME=no), we could print an additional message stating that the specification was not found (before/after the help message). If --help wasn't used, the same message would be reported as an error by elektraOpen.

qwepoizt · 2021-08-26T13:53:09Z

Thanks for your comment, I'll continue work on this issue at a later point in time, probably after fixing #3999.

qwepoizt · 2021-08-31T09:48:30Z

@kodebach Thanks, I agree with everything you wrote!

Suggested fix

I suggest to detect the state of SFM and SME within elektra.c:elektraOpen(...) at line 119-121.

libelektra/src/libs/highlevel/elektra.c

Lines 119 to 121 in 5702ce0

    
           const int kdbGetResult = kdbGet (kdb, config, parentKey); 
        
           if (kdbGetResult == -1)

If SFM=false or SME=false, we can return NULL and set a a sensible error message within the error param, which the application can then show to the user.

But: How can we detect the state of SFM and SME?

SFM

I have diff'ed kdb ls / with SFM=false and SFM=true. Once SFM=true kdb ls / returns a lot of new keys, most importantly:

spec:PARENT_KEY_OF_APPLICATION (e.g. spec:/sw/redshift/#0/current).

Therefore, would it suffice to check if ksLookup(spec:PARENT_KEY_OF_APPLICATION, ...) returns a non-NULL pointer after elektraOpen at line 119 (see above) ?

E.g.
bool SFM = ksLookup('spec:/sw/redshift/#0/current', ...) != NULL

SME

Do you have a suggestion on how to check for SME?
The kdb ls / diff shows that after SME=true, many new keys are returned. Most interesting are the ones under system:/elektra/mountpoints/PARENT_KEY_OF_APPLICATION_ESCAPED.

Can any of them be used as an indicator for SME? Maybe

system:/elektra/mountpoints/PARENT_KEY_OF_APPLICATION_ESCAPED/mountpoint
or
system:/elektra/mountpoints/PARENT_KEY_OF_APPLICATION_ESCAPED/config
?

E.g. for redshift:

system:/elektra/mountpoints/\/sw\/redshift\/#0\/current/config
system:/elektra/mountpoints/\/sw\/redshift\/#0\/current/config/fcrypt/textmode
system:/elektra/mountpoints/\/sw\/redshift\/#0\/current/config/path
system:/elektra/mountpoints/\/sw\/redshift\/#0\/current/errorplugins
system:/elektra/mountpoints/\/sw\/redshift\/#0\/current/errorplugins/#5#resolver_fm_hpu_b#resolver#
system:/elektra/mountpoints/\/sw\/redshift\/#0\/current/getplugins
system:/elektra/mountpoints/\/sw\/redshift\/#0\/current/getplugins/#0#resolver
system:/elektra/mountpoints/\/sw\/redshift\/#0\/current/getplugins/#5#dump#storage#
system:/elektra/mountpoints/\/sw\/redshift\/#0\/current/getplugins/#7#range#range#
system:/elektra/mountpoints/\/sw\/redshift\/#0\/current/getplugins/#8#type#type#
system:/elektra/mountpoints/\/sw\/redshift\/#0\/current/getplugins/#9#date#date#
system:/elektra/mountpoints/\/sw\/redshift\/#0\/current/mountpoint
system:/elektra/mountpoints/\/sw\/redshift\/#0\/current/setplugins
system:/elektra/mountpoints/\/sw\/redshift\/#0\/current/setplugins/#0#resolver
system:/elektra/mountpoints/\/sw\/redshift\/#0\/current/setplugins/#1#date
system:/elektra/mountpoints/\/sw\/redshift\/#0\/current/setplugins/#2#type
system:/elektra/mountpoints/\/sw\/redshift\/#0\/current/setplugins/#3#range
system:/elektra/mountpoints/\/sw\/redshift\/#0\/current/setplugins/#4#validation#validation#
system:/elektra/mountpoints/\/sw\/redshift\/#0\/current/setplugins/#5#storage
system:/elektra/mountpoints/\/sw\/redshift\/#0\/current/setplugins/#7#resolver

qwepoizt · 2021-08-31T09:49:37Z

@markus2330 I noticed I never mentioned you in this issue (as you've asked me to). Sorry, I'm doing it now.

kodebach · 2021-08-31T14:22:49Z

Therefore, would it suffice to check if ksLookup(spec:PARENT_KEY_OF_APPLICATION, ...) returns a non-NULL pointer after elektraOpen at line 119 (see above) ?

It is better than nothing, but it doesn't really tell you that the spec file was mounted. It could be that just the parent key was set. Checking system:/elektra/mountpoints might be more reliable.

You already noticed this for SME, but it works for SFM too. Just check that system:/elektra/mountpoints/spec:PARENT_KEY_OF_APPLICATION_ESCAPED/mountpoint exists, or even better that it's value is spec:PARENT_KEY_OF_APPLICATION. (*)

Do you have a suggestion on how to check for SME?

As you noticed, you can use system:/elektra/mountpoints. The check would be similar to the one proposed above for SFM, but with a cascading key instead. So you need to look for a system:/elektra/mountpoints/PARENT_KEY_OF_APPLICATION_ESCAPED/mountpoint with value PARENT_KEY_OF_APPLICATION. (*)

For now, I think the simple checks above are enough. But...

The above checks don't really validate that the correct spec was mounted (just that something is mounted at spec:PARENT_KEY_OF_APPLICATION) or that kdb spec-mount was executed (just that something is mounted at PARENT_KEY_OF_APPLICATION).

I don't think there is a 100% foolproof way to check SFM and SME, but we could do a bit better than above. To give a bit more certainty whether the correct spec was mounted, we could look for a token value.

The code-generator could just add something like this to the spec it generates (i.e. the spec that should be mounted):

[]
mountpoint = some_file.ext ; already there
token = some_random_value_or_maybe_just_hash_of_spec ; new

; rest of spec ...

The elektraOpen can simply check for this value on spec:PARENT_KEY_OF_APPLICATION and be reasonably certain that SFM=true.

For SME this would be bit more complicated. We could create a plugin that looks for the token metakey and prefixes the value with CONFIRMED_. Then we can look for this prefixed value on PARENT_KEY_OF_APPLICATION, an be reasonably certain that SME=true.

(*) Technically this is not a 100% correct check, but it works most of the time (namely when kdb mount/kdb spec-mount was used). The layout of system:/elektra/mountpoints will change with #3693, so "works most of the time" is fine for now. When the backend implementation is done, we can create a better solution.

qwepoizt · 2021-08-31T18:18:34Z

Thanks, that's helpful. I believe it is sensible to implement the simple checks now and something more elaborate later, sometime after #3693 is solved.

@markus2330 Do you agree?

markus2330 · 2021-09-09T09:37:01Z

I like the idea with the token, it is a sensible choice for code generation as you can easily generate unique tokens and the check is a simple string comparison (if the same token is compiled in the application, is present in spec: and is used for spec-mount).

I think, without correct token, we should fail at application startup, and not only for the --help message.

markus2330 · 2021-09-09T09:41:55Z

For SME this would be bit more complicated. We could create a plugin that looks for the token metakey and prefixes the value with CONFIRMED_. Then we can look for this prefixed value on PARENT_KEY_OF_APPLICATION, an be reasonably certain that SME=true.

Why not simply let spec-mount copy the token to the mountpoint definition, e.g. system:/elektra/mountpoints/\/sw\/redshift\/#0\/current/config/token.

kodebach · 2021-09-09T10:30:18Z

I think, without correct token, we should fail at application startup, and not only for the --help message.

Yes, I would add a new return code for loadConfiguration that indicates "mountpoints incorrect". Then we could in future generate a function setupMountpoints for automatic setup. This cannot be done yet, because the tools library is still C++.

Why not simply let spec-mount copy the token to the mountpoint definition

Yes, that should work too and might not even require any changes to spec-mount.

qwepoizt · 2021-09-09T10:51:24Z

Thanks for your comments!

This week I have implemented a version that:

checks SFM and SME on elektraOpen() by checking existence and value of the mountpoints as discussed above (see e.g Help message not printed when high level API with code generation is used #3998 (comment) )
sets a user-friendly error description and returns NULL, if SFM or SME is false.

I will extend this version according to the decisions below..

I propose to postpone a new return code until setupMountpoints for automatic setup has been implemented.

Decisions from call @markus2330 and @qwepoizt 09th Sep 2021:

Use a token mechanism for checking SFM and SME.
Generate token with cryptographic hash (yet to decide: full file or only keynames+values).
By default, HL API does not check for SFM and SME.
- Checks can be enabled via new key in contract: system:/elektra/contract/highlevel/check/spectoken/ = 1.
- Code generated by kdb gen always sets this key.

markus2330 · 2021-09-09T11:01:42Z

system:/elektra/contract/highlevel/check/spectoken/ = 1

Where will the actual token be?

markus2330 · 2021-09-09T11:04:11Z

Yes, I would add a new return code for loadConfiguration that indicates "mountpoints incorrect". Then we could in future generate a function setupMountpoints for automatic setup. This cannot be done yet, because the tools library is still C++.

It is not only a program language barrier problem but currently also a permission problem: mounting without #1074 requires root.

qwepoizt · 2021-09-09T11:07:35Z

Where will the actual token be?

Good point, thanks! You're right, it will also have to be part of the contract.

Then the keys could be:

system:/elektra/contract/highlevel/check/spectoken/enable = 1
system:/elektra/contract/highlevel/check/spectoken/token = THE_HASH_VALUE

kodebach · 2021-09-09T11:27:25Z

If generated code always enables the check, I don't think you need the .../enable. Just check wether .../token exists.

qwepoizt · 2021-09-09T11:46:51Z

Good point, thanks. I'll look at it in more detail when I start implementing it!

While working on redshift's automake configuration, looking at https://github.com/kodebach/lcdproc/blob/3e2b9d3f5cade3bbe21fdb5a9ea1c9ffc1c994fa/server/Makefile.am and the kdb-gen-highlevel manpages, I have just now discovered that the SFM/SME checks I have implemented were actually already partially implemented by @kodebach in elektra.c:

libelektra/src/libs/highlevel/elektra.c

Lines 327 to 378 in e9d0afa

    
           } 
        
           static bool minimalValidation (const char * application) 
        
           { 
        
           	Key * parent = keyNew ("system:/elektra/mountpoints", KEY_END); 
        
           	KDB * kdb = kdbOpen (NULL, parent); 
        
           	KeySet * mountpoints = ksNew (0, KS_END); 
        
           	if (kdbGet (kdb, mountpoints, parent) < 0) 
        
           	{ 
        
           		ksDel (mountpoints); 
        
           		kdbClose (kdb, parent); 
        
           		keyDel (parent); 
        
           		return false; 
        
           	} 
        
           	char * specName = elektraFormat ("spec%s", application); 
        
           	Key * lookup = keyNew ("system:/elektra/mountpoints", KEY_END); 
        
           	keyAddBaseName (lookup, specName); 
        
           	elektraFree (specName); 
        
           	if (ksLookup (mountpoints, lookup, 0) == NULL) 
        
           	{ 
        
           		keyDel (lookup); 
        
           		ksDel (mountpoints); 
        
           		kdbClose (kdb, parent); 
        
           		keyDel (parent); 
        
           		return false; 
        
           	} 
        
           	keyDel (lookup); 
        
           	lookup = keyNew ("system:/elektra/mountpoints", KEY_END); 
        
           	keyAddBaseName (lookup, application); 
        
           	if (ksLookup (mountpoints, lookup, 0) == NULL) 
        
           	{ 
        
           		keyDel (lookup); 
        
           		ksDel (mountpoints); 
        
           		kdbClose (kdb, parent); 
        
           		keyDel (parent); 
        
           		return false; 
        
           	} 
        
           	keyDel (lookup); 
        
           	ksDel (mountpoints); 
        
           	kdbClose (kdb, parent); 
        
           	keyDel (parent); 
        
           	return true; 
        
           }

I guess, I need to schedule an appointment with my optican 😆 .

That means:

The minimalValidation() checks for existence of the mountpoint keys, but doesn't check their value.
It also doesn't check integrity of the specification.
I'll probably remove the minimalValidation() in favor of my implementation, because I put a lot of effort in documentation and modularisation.
minimal validation was optional before and had to be explicitly enabled via flag to kdb gen highlevel .... It will be superseeded by the new token mechanism and always on when using kdb gen highlevel ....

qwepoizt · 2021-09-09T15:41:44Z

@markus2330 @kodebach I had further insights that complicate this issue. I think we'll need to make further decisions!

I'll use redshift as an example to make the following easier to understand.

Insights

The HL API intends the specification file to only be used by kdb gen. The file itself is actually not mounted during installation or runtime of redshift!
- Therefore, we can't generate a token based on the full textual content of the specification file, because that file is not used/available during installation/runtime.
- Instead the token needs to be generated from the key names + (meta-values) representing the specification.
Actually, the HL API uses the specload plugin to mount the specification into spec:/sw/redshift/#0/current.
- This plugin uses the specification embedded within redshift to populate spec:/sw/redshift/#0/current.
  It calls redshift --elektra-spec and reads the spec from stdin.
- This plugin prohibits changes to keys within spec:/sw/redshift/#0/current. Only some changes are allowed.
  e.g.:
```
$ sudo kdb meta-rm spec:/sw/redshift/#0/current/version default
Sorry, module specload issued the error C01100:
Resource: This kind of change is not allowed
```

This raises several points for discussion. I'll think about them and appreciate your thoughts:

If the specload plugin already checks and prevents changes to the specification, would there be any benefit left in implementing our token mechanism?
If there is a benefit, can the specload plugin and the token mechanism coexist? How?
Would it make more sense to instead modify the specload plug-in to simply prohibit any change to any key within spec:/sw/redshift/#0/current? As the specload plugin uses the specification embedded within redshift, we'll know that it wasn't changed by a user.
Is there another way in Elektra to mark the spec:/sw/redshift/#0/current sub-tree as read-only?

markus2330 · 2021-09-09T20:10:21Z

I'll probably remove the minimalValidation() in favor of my implementation, because I put a lot of effort in documentation and modularisation.
minimal validation was optional before and had to be explicitly enabled via flag to kdb gen highlevel .... It will be superseeded by the new token mechanism and always on when using kdb gen highlevel ....

Yes, please remove 👍

Actually, the HL API uses the specload plugin to mount the specification into spec:/sw/redshift/#0/current.

Where did you see this? This is probably outdated docu or a non-working script. The specload plugin was an approach to solve the SFM/SME problems but unfortunately it did not work well, e.g., it makes the application binaries too big.

If the specload plugin already checks and prevents changes to the specification, would there be any benefit left in implementing our token mechanism?

Yes, as specload is not mandatory and in its current state not even recommendable.

If there is a benefit, can the specload plugin and the token mechanism coexist? How?

If the token mechanism works well and we don't find solutions for specload we will remove specload.

Would it make more sense to instead modify the specload plug-in to simply prohibit any change to any key within spec:/sw/redshift/#0/current?

These changes are allowed on purpose (they are harmless).

But your question is probably something else: Fixing the problems of specload is probably much harder (e.g. finding an encoding that doesn't bloat the binaries so much) than implementing the token mechanism. If you think specload is the way to go, you are free to fix the problems and use it for redshift.

Is there another way in Elektra to mark the spec:/sw/redshift/#0/current sub-tree as read-only?

This wouldn't prevent from installing wrong versions of the specification.

kodebach · 2021-09-09T21:08:38Z

Where did you see this? This is probably outdated docu or a non-working script.

The specload stuff should still work and AFAIK is currently the default config for the code-generator. There should also be an option to generate a file that can be mounted directly instead of using specload.

e.g., it makes the application binaries too big.

I think the binary size was an issue mainly for embedded systems. But there are other issues as well, related to the other goals (allow only "safe" modifications).

e.g. finding an encoding that doesn't bloat the binaries so much

The way I understand it, the issue was that on some embedded systems there is limited amount of storage for executable data, but more additional (often slower) storage for non-executable data. If the whole thing is a single file, everything has to be in one type of storage. In these cases I don't think it will ever really make sense to embed the config, without some other clear benefit.

If the token mechanism works well and we don't find solutions for specload we will remove specload.

Yes, I too think, specload can be removed. It was a nice idea, but didn't work. The "ask the application for it's own spec" part, could also be implemented via a simpler plugin that uses something like pluginprocess.

However, a new optional plugin to enhance the token mechanism might be something to think about. This new plugin would generate the hash/token on-the-fly and replace the stored value during kdbGet. Then checking the token does actually give some indication of the state of the spec. The plugin should still be optional because calculating a cryptographic hash needs a dependency, which not everybody will want.

qwepoizt · 2021-09-10T10:27:52Z

Thanks for your input! I thought some more about this and came up with a list of planned changes (see below).

@markus2330 Are you fine with this plan? I tried to keep it simple without over-engineering, but maybe it can be simplified? If you're fine with it, please let me know and I'll start working on it!

Planned changes

Token mechanism

is a SHA1 or SHA256 hash
input is binary representation of keys within /sw/redshift/#0/current (cascading key) and all their meta-keys.
- idea: fetch a KeySet of this subtree, set all key values to NULL, use quickdump plugin to retrieve binary representation, hash the binary representation.
calculated during kdb gen, written into generated redshift-conf.c as part of the contract for elektraOpen().
during installation of redshift, sudo kdb mount PATH_OF_SPECFILE/redshift.ini spec:/sw/redshift/#0/current ni and kdb spec-mount /sw/redshift/#0/current are executed.
any modification by user to keys (remove, rename, add) or their meta-keys (remove, rename, add, change value) is illegal and will result in application not starting up. modifications to key values are legal.

Checking SFM and SME

SFM=yes and SME=yes are checked on every application start up:
1. check if both mountpoints exist and have value /sw/redshift/#0/current
2. generate token from keys+meta-keys as explained above. input is /sw/redshift/#0/current (cascading).
3. compare with the token from the contract (see above).
4. tokens equal -> check ok
5. tokens not equal -> check not ok, because user modified specification.
token is not stored inside mountpoints.

Removing features

plugin specload will be removed.
feature "embedding specification within application" will be removed.
feature "minimal validation" will be replaced by my implementation and made mandatory.

Answers on your questions/comments

Yes, currently specload works and is default for kdb gen. I am currently using it for redshift.
If you both find the token mechanism to be the way to go, I'm certain it is the better solution!
@markus2330 pointed out, that implementations of SHA1 are so short that it is justifiable to include them within Elektra. Thus no dependency is required.

qwepoizt · 2021-09-10T10:38:54Z

I am planning to use https://github.com/CTrabant/teeny-sha1 , because:

it's small.
license is compatible. teeny-sha1 uses MIT license, like other third party code that Elektra already uses. I found no alternative C implementation with BSD 3-clause license.

markus2330 · 2021-09-11T17:06:04Z

plugin specload will be removed.

See #4032 for further discussion. I am surprised that the binary size barely increased (probably because the spec is much smaller). Furthermore, it worked for me without problems and has the big advantage that the spec: automatically updates with any updates of the binary.

But it does not really matter for the token implementation, also with specload inconsistency can arise (wrong binary mounted, spec-mount not executed), so the token solution will be required in any case.

feature "minimal validation" will be replaced by my implementation and made mandatory.

full ack

If you both find the token mechanism to be the way to go, I'm certain it is the better solution!

Please also look into specload and see if it is somehow maintainable and does a good job.

https://github.com/CTrabant/teeny-sha1

Ok, I did not look into alternatives, though. sh256 would probably a bit more future-safe, it sounds reasonable that people at some point will try to trick applications to use a wrong specification.

Btw. a plugin that implements a check if values or meta-data were modified without using Elektra would be very nice. Maybe you can make the implementation in a way, that implementing such a plugin gets easy (which would be the case if you implement it in a function of the code-gen-support library, as this library could also be used in the plugin).

qwepoizt · 2021-09-13T11:45:35Z

Decisions from Elektra meeting 13th Sep 2021:

Add token mechanism.
Remove specload (if nothing speaking against it comes up).
Mechanism that allows administrators to perform sensible changes to specification may be added in future.

qwepoizt · 2021-09-13T11:46:43Z

sh256 would probably a bit more future-safe

Good point, thanks. I'll use https://github.com/amosnier/sha-2 instead, which is also short and license compatible.

kodebach · 2021-09-13T12:03:11Z

amosnier/sha-2 looks easier to use anyway, since it has a streaming API. The other library would have required creating one single, big buffer for all the data you want to hash. Hashing a complete specification would have meant creating a complete duplicate of it just to calculate the hash.

qwepoizt · 2021-09-14T12:10:46Z

if you implement it in a function of the code-gen-support library, as this library could also be used in the plugin)

@markus2330 Which library are you referring to? All of the kdb gen code seems to be C++ and within the folder src\tools\kdb\gen. Unfortunately, I can't find any C "support library" .

kodebach · 2021-09-14T12:38:12Z

You could simply include the sha256.c and sha256.h files in libelektra-ease and then add a function

int elektraSha256Hash (uint8_t hash[32], KeySet * input)

qwepoizt · 2021-09-17T20:18:02Z

@markus2330 I have an update: This is progressing well!

Done

SFM and SME check implemented
sha-256 calculation implemented
Token calculation implemented
kdb gen adds token to contract
HL API checks token on application startup
Tests implemented
Inline documentation of code from other authors slightly improved
"minimal validation" removed from HL API and kdb gen

Current issue

Currently, I am seeing some problems with arrays:

Adding array items in the KDB copies the metakeys from the array key (e.g. /myfloatarray/#) to each item (e.g. /myfloatarray/#0).
Such metakeys are e.g., type = float or default = 2.5
During kdb gen the array is empty, so the token doesn't include any metakeys for array items.
Once the application starts up the array might have items.
Now the tokens don't match anymore!

My first idea was to ignore all metadata on array items. I.e. keys whose basenames start with #N where N a number > 0.

@markus2330 What do you think about this? Is there a better approach?

markus2330 · 2021-09-20T17:44:30Z

Adding array items in the KDB copies the metakeys from the array key (e.g. /myfloatarray/#) to each item (e.g. /myfloatarray/#0).

There is a similar functionality with _.

I think they would not be copied if you only work on spec: keys. What is the reason you look at other keys now? We said that these checks would be done by the spec plugin (which will handle _ and # properly).

kodebach · 2021-09-20T18:26:43Z

Yes, the spec plugin shouldn't add/modify/delete any spec:/ keys. In the case of array and wildcard specs it may generate new keys, but only in other namespaces.

qwepoizt · 2021-09-21T08:20:38Z

What is the reason you look at other keys now?

Actually, the algorithm should ignore any keys that are not in the spec namespace.

I'll create a small test to further debug this!

kodebach added bug important labels Aug 19, 2021

qwepoizt mentioned this issue Aug 20, 2021

Help message does not contain all command line options #3999

Closed

qwepoizt mentioned this issue Sep 8, 2021

Draft: highlevel: detect if spec properly mounted (reopened in #4047) #4027

Closed

qwepoizt mentioned this issue Sep 15, 2021

highlevel: detect if spec properly mounted and unmodified #4047

Merged

20 tasks

qwepoizt self-assigned this Sep 17, 2021

qwepoizt mentioned this issue Sep 18, 2021

redshift: CLI arguments not working #4041

Closed

mpranj closed this as completed in #4047 Sep 28, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Help message not printed when high level API with code generation is used #3998

Help message not printed when high level API with code generation is used #3998

qwepoizt commented Aug 19, 2021 •

edited

Loading

kodebach commented Aug 19, 2021

qwepoizt commented Aug 20, 2021

qwepoizt commented Aug 20, 2021 •

edited

Loading

kodebach commented Aug 20, 2021

qwepoizt commented Aug 26, 2021 •

edited

Loading

qwepoizt commented Aug 31, 2021

qwepoizt commented Aug 31, 2021

kodebach commented Aug 31, 2021

qwepoizt commented Aug 31, 2021

markus2330 commented Sep 9, 2021

markus2330 commented Sep 9, 2021

kodebach commented Sep 9, 2021

qwepoizt commented Sep 9, 2021

markus2330 commented Sep 9, 2021

markus2330 commented Sep 9, 2021 •

edited

Loading

qwepoizt commented Sep 9, 2021 •

edited

Loading

kodebach commented Sep 9, 2021

qwepoizt commented Sep 9, 2021

qwepoizt commented Sep 9, 2021 •

edited

Loading

markus2330 commented Sep 9, 2021

kodebach commented Sep 9, 2021

qwepoizt commented Sep 10, 2021

qwepoizt commented Sep 10, 2021

markus2330 commented Sep 11, 2021

qwepoizt commented Sep 13, 2021

qwepoizt commented Sep 13, 2021

kodebach commented Sep 13, 2021

qwepoizt commented Sep 14, 2021

kodebach commented Sep 14, 2021

qwepoizt commented Sep 17, 2021 •

edited

Loading

markus2330 commented Sep 20, 2021

kodebach commented Sep 20, 2021

qwepoizt commented Sep 21, 2021

Help message not printed when high level API with code generation is used #3998

Help message not printed when high level API with code generation is used #3998

Comments

qwepoizt commented Aug 19, 2021 • edited Loading

Steps to Reproduce the Problem

Expected Result

Actual Result

System Information

kodebach commented Aug 19, 2021

qwepoizt commented Aug 20, 2021

qwepoizt commented Aug 20, 2021 • edited Loading

Mounting before executing application

Behavior of help message depends on mount status

kodebach commented Aug 20, 2021

qwepoizt commented Aug 26, 2021 • edited Loading

qwepoizt commented Aug 31, 2021

Suggested fix

SFM

SME

qwepoizt commented Aug 31, 2021

kodebach commented Aug 31, 2021

qwepoizt commented Aug 31, 2021

markus2330 commented Sep 9, 2021

markus2330 commented Sep 9, 2021

kodebach commented Sep 9, 2021

qwepoizt commented Sep 9, 2021

Decisions from call @markus2330 and @qwepoizt 09th Sep 2021:

markus2330 commented Sep 9, 2021

markus2330 commented Sep 9, 2021 • edited Loading

qwepoizt commented Sep 9, 2021 • edited Loading

kodebach commented Sep 9, 2021

qwepoizt commented Sep 9, 2021

qwepoizt commented Sep 9, 2021 • edited Loading

Insights

markus2330 commented Sep 9, 2021

kodebach commented Sep 9, 2021

qwepoizt commented Sep 10, 2021

Planned changes

Token mechanism

Checking SFM and SME

Removing features

Answers on your questions/comments

qwepoizt commented Sep 10, 2021

markus2330 commented Sep 11, 2021

qwepoizt commented Sep 13, 2021

Decisions from Elektra meeting 13th Sep 2021:

qwepoizt commented Sep 13, 2021

kodebach commented Sep 13, 2021

qwepoizt commented Sep 14, 2021

kodebach commented Sep 14, 2021

qwepoizt commented Sep 17, 2021 • edited Loading

Done

Current issue

markus2330 commented Sep 20, 2021

kodebach commented Sep 20, 2021

qwepoizt commented Sep 21, 2021

qwepoizt commented Aug 19, 2021 •

edited

Loading

qwepoizt commented Aug 20, 2021 •

edited

Loading

qwepoizt commented Aug 26, 2021 •

edited

Loading

markus2330 commented Sep 9, 2021 •

edited

Loading

qwepoizt commented Sep 9, 2021 •

edited

Loading

qwepoizt commented Sep 9, 2021 •

edited

Loading

qwepoizt commented Sep 17, 2021 •

edited

Loading