produce failed enrollment renewal activity#44511
Merged
MagnusHJensen merged 3 commits intoMay 1, 2026
Merged
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
MagnusHJensen
had a problem deploying
to
Docker Hub
GitHub Actions
Failure
Contributor
WalkthroughAdds a new activity type 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 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. Review rate limit: 6/8 reviews remaining, refill in 7 minutes and 46 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/fleet/activities.go`:
- Around line 1988-1996: The new struct
ActivityTypeFailedEnrollmentProfileRenewal and its ActivityName() method were
added but not registered in the ActivityDetailsList used for docs generation;
update the ActivityDetailsList to include an entry for
ActivityTypeFailedEnrollmentProfileRenewal keyed by its ActivityName() (i.e.,
"failed_enrollment_profile_renewal") with the appropriate details/description
and example payload so it appears in generated activity documentation. Locate
the ActivityDetailsList declaration and add the mapping referencing
ActivityTypeFailedEnrollmentProfileRenewal and its fields to maintain consistent
docs generation.
🪄 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: f3f3b203-cbe4-41a4-a123-e81ec445ea4e
📒 Files selected for processing (11)
changes/40623-failed-enrollment-renewalserver/datastore/mysql/apple_mdm.goserver/datastore/mysql/hosts.goserver/fleet/activities.goserver/fleet/datastore.goserver/fleet/hosts.goserver/mdm/apple/profile_verifier.goserver/mdm/lifecycle/lifecycle.goserver/mock/datastore_mock.goserver/service/apple_mdm.goserver/service/integration_mdm_test.go
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds an activity record when an Apple enrollment profile renewal (SCEP/ACME) fails to install, enabling visibility into renewal failures (Resolves #41418).
Changes:
- Introduces a new activity type for failed enrollment profile renewal events.
- Detects “enrollment renewal” install-profile failures via datastore lookup and emits an activity instead of surfacing a not-found error.
- Adds an integration test covering when the activity should/shouldn’t be produced.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| server/mdm/apple/profile_verifier.go | Emits the new activity when a failed InstallProfile result matches an enrollment renewal command. |
| server/datastore/mysql/apple_mdm.go | Adds datastore query to determine whether a command UUID is an enrollment renewal command for a host. |
| server/service/apple_mdm.go | Passes the service activity function down to the profile install result handler. |
| server/fleet/activities.go | Defines failed_enrollment_profile_renewal activity payload and metadata. |
| server/fleet/datastore.go | Extends datastore interfaces to support renewal-command detection and host-lite lookup. |
| server/fleet/hosts.go | Adds HostLite.DisplayName() and fields needed to compute it. |
| server/datastore/mysql/hosts.go | Loads additional HostLite fields from DB. |
| server/mock/datastore_mock.go | Extends datastore mock to support the new method. |
| server/service/integration_mdm_test.go | Adds integration coverage for activity emission rules. |
| changes/40623-failed-enrollment-renewal | Documents the user-visible change. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
MagnusHJensen
temporarily deployed
to
Docker Hub
GitHub Actions
Inactive
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #44511 +/- ##
==========================================
+ Coverage 66.65% 66.77% +0.11%
==========================================
Files 2645 2637 -8
Lines 212854 212402 -452
Branches 9610 9549 -61
==========================================
- Hits 141888 141832 -56
+ Misses 58091 57697 -394
+ Partials 12875 12873 -2
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:
|
JordanMontgomery
previously approved these changes
May 1, 2026
5 tasks
JordanMontgomery
pushed a commit
that referenced
this pull request
May 1, 2026
This is no longer used, but gets flagged by AI. #44511 (comment) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Removed a now-redundant exported activity list from the codebase, simplifying internal activity declarations. This streamlines internal structures without changing user-visible behavior or altering existing activity types. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
JordanMontgomery
approved these changes
May 1, 2026
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
server/service/integration_mdm_test.go (1)
24217-24227: ⚡ Quick winAssert expected negative-path errors instead of discarding them.
Line 24217 and Line 24226 ignore
HandleHostMDMProfileInstallResulterrors, which can hide unrelated regressions. Prefer asserting the expected not-found behavior in both cases.Suggested test tightening
- _ = apple_mdm.HandleHostMDMProfileInstallResult(ctx, s.ds, host.UUID, case1Cmd, &failed, "boom", s.fleetSvc.NewActivity) + err := apple_mdm.HandleHostMDMProfileInstallResult(ctx, s.ds, host.UUID, case1Cmd, &failed, "boom", s.fleetSvc.NewActivity) + require.Error(t, err) + require.True(t, fleet.IsNotFound(err)) ... - _ = apple_mdm.HandleHostMDMProfileInstallResult(ctx, s.ds, host.UUID, case2OtherCmd, &failed, "boom", s.fleetSvc.NewActivity) + err = apple_mdm.HandleHostMDMProfileInstallResult(ctx, s.ds, host.UUID, case2OtherCmd, &failed, "boom", s.fleetSvc.NewActivity) + require.Error(t, err) + require.True(t, fleet.IsNotFound(err))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/service/integration_mdm_test.go` around lines 24217 - 24227, The test currently ignores errors returned by apple_mdm.HandleHostMDMProfileInstallResult for the negative cases (case1Cmd and case2OtherCmd); capture the returned error (err := apple_mdm.HandleHostMDMProfileInstallResult(...)) and assert it matches the expected not-found/failure type instead of discarding it (use require.Error/require.ErrorIs or require.ErrorContains as appropriate), doing this for both the call with case1Cmd and the call with case2OtherCmd so regressions are surfaced.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@server/service/integration_mdm_test.go`:
- Around line 24217-24227: The test currently ignores errors returned by
apple_mdm.HandleHostMDMProfileInstallResult for the negative cases (case1Cmd and
case2OtherCmd); capture the returned error (err :=
apple_mdm.HandleHostMDMProfileInstallResult(...)) and assert it matches the
expected not-found/failure type instead of discarding it (use
require.Error/require.ErrorIs or require.ErrorContains as appropriate), doing
this for both the call with case1Cmd and the call with case2OtherCmd so
regressions are surfaced.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3f3edc3e-cbff-4e59-95a9-aca947e2139d
📒 Files selected for processing (4)
server/fleet/activities.goserver/fleet/datastore.goserver/mock/datastore_mock.goserver/service/integration_mdm_test.go
✅ Files skipped from review due to trivial changes (1)
- server/fleet/activities.go
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
MagnusHJensen
added a commit
that referenced
this pull request
May 1, 2026
…44530) Backend PR: #44511 <!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **Related issue:** Resolves #41422 <img width="618" height="244" alt="image" src="https://github.com/user-attachments/assets/c223e37d-7051-46a6-a2ea-6bd1bdcbb53e" /> <img width="777" height="780" alt="image" src="https://github.com/user-attachments/assets/3b9ef4e9-2181-406b-a22e-e6773eba67af" /> <img width="649" height="236" alt="image" src="https://github.com/user-attachments/assets/3985faf0-a1e4-404a-b190-cb623f52339a" /> <img width="1083" height="768" alt="image" src="https://github.com/user-attachments/assets/2d4df607-4b34-435c-88db-6dc0fa09db2e" /> # 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](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. Part of backend PR - [x] 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. - [x] Timeouts are implemented and retries are limited to avoid infinite loops - [x] If paths of existing endpoints are modified without backwards compatibility, checked the frontend/CLI for any necessary changes ## Testing - [x] QA'd all new/changed functionality manually <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added "Enrollment profile renewal failed" activity type and label. * Failure entries now appear in activity feeds and host details with a dedicated activity item and a details flow. * Users can open a failure details modal showing a status icon, host name (with fallback), relative failure time, guidance about certificate expiration, and a link to Fleet support. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issue: Resolves #41418
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.Timeouts are implemented and retries are limited to avoid infinite loops
If paths of existing endpoints are modified without backwards compatibility, checked the frontend/CLI for any necessary changes
Testing
To manually QA, I put an early return with
msg.Failin themdm_scep.gofile under PKIOperation method, and then triggered a SCEP renewal.Summary by CodeRabbit
New Features
Tests