Jira (PUP-10417) Custom Ruby functions that define a JSON module can ambiguate its scope and break Puppet

[Garrett Guillotte (Jira)]

Garrett Guillotte created an issue

Puppet / PUP-10417 Custom Ruby functions that define a JSON module can ambiguate its scope and break Puppet Issue Type: Bug Assignee: Unassigned Created: 2020/04/08 5:19 PM Priority: Normal Reporter: Garrett Guillotte Puppet Version: Seen in Puppet 5 and 6 Puppet Server Version: N/A OS Name/Version: RHEL 7 If a custom function defines a Ruby module also used by Puppet (ie. module JSON), and other loaded modules attempt to use related functions (ie. JSON functions from stdlib that call JSON), they fail because Puppet runs the custom function code within the Puppet::Pops::Loader::RubyFunctionInstantiator scope instead of top scope: 2020-03-18T00:00:29.762-05:00 ERROR [qtp802169583-480691] [puppetserver] Puppet Evaluation Error: Error while evaluating a Function Call, undefined method `pretty_generate' for Puppet::Pops::Loader::RubyFunctionInstantiator::JSON:Module (file: /etc/puppetlabs/code/environments/production/modules/npc_coreservices/manifests/components/cache_proxy/install.pp, line: 106, column: 22) on node pv20616cspgl01.npc.lan /etc/puppetlabs/code/environments/production/modules/stdlib/lib/puppet/functions/to_json_pretty.rb:36:in `to_json_pretty' /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/pops/functions/dispatch.rb:60:in `invoke' /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/pops/functions/dispatcher.rb:43:in `block in dispatch' org/jruby/RubyKernel.java:1180:in `catch' /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/pops/functions/dispatcher.rb:42:in `dispatch' /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/pops/functions/function.rb:46:in `block in call' org/jruby/RubyKernel.java:1180:in `catch' /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/pops/functions/function.rb:45:in `call' /etc/puppetlabs/code/environments/production/modules/npc_coreservices/manifests/components/cache_proxy/install.pp:106:in `<eval>' org/jruby/RubyKernel.java:1037:in `eval' ... For example, a Puppet module with a custom Ruby function that declares: require 'json'   module JSON ... and another Puppet module that attempts to prettify JSON via stdlib: $json = to_json_pretty($some_json_content) can produce the above error. For another example, see: https://github.com/sensu/puppet-module-sensuclassic/issues/24 and the patch to work around it: https://github.com/sensu/puppet-module-sensuclassic/pull/25/files Desired Behavior: Users can define a Ruby JSON module in a custom function if necessary without breaking default behavior. Actual Behavior: stdlib doesn't explicitly defines the correct scope when invoking JSON (MODULES ticket tbd), so an eval() statement in RubyFunctionInstantiator executes the code within its scope instead. https://github.com/puppetlabs/puppet/blob/master/lib/puppet/pops/loader/ruby_function_instantiator.rb#L22 If custom Ruby functions cannot be guaranteed to run in the expected scope, or if module declarations are unsafe, this behavior should at a minimum be documented, and if possible the error messages should indicate the scope conflict. Add Comment

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

[Garrett Guillotte (Jira)]

Garrett Guillotte updated an issue

Puppet / PUP-10417 Custom Ruby functions that define a JSON module can ambiguate its scope and break Puppet

Change By: Garrett Guillotte *Puppet Version:* Seen in Puppet 5 and 6 *Puppet Server Version:* N/A *OS Name/Version:* RHEL 7 If a custom function defines a Ruby module also used by Puppet (ie. {{module JSON}}), and other loaded modules attempt to use related functions (ie. JSON functions from {{stdlib}} that call {{JSON}}), they fail because Puppet runs the custom function code within the {{Puppet::Pops::Loader::RubyFunctionInstantiator}} scope instead of top scope: {code}

2020-03-18T00:00:29.762-05:00 ERROR [qtp802169583-480691] [puppetserver] Puppet Evaluation Error: Error while evaluating a Function Call, undefined method `pretty_generate' for Puppet::Pops::Loader::RubyFunctionInstantiator::JSON:Module (file: /etc/puppetlabs/code/environments/production/modules/npc_coreservices/manifests/components/cache_proxy/install.pp, line: 106, column: 22) on node pv20616cspgl01.npc.lan /etc/puppetlabs/code/environments/production/modules/stdlib/lib/puppet/functions/to_json_pretty.rb:36:in `to_json_pretty' /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/pops/functions/dispatch.rb:60:in `invoke' /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/pops/functions/dispatcher.rb:43:in `block in dispatch' org/jruby/RubyKernel.java:1180:in `catch' /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/pops/functions/dispatcher.rb:42:in `dispatch' /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/pops/functions/function.rb:46:in `block in call' org/jruby/RubyKernel.java:1180:in `catch' /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/pops/functions/function.rb:45:in `call'

/etc/puppetlabs/code/environments/production/modules/ npc_coreservices ... /manifests/components/cache_proxy/install.pp:106:in `<eval>'

org/jruby/RubyKernel.java:1037:in `eval' ...

{code}

For example, a Puppet module with a custom Ruby function that declares:

{code}

require 'json' module JSON ...

{code}

and another Puppet module that attempts to prettify JSON via {{stdlib}}:

{code} $json          = to_json_pretty($some_json_content) {code}

can produce the above error. For another example, see: https://github.com/sensu/puppet-module-sensuclassic/issues/24 and the patch to work around it: https://github.com/sensu/puppet-module-sensuclassic/pull/25/files

*Desired Behavior:*

Users can define a Ruby JSON module in a custom function if necessary without breaking default behavior.

*Actual Behavior:* {{stdlib}} doesn't explicitly defines the correct scope when invoking {{JSON}} (MODULES ticket tbd), so an {{eval()}} statement in {{RubyFunctionInstantiator}} executes the code within its scope instead.

https://github.com/puppetlabs/puppet/blob/master/lib/puppet/pops/loader/ruby_function_instantiator.rb#L22 If custom Ruby functions cannot be guaranteed to run in the expected scope, or if module declarations are unsafe, this behavior should at a minimum be documented, and if possible the error messages should indicate the scope conflict.

Add Comment

[Garrett Guillotte (Jira)]

Garrett Guillotte updated an issue

Puppet / PUP-10417 Custom Ruby functions that define a JSON module can ambiguate its scope and break Puppet

Change By: Garrett Guillotte Team: Night's Watch

Add Comment

[Garrett Guillotte (Jira)]

Garrett Guillotte updated an issue

Puppet / PUP-10417 Custom Ruby functions that define a JSON module can ambiguate its scope and break Puppet Change By: Garrett Guillotte

*Puppet Version:* Seen in Puppet 5 and 6 *Puppet Server Version:* N/A *OS Name/Version:* RHEL 7 If a custom function defines a Ruby module also used by Puppet (ie. {{module JSON}}), and other loaded modules attempt to use related functions (ie. JSON functions from {{stdlib}} that call {{JSON}}), they fail because Puppet runs the custom function code within the {{Puppet::Pops::Loader::RubyFunctionInstantiator}} scope instead of top scope: {code} 2020-03-18T00:00:29.762-05:00 ERROR [qtp802169583-480691] [puppetserver] Puppet Evaluation Error: Error while evaluating a Function Call, undefined method `pretty_generate' for Puppet::Pops::Loader::RubyFunctionInstantiator::JSON:Module (file: /etc/puppetlabs/code/environments/production/modules/npc_coreservices/manifests/components/cache_proxy/install.pp, line: 106, column: 22) on node pv20616cspgl01.npc.lan /etc/puppetlabs/code/environments/production/modules/stdlib/lib/puppet/functions/to_json_pretty.rb:36:in `to_json_pretty' /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/pops/functions/dispatch.rb:60:in `invoke' /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/pops/functions/dispatcher.rb:43:in `block in dispatch' org/jruby/RubyKernel.java:1180:in `catch' /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/pops/functions/dispatcher.rb:42:in `dispatch' /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/pops/functions/function.rb:46:in `block in call' org/jruby/RubyKernel.java:1180:in `catch' /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/pops/functions/function.rb:45:in `call'

/etc/puppetlabs/code/environments/production/modules/.../manifests/components/cache_proxy/install.pp:106:in `<eval>'

org/jruby/RubyKernel.java:1037:in `eval' ... {code} For example, a Puppet module with a custom Ruby function that declares: {code} require 'json' module JSON ... {code} and another Puppet module that attempts to prettify JSON via {{stdlib}}: {code} $json          = to_json_pretty($some_json_content) {code} can produce the above error. For another example, see: https://github.com/sensu/puppet-module-sensuclassic/issues/24 and the patch to work around it: https://github.com/sensu/puppet-module-sensuclassic/pull/25/files *Desired Behavior:* Users can define a Ruby JSON module in a custom function if necessary without breaking default behavior. *Actual Behavior:*

{{stdlib}} doesn't explicitly defines the correct scope when invoking {{JSON}} (MODULES ticket tbd -10623 ), so an {{eval()}} statement in {{RubyFunctionInstantiator}} executes the code within its scope instead.

https://github.com/puppetlabs/puppet/blob/master/lib/puppet/pops/loader/ruby_function_instantiator.rb#L22 If custom Ruby functions cannot be guaranteed to run in the expected scope, or if module declarations are unsafe, this behavior should at a minimum be documented, and if possible the error messages should indicate the scope conflict.

Add Comment

[Jarret Lavallee (Jira)]

Jarret Lavallee updated an issue

Puppet / PUP-10417 Custom Ruby functions that define a JSON module can ambiguate its scope and break Puppet

Change By: Jarret Lavallee CS Priority: Needs Priority

Add Comment

[Justin Stoller (Jira)]

Justin Stoller commented on PUP-10417

Re: Custom Ruby functions that define a JSON module can ambiguate its scope and break Puppet I looked into this a little bit today and: I looked into changing how we do that eval and I'm very hesitant to change bindings w/o a lot more leg work, and probably not on anything but a major version boundary. Their "workaround" of "Don't monkey patch JSON" is less a dirty hack and more a best practice. Considering how the implementation of function loading was implemented, I think the assumption was that users would not be defining custom modules in their function files. The alternative would be to do what is recommended for library code w/in types and providers. I didn't see this recommendation on our docs but something like the "Shared Libraries" section of the old https://www.oreilly.com/library/view/puppet-types-and/9781449339319/ch04.html. (The weird "File.expand_path"s can be removed for a more modern "require_relative"). So I think we should document something to the effect of: If you want to factor out helper functions within your 4x style Ruby based Puppet functions you can define those inside the block given to Puppet::Functions.create_function. If you'd like to break those helper functions out to be easily unit testable, share them with other functions, or group them logically into a Ruby Module, you should define them in a namespaced helper file, and require that file within the body of the block you pass to Puppet::Functions.create_function. eg in some Ruby-ish pseudo code: # cat mymodule/lib/puppet/functions/mymodule/foo.rb Puppet::Functions.create_function("mymodule::foo") do require_relative "../../../puppet_x/my_org/my_module/helpers.rb   dispatch "foo" do ...not pertinent... end   def foo(args) PuppetX::MyOrg::MyModule::Helpers.do_a_thing(args) end end   # cat mymodule/lib/puppet_x/my_org/my_module/helpers.rb require 'your/hearts/content'   module PuppetX module MyOrg module MyModule module Helpers def self.do_a_thing(args) ...namespaced to not interfere with puppet & unit testable without requiring puppet! end end end end end

Add Comment

[Charlie Sharpsteen (Jira)]

Charlie Sharpsteen commented on PUP-10417

Re: Custom Ruby functions that define a JSON module can ambiguate its scope and break Puppet

I think the problem is the way customer function definitions are passed to eval(): https://github.com/puppetlabs/puppet/blob/6.14.0/lib/puppet/pops/loader/ruby_function_instantiator.rb#L22 The result is that custom Puppet 4 functions end up sharing Puppet::Pops::Loader::RubyFunctionInstantiator as an enclosing scope instead of being completely isolated from each other. This causes constants declared outside the Puppet::Functions.create_function block to "leak" between Puppet 4 functions. So, someone could run into this even if they weren't trying to monkey-patch the JSON library and just created a new constant for which they decided JSON was the most appropriate name. We could probably fix this, but I also wonder how many people have noticed this behavior and assumed it was an intentional feature they could use to share helper code between functions instead of a flaw in function isolation.

Add Comment

[Garrett Guillotte (Jira)]

Garrett Guillotte commented on PUP-10417

Re: Custom Ruby functions that define a JSON module can ambiguate its scope and break Puppet

In the case of the Sensu Classic module, pretty-printed output is gated behind a configuration flag and ugly output is the default. The core issue seems to be that the behavior when including/declaring JSON is unpredictable and apparently undocumented.

Add Comment

[Jarret Lavallee (Jira)]

Jarret Lavallee updated an issue

Puppet / PUP-10417

Custom Ruby functions that define a JSON module can ambiguate its scope and break Puppet

Change By: Jarret Lavallee CS Priority: Needs Priority Reviewed

Add Comment

[Henrik Lindberg (Jira)]

Henrik Lindberg commented on PUP-10417

Re: Custom Ruby functions that define a JSON module can ambiguate its scope and break Puppet FWIW, and IIRC - the spec says that it is illegal to define new Ruby constants in a 4.x function. Irrespective of where they end up in the ruby runtime they will cause problems

Add Comment

[Justin Stoller (Jira)]

Justin Stoller commented on PUP-10417

Re: Custom Ruby functions that define a JSON module can ambiguate its scope and break Puppet

Thanks, Henrik! Based on that I would say that this should be a documentation ticket that additional helper methods should all live inside the body of the block passed to create_function and the be made a documentation ticket.

Add Comment

[Mihai Buzgau (Jira)]

Mihai Buzgau updated an issue

Puppet / PUP-10417

Custom Ruby functions that define a JSON module can ambiguate its scope and break Puppet

Change By: Mihai Buzgau Team: Night's Watch Froyo

Add Comment

[Maggie Dreyer (Jira)]

Maggie Dreyer commented on PUP-10417

Re: Custom Ruby functions that define a JSON module can ambiguate its scope and break Puppet Did this ever get documented? /cc Kate Medred

Add Comment

[Kate Medred (Jira)]

Kate Medred commented on PUP-10417

Re: Custom Ruby functions that define a JSON module can ambiguate its scope and break Puppet

Maggie Dreyer, I'm asking around, but I don't believe it did. I can pick it up if it hasn't been and needs to be. cc: Jean Bond + Melissa Amos for visibility.

Add Comment

[Jean Bond (Jira)]

Jean Bond commented on PUP-10417

Re: Custom Ruby functions that define a JSON module can ambiguate its scope and break Puppet

There's no linked docs ticket, nor is there anything flagged for docs (ping or DOCS tab), so I imagine it has not been documented. Kate Medred, you can pick this up; Maggie Dreyer or Justin Stoller, where specifically would you suggest this be documented?

Add Comment

[Justin Stoller (Jira)]

Justin Stoller commented on PUP-10417

Re: Custom Ruby functions that define a JSON module can ambiguate its scope and break Puppet

Probably here: https://puppet.com/docs/puppet/6.17/functions_ruby_implementation.html.

Add Comment

[Josh Cooper (Jira)]

Josh Cooper updated an issue

Puppet / PUP-10417

Custom Ruby functions that define a JSON module can ambiguate its scope and break Puppet

Change By: Josh Cooper Team: Froyo Phoenix

Add Comment

This message was sent by Atlassian Jira (v8.20.11#820011-sha1:0629dd8)