-
Notifications
You must be signed in to change notification settings - Fork 663
[Bug] ESQL Remote Validation Ignoring Rule Min-Stack #6223
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -46,7 +46,12 @@ | |
| parse_datasets, | ||
| ) | ||
| from .rule import EQLRuleData, QueryRuleData, QueryValidator, RuleMeta, TOMLRuleContents, set_eql_config | ||
| from .schemas import get_latest_stack_version, get_stack_schemas, get_stack_versions | ||
| from .schemas import ( | ||
| get_latest_stack_version, | ||
| get_min_supported_stack_version, | ||
| get_stack_schemas, | ||
| get_stack_versions, | ||
| ) | ||
| from .schemas.definitions import ESQL_DYNAMIC_FIELD_PREFIXES, FROM_SOURCES_REGEX | ||
|
|
||
| EQL_ERROR_TYPES = ( | ||
|
|
@@ -924,10 +929,21 @@ def remote_validate_rule( # noqa: PLR0913 | |
| # mismatch error, as the EsqlSchemaError and EsqlSyntaxError errors from the stack | ||
| # will not be impacted by the difference in schema type mapping. | ||
| mappings_lookup: dict[str, dict[str, Any]] = {stack_version: combined_mappings} | ||
| versions = get_stack_versions() | ||
| for version in versions: | ||
| # Only validate against stack versions the rule actually targets. A rule floored at | ||
| # min_stack_version is never backported below it, so building mappings for older stacks | ||
| # resolves integration packages that predate the rule's data sources (e.g. a new data | ||
| # stream) and raises spurious EsqlUnknownIndexError. Fall back to the minimum supported | ||
| # stack when the rule does not pin a min_stack_version. | ||
| min_stack = Version.parse( | ||
| str(metadata.min_stack_version or get_min_supported_stack_version()), | ||
| optional_minor_and_patch=True, | ||
| ) | ||
| for version in get_stack_versions(): | ||
| if version in mappings_lookup: | ||
| continue | ||
| if Version.parse(version) < min_stack: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If for instance we min stack a rule to 9.4.1, it may not go through this section of the validation if the latest stack version is 9.4.0. Not inherently a problem, just that we need to be sure that the min stacks will not be a min stacked to a version newer than the latest version in the manifest. |
||
| self.log(f"Skipping {version} stack: below rule min_stack_version {min_stack}") | ||
| continue | ||
| _, _, combined_mappings = prepare_mappings( | ||
| elastic_client, indices, event_dataset_integrations, metadata, version, self.log | ||
| ) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit. Is
get_min_supported_stack_versionneeded? Its loading from the schema andget_stack_versionsalso loads verbatim from the schema so it should never be different right?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also
get_min_supported_stack_versionalready returns a Version object so the cast to string and then back to Version is unnecessary.