Merge pull request #224 from vicinus/master

improve ordering of options
puppetlabs · Mar 10, 2016 · d66a33b · d66a33b
2 parents 22a608f + f47f4b7
commit d66a33b
Show file tree

Hide file tree

Showing 8 changed files with 185 additions and 16 deletions.
diff --git a/README.md b/README.md
@@ -436,6 +436,7 @@ haproxy::frontend { 'ft_allapps':
 ####Public classes
 
 * [`haproxy`](#class-haproxy): Main configuration class.
+* [`haproxy::globals`](#class-haproxy-globals): Global configuration options.
 
 ####Private classes
 
@@ -557,6 +558,14 @@ Main class, includes all other classes.
 
 * `config_dir`: Path to the directory in which the main configuration file `haproxy.cfg` resides. Will also be used for storing any managed map files (see [`haproxy::mapfile`](#define-haproxymapfile). Default depends on platform.
 
+#### Class: `haproxy::globals`
+
+For global configuration options used by all haproxy instances.
+
+##### Parameters (all optional)
+
+* `sort_options_alphabetic`: Sort options either alphabetic or custom like haproxy internal sorts them. Defaults to true.
+
 #### Define: `haproxy::balancermember`
 
 Configures a service inside a listening or backend service configuration block in haproxy.cfg.
@@ -603,6 +612,8 @@ Sets up a backend service configuration block inside haproxy.cfg. Each backend s
 
 * `instance`: *Optional.* When using `haproxy::instance` to run multiple instances of Haproxy on the same machine, this indicates which instance.  Defaults to "haproxy".
 
+* `sort_options_alphabetic`: Sort options either alphabetic or custom like haproxy internal sorts them. Defaults to `haproxy::globals::sort_options_alphabetic`.
+
 #### Define: `haproxy::frontend`
 
 Sets up a frontend service configuration block inside haproxy.cfg. Each frontend service needs one or more balancermember services (declared with the [`haproxy::balancermember` define](#define-haproxybalancermember)).
@@ -645,6 +656,8 @@ For more information, see the [HAProxy Configuration Manual](http://cbonte.githu
 
 * `instance`: *Optional.* When using `haproxy::instance` to run multiple instances of Haproxy on the same machine, this indicates which instance.  Defaults to "haproxy".
 
+* `sort_options_alphabetic`: Sort options either alphabetic or custom like haproxy internal sorts them. Defaults to `haproxy::globals::sort_options_alphabetic`.
+
 #### Define: `haproxy::listen`
 
 Sets up a listening service configuration block inside haproxy.cfg. Each listening service configuration needs one or more balancermember services (declared with the [`haproxy::balancermember` define](#define-haproxybalancermember)).
@@ -679,6 +692,7 @@ For more information, see the [HAProxy Configuration Manual](http://cbonte.githu
 
 * `ports`: *Required unless `bind` is specified.* Specifies which ports to listen on for the address specified in `ipaddress`. Valid options: a single comma-delimited string or an array of strings. Each string can contain a port number or a hyphenated range of port numbers (e.g., 8443-8450).
 
+* `sort_options_alphabetic`: Sort options either alphabetic or custom like haproxy internal sorts them. Defaults to `haproxy::globals::sort_options_alphabetic`.
 
 #### Define: `haproxy::userlist`
 

diff --git a/manifests/backend.pp b/manifests/backend.pp
@@ -34,6 +34,10 @@
 #    haproxy::balancermember with array arguments, which allows you to deploy
 #    everything in 1 run)
 #
+# [*sort_options_alphabetic*]
+#   Sort options either alphabetic or custom like haproxy internal sorts them.
+#   Defaults to true.
+#
 # === Examples
 #
 #  Exporting the resource for a backend member:
@@ -54,16 +58,17 @@
 # Jeremy Kitchen <jeremy@nationbuilder.com>
 #
 define haproxy::backend (
-  $mode             = undef,
-  $collect_exported = true,
-  $options          = {
+  $mode                    = undef,
+  $collect_exported        = true,
+  $options                 = {
     'option'  => [
       'tcplog',
     ],
     'balance' => 'roundrobin'
   },
-  $instance         = 'haproxy',
-  $section_name     = $name,
+  $instance                = 'haproxy',
+  $section_name            = $name,
+  $sort_options_alphabetic = undef,
 ) {
   if defined(Haproxy::Listen[$section_name]) {
     fail("An haproxy::listen resource was discovered with the same name (${section_name}) which is not supported")
@@ -77,6 +82,8 @@
     $instance_name = "haproxy-${instance}"
     $config_file = inline_template($haproxy::params::config_file_tmpl)
   }
+  include haproxy::globals
+  $_sort_options_alphabetic = pick($sort_options_alphabetic, $haproxy::globals::sort_options_alphabetic)
 
   # Template uses: $section_name, $ipaddress, $ports, $options
   concat::fragment { "${instance_name}-${section_name}_backend_block":

diff --git a/manifests/frontend.pp b/manifests/frontend.pp
@@ -42,6 +42,10 @@
 #   A hash of options that are inserted into the frontend service
 #    configuration block.
 #
+# [*sort_options_alphabetic*]
+#   Sort options either alphabetic or custom like haproxy internal sorts them.
+#   Defaults to true.
+#
 # === Examples
 #
 #  Exporting the resource for a balancer member:
@@ -66,20 +70,21 @@
 # Gary Larizza <gary@puppetlabs.com>
 #
 define haproxy::frontend (
-  $ports            = undef,
-  $ipaddress        = undef,
-  $bind             = undef,
-  $mode             = undef,
-  $collect_exported = true,
-  $options          = {
+  $ports                   = undef,
+  $ipaddress               = undef,
+  $bind                    = undef,
+  $mode                    = undef,
+  $collect_exported        = true,
+  $options                 = {
     'option'  => [
       'tcplog',
     ],
   },
-  $instance         = 'haproxy',
-  $section_name     = $name,
+  $instance                = 'haproxy',
+  $section_name            = $name,
+  $sort_options_alphabetic = undef,
   # Deprecated
-  $bind_options     = undef,
+  $bind_options            = undef,
 ) {
   if $ports and $bind {
     fail('The use of $ports and $bind is mutually exclusive, please choose either one')
@@ -102,6 +107,8 @@
     $instance_name = "haproxy-${instance}"
     $config_file = inline_template($haproxy::params::config_file_tmpl)
   }
+  include haproxy::globals
+  $_sort_options_alphabetic = pick($sort_options_alphabetic, $haproxy::globals::sort_options_alphabetic)
 
   # Template uses: $section_name, $ipaddress, $ports, $options
   concat::fragment { "${instance_name}-${section_name}_frontend_block":

diff --git a/manifests/globals.pp b/manifests/globals.pp
@@ -0,0 +1,15 @@
+# == Class: haproxy::globals
+#
+# For global configuration options used by all haproxy instances.
+#
+# === Parameters
+#
+# [*sort_options_alphabetic*]
+#   Sort options either alphabetic or custom like haproxy internal sorts them.
+#   Defaults to true.
+#
+class haproxy::globals (
+  $sort_options_alphabetic = true,
+) {
+  validate_bool($sort_options_alphabetic)
+}
diff --git a/manifests/listen.pp b/manifests/listen.pp
@@ -54,6 +54,10 @@
 #    know the full set of balancermembers in advance and use haproxy::balancermember
 #    with array arguments, which allows you to deploy everything in 1 run)
 #
+# [*sort_options_alphabetic*]
+#   Sort options either alphabetic or custom like haproxy internal sorts them.
+#   Defaults to true.
+#
 # === Examples
 #
 #  Exporting the resource for a balancer member:
@@ -89,6 +93,7 @@
   },
   $instance                     = 'haproxy',
   $section_name                 = $name,
+  $sort_options_alphabetic      = undef,
   # Deprecated
   $bind_options                 = '',
 ) {
@@ -117,6 +122,8 @@
     $instance_name = "haproxy-${instance}"
     $config_file = inline_template($haproxy::params::config_file_tmpl)
   }
+  include haproxy::globals
+  $_sort_options_alphabetic = pick($sort_options_alphabetic, $haproxy::globals::sort_options_alphabetic)
 
   # Template uses: $section_name, $ipaddress, $ports, $options
   concat::fragment { "${instance_name}-${section_name}_listen_block":

diff --git a/spec/acceptance/basic_spec.rb b/spec/acceptance/basic_spec.rb
@@ -57,6 +57,33 @@ class { 'haproxy': }
         end
       end
     end
+
+    describe "with sort_options_alphabetic false" do
+      it 'should start' do
+        pp = <<-EOS
+        class { 'haproxy::params':
+          sort_options_alphabetic => false,
+        }
+        class { 'haproxy': }
+        haproxy::listen { 'stats':
+          ipaddress => '127.0.0.1',
+          ports     => ['9090','9091'],
+          mode      => 'http',
+          options   => { 'stats' => ['uri /','auth puppet:puppet'], },
+        }
+        EOS
+        apply_manifest(pp, :catch_failures => true)
+      end
+
+      it 'should have stats listening on each port' do
+        ['9090','9091'].each do |port|
+          shell("/usr/bin/curl -u puppet:puppet localhost:#{port}") do |r|
+            r.stdout.should =~ /HAProxy/
+            r.exit_code.should == 0
+          end
+        end
+      end
+    end
   end
 
   # C9934

diff --git a/spec/defines/listen_spec.rb b/spec/defines/listen_spec.rb
@@ -290,4 +290,61 @@
     ) }
   end
 
+  context "when listen options are specified with sort_options_alphabetic" do
+    let(:params) do
+      {
+        :name => 'apache',
+        :bind => {
+          '0.0.0.0:48001-48003' => [],
+        },
+        :mode => 'http',
+        :options => {
+          'reqadd'                 => 'X-Forwarded-Proto:\ https',
+          'reqidel'                => '^X-Forwarded-For:.*',
+          'default_backend'        => 'dev00_webapp',
+          'capture request header' => [ 'X-Forwarded-For len 50', 'Host len 15', 'Referrer len 15' ],
+          'acl'                    => [ 'dst_dev01 dst_port 48001', 'dst_dev02 dst_port 48002', 'dst_dev03 dst_port 48003' ],
+          'use_backend'            => [ 'dev01_webapp if dst_dev01', 'dev02_webapp if dst_dev02', 'dev03_webapp if dst_dev03' ],
+          'option'                 => [ 'httplog', 'http-server-close', 'forwardfor except 127.0.0.1' ],
+          'compression'            => 'algo gzip',
+          'bind-process'           => 'all',
+        },
+      }
+    end
+    it { should contain_concat__fragment('haproxy-apache_listen_block').with(
+      'order'   => '20-apache-00',
+      'target'  => '/etc/haproxy/haproxy.cfg',
+      'content' => "\nlisten apache\n  bind 0.0.0.0:48001-48003 \n  mode http\n  acl dst_dev01 dst_port 48001\n  acl dst_dev02 dst_port 48002\n  acl dst_dev03 dst_port 48003\n  bind-process all\n  capture request header X-Forwarded-For len 50\n  capture request header Host len 15\n  capture request header Referrer len 15\n  compression algo gzip\n  default_backend dev00_webapp\n  option httplog\n  option http-server-close\n  option forwardfor except 127.0.0.1\n  reqadd X-Forwarded-Proto:\\ https\n  reqidel ^X-Forwarded-For:.*\n  use_backend dev01_webapp if dst_dev01\n  use_backend dev02_webapp if dst_dev02\n  use_backend dev03_webapp if dst_dev03\n"
+    ) }
+  end
+
+  context "when listen options are specified without sort_options_alphabetic" do
+    let(:params) do
+      {
+        :name => 'apache',
+        :bind => {
+          '0.0.0.0:48001-48003' => [],
+        },
+        :mode => 'http',
+        :sort_options_alphabetic => false,
+        :options => {
+          'reqadd'                 => 'X-Forwarded-Proto:\ https',
+          'reqidel'                => '^X-Forwarded-For:.*',
+          'default_backend'        => 'dev00_webapp',
+          'capture request header' => [ 'X-Forwarded-For len 50', 'Host len 15', 'Referrer len 15' ],
+          'acl'                    => [ 'dst_dev01 dst_port 48001', 'dst_dev02 dst_port 48002', 'dst_dev03 dst_port 48003' ],
+          'use_backend'            => [ 'dev01_webapp if dst_dev01', 'dev02_webapp if dst_dev02', 'dev03_webapp if dst_dev03' ],
+          'option'                 => [ 'httplog', 'http-server-close', 'forwardfor except 127.0.0.1' ],
+          'compression'            => 'algo gzip',
+          'bind-process'           => 'all',
+        },
+      }
+    end
+    it { should contain_concat__fragment('haproxy-apache_listen_block').with(
+      'order'   => '20-apache-00',
+      'target'  => '/etc/haproxy/haproxy.cfg',
+      'content' => "\nlisten apache\n  bind 0.0.0.0:48001-48003 \n  mode http\n  acl dst_dev01 dst_port 48001\n  acl dst_dev02 dst_port 48002\n  acl dst_dev03 dst_port 48003\n  bind-process all\n  capture request header X-Forwarded-For len 50\n  capture request header Host len 15\n  capture request header Referrer len 15\n  compression algo gzip\n  default_backend dev00_webapp\n  option httplog\n  option http-server-close\n  option forwardfor except 127.0.0.1\n  reqidel ^X-Forwarded-For:.*\n  reqadd X-Forwarded-Proto:\\ https\n  use_backend dev01_webapp if dst_dev01\n  use_backend dev02_webapp if dst_dev02\n  use_backend dev03_webapp if dst_dev03\n"
+    ) }
+  end
+
 end
diff --git a/templates/fragments/_options.erb b/templates/fragments/_options.erb
@@ -1,5 +1,40 @@
+<%-
+  # non mentioned option have a value of 0
+  # lower values come first
+  if @_sort_options_alphabetic == true
+    option_order = {}
+  else
+    option_order = {
+      'acl' => -1,
+      'tcp-request' => 2,
+      'block' => 3,
+      'http-request' => 4,
+      'reqallow' => 5,
+      'reqdel' => 5,
+      'reqdeny' => 5,
+      'reqidel' => 5,
+      'reqideny' => 5,
+      'reqipass' => 5,
+      'reqirep' => 5,
+      'reqitarpit' => 5,
+      'reqpass' => 5,
+      'reqrep' => 5,
+      'reqtarpit' => 5,
+      'reqadd' => 6,
+      'redirect' => 7,
+      'use_backend' => 8,
+      'use-server' => 9,
+      'server' => 100,
+    }
+  end
+-%>
 <% if @options.is_a?(Hash) -%>
-<% @options.sort.each do |key, val| -%>
+<%# ruby sorts arrays like strings: the first elements are compared, if they -%>
+<%# are equal the next elements are compared and so on. The following sort -%>
+<%# provides a two element array, the first element a classification of the -%>
+<%# option with a default value of 0 if the option is not found in the -%>
+<%# option_order hash and the second element the option itself. -%>
+<% @options.sort{|a,b| [option_order.fetch(a[0],0), a[0]] <=> [option_order.fetch(b[0],0), b[0]] }.each do |key, val| -%>
 <% Array(val).each do |item| -%>
   <%= key %> <%= item %>
 <% end -%>
@@ -10,7 +45,7 @@
 <%# sorted by key name before outputting the key name (= option name) and its -%>
 <%# value (or values, one per line) -%>
 <% @options.each do |option| -%>
-<% option.sort.map do |key, val| -%>
+<% option.sort{|a,b| [option_order.fetch(a[0],0), a[0]] <=> [option_order.fetch(b[0],0), b[0]] }.map do |key, val| -%>
 <% Array(val).each do |item| -%>
   <%= key %> <%= item %>
 <% end -%>