Jira (PUP-10222) Puppet lookup invocation recursion detection not thread-safe

15 views
Skip to first unread message

Maggie Dreyer (JIRA)

unread,
Jan 7, 2020, 1:39:04 PM1/7/20
to puppe...@googlegroups.com
Maggie Dreyer created an issue
 
Puppet / Bug PUP-10222
Puppet lookup invocation recursion detection not thread-safe
Issue Type: Bug Bug
Assignee: Unassigned
Created: 2020/01/07 10:38 AM
Priority: Normal Normal
Reporter: Maggie Dreyer

Initial production testing with Puppet in multithreaded mode has revealed that the code to detect recursive hiera lookups is not thread-safe:

2020-01-07T08:41:31.500-08:00 ERROR [qtp612319309-15848] [puppetserver] Puppet Evaluation Error: Error while evaluating a Resource Statement, Lookup of key 'ntp::config_template' failed: Recursive lookup detected in [epel::epel_debuginfo_sslclientkey, ntp::tinker, ntp::leapfile, ntp::config_template] (file: /etc/puppetlabs/code/environments/production/site/profile/manifests/time.pp, line: 2, column: 3) on node ldap-testfixture-prod-2.delivery.puppetlabs.net

These errors come from https://github.com/puppetlabs/puppet/blob/master/lib/puppet/pops/lookup/invocation.rb#L84. Current theory is that since the @name_stack class variable is not thread-safe, if multiple threads look for the same key at the same time, this error could occur.

Add Comment Add Comment
 
This message was sent by Atlassian JIRA (v7.7.1#77002-sha1:e75ca93)
Atlassian logo

Maggie Dreyer (JIRA)

unread,
Jan 7, 2020, 1:40:04 PM1/7/20
to puppe...@googlegroups.com

Maggie Dreyer (JIRA)

unread,
Jan 7, 2020, 4:16:05 PM1/7/20
to puppe...@googlegroups.com
Maggie Dreyer updated an issue
Initial production testing with Puppet in multithreaded mode has revealed that the code to detect recursive hiera lookups is not thread-safe:
{code}

2020-01-07T08:41:31.500-08:00 ERROR [qtp612319309-15848] [puppetserver] Puppet Evaluation Error: Error while evaluating a Resource Statement, Lookup of key 'ntp::config_template' failed: Recursive lookup detected in [epel::epel_debuginfo_sslclientkey, ntp::tinker, ntp::leapfile, ntp::config_template] (file: /etc/puppetlabs/code/environments/production/site/profile/manifests/time.pp, line: 2, column: 3) on node ldap-testfixture-prod-2.delivery.puppetlabs.net
{code}

These errors come from https://github.com/puppetlabs/puppet/blob/master/lib/puppet/pops/lookup/invocation.rb#L84. Current theory is that since the {{@
name_stack current }} class variable is not thread-safe, if multiple threads look for the same key at the same time, this error could occur.

Maggie Dreyer (JIRA)

unread,
Jan 7, 2020, 4:17:03 PM1/7/20
to puppe...@googlegroups.com
Maggie Dreyer updated an issue
Change By: Maggie Dreyer
Method Found: Needs Assessment Customer Feedback

Maggie Dreyer (JIRA)

unread,
Jan 7, 2020, 4:17:04 PM1/7/20
to puppe...@googlegroups.com
Maggie Dreyer updated an issue
Initial production testing by Customer0 with Puppet in multithreaded mode has revealed that the code to detect recursive hiera lookups is not thread-safe:

{code}
2020-01-07T08:41:31.500-08:00 ERROR [qtp612319309-15848] [puppetserver] Puppet Evaluation Error: Error while evaluating a Resource Statement, Lookup of key 'ntp::config_template' failed: Recursive lookup detected in [epel::epel_debuginfo_sslclientkey, ntp::tinker, ntp::leapfile, ntp::config_template] (file: /etc/puppetlabs/code/environments/production/site/profile/manifests/time.pp, line: 2, column: 3) on node ldap-testfixture-prod-2.delivery.puppetlabs.net
{code}

These errors come from https://github.com/puppetlabs/puppet/blob/master/lib/puppet/pops/lookup/invocation.rb#L84. Current theory is that since the {{@current}} class variable is not thread-safe, if multiple threads look for the same key at the same time, this error could occur. In their environments, this was happening when doing automatic lookup of class params via hiera.

Maggie Dreyer (JIRA)

unread,
Jan 7, 2020, 4:17:04 PM1/7/20
to puppe...@googlegroups.com
Maggie Dreyer updated an issue
 
Puppet / Bug PUP-10222
Puppet lookup invocation recursion detection not thread-safe
Change By: Maggie Dreyer
Initial production testing by Customer0 with Puppet in multithreaded mode has revealed that the code to detect recursive hiera lookups is not thread-safe:
{code}
2020-01-07T08:41:31.500-08:00 ERROR [qtp612319309-15848] [puppetserver] Puppet Evaluation Error: Error while evaluating a Resource Statement, Lookup of key 'ntp::config_template' failed: Recursive lookup detected in [epel::epel_debuginfo_sslclientkey, ntp::tinker, ntp::leapfile, ntp::config_template] (file: /etc/puppetlabs/code/environments/production/site/profile/manifests/time.pp, line: 2, column: 3) on node ldap-testfixture-prod-2.delivery.puppetlabs.net
{code}

These errors come from https://github.com/puppetlabs/puppet/blob/master/lib/puppet/pops/lookup/invocation.rb#L84. Current theory is that since the {{@current}} class variable is not thread-safe, if multiple threads look for the same key at the same time, this error could occur.

Maggie Dreyer (JIRA)

unread,
Jan 8, 2020, 7:03:04 PM1/8/20
to puppe...@googlegroups.com
Maggie Dreyer commented on Bug PUP-10222
 
Re: Puppet lookup invocation recursion detection not thread-safe

So, here's what I think is happening: as we are looking up a key, we store a copy of the current instance of Invocation in the @current class variable here. If, when a new Invocation is initialized, @current is not nil, we think that the value of @current is our parent, so we copy the name_stack from that "parent" to the @name_stack for this instance.

However, in a multithreaded environment, it is possible for @current to have been set by a different thread, so the new instance will think it has a parent when it does not. If that parent Invocation was trying to look up a key of the same name as the current instance, we will throw a RecursiveLookup error, because the name stack contains our key already.

Maggie Dreyer (JIRA)

unread,
Jan 9, 2020, 5:49:03 PM1/9/20
to puppe...@googlegroups.com

Maggie Dreyer (JIRA)

unread,
Jan 9, 2020, 5:49:04 PM1/9/20
to puppe...@googlegroups.com
Reply all
Reply to author
Forward
0 new messages