There is no Title

It's a blog.

0 notes

Puppet’s Automatic Parameter Lookup is like PHP’s Register Globals

It’s best practice in any programming language to separate code from configuration. Puppet is no different, but it has an interesting system called Hiera that allows you so specify the configuration defaults in YAML or JSON.

For example, you could specify the configuration of a service (in YAML):

foo::services:
  bar:
    ensure: present
    port: 3000
  baz:
    ensure: present
    port: 4000

In Puppet you can query these values and instantiate a resource using something like:

$services = hiera_hash('foo::services', { })
create_resource('foo::service', $serivces)

But what makes Hiera interesting is that you can specify hierachical configurations that are deeply merged. So for example, a common configuration can contain

foo::services:
  bar:
    ensure: absent
    port: 3000
  baz:
    ensure: absent
    port: 4000

and in a another configuration file, chosen by a fact about the node, such as its location, role, hostname:

foo::services:
  bar:
    ensure: present

This allows you to specify consistent parameters across your network, such as port numbers, but only enable the services on machines that are actually configured to have them.

So you can have a class like

class foo() {
  $services = hiera_hash('foo::services', { })
  create_resource('foo::service', $services)
}

and include it universally without worrying that you’ll create services where they shouldn’t be created. Or you can also specify what classes to load in Hiera, using hiera_include:

hiera_include('classes')

and in your Hiera configuration:

classes:
- foo

Puppet v3 has a magical feature called automatic parameter lookup that sets default values using Hiera, for example:

class foo(
  $services = { },
) {
  create_resource('foo::service', $services)
}

This looks up foo::services from Hiera automatically. It makes the code cleaner, but at the risk of being less clear.

The rationale for this in the documentation is that

Automatic parameter lookup is good for writing reusable code because it is regular and predictable.

I disagree that the code is regular and predictable. Notice that I said Hiera “can” do deeper merging of hashes. That’s because it’s not the default setting.

Automatic parameter lookups ignore any changes to the merge settings, and do simple lookups without merging, because they use the plain hiera function that doesn’t support deep merging. This is a documented limitation, though one that is easily overlooked.

So in the above example, Puppet will see the enabled services with no port values, and use whatever the default port is for the foo::service resource (or fail if there is no default port parameter, which is safer).

This is a subtle, and potentially dangerous bug. It also reminds me of the following code smell:

class foo(
  $services = hiera_hash('foo::services', { }),
) {
  create_resource('foo::service', $services)
}

The call to hiera_hash here is useless, because if the value is in Hiera, then the (un-merged) value will be used. Otherwise it will be empty.

This reminds me of the register globals misfeature of older PHP versions, where a call to a URL with a CGI parameter like “/foo.php?bar=baz” initializes the variable $bar with the value “baz”. (See the link for examples why this is dangerous.)

Now Hiera data shouldn’t be coming from untrusted sources, so it doesn’t have the security implications that PHP’s register globals feature does.

But it’s similar in that variables can be assigned values when you don’t expect it, and dangerous when it doesn’t consistently assign values the same way it would to a normal call to hiera_hash or hiera_array in a class body.

Puppet doesn’t warn you about a useless call to Hiera functions here, nor does the (current as of this posting) version of puppet-lint. So it’s easy for people writing Puppet code to forget about this limitation and get this wrong. And getting these things wrong in a DevOps system like Puppet can break a server.

Edit: I’ve corrected some typos and changed the wording about the automatic lookup’s behaviour to be clearer.

Filed under puppet php devops hiera