Jira (PUP-10356) Empty YAML files cause warnings in yaml_data and eyaml_lookup_key functions

12 views
Skip to first unread message

Glenn Sarti (Jira)

unread,
Mar 4, 2020, 9:18:03 PM3/4/20
to puppe...@googlegroups.com
Glenn Sarti created an issue
 
Puppet / Bug PUP-10356
Empty YAML files cause warnings in yaml_data and eyaml_lookup_key functions
Issue Type: Bug Bug
Assignee: Unassigned
Created: 2020/03/04 6:17 PM
Environment:

N/A

Priority: Normal Normal
Reporter: Glenn Sarti

Puppet Version: 6.x
Puppet Server Version: N/A
OS Name/Version:

 

When using a valid but empty YAML file in the hiera hierarchy the logs spew out a lot of warnings about expecting a hash.

While I can understand the warnings when there's a type mismatch (e.g. Got a String instead of a Hash), and empty file is deliberate act with no data.  Therefore Puppet should not raise a warning for empty YAML files.

 

Desired Behavior:

Empty YAML files do not raise warnings

Actual Behavior:

Warnings can be seen in puppet apply or puppet agent command

Add Comment Add Comment
 
This message was sent by Atlassian Jira (v8.5.2#805002-sha1:a66f935)
Atlassian logo

Glenn Sarti (Jira)

unread,
Mar 4, 2020, 9:24:03 PM3/4/20
to puppe...@googlegroups.com
Glenn Sarti updated an issue
Change By: Glenn Sarti
*Puppet Version: 6.x*
*Puppet Server Version: N/A*
*OS Name/Version:*


 

When using a valid but empty YAML file in the hiera hierarchy the logs spew out a lot of warnings about expecting a hash.

While I can understand the warnings when there's a type mismatch (e.g. Got a String instead of a Hash), and but an empty file is deliberate act with no data (e . g. commenting everything out).   Therefore Puppet should not raise a warning for empty YAML files.

 

*Desired Behavior:*


Empty YAML files do not raise warnings

*Actual Behavior:*


Warnings can be seen in puppet apply or puppet agent command

Jorie Tappa (Jira)

unread,
Mar 9, 2020, 2:20:03 PM3/9/20
to puppe...@googlegroups.com

Jorie Tappa (Jira)

unread,
Mar 16, 2020, 4:22:02 PM3/16/20
to puppe...@googlegroups.com
Jorie Tappa commented on Bug PUP-10356
 
Re: Empty YAML files cause warnings in yaml_data and eyaml_lookup_key functions

One thing confusing, brought up during Coremunity planning:

Is having an empty YAML file actually valid to Hiera 5?

Josh Cooper (Jira)

unread,
Mar 16, 2020, 5:38:04 PM3/16/20
to puppe...@googlegroups.com
Josh Cooper commented on Bug PUP-10356

When running a pdk new module or pdk convert the current pdk-template adds in a hiera.yaml and common.yaml

To clarify, pdk adds a default hiera.yaml like:

---
version: 5
 
defaults:  # Used for any hierarchy level that omits these keys.
  datadir: data         # This path is relative to hiera.yaml's directory.
  data_hash: yaml_data  # Use the built-in YAML backend.
 
hierarchy:
  - name: "osfamily/major release"
    paths:
      - "os/%{facts.os.family}/%{facts.os.release.major}.yaml"
        # Used for Solaris
      - "os/%{facts.os.family}/%{facts.kernelrelease}.yaml"
        # Used to distinguish between Debian and Ubuntu
      - "os/%{facts.os.name}/%{facts.os.release.major}.yaml"
  - name: "osfamily"
    paths:
      - "os/%{facts.os.family}.yaml"
      - "os/%{facts.os.name}.yaml"
  - name: 'common'
    path: 'common.yaml'

so that modules are more "self documenting" and follow best practices. The pdk also adds an empty common.yaml, and it's that empty file that causes problems, since the yaml_data and eyaml_lookup_key functions require the file to contain a valid yaml *hash*. So while an empty file is valid yaml (it's the yaml scalar null), it doesn't contain a hash.

We could make the functions more lenient, but I think that's going in the wrong direction. The pdk could just as easily add a "common" section that is commented out, or it could create a common.yaml containing a empty hash:

--- {}

In the PDK ticket there is conversation about common.yaml adding additional lookups, so the former may be better.

I'm inclined to close this as won't fix, thoughts Maggie Dreyer, Justin Stoller, Patrick Carlisle?

Maggie Dreyer (Jira)

unread,
Mar 16, 2020, 5:48:04 PM3/16/20
to puppe...@googlegroups.com

I tend to agree, I feel like if in general we really expect the data files to contain a hash as the base element, we should continue enforcing that. Could help remind someone that they have a useless file sitting there (maybe some commented data that they need to put back eventually).

It doesn't seem great that our tooling is creating one of these though. Don't have a preference of which way the PDK fixes it, but I do think it should be fixed there.

Henrik Lindberg (Jira)

unread,
Mar 16, 2020, 6:13:04 PM3/16/20
to puppe...@googlegroups.com

IIRC there was an earlier ticket about something like (or very similar), and also IIRC there were some (possibly historic) issues where you could get a nil back under some other conditions when parsing yaml - thus making it really hard to find if a returned nil was to be taken as an empty hash.

Think you came to the right conclusion (fix it in PDK and leave the hiera impl as it is).

Reply all
Reply to author
Forward
0 new messages