Jira (PUP-10719) Puppet's feature detection can leave rubygems in a bad state

19 views
Skip to first unread message

Josh Cooper (Jira)

unread,
Oct 14, 2020, 4:10:02 PM10/14/20
to puppe...@googlegroups.com
Josh Cooper created an issue
 
Puppet / Bug PUP-10719
Puppet's feature detection can leave rubygems in a bad state
Issue Type: Bug Bug
Assignee: Unassigned
Created: 2020/10/14 1:09 PM
Priority: Normal Normal
Reporter: Josh Cooper

If a gem is managed by a native package manager (rpm, etc), and it is updated during a puppet run, it is possible for rubygems to be left in a bad state causing the run to fail.

This can be reproduced using:

require 'fileutils'
require 'rubygems'if File.exist?("/tmp/thor-1.0.1.gemspec")
  FileUtils.mv("/tmp/thor-1.0.1.gemspec", "/opt/puppetlabs/puppet/lib/ruby/gems/2.5.0/specifications/thor-1.0.1.gemspec")
end
Gem.clear_paths
begin
  require 'msgpack'
rescue LoadError => e
end
FileUtils.mv("/opt/puppetlabs/puppet/lib/ruby/gems/2.5.0/specifications/thor-1.0.1.gemspec", "/tmp/thor-1.0.1.gemspec")
Gem::Specification.latest_specs

Rubygems monkey patches Kernel.require so that if the gem fails to load, it will try to activate it. This has the side effect of caching the stub specifications for all gems.

If the gemspec is then removed via rpm upgrade, then the cached stub specification will refer to a gemspec that no longer exists.

If we later try to load the full specifications (such as when autoloading a terminus), then rubygems will raise:

/opt/puppetlabs/puppet/lib/ruby/2.5.0/rubygems/specification.rb:743:in `_all': pid: 9272 nil spec! included in [...#<Gem::StubSpecification:0x0000000001bf0bc0 @extension_dir=nil, @full_gem_path=nil, @gem_dir=nil, @ignored=nil, @loaded_from="/opt/puppetlabs/puppet/lib/ruby/gems/2.5.0/specifications/thor-1.0.1.gemspec", @data=#<Gem::StubSpecification::StubLine:0x0000000001c44810 @name="thor", @version=#<Gem::Version "1.0.1">...]

Add Comment Add Comment
 
This message was sent by Atlassian Jira (v8.5.2#805002-sha1:a66f935)
Atlassian logo

Josh Cooper (Jira)

unread,
Oct 14, 2020, 4:12:03 PM10/14/20
to puppe...@googlegroups.com
Josh Cooper commented on Bug PUP-10719
 
Re: Puppet's feature detection can leave rubygems in a bad state

One option is to call Gem.clear_paths if a feature fails to load.

Or rescue the exception when calling Gem::Specifications.latest_specs, calling Gem.clear_paths and then retry once.

Josh Cooper (Jira)

unread,
Oct 14, 2020, 4:13:03 PM10/14/20
to puppe...@googlegroups.com
Josh Cooper updated an issue
 
Change By: Josh Cooper
If a gem is managed by a native package manager (rpm, etc), and it is updated during a puppet run, it is possible for rubygems to be left in a bad state causing the run to fail.

This can be reproduced using:

{code:ruby}
require 'fileutils'
require 'rubygems'

if File.exist?("/tmp/thor-1.0.1.gemspec")
  FileUtils.mv("/tmp/thor-1.0.1.gemspec", "/opt/puppetlabs/puppet/lib/ruby/gems/2.5.0/specifications/thor-1.0.1.gemspec")
end
Gem.clear_paths
begin
  require 'msgpack'
rescue LoadError => e
end
FileUtils.mv("/opt/puppetlabs/puppet/lib/ruby/gems/2.5.0/specifications/thor-1.0.1.gemspec", "/tmp/thor-1.0.1.gemspec")
Gem::Specification.latest_specs
{code}


Rubygems monkey patches {{Kernel.require}} so that if the gem fails to load, it will try to activate it. This has the side effect of caching the stub specifications for all gems.

If the gemspec is then removed via rpm upgrade, then the cached stub specification will refer to a gemspec that no longer exists.

If we later try to load the full specifications (such as when autoloading a terminus), then rubygems will raise:

{noformat}

/opt/puppetlabs/puppet/lib/ruby/2.5.0/rubygems/specification.rb:743:in `_all': pid: 9272 nil spec! included in [...#<Gem::StubSpecification:0x0000000001bf0bc0 @extension_dir=nil, @full_gem_path=nil, @gem_dir=nil, @ignored=nil, @loaded_from="/opt/puppetlabs/puppet/lib/ruby/gems/2.5.0/specifications/thor-1.0.1.gemspec", @data=#<Gem::StubSpecification::StubLine:0x0000000001c44810 @name="thor", @version=#<Gem::Version "1.0.1">...]
{noformat}

Josh Cooper (Jira)

unread,
Oct 14, 2020, 4:17:03 PM10/14/20
to puppe...@googlegroups.com


It can also be reproduced in puppet using:

{code:ruby}
require 'fileutils'
require 'puppet'


if File.exist?("/tmp/thor-1.0.1.gemspec")
  FileUtils.mv("/tmp/thor-1.0.1.gemspec", "/opt/puppetlabs/puppet/lib/ruby/gems/2.5.0/specifications/thor-1.0.1.gemspec")
end

Puppet.initialize_settings
Puppet[:log_level] = 'debug'

Puppet.features.msgpack?

FileUtils.mv("/opt/puppetlabs/puppet/lib/ruby/gems/2.5.0/specifications/thor-1.0.1.gemspec", "/tmp/thor-1.0.1.gemspec")
Puppet::FileBucket::File.indirection.terminus(:rest)
{code}

Josh Cooper (Jira)

unread,
Oct 14, 2020, 8:14:03 PM10/14/20
to puppe...@googlegroups.com
Josh Cooper commented on Bug PUP-10719
 
Re: Puppet's feature detection can leave rubygems in a bad state

Interestingly, the feature that enables puppet to load extensions from gems causes the number of file I/O operations to increase by ~2.5 times during catalog compilation using the pupetserver perf control repo. Seems completely unnecessary for the rubygems cache to be cleared during compilation.

[root@unheeded-statue ~]# strace -e trace=file -o trace.log -- puppet catalog compile -E perf_control  > /dev/null
[root@unheeded-statue ~]# grep /opt/puppetlabs/puppet/lib/ruby/gems/2.5.0 trace.log  | wc -l
13743
[root@unheeded-statue ~]# strace -e trace=file -o trace-nogems.log -- puppet catalog compile -E perf_control  > /dev/null
...
[root@unheeded-statue ~]# grep /opt/puppetlabs/puppet/lib/ruby/gems/2.5.0 trace-nogems.log | wc -l
5518

Josh Cooper (Jira)

unread,
Oct 14, 2020, 8:22:02 PM10/14/20
to puppe...@googlegroups.com

Josh Cooper (Jira)

unread,
Oct 14, 2020, 8:25:02 PM10/14/20
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Sprint: Platform Core KANBAN

Josh Cooper (Jira)

unread,
Oct 14, 2020, 8:25:02 PM10/14/20
to puppe...@googlegroups.com

Josh Cooper (Jira)

unread,
Oct 14, 2020, 8:25:03 PM10/14/20
to puppe...@googlegroups.com
Josh Cooper commented on Bug PUP-10719

One option is to call Gem.clear_paths if a feature fails to load.

That causes a slow down because we end up repeatedly failing and retrying for gems that really aren't there, like msgpack.

Or rescue the exception when calling Gem::Specifications.latest_specs

That means we're still loading the full gemspec unnecessarily.

Josh Cooper (Jira)

unread,
Oct 19, 2020, 8:39:03 PM10/19/20
to puppe...@googlegroups.com

Josh Cooper (Jira)

unread,
Oct 19, 2020, 8:42:03 PM10/19/20
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Release Notes: Bug Fix
Release Notes Summary: Fixes a rubygems caching issue that could prevent the agent from applying a catalog if a gem is managed using the native package manager, such as yum or apt.

Josh Cooper (Jira)

unread,
Oct 20, 2020, 4:40:03 PM10/20/20
to puppe...@googlegroups.com

Mihai Buzgau (Jira)

unread,
Nov 5, 2020, 3:58:02 AM11/5/20
to puppe...@googlegroups.com

Claire Cadman (Jira)

unread,
Nov 9, 2020, 9:50:03 AM11/9/20
to puppe...@googlegroups.com
Reply all
Reply to author
Forward
0 new messages