Simpify traditional access control parameters

Instead of allowing `undef`, plain strings or arrays of strings, just allow arrays of strings. Parameters that previously defaulted to `undef` now default to empty arrays. This allows us to simplify the templates and we no longer have `Optional` parameters that have default values set. (`ro_community` defaulting to `'public'` but allowing `undef` didn't make much sense previously.) View-based Access Control Model (VACM) is still recommended, but traditional access control is no longer deprecated.
voxpupuli · Feb 24, 2019 · ab5c009 · ab5c009
1 parent 5f9273d
commit ab5c009
Show file tree

Hide file tree

Showing 8 changed files with 82 additions and 88 deletions.
diff --git a/README.md b/README.md
@@ -57,23 +57,7 @@ include snmp
 
 ### Upgrading
 
-#### Deprecation Warning
-
-##### Past module 3.x series
-
- * The classes `snmp::server` and `snmp::trapd` have been merged into class `snmp`.  All of their class parameters available in the `snmp` class.
-
-##### Current module 4.x series
-
- * The parameter `install_client` is renamed to `manage_client`.
-
- * Support for Puppet < 4 is removed.
-
-##### Future module 5.x series
-
- * The parameters `ro_community`, `rw_community`, `ro_network`, and `rw_network` will be removed.
-
- * The snmptrapd parameter name will become `authcommunity`.
+Please see the [CHANGELOG](CHANGELOG.md) for details of breaking changes between major releases.
 
 ## Usage
 
@@ -92,8 +76,8 @@ To change the SNMP community from the default value and limit the netblocks that
 ```puppet
 class { 'snmp':
   agentaddress => [ 'udp:161', ],
-  ro_community => 'myPassword',
-  ro_network   => '192.168.0.0/16',
+  ro_community => ['myPassword'],
+  ro_network   => ['192.168.0.0/16'],
 }
 ```
 
@@ -187,11 +171,13 @@ snmp::snmpv3_user { 'myuser':
 
 ### Access Control
 
+For access control, it is recommended to configure VACM, (see below), and turn off traditional access control.
+
 With traditional access control, you can give a simple password and (optional) network restriction:
 ```puppet
 class { 'snmp':
-  ro_community => 'myPassword',
-  ro_network   => '10.0.0.0/8',
+  ro_community => ['myPassword'],
+  ro_network   => ['10.0.0.0/8'],
 }
 ```
 and it becomes this in snmpd.conf:
@@ -200,6 +186,14 @@ rocommunity myPassword 10.0.0.0/8
 ```
 This says that any host on network 10.0.0.0/8 can read any SNMP value via SNMP versions 1 and 2c as long as they provide the password 'myPassword'.
 
+To disable traditional access control make sure you override the `ro_community` and `ro_community6` parameters.
+```puppet
+class { 'snmp':
+  ro_community  => [],
+  ro_community6 => [],
+  # ...
+}
+
 With View-based Access Control Model (VACM), you can do this (more complex) configuration instead:
 ```puppet
 class { 'snmp':
@@ -233,7 +227,7 @@ Reference: [Manpage of snmpd.conf - Access Control](http://www.net-snmp.org/docs
 In traditional access control, you can also pass multiple networks for the community string.
 ```puppet
 class { 'snmp':
-  ro_community => 'shibboleth',
+  ro_community => ['shibboleth'],
   ro_network   => [ '192.168.0.0/16', '1.2.3.4/32', ],
 }
 ```

diff --git a/REFERENCE.md b/REFERENCE.md
@@ -36,7 +36,7 @@ class { 'snmp':
 
 # Only configure and run the snmptrap daemon:
 class { 'snmp':
-  ro_community        => 'SeCrEt',
+  ro_community        => ['SeCrEt'],
   service_ensure      => 'stopped',
   trap_service_ensure => 'running',
   trap_handlers       => [
@@ -69,67 +69,67 @@ Default value: [ 'udp:127.0.0.1:162', 'udp6:[::1]:162' ]
 
 ##### `ro_community`
 
-Data type: `Variant[Undef, String[1], Array[String[1]]]`
+Data type: `Array[String[1]]`
 
-Read-only (RO) community string or array for agent and snmptrap daemon.
+Read-only (RO) array of community strings for agent and snmptrap daemon.
 
-Default value: 'public'
+Default value: ['public']
 
 ##### `ro_community6`
 
-Data type: `Variant[Undef, String[1], Array[String[1]]]`
+Data type: `Array[String[1]]`
 
-Read-only (RO) community string or array for IPv6 agent.
+Read-only (RO) array of community strings for IPv6 agent.
 
-Default value: 'public'
+Default value: ['public']
 
 ##### `rw_community`
 
-Data type: `Variant[Undef, String[1], Array[String[1]]]`
+Data type: `Array[String[1]]`
 
-Read-write (RW) community string or array agent.
+Read-write (RW) array of community strings for agent.
 
-Default value: `undef`
+Default value: []
 
 ##### `rw_community6`
 
-Data type: `Variant[Undef, String[1], Array[String[1]]]`
+Data type: `Array[String[1]]`
 
-Read-write (RW) community string or array for IPv6 agent.
+Read-write (RW) array of community strings for IPv6 agent.
 
-Default value: `undef`
+Default value: []
 
 ##### `ro_network`
 
-Data type: `Variant[Array, Stdlib::IP::Address::V4, Stdlib::IP::Address::V4::CIDR]`
+Data type: `Array[Stdlib::IP::Address::V4]`
 
-Network that is allowed to RO query the daemon.  Can be string or array.
+Networks that are allowed to RO query the daemon.
 
-Default value: '127.0.0.1'
+Default value: ['127.0.0.1']
 
 ##### `ro_network6`
 
-Data type: `Variant[Array, Stdlib::IP::Address::V6, Stdlib::IP::Address::V6::CIDR]`
+Data type: `Array[Stdlib::IP::Address::V6]`
 
-Network that is allowed to RO query the daemon via IPv6.  Can be string or array.
+Networks that are allowed to RO query the daemon via IPv6.
 
-Default value: '::1'
+Default value: ['::1']
 
 ##### `rw_network`
 
-Data type: `Variant[Array, Stdlib::IP::Address::V4, Stdlib::IP::Address::V4::CIDR]`
+Data type: `Array[Stdlib::IP::Address::V4]`
 
-Network that is allowed to RW query the daemon.  Can be string or array.
+Networks that are allowed to RW query the daemon.
 
-Default value: '127.0.0.1'
+Default value: ['127.0.0.1']
 
 ##### `rw_network6`
 
-Data type: `Variant[Array, Stdlib::IP::Address::V6, Stdlib::IP::Address::V6::CIDR]`
+Data type: `Array[Stdlib::IP::Address::V6]`
 
-Network that is allowed to RW query the daemon via IPv6.  Can be string or array.
+Networks that are allowed to RW query the daemon via IPv6.
 
-Default value: '::1'
+Default value: ['::1']
 
 ##### `contact`
 

diff --git a/examples/snmpd-snmptrapd.pp b/examples/snmpd-snmptrapd.pp
@@ -1,5 +1,5 @@
 class { 'snmp':
-  ro_community        => 'SeCrEt',
+  ro_community        => ['SeCrEt'],
   trap_service_ensure => 'running',
   trap_service_enable => true,
   trap_handlers       => [

diff --git a/examples/trapd.pp b/examples/trapd.pp
@@ -1,5 +1,5 @@
 class { 'snmp':
-  ro_community        => 'SeCrEt',
+  ro_community        => ['SeCrEt'],
   service_ensure      => 'stopped',
   trap_service_ensure => 'running',
   trap_service_enable => true,

diff --git a/manifests/init.pp b/manifests/init.pp
@@ -10,7 +10,7 @@
 #
 #   # Only configure and run the snmptrap daemon:
 #   class { 'snmp':
-#     ro_community        => 'SeCrEt',
+#     ro_community        => ['SeCrEt'],
 #     service_ensure      => 'stopped',
 #     trap_service_ensure => 'running',
 #     trap_handlers       => [
@@ -27,28 +27,28 @@
 #   SNMP notifications.
 #
 # @param ro_community
-#   Read-only (RO) community string or array for agent and snmptrap daemon.
+#   Read-only (RO) array of community strings for agent and snmptrap daemon.
 #
 # @param ro_community6
-#   Read-only (RO) community string or array for IPv6 agent.
+#   Read-only (RO) array of community strings for IPv6 agent.
 #
 # @param rw_community
-#   Read-write (RW) community string or array agent.
+#   Read-write (RW) array of community strings for agent.
 #
 # @param rw_community6
-#   Read-write (RW) community string or array for IPv6 agent.
+#   Read-write (RW) array of community strings for IPv6 agent.
 #
 # @param ro_network
-#   Network that is allowed to RO query the daemon.  Can be string or array.
+#   Networks that are allowed to RO query the daemon.
 #
 # @param ro_network6
-#   Network that is allowed to RO query the daemon via IPv6.  Can be string or array.
+#   Networks that are allowed to RO query the daemon via IPv6.
 #
 # @param rw_network
-#   Network that is allowed to RW query the daemon.  Can be string or array.
+#   Networks that are allowed to RW query the daemon.
 #
 # @param rw_network6
-#   Network that is allowed to RW query the daemon via IPv6.  Can be string or array.
+#   Networks that are allowed to RW query the daemon via IPv6.
 #
 # @param contact
 #   Responsible person for the SNMP system.
@@ -245,23 +245,23 @@
 #   Group of `var_net_snmp` directory.
 #
 class snmp (
-  Enum['present','absent']                                        $ensure        = 'present',
-  Array[String[1]]                                                $agentaddress  = [ 'udp:127.0.0.1:161', 'udp6:[::1]:161' ],
-  Array[String[1]]                                                $snmptrapdaddr = [ 'udp:127.0.0.1:162', 'udp6:[::1]:162' ],
-  Variant[Undef, String[1], Array[String[1]]]                     $ro_community  = 'public',
-  Variant[Undef, String[1], Array[String[1]]]                     $ro_community6 = 'public',
-  Variant[Undef, String[1], Array[String[1]]]                     $rw_community  = undef,
-  Variant[Undef, String[1], Array[String[1]]]                     $rw_community6 = undef,
-  Variant[Array, Stdlib::IP::Address::V4, Stdlib::IP::Address::V4::CIDR] $ro_network    = '127.0.0.1',
-  Variant[Array, Stdlib::IP::Address::V6, Stdlib::IP::Address::V6::CIDR] $ro_network6   = '::1',
-  Variant[Array, Stdlib::IP::Address::V4, Stdlib::IP::Address::V4::CIDR] $rw_network    = '127.0.0.1',
-  Variant[Array, Stdlib::IP::Address::V6, Stdlib::IP::Address::V6::CIDR] $rw_network6   = '::1',
-  String[1]                                                       $contact       = 'Unknown',
-  String[1]                                                       $location      = 'Unknown',
-  String[1]                                                       $sysname       = $facts['networking']['fqdn'],
-  Integer                                                         $services      = 72,
-  Array[String[1]]                                                $com2sec       = [ 'notConfigUser  default       public' ],
-  Array[String[1]]                                                $com2sec6      = [ 'notConfigUser  default       public' ],
+  Enum['present','absent'] $ensure = 'present',
+  Array[String[1]] $agentaddress  = [ 'udp:127.0.0.1:161', 'udp6:[::1]:161' ],
+  Array[String[1]] $snmptrapdaddr = [ 'udp:127.0.0.1:162', 'udp6:[::1]:162' ],
+  Array[String[1]] $ro_community  = ['public'],
+  Array[String[1]] $ro_community6 = ['public'],
+  Array[String[1]] $rw_community  = [],
+  Array[String[1]] $rw_community6 = [],
+  Array[Stdlib::IP::Address::V4] $ro_network  = ['127.0.0.1'],
+  Array[Stdlib::IP::Address::V6] $ro_network6 = ['::1'],
+  Array[Stdlib::IP::Address::V4] $rw_network  = ['127.0.0.1'],
+  Array[Stdlib::IP::Address::V6] $rw_network6 = ['::1'],
+  String[1] $contact  = 'Unknown',
+  String[1] $location = 'Unknown',
+  String[1] $sysname  = $facts['networking']['fqdn'],
+  Integer   $services = 72,
+  Array[String[1]] $com2sec  = [ 'notConfigUser  default       public' ],
+  Array[String[1]] $com2sec6 = [ 'notConfigUser  default       public' ],
   Array[String[1]] $groups = [
     'notConfigGroup v1            notConfigUser',
     'notConfigGroup v2c           notConfigUser',

diff --git a/spec/classes/snmp_init_spec.rb b/spec/classes/snmp_init_spec.rb
@@ -450,8 +450,8 @@
           end
         end
 
-        describe 'ro_network => "127.0.0.2"' do
-          let(:params) { { ro_network: '127.0.0.2' } }
+        describe 'ro_network => ["127.0.0.2"]' do
+          let(:params) { { ro_network: ['127.0.0.2'] } }
 
           it 'contains File[snmpd.conf] with contents "127.0.0.2"' do
             verify_contents(catalogue, 'snmpd.conf', [
@@ -460,8 +460,8 @@
           end
         end
 
-        describe 'ro_community => [ "a", "b", ] and ro_network => "127.0.0.2"' do
-          let(:params) { { ro_community: %w[a b], ro_network: '127.0.0.2' } }
+        describe 'ro_community => [ "a", "b", ] and ro_network => ["127.0.0.2"]' do
+          let(:params) { { ro_community: %w[a b], ro_network: ['127.0.0.2'] } }
 
           it 'contains File[snmpd.conf] with contents "a 127.0.0.2" and "b 127.0.0.2"' do
             verify_contents(catalogue, 'snmpd.conf', [

diff --git a/templates/snmpd.conf.erb b/templates/snmpd.conf.erb
@@ -36,23 +36,23 @@ agentaddress <%= @agentaddress.join(',') %>
 
 # ------------------------------------------------------------------------------
 # Traditional Access Control
-<%- [*@ro_community].compact.each do |c| -%>
-  <%- [*@ro_network].compact.each do |n| -%>
+<%- @ro_community.each do |c| -%>
+  <%- @ro_network.each do |n| -%>
 rocommunity <%= c %> <%= n %>
   <%- end -%>
 <%- end -%>
-<%- [*@ro_community6].compact.each do |c| -%>
-  <%- [*@ro_network6].compact.each do |n| -%>
+<%- @ro_community6.each do |c| -%>
+  <%- @ro_network6.each do |n| -%>
 rocommunity6 <%= c %> <%= n %>
   <%- end -%>
 <%- end -%>
-<%- [*@rw_community].compact.each do |c| -%>
-  <%- [*@rw_network].compact.each do |n| -%>
+<%- @rw_community.each do |c| -%>
+  <%- @rw_network.each do |n| -%>
 rwcommunity <%= c %> <%= n %>
   <%- end -%>
 <%- end -%>
-<%- [*@rw_community6].compact.each do |c| -%>
-  <%- [*@rw_network6].compact.each do |n| -%>
+<%- @rw_community6.each do |c| -%>
+  <%- @rw_network6.each do |n| -%>
 rocommunity6 <%= c %> <%= n %>
   <%- end -%>
 <%- end -%>

diff --git a/templates/snmptrapd.conf.erb b/templates/snmptrapd.conf.erb
@@ -13,7 +13,7 @@ doNotLogTraps <%= @do_not_log_traps %>
 ################################################################################
 # ACCESS CONTROL
 
-<%- [*@ro_community].compact.each do |c| -%>
+<%- @ro_community.each do |c| -%>
 authCommunity log,execute,net <%= c %>
 <%- end -%>
 disableAuthorization <%= @disable_authorization %>