Provide the option to only update the live value (#15)

There are some sysctl values which, when set in /etc/sysctl.conf, may
cause operational harm to a system.

This patch provides the ability to only update the live value and make
the disk persistence optional.

Additionally:
- Now use prefetching to get the sysctl values
- Updated self.instances to obtain information about all sysctl values
  which provides a more accurate representation of the system when using
  `puppet resource`
- Fail if the user attempts to set a value that is not valid on the
  system unless `silent` is set.
- Updated all tests
- Refactored Travis CI Tests
- Added OpenSUSE support

Closes #14

Author: trevor-vaughan

.gitignore

@@ -1,3 +1,4 @@
+Gemfile.lock
 pkg
 coverage
 .bundle/
@@ -9,6 +10,7 @@ spec/fixtures/manifests
 spec/fixtures/modules
 yardoc/
 .yardoc/
+log/
 
 ## Ruby
 .rvmrc*

.travis.yml

@@ -1,66 +1,20 @@
 language: ruby
 sudo: required
 rvm:
-  - 1.8.7
-  - 1.9.3
-  - 2.0.0
+  - 2.1.9
+  - 2.3.1
+
 notifications:
   email:
    - raphael.pinson@camptocamp.com
 env:
-# base env
-  # Most tests with oldest supported ruby-augeas
-  - PUPPET=3.0.0 RUBY_AUGEAS=0.3.0 AUGEAS=1.1.0
-  - PUPPET=3.2.0 RUBY_AUGEAS=0.3.0 AUGEAS=1.1.0
-  - PUPPET=3.4 RUBY_AUGEAS=0.3.0 AUGEAS=1.1.0
-  # Test the latest ruby-augeas (~>)
-  - PUPPET=3.2.0 RUBY_AUGEAS=0.5
-    # Use this build to publish on the forge
-  - PUPPET=3.4 RUBY_AUGEAS=0.5 FORGE_PUBLISH=true
-  # Test other versions of Augeas
-  - PUPPET=3.4 RUBY_AUGEAS=0.3.0 AUGEAS=0.10.0
-  - PUPPET=3.4 RUBY_AUGEAS=0.3.0 AUGEAS=1.0.0
-  - PUPPET=3.4 RUBY_AUGEAS=0.3.0 AUGEAS=1.1.0
-  - PUPPET=2.7.0 RUBY_AUGEAS=0.3.0 AUGEAS=1.2.0
-  - PUPPET=3.4 RUBY_AUGEAS=0.3.0 AUGEAS=1.2.0
-  # Issue #83: test old Augeas with new lenses
-  - PUPPET=3.4 RUBY_AUGEAS=0.3.0 AUGEAS=1.0.0 LENSES=HEAD
-  - PUPPET=3.4 RUBY_AUGEAS=0.3.0 AUGEAS=1.1.0 LENSES=HEAD
-  - PUPPET=3.4 RUBY_AUGEAS=0.5 AUGEAS=1.0.0 LENSES=HEAD
-  - PUPPET=3.4 RUBY_AUGEAS=0.5 AUGEAS=1.1.0 LENSES=HEAD
-  # Test latest Puppet version
-  - PUPPET=4.0 RUBY_AUGEAS=0.5
-
+  # Most common LTS Puppet Version
+  - PUPPET=4.7 RUBY_AUGEAS=0.5 FORGE_PUBLISH=true
+  # Current LTS Puppet Version
+  - PUPPET=4.10 RUBY_AUGEAS=0.5
 
 matrix:
   fast_finish: true
-  exclude:
-# base exclude
-    # No support for Ruby 2.0 before Puppet 3.2.0 and ruby-augeas 0.5
-    - rvm: 2.0.0
-      env: PUPPET=3.0.0 RUBY_AUGEAS=0.3.0
-    - rvm: 2.0.0
-      env: PUPPET=3.2.0 RUBY_AUGEAS=0.3.0
-    - rvm: 2.0.0
-      env: PUPPET=3.4 RUBY_AUGEAS=0.3.0
-    - rvm: 2.0.0
-      env: PUPPET=3.4 RUBY_AUGEAS=0.3.0 AUGEAS=0.10.0
-    - rvm: 2.0.0
-      env: PUPPET=3.4 RUBY_AUGEAS=0.3.0 AUGEAS=1.0.0
-    - rvm: 2.0.0
-      env: PUPPET=3.4 RUBY_AUGEAS=0.3.0 AUGEAS=1.1.0
-    - rvm: 2.0.0
-      env: PUPPET=3.0.0 RUBY_AUGEAS=0.3.0 AUGEAS=1.1.0
-    - rvm: 2.0.0
-      env: PUPPET=3.4 RUBY_AUGEAS=0.3.0 AUGEAS=1.2.0
-    - rvm: 2.0.0
-      env: PUPPET=3.4 RUBY_AUGEAS=0.3.0 AUGEAS=1.0.0 LENSES=HEAD
-    - rvm: 2.0.0
-      env: PUPPET=3.4 RUBY_AUGEAS=0.3.0 AUGEAS=1.1.0 LENSES=HEAD
-    # No support for Ruby 1.8 in Puppet 4
-    - rvm: 1.8.7
-      env: PUPPET=4.0 RUBY_AUGEAS=0.5
-
 
 install:
   - "travis_retry ./.travis.sh"
@@ -78,5 +32,5 @@ deploy:
     # all_branches is required to use tags
     all_branches: true
     # Only publish if our main Ruby target builds
-    rvm: 1.9.3
+    rvm: 2.1.9
     condition: "$FORGE_PUBLISH = true"

CHANGELOG.md

@@ -1,5 +1,18 @@
 # Changelog
 
+## 2.2.0
+- Removed Travis tests for Puppet < 4.7 since that is the most common LTS
+  release and Puppet 3 is well out of support
+- Added OpenBSD and FreeBSD to the compatibility list
+- Added a :persist option for enabling saving to the /etc/sysctl.conf file
+- Added the capability to update either the live value *or* the disk value
+  independently
+- Now use prefetching to get the sysctl values
+- Updated self.instances to obtain information about *all* sysctl values which
+  provides a more accurate representation of the system when using `puppet
+  resource`
+- Updated all tests
+
 ## 2.1.0
 - Added a :silent option for deliberately ignoring failures when applying the
   live sysctl setting.

Gemfile

@@ -39,6 +39,8 @@ group :development, :unit_tests do
   gem 'puppet-lint-file_ensure-check',                     :require => false
   gem 'puppet-lint-version_comparison-check',              :require => false
   gem 'rspec-puppet-facts',                                :require => false
+  gem 'beaker-rspec',                                      :require => false
+  gem 'simp-beaker-helpers',                               :require => false
 
   gem 'coveralls',                                         :require => false unless RUBY_VERSION =~ /^1\.8/
   gem 'simplecov', '~> 0.7.0',                             :require => false

README.md

@@ -104,6 +104,14 @@ Type documentation can be generated with `puppet doc -r type` or viewed on the
       apply  => false,
     }
 
+### only update the value with the `sysctl` command, do not persist to disk
+
+    sysctl { "net.ipv4.ip_forward":
+      ensure  => present,
+      value   => "1",
+      persist => false,
+    }
+
 ### ignore the application of a yet to be activated sysctl value
 
     sysctl { "net.ipv6.conf.all.autoconf":

lib/puppet/provider/sysctl/augeas.rb

@@ -39,16 +39,24 @@ def self.sysctl_get(key)
 
   confine :feature => :augeas
 
-  def self.instances
-    augopen do |aug|
-      resources = []
+  def self.instances(reference_resource = nil)
+    return @resource_cache if @resource_cache
+
+    resources = nil
+
+    augopen(reference_resource) do |aug|
+      resources ||= []
+
       aug.match("$target/*").each do |spath|
-        resource = {:ensure => :present}
+        resource = {
+          :ensure  => :present,
+          :persist => :true
+        }
 
         basename = spath.split("/")[-1]
         resource[:name] = basename.split("[")[0]
         next unless resource[:name]
-        next if resource[:name] == "#comment"
+        next if (resource[:name] == "#comment")
 
         resource[:value] = aug.get("#{spath}")
 
@@ -62,32 +70,104 @@ def self.instances
           end
         end
 
-        resources << new(resource)
+        resources << resource
+      end
+    end
+
+    # Grab everything else
+    resources ||= []
+
+    sysctl('-a').each_line do |line|
+      value = line.split('=')
+
+      key = value.shift.strip
+
+      value = value.join('=').strip
+
+      existing_index = resources.index{ |x| x[:name] == key }
+
+      if existing_index
+        resources[existing_index][:apply] = :true
+      else
+        resources << {
+          :name    => key,
+          :ensure  => :present,
+          :value   => value,
+          :apply   => :true,
+          :persist => :false
+        }
+      end
+    end
+
+    if resources
+      @resource_cache = resources.map{|x| x = new(x)}
+      return @resource_cache
+    end
+  end
+
+  def self.prefetch(resources)
+    # We need to pass a reference resource so that the proper target is in
+    # scope.
+    instances(resources.first.last).each do |prov|
+      if resource = resources[prov.name]
+        resource.provider = prov
       end
-      resources
     end
   end
 
-  def create 
-    # the value to pass to augeas can come either from the 'value' or the
-    # 'val' type parameter.
-    value = resource[:value] || resource[:val]
+  def create
+    if resource[:persist] == :true
+      if !valid_resource?(resource[:name]) && (resource[:silent] == :false)
+        raise Puppet::Error, "Error: `#{resource[:name]}` is not a valid sysctl key"
+      end
 
-    augopen! do |aug|
-      # Prefer to create the node next to a commented out entry
-      commented = aug.match("$target/#comment[.=~regexp('#{resource[:name]}([^a-z\.].*)?')]")
-      aug.insert(commented.first, resource[:name], false) unless commented.empty?
-      aug.set(resource_path, value)
-      setvars(aug)
+      # the value to pass to augeas can come either from the 'value' or the
+      # 'val' type parameter.
+      value = resource[:value] || resource[:val]
 
-      if resource[:comment]
-        aug.insert('$resource', "#comment", true)
-        aug.set("$target/#comment[following-sibling::*[1][self::#{resource[:name]}]]",
-                "#{resource[:name]}: #{resource[:comment]}")
+      augopen! do |aug|
+        # Prefer to create the node next to a commented out entry
+        commented = aug.match("$target/#comment[.=~regexp('#{resource[:name]}([^a-z\.].*)?')]")
+        aug.insert(commented.first, resource[:name], false) unless commented.empty?
+        aug.set(resource_path, value)
+        setvars(aug)
       end
     end
   end
 
+  def valid_resource?(name)
+    @property_hash.is_a?(Hash) && !@property_hash.empty? && (@property_hash[:apply] == :true)
+  end
+
+  def exists?
+    # If in silent mode, short circuit the process on an invalid key
+    #
+    # This only matters when creating entries since invalid missing entries
+    # might be used to clean up /etc/sysctl.conf
+    if resource[:ensure] != :absent
+      if !valid_resource?(resource[:name])
+        if resource[:silent] == :true
+          debug("augeasproviders_sysctl: `#{resource[:name]}` is not a valid sysctl key")
+          return true
+        else
+          raise Puppet::Error, "Error: `#{resource[:name]}` is not a valid sysctl key"
+        end
+      end
+    end
+
+    if @property_hash[:ensure] == :present
+      # Short circuit this if there's nothing to do
+      if (resource[:ensure] == :absent) && (@property_hash[:persist] == :false)
+        return false
+      else
+        return true
+      end
+    else
+      super
+    end
+  end
+
+
   define_aug_method!(:destroy) do |aug, resource|
     aug.rm("$target/#comment[following-sibling::*[1][self::#{resource[:name]}]][. =~ regexp('#{resource[:name]}:.*')]")
     aug.rm('$resource')
@@ -131,11 +211,26 @@ def live_value
   end
 
   def flush
-    super
-    value = resource[:value] || resource[:val]
-    if resource[:apply] == :true && !value.nil?
-      silent = (resource[:silent] == :true)
-      self.class.sysctl_set(resource[:name], value, silent)
+    if resource[:ensure] == :absent
+      super
+    else
+      if resource[:apply] == :true
+        value = resource[:value] || resource[:val]
+        if value
+          silent = (resource[:silent] == :true)
+          self.class.sysctl_set(resource[:name], value, silent)
+        end
+      end
+
+      # Ensures that we only save to disk when we're supposed to
+      if resource[:persist] == :true
+        # Create the entry on disk if it's not already there
+        if @property_hash[:persist] == :false
+          create
+        end
+
+        super
+      end
     end
   end
 end

lib/puppet/type/sysctl.rb

@@ -15,12 +15,25 @@
 
   module SysctlValueSync
     def insync?(is)
-      if resource[:apply] == :true
-        @live_value = provider.live_value
-        equal(should, is) and equal(should, @live_value)
+      _is_insync = true
+
+      if provider.valid_resource?(resource[:name])
+        if resource[:apply] == :true
+          @live_value = provider.live_value
+
+          _is_insync = equal(should, @live_value)
+        end
+
+        if _is_insync && (resource[:persist] == :true)
+            _is_insync = equal(should, is)
+        end
       else
-        equal(should, is)
+        # We won't get here unless exists? has been short circuited so we can
+        # rely on that to raise an approprite error.
+        debug("augeasproviders_sysctl: skipping insync? due to invalid resource `#{resource[:name]}`")
       end
+
+      return _is_insync
     end
 
     def change_to_s(current, new)
@@ -30,15 +43,19 @@ def change_to_s(current, new)
         elsif equal(@live_value, new)
           return "changed configuration value from '#{current}' to '#{new}'"
         else
-          return "changed configuration value from '#{current}' to '#{new}' and live value from '#{@live_value}' to '#{new}'"
+          if resource[:persist] == :true
+            return "changed configuration value from '#{current}' to '#{new}' and live value from '#{@live_value}' to '#{new}'"
+          else
+            return "changed live value from '#{@live_value}' to '#{new}'"
+          end
         end
       else
         return "changed configuration value from '#{current}' to '#{new}'"
       end
     end
 
     def equal(a, b)
-      a.gsub(/\s+/, ' ') == b.gsub(/\s+/, ' ')
+      a && b && (a.gsub(/\s+/, ' ') == b.gsub(/\s+/, ' '))
     end
   end
 
@@ -77,6 +94,12 @@ def equal(a, b)
     defaultto(:true)
   end
 
+  newparam(:persist, :boolean => true) do
+    desc "Persist the value in the on-disk file ($target)."
+    newvalues(:true, :false)
+    defaultto(:true)
+  end
+
   newparam(:silent, :boolean => true) do
     desc "If set, do not report an error if the system key does not exist. This is useful for systems that may need to load a kernel module prior to the sysctl values existing."
     newvalues(:true, :false)