[HI-220] Deprecate interpolation via function in hiera.yaml Created: 2014/02/25  Updated: 2016/03/17  Resolved: 2015/12/22

Status: Closed
Project: Hiera
Component/s: None
Affects Version/s: HI 1.3.0
Fix Version/s: HI 3.1.0

Type: Bug Priority: Normal
Reporter: Charlie Sharpsteen Assignee: Unassigned
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified
Environment:

CentOS 6.5
Hiera 1.3.1 from PL yum repos


Attachments: HTML File hi-220-init-manifest     HTML File hi-220-simple-init-manifest     Text File hiera-recursive-lookup.patch    
Template:
Story Points: 1
Sprint: Language 2015-12-16, Language 2015-12-30
Release Notes: Bug Fix
Release Notes Summary: Calling {{hiera}} interpolation method from within a hiera.yaml caused infinite reqursion. This was fixed by making it an error to se {{hiera}} method in interpolation in hiera.yaml. Also, the use of other interpolation methods in hiera.yaml will now yield a deprecation warning as those will not be allowed in hiera 4.0.

 Description   

Given the following hiera.yaml file where an interpolation function is used to define the hierarchy:

---
:backends:
  - yaml
:logger: console
:hierarchy:
  - "roles/%{hiera('role')}"
  - common
 
:yaml:
   :datadir: /etc/puppet/environments/%{environment}/extdata

This will result in infinite recursion on any lookup:

# hiera foo -d
DEBUG: Tue Feb 25 19:57:34 -0800 2014: Hiera YAML backend starting
DEBUG: Tue Feb 25 19:57:34 -0800 2014: Looking up foo in YAML backend
DEBUG: Tue Feb 25 19:57:34 -0800 2014: Looking up role in YAML backend
 
... repeats for a bit ...
 
DEBUG: Tue Feb 25 19:57:34 -0800 2014: Looking up role in YAML backend
DEBUG: Tue Feb 25 19:57:39 -0800 2014: Looking up role in YAML backend
DEBUG: Tue Feb 25 19:57:39 -0800 2014: Looking up role in YAML backend
/usr/lib/ruby/site_ruby/1.8/hiera/backend.rb:172:in `lookup': stack level too deep (SystemStackError)
	from /usr/lib/ruby/site_ruby/1.8/hiera/backend.rb:171:in `each'
	from /usr/lib/ruby/site_ruby/1.8/hiera/backend.rb:171:in `lookup'
	from /usr/lib/ruby/site_ruby/1.8/hiera/interpolate.rb:43:in `hiera_interpolate'
	from /usr/lib/ruby/site_ruby/1.8/hiera/interpolate.rb:13:in `send'
	from /usr/lib/ruby/site_ruby/1.8/hiera/interpolate.rb:13:in `interpolate'
	from /usr/lib/ruby/site_ruby/1.8/hiera/recursive_guard.rb:16:in `check'
	from /usr/lib/ruby/site_ruby/1.8/hiera/interpolate.rb:11:in `interpolate'
	from /usr/lib/ruby/site_ruby/1.8/hiera/backend.rb:93:in `parse_string'
	 ... 1747 levels...
	from /usr/lib/ruby/site_ruby/1.8/hiera/backend.rb:171:in `each'
	from /usr/lib/ruby/site_ruby/1.8/hiera/backend.rb:171:in `lookup'
	from /usr/lib/ruby/site_ruby/1.8/hiera.rb:60:in `lookup'
	from /usr/bin/hiera:221

The docs mention that using hiera():

...generally nonsensical and not useful in Hiera’s settings

However, there are some semi-useful use cases. For example, the following hierarchy is functional:

:hierarchy:
  - "%{::certname}" # <-- "role" is defined here. Always.
  - "roles/%{hiera('role')}"
  - common

Given that infinite recursion is A Bad Thing™, we should either guard this use case or disallow the use of interpolation functions in the Hiera settings file.



 Comments   
Comment by Nicholas Fagerlund [ 2014/02/26 ]

I've changed the wording of the docs warning to link here and be more emphatic about avoiding hiera() in the config file. (This change will go out with the hiera 1.3.2 final release.)

Comment by Markus Lidel [ 2015/10/15 ]

The reason for the loop is very simple, to lookup the key (in the example above "role") the yaml-backend has to read in all files listed in :hierarchy. But if the file contains an interpolation, which couldn't be resolved, it calls the lookup function again, and the endless loop begins. To solve this, the attached patch only call the lookup function for each key once and return nil on the second call.

Comment by Sander [ 2015/12/02 ]

Hello. Is there an ETA for fixing this issue? We'd like to make use of the use case above but are hold back by this issue. I'd be willing to make a PR based on Markus Lidel his fix if that would speed up the process.

Comment by Henrik Lindberg [ 2015/12/02 ]

Ping Thomas Hallgren what does 'hiera 4'/data in modules do when fed the same as in the description?

We need to decide on how to handle a recursion like this: just run with paths up to the known point (i.e. the paths already processed when the reentrant calls come to lookup). Or do something else to support this usecase?

Comment by Thomas Hallgren [ 2015/12/03 ]

Henrik Lindberg, this is already covered in 'hiera 4'/data in modules. There, it's not allowed to use the interpolation method syntax, i.e.

hierarchy:
  - "%{::certname}"           # this is OK.
  - "roles/%{hiera('role')}"  # Not OK and will result in the error "Interpolation using method syntax is not allowed in this context"

Comment by Henrik Lindberg [ 2015/12/03 ]

Ok, I think that is the right decision (not allowing method calls). We should deprecated this in the next relase of hiera (3.1.0)

Comment by Sander [ 2015/12/03 ]

We really like the idea of specifying the role in a hiera node file and through that, include the hiera role file. This seems more transparant than specifying the role in another (Facter) file or in an external source. As far as we know, the above method is the only way to achieve this. Is there another option to load a hira file based on a property in another hiera file?

Comment by R.I.Pienaar [ 2015/12/03 ]

Sander if I get what you're saying then this will work:

node default {
  $role = hiera("role", "bootstrap")
  include(hiera("classes"))
}

and you can then just use

%{role}

in your hiera.yaml and supply role however you want

Comment by Sander [ 2015/12/03 ]

Works like a charm, thanks!

Comment by Henrik Lindberg [ 2015/12/14 ]

Deprecation warning for calling methods, and error for calling hiera function in hiera's conf, merged to master: 87a1827

Comment by Thomas Hallgren [ 2015/12/14 ]

Reverted the merge of PR-328. I improved the solution and created PR-330 instead. Please re-review.

Comment by Henrik Lindberg [ 2015/12/17 ]

Merged to master at: 8e34c4f

Comment by Sean Griffin [ 2015/12/21 ]

I don't see any deprication warning message when causing a call to hiera() during interpolation of the hiera config file. The discussion and title of the ticket would suggest that I should see such a warning.

I set up a manifest and a hiera configuration, in which hiera.yaml calls hiera('role') and a module manifest that makes the function call hiera('somename'). (See attached init-manifest)

Comparing the old hiera (commit sha 51d74ff8f693188d07297c80d9b3f2f8cc9ec310) to the new sha (8e34c4fec5d1afc3dd89d01520debf7daab2f741 == head), I see following differences:

1. function call from the module manifest returns different error messages:

old msg:

Error: Could not run: stack level too deep

new msg:

Error: Evaluation Error: Error while evaluating a Function Call, Lookup recursion detected in [hiera('role')] at /etc/puppetlabs/code/environments/production/modules/hieratest/manifests/init.pp:11:10 on node rg9zuk876be3znp.delivery.puppetlabs.net

2. These error messages are displayed in response to the command: 'puppet lookup somename'.

3. The same is also true for 'hiera somename', except in the fixed version I also see a stack trace.

So... Deprication? It seems it got merged before and then reverted. Was this omitted or do we no longer want that warning?

Comment by Sean Griffin [ 2015/12/21 ]

Hi Thomas Hallgren. See comment above.
Thanks.

Comment by Thomas Hallgren [ 2015/12/21 ]

The reason you don't see a deprecation warning is that your hiera.yaml configuration contains a circular recursion error. If you fix that, then the warning will be printed instead.

Explanation
When the hiera.yaml file contains a %{hiera('somekey')} this will lead to that a new hiera lookup is performed and hence that the configuration is subject to a new interpolation. This will work for cases when the interpolated expression is resolved by a lookup that uses entries declared earlier in the hierarchy. When that's not the case (as in your example), then the same entry will be subject to interpolation again and the recursion is a fact.

The unit spec tests contains a case that covers this.

What to expect
The expected result of applying the PR is:
1. You will never get a "stack level to deep" error as the result of using a %{hiera('<some_key>')} declaration in the hiera.yaml file.
2. For the cases when you did get a "stack level too deep" before, you will now instead get a "lookup recursion" error.
3. For all other cases where a %{<valid_method>('<some_key>')} is encountered in the hiera.yaml file, a deprecation warning will be output.

Comment by Sean Griffin [ 2015/12/22 ]

Thanks Thomas. I had (mis)understood that any call to hiera from within the configuration file would generate the deprecation warning, and that even if there were a recursion error, there would be a warning. I will test again with this in mind. Thanks.

Comment by Sean Griffin [ 2015/12/22 ]

It appears NO deprecation warning is issued when the user executes the hiera shell command. The command 'heira somekey' causes no deprecation warning.

[root@qq9wi3yxlwklr7m roles]# hiera somekey
someval
[root@qq9wi3yxlwklr7m roles]#

The deprecation warning is seen when the command 'puppet lookup somekey' is executed, or when either function hiera() or lookup() is called from a manifest.

I am using puppet-agent build sha 9a1dfae64751da59ce8763730846669356da96dd for this test (http://builds.puppetlabs.lan/puppet-agent/9a1dfae64751da59ce8763730846669356da96dd/), installed by beaker.

I retested using a simple manifest with no modules defined. The hiera configuration looks like this:

/etc/puppetlabs/code/
├── environments
│   └── production
├── hieradata
│   ├── global.yaml
│   └── roles
│       └── world.yaml
└── hiera.yaml

hiera.yaml:

---
    :backends:
      - "yaml"
    :logger: "console"
    :hierarchy:
      - "global"
      - "roles/%{hiera('hello')}"
 
    :yaml:
      :datadir: "/etc/puppetlabs/code/hieradata"

hieradata/global.yaml:

---
    hello: "world"

hieradata/roles/world.yaml

---
    somekey: "someval"

See attached file: hi-220-simple-init-manifest.

In this configuration, global.yaml is searched first, facilitating lookup of "hello" (evaluates to "world"). Key: "somekey" is resolved in hieradata/roles/world.yaml without runaway recursion. In the fixed version I see the deprecation warning, then the correct value for the key ("someval").

3 other observations:
1. The command 'puppet lookup somekey' results in 4 copies of the warning message.

[root@qq9wi3yxlwklr7m roles]# puppet lookup somekey
Notice: hiera(): Use of interpolation methods in hiera configuration file is deprecated
Notice: hiera(): Use of interpolation methods in hiera configuration file is deprecated
Notice: hiera(): Use of interpolation methods in hiera configuration file is deprecated
Notice: hiera(): Use of interpolation methods in hiera configuration file is deprecated
--- someval
...
[root@qq9wi3yxlwklr7m roles]#

2. The command: puppet apply -e "notice(hiera('somekey'))" results in two copies of the warning.

[root@qq9wi3yxlwklr7m code]# puppet apply -e "notice(hiera('somekey'))"
Notice: hiera(): Use of interpolation methods in hiera configuration file is deprecated
Notice: hiera(): Use of interpolation methods in hiera configuration file is deprecated
Notice: Scope(Class[main]): someval
Notice: Compiled catalog for qq9wi3yxlwklr7m.delivery.puppetlabs.net in environment production in 0.08 seconds
Notice: Applied catalog in 0.02 seconds
[root@qq9wi3yxlwklr7m code]#

3. The command: puppet apply -e "notice(lookup('somekey'))" results in four copies of the warning.

[root@j8n2u9bp4askgom hieradata]# puppet apply -e "notice(lookup('somekey'))"
Notice: hiera(): Use of interpolation methods in hiera configuration file is deprecated
Notice: hiera(): Use of interpolation methods in hiera configuration file is deprecated
Notice: hiera(): Use of interpolation methods in hiera configuration file is deprecated
Notice: hiera(): Use of interpolation methods in hiera configuration file is deprecated
Notice: Scope(Class[main]): someval
Notice: Compiled catalog for j8n2u9bp4askgom.delivery.puppetlabs.net in environment production in 0.12 seconds
Notice: Applied catalog in 0.03 seconds
[root@j8n2u9bp4askgom hieradata]#

Having previously established that the recursion error messages are properly improved, the only outstanding questions are:
1. Should hiera shell command generate a deprecation warning?
2. Though not serious, is the repeated deprecation warning expected? Tolerable? Easy or hard to fix? I just seems more presentable if the warning were displayed only once.

Comment by Thomas Hallgren [ 2015/12/22 ]

1. The reason you don't see the warning when using the hiera shell command is that both warning and debug messages are suppressed unless you run the command using the -d or --debug. Room for improvement for sure but not in scope for this JIRA.

2. It is expected that you see multiple deprecation warnings since Hiera requests the interpolation method at least twice per invocation. When the lookup stems from Puppet there will be two such invocations (one for the lookup_options and one for the actual key) and hence four messages. The only way to get rid of all duplicates is probably to add some logic to the logger to keep it from repeat the same message more than once. Such logic must deal with the fact that debug messages will be emitted in between the warnings. Personally, I don't think it's worth the effort.

Comment by Sean Griffin [ 2015/12/22 ]

Thank you Thomas Hallgren. Verified the warning is visible when running hiera --debug. Repeated warnings is a fact of life at the moment. Marking this one resolved.

Generated at Thu Jul 18 12:10:17 PDT 2019 using JIRA 7.7.1#77002-sha1:e75ca93d5574d9409c0630b81c894d9065296414.