-
Notifications
You must be signed in to change notification settings - Fork 664
[Bug] ESQL Remote Validation Data Stream and Patch Version Validation #6251
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
Changes from 5 commits
eb4069b
f67b60a
d4ca0b0
4d0a236
911c0b7
512229a
e3fb131
44220e1
d8bc779
3bc6a51
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 |
|---|---|---|
|
|
@@ -9,7 +9,7 @@ | |
| import gzip | ||
| import json | ||
| from collections import OrderedDict, defaultdict | ||
| from collections.abc import Iterator | ||
| from collections.abc import Iterable, Iterator | ||
| from pathlib import Path | ||
| from typing import TYPE_CHECKING, Any | ||
|
|
||
|
|
@@ -244,16 +244,57 @@ def _satisfies_kibana_range(stack: Version, version_requirement: str) -> bool: | |
| return any(lo <= stack and (hi is None or stack < hi) for lo, hi in _parse_kibana_range(version_requirement)) | ||
|
|
||
|
|
||
| def find_latest_integration_patch_for_minor(packages: Iterable[str], major: int, minor: int) -> int: | ||
| """Find the latest stack patch the given integration packages need for a major.minor.""" | ||
| # The stack-schema-map keys stacks at MAJOR.MINOR.0, but an integration may gate its latest | ||
|
eric-forte-elastic marked this conversation as resolved.
|
||
| # package (and newly-added data streams) behind a later patch (e.g. azure ~8.19.10). Resolving | ||
| # against the literal .0 falls back to an older package that predates the stream. Return the | ||
| # latest patch a package gates on for the minor, i.e. the stack patch needed to receive the most | ||
| # up-to-date integration package on that minor. Scan each package once and track the newest | ||
| # matching package manifest. | ||
| manifests = load_integrations_manifests() | ||
|
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. load_integrations_manifests() is called on every invocation, and it's called in a for version in get_stack_versions() loop in rule_validators.py. Can we think of caching this somewhere. If the function isn't memoized/cached, this is operationally heavy and does the same load over and over again per version. Cache or optimised calls of manifest loads is a good idea. Why this calls for optimised calls we have seen Manifest growing big with growing integration versions. Especially for AWS and Azure.
Contributor
Author
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. Unfortunately, no we cannot directly cache them given that each rule requirements (fields, versions, etc.) are different/potentially different per rule. Given this, we would need to evaluate them on a per rule level. Granted we could build a hash map as an optimization so if a rule has the exact same integration info passed that we do not need to compute it again, but the goal of this PR was to go for a less complex approach first, de-duplicate with #6208 and then polish as needed.
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. ++ Agreed. We should revisit this optimisations in future it will greatly enhance execution times. |
||
| latest_patch = 0 | ||
| for package in packages: | ||
| latest_package_version: Version | None = None | ||
| latest_package_patch = 0 | ||
| for package_version, manifest in manifests.get(package, {}).items(): | ||
| version_requirement = manifest.get("conditions", {}).get("kibana", {}).get("version") | ||
| if not version_requirement: | ||
| continue | ||
| try: | ||
| clauses = _parse_kibana_range(version_requirement) | ||
| except ValueError: | ||
| # Skip manifests whose kibana condition uses tokens we cannot parse. | ||
| continue | ||
| floors = [lo.patch for lo, _ in clauses if lo.major == major and lo.minor == minor] | ||
| if not floors: | ||
| continue | ||
| parsed_package_version = Version.parse(package_version) | ||
| if latest_package_version is None or parsed_package_version > latest_package_version: | ||
| latest_package_version = parsed_package_version | ||
| latest_package_patch = max(floors) | ||
| latest_patch = max(latest_patch, latest_package_patch) | ||
| return latest_patch | ||
|
|
||
|
|
||
| def find_least_compatible_version( | ||
| package: str, | ||
| integration: str, | ||
| integration: str | None, | ||
| current_stack_version: str, | ||
| packages_manifest: dict[str, Any], | ||
| ) -> str: | ||
| """Finds least compatible version for specified integration based on stack version supplied.""" | ||
| integration_manifests = dict(sorted(packages_manifest[package].items(), key=lambda x: Version.parse(x[0]))) | ||
| stack_version = Version.parse(current_stack_version, optional_minor_and_patch=True) | ||
|
|
||
| # The manifest's kibana condition only tells us whether the *package* installs on the stack, not | ||
| # whether this particular integration/data stream exists yet in that package version (e.g. azure | ||
| # added aadgraphactivitylogs in 1.37.0, but 1.0.0 already installs on 8.19). The schemas record | ||
| # the data streams present per package version, so use them to skip versions that predate the | ||
| # integration. Only filter when schema data exists for a version, otherwise fall back to kibana | ||
| # compatibility alone (e.g. for synthetic manifests in tests). | ||
| package_schemas = load_integrations_schemas().get(package, {}) | ||
|
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.
Contributor
Author
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. Yes, we are tracking this 👍
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. ++ im going to update the other PR after this lands
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. Thank you.
eric-forte-elastic marked this conversation as resolved.
Outdated
|
||
|
|
||
| # filter integration_manifests to only the latest major entries | ||
| major_versions = sorted( | ||
| {Version.parse(manifest_version).major for manifest_version in integration_manifests}, | ||
|
|
@@ -270,8 +311,11 @@ def find_least_compatible_version( | |
| sorted(major_integration_manifests.items(), key=lambda x: Version.parse(x[0])) | ||
| ).items(): | ||
| version_requirement = manifest["conditions"]["kibana"]["version"] | ||
| if _satisfies_kibana_range(stack_version, version_requirement): | ||
| return f"^{version}" | ||
| if not _satisfies_kibana_range(stack_version, version_requirement): | ||
| continue | ||
| if integration and version in package_schemas and integration not in package_schemas[version]: | ||
| continue | ||
| return f"^{version}" | ||
|
eric-forte-elastic marked this conversation as resolved.
|
||
|
|
||
| raise ValueError(f"no compatible version for integration {package}:{integration}") | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.