Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

improve ordering of options #224

Merged
merged 2 commits into from
Mar 10, 2016
Merged

improve ordering of options #224

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b3202dd
improve ordering of options
rvicinus Feb 20, 2016
f47f4b7
Merge branch 'master' of https://github.com/puppetlabs/puppetlabs-hap…
rvicinus Mar 8, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)).
Expand Down Expand Up @@ -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)).
Expand Down Expand Up @@ -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`

Expand Down
17 changes: 12 additions & 5 deletions manifests/backend.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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")
Expand All @@ -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)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it impossible to put the sort_options_alphabetic parameter in the base haproxy class and follow this pattern as usual, instead of having a haproxy::globals class? We only use globals when it is otherwise impossible to use the base class.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is impossible because you can use the haproxy module without using the haproxy class, only using haproxy::instance resources.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right, because the base class declares a default instance.


# Template uses: $section_name, $ipaddress, $ports, $options
concat::fragment { "${instance_name}-${section_name}_backend_block":
Expand Down
25 changes: 16 additions & 9 deletions manifests/frontend.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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')
Expand All @@ -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":
Expand Down
15 changes: 15 additions & 0 deletions manifests/globals.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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)
}
7 changes: 7 additions & 0 deletions manifests/listen.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -89,6 +93,7 @@
},
$instance = 'haproxy',
$section_name = $name,
$sort_options_alphabetic = undef,
# Deprecated
$bind_options = '',
) {
Expand Down Expand Up @@ -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":
Expand Down
27 changes: 27 additions & 0 deletions spec/acceptance/basic_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
57 changes: 57 additions & 0 deletions spec/defines/listen_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
39 changes: 37 additions & 2 deletions templates/fragments/_options.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 -%>
Expand All @@ -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 -%>
Expand Down