From 8f1c4b985df707f4dd60001801e1d0f6df4b3864 Mon Sep 17 00:00:00 2001 From: Eimhin Laverty Date: Tue, 9 Apr 2019 14:22:06 +0100 Subject: [PATCH 1/2] (MODULES-8886) Revert removal of deepmerge function Reverting this change as the deepmerge function in stdlib does not work the same as the mysql function. The mysql config is generated with incorrect duplicate entries in which '-' and '_' are mixed up when using the stdlib function. --- lib/puppet/functions/mysql/deepmerge.rb | 66 +++++++++++++++++ manifests/backup/mysqlbackup.pp | 2 +- manifests/server.pp | 2 +- spec/functions/mysql_deepmerge_spec.rb | 94 +++++++++++++++++++++++++ 4 files changed, 162 insertions(+), 2 deletions(-) create mode 100644 lib/puppet/functions/mysql/deepmerge.rb create mode 100644 spec/functions/mysql_deepmerge_spec.rb diff --git a/lib/puppet/functions/mysql/deepmerge.rb b/lib/puppet/functions/mysql/deepmerge.rb new file mode 100644 index 000000000..6196fab4e --- /dev/null +++ b/lib/puppet/functions/mysql/deepmerge.rb @@ -0,0 +1,66 @@ +# @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/manifests/backup/mysqlbackup.pp b/manifests/backup/mysqlbackup.pp index 24fc99ad7..1d1f7a5e4 100644 --- a/manifests/backup/mysqlbackup.pp +++ b/manifests/backup/mysqlbackup.pp @@ -93,7 +93,7 @@ 'password' => $backuppassword, } } - $options = $default_options.deep_merge($mysql::server::override_options) + $options = mysql::deepmerge($default_options, $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 1f3e5d3be..ce1b5e0fd 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::params::default_options.deep_merge($override_options) + $options = mysql::deepmerge($mysql::params::default_options, $override_options) Class['mysql::server::root_password'] -> Mysql::Db <| |> diff --git a/spec/functions/mysql_deepmerge_spec.rb b/spec/functions/mysql_deepmerge_spec.rb new file mode 100644 index 000000000..e5e9a66ef --- /dev/null +++ b/spec/functions/mysql_deepmerge_spec.rb @@ -0,0 +1,94 @@ +#! /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 From 6b7d88b3d8c368a5345ae6a041da7369d96d1587 Mon Sep 17 00:00:00 2001 From: Eimhin Laverty Date: Thu, 11 Apr 2019 12:12:18 +0100 Subject: [PATCH 2/2] (MODULES-8886) Rename deepmerge to normalise_and_deepmerge --- .../{deepmerge.rb => normalise_and_deepmerge.rb} | 13 +++++++------ manifests/backup/mysqlbackup.pp | 2 +- manifests/server.pp | 2 +- ...pec.rb => mysql_normalise_and_deepmerge_spec.rb} | 10 ++++------ 4 files changed, 13 insertions(+), 14 deletions(-) rename lib/puppet/functions/mysql/{deepmerge.rb => normalise_and_deepmerge.rb} (73%) rename spec/functions/{mysql_deepmerge_spec.rb => mysql_normalise_and_deepmerge_spec.rb} (93%) diff --git a/lib/puppet/functions/mysql/deepmerge.rb b/lib/puppet/functions/mysql/normalise_and_deepmerge.rb similarity index 73% rename from lib/puppet/functions/mysql/deepmerge.rb rename to lib/puppet/functions/mysql/normalise_and_deepmerge.rb index 6196fab4e..70c03f599 100644 --- a/lib/puppet/functions/mysql/deepmerge.rb +++ b/lib/puppet/functions/mysql/normalise_and_deepmerge.rb @@ -1,9 +1,10 @@ -# @summary Recursively merges two or more hashes together and returns the resulting hash. +# @summary Recursively merges two or more hashes together, normalises keys with differing use of dashesh and underscores, +# then 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) +# $merged_hash = mysql::normalise_and_deepmerge($hash1, $hash2) # # The resulting hash is equivalent to: # # $merged_hash = { 'one' => 1, 'two' => 'dos', 'three' => { 'four' => 4, 'five' => 5 } } # @@ -11,10 +12,10 @@ # - 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) +Puppet::Functions.create_function(:'mysql::normalise_and_deepmerge') do + def normalise_and_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 } + raise Puppet::ParseError, _('mysql::normalise_and_deepmerge(): wrong number of arguments (%{args_length}; must be at least 2)') % { args_length: args.length } end result = {} @@ -22,7 +23,7 @@ def deepmerge(*args) 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 } + raise Puppet::ParseError, _('mysql::normalise_and_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 diff --git a/manifests/backup/mysqlbackup.pp b/manifests/backup/mysqlbackup.pp index 1d1f7a5e4..0da68bf19 100644 --- a/manifests/backup/mysqlbackup.pp +++ b/manifests/backup/mysqlbackup.pp @@ -93,7 +93,7 @@ 'password' => $backuppassword, } } - $options = mysql::deepmerge($default_options, $mysql::server::override_options) + $options = mysql::normalise_and_deepmerge($default_options, $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 ce1b5e0fd..f70ca4146 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::normalise_and_deepmerge($mysql::params::default_options, $override_options) Class['mysql::server::root_password'] -> Mysql::Db <| |> diff --git a/spec/functions/mysql_deepmerge_spec.rb b/spec/functions/mysql_normalise_and_deepmerge_spec.rb similarity index 93% rename from spec/functions/mysql_deepmerge_spec.rb rename to spec/functions/mysql_normalise_and_deepmerge_spec.rb index e5e9a66ef..d1c94f8ff 100644 --- a/spec/functions/mysql_deepmerge_spec.rb +++ b/spec/functions/mysql_normalise_and_deepmerge_spec.rb @@ -1,8 +1,6 @@ -#! /usr/bin/env ruby -S rspec # rubocop:disable Lint/ScriptPermission - require 'spec_helper' -describe 'mysql::deepmerge' do +describe 'mysql::normalise_and_deepmerge' do it 'exists' do is_expected.not_to eq(nil) end @@ -22,14 +20,14 @@ # rubocop:disable RSpec/NamedSubject index_values = ['one', 'two', 'three'] expected_values_one = ['1', '2', '2'] - it 'is able to mysql_deepmerge two hashes' do + it 'merge 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 + it 'merges multiple hashes' do hash = subject.execute({ 'one' => 1 }, { 'one' => '2' }, 'one' => '3') expect(hash['one']).to eq('3') end @@ -39,7 +37,7 @@ end expected_values_two = [1, 2, 'four' => 4] - it 'mysql_deepmerges subhashes' do + it 'merges 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])