Skip to content
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

Enhance OS version parsing to handle SUSE SP notation #248

Merged
merged 4 commits into from
Dec 2, 2024

Conversation

sidis-zafran
Copy link
Contributor

Description

This PR enhances the versionGrok regex pattern within makeOSInfo to support SUSE Linux Enterprise Server (SLES) versions that use SP notation for Service Packs, such as 15-SP5.

The updated regex now recognizes SP as a valid separator for the minor version, allowing accurate parsing of SLES versions alongside standard major.minor.patch formats (e.g., 22.04). This change resolves issues where Minor was incorrectly set to 0 for SP-based versions and ensures compatibility with both SUSE-specific and conventional version formats.

Changes

Updated versionGrok regex: Added optional SP notation support as a prefix for the minor version.
Compatibility maintained: Supports both . and - delimiters to handle multiple versioning schemes.

Testing

Test cases added: Included tests for both SLES 15-SP5 and SLED 15-SP5 to validate correct assignment of major, minor, and patch values.

…ats and SUSE-specific SP notation for minor versions.
@sidis-zafran sidis-zafran changed the title Enhance version regex to support both standard major.minor.patch form… Enhance OS version parsing to handle SUSE SP notation Nov 13, 2024
Copy link

@MaorRayzinZ MaorRayzinZ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good (not that anybody can read regex)

@andrewkroh
Copy link
Member

not that anybody can read regex

That also means the maintainability of the regex will suffer. I would like to see another revision where the code is modified to have a separate regex for suse. I think is will be more readable and better to maintain. Then either we try each regex to see which one matches OR we use the ID_LIKE="suse" to choose the right regex to apply.

@andrewkroh andrewkroh added the enhancement New feature or request label Nov 20, 2024
Use extractVersionDetails method
@sidis-zafran
Copy link
Contributor Author

not that anybody can read regex

That also means the maintainability of the regex will suffer. I would like to see another revision where the code is modified to have a separate regex for suse. I think is will be more readable and better to maintain. Then either we try each regex to see which one matches OR we use the ID_LIKE="suse" to choose the right regex to apply.

Totally agreed. Updated with slight changes.

@sidis-zafran
Copy link
Contributor Author

@andrewkroh
Added release-note. Please rerun checks so we may merge this one 🙏

andrewkroh
andrewkroh previously approved these changes Dec 1, 2024
.changelog/248.txt Outdated Show resolved Hide resolved
Co-authored-by: Andrew Kroh <[email protected]>
@andrewkroh andrewkroh merged commit 43f710e into elastic:main Dec 2, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants