macOS & iOS/iPadOS: Rename hosts - #48690
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #48690 +/- ##
==========================================
- Coverage 68.08% 67.99% -0.10%
==========================================
Files 3784 3805 +21
Lines 239276 240633 +1357
Branches 12575 12656 +81
==========================================
+ Hits 162918 163618 +700
- Misses 61573 62152 +579
- Partials 14785 14863 +78
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:
|
815c4a3 to
ce5ebc1
Compare
WalkthroughAdds Premium host name templates for macOS, iOS, and iPadOS across fleet and No team scopes. Templates support API, GitOps, validation, activity logging, MDM enforcement, verification, failure handling, resend, transfer reconciliation, and persistence. Host details, OS settings summaries, filters, Controls pages, and command palette entries expose the resulting status. Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
a0fe844 to
0b48eed
Compare
ae9760c to
070c04f
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
frontend/pages/hosts/details/OSSettingsModal/OSSettingsTable/OSSettingsTableConfig.tests.tsx (1)
137-157: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate test case.
This "No-team" test block (lines 137-157) is byte-for-byte identical logic to the earlier test at lines 28-48 (same platforms, same
hostNameSetting, same assertions). The row generator is confirmed team-agnostic by the code comment itself, so this second test doesn't exercise any different code path — it re-runs the exact same assertions under a different name. Consider removing it or, if you want to document the team-agnostic behavior, replacing it with a single comment on the original test rather than a full duplicateit.each.🤖 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 `@frontend/pages/hosts/details/OSSettingsModal/OSSettingsTable/OSSettingsTableConfig.tests.tsx` around lines 137 - 157, Remove the duplicate No-team it.each test block around generateTableData; the earlier host-name row test already covers the same platforms, fixture, and assertions. If needed, document the team-agnostic behavior with a comment on the original test instead of retaining a second test.server/service/osquery.go (1)
1408-1412: 🚀 Performance & Scalability | 🔵 TrivialLogic is correct; consider whether this should run when the feature is unused.
This block issues an
UpdateHostDeviceNameStatusFromReportcall for every macOS MDM host on each detail update, even in deployments with no host-name template configured on any team or "No team". The datastore call is a no-op for hosts without an enforcement row, but it's still an extra write round-trip on the osquery detail path. If a cheap "any template configured" signal is available (e.g. fromac), gating on it would avoid the call for the common no-feature case.🤖 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 `@server/service/osquery.go` around lines 1408 - 1412, Add a cheap feature-usage guard to the block around UpdateHostDeviceNameStatusFromReport, using the available ac configuration to determine whether any host-name template is configured for a team or “No team.” Keep the existing detailUpdated, MDM, macOS, and ComputerName checks, and skip the datastore call entirely when no template is configured.server/service/integration_enterprise_test.go (1)
33552-33565: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winVerify no partial persistence after the invalid-template spec applies.
The two invalid
name_templatespec-apply calls (real anddry_run=true) at lines 33556-33558 don't assertgetTemplate()is unchanged afterward, unlike the equivalent valid-dry-run block immediately below (line 33562 checksrequire.Equal(t, tmpl, getTemplate())). Adding the same assertion here is cheap and guards against a validation failure leaving a partial write.♻️ Suggested addition
s.Do("POST", "/api/latest/fleet/spec/fleets", specWith(map[string]any{"name_template": "X-$FLEET_VAR_NOPE"}), http.StatusUnprocessableEntity) s.Do("POST", "/api/latest/fleet/spec/fleets", specWith(map[string]any{"name_template": "X-$FLEET_VAR_NOPE"}), http.StatusUnprocessableEntity, "dry_run", "true") + require.Equal(t, tmpl, getTemplate())🤖 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 `@server/service/integration_enterprise_test.go` around lines 33552 - 33565, Add assertions after the invalid-template spec-apply calls in the team-spec test to verify getTemplate() still equals tmpl, covering both the real and dry-run requests. Use the existing tmpl and getTemplate symbols and preserve the valid dry-run assertion below.
🤖 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 `@server/datastore/mysql/apple_device_names.go`:
- Around line 343-349: The team name-template SQL path must not treat JSON null
as a valid template. Confirm the declared type and marshaling behavior of
TeamMDM.HostNameTemplate; if it can marshal to null, add the same JSON_TYPE
string guard used by deviceNameNoTeamTemplateExpr to both the team filter here
and reconcileHostDeviceNamesForTeamDB, preserving valid non-empty templates.
In `@server/service/appconfig.go`:
- Around line 1887-1896: Clear mdm.HostNameTemplate during the license-downgrade
handling in server/service/appconfig.go at lines 1887-1896, alongside the
existing Fleet Desktop reset, so Premium values are not retained on Free. The
reconcile logic in server/service/apple_device_names.go at lines 26-46 requires
no direct change because removing the stored template resolves the
stale-template path; preserve its existing MDM.EnabledAndConfigured behavior.
In `@server/service/apple_device_names.go`:
- Around line 85-99: Update the too-long and matching-name branches in the host
processing switch to handle SetHostDeviceNameStatus errors like the default
enqueue branch: log the failure with relevant host context and continue
processing pending hosts without returning from the batch function. Preserve
their existing status updates and notify behavior for successful calls.
In `@server/service/handler.go`:
- Line 847: Add the `/api/_version_/fleet/hosts/{host_id}/name_template/resend`
route to `service.mdmConfigurationRequiredEndpoints` so the
`resendHostNameTemplateEndpoint` registered through `mdmAnyMW` is included in
MDM-not-configured checks.
---
Nitpick comments:
In
`@frontend/pages/hosts/details/OSSettingsModal/OSSettingsTable/OSSettingsTableConfig.tests.tsx`:
- Around line 137-157: Remove the duplicate No-team it.each test block around
generateTableData; the earlier host-name row test already covers the same
platforms, fixture, and assertions. If needed, document the team-agnostic
behavior with a comment on the original test instead of retaining a second test.
In `@server/service/integration_enterprise_test.go`:
- Around line 33552-33565: Add assertions after the invalid-template spec-apply
calls in the team-spec test to verify getTemplate() still equals tmpl, covering
both the real and dry-run requests. Use the existing tmpl and getTemplate
symbols and preserve the valid dry-run assertion below.
In `@server/service/osquery.go`:
- Around line 1408-1412: Add a cheap feature-usage guard to the block around
UpdateHostDeviceNameStatusFromReport, using the available ac configuration to
determine whether any host-name template is configured for a team or “No team.”
Keep the existing detailUpdated, MDM, macOS, and ComputerName checks, and skip
the datastore call entirely when no template is configured.
🪄 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: 969dfd9b-19f9-4f20-8921-3ecc84cee5a2
📒 Files selected for processing (105)
changes/38806-macos-iosipados-rename-hostscmd/fleet/cron.gocmd/fleetctl/fleetctl/generate_gitops.gocmd/fleetctl/fleetctl/gitops_test.gocmd/fleetctl/fleetctl/testdata/expectedGetConfigAppConfigJson.jsoncmd/fleetctl/fleetctl/testdata/expectedGetConfigAppConfigTeamMaintainerJson.jsoncmd/fleetctl/fleetctl/testdata/expectedGetConfigAppConfigTeamMaintainerYaml.ymlcmd/fleetctl/fleetctl/testdata/expectedGetConfigAppConfigYaml.ymlcmd/fleetctl/fleetctl/testdata/expectedGetConfigIncludeServerConfigJson.jsoncmd/fleetctl/fleetctl/testdata/expectedGetConfigIncludeServerConfigYaml.ymlcmd/fleetctl/fleetctl/testdata/expectedGetTeamsJson.jsoncmd/fleetctl/fleetctl/testdata/expectedGetTeamsYaml.ymlcmd/fleetctl/fleetctl/testdata/generateGitops/appConfig.jsoncmd/fleetctl/fleetctl/testdata/generateGitops/expectedGlobalControls.yamlcmd/fleetctl/fleetctl/testdata/generateGitops/teamConfig.jsoncmd/fleetctl/fleetctl/testdata/generateGitops/test_dir_premium/fleets/team-a-thumbsup.ymlcmd/fleetctl/fleetctl/testdata/generateGitops/test_dir_premium/fleets/unassigned.ymlcmd/fleetctl/fleetctl/testdata/macosSetupExpectedAppConfigEmpty.ymlcmd/fleetctl/fleetctl/testdata/macosSetupExpectedAppConfigSet.ymlcmd/fleetctl/fleetctl/testdata/macosSetupExpectedTeam1And2Empty.ymlcmd/fleetctl/fleetctl/testdata/macosSetupExpectedTeam1And2Set.ymlcmd/fleetctl/fleetctl/testdata/macosSetupExpectedTeam1Empty.ymlcmd/fleetctl/fleetctl/testdata/macosSetupExpectedTeam1Set.ymlcmd/fleetctl/fleetctl/testing_utils/testing_utils.goee/server/service/service.goee/server/service/teams.gofrontend/components/CommandPalette/CommandPalette.tsxfrontend/components/CommandPalette/groups/controls.tsfrontend/components/CommandPalette/helpers.tests.tsfrontend/components/CommandPalette/helpers.tsfrontend/interfaces/config.tsfrontend/interfaces/host.tsfrontend/interfaces/mdm.tsfrontend/interfaces/team.tsfrontend/pages/ManageControlsPage/OSSettings/OSSettingsNavItems.tests.tsxfrontend/pages/ManageControlsPage/OSSettings/OSSettingsNavItems.tsxfrontend/pages/ManageControlsPage/OSSettings/cards/HostNameTemplate/HostNameTemplate.tests.tsxfrontend/pages/ManageControlsPage/OSSettings/cards/HostNameTemplate/HostNameTemplate.tsxfrontend/pages/ManageControlsPage/OSSettings/cards/HostNameTemplate/_styles.scssfrontend/pages/ManageControlsPage/OSSettings/cards/HostNameTemplate/index.tsfrontend/pages/hosts/details/HostDetailsPage/HostDetailsPage.tsxfrontend/pages/hosts/details/OSSettingsModal/OSSettingsModal.tsxfrontend/pages/hosts/details/OSSettingsModal/OSSettingsTable/OSSettingStatusCell/OSSettingStatusCell.tests.tsxfrontend/pages/hosts/details/OSSettingsModal/OSSettingsTable/OSSettingStatusCell/OSSettingStatusCell.tsxfrontend/pages/hosts/details/OSSettingsModal/OSSettingsTable/OSSettingStatusCell/helpers.tsfrontend/pages/hosts/details/OSSettingsModal/OSSettingsTable/OSSettingsResendCell/OSSettingsResendCell.tests.tsxfrontend/pages/hosts/details/OSSettingsModal/OSSettingsTable/OSSettingsResendCell/OSSettingsResendCell.tsxfrontend/pages/hosts/details/OSSettingsModal/OSSettingsTable/OSSettingsTable.tsxfrontend/pages/hosts/details/OSSettingsModal/OSSettingsTable/OSSettingsTableConfig.tests.tsxfrontend/pages/hosts/details/OSSettingsModal/OSSettingsTable/OSSettingsTableConfig.tsxfrontend/pages/hosts/details/cards/HostSummary/HostSummary.tests.tsxfrontend/pages/hosts/details/cards/HostSummary/HostSummary.tsxfrontend/pages/hosts/details/helpers.tsfrontend/router/paths.tsfrontend/services/entities/host_name_template.tsfrontend/services/entities/hosts.tsfrontend/utilities/endpoints.tspkg/spec/gitops.gopkg/spec/gitops_test.goserver/datastore/mysql/apple_device_names.goserver/datastore/mysql/apple_device_names_test.goserver/datastore/mysql/apple_mdm.goserver/datastore/mysql/hosts.goserver/datastore/mysql/hosts_test.goserver/datastore/mysql/labels.goserver/datastore/mysql/migrations/tables/20260713115453_CreateHostMDMAppleDeviceNames.goserver/datastore/mysql/schema.sqlserver/datastore/mysql/teams.goserver/fleet/activities.goserver/fleet/app.goserver/fleet/datastore.goserver/fleet/hosts.goserver/fleet/hosts_test.goserver/fleet/mdm.goserver/fleet/name_template.goserver/fleet/name_template_test.goserver/fleet/service.goserver/fleet/teams.goserver/fleet/teams_test.goserver/mdm/apple/commander.goserver/mdm/apple/commander_test.goserver/mdm/lifecycle/lifecycle.goserver/mdm/lifecycle/lifecycle_test.goserver/mock/datastore_mock.goserver/mock/service/service_mock.goserver/service/appconfig.goserver/service/appconfig_test.goserver/service/apple_device_names.goserver/service/apple_mdm.goserver/service/apple_mdm_test.goserver/service/client.goserver/service/handler.goserver/service/hosts.goserver/service/hosts_test.goserver/service/integration_core_test.goserver/service/integration_enterprise_test.goserver/service/integration_mdm_test.goserver/service/mdm.goserver/service/mdm_test.goserver/service/osquery.goserver/service/osquery_test.gotools/cloner-check/generated_files/appconfig.txttools/cloner-check/generated_files/teamconfig.txttools/cloner-check/generated_files/teammdm.txttools/dibble/pkg/seed/activities.go
| // POST /hosts/{host_id:[0-9]+}/configuration_profiles/{profile_uuid}/resend endpoint. | ||
| mdmAnyMW.POST("/api/_version_/fleet/hosts/{host_id:[0-9]+}/configuration_profiles/resend/{profile_uuid}", resendHostMDMProfileEndpoint, resendHostMDMProfileRequest{}) | ||
| mdmAnyMW.POST("/api/_version_/fleet/hosts/{host_id:[0-9]+}/configuration_profiles/{profile_uuid}/resend", resendHostMDMProfileEndpoint, resendHostMDMProfileRequest{}) | ||
| mdmAnyMW.POST("/api/_version_/fleet/hosts/{host_id:[0-9]+}/name_template/resend", resendHostNameTemplateEndpoint, resendHostNameTemplateRequest{}) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether the name_template/resend path is registered in mdmConfigurationRequiredEndpoints.
rg -nP --type=go -C2 'mdmConfigurationRequiredEndpoints' server/service
rg -nP --type=go 'name_template/resend' server/serviceRepository: fleetdm/fleet
Length of output: 2690
🏁 Script executed:
#!/bin/bash
sed -n '806,940p' server/service/testing_utils_test.go | cat -nRepository: fleetdm/fleet
Length of output: 7147
Add name_template/resend to mdmConfigurationRequiredEndpoints. This MDM endpoint is registered under mdmAnyMW, but it’s missing from service.mdmConfigurationRequiredEndpoints, so it won’t be covered by the MDM-not-configured checks.
🤖 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 `@server/service/handler.go` at line 847, Add the
`/api/_version_/fleet/hosts/{host_id}/name_template/resend` route to
`service.mdmConfigurationRequiredEndpoints` so the
`resendHostNameTemplateEndpoint` registered through `mdmAnyMW` is included in
MDM-not-configured checks.
| // distinguishes it from a company-owned device. | ||
| const deviceNameEligibleHostsJoins = ` | ||
| FROM hosts h | ||
| JOIN nano_enrollments ne ON ne.device_id = h.uuid |
There was a problem hiding this comment.
This is minor but you can likely just do ne.id=h.uuid. For all devices there's a row with id=uuid and device_id=uuid, then there are for some devices user-enrollment rows with ne.device_id=uuid and id=uuid+":"+[user uuid]. So I think using the id directly might be a small DB optimization, and lessen the chance the wrong enrollment is grabbed if deviceNameEligibleHostsWhere changes(right now it's 'Device' filter on type should do that)
| // SendNotifications afterwards. This lets a bulk sender enqueue one command per | ||
| // host and then wake every device with a single batched push instead of one | ||
| // APNs request per host. | ||
| func (svc *MDMAppleCommander) DeviceNameSettingWithoutNotifications(ctx context.Context, hostUUID, cmdUUID, deviceName string) error { |
There was a problem hiding this comment.
I like this pattern a lot and think we might need to consider doing it elsewhere. Or having some process that batches up pushes for most bulk commands. Nicely done
| teamFilter, args := deviceNameTeamScope(teamID) | ||
|
|
||
| stmt := ` | ||
| INSERT INTO host_mdm_apple_device_names (host_uuid, status) |
There was a problem hiding this comment.
One optimization that might be worth making here is setting any existing enqueued commands for pending rows to active=0 in nano_enrollment_queue in the process of doing this. Reason for this is that we attempt to execute commands in order but there's no guarantee that commands get executed in order.
If you can imagine the case of a NotNow command, when we get that response we go and find the next non-NotNow command to handle the device. The device may still be in the state generating the NotNows when it gets that, or it may not(e.g. when a user's logging in at the login window it'll return NotNow for certain commands until they get their password entered all the way but once they hit the desktop it starts responding normally). Without clearing any pending NotNow commands there's a chance the new one with the updated name gets executed before the old pending one in this sort of scenario and device gets the wrong name.
IIRC we try to do this for certain profile reconciliation cases for this reason
Settings is not on Apple's list of commands that don't return NotNow so I suspect htis is a possibility though a small one
Resolves #38806 Add an IT-admin naming convention for macOS/iOS/iPadOS hosts. An admin sets a name template (e.g. "iPad $FLEET_VAR_HOST_HARDWARE_SERIAL") under Controls > OS settings > Host names for a fleet or for "No team"; Fleet resolves it per host, delivers it via an Apple `Settings`/`DeviceName` MDM command, renames its own record on ACK, then verifies the name via osquery (macOS) or a DeviceInformation refetch (iOS/iPadOS). Clearing the template stops enforcement without renaming any host. Fleet Premium only, mirroring disk encryption. Backend: - New `host_mdm_apple_device_names` enforcement table (status/command_uuid/ expected_device_name/detail) with a NULL-status = queued state machine, plus datastore methods for upsert/delete/list/set/verify/resend/reconcile. - `name_template` config key on TeamMDM, TeamSpecMDM, TeamPayloadMDM, and the global AppConfig.MDM (for "No team"); template validation (built-in vars only, no secrets, charset, 255-char and 63-byte limits) and a per-host resolver; `edited_host_name_template` activity. - `POST /api/v1/fleet/host_name_template` (fleet_id omitted/0 targets "No team") and `POST /api/v1/fleet/hosts/{id}/name_template/resend`. - `Settings`/`DeviceName` commander builder with a `DEVNAME-` command prefix and batched APNs push from the profile-manager cron. - Command-result handler (ACK -> rename + verifying; error -> failed), drift detection at both ingestion sites with a 60s grace window, and enforcement reconciliation on team transfer and MDM enrollment. - GitOps `controls.name_template` for fleets and "No team", including generate-gitops round-tripping. - Eligibility excludes non-MDM, BYOD/personal, and account-driven user enrollments; ineligible hosts get no row (recovery-lock precedent). Frontend: - Controls > OS settings "Host names" card (fleet + No team), premium gate, MDM-not-configured empty state, and GitOps-mode disabled treatment. - Synthetic "Host name" row in the host details OS settings modal for darwin/ios/ipados with statuses and a resend button. - Command-palette entry; host name statuses fold into the OS-settings aggregate cards and os_settings host-list filter.
070c04f to
24b35ca
Compare
3 participants
Related issue: Resolves #38806
Add an IT-admin naming convention for macOS/iOS/iPadOS hosts. An admin sets a name template (e.g. "iPad $FLEET_VAR_HOST_HARDWARE_SERIAL") under Controls > OS settings > Host names for a fleet or for "No team"; Fleet resolves it per host, delivers it via an Apple
Settings/DeviceNameMDM command, renames its own record on ACK, then verifies the name via osquery (macOS) or a DeviceInformation refetch (iOS/iPadOS). Clearing the template stops enforcement without renaming any host. Fleet Premium only, mirroring disk encryption.sequenceDiagram participant Admin participant UI participant API as Core service participant EE as EE service participant DB participant Cron as Profile-manager cron participant Cmdr as MDMAppleCommander participant Device as macOS / iOS / iPadOS participant Ingest as osquery / DeviceInformation note over Admin,DB: 1. Set template Admin->>UI: Save "iPad $FLEET_VAR_HOST_HARDWARE_SERIAL" UI->>API: POST /api/v1/fleet/host_name_template {fleet_id, name_template} API->>API: license + authz (MDMAppleSettingsPayload, ActionWrite) API->>API: validate template (vars allow-list, charset) API->>EE: EnterpriseOverrides.UpdateTeamMDMHostNameTemplate EE->>DB: SaveTeam(config JSON) + edited_host_name_template activity EE->>DB: upsert host_mdm_apple_device_names rows (status=NULL) for eligible hosts note over Cron,Device: 2. Enforce Cron->>DB: rows WHERE status IS NULL (BYOD/personal enrollments never get rows) Cron->>Cron: resolve template per host (expand FLEET_VARs) Cron->>Cmdr: Settings{DeviceName: resolved} (UUID prefix DEVNAME-) Cmdr->>Device: APNs push + command DB->>DB: row status='pending', command_uuid, expected_device_name Device-->>API: Acknowledged / Error (CommandAndReportResults) alt Acknowledged API->>DB: hosts.computer_name/hostname = expected + host_display_names API->>DB: row status='verifying' else Error API->>DB: row status='failed', detail=error chain end note over Ingest,DB: 3. Verify + drift detection Ingest-->>API: reported computer_name (osquery) / DeviceName (refetch) API->>DB: reported == expected ? 'verified' : 'failed' (drift) note over Admin,DB: 4. Lifecycle Admin->>API: transfer host / new enrollment / resend API->>DB: dest fleet has template ? row reset to NULL (Pending) : row deletedChecklist 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.Testing
Database migrations
COLLATE utf8mb4_unicode_ci).New Fleet configuration settings
fleetctl generate-gitopsSummary by CodeRabbit