Schema adherence validator

[l50]

Proposed Changes

• Created YAML schema for TTPs (docs/ttpforge-spec.yaml)
• Streamlined the YAML validation process, introducing enhanced clarity and comprehensive error messaging.
• Consolidated YAML validation into a shared function and introduced a pre-commit hook mechanism to preemptively prevent committing non-conforming YAML files.
• Updated .pre-commit-config.yaml to include the new validate-yaml-files hook and added a schema file ttpforge-spec.yaml for TTP YAML files structure and properties definition.
• Enhanced magefile functions for more robust validation and improved handling of various scenarios.

Related Issue(s)

N/A

Testing

• Introduced a pre-commit hook script validate-yaml.sh to validate all committed YAML files against a predefined schema.
• Added and improved functions in magefile.go, such as ValidateYAML, which leverages the newly defined schema for validation.
• Dependencies were updated and tested for compatibility.
• Detailed testing information can be found here: Schema adherence validator #77 (comment)

Documentation

• Updated the documentation to include a new schema file ttpforge-spec.yaml under the docs/ directory.
• The new validate-yaml-files hook was documented in .pre-commit-config.yaml.
• Additional documentation regarding enhanced validation and error messaging was integrated into relevant function definitions and usage scenarios.

Screenshots/GIFs (optional)

N/A

Checklist

• Ran mage runprecommit locally and fixed any issues that arose.
• Curated your commit(s) so they are legible and easy to read and understand.
• 🚀

[l50]

This PR is a blocking pre-requisite for facebookincubator/TTPForge#83

[d3sch41n]

Hey there, could you please post a couple (3-ish) screenshots of what different kinds of YAML validation failures look like to the end user who runs pre-commit.

This will help determine whether this should be a user-facing feature baked into pre-commit or something used ad-hoc for ART imports.

[l50]

Hey there, could you please post a couple (3-ish) screenshots of what different kinds of YAML validation failures look like to the end user who runs pre-commit.

This will help determine whether this should be a user-facing feature baked into pre-commit or something used ad-hoc for ART imports.

Absolutely! Here's an example of how we can get this to flag an invalid TTP:

cp ttps/examples/sub-ttps/example.yaml ttps/examples/sub-ttps/broken-example.yaml

Next, we make the following changes to broken-example.yaml:

diff ttps/examples/sub-ttps/example.yaml ttps/examples/sub-ttps/broken-example.yaml
5,6c5
<   - name: first_sub_ttp
<     ttp: examples/sub-ttps/my-sub-ttps/ttp1.yaml
---
>   - ttp: examples/sub-ttps/my-sub-ttps/ttp1.yaml

As you can see, the step no longer has a name, which is not going to work.

Next, we add the commit:

git add ttps/examples/sub-ttps/broken-example.yaml

When we run git commit, the following feedback will be provided by pre-commit:

...
Validate all committed YAML files against TTPForge schema.........................Failed
- hook id: validate-yaml-files
- exit code: 1

Files to validate: ttps/examples/sub-ttps/broken-example.yaml
Error: the "name" property is missing when "ttp" is defined in a step
Failed validation for: ttps/examples/sub-ttps/broken-example.yaml

Here's another example:

cp ttps/examples/steps/edit-step/ttp.yaml ttps/examples/steps/edit-step/broken-ttp.yaml 

Next, we make the following changes to broken-ttp.yaml:

diff ttps/examples/steps/edit-step/ttp.yaml ttps/examples/steps/edit-step/broken-ttp.yaml
9,10c9
<       - old: REPLACE_ME
<         new: REPLACED_BY_EDIT
---
>       - new: REPLACED_BY_EDIT

As you can see, the step no longer has an old step, which is not going to work.

Next, we add the commit:

git add ttps/examples/steps/edit-step/broken-ttp.yaml

When we run git commit, the following feedback will be provided:

Validate all committed YAML files against TTPForge schema.........................Failed
- hook id: validate-yaml-files
- exit code: 1

Files to validate: ttps/examples/steps/edit-step/broken-ttp.yaml
Error: an edit item is missing the 'old' field
Failed validation for: ttps/examples/steps/edit-step/broken-ttp.yaml

For the third example:

cp ttps/examples/args/define-args.yaml ttps/examples/args/broken-args.yaml

Next, we make the following changes to broken-args.yaml:

diff ttps/examples/args/define-args.yaml ttps/examples/args/broken-args.yaml
7,8c7
<   - name: a_number
<     type: int
---
>   - type: int

As you can see, the step no longer has a name, which is not going to work.

Next, we add the commit:

git add ttps/examples/args/broken-args.yaml

When we run git commit, the following feedback will be provided:

Validate all committed YAML files against TTPForge schema.........................Failed
- hook id: validate-yaml-files
- exit code: 1

Files to validate: ttps/examples/args/broken-args.yaml
Error: an argument item is missing the 'name' field
Failed validation for: ttps/examples/args/broken-args.yaml

[d3sch41n]

Thank you for the test cases. Seems like there are two possible approaches:

1. Validate adherence within the existing Validate(...) functions of the TTP and steps
   • Pros - can add more complex validation conditions, since we can write them in golang
   • Cons - potentially more verbose because the YAML setup we are using doesn't make it easy to mark fields as required
2. Use a schema as shown here
   • Pros - pretty good error messages out of the box
   • Cons - need to write everything twice, once in the YAML config for the structs within Go, and once here. More dev overhead

The existing code catches all of these test cases, but it is only called from run, so we'd need to add a skeleton validate command to TTPForge to use it in this context:

I'm cool with stamping this as an experiment that will tell us which approach is better, but I'm maxed out so @l50 you'll need to assume sole responsibility for keeping this schema up to date. Cool?

[l50]

I'm good with ownership of the schema to land it for now. I plan to automate its generation eventually - will just take some concerted effort on my part.

In the meantime, I've cut an FR to track it: #80

[d3sch41n]

cool, ship

[d3sch41n]

fat fingered close - my bad, LGTM after discussion on schema ownership