From 75d3c9128bb5d6d4f214e8225bd77f9709526b5b Mon Sep 17 00:00:00 2001 From: Chris Denneen Date: Tue, 10 Jan 2017 15:55:27 -0500 Subject: [PATCH] Updates for strict variables. Removes top scope overrides as this would break strict variable checking --- .puppet-lint.rc | 1 + manifests/params.pp | 250 +++++--------------------- spec/classes/snmp_client_spec.rb | 14 +- spec/classes/snmp_init_spec.rb | 25 +-- spec/defines/snmp_snmpv3_user_spec.rb | 14 +- 5 files changed, 77 insertions(+), 227 deletions(-) diff --git a/.puppet-lint.rc b/.puppet-lint.rc index c50f9c70..10c0294a 100644 --- a/.puppet-lint.rc +++ b/.puppet-lint.rc @@ -1,6 +1,7 @@ --fail-on-warnings --relative --no-80chars-check +--no-140chars-check --no-class_inherits_from_params_class-check --no-documentation-check --no-single_quote_string_with_variables-check diff --git a/manifests/params.pp b/manifests/params.pp index 73702b5c..d2f66033 100644 --- a/manifests/params.pp +++ b/manifests/params.pp @@ -1,9 +1,6 @@ # == Class: snmp::params # -# This class handles OS-specific configuration of the snmp module. It -# looks for variables in top scope (probably from an ENC such as Dashboard). If -# the variable doesn't exist in top scope, it falls back to a hard coded default -# value. +# This class handles OS-specific configuration of the snmp module. # # === Authors: # @@ -14,263 +11,107 @@ # Copyright (C) 2012 Mike Arnold, unless otherwise noted. # class snmp::params { - # If we have a top scope variable defined, use it, otherwise fall back to a - # hardcoded value. - $agentaddress = $::snmp_agentaddress ? { - undef => [ 'udp:127.0.0.1:161', 'udp6:[::1]:161' ], - default => $::snmp_agentaddress, - } - - $snmptrapdaddr = $::snmp_snmptrapdaddr ? { - undef => [ 'udp:127.0.0.1:162', 'udp6:[::1]:162' ], - default => $::snmp_snmptrapdaddr, - } - - $ro_community = $::snmp_ro_community ? { - undef => 'public', - default => $::snmp_ro_community, - } - - $ro_community6 = $::snmp_ro_community6 ? { - undef => 'public', - default => $::snmp_ro_community6, - } - - $rw_community = $::snmp_rw_community ? { - undef => undef, - default => $::snmp_rw_community, - } - $rw_community6 = $::snmp_rw_community6 ? { - undef => undef, - default => $::snmp_rw_community6, - } - - $ro_network = $::snmp_ro_network ? { - undef => '127.0.0.1', - default => $::snmp_ro_network, - } - - $ro_network6 = $::snmp_ro_network6 ? { - undef => '::1', - default => $::snmp_ro_network6, - } - - $rw_network = $::snmp_rw_network ? { - undef => '127.0.0.1', - default => $::snmp_rw_network, - } - - $rw_network6 = $::snmp_rw_network6 ? { - undef => '::1', - default => $::snmp_rw_network6, - } - - $contact = $::snmp_contact ? { - undef => 'Unknown', - default => $::snmp_contact, - } - - $location = $::snmp_location ? { - undef => 'Unknown', - default => $::snmp_location, - } - - $sysname = $::snmp_sysname ? { - undef => $::fqdn, - default => $::snmp_sysname, - } - - $com2sec = $::snmp_com2sec ? { - undef => [ - "notConfigUser default public", - ], - default => $::snmp_com2sec, - } - - $com2sec6 = $::snmp_com2sec6 ? { - undef => [ - "notConfigUser default public", - ], - default => $::snmp_com2sec6, - } - - $groups = $::snmp_groups ? { - undef => [ + $agentaddress = [ 'udp:127.0.0.1:161', 'udp6:[::1]:161' ] + $snmptrapdaddr = [ 'udp:127.0.0.1:162', 'udp6:[::1]:162' ] + $ro_community = 'public' + $ro_community6 = 'public' + $rw_community = undef + $rw_community6 = undef + $ro_network = '127.0.0.1' + $ro_network6 = '::1' + $rw_network = '127.0.0.1' + $rw_network6 = '::1' + $contact = 'Unknown' + $location = 'Unknown' + $sysname = $::fqdn + $com2sec = [ 'notConfigUser default public' ] + $com2sec6 = [ 'notConfigUser default public' ] + $groups = [ 'notConfigGroup v1 notConfigUser', 'notConfigGroup v2c notConfigUser', - ], - default => $::snmp_groups, - } - - $services = $::snmp_services ? { - undef => 72, - default => $::snmp_services, - } - - $openmanage_enable = $::openmanage_enable ? { - undef => false, - default => $::openmanage_enable - } - - $views = $::snmp_views ? { - undef => [ + ] + $services = 72 + $openmanage_enable = false + $views = [ 'systemview included .1.3.6.1.2.1.1', 'systemview included .1.3.6.1.2.1.25.1.1', - ], - default => $::snmp_views, - } - - $accesses = $::snmp_accesses ? { - undef => [ + ] + $accesses = [ 'notConfigGroup "" any noauth exact systemview none none', - ], - default => $::snmp_accesses, - } - - $dlmod = $::snmp_dlmod ? { - undef => [], - default => $::snmp_dlmod, - } - - $disable_authorization = $::snmp_disable_authorization ? { - undef => 'no', - default => $::snmp_disable_authorization, - } - - $do_not_log_traps = $::snmp_do_not_log_traps ? { - undef => 'no', - default => $::snmp_do_not_log_traps, - } - - $do_not_log_tcpwrappers = $::snmp_do_not_log_tcpwrappers ? { - undef => 'no', - default => $::snmp_do_not_log_tcpwrappers, - } - - $trap_handlers = $::snmp_trap_handlers ? { - undef => [], - default => $::snmp_trap_handlers, - } - - $trap_forwards = $::snmp_trap_forwards ? { - undef => [], - default => $::snmp_trap_forwards, - } - - $snmp_config = $::snmp_snmp_config ? { - undef => [], - default => $::snmp_snmp_config, - } - - $snmpd_config = $::snmp_snmpd_config ? { - undef => [], - default => $::snmp_snmpd_config, - } - - $snmptrapd_config = $::snmp_snmptrapd_config ? { - undef => [], - default => $::snmp_snmptrapd_config, - } + ] + $dlmod = [] + $disable_authorization = 'no' + $do_not_log_traps = 'no' + $do_not_log_tcpwrappers = 'no' + $trap_handlers = [] + $trap_forwards = [] + $snmp_config = [] + $snmpd_config = [] + $snmptrapd_config = [] ### The following parameters should not need to be changed. - $ensure = $::snmp_ensure ? { - undef => 'present', - default => $::snmp_ensure, - } + $ensure = 'present' - $service_ensure = $::snmp_service_ensure ? { - undef => 'running', - default => $::snmp_service_ensure, - } + $service_ensure = 'running' - $trap_service_ensure = $::snmp_trap_service_ensure ? { - undef => 'stopped', - default => $::snmp_trap_service_ensure, - } + $trap_service_ensure = 'stopped' # Since the top scope variable could be a string (if from an ENC), we might # need to convert it to a boolean. - $autoupgrade = $::snmp_autoupgrade ? { - undef => false, - default => $::snmp_autoupgrade, - } + $autoupgrade = false if is_string($autoupgrade) { $safe_autoupgrade = str2bool($autoupgrade) } else { $safe_autoupgrade = $autoupgrade } - $install_client = $::snmp_install_client ? { - undef => undef, - default => $::snmp_install_client, - } + $install_client = undef - $manage_client = $::snmp_manage_client ? { - undef => false, - default => $::snmp_manage_client, - } + $manage_client = false if is_string($manage_client) { $safe_manage_client = str2bool($manage_client) } else { $safe_manage_client = $manage_client } - $service_enable = $::snmp_service_enable ? { - undef => true, - default => $::snmp_service_enable, - } + $service_enable = true if is_string($service_enable) { $safe_service_enable = str2bool($service_enable) } else { $safe_service_enable = $service_enable } - $service_hasstatus = $::snmp_service_hasstatus ? { - undef => true, - default => $::snmp_service_hasstatus, - } + $service_hasstatus = true if is_string($service_hasstatus) { $safe_service_hasstatus = str2bool($service_hasstatus) } else { $safe_service_hasstatus = $service_hasstatus } - $service_hasrestart = $::snmp_service_hasrestart ? { - undef => true, - default => $::snmp_service_hasrestart, - } + $service_hasrestart = true if is_string($service_hasrestart) { $safe_service_hasrestart = str2bool($service_hasrestart) } else { $safe_service_hasrestart = $service_hasrestart } - $trap_service_enable = $::snmp_trap_service_enable ? { - undef => false, - default => $::snmp_trap_service_enable, - } + $trap_service_enable = false if is_string($trap_service_enable) { $safe_trap_service_enable = str2bool($trap_service_enable) } else { $safe_trap_service_enable = $trap_service_enable } - $trap_service_hasstatus = $::snmp_trap_service_hasstatus ? { - undef => true, - default => $::snmp_trap_service_hasstatus, - } + $trap_service_hasstatus = true if is_string($trap_service_hasstatus) { $safe_trap_service_hasstatus = str2bool($trap_service_hasstatus) } else { $safe_trap_service_hasstatus = $trap_service_hasstatus } - $trap_service_hasrestart = $::snmp_trap_service_hasrestart ? { - undef => true, - default => $::snmp_trap_service_hasrestart, - } + $trap_service_hasrestart = true if is_string($trap_service_hasrestart) { $safe_trap_service_hasrestart = str2bool($trap_service_hasrestart) } else { @@ -351,6 +192,7 @@ $client_config = '/etc/snmp/snmp.conf' $trap_service_config = '/etc/snmp/snmptrapd.conf' + $trap_service_name = 'snmptrapd' $snmptrapd_options = '-Lsd -p /var/run/snmptrapd.pid' } 'Suse': { diff --git a/spec/classes/snmp_client_spec.rb b/spec/classes/snmp_client_spec.rb index b62719e0..813dfa8d 100644 --- a/spec/classes/snmp_client_spec.rb +++ b/spec/classes/snmp_client_spec.rb @@ -30,9 +30,10 @@ describe "for osfamily RedHat, operatingsystem #{os}" do let(:params) {{}} let :facts do { - :osfamily => 'RedHat', - :operatingsystem => os, - :operatingsystemrelease => '6.4' + :osfamily => 'RedHat', + :operatingsystem => os, + :operatingsystemmajrelease => '6', + :operatingsystemrelease => '6.4' } end it { should contain_package('snmp-client').with( @@ -122,9 +123,10 @@ context 'on a supported osfamily, custom parameters' do let :facts do { - :osfamily => 'RedHat', - :operatingsystem => 'RedHat', - :operatingsystemrelease => '6.4' + :osfamily => 'RedHat', + :operatingsystem => 'RedHat', + :operatingsystemmajrelease => '6', + :operatingsystemrelease => '6.4' } end diff --git a/spec/classes/snmp_init_spec.rb b/spec/classes/snmp_init_spec.rb index a8c68ffd..0569c47b 100644 --- a/spec/classes/snmp_init_spec.rb +++ b/spec/classes/snmp_init_spec.rb @@ -29,10 +29,11 @@ describe "for osfamily RedHat, operatingsystem RedHat, operatingsystemrelease 5.9" do let(:params) {{}} let :facts do { - :osfamily => 'RedHat', - :operatingsystem => 'RedHat', - :operatingsystemrelease => '5.9', - :fqdn => 'myhost.localdomain' + :osfamily => 'RedHat', + :operatingsystem => 'RedHat', + :operatingsystemmajrelease => '5', + :operatingsystemrelease => '5.9', + :fqdn => 'myhost.localdomain' } end it { should contain_package('snmpd').with( @@ -144,10 +145,11 @@ describe "for osfamily RedHat, operatingsystem RedHat, operatingsystemrelease 6.4" do let(:params) {{}} let :facts do { - :osfamily => 'RedHat', - :operatingsystem => 'RedHat', - :operatingsystemrelease => '6.4', - :fqdn => 'myhost.localdomain' + :osfamily => 'RedHat', + :operatingsystem => 'RedHat', + :operatingsystemmajrelease => '6', + :operatingsystemrelease => '6.4', + :fqdn => 'myhost.localdomain' } end it { should contain_package('snmpd').with( @@ -565,9 +567,10 @@ context 'on a supported osfamily (RedHat), custom parameters' do let :facts do { - :osfamily => 'RedHat', - :operatingsystem => 'RedHat', - :operatingsystemrelease => '6.4' + :osfamily => 'RedHat', + :operatingsystem => 'RedHat', + :operatingsystemmajrelease => '6', + :operatingsystemrelease => '6.4' } end diff --git a/spec/defines/snmp_snmpv3_user_spec.rb b/spec/defines/snmp_snmpv3_user_spec.rb index 844068b7..5da470d0 100644 --- a/spec/defines/snmp_snmpv3_user_spec.rb +++ b/spec/defines/snmp_snmpv3_user_spec.rb @@ -6,9 +6,10 @@ context 'on a supported osfamily' do let :facts do { - :osfamily => 'RedHat', - :operatingsystem => 'CentOS', - :operatingsystemrelease => '6.4' + :osfamily => 'RedHat', + :operatingsystem => 'CentOS', + :operatingsystemmajrelease => '6', + :operatingsystemrelease => '6.4' } end @@ -63,9 +64,10 @@ context 'on a supported osfamily, RedHat' do let :facts do { - :osfamily => 'RedHat', - :operatingsystem => 'CentOS', - :operatingsystemrelease => '6.4' + :osfamily => 'RedHat', + :operatingsystem => 'CentOS', + :operatingsystemmajrelease => '6', + :operatingsystemrelease => '6.4' } end