Equivocal Puppet Class Parameters

While working with Puppet recently I have noticed that there is some funny business going on with the rules of parameter naming. The Puppet’s documentation states:

Parameter names begin with a dollar sign prefix ($). The parameter name after the prefix:

Must begin with a lowercase letter.

Can include lowercase letters.

Can include digits.

Can include underscores.

Let’s see if this is true. Tried applying this manifest with Puppet 5.5.10 which is available on Ubuntu Focal Fossa (20.04):

class test(
  String $_content
){
  file {'/tmp/helloworld':
    content =>  "${_content}\n",
  }
}


class { 'test':
  _content => "123"
}

And it does not work as expected:

$ puppet apply hello.pp
Error: Could not parse for environment production: Syntax error at '_content' (file: /home/gstatkevicius/dev/puppet_testing/manifest/hellofile.pp, line: 11, column: 3) on node gstatkevicius-desktop

Now, let’s try creating a simple hiera configuration:

---
:backends:
  - yaml
:yaml:
  :datadir: /home/gstatkevicius/dev/puppet_testing/hieradata
:hierarchy:
  - test
:logger: console
$ cat hieradata/test.yaml 
test::_a: "Hello World!"

It looks like the hiera part works:

hiera -c ./hiera.yaml -d test::_a
DEBUG: 2020-11-03 23:39:00 +0200: Hiera YAML backend starting
DEBUG: 2020-11-03 23:39:00 +0200: Looking up test::_a in YAML backend
DEBUG: 2020-11-03 23:39:00 +0200: Looking for data source test
DEBUG: 2020-11-03 23:39:00 +0200: Found test::_a in test
Hello World!

Now let’s see if applying the manifest with Puppet works:

$ puppet apply --hiera_config=hiera.yaml manifest/hellofile.pp
Notice: Applied catalog in 0.01 seconds
$ cat /tmp/helloworld
Hello World!

Oh, so now everything is OK? It seems that for APL – automatic parameter look-up – the rules are a bit different. My guess at this point would be that they are treated as regular variables instead. I personally haven’t found a way to instantiate a class where one parameter starts with an underscore. Thus, I think we can formulate one lesson:

To prevent your class from ever being instantiated by other classes in Puppet with explicit arguments, start at least one class parameter with an underscore.

But, one question remains – why is it actually considered a syntax error? What makes the underscore character forbidden in names of class parameters whereas it works for regular variables?

Now, I’m not an expert in how Puppet parsing but let’s take a short trip down Puppet’s code-base and see what’s happening.

A quick grep of Syntax error at shows that there is some kind of function SYNTAX_ERROR that gets used whenever there is a need to print a message that a syntax error has occurred.

Digging a bit deeper, it seems that there is some kind of parser being generated from a grammar. Other Puppet developers have kindly documented this process for us in docs/parser_work.md. We are finally able to find the grammar in lib/puppet/pops/parser/egrammar.ra.

Valid name of a variable seems to be expressed here in lexer2.rb:

PATTERN_DOLLAR_VAR     = %r{\$(::)?(\w+::)*\w+}

I believe that what is a valid argument passed to a class is defined here:

#---ATTRIBUTE OPERATIONS (Not an expression)
#
attribute_operations
  :                                                { result = [] }
  | attribute_operation                            { result = [val[0]] }
  | attribute_operations COMMA attribute_operation { result = val[0].push(val[2]) }

  # Produces String
  # QUESTION: Why is BOOLEAN valid as an attribute name?
  #
  attribute_name
    : NAME
    | keyword

A valid NAME is defined here in lib/puppet/pops/patterns.rb:

NAME = %r{\A((::)?[a-z]\w*)(::[a-z]\w*)*\z}

So, it seems like the argument’s name is rejected because it does not follow this regular expression even though it is accepted by the lexer. To be fair, Puppet’s documentation also states:

CAUTION: In some cases, names containing unsupported characters might still work. Such cases are bugs and could cease to work at any time. Removal of these bug cases is not limited to major releases.

All in all, it’s probably best to follow the letter of the law laid out in Puppet’s documentation as it says here but if you want to forbid the users of your class to pass arguments explicitly, start one of them with an underscore 🙃.

How to solve puppetdb and SIGKILL problems

In this post, I wanted to share some knowledge that I have gained recently. Sometimes your Puppet server might start returning 500 HTTP status codes. You should check puppetserver‘s logs first of all, of course, but another thing to check is the puppetdb. puppetdb is the component which is responsible for storing Puppet-related data. In this case, there were memory-related problems on the puppetdb instance. The puppetdb logs in this case have looked like this:

Apr 02 15:25:19 hostname systemd[1]: Started puppetdb Service.
Apr 04 18:54:07 hostname systemd[1]: puppetdb.service: main process exited, code=killed, status=9/KILL
Apr 04 18:54:07 hostname systemd[1]: Unit puppetdb.service entered failed state.
Apr 04 18:54:07 hostname systemd[1]: puppetdb.service failed.
Apr 04 18:54:08 hostname systemd[1]: puppetdb.service holdoff time over, scheduling restart.
Apr 04 18:54:08 hostname systemd[1]: Stopped puppetdb Service.
Apr 04 18:54:08 hostname systemd[1]: Starting puppetdb Service...

The regular logs in /var/log/puppetlabs/puppetdb/puppetdb.log also did not show any particular problems on why it could have been killed by that signal. So, what could have been the issue?

It turns out that the Java’s virtual machine supports some interesting parameters. One of them is -XX:OnOutOfMemoryError. With that, you can specify what kind of command should be executed once the JVM runs out of memory. In this case, it was set to -XX:OnOutOfMemoryError=kill -9 %p. It means that SIGKILL (9) is sent to the JVM process if it runs out of memory.

A quick GitHub search shows that it is a pretty prevalent thing to do. However, the problem with it is that it is relatively trivial to do a denial-of-service attack because there is no graceful load shedding. It only takes sending a bunch of requests to it that would allocate some memory. That is it if there is no queuing mechanism in place. Also, it provides no way of knowing where all of the memory was allocated when it runs into such a situation. Plus, the state could get so bad that it might be impossible to spawn a new process that would send that signal. So, it should be avoided. But, hey, at least systemd conveniently shows that the process has been turned off with SIGKILL (9) in this scenario.

Obviously, when this happens then you should adjust the -Xmx and -Xms parameters on your JVM process to permit it to allocate more memory. Of course, it is only possible if there is enough memory available.

Looking into the past it seems that this has appeared in puppetdb with this commit. As far as I can tell, this has been added because old JVM versions (pre-2016) have not supported any other way of dealing with this situation besides adding a huge try/catch block & dealing with all of the possible situations or just crashing out. Plus, the exception could even appear in a separate thread that is not under our control so it requires a lot of extra effort for not much gain.

But, since 2016 new, better ways of dealing with this situation have been introduced. Consider using +XX:ExitOnOutOfMemoryError or +XX:CrashOnOutOfMemoryError as per these release notes if your JVM is new enough. They avoid some of the problems mentioned earlier such as being unable to start another process. It is worth mentioning that other users of that flag such as prestodb are slowly moving towards the new ones with commits such as this.

In general, it is probably best to enable options such as -XX:+HeapDumpOnOutOfMemoryError or -XX:HeapDumpPath if you have enough spare space on your servers where JVM processes are running.

This has also reminded me of a great article on LWN about crash-only software. Perhaps it is really easier to just crash and quickly restart if something goes wrong? But at the very least the software should print some diagnostic messages before destroying itself to help its users understand what is going on in situations like these.