search

salt-formulas / reclass

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

Regression in class_mappings after refactoring #84

Open uberspot opened 5 years ago

uberspot commented 5 years ago

I'm not 100% sure that's the issue, but after updating to reclass 1.6.0 the following in reclass-config.yml stopped working:

class_mappings:
  - temp/*        type.test

and it did work before. I also tested:

class_mappings:
  - \*        type.test

and it does work.

uberspot commented 5 years ago

After searching a bit and trying different commits from reclass I think the bug is introduced in https://github.com/salt-formulas/reclass/pull/68 specifically commit d2762b0

uberspot commented 5 years ago

@epcim gentle bump ^ :)

AndrewPickford commented 4 years ago

The proposed fix adds back the line name = os.path.splitext(relpath)[0] from the get_node method in yaml_fs. This was removed as part of a patch to fix the use of relative class names. But relative class name loading still works with the line added back in, so there does not seem to be a problem there.

By default node names are just the name of the node file less the .yml extension without any path information so it would be strange for the class_mappings to then wildcard match based on the path of the node file. Having path based wildcards work initially may have been unintentional.

However I don't see why path based wild carding can't be supported. My proposal is to add a path parameter to the Entity class, set it to the value of os.path.splitext(relpath)[0] (and add something equivalent to the get_node method in yaml_git). An option for class_mappings to wildcard either the node name or the path could then be added. It's a small amount of code so if this is acceptable I can code it up.

AndrewPickford commented 4 years ago

Added pull request #93, with the fix to allow class mappings to match either the node name or node path depending on a configuration option.