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

Validate global_options and defaults_options. #207

Merged
merged 1 commit into from
Dec 7, 2015
Merged

Validate global_options and defaults_options. #207

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions manifests/init.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions manifests/instance.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
132 changes: 132 additions & 0 deletions spec/classes/haproxy_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,138 @@
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|
Copy link

Choose a reason for hiding this comment

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

i'm confused why this is a loop

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for raising that. I'm copying the style of this kind of test above on line 373 (https://github.com/puppetlabs/puppetlabs-haproxy/pull/207/files#diff-7be58596f7d58684b9342fe66e3aeee3L373)

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
Expand Down