Jira (PUP-9254) Agent Functions - add eval() function

5 views
Skip to first unread message

Henrik Lindberg (JIRA)

unread,
Oct 19, 2018, 10:23:02 AM10/19/18
to puppe...@googlegroups.com
Henrik Lindberg created an issue
 
Puppet / New Feature PUP-9254
Agent Functions - add eval() function
Issue Type: New Feature New Feature
Assignee: Henrik Lindberg
Created: 2018/10/19 7:22 AM
Priority: Normal Normal
Reporter: Henrik Lindberg

Some use cases of Deferred makes logic both complicated (and ugly). As an example when a deferred value needs to be formatted and this formatting must take place on the agent. Likewise, if a deferred value requires transformation before formatting (digging out part of the value, or needing a map, reduce etc.). While some use cases (formatting) can be solved by using a deferred call to inline_epp, that does not work when the resulting value is required to be something other than a string.

To overcome this, a new function that evaluates a string of puppet code should be added. This eval() function is essentially a general purpose inline_epp.

When adding this, it is also of great value to have a "pp" syntax checker for heredoc as a typical use case would be to specify the logic to evaluate using a heredoc. By adding a syntax checker, validation is then at compile time rather than just being done at apply time.
(A Separate ticket is needed for that as it is an independent feature).

This is the signature of the function:

function eval(String $code, Hash[String[1], Any] $variable_bindings) >> Any

When eval evaluates the string it should take place in a local scope nested under top scope. The variables given in $variable_bindings should be set in the local scope. The function should raise an error if the syntax is wrong, or if evaluation raises an error.

For consideration:

  • Add a parameter constraining the return type
Add Comment Add Comment
 
This message was sent by Atlassian JIRA (v7.7.1#77002-sha1:e75ca93)
Atlassian logo

Maggie Dreyer (JIRA)

unread,
Nov 1, 2018, 6:39:03 PM11/1/18
to puppe...@googlegroups.com

Henrik Lindberg (JIRA)

unread,
Nov 16, 2018, 9:32:03 AM11/16/18
to puppe...@googlegroups.com
Henrik Lindberg assigned an issue to Unassigned
Change By: Henrik Lindberg
Assignee: Henrik Lindberg

Henrik Lindberg (JIRA)

unread,
Nov 16, 2018, 9:33:03 AM11/16/18
to puppe...@googlegroups.com
Henrik Lindberg updated an issue
Change By: Henrik Lindberg
Fix Version/s: PUP 6.1.0

Henrik Lindberg (JIRA)

unread,
Nov 16, 2018, 9:33:03 AM11/16/18
to puppe...@googlegroups.com
Henrik Lindberg commented on New Feature PUP-9254
 
Re: Agent Functions - add eval() function

The PR for this includes PUP-9255 - ability in PAL to evaluate in a local scope

Josh Cooper (JIRA)

unread,
Dec 3, 2018, 1:51:19 PM12/3/18
to puppe...@googlegroups.com
Josh Cooper updated an issue
 
Change By: Josh Cooper
Team: Server

Eric Thompson (JIRA)

unread,
Dec 7, 2018, 11:23:03 AM12/7/18
to puppe...@googlegroups.com
Eric Thompson commented on New Feature PUP-9254
 
Re: Agent Functions - add eval() function

Not Eric Sorenson are you aware of this? We wanted to have a discussion about what use cases this is solving and it'd be better if we could delay a release (6.1.0 stop-ship is 12/12). if you are aware of it, what is its relative priority

Eric Sorenson (JIRA)

unread,
Dec 7, 2018, 12:53:05 PM12/7/18
to puppe...@googlegroups.com
Eric Sorenson commented on New Feature PUP-9254

no, I wasn't aware of this work. On the merits it seems like kind of a bad idea.

If you had formatting, munging, etc, to do from a Deferred value, why wouldn't you just do that in the function itself? To me, that boundary is very clean and simple. Adding more complexity to the agent-side language evaluation muddies the waters quite a lot.

Henrik Lindberg has any user asked for this? Without some really concrete use cases and problems that cannot be solved any other way, I would be opposed to continuing down this path.

Henrik Lindberg (JIRA)

unread,
Dec 7, 2018, 1:21:03 PM12/7/18
to puppe...@googlegroups.com

This is because it is not possible to do more than "call a function" but you cannot pass a lambda. The only thing you can do now is to use an EPP template to do the same - but that only works if the result is a string. I added this after having had discussions with people in chat.

This function (eval()) is basically the same as EPP, but you can return something different than a string, and you can use lambdas.

If not liking the idea that you can evaluate puppet logic, then EPP must also be blocked as it does the same.

Sean Millichamp (JIRA)

unread,
Dec 8, 2018, 7:11:05 AM12/8/18
to puppe...@googlegroups.com

Eric Sorenson I was one of the users in one of the chats Henrik Lindberg had about this. I haven't started using Deferred yet (since we're on 2018) so I don't currently have something blocking waiting on eval, but I believe one of the discussions basically led to eval() as the best solution (sorry, I can't recall the particulars). Regardless, I like having options and having access to more than just string return values seems like something that should definitely be possible.

Kenn Hussey (JIRA)

unread,
Dec 10, 2018, 10:28:04 AM12/10/18
to puppe...@googlegroups.com
Kenn Hussey updated an issue
 
Change By: Kenn Hussey
Flagged: Impediment

Henrik Lindberg (JIRA)

unread,
Dec 10, 2018, 1:31:03 PM12/10/18
to puppe...@googlegroups.com
Henrik Lindberg commented on New Feature PUP-9254
 
Re: Agent Functions - add eval() function

Here is some input with why it is beneficial to have an eval() function.

Say you make a deferred call to get information (secret lookup, or some lookup from some cloud service) and the information is more than what you actually want - say you get a hash/ structure back, and you need to a) dig out a value, or b) that you get an array back and you want to filter certain things out, or you get a hash back, and you c) need to use that to call another function that takes two values that both come from the obtained hash.

(a) Get value from deferred result

# using only Deferred
Deferred('get', [Deferred('some_lookup', ['some_key']), 'subkey'])
# using eval
Deferred('eval', ["some_lookup('some_key')['subkey']"])

(b) Get array back, want to filter something out

# Have to write a special 'my_filter' 4.x Ruby function and call it
Deferred('my_filter', [Deferred('some_lookup', ['some_key']). 'red'])
# With eval, use a lambda
Deferred('eval', ["some_lookup('some_key').filter |$x| { $x != 'red' }"])

(c) Want two different values as arguments to a call where values come from a first call

# Showing eval first since the Deferred variant is really bad...
Deferred('eval', ['$tmp = secret_lookup("some_key"); other_func($tmp[0], $tmp[3])']
# With deferred - since there is no way to "tee", the glue must either be written as a specific 4.x Ruby function, or you need to lookup twice
Deferred('other_func', [Deferred('get', [Deferred('some_lookup', ['some_key']), '0']), Deferred('get', [Deferred('some_lookup', ['some_key']), '3'])])

There are many variations on those three use cases but I think those three serve as illustration of how eval makes it a lot easier to perform a deferred computation, and how it removes the need to write use case specific 4x Ruby functions.

Without eval() you can achieve the same by using EPP/ERB and letting the result be JSON or YAML text that is then parsed. That requires a different set of hoops to jump through, but would be better than having to write use case specific functions.

Jean Bond (JIRA)

unread,
Dec 10, 2018, 7:54:03 PM12/10/18
to puppe...@googlegroups.com
Jean Bond commented on New Feature PUP-9254

If we do go forward with these changes, I will need predocs for how users do this, why they want to, etc.

Henrik Lindberg (JIRA)

unread,
Dec 11, 2018, 1:01:04 PM12/11/18
to puppe...@googlegroups.com

Eric Thompson (JIRA)

unread,
Dec 11, 2018, 1:03:04 PM12/11/18
to puppe...@googlegroups.com
Eric Thompson updated an issue
 
Change By: Eric Thompson
Fix Version/s: PUP 6.1.0
Fix Version/s: PUP 6.y

Kenn Hussey (JIRA)

unread,
Dec 12, 2018, 11:00:06 AM12/12/18
to puppe...@googlegroups.com
Kenn Hussey commented on New Feature PUP-9254
 
Re: Agent Functions - add eval() function

Henrik Lindberg are you (still) trying to get these changes in for 6.1 (in which case the PR needs to be merged today)?

Henrik Lindberg (JIRA)

unread,
Dec 12, 2018, 1:51:04 PM12/12/18
to puppe...@googlegroups.com

Kenn Hussey The ticket was changed to 6.y already - I would like it merged for 6.1, but not up to me.

Kenn Hussey (JIRA)

unread,
Dec 12, 2018, 2:20:04 PM12/12/18
to puppe...@googlegroups.com
Kenn Hussey commented on New Feature PUP-9254

Henrik Lindberg thanks, I missed that in the ticket history. I bumped the related tickets (PUP-9252 and PUP-9255) to 6.y as well.

Kenn Hussey (JIRA)

unread,
Dec 12, 2018, 4:06:03 PM12/12/18
to puppe...@googlegroups.com
Kenn Hussey updated an issue
 
Change By: Kenn Hussey
Flagged: Impediment

Gene Liverman (JIRA)

unread,
Feb 12, 2019, 9:25:03 AM2/12/19
to puppe...@googlegroups.com
Gene Liverman commented on New Feature PUP-9254
 
Re: Agent Functions - add eval() function

I am very much in favor of this or something like it. I have already run into instance where I want to use Deferred to call a function so that the call is coming from the agent and then transform that data. Here is an example of what I am doing in a non-Deferred manor and:

  $consul_name_servers = consul_data::get_service_nodes($consul_server, 'consul')
  $consul_dns_port = pick(consul_data::get_key($consul_server, 'consul-dns/port'), '8600')
 
  $nameserver_hash = $consul_name_servers.reduce( {} ) |$memo, $nameserver| {
    $memo + { "${nameserver['Node']}" => "${nameserver['Address']}:${consul_dns_port}" }
  }
 
  haproxy::resolver { 'consul':
    nameservers           => $nameserver_hash,
    resolve_retries       => 3,
    timeout               => {
      'retry' => '2s',
    },
    accepted_payload_size => 8192,
  }

My preference would be to change the first two lines to run agent-side so they could instead just talk to the Consul agent on localhost. The problem with doing this is that, as it stands today, all the subsequent lines will fail to work properly because they are passed a Deferred object instead of the hash that is returned by consul_data::get_service_nodes().

Using eval() this could be done like so:

$deferred_nameserver_hash = Deferred('eval',[@(SOURCE), {'server' => $consul_server}])
  $consul_name_servers = consul_data::get_service_nodes($server, 'consul')
  $consul_dns_port = pick(consul_data::get_key($consul_server, 'consul-dns/port'), '8600')
  $consul_name_servers.reduce( {} ) |$memo, $nameserver| {
    $memo + { "${nameserver['Node']}" => "${nameserver['Address']}:${consul_dns_port}" }
  }
  | SOURCE
 
haproxy::resolver { 'consul':
  nameservers           => $deferred_nameserver_hash,
  resolve_retries       => 3,
  timeout               => {
    'retry' => '2s',
  },
  accepted_payload_size => 8192,
}

Branan Riley (JIRA)

unread,
Feb 12, 2019, 7:33:03 PM2/12/19
to puppe...@googlegroups.com
Branan Riley commented on New Feature PUP-9254

Just spitballing, but: would deferrable puppet-language functions, perhaps enhanced with "file-local functions" or even deferrable lambdas of some sort, solve the problem that this is trying to solve without needing the giant footgun that is a full eval?

Henrik Lindberg (JIRA)

unread,
Feb 13, 2019, 8:53:03 AM2/13/19
to puppe...@googlegroups.com

The eval() function works like a "lambda" - it is evaluated in a local scope (just like a lambda), and you can set variables in it (just like a lambda).
The difference being that a lambda in the language is not a string and is thus parsed and syntax checked by the parser. The same is achieved with eval() by using a heredoc with PP syntax checking - e.g. @(SOURCE:PP).

So - what is the bad thing here? I think it is because the string can be composed. Which is exactly the same as what you can do with an EPP (or worse, with ERB (which we do not allow on the agent)). Also note that eval() evaulates in "script mode", you cannot do eval of any language constructs that have anything to do with a catalog.

Being able to use puppet functions on the agent does not really help - since you would need to write special functions for every use case where you need a lambda. For those, what you really want is the ability to pass a lambda.

We could naturally send lambdas in the catalog. I considered serializing the AST, but I decided against that since that just creates a lot more output in the catalog that would likely cause issues downstream. Instead, the source of the lambda could be serialized and then given to the eval function... When I reached this conclusion I realized that the simplest thing to do was to add the eval() function.

As an exercise, try to implement Gene Liverman's example using only Deferred with support for lambdas. (That case would work better as a separate function in puppet language that then gets called in a deferred way, but adding pluginsync of puppet functions is a much bigger task).

In my view:

  • eval() is no more harmful than epp() and inline_epp()
  • we want to pluginsync puppet functions (lots of implications, performance, organization of plugged-in-synced-content, etc. and would take time)
  • we could add lambda support to Deferred and serialize the string and call eval. This looks nicer, but is essentially the same as eval() since I could just call with() deferred and give it a lambda.

Gene Liverman (JIRA)

unread,
Feb 13, 2019, 9:55:03 AM2/13/19
to puppe...@googlegroups.com
Gene Liverman commented on New Feature PUP-9254

As a sidebar, I have often wanted to be able to use puppet functions on agented nodes while calling puppet apply to test something so pluginsync'ing them is something that sounds appealing to me (totally leaving out other implications). Just my $0.02 on that idea.

Reid Vandewiele (JIRA)

unread,
Apr 26, 2019, 7:34:02 PM4/26/19
to puppe...@googlegroups.com

Another use case for this: plans. Allowing users to supply Puppet code to a plan to be part of an apply block. Opens up the ability to create and use powerful content-development plan-based tools.

Henrik Lindberg (JIRA)

unread,
Apr 27, 2019, 7:48:03 AM4/27/19
to puppe...@googlegroups.com

Josh Cooper Eric Sorenson Given the input above in favor of adding this - can we go ahead and merge this?

Josh Cooper (Jira)

unread,
Jun 12, 2020, 4:50:03 PM6/12/20
to puppe...@googlegroups.com
Josh Cooper commented on New Feature PUP-9254

The eval PR was declined, resetting this ticket status.

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

Josh Cooper (Jira)

unread,
Jun 11, 2021, 8:22:01 PM6/11/21
to puppe...@googlegroups.com
Josh Cooper commented on New Feature PUP-9254

We don't have plans on implementing this anytime soon, so I'm going to close. Please reopen if this is needed.

This message was sent by Atlassian Jira (v8.13.2#813002-sha1:c495a97)
Atlassian logo
Reply all
Reply to author
Forward
0 new messages