Bug #10084
Debug output includes way too much selinux details.
| Status: | Needs Decision | Start date: | 10/14/2011 | |
|---|---|---|---|---|
| Priority: | Normal | Due date: | ||
| Assignee: |  Sean Millichamp |
% Done: | 0% |
|
| Category: | - | |||
| Target version: | 2.7.x | |||
| Affected Puppet version: | 2.6.11 | Branch: | ||
| Keywords: | ||||
| Votes: | 1 |
Description
When I run “puppet agent —debug” I get about 1400 lines of selinux details, and about 120 lines of actual debug. These aren’t actually changes to selinux attributes, just noting the selinux attribute of every file opened.
debug: /File[/etc/puppet/auth.conf]/seluser: Found seluser default 'system_u' for /etc/puppet/auth.conf
debug: /File[/etc/puppet/auth.conf]/selrole: Found selrole default 'object_r' for /etc/puppet/auth.conf
debug: /File[/etc/puppet/auth.conf]/seltype: Found seltype default 'etc_t' for /etc/puppet/auth.conf
debug: /File[/etc/snmp/snmpd.conf]/seluser: Found seluser default 'system_u' for /etc/snmp/snmpd.conf
debug: /File[/etc/snmp/snmpd.conf]/selrole: Found selrole default 'object_r' for /etc/snmp/snmpd.conf
debug: /File[/etc/snmp/snmpd.conf]/seltype: Found seltype default 'etc_t' for /etc/snmp/snmpd.conf
debug: /File[/etc/nsswitch.conf]/seluser: Found seluser default 'system_u' for /etc/nsswitch.conf
debug: /File[/etc/nsswitch.conf]/selrole: Found selrole default 'object_r' for /etc/nsswitch.conf
debug: /File[/etc/nsswitch.conf]/seltype: Found seltype default 'etc_t' for /etc/nsswitch.conf
I believe that this is unnecessary debug and makes it very difficult to track down other problems. Would Puppetlabs accept a feature request to move this selinux attribute echoing to higher or different debug level?
History
Updated by Jacob Helwig 6 months ago
- Description updated (diff)
Updated by Jacob Helwig 6 months ago
- Status changed from Unreviewed to Needs Decision
- Assignee set to Jacob Helwig
We’d need to create a new log level to handle this, and have a way of exposing it to the command-line.
Right now, debug is the most verbose log level we have (from lib/puppet/util/log.rb):
@levels = [:debug,:info,:notice,:warning,:err,:alert,:emerg,:crit]
The current setup of --verbose and --debug don’t quite play well with having multiple debugging levels. I could see changing --debug to take an optional debug level (though I don’t think this currently plays well with our option handling).
Even better (to me, anyway) would be if we deprecated the dual --verbose/--debug flags for a single --verbose [level] flag (possibly supporting the -vvvv style of “more ‘v’s == more verbose”.
In any case, we’d need to come up with another log level to move the output to, and a way to allow people to view it from the command line (preferably without adding yet another verbosity related flag).
Updated by Jacob Helwig 5 months ago
- Assignee changed from Jacob Helwig to Nigel Kersten
Updated by Nigel Kersten 5 months ago
Is this actually useful info for an SELinux user to be presented in debug level anyway?
(not disagreeing with anything Jacob posted above, but I’m wondering whether this is actually even necessary debug output)
Updated by Jo Rhett 5 months ago
Nigel Kersten wrote:
Is this actually useful info for an SELinux user to be presented in debug level anyway?
(not disagreeing with anything Jacob posted above, but I’m wondering whether this is actually even necessary debug output)
I would say that I have debugged selinux problems before, and I don’t believe this would have ever helped me in the past.
Updated by Nigel Kersten 5 months ago
- Status changed from Needs Decision to Accepted
- Assignee deleted (
Nigel Kersten) - Target version set to 2.7.x
I vote we remove them, as I can’t think of a situation where this particular info would have helped.
Updated by James Turnbull 5 months ago
- Status changed from Accepted to Needs Decision
- Assignee set to Sean Millichamp
Sean and Mike – do you have a view on this?
Updated by Michael Stahnke 5 months ago
I am of the opinion this is valid debug output. When you don’t have selinux enabled, you don’t see those lines. When you do, you should see those lines, as they could help with debugging. Debug logs are supposed to contain the maximum amount of information to assist with finding a problem. I think they’ve done that here. My opinion on this isn’t super-strong though, so if somebody has some good arguments against, I’d be happy to hear them.
Updated by Sean Millichamp 5 months ago
In general, even when debugging an SELinux problem, they are probably not typically useful. I used them when I was developing the support way back when and have grumbled about them filling my debug output since.
However, I have stopped short of submitting a patch to remove them because, to Mike’s point, they do fully explain what is going on – even if it is a lot of noise 99% of the time.
I could be persuaded either way on this. Perhaps an acceptable compromise would be to enhance the file type documentation slightly to say something along the lines of “if you need to determine what default value Puppet will be using for a given resource, you should run
matchpathcon "full path to file"
and then remove the debug statements.
Updated by Jo Rhett 5 months ago
Michael Stahnke wrote:
I am of the opinion this is valid debug output. When you don’t have selinux enabled, you don’t see those lines. When you do, you should see those lines, as they could help with debugging. Debug logs are supposed to contain the maximum amount of information to assist with finding a problem.
The problem is that it turns 120 lines of debugging into 1200 lines of debug on a small manifest, and 500 lines of output into 1500 lines of output on a large manifest. You can’t find anything you care about because it’s completely surrounded with debug output that isn’t useful.
I don’t mind selinux debugging that tells us changes in selinux attributes or state. This is useful and necessary. I don’t find the debug output that simply says “this file is this state” when that state is neither being evaluated nor changed in a manifest. For example, the selinux output above which tells us the selinux state of each configuration file that puppet loads.