Add semantic validation for required variable groups#856
Add semantic validation for required variable groups#856jsoriano merged 6 commits intoelastic:mainfrom
Conversation
code/go/internal/validator/semantic/validate_required_vargroups_test.go
Outdated
Show resolved
Hide resolved
| d, err := fs.ReadFile(fsys, "manifest.yml") | ||
| if err != nil { | ||
| return specerrors.ValidationErrors{specerrors.NewStructuredErrorf("failed to read manifest: %w", err)} | ||
| } |
There was a problem hiding this comment.
Could these groups be defined in data stream manifests too?
If so, it would be needed to check for the required_vars there too
And another doubt, vars defined in the package manifest should also be merged in the vars defined in the data stream manifest for this validation?
There was a problem hiding this comment.
Umm, yeah, not sure. Let's wait for the related discussion in the spec change. I remember some complexities merging these configurations in Fleet, we'd have to mimic the behavior here.
There was a problem hiding this comment.
Added support for data streams, not sure if it handles all the places where variables can be defined, but we can add more cases later if there is any. Let me know if you can think about some additional case.
There was a problem hiding this comment.
Added support for data streams, not sure if it handles all the places where variables can be defined, but we can add more cases later if there is any. Let me know if you can think about some additional case.
I don't recall any other additional case now, thanks for adding this support!
seanrathier
left a comment
There was a problem hiding this comment.
Thanks for the follow up! Much appreciated
💚 Build Succeeded
History
cc @jsoriano |
What does this PR do?
Add semantic validation for required variable groups added in #855.
Why is it important?
Adds some checks that can be useful to prevent mistakes when configuring required variable groups.
Related issues