[PUP-9357] Noop exec debug logging should include the command Created: 2018/12/11  Updated: 2019/03/26  Resolved: 2019/03/11

Status: Closed
Project: Puppet
Component/s: None
Affects Version/s: None
Fix Version/s: PUP 5.5.12, PUP 6.0.7, PUP 6.4.0

Type: Improvement Priority: Normal
Reporter: Chris Roddy Assignee: Kris Bosland
Resolution: Fixed Votes: 0
Labels: resolved-issue-added
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Template:
Epic Link: Agent Logging
Team: Coremunity
Sprint: Platform Core KANBAN
Release Notes: Enhancement
Release Notes Summary: The exec resource will now print a debug message with the command, if checks prevent it from being executed.
Also, if any of the :command, :onlyif, or :unless parameters are marked sensitive, all commands will be redacted from the log output.
QA Risk Assessment: Needs Assessment

 Description   

When running in noop mode, debug logging for exec resources includes e.g. the unless and onlyif commands, but it does not log the command that would be run, instead merely logging that it has not yet been run and should have been. Because an exec's command is frequently assembled using variable interpolation (and therefore can't be just read from the manifest conclusively), and because the resource will often have a custom title, users running with --noop --debug would likely benefit from seeing the command itself in debug output.

There are other ways of getting this information, so it's not a big problem, but it would be a nice thing to have.

% /opt/puppetlabs/puppet/bin/puppet apply --noop --debug -e "exec { 'example': command => '/bin/true', unless => '/bin/false', }"
--- 8< ---
Info: Applying configuration version '1544556833'
Debug: Exec[example](provider=posix): Executing check '/bin/false'
Debug: Executing: '/bin/false'
Notice: /Stage[main]/Main/Exec[example]/returns: current_value 'notrun', should be ['0'] (noop)
Debug: /Stage[main]/Main/Exec[example]: The container Class[Main] will propagate my refresh event
Notice: Class[Main]: Would have triggered 'refresh' from 1 event
Debug: Class[Main]: The container Stage[main] will propagate my refresh event
Notice: Stage[main]: Would have triggered 'refresh' from 1 event
Debug: Finishing transaction 47213377716240
Debug: Storing state
Debug: Pruned old state cache entries in 0.00 seconds
Debug: Stored state in 0.01 seconds
Notice: Applied catalog in 0.10 seconds
--- 8< ---



 Comments   
Comment by Melissa Stone [ 2019/01/31 ]

pr: https://github.com/puppetlabs/puppet/pull/7368

 

there's a typo in the commit message, which is why the PR isn't being linked to the ticket

Comment by Kris Bosland [ 2019/01/31 ]

Eric Sorenson, I am concerned that 'unless', 'onlyif', and other checks may need to support sensitive for the same reason that the command of an exec should, as I am working on this ticket.  Do you agree?  Have you heard of any customer requests?  Thanks.

Comment by Chris Roddy [ 2019/01/31 ]

I don't have specific customer stories to support it, but it seems intuitively obvious to me that those attributes should support Sensitive.

Comment by Eric Sorenson [ 2019/02/01 ]

Hm, so the code snippet would be

exec { 'example': 
  command => Sensitive.new('/bin/true'), 
  unless => Sensitive.new('/bin/false'), 
}

Like Chris Roddy I don't have any evidence that people need this today but if it's easy to add, then sure, go for it.

Comment by Josh Cooper [ 2019/02/15 ]

Merged to 5.5.x in https://github.com/puppetlabs/puppet/commit/911e9264b7b3c96969e1fac84f15865eb2013337.

Comment by Kris Bosland [ 2019/02/20 ]

Hey Eric Sorenson we were talking about the second PR on this ticket, if we should change the exec(provider) run method, or just revert the change of supporting sensitive on :onlyif and :unless at all, and just going back to the user needing to set sensitive on command or getting no redaction.  Can you help us decide?

Comment by Kris Bosland [ 2019/02/20 ]

We decided to mark all commands here as sensitive if one is marked sensitive.

Comment by Josh Cooper [ 2019/02/26 ]

Follow up PR merged to 5.5.x in https://github.com/puppetlabs/puppet/commit/65bbfe02358629b8a60cfbcc8bf049e4d022d600

Comment by Josh Cooper [ 2019/03/11 ]

Merged to master in b98e6938a2, and passed CI in ee27e9ed1b

Generated at Tue Sep 29 23:49:12 PDT 2020 using Jira 8.5.2#805002-sha1:a66f9354b9e12ac788984e5d84669c903a370049.