From 52bc6f8dfc46974ca0096a4f4a704c0cbae2bbf4 Mon Sep 17 00:00:00 2001 From: Hunter Haugen Date: Mon, 7 Mar 2016 11:33:32 -0800 Subject: [PATCH] Remove inifile class, test, and dependency PR #156 merged a base class called `inifile` that dynamically queried hiera using explicit `hiera_hash()` functions and required functions from stdlib. It is an anti-pattern for component classes to contain hiera functions (even ones that do not trigger under all circumstances) as this leads to tight-coupling with hiera data structures. It also promotes the anti-pattern of declaring resources directly in hiera. Also, inifile is not going to receive a major version increase for a while (we hope) and so adding a dependency is not desirable at this point. Unfortunately this feedback was missed while reviewing the original PR, so it has to be reverted. --- .fixtures.yml | 4 - manifests/init.pp | 46 ----------- metadata.json | 1 - spec/classes/init_spec.rb | 166 -------------------------------------- 4 files changed, 217 deletions(-) delete mode 100644 manifests/init.pp delete mode 100644 spec/classes/init_spec.rb diff --git a/.fixtures.yml b/.fixtures.yml index 2d1290c..db98538 100644 --- a/.fixtures.yml +++ b/.fixtures.yml @@ -1,7 +1,3 @@ fixtures: - repositories: - stdlib: - repo: 'git://github.com/puppetlabs/puppetlabs-stdlib.git' - ref: '4.2.0' symlinks: inifile: "#{source_dir}" diff --git a/manifests/init.pp b/manifests/init.pp deleted file mode 100644 index 3590216..0000000 --- a/manifests/init.pp +++ /dev/null @@ -1,46 +0,0 @@ -# == Class: inifile -# -# Use create_resources() to allow the specification of ini_setting and -# ini_subsetting entries. -# -class inifile ( - $ini_settings = undef, - $ini_subsettings = undef, - $ini_settings_hiera_merge = true, - $ini_subsettings_hiera_merge = true, -) { - - if is_string($ini_settings_hiera_merge) == true { - $ini_settings_hiera_merge_bool = str2bool($ini_settings_hiera_merge) - } else { - $ini_settings_hiera_merge_bool = $ini_settings_hiera_merge - } - validate_bool($ini_settings_hiera_merge_bool) - - if is_string($ini_subsettings_hiera_merge) == true { - $ini_subsettings_hiera_merge_bool = str2bool($ini_subsettings_hiera_merge) - } else { - $ini_subsettings_hiera_merge_bool = $ini_subsettings_hiera_merge - } - validate_bool($ini_subsettings_hiera_merge_bool) - - if $ini_settings != undef { - if $ini_settings_hiera_merge_bool == true { - $ini_settings_real = hiera_hash('inifile::ini_settings') - } else { - $ini_settings_real = $ini_settings - } - validate_hash($ini_settings_real) - create_resources('ini_setting',$ini_settings_real) - } - - if $ini_subsettings != undef { - if $ini_subsettings_hiera_merge_bool == true { - $ini_subsettings_real = hiera_hash('inifile::ini_subsettings') - } else { - $ini_subsettings_real = $ini_subsettings - } - validate_hash($ini_subsettings_real) - create_resources('ini_subsetting',$ini_subsettings_real) - } -} diff --git a/metadata.json b/metadata.json index f14fd57..200673e 100644 --- a/metadata.json +++ b/metadata.json @@ -8,7 +8,6 @@ "project_page": "https://github.com/puppetlabs/puppetlabs-inifile", "issues_url": "https://tickets.puppetlabs.com/browse/MODULES", "dependencies": [ - {"name":"puppetlabs/stdlib","version_requirement":">= 4.2.0 < 5.0.0"} ], "data_provider": null, "operatingsystem_support": [ diff --git a/spec/classes/init_spec.rb b/spec/classes/init_spec.rb deleted file mode 100644 index 14faf53..0000000 --- a/spec/classes/init_spec.rb +++ /dev/null @@ -1,166 +0,0 @@ -require 'spec_helper' -describe 'inifile' do - - describe 'with default options' do - it { should compile.with_all_deps } - it { should contain_class('inifile') } - end - - describe 'with parameter ini_settings_hiera_merge' do - context 'set to an invalid type (non-string and a non-boolean)' do - let(:params) { { :ini_settings_hiera_merge => ['invalid','type'] } } - - it 'should fail' do - expect { - should contain_class('inifile') - }.to raise_error(Puppet::Error,/\["invalid", "type"\] is not a boolean./) - end - end - - ['true',true,'false',false].each do |value| - context "set to #{value}" do - let(:params) { { :ini_settings_hiera_merge => value } } - - it { should contain_class('inifile') } - end - end - end - - describe 'with parameter ini_settings' do - context 'set to an invalid type (non-hash)' do - let(:params) do - { - :ini_settings => ['invalid','type'], - :ini_settings_hiera_merge => false, - } - end - - it 'should fail' do - expect { - should contain_class('inifile') - }.to raise_error(Puppet::Error,/\["invalid", "type"\] is not a Hash./) - end - end - - context 'set to a valid hash' do - let(:params) { { :ini_settings_hiera_merge => false, - :ini_settings => { - 'sample setting' => { - 'ensure' => 'absent', - 'path' => '/tmp/foo.ini', - 'section' => 'foo', - 'setting' => 'foosetting', - 'value' => 'FOO!', - }, - 'colorize_git' => { - 'ensure' => 'present', - 'path' => '/root/.gitconfig', - 'section' => 'color', - 'setting' => 'ui', - 'value' => 'auto', - } - } } } - - it { should contain_class('inifile') } - - it { - should contain_ini_setting('sample setting').with({ - 'ensure' => 'absent', - 'path' => '/tmp/foo.ini', - 'section' => 'foo', - 'setting' => 'foosetting', - 'value' => 'FOO!', - }) - } - - it { - should contain_ini_setting('colorize_git').with({ - 'ensure' => 'present', - 'path' => '/root/.gitconfig', - 'section' => 'color', - 'setting' => 'ui', - 'value' => 'auto', - }) - } - end - end - - describe 'with parameter ini_subsettings_hiera_merge' do - context 'set to an invalid type (non-string and a non-boolean)' do - let(:params) { { :ini_subsettings_hiera_merge => ['invalid','type'] } } - - it 'should fail' do - expect { - should contain_class('inifile') - }.to raise_error(Puppet::Error,/\["invalid", "type"\] is not a boolean./) - end - end - - ['true',true,'false',false].each do |value| - context "set to #{value}" do - let(:params) { { :ini_subsettings_hiera_merge => value } } - - it { should contain_class('inifile') } - end - end - end - - describe 'with parameter ini_subsettings' do - context 'set to an invalid type (non-hash)' do - let(:params) do - { - :ini_subsettings => ['invalid','type'], - :ini_subsettings_hiera_merge => false, - } - end - - it 'should fail' do - expect { - should contain_class('inifile') - }.to raise_error(Puppet::Error,/\["invalid", "type"\] is not a Hash./) - end - end - - context 'set to a valid hash' do - let(:params) { { :ini_subsettings_hiera_merge => false, - :ini_subsettings => { - 'sample setting' => { - 'ensure' => 'absent', - 'path' => '/tmp/foo.ini', - 'section' => 'foo', - 'setting' => 'foosetting', - 'value' => 'FOO!', - }, - 'colorize_git' => { - 'ensure' => 'present', - 'path' => '/root/.gitconfig', - 'section' => 'color', - 'setting' => 'ui', - 'value' => 'auto', - } - } } } - - it { should contain_class('inifile') } - - it { - should contain_ini_subsetting('sample setting').with({ - 'ensure' => 'absent', - 'path' => '/tmp/foo.ini', - 'section' => 'foo', - 'setting' => 'foosetting', - 'value' => 'FOO!', - }) - } - - it { - should contain_ini_subsetting('colorize_git').with({ - 'ensure' => 'present', - 'path' => '/root/.gitconfig', - 'section' => 'color', - 'setting' => 'ui', - 'value' => 'auto', - }) - } - end - end -end