Skip software_id=0 and log, but otherwise complete counts, when counting host software on a host_software table including rows with software ID zero

[iansltx]

Fixes #30522.

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/ or ee/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)
• Added/updated automated tests
• Manual QA for all new/changed functionality

Summary by CodeRabbit

• Bug Fixes

  • Resolved an issue where host software counts were not updated if the database contained rows with a zero software ID.

• Tests

  • Enhanced tests to verify correct handling of host software records with a zero software ID.

[codecov]

Codecov Report

Attention: Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 64.15%. Comparing base (efa265f) to head (9d86d2b).
Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
cmd/fleet/vuln_process.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #30523   +/-   ##
=======================================
  Coverage   64.15%   64.15%           
=======================================
  Files        1874     1874           
  Lines      184168   184173    +5     
  Branches     5387     5387           
=======================================
+ Hits       118154   118158    +4     
- Misses      56697    56698    +1     
  Partials     9317     9317           

Flag	Coverage Δ
backend	65.03% <80.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[coderabbitai]

Walkthrough

The changes address an issue where host software counts were not updated if the host_software table contained rows with a software_id of zero. The fix involves skipping such entries during counting, logging a warning, and updating tests to ensure this scenario is correctly handled.

Changes

File(s)	Change Summary
server/datastore/mysql/software.go	Added logic to skip software_id 0 during host software count sync and log a warning.
server/datastore/mysql/software_test.go	Updated test to insert a software_id 0 row and verify correct handling and logging.
cmd/fleet/vuln_process.go	Updated MySQL datastore initialization to include a logger option.
changes/30522-zero-software-id-counts	Added changelog entry describing the fix for zero software_id in host software counts.

Sequence Diagram(s)

sequenceDiagram
    participant VulnerabilityJob
    participant Datastore
    participant Logger
    VulnerabilityJob->>Datastore: SyncHostsSoftware()
    Datastore->>Datastore: Query min/max software_id from host_software
    alt min_software_id == 0
        Datastore->>Logger: Log warning about software_id 0
        Datastore->>Datastore: Set min_software_id = 1
    end
    Datastore->>Datastore: Count host software (excluding software_id 0)
    Datastore-->>VulnerabilityJob: Return updated counts

Loading

Assessment against linked issues

Objective	Addressed	Explanation
Fix host software counts not updating when host_software includes a row with software_id 0 (#30522)	✅
Log a warning and skip software_id 0, increment min ID by 1 in sync logic (#30522)	✅

Assessment against linked issues: Out-of-scope changes

No out-of-scope changes found.

Poem

A software count with zero ID
Once caused the job to stall—
But now a warning’s logged with glee,
And counts proceed for all!
The rabbits cheer, the bug is gone,
Host numbers now are right—
🐇✨ Code hops along from dusk till dawn,
Fixing bugs with delight!

---

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between efa265f and 9d86d2b.

📒 Files selected for processing (4)

• changes/30522-zero-software-id-counts (1 hunks)
• cmd/fleet/vuln_process.go (1 hunks)
• server/datastore/mysql/software.go (1 hunks)
• server/datastore/mysql/software_test.go (2 hunks)

🧰 Additional context used

🧬 Code Graph Analysis (2)

cmd/fleet/vuln_process.go (1)

server/datastore/mysql/config.go (1)

• Logger (21-26)

server/datastore/mysql/software_test.go (1)

server/datastore/mysql/testing_utils.go (1)

• ExecAdhocSQL (412-416)

⏰ Context from checks skipped due to timeout of 90000ms (28)

• GitHub Check: test-go (vuln, ubuntu-latest, mysql:8.0.36, false)
• GitHub Check: test-go (main, ubuntu-latest, mysql:8.0.36, false)
• GitHub Check: test-go (vuln, ubuntu-latest, mysql:9.3.0, false)
• GitHub Check: test-go (service, ubuntu-latest, mysql:9.3.0, false)
• GitHub Check: test-go (service, ubuntu-latest, mysql:8.0.36, false)
• GitHub Check: test-go (mysql, ubuntu-latest, mysql:8.0.36, false)
• GitHub Check: test-go (main, ubuntu-latest, mysql:9.3.0, false)
• GitHub Check: test-go (scripts, ubuntu-latest, mysql:8.0.36, false)
• GitHub Check: test-go (integration-core, ubuntu-latest, mysql:8.0.36, false)
• GitHub Check: test-go (mysql, ubuntu-latest, mysql:9.3.0, false)
• GitHub Check: test-go (integration-enterprise, ubuntu-latest, mysql:9.3.0, false)
• GitHub Check: test-go (integration-mdm, ubuntu-latest, mysql:9.3.0, false)
• GitHub Check: test-go (fleetctl, ubuntu-latest, mysql:9.3.0, false)
• GitHub Check: test-go (fast, ubuntu-latest, mysql:8.0.36, false)
• GitHub Check: test-go (integration-mdm, ubuntu-latest, mysql:8.0.36, false)
• GitHub Check: test-go (fleetctl, ubuntu-latest, mysql:8.0.36, false)
• GitHub Check: test-go (integration-enterprise, ubuntu-latest, mysql:8.0.36, false)
• GitHub Check: test-go (integration-core, ubuntu-latest, mysql:9.3.0, false)
• GitHub Check: build-binaries
• GitHub Check: lint (windows-latest)
• GitHub Check: lint (ubuntu-latest)
• GitHub Check: publish
• GitHub Check: lint (macos-latest)
• GitHub Check: test-db-changes
• GitHub Check: build-and-check
• GitHub Check: Analyze (go)
• GitHub Check: Analyze (javascript)
• GitHub Check: check-doc-gen

🔇 Additional comments (5)

changes/30522-zero-software-id-counts (1)

1-1: LGTM! Clear and accurate changelog entry.

The changelog entry accurately describes the fix and provides sufficient context for users to understand what issue was resolved.

cmd/fleet/vuln_process.go (1)

55-55: LGTM! Proper addition of logger option.

The logger option is correctly added to enable logging functionality in the MySQL datastore, which supports the main fix for handling zero software_id entries. The implementation is clean and maintains existing error handling patterns.

server/datastore/mysql/software_test.go (2)

20-20: LGTM! Import addition is necessary and properly formatted.

The kitlog import is required for the logger manipulation in the test and follows Go naming conventions.

---

1461-1473: LGTM! Well-structured test for zero software_id edge case.

The test implementation effectively validates the PR objectives:

• Properly inserts a test record with software_id=0 to reproduce the edge case
• Uses safe parameterized SQL queries to prevent injection
• Appropriately suppresses expected warning logs to avoid test output pollution
• Follows established error handling patterns with require.NoError

The logger manipulation approach is reasonable for this test context where concurrent access isn't a concern.

server/datastore/mysql/software.go (1)

1868-1871: LGTM! Well-handled edge case for invalid software IDs.

This is a clean fix for the issue where software_id=0 entries prevented host software counts from updating. The solution:

1. ✅ Correctly skips invalid entries: Resetting minMax.Min to 1 ensures the batch counting loop excludes software_id=0
2. ✅ Maintains visibility: The warning log provides good observability for this edge case
3. ✅ Handles all scenarios: Works whether there are only zero IDs, mixed IDs, or no zero IDs at all
4. ✅ Minimal impact: Targeted change that doesn't affect normal operation

The approach of logging and continuing (rather than failing) is appropriate since these appear to be invalid/placeholder entries that should be excluded from counts anyway.

✨ Finishing Touches

• 📝 Generate Docstrings

---

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Explain this complex logic.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai explain this code block.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and explain its main purpose.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai generate docstrings to generate docstrings for this PR.
• @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[jahzielv]

good catch!