Fixed certificate template fetch failing with misleading DNS errors#42625
Fixed certificate template fetch failing with misleading DNS errors#42625
Conversation
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Pull request overview
Fixes the Android agent’s certificate template GET request behavior to avoid sending a Content-Type: application/json header on bodyless GETs, which can be rejected by intermediaries and surface as misleading DNS errors (per #42624).
Changes:
- Only set
Content-Type: application/jsonwhen a request actually writes a JSON body (non-GET with non-null body). - Remove dead code in
getCertificateTemplatethat previously constructed a request body for a GET. - Add a regression test asserting GET certificate template retries do not include a
Content-Typeheader.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| android/app/src/main/java/com/fleetdm/agent/ApiClient.kt | Moves Content-Type header setting into the body-writing path; removes unused GET-body code for certificate templates. |
| android/app/src/test/java/com/fleetdm/agent/ApiClientReenrollTest.kt | Adds assertion that GET requests (certificate template fetch) do not include Content-Type. |
| android/changes/42624-fix-certificate-template-get-content-type | Adds Android changelog entry for the fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughThis PR modifies the Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #42625 +/- ##
=======================================
Coverage 66.67% 66.67%
=======================================
Files 2533 2533
Lines 203216 203208 -8
Branches 9231 9228 -3
=======================================
- Hits 135485 135482 -3
+ Misses 55469 55465 -4
+ Partials 12262 12261 -1
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:
|
Related issue: Resolves #42624
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), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters.If paths of existing endpoints are modified without backwards compatibility, checked the frontend/CLI for any necessary changes
Testing
Added/updated automated tests
Where appropriate, automated tests simulate multiple hosts and test for host isolation (updates to one hosts's records do not affect another)
QA'd all new/changed functionality manually
For unreleased bug fixes in a release candidate, one of:
Database migrations
COLLATE utf8mb4_unicode_ci).New Fleet configuration settings
If you didn't check the box above, follow this checklist for GitOps-enabled settings:
fleetctl generate-gitopsfleetd/orbit/Fleet Desktop
runtime.GOOSis used as needed to isolate changesSummary by CodeRabbit