Updated osquery perf for #45550#48935
Conversation
|
@coderabbitai full review |
|
/agentic_review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Pull request overview
Updates osquery-perf to better simulate Windows MDM SCEP certificate issuance and reporting so Fleet can observe the expected certificate markers during perf/load scenarios related to Windows SCEP proxy behavior.
Changes:
- Track and report certificates issued via Windows MDM SCEP exchanges in the
certificates_windowsdetail query output. - Keep SCEP-issued cert reporting stable across refreshes (keyed/sorted by SCEP CSP unique ID).
- Add a small (~5%) population of simulated hosts that intentionally withhold reporting their SCEP-issued certs to exercise the server’s verification backstop path.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| cmd/osquery-perf/certificates.go | Adds storage/serialization of SCEP-issued certs into the simulated certificates detail query. |
| cmd/osquery-perf/agent.go | Adds per-agent SCEP cert withholding behavior and stores issued SCEP certs on successful exchanges for later reporting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
WalkthroughThis change updates the osquery-perf agent to simulate Windows MDM SCEP certificate behavior. The agent now tracks issued SCEP certificates per host, marks a deterministic subset of hosts as withholding them, and skips storing nil or withheld SCEP results during check-in. New helpers convert X.509 certificates into simulated certificate specs, store them under a mutex, and include them in generated certificate specs in sorted order. Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Code Review by Qodo
1.
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #48935 +/- ##
==========================================
- Coverage 68.11% 68.10% -0.02%
==========================================
Files 3709 3709
Lines 235118 235153 +35
Branches 12464 12464
==========================================
Hits 160148 160148
- Misses 60594 60631 +37
+ Partials 14376 14374 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/osquery-perf/agent.go`:
- Around line 737-738: The withholding cohort is currently derived from
agentIndex, which only selects every shard’s local 20th agent instead of a
consistent global 5% sample. Update the withholdSCEPCert assignment in agent.go
to base the decision on the global host index using hostIndexOffset plus
agentIndex, so the test-only failure cohort is stable across distributed runs.
Use the existing hostIndexOffset and agentIndex symbols in the same
host-generation logic to locate and adjust the condition.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ce173bc4-5c56-4f99-b13a-31769380eebf
📒 Files selected for processing (2)
cmd/osquery-perf/agent.gocmd/osquery-perf/certificates.go
🚧 Files skipped from review as they are similar to previous changes (1)
- cmd/osquery-perf/certificates.go
| // Every 20th host (5%) withholds its issued SCEP certs to exercise the server's verification backstop (test-only failure). | ||
| withholdSCEPCert: agentIndex%20 == 0, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant file and nearby context.
git ls-files cmd/osquery-perf/agent.go
wc -l cmd/osquery-perf/agent.go
sed -n '680,780p' cmd/osquery-perf/agent.go
# Find definitions and call sites for the relevant identifiers.
rg -n "withholdSCEPCert|agentIndex|hostIndexOffset|newAgent\(" cmd/osquery-perf/agent.go cmd/osquery-perf -SRepository: fleetdm/fleet
Length of output: 8286
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files cmd/osquery-perf/agent.go
wc -l cmd/osquery-perf/agent.go
sed -n '680,780p' cmd/osquery-perf/agent.go
rg -n "withholdSCEPCert|agentIndex|hostIndexOffset|newAgent\(" cmd/osquery-perf/agent.go cmd/osquery-perf -SRepository: fleetdm/fleet
Length of output: 8286
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the newAgent signature and the call sites that pass hostIndexOffset / agentIndex.
rg -n "func newAgent|newAgent\(" cmd/osquery-perf/agent.go cmd/osquery-perf -S -A 4 -B 4
# Show any calculations that define agentIndex or hostIndexOffset in the same file.
rg -n "hostIndexOffset|agentIndex" cmd/osquery-perf/agent.go -S -A 3 -B 3Repository: fleetdm/fleet
Length of output: 7544
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read only the relevant slice with line numbers.
nl -ba cmd/osquery-perf/agent.go | sed -n '700,760p'
# Show the declaration/call site context for hostIndexOffset and agentIndex.
rg -n "hostIndexOffset|agentIndex|withholdSCEPCert" cmd/osquery-perf/agent.go -n -A 6 -B 6Repository: fleetdm/fleet
Length of output: 191
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path("cmd/osquery-perf/agent.go")
lines = p.read_text().splitlines()
for start, end in [(2168,2208),(3884,3924),(728,742)]:
print(f"\n--- {start}-{end} ---")
for i in range(start, end+1):
if i <= len(lines):
print(f"{i:4d}: {lines[i-1]}")
PYRepository: fleetdm/fleet
Length of output: 4188
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path("cmd/osquery-perf/agent.go")
lines = p.read_text().splitlines()
for start, end in [(3648, 3672), (3880, 3935), (2188, 2210)]:
print(f"\n--- {start}-{end} ---")
for i in range(start, end + 1):
if i <= len(lines):
print(f"{i:4d}: {lines[i-1]}")
PYRepository: fleetdm/fleet
Length of output: 5219
Base the withholding cohort on the global host index. agentIndex is 1-based, so agentIndex%20 == 0 only withholds on each shard’s local 20th agent. Use (hostIndexOffset + agentIndex - 1) % 20 == 0 so the 5% cohort is consistent across distributed runs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/osquery-perf/agent.go` around lines 737 - 738, The withholding cohort is
currently derived from agentIndex, which only selects every shard’s local 20th
agent instead of a consistent global 5% sample. Update the withholdSCEPCert
assignment in agent.go to base the decision on the global host index using
hostIndexOffset plus agentIndex, so the test-only failure cohort is stable
across distributed runs. Use the existing hostIndexOffset and agentIndex symbols
in the same host-generation logic to locate and adjust the condition.
Related issue: Resolves #45550
Checklist for submitter
Summary by CodeRabbit
New Features
Bug Fixes
Testing