Use ::osfamily instead of ::operatingsystem.

Add module dependency of puppetlabs/stdlib and razorsedge/lsb.
Refactor rspec tests.
Convert standard template parameters (like autoupdate) to be top
scope-aware.
Validate our booleans

.fixtures.yml

@@ -1,3 +1,8 @@
 fixtures:
+  repositories:
+    stdlib:
+      repo: "git://github.com/puppetlabs/puppetlabs-stdlib.git"
+      ref: "2.6.0"
+    lsb: "git://github.com/razorsedge/puppet-lsb.git"
   symlinks:
     snmp: "#{source_dir}"

.project

@@ -3,6 +3,8 @@
 	<name>snmp</name>
 	<comment></comment>
 	<projects>
+		<project>lsb</project>
+		<project>stdlib</project>
 	</projects>
 	<buildSpec>
 		<buildCommand>

Modulefile

@@ -7,6 +7,8 @@ project_page '/~https://github.com/razorsedge/puppet-snmp'
 source 'git://github.com/razorsedge/puppet-snmp.git'
 summary 'Install and manage SNMP services.'
 description 'This module manages the installation of the SNMP server, SNMP client, and SNMP trap server.  It also can create a SNMPv3 user with authentication and privacy passwords.'
+dependency 'puppetlabs/stdlib', '>=2.3.0'
+dependency 'razorsedge/lsb', '>=1.0.0'
 
 # Generate the changelog file
 system("git-log-to-changelog > CHANGELOG")

manifests/init.pp

@@ -33,9 +33,11 @@
 # Copyright (C) 2012 Mike Arnold, unless otherwise noted.
 #
 class snmp (
-  $ensure      = 'present',
-  $autoupgrade = false
+  $ensure      = $snmp::params::ensure,
+  $autoupgrade = $snmp::params::safe_autoupgrade
 ) inherits snmp::params {
+  # Validate our booleans
+  validate_bool($autoupgrade)
 
   case $ensure {
     /(present)/: {
@@ -79,7 +81,6 @@
     require => Package['snmpd'],
   }
 
-  #TODO var-net-snmp ensure => 'directory'
   file { 'var-net-snmp':
     ensure  => 'directory',
     mode    => $snmp::params::varnetsnmp_perms,

manifests/params.pp

@@ -1,6 +1,9 @@
 # == Class: snmp::params
 #
-# This class handles OS-specific configuration of the snmp module.
+# 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.
 #
 # === Authors:
 #
@@ -11,6 +14,9 @@
 # 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.
+# TODO
   $ro_community = 'public'
   $rw_community = 'private'
   $ro_network   = '127.0.0.1'
@@ -25,18 +31,50 @@
     'access  notConfigGroup ""      any       noauth    exact  systemview none none',
   ]
 
+### The following parameters should not need to be changed.
 
-# These should not need to be changed.
-  case $::operatingsystem {
-    'RedHat', 'CentOS', 'Scientific', 'SLC', 'OracleLinux', 'OEL': {
+  $ensure = $::snmp_ensure ? {
+    undef => 'present',
+    default => $::snmp_ensure,
+  }
+
+  $service_ensure = $::snmp_service_ensure ? {
+    undef => 'running',
+    default => $::snmp_service_ensure,
+  }
+
+  # 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,
+  }
+  if is_string($autoupgrade) {
+    $safe_autoupgrade = str2bool($autoupgrade)
+  } else {
+    $safe_autoupgrade = $autoupgrade
+  }
+
+  $service_enable = $::snmp_service_enable ? {
+    undef => true,
+    default => $::snmp_service_enable,
+  }
+  if is_string($service_enable) {
+    $safe_service_enable = str2bool($service_enable)
+  } else {
+    $safe_service_enable = $service_enable
+  }
+
+  case $::osfamily {
+    'RedHat': {
       #TODO: Use $::lsbmajdistrelease or $majdistrelease?
       #$majdistrelease = regsubst($::operatingsystemrelease,'^(\d+)\.(\d+)','\1')
 
       $package_name        = 'net-snmp'
       $service_config      = '/etc/snmp/snmpd.conf'
       $service_config_perms= '0644'
       $service_name        = 'snmpd'
-      if $::lsbmajdistrelease <= '5' {
+      if ($::lsbmajdistrelease <= '5') and ($::operatingsystem != 'Fedora') {
         $sysconfig         = '/etc/sysconfig/snmpd.options'
         $var_net_snmp      = '/var/net-snmp'
         $varnetsnmp_perms  = '0700'
@@ -53,32 +91,12 @@
 
       $trap_service_config = '/etc/snmp/snmptrapd.conf'
       $trap_service_name   = 'snmptrapd'
-      if $::lsbmajdistrelease <= '5' {
+      if ($::lsbmajdistrelease <= '5') and ($::operatingsystem != 'Fedora') {
         $trap_sysconfig    = '/etc/sysconfig/snmptrapd.options'
       } else {
         $trap_sysconfig    = '/etc/sysconfig/snmptrapd'
       }
     }
-    'Fedora': {
-      fail("Module snmp is not yet supported on ${::operatingsystem}")
-    }
-    'Ubuntu': {
-      $package_name        = 'snmpd'
-      $service_config      = '/etc/snmp/snmpd.conf'
-      $service_config_perms= '0600'
-      $service_name        = 'snmpd'
-      $sysconfig           = '/etc/default/snmp'
-      $var_net_snmp        = '/var/lib/snmp'
-      $varnetsnmp_perms    = '0700'
-      $varnetsnmp_owner    = 'snmp'
-      $varnetsnmp_group    = 'snmp'
-
-      $client_package_name = 'snmp'
-      $client_config       = '/etc/snmp/snmp.conf'
-
-      $trap_service_config = '/etc/snmp/snmptrapd.conf'
-      $trap_service_name   = 'snmptrapd'
-    }
     'Debian': {
       $package_name        = 'snmpd'
       $service_config      = '/etc/snmp/snmpd.conf'
@@ -97,7 +115,7 @@
       $trap_service_name   = 'snmptrapd'
     }
     default: {
-      fail("Module snmp is not supported on ${::operatingsystem}")
+      fail("Module ${::module} is not supported on ${::operatingsystem}")
     }
   }
 }

manifests/server.pp

@@ -95,15 +95,21 @@
   $location           = $snmp::params::location,
   $views              = $snmp::params::views,
   $accesses           = $snmp::params::accesses,
-  $ensure             = 'present',
-  $autoupgrade        = false,
+  $ensure             = $snmp::params::ensure,
+  $autoupgrade        = $snmp::params::safe_autoupgrade,
   $package_name       = $snmp::params::package_name,
   $service_ensure     = 'running',
   $service_name       = $snmp::params::service_name,
   $service_enable     = true,
   $service_hasstatus  = true,
   $service_hasrestart = true
 ) inherits snmp::params {
+  # Validate our booleans
+  validate_bool($autoupgrade)
+  validate_bool($service_enable)
+  validate_bool($service_hasstatus)
+  validate_bool($service_hasrestart)
+
   include snmp
 
   case $ensure {

manifests/trapd.pp

@@ -64,7 +64,7 @@
   $ro_community       = $snmp::params::ro_community,
   $rw_community       = $snmp::params::rw_community,
   $trap_handlers      = [],
-  $ensure             = 'present',
+  $ensure             = $snmp::params::ensure,
   $service_ensure     = 'running',
   $service_name       = $snmp::params::trap_service_name,
   $service_enable     = true,

spec/classes/snmp_init_spec.rb

@@ -0,0 +1,122 @@
+#!/usr/bin/env rspec
+
+require 'spec_helper'
+
+describe 'snmp', :type => 'class' do
+
+  context 'on a non-supported osfamily' do
+    let(:params) {{}}
+    let :facts do {
+      :osfamily        => 'foo',
+      :operatingsystem => 'bar'
+    }
+    end
+    it 'should fail' do
+      expect {
+        should raise_error(Puppet::Error, /Module snmp is not supported on bar/)
+      }
+    end
+  end
+
+  redhatish = ['RedHat', 'Fedora']
+  debianish = ['Debian', 'Ubuntu']
+
+  context 'on a supported osfamily, default parameters' do
+    redhatish.each do |os|
+      describe "for osfamily RedHat, operatingsystem #{os}" do
+        let(:params) {{}}
+        let :facts do {
+          :osfamily          => 'RedHat',
+          :operatingsystem   => os,
+          :lsbmajdistrelease => '6'
+        }
+        end
+        it { should contain_package('snmpd').with(
+          :ensure => 'present',
+          :name   => 'net-snmp'
+        )}
+        it { should contain_package('snmp-client').with(
+          :ensure => 'present',
+          :name   => 'net-snmp-utils'
+        )}
+        it { should contain_file('snmp.conf').with(
+          :ensure  => 'present',
+          :mode    => '0644',
+          :owner   => 'root',
+          :group   => 'root',
+          :path    => '/etc/snmp/snmp.conf',
+          :require => 'Package[snmpd]'
+        )}
+        it { should contain_file('var-net-snmp').with(
+          :ensure  => 'directory',
+          :mode    => '0755',
+          :owner   => 'root',
+          :group   => 'root',
+          :path    => '/var/lib/net-snmp',
+          :require => 'Package[snmpd]'
+        )}
+      end
+    end
+
+    debianish.each do |os|
+      describe "for osfamily Debian, operatingsystem #{os}" do
+        let(:params) {{}}
+        let :facts do {
+          :osfamily        => 'Debian',
+          :operatingsystem => os
+        }
+        end
+        it { should contain_package('snmpd').with(
+          :ensure => 'present',
+          :name   => 'snmpd'
+        )}
+        it { should contain_package('snmp-client').with(
+          :ensure => 'present',
+          :name   => 'snmp'
+        )}
+        it { should contain_file('snmp.conf').with(
+          :ensure  => 'present',
+          :mode    => '0644',
+          :owner   => 'root',
+          :group   => 'root',
+          :path    => '/etc/snmp/snmp.conf',
+          :require => 'Package[snmpd]'
+        )}
+        it { should contain_file('var-net-snmp').with(
+          :ensure  => 'directory',
+          :mode    => '0755',
+          :owner   => 'snmp',
+          :group   => 'snmp',
+          :path    => '/var/lib/snmp',
+          :require => 'Package[snmpd]'
+        )}
+      end
+    end
+  end
+
+  context 'on a supported osfamily, custom parameters' do
+    let :facts do {
+      :osfamily          => 'RedHat',
+      :operatingsystem   => 'RedHat',
+      :lsbmajdistrelease => '6'
+    }
+    end
+
+    describe 'ensure => absent' do
+      let(:params) {{ :ensure => 'absent' }}
+      it { should contain_package('snmpd').with_ensure('absent') }
+      it { should contain_package('snmp-client').with_ensure('absent') }
+      it { should contain_file('snmp.conf').with_ensure('absent') }
+      it { should contain_file('var-net-snmp').with_ensure('directory') }
+    end
+
+    describe 'autoupgrade => true' do
+      let(:params) {{ :autoupgrade => true }}
+      it { should contain_package('snmpd').with_ensure('latest') }
+      it { should contain_package('snmp-client').with_ensure('latest') }
+      it { should contain_file('snmp.conf').with_ensure('present') }
+      it { should contain_file('var-net-snmp').with_ensure('directory') }
+    end
+  end
+
+end