From d03585760777897e1948ee637e18668d138f41db Mon Sep 17 00:00:00 2001 From: "tlimoncelli@stackoverflow.com" Date: Fri, 20 Nov 2015 19:52:17 +0000 Subject: [PATCH] Validate global_options and defaults_options. * validate_hash() global_options and defaults_options * Add spec tests to verify that non-hash values produce errors * Add spec test to verify functionality of non-merge behavior --- manifests/init.pp | 1 + manifests/instance.pp | 5 +- spec/classes/haproxy_spec.rb | 215 +++++++++++++++++++++++++++++++++++ 3 files changed, 219 insertions(+), 2 deletions(-) diff --git a/manifests/init.pp b/manifests/init.pp index 80697ae3..857f415b 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -123,6 +123,7 @@ validate_bool($service_manage) validate_bool($merge_options) validate_string($service_options) + validate_hash($global_options, $defaults_options) # NOTE: These deprecating parameters are implemented in this class, # not in haproxy::instance. haproxy::instance is new and therefore diff --git a/manifests/instance.pp b/manifests/instance.pp index 469ca091..ddc4c944 100644 --- a/manifests/instance.pp +++ b/manifests/instance.pp @@ -160,8 +160,9 @@ # Therefore, we "include haproxy::params" for any parameters we need. include haproxy::params - $_global_options = pick($global_options, $haproxy::params::global_options, []) - $_defaults_options = pick($defaults_options, $haproxy::params::defaults_options, []) + $_global_options = pick($global_options, $haproxy::params::global_options) + $_defaults_options = pick($defaults_options, $haproxy::params::defaults_options) + validate_hash($_global_options,$_defaults_options) # Determine instance_name based on: # single-instance hosts: haproxy diff --git a/spec/classes/haproxy_spec.rb b/spec/classes/haproxy_spec.rb index d68e61e7..83fe5b5f 100644 --- a/spec/classes/haproxy_spec.rb +++ b/spec/classes/haproxy_spec.rb @@ -463,6 +463,221 @@ end end end + + describe 'when overriding global and defaults options with user-supplied overrides and additions' do + # For testing the merging functionality we restrict ourselves to + # Debian OS family so that we don't have to juggle different sets of + # global_options and defaults_options (like for FreeBSD). + ['Debian' ].each do |osfamily| + context "on #{osfamily} family operatingsystems" do + let(:facts) do + { :osfamily => osfamily }.merge default_facts + end + let(:contents) { param_value(catalogue, 'concat::fragment', 'haproxy-haproxy-base', 'content').split("\n") } + let(:params) do + { + 'merge_options' => false, + 'global_options' => { + 'log-send-hostname' => '', + 'chroot' => '/srv/haproxy-chroot', + 'stats' => [ + 'socket /var/lib/haproxy/admin.sock mode 660 level admin', + 'timeout 30s' + ] + }, + 'defaults_options' => { + 'mode' => 'http', + 'option' => [ + 'abortonclose', + 'logasap', + 'dontlognull', + 'httplog', + 'http-server-close', + 'forwardfor except 127.0.0.1', + ], + 'timeout' => [ + 'connect 5s', + 'client 1m', + 'server 1m', + 'check 7s', + ] + }, + } + end + it 'should manage a custom chroot directory' do + subject.should contain_file('/srv/haproxy-chroot').with( + 'ensure' => 'directory', + ) + end + it 'should contain global and defaults sections' do + contents.should include('global') + contents.should include('defaults') + end + it 'should send hostname with log in global options' do + contents.should include(' log-send-hostname ') + end + it 'should enable admin stats and stats timeout in global options' do + contents.should include(' stats socket /var/lib/haproxy/admin.sock mode 660 level admin') + contents.should include(' stats timeout 30s') + end + it 'should set mode http in default options' do + contents.should include(' mode http') + end + it 'should not set the global parameter "maxconn"' do + contents.should_not include(' maxconn 4000') + end + it 'should set various options in defaults, removing the "redispatch" option' do + contents.should_not include(' option redispatch') + contents.should include(' option abortonclose') + contents.should include(' option logasap') + contents.should include(' option dontlognull') + contents.should include(' option httplog') + contents.should include(' option http-server-close') + contents.should include(' option forwardfor except 127.0.0.1') + end + it 'should set timeouts in defaults, removing the "http-request 10s" and "queue 1m" timeout' do + contents.should_not include(' timeout http-request 10s') + contents.should_not include(' timeout queue 1m') + contents.should include(' timeout connect 5s') + contents.should include(' timeout check 7s') + contents.should include(' timeout client 1m') + contents.should include(' timeout server 1m') + end + end + end + end + + describe 'blah blah blah' do + # For testing the merging functionality we restrict ourselves to + # Debian OS family so that we don't have to juggle different sets of + # global_options and defaults_options (like for FreeBSD). + ['Debian' ].each do |osfamily| + context "on #{osfamily} family operatingsystems" do + let(:facts) do + { :osfamily => osfamily }.merge default_facts + end + let(:contents) { param_value(catalogue, 'concat::fragment', 'haproxy-haproxy-base', 'content').split("\n") } + let(:params) do + { + 'merge_options' => false, + 'global_options' => { + 'log-send-hostname' => '', + 'chroot' => '/srv/haproxy-chroot', + 'stats' => [ + 'socket /var/lib/haproxy/admin.sock mode 660 level admin', + 'timeout 30s' + ] + }, + 'defaults_options' => { + 'mode' => 'http', + 'option' => [ + 'abortonclose', + 'logasap', + 'dontlognull', + 'httplog', + 'http-server-close', + 'forwardfor except 127.0.0.1', + ], + 'timeout' => [ + 'connect 5s', + 'client 1m', + 'server 1m', + 'check 7s', + ] + }, + } + end + it 'should manage a custom chroot directory' do + subject.should contain_file('/srv/haproxy-chroot').with( + 'ensure' => 'directory', + ) + end + it 'should contain global and defaults sections' do + contents.should include('global') + contents.should include('defaults') + end + it 'should send hostname with log in global options' do + contents.should include(' log-send-hostname ') + end + it 'should enable admin stats and stats timeout in global options' do + contents.should include(' stats socket /var/lib/haproxy/admin.sock mode 660 level admin') + contents.should include(' stats timeout 30s') + end + it 'should set mode http in default options' do + contents.should include(' mode http') + end + it 'should not set the global parameter "maxconn"' do + contents.should_not include(' maxconn 4000') + end + it 'should set various options in defaults, removing the "redispatch" option' do + contents.should_not include(' option redispatch') + contents.should include(' option abortonclose') + contents.should include(' option logasap') + contents.should include(' option dontlognull') + contents.should include(' option httplog') + contents.should include(' option http-server-close') + contents.should include(' option forwardfor except 127.0.0.1') + end + it 'should set timeouts in defaults, removing the "http-request 10s" and "queue 1m" timeout' do + contents.should_not include(' timeout http-request 10s') + contents.should_not include(' timeout queue 1m') + contents.should include(' timeout connect 5s') + contents.should include(' timeout check 7s') + contents.should include(' timeout client 1m') + contents.should include(' timeout server 1m') + end + end + end + end + +# describe 'when specifying global_options with arrays instead of hashes' do +# # For testing input validation we restrict ourselves to +# # Debian OS family so that we don't have to juggle different sets of +# # global_options and defaults_options (like for FreeBSD). +# ['Debian' ].each do |osfamily| +# context "on #{osfamily} family operatingsystems" do +# let(:facts) do +# { :osfamily => osfamily }.merge default_facts +# end +# let(:contents) { param_value(catalogue, 'concat::fragment', 'haproxy-haproxy-base', 'content').split("\n") } +# let(:params) do +# { +# 'merge_options' => true, +# 'global_options' => [ 'log-send-hostname', 'chroot /srv/haproxy-chroot' ] +# } +# end +# it 'should raise error' do +# expect { catalogue }.to raise_error Puppet::Error, /is not a Hash/ +# end +# end +# end +# end + describe 'when specifying defaults_options with arrays instead of hashes' do + # For testing input validation we restrict ourselves to + # Debian OS family so that we don't have to juggle different sets of + # global_options and defaults_options (like for FreeBSD). + ['Debian' ].each do |osfamily| + context "on #{osfamily} family operatingsystems" do + let(:facts) do + { :osfamily => osfamily }.merge default_facts + end + let(:contents) { param_value(catalogue, 'concat::fragment', 'haproxy-haproxy-base', 'content').split("\n") } + let(:params) do + { + 'merge_options' => true, + 'defaults_options' => [ + 'mode http', + 'timeout connect 5s', + 'timeout client 1m' + ] + } + end + it 'should raise error' do + expect { catalogue }.to raise_error Puppet::Error, /is not a Hash/ + end + end + end + end end context 'on unsupported operatingsystems' do