Command line argument for custom validators

[jhe-iqbis]

First of all, thank you for this great tool. It is a crucial part of our build pipeline to maintain code quality and therefore incredibly useful to our project.

As Yamale already features a modular design, able to be extended by custom validators, my pull request aims to allow this using command line arguments.

There are no breaking changes in my pull request and I wrote a test that completes successfully for python 3.6, 3.8 and (with a newer pytest version) 3.10 on my system.

[lgtm-com[bot]]

This pull request introduces 1 alert when merging 09b74794c7b887b55e23ef647b1b61058b36819d into d7f96574b779a66dc25085cc172182f8b7f351db - view on LGTM.com

new alerts:

• 1 for Syntax error

[mildebrandt]

Thanks for your contribution! We usually like to have a discussion in an issue first to ensure not too much coding happens in the wrong direction. This seems pretty contained though, so not a problem. Just to keep in mind for the future.

I'll be out for a week, so it'll take a while for me to get to this. Initially, on the surface, it does look good though.

[mechie]

If this is accepted/merged, the new flag should be documented in the README, probably along with a section that links to the example custom validators and a shows how to use it.

[mechie]

Just going to quote from https://github.com/23andMe/Yamale/issues/166#issuecomment-895585284 here, since that issue is the mirror of this one (custom validators inlined in schemas), and the concerns mostly apply:

Some things to think about:

• What happens when the user mistakenly adds two validators with the same tag? Is that an error or a warning?
• What happens when the user attempts to override an existing validator?
• Will this work in conjunction with loading a separate custom validator via code?
• Keep in mind the other implementation in case we'd like to do that in the future as well. I'd hate to totally rewrite this to fit the other implementation.
• Tests, tests, tests....someone else will be maintaining your code.

[mildebrandt]

I appreciate the reminder of the previous concerns. I believe this is a little different since it's not embedding the validators in the schema itself, but it's in a separate python file that is pointed to within the command line options. This makes it far less dangerous as well, and shouldn't require an extra warning in the README.

However, I've only looked at this for 5 minutes, so please let me know if I missed something. :)

[mechie]

Ah yep, same, just did an initial read and the non-technical concerns popped in my head, will need to come back and look deeper. I consume yamale only as a library, so the concern is not personal.

I'm not sure what promises/guarantees the CLI makes (is there an implicit "only the schema flag is dangerous" due to the warning?). From a user POV it's "I give flag A file X, and flag B file Y" so warning about A (schema) but not B (includes) when both can lead to undefined behavior (schema via ast exploits, includes via it being loaded as a python module) seems odd.

[mildebrandt]

The danger comes from offering Yamale as a service. If that service is accepting a schema/data pair and offering back a result, there are concerns that processing uncontrolled schema would leave the service open to malicious attacks.

When limited to command line, the caller of the yamale executable has full control over the additional validators that are imported via the command line option.

I understand your concern, and I don't mind putting a line in the README saying to "ensure the imported validators come from trusted sources" or something similar.

[mildebrandt]

I wanted to do a little check in here. Work is really busy at the moment. This hasn't been forgotten, but will take a bit for me to get to....my apologies.