From fa6e8299c1e20b5d79b8dd15efd1c26476a02570 Mon Sep 17 00:00:00 2001 From: Tom Downes Date: Tue, 2 Aug 2016 13:53:10 -0500 Subject: [PATCH 1/8] 1. Create a puppet 4 branch for development of improved hiera lookups 2. Resolve weirdness of namepsace where sysctl is not a class but sysctl::base by making sysctl a class and moving defined resource to sysctl::configuration. --- CHANGELOG.md | 5 ++ data/default.yaml | 14 ++++ data/os/Debian-8.yaml | 2 + data/os/FreeBSD.yaml | 2 + data/os/RedHat-7.yaml | 2 + hiera.yaml | 12 +++ manifests/base.pp | 51 ------------ manifests/configuration.pp | 104 +++++++++++++++++++++++++ manifests/init.pp | 129 ++++++------------------------- manifests/params.pp | 27 ------- metadata.json | 3 +- spec/classes/sysctl_base_spec.rb | 9 --- spec/classes/sysctl_spec.rb | 9 +++ spec/defines/sysctl_init_spec.rb | 2 +- 14 files changed, 178 insertions(+), 193 deletions(-) create mode 100644 data/default.yaml create mode 100644 data/os/Debian-8.yaml create mode 100644 data/os/FreeBSD.yaml create mode 100644 data/os/RedHat-7.yaml create mode 100644 hiera.yaml delete mode 100644 manifests/base.pp create mode 100644 manifests/configuration.pp delete mode 100644 manifests/params.pp delete mode 100644 spec/classes/sysctl_base_spec.rb create mode 100644 spec/classes/sysctl_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 3433040..a487574 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +#### 2016-08-02 - 1.0.6-puppet4 +* Create a Puppet 4 branch to take advantage of improved hiera lookups +* Because Puppet 4 is a major upgrade that breaks many things, this is not + intended to be backwards compatible + #### 2016-02-05 - 1.0.6 * Revert previous incorrect change, more work is needed to cover all cases. diff --git a/data/default.yaml b/data/default.yaml new file mode 100644 index 0000000..3ff0cb2 --- /dev/null +++ b/data/default.yaml @@ -0,0 +1,14 @@ +--- +lookup_options: + sysctl::values: + merge: deep + +sysctl::purge: false +sysctl::values: {} +sysctl::symlink99: false + +sysctl::sysctl_dir: true +sysctl::sysctl_dir_path: '/etc/sysctl.d' +sysctl::sysctl_dir_owner: root +sysctl::sysctl_dir_group: root +sysctl::sysctl_dir_mode: '0755' diff --git a/data/os/Debian-8.yaml b/data/os/Debian-8.yaml new file mode 100644 index 0000000..ac051ce --- /dev/null +++ b/data/os/Debian-8.yaml @@ -0,0 +1,2 @@ +--- +sysctl::symlink99: true diff --git a/data/os/FreeBSD.yaml b/data/os/FreeBSD.yaml new file mode 100644 index 0000000..1f68223 --- /dev/null +++ b/data/os/FreeBSD.yaml @@ -0,0 +1,2 @@ +--- +sysctl::sysctl_dir: false diff --git a/data/os/RedHat-7.yaml b/data/os/RedHat-7.yaml new file mode 100644 index 0000000..ac051ce --- /dev/null +++ b/data/os/RedHat-7.yaml @@ -0,0 +1,2 @@ +--- +sysctl::symlink99: true diff --git a/hiera.yaml b/hiera.yaml new file mode 100644 index 0000000..45cdeb5 --- /dev/null +++ b/hiera.yaml @@ -0,0 +1,12 @@ +--- +version: 4 +datadir: data +hierarchy: + - name: "OS family" + backend: yaml + path: "os/%{facts.os.family}" + - name: "OS family and release" + backend: yaml + path: "os/%{facts.os.family}-${facts.release.major}" + - name: default + backend: yaml diff --git a/manifests/base.pp b/manifests/base.pp deleted file mode 100644 index ab56302..0000000 --- a/manifests/base.pp +++ /dev/null @@ -1,51 +0,0 @@ -# Class: sysctl::base -# -# Common part for the sysctl definition. Not meant to be used on its own. -# -class sysctl::base ( - $purge = false, - $values = undef, - $hiera_merge_values = false, - $symlink99 = $::sysctl::params::symlink99, - $sysctl_dir = $::sysctl::params::sysctl_dir, - $sysctl_dir_path = $::sysctl::params::sysctl_dir_path, - $sysctl_dir_owner = $::sysctl::params::sysctl_dir_owner, - $sysctl_dir_group = $::sysctl::params::sysctl_dir_group, - $sysctl_dir_mode = $::sysctl::params::sysctl_dir_mode, -) inherits ::sysctl::params { - - # Hiera support - if $hiera_merge_values == true { - $values_real = hiera_hash('sysctl::base::values', {}) - } else { - $values_real = $values - } - if $values_real != undef { - create_resources(sysctl,$values_real) - } - - if $sysctl_dir { - if $purge { - $recurse = true - } else { - $recurse = false - } - file { $sysctl_dir_path: - ensure => directory, - owner => $sysctl_dir_owner, - group => $sysctl_dir_group, - mode => $sysctl_dir_mode, - # Magic hidden here - purge => $purge, - recurse => $recurse, - } - if $symlink99 and $sysctl_dir_path =~ /^\/etc\/[^\/]+$/ { - file { "${sysctl_dir_path}/99-sysctl.conf": - ensure => link, - target => '../sysctl.conf', - } - } - } - -} - diff --git a/manifests/configuration.pp b/manifests/configuration.pp new file mode 100644 index 0000000..f00d364 --- /dev/null +++ b/manifests/configuration.pp @@ -0,0 +1,104 @@ +# Manage sysctl variable values. +# +# Parameters: +# $value: +# The value for the sysctl parameter. Mandatory, unless $ensure is 'absent'. +# $prefix: +# Optional prefix for the sysctl.d file to be created. Default: none. +# $ensure: +# Whether the variable's value should be 'present' or 'absent'. +# Defaults to 'present'. +# +# Sample Usage : +# sysctl::configuration { 'net.ipv6.bindv6only': value => '1' } +define sysctl::configuration ( + $ensure = 'present', + $value = undef, + $prefix = undef, + $suffix = '.conf', + $comment = undef, + $content = undef, + $source = undef, + $enforce = true, +) { + + # If we have a prefix, then add the dash to it + if $prefix { + $_sysctl_d_file = "${prefix}-${title}${suffix}" + } else { + $_sysctl_d_file = "${title}${suffix}" + } + + # Some sysctl keys contain a slash, which is not valid in a filename. + # Most common at those on VLANs: net.ipv4.conf.eth0/1.arp_accept = 0 + $sysctl_d_file = regsubst($_sysctl_d_file, '[/ ]', '_', 'G') + + # If we have an explicit content or source, use them + if $content or $source { + $file_content = $content + $file_source = $source + } else { + $file_content = template("${module_name}/sysctl.d-file.erb") + $file_source = undef + } + + if $ensure != 'absent' { + + # Present + + # The permanent change + file { "/etc/sysctl.d/${sysctl_d_file}": + ensure => $ensure, + owner => 'root', + group => 'root', + mode => '0644', + content => $file_content, + source => $file_source, + notify => [ + Exec["sysctl-${title}"], + Exec["update-sysctl.conf-${title}"], + ], + } + + # The immediate change + re-check on each run "just in case" + exec { "sysctl-${title}": + command => "sysctl -p /etc/sysctl.d/${sysctl_d_file}", + path => [ '/usr/sbin', '/sbin', '/usr/bin', '/bin' ], + refreshonly => true, + require => File["/etc/sysctl.d/${sysctl_d_file}"], + } + + # For the few original values from the main file + exec { "update-sysctl.conf-${title}": + command => "sed -i -e 's#^${title} *=.*#${title} = ${value}#' /etc/sysctl.conf", + path => [ '/usr/sbin', '/sbin', '/usr/bin', '/bin' ], + refreshonly => true, + onlyif => "grep -E '^${title} *=' /etc/sysctl.conf", + } + + # Enforce configured value during each run (can't work with custom files) + if $enforce and ! ( $content or $source ) { + $qtitle = shellquote($title) + # Value may contain '|' and others, we need to quote to be safe + # Convert any numerical to expected string, 0 instead of '0' would fail + # lint:ignore:only_variable_string Convert numerical to string + $qvalue = shellquote("${value}") + # lint:endignore + exec { "enforce-sysctl-value-${qtitle}": + unless => "/usr/bin/test \"$(/sbin/sysctl -n ${qtitle})\" = ${qvalue}", + command => "/sbin/sysctl -w ${qtitle}=${qvalue}", + } + } + + } else { + + # Absent + # We cannot restore values, since defaults can not be known... reboot :-/ + + file { "/etc/sysctl.d/${sysctl_d_file}": + ensure => absent, + } + + } + +} diff --git a/manifests/init.pp b/manifests/init.pp index 70e2ee5..5111e6e 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -1,110 +1,31 @@ -# Define: sysctl -# -# Manage sysctl variable values. -# -# Parameters: -# $value: -# The value for the sysctl parameter. Mandatory, unless $ensure is 'absent'. -# $prefix: -# Optional prefix for the sysctl.d file to be created. Default: none. -# $ensure: -# Whether the variable's value should be 'present' or 'absent'. -# Defaults to 'present'. -# -# Sample Usage : -# sysctl { 'net.ipv6.bindv6only': value => '1' } -# -define sysctl ( - $ensure = undef, - $value = undef, - $prefix = undef, - $suffix = '.conf', - $comment = undef, - $content = undef, - $source = undef, - $enforce = true, -) { - - include '::sysctl::base' - - # If we have a prefix, then add the dash to it - if $prefix { - $_sysctl_d_file = "${prefix}-${title}${suffix}" - } else { - $_sysctl_d_file = "${title}${suffix}" - } - - # Some sysctl keys contain a slash, which is not valid in a filename. - # Most common at those on VLANs: net.ipv4.conf.eth0/1.arp_accept = 0 - $sysctl_d_file = regsubst($_sysctl_d_file, '[/ ]', '_', 'G') - - # If we have an explicit content or source, use them - if $content or $source { - $file_content = $content - $file_source = $source - } else { - $file_content = template("${module_name}/sysctl.d-file.erb") - $file_source = undef - } - - if $ensure != 'absent' { - - # Present - - # The permanent change - file { "/etc/sysctl.d/${sysctl_d_file}": - ensure => $ensure, - owner => 'root', - group => 'root', - mode => '0644', - content => $file_content, - source => $file_source, - notify => [ - Exec["sysctl-${title}"], - Exec["update-sysctl.conf-${title}"], - ], - } - - # The immediate change + re-check on each run "just in case" - exec { "sysctl-${title}": - command => "sysctl -p /etc/sysctl.d/${sysctl_d_file}", - path => [ '/usr/sbin', '/sbin', '/usr/bin', '/bin' ], - refreshonly => true, - require => File["/etc/sysctl.d/${sysctl_d_file}"], - } - - # For the few original values from the main file - exec { "update-sysctl.conf-${title}": - command => "sed -i -e 's#^${title} *=.*#${title} = ${value}#' /etc/sysctl.conf", - path => [ '/usr/sbin', '/sbin', '/usr/bin', '/bin' ], - refreshonly => true, - onlyif => "grep -E '^${title} *=' /etc/sysctl.conf", +class sysctl (Boolean $purge, + Hash $values, + Boolean $symlink99, + Boolean $sysctl_dir, + String $sysctl_dir_path, + String $sysctl_dir_owner, + String $sysctl_dir_group, + String $sysctl_dir_mode) { + + create_resources(sysctl::configuration, $values) + + if $sysctl_dir { + # if we're purging we should also recurse + $recurse = $purge + file { $sysctl_dir_path: + ensure => directory, + owner => $sysctl_dir_owner, + group => $sysctl_dir_group, + mode => $sysctl_dir_mode, + purge => $purge, + recurse => $recurse, } - # Enforce configured value during each run (can't work with custom files) - if $enforce and ! ( $content or $source ) { - $qtitle = shellquote($title) - # Value may contain '|' and others, we need to quote to be safe - # Convert any numerical to expected string, 0 instead of '0' would fail - # lint:ignore:only_variable_string Convert numerical to string - $qvalue = shellquote("${value}") - # lint:endignore - exec { "enforce-sysctl-value-${qtitle}": - unless => "/usr/bin/test \"$(/sbin/sysctl -n ${qtitle})\" = ${qvalue}", - command => "/sbin/sysctl -w ${qtitle}=${qvalue}", + if $symlink99 and $sysctl_dir_path =~ /^\/etc\/[^\/]+$/ { + file { "${sysctl_dir_path}/99-sysctl.conf": + ensure => link, + target => '../sysctl.conf', } } - - } else { - - # Absent - # We cannot restore values, since defaults can not be known... reboot :-/ - - file { "/etc/sysctl.d/${sysctl_d_file}": - ensure => absent, - } - } - } - diff --git a/manifests/params.pp b/manifests/params.pp deleted file mode 100644 index b2d255d..0000000 --- a/manifests/params.pp +++ /dev/null @@ -1,27 +0,0 @@ -class sysctl::params { - - # Keep the original symlink if we purge, to avoid ping-pong with initscripts - case "${::osfamily}-${::operatingsystemmajrelease}" { - 'RedHat-7','Debian-8': { - $symlink99 = true - } - default: { - $symlink99 = false - } - } - - case $::osfamily { - 'FreeBSD': { - $sysctl_dir = false - } - default: { - $sysctl_dir = true - $sysctl_dir_path = '/etc/sysctl.d' - $sysctl_dir_owner = 'root' - $sysctl_dir_group = 'root' - $sysctl_dir_mode = '0755' - } - } - -} - diff --git a/metadata.json b/metadata.json index 5283850..40626b6 100644 --- a/metadata.json +++ b/metadata.json @@ -36,5 +36,6 @@ "version_requirement": ">=2.7.20 <4.1.0" } ], - "dependencies": [] + "dependencies": [], + "data_provider": "hiera" } diff --git a/spec/classes/sysctl_base_spec.rb b/spec/classes/sysctl_base_spec.rb deleted file mode 100644 index a1d47a2..0000000 --- a/spec/classes/sysctl_base_spec.rb +++ /dev/null @@ -1,9 +0,0 @@ -require 'spec_helper' - -describe 'sysctl::base', :type => :class do - - it { should create_class('sysctl::base') } - it { should contain_file('/etc/sysctl.d') } - -end - diff --git a/spec/classes/sysctl_spec.rb b/spec/classes/sysctl_spec.rb new file mode 100644 index 0000000..c33a432 --- /dev/null +++ b/spec/classes/sysctl_spec.rb @@ -0,0 +1,9 @@ +require 'spec_helper' + +describe 'sysctl', :type => :class do + + it { should create_class('sysctl') } + it { should contain_file('/etc/sysctl.d') } + +end + diff --git a/spec/defines/sysctl_init_spec.rb b/spec/defines/sysctl_init_spec.rb index 1f8db67..c000a53 100644 --- a/spec/defines/sysctl_init_spec.rb +++ b/spec/defines/sysctl_init_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe 'sysctl', :type => :define do +describe 'sysctl::configuration', :type => :define do let(:title) { 'net.ipv4.ip_forward'} context 'present' do From 4a8cca3091ddc54763784171eb22089cf23799ba Mon Sep 17 00:00:00 2001 From: Tom Downes Date: Tue, 2 Aug 2016 15:01:54 -0500 Subject: [PATCH 2/8] 1. Fix internal inconsistency with location of sysctl_dir_path 2. Do better job of ensuring that bad configuration files are not created 3. Variable type should not rely on title alone, but should default to the title. This allows the rule in the logs to take on a clear name based on purpose while also allowing for the simple use of kernel variable. --- data/default.yaml | 1 + data/os/Debian.yaml | 2 + manifests/configuration.pp | 81 ++++++++++++++++++-------------------- manifests/init.pp | 17 +++++--- 4 files changed, 52 insertions(+), 49 deletions(-) create mode 100644 data/os/Debian.yaml diff --git a/data/default.yaml b/data/default.yaml index 3ff0cb2..94f00f1 100644 --- a/data/default.yaml +++ b/data/default.yaml @@ -7,6 +7,7 @@ sysctl::purge: false sysctl::values: {} sysctl::symlink99: false +sysctl::sysctl_binary: '/usr/sbin/sysctl' sysctl::sysctl_dir: true sysctl::sysctl_dir_path: '/etc/sysctl.d' sysctl::sysctl_dir_owner: root diff --git a/data/os/Debian.yaml b/data/os/Debian.yaml new file mode 100644 index 0000000..d0a4180 --- /dev/null +++ b/data/os/Debian.yaml @@ -0,0 +1,2 @@ +--- +sysctl::sysctl_binary: '/sbin/sysctl' diff --git a/manifests/configuration.pp b/manifests/configuration.pp index f00d364..1f6b9a5 100644 --- a/manifests/configuration.pp +++ b/manifests/configuration.pp @@ -12,21 +12,24 @@ # Sample Usage : # sysctl::configuration { 'net.ipv6.bindv6only': value => '1' } define sysctl::configuration ( - $ensure = 'present', - $value = undef, - $prefix = undef, - $suffix = '.conf', - $comment = undef, - $content = undef, - $source = undef, - $enforce = true, + String $variable = $title, + String $ensure = 'present', + String $value = undef, + String $prefix = undef, + String $suffix = '.conf', + String $comment = undef, + String $content = undef, + String $source = undef, + Boolean $enforce = true, + String $sysctl_binary, + String $sysctl_dir_path, ) { # If we have a prefix, then add the dash to it if $prefix { - $_sysctl_d_file = "${prefix}-${title}${suffix}" + $_sysctl_d_file = "${prefix}-${variable}${suffix}" } else { - $_sysctl_d_file = "${title}${suffix}" + $_sysctl_d_file = "${variable}${suffix}" } # Some sysctl keys contain a slash, which is not valid in a filename. @@ -46,59 +49,51 @@ # Present - # The permanent change - file { "/etc/sysctl.d/${sysctl_d_file}": - ensure => $ensure, - owner => 'root', - group => 'root', - mode => '0644', - content => $file_content, - source => $file_source, - notify => [ - Exec["sysctl-${title}"], - Exec["update-sysctl.conf-${title}"], - ], - } - - # The immediate change + re-check on each run "just in case" - exec { "sysctl-${title}": - command => "sysctl -p /etc/sysctl.d/${sysctl_d_file}", - path => [ '/usr/sbin', '/sbin', '/usr/bin', '/bin' ], - refreshonly => true, - require => File["/etc/sysctl.d/${sysctl_d_file}"], + # Temporary file created and "sysctl -p filename" is run on that + # If exit code is 0, the kernel setting has changed and puppet + # copies the file to its permanent location. + # Advantage: file in /etc/sysctl.d only if setting is good + # Disadvantage: kernel setting will change but will fail on reboot + # if there is a problem creating file (an obvious failure in the logs) + file { "${sysctl_dir_path}/${sysctl_d_file}": + ensure => $ensure, + owner => 'root', + group => 'root', + mode => '0644', + content => $file_content, + source => $file_source, + notify => Exec["update-sysctl.conf-${variable}"], + validate_cmd => "${sysctl_binary} -p %", } # For the few original values from the main file - exec { "update-sysctl.conf-${title}": - command => "sed -i -e 's#^${title} *=.*#${title} = ${value}#' /etc/sysctl.conf", + exec { "update-sysctl.conf-${variable}": + command => "sed -i -e 's#^${variable} *=.*#${variable} = ${value}#' /etc/sysctl.conf", path => [ '/usr/sbin', '/sbin', '/usr/bin', '/bin' ], refreshonly => true, - onlyif => "grep -E '^${title} *=' /etc/sysctl.conf", + onlyif => "grep -E '^${variable} *=' /etc/sysctl.conf", } # Enforce configured value during each run (can't work with custom files) if $enforce and ! ( $content or $source ) { - $qtitle = shellquote($title) + $qvariable = shellquote($variable) # Value may contain '|' and others, we need to quote to be safe # Convert any numerical to expected string, 0 instead of '0' would fail # lint:ignore:only_variable_string Convert numerical to string $qvalue = shellquote("${value}") # lint:endignore - exec { "enforce-sysctl-value-${qtitle}": - unless => "/usr/bin/test \"$(/sbin/sysctl -n ${qtitle})\" = ${qvalue}", - command => "/sbin/sysctl -w ${qtitle}=${qvalue}", + exec { "enforce-sysctl-value-${qvariable}": + command => "${sysctl_binary} -w ${qtitle}=${qvalue}", + path => [ '/usr/sbin', '/sbin', '/usr/bin', '/bin' ], + unless => "test \"$(${sysctl_binary} -n ${qvariable})\" = ${qvalue}", } } } else { - - # Absent - # We cannot restore values, since defaults can not be known... reboot :-/ - - file { "/etc/sysctl.d/${sysctl_d_file}": + # absent: cannot restore values, since defaults unknown... reboot :-/ + file { "${sysctl_dir_path}/${sysctl_d_file}": ensure => absent, } - } } diff --git a/manifests/init.pp b/manifests/init.pp index 5111e6e..c877991 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -1,13 +1,18 @@ class sysctl (Boolean $purge, - Hash $values, + Hash $values, Boolean $symlink99, + String $sysctl_binary, Boolean $sysctl_dir, - String $sysctl_dir_path, - String $sysctl_dir_owner, - String $sysctl_dir_group, - String $sysctl_dir_mode) { + String $sysctl_dir_path, + String $sysctl_dir_owner, + String $sysctl_dir_group, + String $sysctl_dir_mode) { - create_resources(sysctl::configuration, $values) + $defaults = { + sysctl_binary => $sysctl_binary, + sysctl_dir_path => $sysctl_dir_path, + } + create_resources(sysctl::configuration, $values, $defaults) if $sysctl_dir { # if we're purging we should also recurse From bbb0ae7d731c48a51a27319aa013ed29516946b4 Mon Sep 17 00:00:00 2001 From: Tom Downes Date: Tue, 2 Aug 2016 16:24:33 -0500 Subject: [PATCH 3/8] 1. refreshonly and onlyif don't work together! Remove the refreshonly, leave the notify (so that it runs after sysctl.d file is created) but rely on the onlyif to prevent unnecessary executions 2. Modify behavior of /etc/sysctl.conf management to REMOVE entries because we are managing them under /etc/sysctl.d. Current behavior is confusing because it will add a variable in two locations if that variable was previously present in /etc/sysctl.conf. 3. ensure that the file under /etc/sysctl.d gets created prior to running the enforcer. The enforcer should really only be running when a user has manually run sysctl Bla --- manifests/configuration.pp | 77 ++++++++++++++++++++++---------------- tests/base.pp | 1 - tests/configuration.pp | 9 +++++ tests/init.pp | 10 +---- 4 files changed, 54 insertions(+), 43 deletions(-) delete mode 100644 tests/base.pp create mode 100644 tests/configuration.pp diff --git a/manifests/configuration.pp b/manifests/configuration.pp index 1f6b9a5..eef0eb2 100644 --- a/manifests/configuration.pp +++ b/manifests/configuration.pp @@ -12,17 +12,17 @@ # Sample Usage : # sysctl::configuration { 'net.ipv6.bindv6only': value => '1' } define sysctl::configuration ( - String $variable = $title, - String $ensure = 'present', - String $value = undef, - String $prefix = undef, - String $suffix = '.conf', - String $comment = undef, - String $content = undef, - String $source = undef, - Boolean $enforce = true, - String $sysctl_binary, - String $sysctl_dir_path, + String $variable = $title, + String $ensure = 'present', + Variant[String,Undef] $value = undef, + Variant[String,Undef] $prefix = undef, + String $suffix = '.conf', + Variant[String,Undef] $comment = undef, + Variant[String,Undef] $content = undef, + Variant[String,Undef] $source = undef, + Boolean $enforce = true, + String $sysctl_binary = '/sbin/sysctl', + String $sysctl_dir_path = '/etc/sysctl.d', ) { # If we have a prefix, then add the dash to it @@ -49,6 +49,30 @@ # Present + # Enforce configured value during each run when set by value + # (doesn't work with custom files) + $enforcing = $enforce and ! ( $content or $source ) + if $enforcing { + $qvariable = shellquote($variable) + # Value may contain '|' and others, we need to quote to be safe + # Convert any numerical to expected string, 0 instead of '0' would fail + # lint:ignore:only_variable_string Convert numerical to string + $qvalue = shellquote("${value}") + # lint:endignore + exec { "enforce-sysctl-value-${qvariable}": + command => "${sysctl_binary} -w ${qvariable}=${qvalue}", + path => [ '/usr/sbin', '/sbin', '/usr/bin', '/bin' ], + unless => "test \"$(${sysctl_binary} -n ${qvariable})\" = ${qvalue}", + } + } + + # if we are enforcing values on every run, do so only if the file + # doesn't need updating + $before = $enforcing ? { + true => Exec["enforce-sysctl-value-${qvariable}"], + false => undef, + } + # Temporary file created and "sysctl -p filename" is run on that # If exit code is 0, the kernel setting has changed and puppet # copies the file to its permanent location. @@ -62,31 +86,18 @@ mode => '0644', content => $file_content, source => $file_source, - notify => Exec["update-sysctl.conf-${variable}"], + notify => Exec["remove-${variable}-from-sysctl.conf"], validate_cmd => "${sysctl_binary} -p %", + before => $before, } - # For the few original values from the main file - exec { "update-sysctl.conf-${variable}": - command => "sed -i -e 's#^${variable} *=.*#${variable} = ${value}#' /etc/sysctl.conf", - path => [ '/usr/sbin', '/sbin', '/usr/bin', '/bin' ], - refreshonly => true, - onlyif => "grep -E '^${variable} *=' /etc/sysctl.conf", - } - - # Enforce configured value during each run (can't work with custom files) - if $enforce and ! ( $content or $source ) { - $qvariable = shellquote($variable) - # Value may contain '|' and others, we need to quote to be safe - # Convert any numerical to expected string, 0 instead of '0' would fail - # lint:ignore:only_variable_string Convert numerical to string - $qvalue = shellquote("${value}") - # lint:endignore - exec { "enforce-sysctl-value-${qvariable}": - command => "${sysctl_binary} -w ${qtitle}=${qvalue}", - path => [ '/usr/sbin', '/sbin', '/usr/bin', '/bin' ], - unless => "test \"$(${sysctl_binary} -n ${qvariable})\" = ${qvalue}", - } + # Remove any entries from the main sysctl.conf because we have + # already set them in /etc/sysctl.d + exec { "remove-${variable}-from-sysctl.conf": + command => "sed -i -e '/^${variable} *=/d' /etc/sysctl.conf", + path => [ '/usr/sbin', '/sbin', '/usr/bin', '/bin' ], + onlyif => "grep -E '^${variable} *=' /etc/sysctl.conf", + require => File["${sysctl_dir_path}/${sysctl_d_file}"], } } else { diff --git a/tests/base.pp b/tests/base.pp deleted file mode 100644 index a486871..0000000 --- a/tests/base.pp +++ /dev/null @@ -1 +0,0 @@ -include sysctl::base diff --git a/tests/configuration.pp b/tests/configuration.pp new file mode 100644 index 0000000..dd1cf29 --- /dev/null +++ b/tests/configuration.pp @@ -0,0 +1,9 @@ +sysctl::configuration { 'net.ipv4.ip_forward': value => '1' } +sysctl::configuration { 'net.core.somaxconn': value => '65536' } +sysctl::configuration { 'vm.swappiness': ensure => absent } +sysctl::configuration { 'net.ipv4.conf.eth0/1.forwarding': value => 1 } +sysctl::configuration { 'kernel.domainname': value => "foobar.'backquote.com" } +sysctl::configuration { 'kernel.core_pattern': + value => '|/scripts/core-gzip.sh /var/tmp/core/core.%e.%p.%h.%t.gz', + comment => 'wrapper script to gzip core dumps', +} diff --git a/tests/init.pp b/tests/init.pp index 99a6e38..e472782 100644 --- a/tests/init.pp +++ b/tests/init.pp @@ -1,9 +1 @@ -sysctl { 'net.ipv4.ip_forward': value => '1' } -sysctl { 'net.core.somaxconn': value => '65536' } -sysctl { 'vm.swappiness': ensure => absent } -sysctl { 'net.ipv4.conf.eth0/1.forwarding': value => 1 } -sysctl { 'kernel.domainname': value => "foobar.'backquote.com" } -sysctl { 'kernel.core_pattern': - value => '|/scripts/core-gzip.sh /var/tmp/core/core.%e.%p.%h.%t.gz', - comment => 'wrapper script to gzip core dumps', -} +include sysctl From 91cdc9e3d70a3a9b0aa2c1c8f4dcc4cb140c0ed7 Mon Sep 17 00:00:00 2001 From: Tom Downes Date: Sat, 20 Aug 2016 11:10:32 -0500 Subject: [PATCH 4/8] Point to correct location of sysctl binary on RHEL6-derived platforms --- data/os/RedHat-6.yaml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 data/os/RedHat-6.yaml diff --git a/data/os/RedHat-6.yaml b/data/os/RedHat-6.yaml new file mode 100644 index 0000000..d0a4180 --- /dev/null +++ b/data/os/RedHat-6.yaml @@ -0,0 +1,2 @@ +--- +sysctl::sysctl_binary: '/sbin/sysctl' From 20c07f5b32390fc74f0771b0ed5464030b9c72fb Mon Sep 17 00:00:00 2001 From: Tom Downes Date: Sat, 20 Aug 2016 16:53:54 -0500 Subject: [PATCH 5/8] Make /sbin/sysctl the default path since it appears to be the location in all modern Debian/RHEL/FreeBSD --- data/default.yaml | 2 +- data/os/Debian-8.yaml | 2 -- data/os/Debian.yaml | 2 +- data/os/RedHat-6.yaml | 2 -- data/os/RedHat-7.yaml | 2 -- hiera.yaml | 3 --- 6 files changed, 2 insertions(+), 11 deletions(-) delete mode 100644 data/os/Debian-8.yaml delete mode 100644 data/os/RedHat-6.yaml delete mode 100644 data/os/RedHat-7.yaml diff --git a/data/default.yaml b/data/default.yaml index 94f00f1..44eb1dd 100644 --- a/data/default.yaml +++ b/data/default.yaml @@ -7,7 +7,7 @@ sysctl::purge: false sysctl::values: {} sysctl::symlink99: false -sysctl::sysctl_binary: '/usr/sbin/sysctl' +sysctl::sysctl_binary: '/sbin/sysctl' sysctl::sysctl_dir: true sysctl::sysctl_dir_path: '/etc/sysctl.d' sysctl::sysctl_dir_owner: root diff --git a/data/os/Debian-8.yaml b/data/os/Debian-8.yaml deleted file mode 100644 index ac051ce..0000000 --- a/data/os/Debian-8.yaml +++ /dev/null @@ -1,2 +0,0 @@ ---- -sysctl::symlink99: true diff --git a/data/os/Debian.yaml b/data/os/Debian.yaml index d0a4180..ac051ce 100644 --- a/data/os/Debian.yaml +++ b/data/os/Debian.yaml @@ -1,2 +1,2 @@ --- -sysctl::sysctl_binary: '/sbin/sysctl' +sysctl::symlink99: true diff --git a/data/os/RedHat-6.yaml b/data/os/RedHat-6.yaml deleted file mode 100644 index d0a4180..0000000 --- a/data/os/RedHat-6.yaml +++ /dev/null @@ -1,2 +0,0 @@ ---- -sysctl::sysctl_binary: '/sbin/sysctl' diff --git a/data/os/RedHat-7.yaml b/data/os/RedHat-7.yaml deleted file mode 100644 index ac051ce..0000000 --- a/data/os/RedHat-7.yaml +++ /dev/null @@ -1,2 +0,0 @@ ---- -sysctl::symlink99: true diff --git a/hiera.yaml b/hiera.yaml index 45cdeb5..9e0e414 100644 --- a/hiera.yaml +++ b/hiera.yaml @@ -5,8 +5,5 @@ hierarchy: - name: "OS family" backend: yaml path: "os/%{facts.os.family}" - - name: "OS family and release" - backend: yaml - path: "os/%{facts.os.family}-${facts.release.major}" - name: default backend: yaml From fe15776625a7d6630c6dd2399c3cbe1698688d19 Mon Sep 17 00:00:00 2001 From: Tom Downes Date: Thu, 22 Sep 2016 09:59:37 -0500 Subject: [PATCH 6/8] Use proper variable in sysctl.conf.d template Previous commits allow the user to define a sysctl resource title in the vernacular rather than compel them to set the title equal to the name of the kernel variables. Improves read-ability of logs. --- templates/sysctl.d-file.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/sysctl.d-file.erb b/templates/sysctl.d-file.erb index 0d4863c..d8a63d1 100644 --- a/templates/sysctl.d-file.erb +++ b/templates/sysctl.d-file.erb @@ -7,4 +7,4 @@ # <%= line.strip %> <% end -%> <% end -%> -<%= @title %> = <%= @value %> +<%= @variable %> = <%= @value %> From a944b2602fb7abccc608c491891fd5c756300caa Mon Sep 17 00:00:00 2001 From: Tom Downes Date: Thu, 6 Oct 2016 20:55:40 -0500 Subject: [PATCH 7/8] Improve sysctl enforcement so that it works for kernel variables that are multi-valued, so long as the user provides their value with a single space between the values. --- manifests/configuration.pp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/manifests/configuration.pp b/manifests/configuration.pp index eef0eb2..22de56d 100644 --- a/manifests/configuration.pp +++ b/manifests/configuration.pp @@ -57,12 +57,12 @@ # Value may contain '|' and others, we need to quote to be safe # Convert any numerical to expected string, 0 instead of '0' would fail # lint:ignore:only_variable_string Convert numerical to string - $qvalue = shellquote("${value}") + $qvalue = shellquote($value) # lint:endignore exec { "enforce-sysctl-value-${qvariable}": command => "${sysctl_binary} -w ${qvariable}=${qvalue}", path => [ '/usr/sbin', '/sbin', '/usr/bin', '/bin' ], - unless => "test \"$(${sysctl_binary} -n ${qvariable})\" = ${qvalue}", + unless => "test \"$(${sysctl_binary} -n ${qvariable} | sed \"s,[[:space:]]\+, ,g\")\" = ${qvalue}", } } From a6e61d4194e890e28a3f2ba5ad14a4a4a613d13c Mon Sep 17 00:00:00 2001 From: Tom Downes Date: Fri, 7 Oct 2016 08:36:35 -0500 Subject: [PATCH 8/8] Eliminate Unrecognized escape sequence warning in puppetserver logs --- manifests/configuration.pp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/manifests/configuration.pp b/manifests/configuration.pp index 22de56d..5025726 100644 --- a/manifests/configuration.pp +++ b/manifests/configuration.pp @@ -62,7 +62,7 @@ exec { "enforce-sysctl-value-${qvariable}": command => "${sysctl_binary} -w ${qvariable}=${qvalue}", path => [ '/usr/sbin', '/sbin', '/usr/bin', '/bin' ], - unless => "test \"$(${sysctl_binary} -n ${qvariable} | sed \"s,[[:space:]]\+, ,g\")\" = ${qvalue}", + unless => "test \"$(${sysctl_binary} -n ${qvariable} | sed \"s,[[:space:]]\\+, ,g\")\" = ${qvalue}", } }