From 32a62f9a177d2d79b42fe8f4e496ecbb437f3363 Mon Sep 17 00:00:00 2001 From: David Swan Date: Fri, 7 Dec 2018 13:55:27 +0000 Subject: [PATCH 1/2] (MODULES-8193) - Removal of inbuilt deepmerge and dirname functions Both functions have been removed from the code and their function calls within the manifest's updated as they already exist within puppet itself, in the case of deepmerge and within stdlib in the case of dirname. --- lib/puppet/functions/mysql/deepmerge.rb | 66 ------------- lib/puppet/functions/mysql/dirname.rb | 20 ---- .../parser/functions/mysql_deepmerge.rb | 58 ------------ lib/puppet/parser/functions/mysql_dirname.rb | 21 ----- manifests/backup/mysqlbackup.pp | 2 +- manifests/server.pp | 2 +- manifests/server/binarylog.pp | 2 +- manifests/server/config.pp | 6 +- spec/functions/mysql_deepmerge_spec.rb | 94 ------------------- spec/functions/mysql_dirname_spec.rb | 19 ---- 10 files changed, 6 insertions(+), 284 deletions(-) delete mode 100644 lib/puppet/functions/mysql/deepmerge.rb delete mode 100644 lib/puppet/functions/mysql/dirname.rb delete mode 100644 lib/puppet/parser/functions/mysql_deepmerge.rb delete mode 100644 lib/puppet/parser/functions/mysql_dirname.rb delete mode 100644 spec/functions/mysql_deepmerge_spec.rb delete mode 100644 spec/functions/mysql_dirname_spec.rb diff --git a/lib/puppet/functions/mysql/deepmerge.rb b/lib/puppet/functions/mysql/deepmerge.rb deleted file mode 100644 index 0dbc995ba..000000000 --- a/lib/puppet/functions/mysql/deepmerge.rb +++ /dev/null @@ -1,66 +0,0 @@ -# @summary Recursively merges two or more hashes together and returns the resulting hash. -# -# @example -# $hash1 = {'one' => 1, 'two' => 2, 'three' => { 'four' => 4 } } -# $hash2 = {'two' => 'dos', 'three' => { 'five' => 5 } } -# $merged_hash = mysql::deepmerge($hash1, $hash2) -# # The resulting hash is equivalent to: -# # $merged_hash = { 'one' => 1, 'two' => 'dos', 'three' => { 'four' => 4, 'five' => 5 } } -# -# - When there is a duplicate key that is a hash, they are recursively merged. -# - When there is a duplicate key that is not a hash, the key in the rightmost hash will "win." -# - When there are conficting uses of dashes and underscores in two keys (which mysql would otherwise equate), the rightmost style will win. -# -Puppet::Functions.create_function(:'mysql::deepmerge') do - def deepmerge(*args) - if args.length < 2 - raise Puppet::ParseError, _('mysql_deepmerge(): wrong number of arguments (%{args_length}; must be at least 2)') % { args_length: args.length } - end - - result = {} - args.each do |arg| - next if arg.is_a?(String) && arg.empty? # empty string is synonym for puppet's undef - # If the argument was not a hash, skip it. - unless arg.is_a?(Hash) - raise Puppet::ParseError, _('mysql_deepmerge: unexpected argument type %{arg_class}, only expects hash arguments.') % { args_class: args.class } - end - - # We need to make a copy of the hash since it is frozen by puppet - current = deep_copy(arg) - - # Now we have to traverse our hash assigning our non-hash values - # to the matching keys in our result while following our hash values - # and repeating the process. - overlay(result, current) - end - result - end - - def normalized?(hash, key) - return true if hash.key?(key) - return false unless key =~ %r{-|_} - other_key = key.include?('-') ? key.tr('-', '_') : key.tr('_', '-') - return false unless hash.key?(other_key) - hash[key] = hash.delete(other_key) - true - end - - def overlay(hash1, hash2) - hash2.each do |key, value| - if normalized?(hash1, key) && value.is_a?(Hash) && hash1[key].is_a?(Hash) - overlay(hash1[key], value) - else - hash1[key] = value - end - end - end - - def deep_copy(inputhash) - return inputhash unless inputhash.is_a? Hash - hash = {} - inputhash.each do |k, v| - hash.store(k, deep_copy(v)) - end - hash - end -end diff --git a/lib/puppet/functions/mysql/dirname.rb b/lib/puppet/functions/mysql/dirname.rb deleted file mode 100644 index 4074a4241..000000000 --- a/lib/puppet/functions/mysql/dirname.rb +++ /dev/null @@ -1,20 +0,0 @@ -# @summary -# Returns the dirname of a path -# -Puppet::Functions.create_function(:'mysql::dirname') do - # @param path - # Path to find the dirname for. - # - # @return - # Directory name of path. - # - dispatch :dirname do - required_param 'Variant[String, Undef]', :path - return_type 'String' - end - - def dirname(path) - return '' if path.nil? - File.dirname(path) - end -end diff --git a/lib/puppet/parser/functions/mysql_deepmerge.rb b/lib/puppet/parser/functions/mysql_deepmerge.rb deleted file mode 100644 index 766028e89..000000000 --- a/lib/puppet/parser/functions/mysql_deepmerge.rb +++ /dev/null @@ -1,58 +0,0 @@ -module Puppet::Parser::Functions - newfunction(:mysql_deepmerge, type: :rvalue, doc: <<-'ENDHEREDOC') do |args| - @summary Recursively merges two or more hashes together and returns the resulting hash. - - @example - $hash1 = {'one' => 1, 'two' => 2, 'three' => { 'four' => 4 } } - $hash2 = {'two' => 'dos', 'three' => { 'five' => 5 } } - $merged_hash = mysql_deepmerge($hash1, $hash2) - # The resulting hash is equivalent to: - # $merged_hash = { 'one' => 1, 'two' => 'dos', 'three' => { 'four' => 4, 'five' => 5 } } - - - When there is a duplicate key that is a hash, they are recursively merged. - - When there is a duplicate key that is not a hash, the key in the rightmost hash will "win." - - When there are conficting uses of dashes and underscores in two keys (which mysql would otherwise equate), - the rightmost style will win. - - @return [Hash] - ENDHEREDOC - - if args.length < 2 - raise Puppet::ParseError, _('mysql_deepmerge(): wrong number of arguments (%{args_length}; must be at least 2)') % { args_length: args.length } - end - - result = {} - args.each do |arg| - next if arg.is_a?(String) && arg.empty? # empty string is synonym for puppet's undef - # If the argument was not a hash, skip it. - unless arg.is_a?(Hash) - raise Puppet::ParseError, _('mysql_deepmerge: unexpected argument type %{arg_class}, only expects hash arguments.') % { args_class: args.class } - end - - # Now we have to traverse our hash assigning our non-hash values - # to the matching keys in our result while following our hash values - # and repeating the process. - overlay(result, arg) - end - return(result) - end -end - -def normalized?(hash, key) - return true if hash.key?(key) - return false unless key =~ %r{-|_} - other_key = key.include?('-') ? key.tr('-', '_') : key.tr('_', '-') - return false unless hash.key?(other_key) - hash[key] = hash.delete(other_key) - true -end - -def overlay(hash1, hash2) - hash2.each do |key, value| - if normalized?(hash1, key) && value.is_a?(Hash) && hash1[key].is_a?(Hash) - overlay(hash1[key], value) - else - hash1[key] = value - end - end -end diff --git a/lib/puppet/parser/functions/mysql_dirname.rb b/lib/puppet/parser/functions/mysql_dirname.rb deleted file mode 100644 index 40a8ad64f..000000000 --- a/lib/puppet/parser/functions/mysql_dirname.rb +++ /dev/null @@ -1,21 +0,0 @@ -module Puppet::Parser::Functions - newfunction(:mysql_dirname, type: :rvalue, doc: <<-EOS - @summary - Returns the dirname of a path - - @param [String] path - Path to find the dirname for. - - @return [String] - Directory name of path. - EOS - ) do |arguments| - - if arguments.empty? - raise Puppet::ParseError, _('mysql_dirname(): Wrong number of arguments given (%{args_length} for 1)') % { args_length: args.length } - end - - path = arguments[0] - return File.dirname(path) - end -end diff --git a/manifests/backup/mysqlbackup.pp b/manifests/backup/mysqlbackup.pp index adcb99e2e..e2f1977cb 100644 --- a/manifests/backup/mysqlbackup.pp +++ b/manifests/backup/mysqlbackup.pp @@ -92,7 +92,7 @@ 'password' => $backuppassword, } } - $options = mysql::deepmerge($default_options, $mysql::server::override_options) + $options = $default_options.deep_merge($mysql::server::override_options) file { 'mysqlbackup-config-file': path => '/etc/mysql/conf.d/meb.cnf', diff --git a/manifests/server.pp b/manifests/server.pp index 4e9ac6be3..da86f81a7 100644 --- a/manifests/server.pp +++ b/manifests/server.pp @@ -116,7 +116,7 @@ } # Create a merged together set of options. Rightmost hashes win over left. - $options = mysql::deepmerge($mysql::params::default_options, $override_options) + $options = $mysql::params::default_options.deep_merge($override_options) Class['mysql::server::root_password'] -> Mysql::Db <| |> diff --git a/manifests/server/binarylog.pp b/manifests/server/binarylog.pp index 68d5c09e1..74214f2cd 100644 --- a/manifests/server/binarylog.pp +++ b/manifests/server/binarylog.pp @@ -11,7 +11,7 @@ $logbin = pick($options['mysqld']['log-bin'], $options['mysqld']['log_bin'], false) if $logbin { - $logbindir = mysql::dirname($logbin) + $logbindir = dirname($logbin) #Stop puppet from managing directory if just a filename/prefix is specified if $logbindir != '.' { diff --git a/manifests/server/config.pp b/manifests/server/config.pp index 6c0b7f017..6294a6788 100644 --- a/manifests/server/config.pp +++ b/manifests/server/config.pp @@ -24,7 +24,7 @@ # on some systems this is /etc/my.cnf.d, while Debian has /etc/mysql/conf.d and FreeBSD something in /usr/local. For the latter systems, # managing this basedir is also required, to have it available before the package is installed. - $includeparentdir = mysql::dirname($includedir) + $includeparentdir = dirname($includedir) if $includeparentdir != '/' and $includeparentdir != '/etc' { file { $includeparentdir: ensure => directory, @@ -43,9 +43,9 @@ # on mariadb systems, $includedir is not defined, but /etc/my.cnf.d has # to be managed to place the server.cnf there - $configparentdir = mysql::dirname($mysql::server::config_file) + $configparentdir = dirname($mysql::server::config_file) if $configparentdir != '/' and $configparentdir != '/etc' and $configparentdir - != $includedir and $configparentdir != mysql::dirname($includedir) { + != $includedir and $configparentdir != dirname($includedir) { file { $configparentdir: ensure => directory, mode => '0755', diff --git a/spec/functions/mysql_deepmerge_spec.rb b/spec/functions/mysql_deepmerge_spec.rb deleted file mode 100644 index e5e9a66ef..000000000 --- a/spec/functions/mysql_deepmerge_spec.rb +++ /dev/null @@ -1,94 +0,0 @@ -#! /usr/bin/env ruby -S rspec # rubocop:disable Lint/ScriptPermission - -require 'spec_helper' - -describe 'mysql::deepmerge' do - it 'exists' do - is_expected.not_to eq(nil) - end - - it 'throws error with no arguments' do - is_expected.to run.with_params.and_raise_error(Puppet::ParseError) - end - - it 'throws error with only one argument' do - is_expected.to run.with_params('one' => 1).and_raise_error(Puppet::ParseError) - end - - it 'accepts empty strings as puppet undef' do - is_expected.to run.with_params({}, '') - end - - # rubocop:disable RSpec/NamedSubject - index_values = ['one', 'two', 'three'] - expected_values_one = ['1', '2', '2'] - it 'is able to mysql_deepmerge two hashes' do - new_hash = subject.execute({ 'one' => '1', 'two' => '1' }, 'two' => '2', 'three' => '2') - index_values.each_with_index do |index, expected| - expect(new_hash[index]).to eq(expected_values_one[expected]) - end - end - - it 'mysql_deepmerges multiple hashes' do - hash = subject.execute({ 'one' => 1 }, { 'one' => '2' }, 'one' => '3') - expect(hash['one']).to eq('3') - end - - it 'accepts empty hashes' do - is_expected.to run.with_params({}, {}, {}).and_return({}) - end - - expected_values_two = [1, 2, 'four' => 4] - it 'mysql_deepmerges subhashes' do - hash = subject.execute({ 'one' => 1 }, 'two' => 2, 'three' => { 'four' => 4 }) - index_values.each_with_index do |index, expected| - expect(hash[index]).to eq(expected_values_two[expected]) - end - end - - it 'appends to subhashes' do - hash = subject.execute({ 'one' => { 'two' => 2 } }, 'one' => { 'three' => 3 }) - expect(hash['one']).to eq('two' => 2, 'three' => 3) - end - - expected_values_three = [1, 'dos', { 'four' => 4, 'five' => 5 }] - it 'appends to subhashes 2' do - hash = subject.execute({ 'one' => 1, 'two' => 2, 'three' => { 'four' => 4 } }, 'two' => 'dos', 'three' => { 'five' => 5 }) - index_values.each_with_index do |index, expected| - expect(hash[index]).to eq(expected_values_three[expected]) - end - end - - index_values_two = ['key1', 'key2'] - expected_values_four = [{ 'a' => 1, 'b' => 99 }, 'c' => 3] - it 'appends to subhashes 3' do - hash = subject.execute({ 'key1' => { 'a' => 1, 'b' => 2 }, 'key2' => { 'c' => 3 } }, 'key1' => { 'b' => 99 }) - index_values_two.each_with_index do |index, expected| - expect(hash[index]).to eq(expected_values_four[expected]) - end - end - - it 'equates keys mod dash and underscore #value' do - hash = subject.execute({ 'a-b-c' => 1 }, 'a_b_c' => 10) - expect(hash['a_b_c']).to eq(10) - end - it 'equates keys mod dash and underscore #not' do - hash = subject.execute({ 'a-b-c' => 1 }, 'a_b_c' => 10) - expect(hash).not_to have_key('a-b-c') - end - - index_values_three = ['a_b_c', 'b-c-d'] - expected_values_five = [10, { 'e-f-g' => 3, 'c_d_e' => 12 }] - index_values_error = ['a-b-c', 'b_c_d'] - index_values_three.each_with_index do |index, expected| - it 'keeps style of the last when keys are equal mod dash and underscore #value' do - hash = subject.execute({ 'a-b-c' => 1, 'b_c_d' => { 'c-d-e' => 2, 'e-f-g' => 3 } }, 'a_b_c' => 10, 'b-c-d' => { 'c_d_e' => 12 }) - expect(hash[index]).to eq(expected_values_five[expected]) - end - it 'keeps style of the last when keys are equal mod dash and underscore #not' do - hash = subject.execute({ 'a-b-c' => 1, 'b_c_d' => { 'c-d-e' => 2, 'e-f-g' => 3 } }, 'a_b_c' => 10, 'b-c-d' => { 'c_d_e' => 12 }) - expect(hash).not_to have_key(index_values_error[expected]) - end - end - # rubocop:enable RSpec/NamedSubject -end diff --git a/spec/functions/mysql_dirname_spec.rb b/spec/functions/mysql_dirname_spec.rb deleted file mode 100644 index 3e407ec0c..000000000 --- a/spec/functions/mysql_dirname_spec.rb +++ /dev/null @@ -1,19 +0,0 @@ -require 'spec_helper' - -describe 'mysql::dirname' do - it 'exists' do - is_expected.not_to eq(nil) - end - - it 'raises a ArgumentError if there is less than 1 arguments' do - is_expected.to run.with_params.and_raise_error(ArgumentError) - end - - it 'raises a ArgumentError if there is more than 1 arguments' do - is_expected.to run.with_params('foo', 'bar').and_raise_error(ArgumentError) - end - - it 'returns path of file' do - is_expected.to run.with_params('spec/functions/mysql_dirname_spec.rb').and_return('spec/functions') - end -end From 599947f54ff27a809032be950e7e111eda28a277 Mon Sep 17 00:00:00 2001 From: David Swan Date: Fri, 7 Dec 2018 16:36:35 +0000 Subject: [PATCH 2/2] =?UTF-8?q?Fix=20for=20failing=20test=E2=80=99s=20Remo?= =?UTF-8?q?ved=20a=20test=20file=20that=20was=20missed=20by=20my=20first?= =?UTF-8?q?=20sweep=20Updated=20if=20statement=20to=20account=20for=20diff?= =?UTF-8?q?erences=20between=20the=20now=20removed=20built=20in=20dirname?= =?UTF-8?q?=20function=20and=20the=20stdlib=20version?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- manifests/server/config.pp | 15 ++- .../puppet/functions/mysql_deepmerge_spec.rb | 102 ------------------ 2 files changed, 10 insertions(+), 107 deletions(-) delete mode 100644 spec/unit/puppet/functions/mysql_deepmerge_spec.rb diff --git a/manifests/server/config.pp b/manifests/server/config.pp index 6294a6788..3dbef24a0 100644 --- a/manifests/server/config.pp +++ b/manifests/server/config.pp @@ -44,11 +44,16 @@ # on mariadb systems, $includedir is not defined, but /etc/my.cnf.d has # to be managed to place the server.cnf there $configparentdir = dirname($mysql::server::config_file) - if $configparentdir != '/' and $configparentdir != '/etc' and $configparentdir - != $includedir and $configparentdir != dirname($includedir) { - file { $configparentdir: - ensure => directory, - mode => '0755', + # Before setting $configparentdir we first check to make sure that it's value is valid + if $configparentdir != '/' and $configparentdir != '/etc' { + # We then check that the value of $includedir is either undefined or that different from $configparentdir + # We first check that it is undefined due to dirname throwing an error when given undef/empty strings + if $includedir == undef or $includedir == '' or + ($configparentdir != $includedir and $configparentdir != dirname($includedir)) { + file { $configparentdir: + ensure => directory, + mode => '0755', + } } } } diff --git a/spec/unit/puppet/functions/mysql_deepmerge_spec.rb b/spec/unit/puppet/functions/mysql_deepmerge_spec.rb deleted file mode 100644 index 0d86b9628..000000000 --- a/spec/unit/puppet/functions/mysql_deepmerge_spec.rb +++ /dev/null @@ -1,102 +0,0 @@ -#! /usr/bin/env ruby -S rspec # rubocop:disable Lint/ScriptPermission - -require 'spec_helper' - -describe Puppet::Parser::Functions.function(:mysql_deepmerge) do - let(:scope) { PuppetlabsSpec::PuppetInternals.scope } - - describe 'when calling mysql_deepmerge from puppet' do - it 'does not compile when no arguments are passed' do - skip('Fails on 2.6.x, see bug #15912') if Puppet.version =~ %r{^2\.6\.} - Puppet[:code] = '$x = mysql_deepmerge()' - expect { - scope.compiler.compile - }.to raise_error(Puppet::ParseError, %r{wrong number of arguments}) - end - - it 'does not compile when 1 argument is passed' do - skip('Fails on 2.6.x, see bug #15912') if Puppet.version =~ %r{^2\.6\.} - Puppet[:code] = "$my_hash={'one' => 1}\n$x = mysql_deepmerge($my_hash)" - expect { - scope.compiler.compile - }.to raise_error(Puppet::ParseError, %r{wrong number of arguments}) - end - end - - describe 'when calling mysql_deepmerge on the scope instance' do - it 'accepts empty strings as puppet undef' do - expect { scope.function_mysql_deepmerge([{}, '']) }.not_to raise_error - end - - index_values = ['one', 'two', 'three'] - expected_values_one = ['1', '2', '2'] - it 'is able to mysql_deepmerge two hashes' do - new_hash = scope.function_mysql_deepmerge([{ 'one' => '1', 'two' => '1' }, { 'two' => '2', 'three' => '2' }]) - index_values.each_with_index do |index, expected| - expect(new_hash[index]).to eq(expected_values_one[expected]) - end - end - - it 'mysql_deepmerges multiple hashes' do - hash = scope.function_mysql_deepmerge([{ 'one' => 1 }, { 'one' => '2' }, { 'one' => '3' }]) - expect(hash['one']).to eq('3') - end - - it 'accepts empty hashes' do - expect(scope.function_mysql_deepmerge([{}, {}, {}])).to eq({}) - end - - expected_values_two = [1, 2, 'four' => 4] - it 'mysql_deepmerges subhashes' do - hash = scope.function_mysql_deepmerge([{ 'one' => 1 }, { 'two' => 2, 'three' => { 'four' => 4 } }]) - index_values.each_with_index do |index, expected| - expect(hash[index]).to eq(expected_values_two[expected]) - end - end - - it 'appends to subhashes' do - hash = scope.function_mysql_deepmerge([{ 'one' => { 'two' => 2 } }, { 'one' => { 'three' => 3 } }]) - expect(hash['one']).to eq('two' => 2, 'three' => 3) - end - - expected_values_three = [1, 'dos', { 'four' => 4, 'five' => 5 }] - it 'appends to subhashes 2' do - hash = scope.function_mysql_deepmerge([{ 'one' => 1, 'two' => 2, 'three' => { 'four' => 4 } }, { 'two' => 'dos', 'three' => { 'five' => 5 } }]) - index_values.each_with_index do |index, expected| - expect(hash[index]).to eq(expected_values_three[expected]) - end - end - - index_values_two = ['key1', 'key2'] - expected_values_four = [{ 'a' => 1, 'b' => 99 }, 'c' => 3] - it 'appends to subhashes 3' do - hash = scope.function_mysql_deepmerge([{ 'key1' => { 'a' => 1, 'b' => 2 }, 'key2' => { 'c' => 3 } }, { 'key1' => { 'b' => 99 } }]) - index_values_two.each_with_index do |index, expected| - expect(hash[index]).to eq(expected_values_four[expected]) - end - end - - it 'equates keys mod dash and underscore #value' do - hash = scope.function_mysql_deepmerge([{ 'a-b-c' => 1 }, { 'a_b_c' => 10 }]) - expect(hash['a_b_c']).to eq(10) - end - it 'equates keys mod dash and underscore #not' do - hash = scope.function_mysql_deepmerge([{ 'a-b-c' => 1 }, { 'a_b_c' => 10 }]) - expect(hash).not_to have_key('a-b-c') - end - - index_values_three = ['a_b_c', 'b-c-d'] - expected_values_five = [10, { 'e-f-g' => 3, 'c_d_e' => 12 }] - index_values_error = ['a-b-c', 'b_c_d'] - index_values_three.each_with_index do |index, expected| - it 'keeps style of the last when keys are euqal mod dash and underscore #value' do - hash = scope.function_mysql_deepmerge([{ 'a-b-c' => 1, 'b_c_d' => { 'c-d-e' => 2, 'e-f-g' => 3 } }, { 'a_b_c' => 10, 'b-c-d' => { 'c_d_e' => 12 } }]) - expect(hash[index]).to eq(expected_values_five[expected]) - end - it 'keeps style of the last when keys are euqal mod dash and underscore #not' do - hash = scope.function_mysql_deepmerge([{ 'a-b-c' => 1, 'b_c_d' => { 'c-d-e' => 2, 'e-f-g' => 3 } }, { 'a_b_c' => 10, 'b-c-d' => { 'c_d_e' => 12 } }]) - expect(hash).not_to have_key(index_values_error[expected]) - end - end - end -end