From 16e35cc4e0e95e1446882c385b79c31912123690 Mon Sep 17 00:00:00 2001 From: Phil Friderici Date: Thu, 3 Aug 2017 08:36:31 +0000 Subject: [PATCH] (GH-786) sensu::plugin does not work on windows without specifying install_path Add Linux/Windows sensitive default values for sensu::plugin::install_path. Adjust calls of sensu::plugin to not set install_path anymore. --- manifests/init.pp | 3 +- manifests/plugin.pp | 5 +- spec/classes/sensu_init_spec.rb | 6 +- spec/defines/sensu_plugin_spec.rb | 183 ++++++++++++++++++++++++++---- 4 files changed, 167 insertions(+), 30 deletions(-) diff --git a/manifests/init.pp b/manifests/init.pp index ba757293cb..3fa9d76c7a 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -303,7 +303,6 @@ # # [*plugins*] # String, Array of strings, Hash. Plugins to install on the node -# Strings and Arrays of strings will set 'install_path' => '/etc/sensu/plugins' as default. # Default: [] # Example string: 'puppet:///data/sensu/plugins/plugin1.rb' # Example array: [ 'puppet:///data/sensu/plugins/plugin1.rb', 'puppet:///data/sensu/plugins/plugin2.rb' ] @@ -743,7 +742,7 @@ sensu::plugin { $plugins_dir: type => 'directory' } } else { case $plugins { - String,Array: { sensu::plugin { $plugins: install_path => '/etc/sensu/plugins' } } + String,Array: { sensu::plugin { $plugins: } } Hash: { create_resources('::sensu::plugin', $plugins, $plugins_defaults ) } default: { fail('Invalid data type for $plugins, must be a string, an array, or a hash.') } } diff --git a/manifests/plugin.pp b/manifests/plugin.pp index 6a2ab5adca..d1eea3b916 100644 --- a/manifests/plugin.pp +++ b/manifests/plugin.pp @@ -53,7 +53,10 @@ # define sensu::plugin ( Enum['file','url','package','directory'] $type = 'file', - String $install_path = '/etc/sensu/plugins', + Stdlib::Absolutepath $install_path = $::osfamily ? { + 'windows' => 'C:/opt/sensu/plugins', + default => '/etc/sensu/plugins', + }, Boolean $purge = true, Boolean $recurse = true, Boolean $force = true, diff --git a/spec/classes/sensu_init_spec.rb b/spec/classes/sensu_init_spec.rb index 4188dbaf7d..fdd2a84c77 100644 --- a/spec/classes/sensu_init_spec.rb +++ b/spec/classes/sensu_init_spec.rb @@ -387,13 +387,13 @@ context 'with plugins => puppet:///data/sensu/plugins/teststring.rb' do let(:params) { {:plugins => 'puppet:///data/sensu/plugins/teststring.rb' } } - it { should contain_sensu__plugin('puppet:///data/sensu/plugins/teststring.rb').with_install_path('/etc/sensu/plugins') } + it { should contain_sensu__plugin('puppet:///data/sensu/plugins/teststring.rb') } end context 'with plugins => [ puppet:///test/array1.rb, puppet:///test/array2.rb ]' do let(:params) { {:plugins => [ 'puppet:///test/array1.rb', 'puppet:///test/array2.rb' ] } } - it { should contain_sensu__plugin('puppet:///test/array1.rb').with_install_path('/etc/sensu/plugins') } - it { should contain_sensu__plugin('puppet:///test/array2.rb').with_install_path('/etc/sensu/plugins') } + it { should contain_sensu__plugin('puppet:///test/array1.rb') } + it { should contain_sensu__plugin('puppet:///test/array2.rb') } end context 'with plugins set to a valid hash containing two entries with different options' do diff --git a/spec/defines/sensu_plugin_spec.rb b/spec/defines/sensu_plugin_spec.rb index 032f15f552..a4b9393ba0 100644 --- a/spec/defines/sensu_plugin_spec.rb +++ b/spec/defines/sensu_plugin_spec.rb @@ -12,10 +12,39 @@ class { '::sensu': context 'file' do let(:title) { 'puppet:///data/plug1' } - context 'defaults' do - it { should contain_file('/etc/sensu/plugins/plug1').with( - :source => 'puppet:///data/plug1' - ) } + context 'running on Linux' do + context 'defaults' do + it do + should contain_sensu__plugins_dir('puppet:///data/plug1-/etc/sensu/plugins').with({ + :path => '/etc/sensu/plugins', + }) + end + + it { should contain_file('/etc/sensu/plugins/plug1').with( + :source => 'puppet:///data/plug1' + ) } + + end + end + + context 'running on Windows' do + let(:facts) do + { + :osfamily => 'windows', + :os => { :release => { :major => '2012 R2' }}, # needed for sensu::package + } + end + context 'defaults' do + it do + should contain_sensu__plugins_dir('puppet:///data/plug1-C:/opt/sensu/plugins').with({ + :path => 'C:/opt/sensu/plugins', + }) + end + + it { should contain_file('C:/opt/sensu/plugins/plug1').with( + :source => 'puppet:///data/plug1' + ) } + end end context 'setting params' do @@ -31,18 +60,62 @@ class { '::sensu': context 'url' do let(:title) { 'https://raw.githubusercontent.com/sensu/sensu-community-plugins/master/plugins/system/check-mem.sh' } - - context 'defaults' do - let(:params) { { + let(:params) do + { :type => 'url', - :pkg_checksum => '1d58b78e9785f893889458f8e9fe8627' - } } + :pkg_checksum => '1d58b78e9785f893889458f8e9fe8627', + } + end - it { should contain_remote_file('https://raw.githubusercontent.com/sensu/sensu-community-plugins/master/plugins/system/check-mem.sh').with( - :ensure => 'present', - :path => '/etc/sensu/plugins/check-mem.sh', - :checksum => '1d58b78e9785f893889458f8e9fe8627' - ) } + context 'running on Linux' do + context 'defaults' do + it do + should contain_sensu__plugins_dir('https://raw.githubusercontent.com/sensu/sensu-community-plugins/master/plugins/system/check-mem.sh-/etc/sensu/plugins').with({ + :path => '/etc/sensu/plugins', + }) + end + + it { should contain_remote_file('https://raw.githubusercontent.com/sensu/sensu-community-plugins/master/plugins/system/check-mem.sh').with( + :ensure => 'present', + :path => '/etc/sensu/plugins/check-mem.sh', + :checksum => '1d58b78e9785f893889458f8e9fe8627' + ) } + + it do + should contain_file('/etc/sensu/plugins/check-mem.sh').with({ + :require => [ 'File[/etc/sensu/plugins]', 'Remote_file[https://raw.githubusercontent.com/sensu/sensu-community-plugins/master/plugins/system/check-mem.sh]', ], + }) + end + end + end + + context 'running on Windows' do + let(:facts) do + { + :osfamily => 'windows', + :os => { :release => { :major => '2012 R2' }}, # needed for sensu::package + } + end + + context 'defaults' do + it do + should contain_sensu__plugins_dir('https://raw.githubusercontent.com/sensu/sensu-community-plugins/master/plugins/system/check-mem.sh-C:/opt/sensu/plugins').with({ + :path => 'C:/opt/sensu/plugins', + }) + end + + it { should contain_remote_file('https://raw.githubusercontent.com/sensu/sensu-community-plugins/master/plugins/system/check-mem.sh').with( + :ensure => 'present', + :path => 'C:/opt/sensu/plugins/check-mem.sh', + :checksum => '1d58b78e9785f893889458f8e9fe8627' + ) } + + it do + should contain_file('C:/opt/sensu/plugins/check-mem.sh').with({ + :require => [ 'File[C:/opt/sensu/plugins]', 'Remote_file[https://raw.githubusercontent.com/sensu/sensu-community-plugins/master/plugins/system/check-mem.sh]', ], + }) + end + end end context 'setting params' do @@ -76,18 +149,45 @@ class { '::sensu': context 'directory' do let(:title) { 'puppet:///data/sensu/plugins' } + let(:params) { { :type => 'directory' } } + + context 'running on Linux' do + context 'defaults' do + it do + should contain_file('/etc/sensu/plugins_for_plugin_puppet:///data/sensu/plugins').with({ + 'ensure' => 'directory', + 'path' => '/etc/sensu/plugins', + 'mode' => '0555', + 'source' => 'puppet:///data/sensu/plugins', + 'recurse' => 'true', + 'purge' => 'true', + 'force' => 'true', + }) + end + end + end - context 'defaults' do - let(:params) { { :type => 'directory' } } + context 'running on Windows' do + let(:facts) do + { + :osfamily => 'windows', + :os => { :release => { :major => '2012 R2' }}, # needed for sensu::package + } + end - it { should contain_file('/etc/sensu/plugins_for_plugin_puppet:///data/sensu/plugins').with( - 'source' => 'puppet:///data/sensu/plugins', - 'path' => '/etc/sensu/plugins', - 'ensure' => 'directory', - 'recurse' => 'true', - 'force' => 'true', - 'purge' => 'true' - ) } + context 'defaults' do + it do + should contain_file('C:/opt/sensu/plugins_for_plugin_puppet:///data/sensu/plugins').with({ + 'ensure' => 'directory', + 'path' => 'C:/opt/sensu/plugins', + 'mode' => '0555', + 'source' => 'puppet:///data/sensu/plugins', + 'recurse' => 'true', + 'purge' => 'true', + 'force' => 'true', + }) + end + end end context 'set install_path' do @@ -187,4 +287,39 @@ class { '::sensu': it { is_expected.to contain_sensu__plugin(title).with(expected)} end end + + describe 'variable type and content validations' do + let(:title) { 'puppet:///data/plug1' } + mandatory_params = {} + + validations = { + 'absolute_path' => { + :name => %w[install_path], + :valid => %w[/absolute/filepath /absolute/directory/], + :invalid => ['./relative/path', %w(array), { 'ha' => 'sh' }, 3, 2.42, true, nil], + :message => 'Evaluation Error: Error while evaluating a Resource Statement', + }, + } + + validations.sort.each do |type, var| + var[:name].each do |var_name| + var[:params] = {} if var[:params].nil? + var[:valid].each do |valid| + context "when #{var_name} (#{type}) is set to valid #{valid} (as #{valid.class})" do + let(:params) { [mandatory_params, var[:params], { :"#{var_name}" => valid, }].reduce(:merge) } + it { should compile } + end + end + + var[:invalid].each do |invalid| + context "when #{var_name} (#{type}) is set to invalid #{invalid} (as #{invalid.class})" do + let(:params) { [mandatory_params, var[:params], { :"#{var_name}" => invalid, }].reduce(:merge) } + it 'should fail' do + expect { should contain_class(subject) }.to raise_error(Puppet::PreformattedError, /#{var[:message]}/) + end + end + end + end # var[:name].each + end # validations.sort.each + end # describe 'variable type and content validations' end