salt-formulas / reclass

A recursive external node classifier for automation tools like Ansible, Puppet, and Salt
Other
53 stars 23 forks source link

raise exception on duplicate keys on the same document hierarchy level #59

Open gburiola opened 6 years ago

gburiola commented 6 years ago

At the moment, reclass allows duplicate keys in any YAML file and silently uses the last value.

For example, this node file:

$ cat /tmp/reclass/nodes/node1.yml
parameters:
    key1: True
    key2: other value
    key1: False

Returns the following:

$ reclass  --inventory -b /tmp/reclass/ | yq -y .nodes.node1.parameters
_reclass_:
  environment: base
  name:
    full: node1
    short: node1
key1: false
key2: other value

In a very large YAML file, this is an easy to make mistake and can have very bad consequences.

We could use something like what's described in the link below to look for duplicate keys on reclass/storage/yamldata.py https://gist.github.com/pypt/94d747fe5180851196eb

That would throw an error on yaml.load():

while constructing a mapping
  in "/tmp/reclass/nodes/node1.yml", line 2, column 5
found duplicate key (key1)
  in "/tmp/reclass/nodes/node1.yml", line 4, column 5

If you're happy with this approach I'm happy to implement it.

epcim commented 6 years ago

Yes, duplicate keys are an issue to avoid. Let's implement it. One request - throw a warning (not error) and format it as "one line".

In the fix description, you can provide a notice, that with version 2.x the warning will be changed to "error". (I know we can control this by settings, however, idea: we can also rather say that since 2.x reclass will be more strict and will not want to keep full "backward compatibility" with the default settings.)