Issue 122 - Puppet ENC via Hiera

hiera/puppet-node-classifier.rb

+#!/usr/bin/env ruby
+# This script is a node classifier for Puppet that operates on top of Hiera
+# and uses a custom hosts.yaml config to map host roles.
+
+require 'getoptlong'
+require 'hiera'
+require 'socket'
+require 'yaml'
+
+# Where to search for the Hiera configuration
+HIERA_CONFIG = ENV.fetch('PUPPET_HIERA_CONFIG', '/etc/puppet/hiera.yaml')
+# Where to search for the Hosts configuration
+HOSTS_CONFIG = ENV.fetch('PUPPET_HOSTS_CONFIG', '/etc/puppet/infrastructure/hiera/private/hosts.yaml')
+
+# The current script name, used for logging and usage hints

[Felix Dahlke, 2015/03/09 23:14:51]

That seems a bit too obvious for a comment IMO. Comments are useful when they
add non-obvious information.

Obvious comments that just repeat what's clearly in the code however are more
likely to cause harm in my experience. For example, if (not when) they get out
of sync with the code.

[mathias, 2015/03/16 12:07:11]

Done.

+BASENAME = File.basename($0)
+
+# Although there's no need for any options in particular yet, the script

[Felix Dahlke, 2015/03/09 23:14:51]

IMO also too obvious. It's a no-brainer that --help makes sense, and it's also a
no-brainer to look up the docs for GetoptLong if one wants to add options.

[mathias, 2015/03/16 12:07:11]

Done.

+# still recognizes a --help option, which is considered the bare minimum by
+# convention. Nonetheless, future options may become integrated here (see
+# also http://ruby-doc.org/stdlib/libdoc/getoptlong/rdoc/GetoptLong.html):
+GetoptLong.new(
+  ['--help', '-h', GetoptLong::NO_ARGUMENT]
+).each do |opt, arg|
+  case opt
+
+    when '--help'
+      puts <<-END
+Usage:  #{BASENAME} [hostname]
+        #{BASENAME} example.com
+        #{BASENAME} --help
+
+Options:
+
+    --help, -h
+        Display this help message and exit gracefully.
+
+Environment:
+
+    PUPPET_HIERA_CONFIG=#{HIERA_CONFIG}
+        Where to find the hiera configuration file.
+    PUPPET_HOSTS_CONFIG=#{HOSTS_CONFIG}
+        Where to find the hosts configuration file.
+
+END
+      exit 0
+
+  end
+end
+
+# Only one additional non-option argument is allowed, in order to explicitly

[Felix Dahlke, 2015/03/09 23:14:51]

Also pretty obvious from the code IMO.

[mathias, 2015/03/16 12:07:11]

Although I've improved some of the comments you've flagged here without further
comment, let me point something out: I'm worrying that we do a pretty good job
at criticizing and finding reasons to avoid comments, yet we do not really do a
good job when it comes to documenting our code. "Too obvious" is a rather
individual term ("obvious for whom?"), not a good set of criteria. Most of the
time it results in "obvious for you" but a big pile of question marks for anyone
who's not familiar with the entire system anyway. Like this very repository.

The more objective practice is to avoid "repeating the code in the comments".
And this snippet here is a very good example: It actually explains the intention
behind the code and thus allows you to argue whether it does what it should do -
or not. This is, IMHO, the bare minimum.

+# specify a hostname to use instead of the default:
+case ARGV.length
+  when 0
+    hostname = Socket.gethostname
+  when 1
+    hostname = ARGV[0][/^[^.]+/]
+  else
+    STDERR.puts <<-END
+#{BASENAME}: unknown option: #{ARGV[0]}
+#{BASENAME}: try #{BASENAME} --help
+END
+    exit 1
+end
+
+# Extract the server -> hostname -> role information from the hosts
+# configuration file:
+begin
+  config = YAML.load_file(HOSTS_CONFIG)
+  servers = config.fetch("servers", {})
+  host = servers.fetch(hostname, {})
+  role = host.fetch("role", "default")
+rescue Exception => error
+  STDERR.puts "#{BASENAME}: #{error.message}: #{HOSTS_CONFIG}"
+  exit 1
+end
+
+# Map Hiera data into the structure Puppet expects an ENC to generate (see
+# https://docs.puppetlabs.com/guides/external_nodes.html for more info):
+begin
+  hiera = Hiera.new(:config => HIERA_CONFIG)
+  scope = {'::hostname' => hostname, '::role' => role}
+  classes = hiera.lookup('classes', {}, scope, nil, :hash)
+  parameters = hiera.lookup('parameters', {}, scope, nil, :hash)
+  parameters['role'] = role
+  result = { 'classes' => classes, 'parameters' => parameters }
+rescue Exception => error
+  STDERR.puts "#{BASENAME}: #{error.message}: #{HIERA_CONFIG}"
+  exit 1
+end
+
+puts result.to_yaml
+