Remove duplicate RHEL kernel in os_version#39746
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #39746 +/- ##
==========================================
- Coverage 66.25% 66.25% -0.01%
==========================================
Files 2438 2439 +1
Lines 195270 195288 +18
Branches 8605 8605
==========================================
+ Hits 129384 129392 +8
- Misses 54171 54177 +6
- Partials 11715 11719 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
WalkthroughThis pull request fixes duplicate kernel reporting for RHEL-family distributions by adding a database migration that unmarks the kernel-core RPM package as a kernel, consolidating kernel identification to use only the "kernel" package name. The migration updates the software_titles table to set is_kernel = 0 for kernel-core entries and deletes corresponding kernel_host_counts rows. The kernel detection logic in osquery utilities is updated to use a single rpmKernelName constant instead of separate legacy constants. Supporting test files and schema references are updated accordingly. Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@server/datastore/mysql/migrations/tables/20260211200153_UnmarkKernelCoreAsKernel_test.go`:
- Line 90: The assertion message for the require.Equal call is wrong: it asserts
require.Equal(t, 1, finalCount, ...) but the message says "Should have 2 ...".
Update the message on the require.Equal that checks finalCount in
UnmarkKernelCoreAsKernel_test.go (the require.Equal invocation) to reflect 1
(e.g., "Should have 1 kernel_host_counts entry after migration (one deleted)")
so the expectation and message match.
- Line 43: The assertion message in the test referencing require.Equal(t, 2,
initialCount, ...) is wrong — it says "Should have 3 kernel_host_counts entries
before migration" while the expected count is 2; update the message text in the
require.Equal call (the one that checks initialCount) to correctly state "Should
have 2 kernel_host_counts entries before migration" so the failure message
matches the expected value and the inserted rows.
| var initialCount int | ||
| err = db.Get(&initialCount, `SELECT COUNT(*) FROM kernel_host_counts`) | ||
| require.NoError(t, err) | ||
| require.Equal(t, 2, initialCount, "Should have 3 kernel_host_counts entries before migration") |
There was a problem hiding this comment.
Assertion message is incorrect — says "3" but should say "2".
Only two kernel_host_counts rows are inserted, and the assertion correctly checks for 2, but the message string says "Should have 3". This will produce a confusing failure message if the assertion ever fails.
Proposed fix
- require.Equal(t, 2, initialCount, "Should have 3 kernel_host_counts entries before migration")
+ require.Equal(t, 2, initialCount, "Should have 2 kernel_host_counts entries before migration")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| require.Equal(t, 2, initialCount, "Should have 3 kernel_host_counts entries before migration") | |
| require.Equal(t, 2, initialCount, "Should have 2 kernel_host_counts entries before migration") |
🤖 Prompt for AI Agents
In
`@server/datastore/mysql/migrations/tables/20260211200153_UnmarkKernelCoreAsKernel_test.go`
at line 43, The assertion message in the test referencing require.Equal(t, 2,
initialCount, ...) is wrong — it says "Should have 3 kernel_host_counts entries
before migration" while the expected count is 2; update the message text in the
require.Equal call (the one that checks initialCount) to correctly state "Should
have 2 kernel_host_counts entries before migration" so the failure message
matches the expected value and the inserted rows.
| var finalCount int | ||
| err := db.Get(&finalCount, `SELECT COUNT(*) FROM kernel_host_counts`) | ||
| require.NoError(t, err) | ||
| require.Equal(t, 1, finalCount, "Should have 2 kernel_host_counts entries after migration (one deleted)") |
There was a problem hiding this comment.
Assertion message is incorrect — says "2" but should say "1".
Same issue: the assertion checks for 1 (correct), but the message says "Should have 2".
Proposed fix
- require.Equal(t, 1, finalCount, "Should have 2 kernel_host_counts entries after migration (one deleted)")
+ require.Equal(t, 1, finalCount, "Should have 1 kernel_host_counts entry after migration (one deleted)")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| require.Equal(t, 1, finalCount, "Should have 2 kernel_host_counts entries after migration (one deleted)") | |
| require.Equal(t, 1, finalCount, "Should have 1 kernel_host_counts entry after migration (one deleted)") |
🤖 Prompt for AI Agents
In
`@server/datastore/mysql/migrations/tables/20260211200153_UnmarkKernelCoreAsKernel_test.go`
at line 90, The assertion message for the require.Equal call is wrong: it
asserts require.Equal(t, 1, finalCount, ...) but the message says "Should have 2
...". Update the message on the require.Equal that checks finalCount in
UnmarkKernelCoreAsKernel_test.go (the require.Equal invocation) to reflect 1
(e.g., "Should have 1 kernel_host_counts entry after migration (one deleted)")
so the expectation and message match.
Related issue: Resolves #39737
Checklist for submitter
If some of the following don't apply, delete the relevant line.
Changes file added for user-visible changes in
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.
Input data is properly validated,
SELECT *is avoided, SQL injection is prevented (using placeholders for values in statements)Testing
Added/updated automated tests
QA'd all new/changed functionality manually
Database migrations
Summary by CodeRabbit
Bug Fixes
Tests