[PUP-6986] Service provider fails when hasstatus => false and the output of 'ps -ef' happens to contains non-ASCII chars Created: 2016/12/07  Updated: 2017/11/07  Resolved: 2017/10/30

Status: Closed
Project: Puppet
Component/s: None
Affects Version/s: None
Fix Version/s: PUP 4.10.9, PUP 5.3.3

Type: Improvement Priority: Major
Reporter: Erik Hansen Assignee: Unassigned
Resolution: Fixed Votes: 0
Labels: i18n, utf-8
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Relates
relates to PUP-6925 Convert Ruby ::File calls for open to... Closed
relates to PUP-7134 Convert Ruby ::File calls for open to... Closed
relates to PUP-6432 Puppet::Util::Execution.execute canno... Ready for Engineering
Template:
Acceptance Criteria:
  • New tests should reproduce this issue and demonstrate the fix
Epic Link: Phase 1.1 Puppet Unicode Adoption Blockers
Team: Platform Core
Story Points: 3
Sprint: AP 2017-01-11, AP 2017-03-08, Agent 2017-03-22, Platform Core KANBAN
Release Notes: Bug Fix
Release Notes Summary: The base service provider would previously fail with a stacktrace if the process line for any given service contained UTF-8 characters and puppet was not running in UTF-8, causing service checks to fail. puppet will now attempt to convert the PS output to UTF-8, and on failure force valid interpretation by replacing invalid UTF-8 characters with replacement characters. This allows puppet to correctly match the running UTF-8 service in PS output to the managed service name.
QA Risk Assessment: No Action
QA Risk Assessment Reason: Provider failure isn't catastrophic; tested in unit, so no additional work required

 Description   

When a service resource uses the parameter `hasstatus => false`, puppet will search the output of `ps -ef` to determine the status of the service.

If the output of `ps -ef` happens to contain non-ASCII characters, we see the following stack trace:

Error: /Stage[main]/Automic/Service[automic]: Could not evaluate: invalid byte sequence in US-ASCII 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/provider/service/base.rb:38:in `match' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/provider/service/base.rb:38:in `block (2 levels) in getpid' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/provider/service/base.rb:37:in `each_line' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/provider/service/base.rb:37:in `block in getpid' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/provider/service/base.rb:36:in `popen' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/provider/service/base.rb:36:in `getpid' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/provider/service/base.rb:66:in `status' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/type/service.rb:110:in `retrieve' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/type.rb:1068:in `retrieve' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/type.rb:1096:in `retrieve_resource' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/transaction/resource_harness.rb:221:in `from_resource' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/transaction/resource_harness.rb:19:in `evaluate' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/transaction.rb:212:in `apply' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/transaction.rb:228:in `eval_resource' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/transaction.rb:151:in `call' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/transaction.rb:151:in `block (2 levels) in evaluate' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/util.rb:386:in `block in thinmark' 
/opt/puppetlabs/puppet/lib/ruby/2.1.0/benchmark.rb:294:in `realtime' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/util.rb:385:in `thinmark' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/transaction.rb:151:in `block in evaluate' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/graph/relationship_graph.rb:118:in `traverse' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/transaction.rb:142:in `evaluate' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/resource/catalog.rb:222:in `block in apply' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/util/log.rb:155:in `with_destination' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/transaction/report.rb:118:in `as_logging_destination' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/resource/catalog.rb:221:in `apply' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/configurer.rb:171:in `block in apply_catalog' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/util.rb:223:in `block in benchmark' 
/opt/puppetlabs/puppet/lib/ruby/2.1.0/benchmark.rb:294:in `realtime' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/util.rb:222:in `benchmark' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/configurer.rb:170:in `apply_catalog' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/configurer.rb:315:in `run_internal' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/configurer.rb:186:in `block in run' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/context.rb:65:in `override' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet.rb:240:in `override' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/configurer.rb:185:in `run' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/agent.rb:45:in `block (4 levels) in run' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/agent/locker.rb:21:in `lock' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/agent.rb:45:in `block (3 levels) in run' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/agent.rb:98:in `with_client' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/agent.rb:42:in `block (2 levels) in run' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/agent.rb:65:in `run_in_fork' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/agent.rb:41:in `block in run' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/application.rb:179:in `call' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/application.rb:179:in `controlled_run' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/agent.rb:39:in `run' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/application/agent.rb:353:in `onetime' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/application/agent.rb:331:in `run_command' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/application.rb:344:in `block in run' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/util.rb:540:in `exit_on_fail' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/application.rb:344:in `run' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/util/command_line.rb:128:in `run' 
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/util/command_line.rb:72:in `execute' 
/opt/puppetlabs/puppet/bin/puppet:5:in `<main>'

Could puppet avoid this error by translating the output to ASCII-8BIT encoding in the Puppet provider before doing regex matches?



 Comments   
Comment by Ethan Brown [ 2016/12/07 ]

This looks pretty similar to the Puppet::Util::Execution.execute issue in PUP-6432 except that the base service provider goes directly through IO.popen at https://github.com/puppetlabs/puppet/blob/master/lib/puppet/provider/service/base.rb#L36. The spirit is pretty similar though - pull in some output, don't explicitly specify encoding, watch bad things happen.

Moses Mendoza and I have done some auditing in the past and are aware of some other calls that use IO directly / indirectly (i.e. File derives from IO in Ruby) and don't specify encoding that are problematic. A number of them are going to be resolved in PUP-6925, but the trickier issues are going to be isolated / fixed independently.

I'm pulling this into the Phase 1 blockers epic because we need to ensure that this case is properly tested / fixed.

Thanks for the report.

Comment by Charlie Sharpsteen [ 2016/12/08 ]

Since pe -ef output can contain whatever arbitrary bytes that other processes happen to feel like adding to their command lines, submitted a quick patch that just does the regex matching in a binary ASCII-8BIT encoding.

Comment by Erik Hansen [ 2016/12/12 ]

Anyone hitting this issue might be able to work around the problem by setting the environment variable LC_CTYPE=en_US.UTF-8 either as a system setting or applied to the environment that the agent runs in.

Comment by Moses Mendoza [ 2017/03/10 ]

the PR for this is no longer linked to the ticket, so I linked it

Comment by Moses Mendoza [ 2017/03/10 ]

comments left in PR, assigning to Charlie Sharpsteen for continuing iteration

Comment by Adrien Thebo [ 2017/09/14 ]

PR is currently targeted at 4.10.x; I'm inquiring into whether we can retarget it at master to make the merge easier to justify.

Comment by Moses Mendoza [ 2017/10/19 ]

merged to 4.10.x at https://github.com/puppetlabs/puppet/commit/74ff444293ac19d6150e5d694e8610b60d5c4343

Generated at Wed Nov 13 13:04:41 PST 2019 using JIRA 7.7.1#77002-sha1:e75ca93d5574d9409c0630b81c894d9065296414.