elastic / package-spec

EPR package specifications
Other
17 stars 70 forks source link

[DX] Add reference documentation for each validation code #747

Open andrewkroh opened 3 months ago

andrewkroh commented 3 months ago

As someone who does not work we package-spec validation issues on a daily basis, I have found some of the errors to be confusing and difficult to find reference information about. My suggestion is to add reference documentation in this repo for each validation code (e.g. SVR00004, JSE00001) much like what exists for Go's staticcheck. The docs would contain information like

Line numbers in these validation messages would be super helpful too.

Examples

validation error:
   1. references found in dashboard kibana/dashboard/foo-749203a0-67b1-11ea-a76f-bf44814e437d.json: foo-21028750-67ca-11ea-a76f-bf44814e437d (search) (SVR00004)

It would save developer time if we could point to docs that explain what "references" means in this context, why they are a problem, and how to fix it (like pointing developers toward https://github.com/elastic/visualizations_integrations_tools?tab=readme-ov-file#inliner-usage).


found 2 validation errors:
   1. file "/Users/akroh/code/elastic/integrations/packages/okta/data_stream/system/elasticsearch/ingest_pipeline/default.yml" is invalid: field processors.133.remove.field: rename "message" to "event.original" processor requires remove "message" processor (JSE00001)
   2. file "/Users/akroh/code/elastic/integrations/packages/okta/data_stream/system/elasticsearch/ingest_pipeline/default.yml" is invalid: field processors.133.remove.if: rename "message" to "event.original" processor requires remove "message" processor with if: 'ctx.event?.original != null' (JSE00001)

I can't really make sense of what these two errors are telling me to fix so having docs would save time because today I need to go lookup the commit that added this validation to see what it's addressing (which I understand now that I saw the PR and tests). And perhaps the validation messages could be improved too.

  1. ... rename ... requires ... remove

The processor in question is not a rename processor so I'm immediately questioning the validation.