update resolver parameters (#526)

* resolver: fix default accepted_payload_size

This fixes the error:
Error: Could not retrieve catalog from remote server: Error 500 on SERVER: Server Error: Evaluation Error: Error while evaluating a Resource Statement, Evaluation Error: Comparison of: Undef Value < Integer, is not possible. Caused by 'Only Strings, Numbers, Timespans, Timestamps, and Versions are comparable'. (file: /etc/puppetlabs/code/modules/haproxy/manifests/resolver.pp, line: 125, column: 30)

HAProxy allows this parameter to be unset, and the code here works with
this ok, aside from the bounds check. Replace the bounds check with a
parameter type declaration, so that puppet itself will do the check for
us.

Update tests accordingly.

* resolver: support parse-resolv-conf parameter

The parse-resolv-conf parameter can be used on its own or in conjunction
with the nameservers parameter.

Adjust the nameservers parameter default to allow the template to work;
an empty hash results in no nameservers specified.

Add a new test.

* resolver: visually re-align parameter assignments

Author: bugfood

manifests/resolver.pp

@@ -15,6 +15,15 @@
 # @param nameservers
 #   Set of id, ip addresses and port options.
 #   $nameservers = { 'dns1' => '10.0.0.1:53', 'dns2' => '10.0.0.2:53' }
+#   Either the 'nameservers' or the 'parse_resolv_conf' parameter must be
+#   specified in order for the resolver to work.
+#   Default: none specified.
+#
+# @param parse_resolv_conf
+#   If true, parse resolv.conf to retrieve an ordered set of nameservers.
+#   This can be used instead of (or in addition to) the 'nameservers'
+#   parameter.
+#   Default: false
 #
 # @param hold
 #   Defines <period> during which the last name resolution should be kept
@@ -87,17 +96,19 @@
 # Ricardo Rosales <[email protected]>
 #
 define haproxy::resolver (
-  $nameservers             = undef,
-  $hold                    = undef,
-  $resolve_retries         = undef,
-  $timeout                 = undef,
-  $accepted_payload_size   = undef,
-  $instance                = 'haproxy',
-  $section_name            = $name,
-  $sort_options_alphabetic = undef,
-  $collect_exported        = true,
-  $config_file             = undef,
-  $defaults                = undef,
+  Hash $nameservers       = {},
+  Boolean $parse_resolv_conf                          = false,
+  $hold                                               = undef,
+  $resolve_retries                                    = undef,
+  $timeout                                            = undef,
+  # https://cbonte.github.io/haproxy-dconv/1.8/configuration.html#5.3.2-accepted_payload_size
+  Optional[Integer[512, 8192]] $accepted_payload_size = undef,
+  $instance                                           = 'haproxy',
+  $section_name                                       = $name,
+  $sort_options_alphabetic                            = undef,
+  $collect_exported                                   = true,
+  $config_file                                        = undef,
+  $defaults                                           = undef,
 ) {
   include haproxy::params
 
@@ -120,12 +131,6 @@
     $order = "25-${defaults}-${section_name}-02"
   }
 
-  # verify accepted_payload_size is withing the allowed range per HAProxy docs
-  # https://cbonte.github.io/haproxy-dconv/1.8/configuration.html#5.3.2-accepted_payload_size
-  if ($accepted_payload_size < 512) or ($accepted_payload_size > 8192) {
-    fail('$accepted_payload_size must be atleast 512 and not more than 8192')
-  }
-
   # Template uses: $section_name
   concat::fragment { "${instance_name}-${section_name}_resolver_block":
     order   => $order,

spec/defines/resolver_spec.rb

@@ -13,6 +13,46 @@
   end
 
   context 'with two resolvers' do
+    let(:title) { 'bar' }
+    let(:params) do
+      {
+        nameservers: { 'dns1' => '1.1.1.1:53', 'dns2' => '1.1.1.2:53' },
+        hold: { 'other' => '30s', 'refused' => '30s', 'nx' => '30s', 'timeout' => '30s', 'valid' => '10s' },
+        resolve_retries: 3,
+        timeout: { 'retry' => '1s' },
+      }
+    end
+
+    it {
+      is_expected.to contain_concat__fragment('haproxy-bar_resolver_block').with(
+        'order'   => '20-bar-01',
+        'target'  => '/etc/haproxy/haproxy.cfg',
+        'content' => "\nresolvers bar\n  nameserver dns1 1.1.1.1:53\n  nameserver dns2 1.1.1.2:53\n  resolve_retries 3\n  timeout retry 1s\n  hold other 30s\n  hold refused 30s\n  hold nx 30s\n  hold timeout 30s\n  hold valid 10s\n", # rubocop:disable Layout/LineLength
+      )
+    }
+  end
+
+  context 'with parse_resolv_conf' do
+    let(:title) { 'bar' }
+    let(:params) do
+      {
+        parse_resolv_conf: true,
+        hold: { 'other' => '30s', 'refused' => '30s', 'nx' => '30s', 'timeout' => '30s', 'valid' => '10s' },
+        resolve_retries: 3,
+        timeout: { 'retry' => '1s' },
+      }
+    end
+
+    it {
+      is_expected.to contain_concat__fragment('haproxy-bar_resolver_block').with(
+        'order'   => '20-bar-01',
+        'target'  => '/etc/haproxy/haproxy.cfg',
+        'content' => "\nresolvers bar\n  parse-resolv-conf\n  resolve_retries 3\n  timeout retry 1s\n  hold other 30s\n  hold refused 30s\n  hold nx 30s\n  hold timeout 30s\n  hold valid 10s\n",
+      )
+    }
+  end
+
+  context 'with accepted_payload_size within range' do
     let(:title) { 'bar' }
     let(:params) do
       {
@@ -45,7 +85,7 @@
       }
     end
 
-    it { is_expected.to compile.and_raise_error(%r{accepted_payload_size must be atleast 512 and not more than 8192}) }
+    it { is_expected.to compile.and_raise_error(%r{expects a value of type Undef or Integer\[512, 8192\]}) }
   end
 
   context 'with accepted_payload_size too large' do
@@ -60,6 +100,6 @@
       }
     end
 
-    it { is_expected.to compile.and_raise_error(%r{accepted_payload_size must be atleast 512 and not more than 8192}) }
+    it { is_expected.to compile.and_raise_error(%r{expects a value of type Undef or Integer\[512, 8192\]}) }
   end
 end

templates/haproxy_resolver_block.erb

@@ -3,6 +3,9 @@ resolvers <%= @section_name %>
 <% @nameservers.each do |id, nameserver| -%>
   nameserver <%= id %> <%= nameserver %>
 <% end -%>
+<% if @parse_resolv_conf -%>
+  parse-resolv-conf
+<% end -%>
 <% if @resolve_retries -%>
   resolve_retries <%= @resolve_retries %>
 <% end -%>