feat: added additional validators for custom command arguments - #1979
feat: added additional validators for custom command arguments#1979Benni0 wants to merge 18 commits into
Conversation
0800d46 to
c3944c2
Compare
…tion for custom command arguments.
c3944c2 to
1abdf45
Compare
kkedziak-splunk
left a comment
There was a problem hiding this comment.
Review Summary
This PR adds support for additional splunklib validators: Set, Match, List, Map, and Duration. The schema definitions, Python code generation, and test coverage are comprehensive.
Issues found:
-
Matchvalidatornamenot required in schema (schema.json): TheCustomMatchValidatorschema only requires["type", "pattern"]but not"name". However the Python code generation usesvalidate.get("name")and embeds it directly invalidators.Match('{name}', '{pattern}'). IfnameisNone, this generatesvalidators.Match('None', '...')— a string literal'None', not the PythonNone. Thenamefield should either be required in the schema, or the code generation should handle the missing case. -
Potential code injection in generated Python (
create_custom_command_python.py): TheMatchvalidator generatesvalidate=validators.Match('{name}', '{pattern}')using f-strings with user-provided values. If a pattern contains a single quote (e.g.^[a-z]+'s$), it would break the generated Python syntax. Similarly for thenamefield. The values should be properly escaped or userepr()for safe string embedding. -
Setvalidator code generation usesstr()+strip('[]'):str(allowed_values).strip('[]')works for simple strings but could produce unexpected output if values contain special characters (e.g. commas, quotes). Using', '.join(repr(v) for v in allowed_values)would be more robust. -
Mapvalidator code generation usesstr(option_map):f"validate=validators.Map(**{str(option_map)})"relies on Python'sstr()representation of a dict matching valid Python syntax. This works for simple types but is fragile. Consider usingrepr()instead. -
Double period in schema description (
schema.json,CustomMapValidator): Description says"...which is replaced by the value.."— double period at the end. -
Doc says
"some types"instead of a count (custom_search_commands.md): Changed from "five types" to "some types" — consider listing the actual count (10 types) or saying "the following types".
Good work adding comprehensive validator support with thorough tests.
…s are defined but search assistant is not required
…stead of custom quoting
91e20ba to
9d1d99b
Compare
7951b2f to
971576d
Compare
|
Hi @kkedziak-splunk, |
PR Type
What kind of change does this PR introduce?
Summary
Adds support for the argument validators Set, Match, List, Map and Duration for custom commands.
Please review Pull Request #1977 first.
Changes
Updated the
schema.jsonto support the new argument validators with their required properties. Also changed the python command generator to generate the appropriate code.User experience
After this update the user has more possibilities to validate and parse custom command arguments:
Checklist
If an item doesn't apply to your changes, leave it unchecked.
Review
Tests
See the testing doc.
Demo/meeting:
Reviewers are encouraged to request meetings or demos if any part of the change is unclear