Semantic validations

[ycombinator]

What does this PR do?

This PR introduces semantic validations to the Package Spec. Semantic validations are validations of the package spec that cannot be achieved simply by adding declarative rules to the package spec, but instead may require some amount of programming to express the rules.

To demonstrate how semantic validations can be written, this PR implements the following semantic validation rule:

Validate that the id fields inside Kibana object files have the same value as the object filename (minus the .json extension, of course)

A follow up PR will implement a second semantic validation rule:

Validate that there are no dangling references to Kibana objects in any Kibana object files within the scope of a package. That is, if a Kibana object file in package P references another Kibana object with ID i, then there must exist a Kibana object file for id i in package P

Why is it important?

To enforce semantic constraints on package elements.

Checklist

• I have added test packages to test/packages that prove my change is effective.
• I have added an entry in versions/N/changelog.yml. There is no change to the spec.

Related issues

• Related [META] Semantic validations #47
• Related Validate that Kibana asset IDs start with lowercase package names #65

[elasticmachine]

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

• Build Cause: Pull request #160 updated

• Start Time: 2021-04-21T20:12:10.513+0000

• Duration: 7 min 2 sec

• Commit: 63a6d6d

Trends 🧪

[mtojek]

I left a couple of questions/ideas in comments. The idea is great as it's much simpler than building a dedicated AST.

We need to remember that it will produce additional technical debt on porting the validator to other languages (but it's acceptable).

As this PR changes interfaces for package-spec, which approach is better:

1. Push this PR to master, then to elastic-package, and integrations (I assume you'll have to fix some packages) and then iterate on new rules.
2. Push this PR to master, add more rules (at least the TODO one) and then push everything further.

[ycombinator]

We need to remember that it will produce additional technical debt on porting the validator to other languages (but it's acceptable).

Agreed. This is why I created a new Golang package, semantic, which is where we should keep all these semantic validation implementations. That way, at least it's easy to find all the semantic validations in a single place when porting them over to another language.

BTW, I think we have a similar issue with the format checkers implementation. Maybe we should move all format checker code into this semantic package as well? WDYT?

[ycombinator]

As this PR changes interfaces for package-spec, which approach is better:

1. Push this PR to master, then to elastic-package, and integrations (I assume you'll have to fix some packages) and then iterate on new rules.
2. Push this PR to master, add more rules (at least the TODO one) and then push everything further.

I think option 1 is better, especially since there may be fixes to some integrations packages as a result. That way the PR to integrations can not only update the elastic-package dependency but also have these fixes, but also remain relatively small in size. Plus this way we can rollout at least one semantic validation out sooner.

Later we can similarly make a second set of PRs for the second semantic validation rule (the one marked as TODO in this PR).

[mtojek]

BTW, I think we have a similar issue with the format checkers implementation. Maybe we should move all format checker code into this semantic package as well? WDYT?

SGTM, you can open an issue to refactor them too or modify in one next PRs (up to you).

[ycombinator]

SGTM, you can open an issue to refactor them too or modify in one next PRs (up to you).

Okay, I will do this in a follow up PR soon after this one is merged.

[ycombinator]

@mtojek I made a non-trivial change to make CI green again on this PR: 4230c1a. Please re-review. Thanks!