From c17fa45d42b12604f25d2e6ec1e2fb65e9bf8d69 Mon Sep 17 00:00:00 2001 From: Steven Pritchard Date: Sun, 22 Feb 2026 00:04:02 +0000 Subject: [PATCH] Remove hiera indirector terminus The Hiera indirector terminus has been removed as deprecated. The convert_merge method has significant branching logic that was never covered by unit tests - the deleted Puppet::Indirector::Hiera spec only exercised the shared Hiera indirection behaviours. Add explicit tests for all convert_merge branches (nil/first, unique, hash, deep, Hash with strategy, MergeStrategy delegation, and the error path) and for the :no_such_key throw in find when Hiera returns its not-found sentinel. Co-Authored-By: Claude Sonnet 4.6 Signed-off-by: Steven Pritchard --- lib/puppet/indirector/data_binding/hiera.rb | 95 +++++++++++++++- lib/puppet/indirector/hiera.rb | 101 ------------------ spec/integration/data_binding_spec.rb | 8 +- .../indirector/data_binding/hiera_spec.rb | 69 ++++++++++++ spec/unit/indirector/hiera_spec.rb | 22 ---- 5 files changed, 164 insertions(+), 131 deletions(-) delete mode 100644 lib/puppet/indirector/hiera.rb delete mode 100644 spec/unit/indirector/hiera_spec.rb diff --git a/lib/puppet/indirector/data_binding/hiera.rb b/lib/puppet/indirector/data_binding/hiera.rb index e40f7eb0cf..eba66cc0fe 100644 --- a/lib/puppet/indirector/data_binding/hiera.rb +++ b/lib/puppet/indirector/data_binding/hiera.rb @@ -1,8 +1,99 @@ # frozen_string_literal: true -require_relative '../../../puppet/indirector/hiera' +require_relative '../../../puppet/indirector/code' require 'hiera/scope' -class Puppet::DataBinding::Hiera < Puppet::Indirector::Hiera +class Puppet::DataBinding::Hiera < Puppet::Indirector::Code desc "Retrieve data using Hiera." + + def initialize(*args) + unless Puppet.features.hiera? + # TRANSLATORS "Hiera" is the name of a code library and should not be translated + raise _("Hiera terminus not supported without hiera library") + end + + super + end + + if defined?(::Psych::SyntaxError) + DATA_BINDING_EXCEPTIONS = [::StandardError, ::Psych::SyntaxError] + else + DATA_BINDING_EXCEPTIONS = [::StandardError] + end + + def find(request) + not_found = Object.new + options = request.options + Puppet.debug { "Performing a hiera indirector lookup of #{request.key} with options #{options.inspect}" } + value = hiera.lookup(request.key, not_found, Hiera::Scope.new(options[:variables]), nil, convert_merge(options[:merge])) + throw :no_such_key if value.equal?(not_found) + value + rescue *DATA_BINDING_EXCEPTIONS => detail + error = Puppet::DataBinding::LookupError.new("DataBinding 'hiera': #{detail.message}") + error.set_backtrace(detail.backtrace) + raise error + end + + private + + # Converts a lookup 'merge' parameter argument into a Hiera 'resolution_type' argument. + # + # @param merge [String,Hash,nil] The lookup 'merge' argument + # @return [Symbol,Hash,nil] The Hiera 'resolution_type' + def convert_merge(merge) + case merge + when nil, 'first' + # Nil is OK. Defaults to Hiera :priority + nil + when Puppet::Pops::MergeStrategy + convert_merge(merge.configuration) + when 'unique' + # Equivalent to Hiera :array + :array + when 'hash' + # Equivalent to Hiera :hash with default :native merge behavior. A Hash must be passed here + # to override possible Hiera deep merge config settings. + { :behavior => :native } + when 'deep' + # Equivalent to Hiera :hash with :deeper merge behavior. + { :behavior => :deeper } + when Hash + strategy = merge['strategy'] + if strategy == 'deep' + result = { :behavior => :deeper } + # Remaining entries must have symbolic keys + merge.each_pair { |k, v| result[k.to_sym] = v unless k == 'strategy' } + result + else + convert_merge(strategy) + end + else + # TRANSLATORS "merge" is a parameter name and should not be translated + raise Puppet::DataBinding::LookupError, _("Unrecognized value for request 'merge' parameter: '%{merge}'") % { merge: merge } + end + end + + public + + def self.hiera_config + hiera_config = Puppet.settings[:hiera_config] + config = {} + + if Puppet::FileSystem.exist?(hiera_config) + config = Hiera::Config.load(hiera_config) + else + Puppet.warning _("Config file %{hiera_config} not found, using Hiera defaults") % { hiera_config: hiera_config } + end + + config[:logger] = 'puppet' + config + end + + def self.hiera + @hiera ||= Hiera.new(:config => hiera_config) + end + + def hiera + self.class.hiera + end end diff --git a/lib/puppet/indirector/hiera.rb b/lib/puppet/indirector/hiera.rb deleted file mode 100644 index b9b7686164..0000000000 --- a/lib/puppet/indirector/hiera.rb +++ /dev/null @@ -1,101 +0,0 @@ -# frozen_string_literal: true - -require_relative '../../puppet/indirector/terminus' -require 'hiera/scope' - -# This class can't be collapsed into Puppet::Indirector::DataBindings::Hiera -# because some community plugins rely on this class directly, see PUP-1843. -# This class is deprecated and will be deleted in a future release. -# Use `Puppet::DataBinding.indirection.terminus(:hiera)` instead. -class Puppet::Indirector::Hiera < Puppet::Indirector::Terminus - def initialize(*args) - unless Puppet.features.hiera? - # TRANSLATORS "Hiera" is the name of a code library and should not be translated - raise _("Hiera terminus not supported without hiera library") - end - - super - end - - if defined?(::Psych::SyntaxError) - DataBindingExceptions = [::StandardError, ::Psych::SyntaxError] - else - DataBindingExceptions = [::StandardError] - end - - def find(request) - not_found = Object.new - options = request.options - Puppet.debug { "Performing a hiera indirector lookup of #{request.key} with options #{options.inspect}" } - value = hiera.lookup(request.key, not_found, Hiera::Scope.new(options[:variables]), nil, convert_merge(options[:merge])) - throw :no_such_key if value.equal?(not_found) - value - rescue *DataBindingExceptions => detail - error = Puppet::DataBinding::LookupError.new("DataBinding 'hiera': #{detail.message}") - error.set_backtrace(detail.backtrace) - raise error - end - - private - - # Converts a lookup 'merge' parameter argument into a Hiera 'resolution_type' argument. - # - # @param merge [String,Hash,nil] The lookup 'merge' argument - # @return [Symbol,Hash,nil] The Hiera 'resolution_type' - def convert_merge(merge) - case merge - when nil, 'first' - # Nil is OK. Defaults to Hiera :priority - nil - when Puppet::Pops::MergeStrategy - convert_merge(merge.configuration) - when 'unique' - # Equivalent to Hiera :array - :array - when 'hash' - # Equivalent to Hiera :hash with default :native merge behavior. A Hash must be passed here - # to override possible Hiera deep merge config settings. - { :behavior => :native } - when 'deep' - # Equivalent to Hiera :hash with :deeper merge behavior. - { :behavior => :deeper } - when Hash - strategy = merge['strategy'] - if strategy == 'deep' - result = { :behavior => :deeper } - # Remaining entries must have symbolic keys - merge.each_pair { |k, v| result[k.to_sym] = v unless k == 'strategy' } - result - else - convert_merge(strategy) - end - else - # TRANSLATORS "merge" is a parameter name and should not be translated - raise Puppet::DataBinding::LookupError, _("Unrecognized value for request 'merge' parameter: '%{merge}'") % { merge: merge } - end - end - - public - - def self.hiera_config - hiera_config = Puppet.settings[:hiera_config] - config = {} - - if Puppet::FileSystem.exist?(hiera_config) - config = Hiera::Config.load(hiera_config) - else - Puppet.warning _("Config file %{hiera_config} not found, using Hiera defaults") % { hiera_config: hiera_config } - end - - config[:logger] = 'puppet' - config - end - - def self.hiera - @hiera ||= Hiera.new(:config => hiera_config) - end - - def hiera - self.class.hiera - end -end diff --git a/spec/integration/data_binding_spec.rb b/spec/integration/data_binding_spec.rb index 2dab19f64f..ad50c2fe71 100644 --- a/spec/integration/data_binding_spec.rb +++ b/spec/integration/data_binding_spec.rb @@ -1,6 +1,4 @@ require 'spec_helper' -require 'puppet/indirector/hiera' - require 'puppet_spec/compiler' require 'puppet/indirector/data_binding/hiera' @@ -53,10 +51,8 @@ }} before do - # Drop all occurances of cached hiera instances. This will reset @hiera in Puppet::Indirector::Hiera, Testing::DataBinding::Hiera, - # and Puppet::DataBinding::Hiera. Different classes are active as indirection depending on configuration - ObjectSpace.each_object(Class).select {|klass| klass <= Puppet::Indirector::Hiera }.each { |klass| klass.instance_variable_set(:@hiera, nil) } - Puppet[:data_binding_terminus] = 'hiera' + # Reset the cached hiera instance on the terminus class + Puppet::DataBinding::Hiera.instance_variable_set(:@hiera, nil) Puppet[:modulepath] = dir end diff --git a/spec/unit/indirector/data_binding/hiera_spec.rb b/spec/unit/indirector/data_binding/hiera_spec.rb index 9cda5aabed..8cd7516204 100644 --- a/spec/unit/indirector/data_binding/hiera_spec.rb +++ b/spec/unit/indirector/data_binding/hiera_spec.rb @@ -16,4 +16,73 @@ end it_should_behave_like "Hiera indirection", Puppet::DataBinding::Hiera, my_fixture_dir + + describe "#find", :if => Puppet.features.hiera? do + let(:data_binder) { described_class.new } + let(:hiera) { double('hiera') } + + def request(key, options = {}) + Puppet::Indirector::Request.new(:hiera, :find, key, nil, options) + end + + before do + allow(described_class).to receive(:hiera).and_return(hiera) + end + + it "throws :no_such_key when hiera returns the not_found sentinel" do + # Returning the default argument signals "not found" to the find method + allow(hiera).to receive(:lookup) { |_key, default, *_| default } + expect { data_binder.find(request('missing')) }.to throw_symbol(:no_such_key) + end + end + + describe "#convert_merge", :if => Puppet.features.hiera? do + let(:data_binder) { described_class.new } + + def convert(merge) + data_binder.send(:convert_merge, merge) + end + + it "returns nil for nil" do + expect(convert(nil)).to be_nil + end + + it "returns nil for 'first'" do + expect(convert('first')).to be_nil + end + + it "returns :array for 'unique'" do + expect(convert('unique')).to eq(:array) + end + + it "returns native hash behavior for 'hash'" do + expect(convert('hash')).to eq({ :behavior => :native }) + end + + it "returns deeper hash behavior for 'deep'" do + expect(convert('deep')).to eq({ :behavior => :deeper }) + end + + it "delegates to the strategy's configuration when given a MergeStrategy" do + strategy = instance_double(Puppet::Pops::MergeStrategy, :configuration => 'unique') + expect(convert(strategy)).to eq(:array) + end + + it "returns deeper hash behavior for a Hash with strategy 'deep'" do + expect(convert({ 'strategy' => 'deep' })).to eq({ :behavior => :deeper }) + end + + it "forwards extra keys alongside deeper behavior for a Hash with strategy 'deep'" do + result = convert({ 'strategy' => 'deep', 'knockout_prefix' => '--' }) + expect(result).to eq({ :behavior => :deeper, :knockout_prefix => '--' }) + end + + it "delegates to the string strategy for a Hash with a non-deep strategy" do + expect(convert({ 'strategy' => 'unique' })).to eq(:array) + end + + it "raises LookupError for an unrecognized merge value" do + expect { convert('bogus') }.to raise_error(Puppet::DataBinding::LookupError, /Unrecognized value.*bogus/) + end + end end diff --git a/spec/unit/indirector/hiera_spec.rb b/spec/unit/indirector/hiera_spec.rb deleted file mode 100644 index 2b4bb6e923..0000000000 --- a/spec/unit/indirector/hiera_spec.rb +++ /dev/null @@ -1,22 +0,0 @@ -require 'spec_helper' -require 'puppet/data_binding' -require 'puppet/indirector/hiera' - -begin - require 'hiera/backend' -rescue LoadError => e - Puppet.warning(_("Unable to load Hiera 3 backend: %{message}") % {message: e.message}) -end - -describe Puppet::Indirector::Hiera, :if => Puppet.features.hiera? do - - module Testing - module DataBinding - class Hiera < Puppet::Indirector::Hiera - end - end - end - - it_should_behave_like "Hiera indirection", Testing::DataBinding::Hiera, my_fixture_dir -end -