Jira (PUP-10819) Improve error handling when a 3x function is deferred

9 views
Skip to first unread message

Nick Walker (Jira)

unread,
Jan 5, 2021, 11:06:03 AM1/5/21
to puppe...@googlegroups.com
Nick Walker commented on Bug PUP-10819
 
Re: Improve error handling when a 3x function is deferred

We should provide a clear error that we could not load a 3x function so that users have a better idea of what to do next. 

For any built-in functions that are still 3x we should convert them to 4x functions.  This will avoid the issue for any built-in functions but we'll still have the error case above for 3x functions from stdlib or other modules.  Generally speaking we should look to convert functions people want to 4x instead of trying to work on loading 3x functions.  

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

Mihai Buzgau (Jira)

unread,
Jan 6, 2021, 4:58:03 AM1/6/21
to puppe...@googlegroups.com

Mihai Buzgau (Jira)

unread,
Jan 6, 2021, 4:58:03 AM1/6/21
to puppe...@googlegroups.com

Mihai Buzgau (Jira)

unread,
Jan 6, 2021, 4:59:04 AM1/6/21
to puppe...@googlegroups.com

Gheorghe Popescu (Jira)

unread,
Jan 7, 2021, 1:33:02 AM1/7/21
to puppe...@googlegroups.com

Josh Cooper (Jira)

unread,
Jan 11, 2021, 7:59:03 PM1/11/21
to puppe...@googlegroups.com
Josh Cooper commented on Bug PUP-10819
 
Re: Improve error handling when a 3x function is deferred

I went down the rabbit hole to understand what the issue was. And I think there's a pretty trivial fix for this.

When resolving deferred values, puppet creates a DeferredResolver which creates a script compiler and scope: https://github.com/puppetlabs/puppet/blob/fdeab5be29364dd7210327ef901eb7d39f7cde35/lib/puppet/pops/evaluator/deferred_resolver.rb#L22. When running puppet apply, the catalog.environment_instance and Puppet.lookup(:current_environment) are the same object. The compiler ends up creating an anonymous module based on the catalog.environment_instance and defining instance methods on that module.

https://github.com/puppetlabs/puppet/blob/fdeab5be29364dd7210327ef901eb7d39f7cde35/lib/puppet/parser/functions.rb#L205-L222

The compiler's scope object then extends the environment module, so that the environment module's methods are instance methods on the scope, e.g.
Scope#function_sprintf.

https://github.com/puppetlabs/puppet/blob/fdeab5be29364dd7210327ef901eb7d39f7cde35/lib/puppet/parser/scope.rb#L1124

The problem is that Puppet::Parser::Functions.environment_module(environment) only memoizes the module if the environment object is exactly the same. It doesn't memoize if a different environment object with the same name:

irb(main):014:0> env = Puppet::Node::Environment.remote('env1')
=> <Puppet::Node::Environment::Remote:70107713527180 @name="env1" @manifest="no_manifest" @modulepath="" >
irb(main):015:0> Puppet::Parser::Functions.environment_module(env)
=> #<Module:0x00007f86428373e8>
irb(main):016:0> Puppet::Parser::Functions.environment_module(env)
=> #<Module:0x00007f86428373e8>
irb(main):017:0> Puppet::Parser::Functions.environment_module(Puppet::Node::Environment.remote('env1'))
=> #<Module:0x00007f86828f99d0>

So the reason this issue fails for the agent is because we download a catalog and create a remote environment instance https://github.com/puppetlabs/puppet/blob/fdeab5be29364dd7210327ef901eb7d39f7cde35/lib/puppet/resource/catalog.rb#L419

Although that environment has the same name as the current environment, it's a different environment object. I pushed up some changes that seem to work: https://github.com/puppetlabs/puppet/compare/main...joshcooper:agent_deferred_3x_functions_10819 Tests haven't been updated yet.

Reply all
Reply to author
Forward
0 new messages