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

Augeas error messages do not mention used lens and are vague #203

Closed markus2330 closed 4 years ago

markus2330 commented 9 years ago

Actual Outcome:

# kdb mount /etc/security/limits.conf system/limits augeas lens=Limits.lns
# cat /etc/security/limits.conf 
*               soft    core            1000000  #1GB
# kdb ls system/limits
The command ls terminated unsuccessfully with the info: 
Error (#85) occurred!
Description: an Augeas error occurred
Ingroup: plugin
Module: storage
At: /home/jenkins/workspace/workspace/elektra-git-buildpackage-wheezy/libelektra/src/plugins/augeas/augeas.c:397
Reason: Iterated lens matched less than it should
Mountpoint: system/limits
Configfile: /etc/security/limits.conf

Expected outcome: 1.) Please add path to lens for augeas related errors (ideally with full path, if available):

Reason: Iterated lens /path/to/Limits.lns matched less than it should

2.) Maybe its possible to overall improve the error messages, its not really clear to me what "Iterated lens matched less than it should" means. What exactly is iterated, which expression should match what? Or is the error message from augeas so weak?

markus2330 commented 9 years ago

Another, related issue:

# sudo chmod 600 limits.conf
# kdb ls system/limits
The command ls terminated unsuccessfully with the info: 
Error (#9) occurred!
Description: Insufficient permissions to open configuration file for writing. You might want to retry as root.
Ingroup: kdb
Module: 
At: /home/jenkins/workspace/workspace/elektra-git-buildpackage-wheezy/libelektra/src/plugins/augeas/augeas.c:378
Reason: Permission denied
Mountpoint: system/limits
Configfile: /etc/security/limits.conf

It should state: Insufficient permissions to open configuration file for reading. (It tried to open with "r")

fberlakovich commented 7 years ago

1.) Please add path to lens for augeas related errors (ideally with full path, if available):

Reason: Iterated lens /path/to/Limits.lns matched less than it should 2.) Maybe its possible to overall improve the error messages, its not really clear to me what "Iterated lens matched less than it should" means. What exactly is iterated, which expression should match what? Or is the error message from augeas so weak?

The errors stem from augeas. Debugging them can be tricky, but unfortunately this is a common problem with any kind of parsers. However, the wiki (https://github.com/hercules-team/augeas/wiki) contains some useful tips on how to track down errors.

fberlakovich commented 7 years ago

It should state: Insufficient permissions to open configuration file for reading. (It tried to open with "r")

Could it be that this issue has already been fixed somewhere else? I cannot find any reference to the stated error in the augeas plugin.

markus2330 commented 7 years ago

It is possible that I already fixed the permission error issue, sorry for mixing up multiple issues here.

The remaining issue (and largest issue of the augeas plugin) is that the file name of the lens is not the expected name of the lens. And to make things worse there is no indication what the actual expected lens parameter needs.

This needs improvements:

markus2330 commented 5 years ago

@sanssecours be aware of these problems when playing around with augeas.

markus2330 commented 5 years ago

@Piankero another very weak error message. Maybe it is already fixed anyway.

ghost commented 5 years ago

Compared to the version of April, 2015 the message itself was already improved. This section did not provide all the information and now does provide the necessary information.

It should state: Insufficient permissions to open configuration file for reading. (It tried to open with "r")

Also the permission error has been improved.

This ticket can be closed

markus2330 commented 5 years ago

For me it does not work, I get a crash:

sudo kdb mount /etc/security/limits.conf system/tests augeas lens=NonExistingLens
kdb ls system/tests

(/etc/security/limits.conf can be any existing file) gives the output:

free(): double free detected in tcache 2

Sorry, I crashed by the signal SIGABRT
This should not have happened!

Please report the issue at https://issues.libelektra.org/
zsh: abort (core dumped)

with following stack trace:

__GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
50      ../sysdeps/unix/sysv/linux/raise.c: Datei oder Verzeichnis nicht gefunden.
(gdb) bt
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x00007ffff7b75535 in __GI_abort () at abort.c:79
#2  0x00007ffff7bcc508 in __libc_message (action=action@entry=do_abort, fmt=fmt@entry=0x7ffff7cd728d "%s\n")
    at ../sysdeps/posix/libc_fatal.c:181
#3  0x00007ffff7bd2c1a in malloc_printerr (str=str@entry=0x7ffff7cd8f58 "free(): double free detected in tcache 2")
    at malloc.c:5341
#4  0x00007ffff7bd46fd in _int_free (av=0x7ffff7d0ec40 <main_arena>, p=0x5555556cac10, have_lock=<optimized out>)
    at malloc.c:4193
#5  0x00007ffff7bc2881 in _IO_new_fclose (fp=fp@entry=0x5555556cac20) at iofclose.c:77
#6  0x00007ffff78d5314 in elektraAugeasGet (handle=<optimized out>, returned=0x5555557b1970, 
    parentKey=0x5555557ae310) at ./src/plugins/augeas/augeas.c:540
#7  0x00007ffff7ed4b6d in elektraGetDoUpdate (parentKey=0x5555557ae310, split=0x5555557a4dc0)
    at ./src/libs/elektra/kdb.c:554
#8  kdbGet (handle=<optimized out>, ks=0x5555556afee0, parentKey=0x5555557ae310) at ./src/libs/elektra/kdb.c:1153
#9  0x0000555555626efd in kdb::KDB::get (parentKey=..., returned=..., this=0x5555556aacd0)
    at ./src/bindings/cpp/include/keyset.hpp:592
#10 LsCommand::execute (this=0x5555556aacc0, cl=...) at ./src/tools/kdb/ls.cpp:33
#11 0x00005555555b1d4d in main (argc=<optimized out>, argv=0x7fffffffe498)
    at /usr/include/c++/8/bits/unique_ptr.h:342

It seems like a bug was introduced when migrating the macros in 339d34dfa331926e486340c6343b62266fb28e14

The previous macro did a return -1, the later one did not.

Please fix the problem and write a test case for it.

Please also make sure that this sort of error did not happen in other places.

ghost commented 4 years ago

Since the crash is removed the necessary information should be shown. We should be able to close this ticket

markus2330 commented 4 years ago

No, the name of the lens is still now shown, see #3120

ghost commented 4 years ago

The error message which is shown does not come from Elektra itself but from the external library augeas.h: https://github.com/ElektraInitiative/libelektra/blob/cc2a31416836c1bff91eb9d9d5f2dd4ad29bb418/src/plugins/augeas/augeas.c#L119

If I try to include the lens like here it simply returns an empty string.

I fear I cannot fix this issue because it is not in our domain

markus2330 commented 4 years ago

lensPath exists outside of this function. You can simply concatenate getAugeasError and the lens name.

ghost commented 4 years ago

No, the name of the lens is still now shown, see #3120

Could you verify and close the issue if it passes?

sanssecours commented 4 years ago

I tried to reproduce the problem using the steps of the issue description. On my machine the command:

kdb ls system/limits

prints the following error message:

Sorry, module augeas issued the error C01200:
Installation: parse_failed
    position: 1:8
    message: Iterated lens matched less than it should
    lens: /usr/local/Cellar/augeas/1.12.0/share/augeas/lenses/dist/limits.aug:66.17-.40:

. As you can see the error message does includes the location of the lens now 👍. There are two minor details we still could improve in my opinion.

  1. The error message contains an empty line at the end.
  2. The location data :66.17-.40: is a little bit confusing. Why is there a colon at the end? Usually I would expect either nothing or a dot at the end of the message.
ghost commented 4 years ago

Your kind of error messages goes into a different if-else branch where you provide full information. you seem to have provided a correct lens, try sudo kdb mount /etc/security/limits.conf system/tests augeas lens=NonExistingLens

The error message contains an empty line at the end.

All error message have this, I do not think it harms the output if there is an extra space.

The location data :66.17-.40: is a little bit confusing. Why is there a colon at the end? Usually I would expect either nothing or a dot at the end of the message.

this comes from augeas itself. You actually just provided lens: whereas the rest comes from the aug_get call

https://github.com/ElektraInitiative/libelektra/blob/e74733627485950e31eb37c7af4badacbce7352f/src/plugins/augeas/augeas.c#L143

sanssecours commented 4 years ago

you seem to have provided a correct lens,…

Yep. I just repeated the steps from the issue description.

… try sudo kdb mount /etc/security/limits.conf system/tests augeas lens=NonExistingLens

I tried the commands

kdb mount /etc/security/limits.conf system/tests augeas lens=NonExistingLens
kdb ls system/limits

, which do not print any error message on my system as described here. Anyway, I am fine with the current situation. I just commented on this issue, since you requested my review in PR #3286.

ghost commented 4 years ago

Alright, since it is merged and everybody agrees I will close this issue