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.

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

Clean up fixtures and codebase hardening #48

Merged
merged 22 commits into from
Feb 15, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
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
10 changes: 2 additions & 8 deletions .fixtures.yml
Original file line number Diff line number Diff line change
@@ -1,10 +1,4 @@
fixtures:
repositories:
'stdlib':
repo: 'https://github.com/puppetlabs/puppetlabs-stdlib.git'
ref: '4.6.0'
'firewall':
repo: 'https://github.com/puppetlabs/puppetlabs-firewall.git'
ref: '1.7.1'
symlinks:
'monit': "#{source_dir}"
'stdlib': 'https://github.com/puppetlabs/puppetlabs-stdlib.git'
'firewall': 'https://github.com/puppetlabs/puppetlabs-firewall.git'
4 changes: 2 additions & 2 deletions REFERENCE.md
Original file line number Diff line number Diff line change
Expand Up @@ -87,15 +87,15 @@ Default value: `$monit::params::check_interval`

##### <a name="-monit--config_file"></a>`config_file`

Data type: `String`
Data type: `Stdlib::Absolutepath`

Specifies a path to the main config file. Default value: varies with operating system

Default value: `$monit::params::config_file`

##### <a name="-monit--config_dir"></a>`config_dir`

Data type: `String`
Data type: `Stdlib::Absolutepath`

Specifies a path to the config directory. Default value: varies with operating system

Expand Down
6 changes: 3 additions & 3 deletions manifests/firewall.pp
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@
if $monit::httpd_bool and $monit::manage_firewall_bool {
if defined('::firewall') {
firewall { "${monit::httpd_port} allow Monit inbound traffic":
action => 'accept',
dport => $monit::httpd_port,
proto => 'tcp',
jump => 'accept',
jay7x marked this conversation as resolved.
Show resolved Hide resolved
dport => $monit::httpd_port,
proto => 'tcp',
}
}
}
Expand Down
27 changes: 10 additions & 17 deletions manifests/init.pp
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,8 @@
class monit (
Array[String] $alert_emails = $monit::params::alert_emails,
Integer[1] $check_interval = $monit::params::check_interval,
String $config_file = $monit::params::config_file,
String $config_dir = $monit::params::config_dir,
Stdlib::Absolutepath $config_file = $monit::params::config_file,
Stdlib::Absolutepath $config_dir = $monit::params::config_dir,
Variant[Boolean, Enum['true', 'false']] $config_dir_purge = $monit::params::config_dir_purge,
Variant[Boolean, Enum['true', 'false']] $httpd = $monit::params::httpd,
Integer[1, 65535] $httpd_port = $monit::params::httpd_port,
Expand All @@ -132,58 +132,51 @@
String $service_name = $monit::params::service_name,
Optional[Integer[1]] $start_delay = $monit::params::start_delay,
) inherits monit::params {
# <stringified variable handling>
if is_string($httpd) == true {
if $httpd =~ String {
$httpd_bool = str2bool($httpd)
} else {
$httpd_bool = $httpd
}
zilchms marked this conversation as resolved.
Show resolved Hide resolved

if is_string($manage_firewall) == true {
if $manage_firewall =~ String {
$manage_firewall_bool = str2bool($manage_firewall)
} else {
$manage_firewall_bool = $manage_firewall
}

if is_string($service_enable) == true {
if $service_enable =~ String {
$service_enable_bool = str2bool($service_enable)
} else {
$service_enable_bool = $service_enable
}

if is_string($service_manage) == true {
if $service_manage =~ String {
$service_manage_bool = str2bool($service_manage)
} else {
$service_manage_bool = $service_manage
}

if is_string($mmonit_https) == true {
if $mmonit_https =~ String {
$mmonit_https_bool = str2bool($mmonit_https)
} else {
$mmonit_https_bool = $mmonit_https
}

if is_string($mmonit_without_credential) == true {
if $mmonit_without_credential =~ String {
$mmonit_without_credential_bool = str2bool($mmonit_without_credential)
} else {
$mmonit_without_credential_bool = $mmonit_without_credential
}

if is_string($config_dir_purge) == true {
if $config_dir_purge =~ String {
$config_dir_purge_bool = str2bool($config_dir_purge)
} else {
$config_dir_purge_bool = $config_dir_purge
}
# </stringified variable handling>

# <variable validations>
validate_absolute_path($config_file)
validate_absolute_path($config_dir)

if $logfile and !($logfile =~ /^syslog(\s+facility\s+log_(local[0-7]|daemon))?/) {
validate_absolute_path($logfile)
assert_type(Stdlib::Absolutepath, $logfile)
}
# </variable validations>

# Use the monit_version fact if available, else use the default for the
# platform.
Expand Down
155 changes: 3 additions & 152 deletions spec/classes/init_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,9 @@
end

it do
is_expected.to contain_firewall('2812 allow Monit inbound traffic').with('action' => 'accept',
'dport' => '2812',
'proto' => 'tcp')
is_expected.to contain_firewall('2812 allow Monit inbound traffic').with('jump' => 'accept',
'dport' => '2812',
'proto' => 'tcp')
end
end

Expand Down Expand Up @@ -316,153 +316,4 @@
end
end
end

describe 'failures' do
let(:facts) do
{
osfamily: 'Debian',
lsbdistcodename: 'squeeze',
monit_version: '5',
}
end

[-1, 65_536].each do |value|
context "when httpd_port is set to invalid value <#{value}>" do
let(:params) do
{
httpd: true,
httpd_port: value,
httpd_address: 'otherhost',
httpd_user: 'tester',
httpd_password: 'Passw0rd',
}
end

it 'fails' do
is_expected.to compile.and_raise_error(%r{expects an Integer\[1, 65535\] value})
end
end
end

context 'when check_interval is set to invalid value <0>' do
let(:params) { { check_interval: 0 } }

it 'fails' do
is_expected.to compile.and_raise_error(%r{expects an Integer\[1})
end
end

context 'when start_delay is set to invalid value <0>' do
let(:params) { { start_delay: 0 } }

it 'fails' do
is_expected.to compile.and_raise_error(%r{expects a value of type Undef or Integer\[1})
end
end
end

describe 'variable type and content validations' do
# set needed custom facts and variables
let(:facts) do
{
osfamily: 'Debian',
operatingsystemrelease: '6.0',
operatingsystemmajrelease: '6',
lsbdistcodename: 'squeeze',
monit_version: '5',
}
end
let(:validation_params) do
{
# :param => 'value',
}
end

validations = {
'absolute_path' => {
name: %w[config_file config_dir],
valid: ['/absolute/filepath', '/absolute/directory/'],
invalid: ['invalid', 3, 2.42, ['array'], { 'ha' => 'sh' }],
message: '(expects a String value|is not an absolute path)',
},
'array' => {
name: ['alert_emails'],
valid: [%w[valid array]],
invalid: ['string', { 'ha' => 'sh' }, 3, 2.42, true],
message: 'expects an Array value',
},
'bool_stringified' => {
name: %w[httpd manage_firewall service_enable service_manage mmonit_https mmonit_without_credential config_dir_purge],
valid: [true, 'true', false, 'false'],
invalid: ['invalid', 3, 2.42, ['array'], { 'ha' => 'sh' }, nil],
message: 'expects a value of type Boolean or Enum',
},
'integer' => {
name: %w[check_interval httpd_port mmonit_port],
valid: [242],
invalid: [2.42, 'invalid', ['array'], { 'ha' => 'sh ' }, true, false, nil],
message: 'expects an Integer',
},
'optional_integer' => {
name: ['start_delay'],
valid: [242],
invalid: [2.42, 'invalid', ['array'], { 'ha' => 'sh ' }, true, false, nil],
message: 'expects a value of type Undef or Integer',
},
'optional_logfile' => {
name: ['logfile'],
valid: ['/absolute/filepath', '/absolute/directory/', 'syslog', 'syslog facility log_local0'],
invalid: ['invalid', 3, 2.42, ['array'], { 'ha' => 'sh' }],
message: '(expects a value of type Undef or String|is not an absolute path)',
},
'optional_hash' => {
name: ['mailformat'],
valid: [{ 'ha' => 'sh' }],
invalid: ['string', 3, 2.42, ['array'], true, false, nil],
message: 'expects a value of type Undef or Hash',
},
'optional_string' => {
name: %w[mailserver mmonit_address],
valid: ['present'],
invalid: [['array'], { 'ha' => 'sh' }],
message: 'expects a value of type Undef or String',
},
'string' => {
name: %w[httpd_address httpd_allow httpd_user httpd_password
package_ensure package_name service_name mmonit_user
mmonit_password],
valid: ['present'],
invalid: [['array'], { 'ha' => 'sh' }],
message: 'expects a String value',
},
'service_ensure_string' => {
name: ['service_ensure'],
valid: ['running'],
invalid: [['array'], { 'ha' => 'sh' }],
message: 'expects a match for Enum\[\'running\', \'stopped\'\]',
},
}

validations.sort.each do |type, var|
var[:name].each do |var_name|
var[:valid].each do |valid|
context "with #{var_name} (#{type}) set to valid #{valid} (as #{valid.class})" do
let(:params) { validation_params.merge("#{var_name}": valid) }

it { is_expected.to compile }
end
end

var[:invalid].each do |invalid|
context "with #{var_name} (#{type}) set to invalid #{invalid} (as #{invalid.class})" do
let(:params) { validation_params.merge("#{var_name}": invalid) }

it 'fails' do
is_expected.to compile.and_raise_error(%r{#{var[:message]}})
end
end
end
end # var[:name].each
end # validations.sort.each
end # describe 'variable type and content validations'
end