Extract Apple MDM initialization out of runServeCmd - #46517
Merged
MagnusHJensen merged 2 commits intoJun 3, 2026
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Contributor
|
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: 📥 CommitsReviewing files that changed from the base of the PR and between 17d62322873c39e5a37d3ad203b47053a1b1374a and 935e767. 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughThis PR refactors Apple MDM initialization logic by extracting inline code from serve.go into a new module (mdm_apple.go) with focused helper functions. New helpers construct MySQL-backed NanoMDM storages, initialize an APNs push service (or no-op in dev mode), and reconcile APNs/SCEP and ABM assets in the datastore. The reconciliation functions load configured certs/keys and insert them when missing, while handling duplicate-key errors gracefully. The serve.go file is refactored to call these helpers and to replace all 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 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 |
![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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/fleet/mdm_apple.go`:
- Around line 62-66: The TLS config passed into buford.NewPushProviderFactory
(see pushProviderFactory / buford.NewPushProviderFactory and the tls.Config used
when constructing fleethttp.NewClient) must explicitly set MinVersion to
tls.VersionTLS12 to enforce TLS 1.2+ for APNs; update the tls.Config literal to
include MinVersion: tls.VersionTLS12 while keeping the existing Certificates
field so the client uses TLS 1.2 or higher.
🪄 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: a3f121e5-d769-480b-911f-0ef520ba1334
📥 Commits
Reviewing files that changed from the base of the PR and between 1f934e1 and 8f41c00e27abe1e13e10c6b20a4374d12875792d.
📒 Files selected for processing (3)
cmd/fleet/mdm_apple.gocmd/fleet/mdm_apple_test.gocmd/fleet/serve.go
Contributor
Author
|
Hey @getvictor @JordanMontgomery - Would you mind taking a review, please? This PR extracts the Apple MDM related logic out of serve.go. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #46517 +/- ##
=======================================
Coverage 66.90% 66.91%
=======================================
Files 2831 2832 +1
Lines 224929 224955 +26
Branches 11487 11487
=======================================
+ Hits 150481 150519 +38
+ Misses 60797 60784 -13
- Partials 13651 13652 +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:
|
Member
|
Hi @raju249 I'll review this one, do you mind fixing the merge conflicts that has appeared? |
Move the Apple MDM storage setup, push service construction, and APNs/SCEP/ABM asset reconciliation out of runServeCmd into testable functions in a new cmd/fleet/mdm_apple.go. The shared asset-existence check that was an inline closure is now a package function reused across all call sites. Adds unit tests for the dev-mode push gate, the asset-existence helper branches, and the no-op and missing-private-key paths of both reconcilers. Behavior is preserved; the full cmd/fleet suite passes unchanged. Part of fleetdm#33370.
Add MinVersion: tls.VersionTLS12 to the APNs push client (pre-existing gap surfaced when moving the code; Apple APNs requires TLS 1.2+). Adds a unit test for the APNs/SCEP reconciler path where assets are already stored, asserting the insert is skipped. Per review feedback on fleetdm#46517.
raju249
force-pushed
the
33370-mdm-apple-init-extraction
branch
from
June 3, 2026 13:56
17d6232 to
935e767
Compare
Contributor
Author
|
Thanks @MagnusHJensen I have fixed the conflicts and rebased to latest main, please take a look and let me know wdyt? |
MagnusHJensen
approved these changes
Jun 3, 2026
MagnusHJensen
left a comment
MagnusHJensen
left a comment
Member
There was a problem hiding this comment.
LGTM, thank you @raju249
1 task
MagnusHJensen
pushed a commit
that referenced
this pull request
Jun 4, 2026
Extracts the MySQL datastore initialization out of `runServeCmd` and into a new `cmd/fleet/datastore.go`. Same pattern as the prior extractions on this issue (#44929, #45343, #45583, #46166, #46421, #46517). Continues the path toward `serve.go` >60% coverage per the discussion on #33370. Three functions come out of the inline block: - `initDatastore` — builds the shared DB connections, the datastore, and the carve store (S3-backed when configured, otherwise the datastore itself). - `buildMySQLOpts` — assembles the DB options: base logger and config, plus the optional read replica, dev SQL interceptor, and tracing. - `evalMigrationStatus` — prints any operator guidance for the migration status and returns whether `runServeCmd` should exit. The `os.Exit` stays in `runServeCmd`, so the boot/refuse-to-boot decision becomes unit-testable without the function terminating the test binary. Behavior is preserved — `runServeCmd` calls these in the same order with the same arguments, the migration-exit conditions are unchanged, and the full `cmd/fleet` suite passes against MySQL + Redis. `initDatastore` returns early after `initFatal` so it's safe when the caller's `initFatal` doesn't terminate (the case in tests). On test scope: `TestEvalMigrationStatus` covers every migration status code across the dev-mode and allow-missing-migrations combinations — that's the real decision logic. I deliberately didn't add unit tests for `initDatastore`/`buildMySQLOpts`: their only failure paths are paranoid `initFatal` wrapping around constructors that don't dial at construction time, and the option builder returns opaque option closures. Those success paths are already exercised by booting the server, so a full datastore mock wasn't worth it for coverage's sake. Remaining slice per the broader plan: Redis init. **Related issue:** Refs #33370 # Checklist for submitter - [x] Added/updated automated tests - Changes file: not applicable — internal refactor with no user-visible behavior change <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Reorganized database startup initialization and migration status evaluation for improved maintainability. * **Tests** * Added comprehensive test coverage for database migration status handling across various scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 task
MagnusHJensen
pushed a commit
that referenced
this pull request
Jun 5, 2026
Extracts the Redis pool and the cached_mysql / mysqlredis datastore wrappers out of `runServeCmd` and into a new `cmd/fleet/redis.go`. Same pattern as the prior extractions on this issue (#44929, #45343, #45583, #46166, #46421, #46517, #46742). Continues the path toward `serve.go` >60% coverage per the discussion on #33370. Three functions come out of the inline block: - `initRedis` — builds the Redis pool, wraps the datastore with `cached_mysql.New`, and applies `mysqlredis.New` with the license-enforced host limit and host-cache options. Returns the pool, the fully wrapped `fleet.Datastore`, and the outermost `*mysqlredis.Datastore` (a few callers need the concrete type). - `buildRedisPoolConfig` — translates `config.RedisConfig` into the `redis.PoolConfig`, including the `redis://` scheme strip. - `validateRedisConfig` — encodes the host-cache invariant: `HostCacheEnabled` requires `HostCacheTTL > 0`. Returns an error so the caller (or in this case `initRedis` via `initFatal`) can refuse boot without that decision being buried inside a pure builder. Behavior is preserved — `runServeCmd` calls these in the same order with the same arguments, the host-cache validation still aborts startup when violated, and the full `cmd/fleet` suite passes against MySQL + Redis. `initRedis` returns early after `initFatal` so it's safe when the caller's `initFatal` doesn't terminate (the case in tests). Following the precedent established on #46742, the caller also has a loud `initFatal` + `return` guard against a nil pool (covers the same nilaway flow we hit on the datastore slice). On test scope: `TestValidateRedisConfig` covers all four combinations of `HostCacheEnabled` and `HostCacheTTL` — that's the real boot/refuse-to-boot decision. `TestBuildRedisPoolConfigStripsScheme` pins the `redis://` scheme-strip contract for Render-style URIs. I didn't add a `buildRedisPoolConfig` field-mapping matrix or an `initRedis` happy-path unit test: the former would just re-state the struct literal, and the latter needs a real Redis pool (the smoke boot exercises it end-to-end instead). This completes the four named init-block extractions on this issue. If further coverage gains are needed beyond what these have already moved, the next conversation is whether to test `runServeCmd` directly via the injected `initFatal`. **Related issue:** Refs #33370 # Checklist for submitter - [x] Added/updated automated tests - Changes file: not applicable — internal refactor with no user-visible behavior change <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Consolidated Redis initialization and datastore wrapping into a dedicated helper; startup now validates the Redis pool and handles initialization failures explicitly. * **Tests** * Added unit tests for Redis address handling and host-cache TTL validation to ensure config behavior is enforced. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 task
MagnusHJensen
pushed a commit
that referenced
this pull request
Jun 8, 2026
Extracts the osquery status, result, and audit JSON logger setup out of `runServeCmd` and into a new `cmd/fleet/logging.go`. Same pattern as the prior extractions on this issue (#44929, #45343, #45583, #46166, #46421, #46517, #46742, #46830). Continues trimming `runServeCmd` toward the `serve.go` coverage goal on #33370 — this is the largest single slice so far (~100 lines out). Three functions come out of the inline block: - `initOsqueryLogging` — builds the status and result loggers, plus the audit logger when enabled. Mutates the shared `logging.Config` per logger in the same sequence as before, so the constructed loggers are identical. - `buildLoggingConfig` — maps `config.FleetConfig` into the common `logging.Config` shared by all three loggers. - `shouldEnableAuditLog` — the premium-and-enabled gate for the audit logger, pulled out so the decision is its own testable unit. Behavior is preserved — `runServeCmd` calls this in the same place with the same arguments, the per-logger config mutation order is unchanged, and the full `cmd/fleet` suite passes against MySQL + Redis. `initOsqueryLogging` returns early after `initFatal` so it's safe when the caller's `initFatal` doesn't terminate (the case in tests), and it guards a nil license up front since the audit gate dereferences it (matching the nil-guard precedent from #46742/#46830). On test scope: `TestShouldEnableAuditLog` covers all four combinations of license tier and the config flag — audit logging is a premium feature, so the gate is the meaningful decision here. `TestBuildLoggingConfigMapsConfig` is a light check that the config mapping is wired through. I didn't add a full `initOsqueryLogging` happy-path unit test: `logging.NewJSONLogger` constructs real log sinks, so that path is exercised by booting the server rather than by standing up logger backends in a unit test. **Related issue:** Refs #33370 # Checklist for submitter - [x] Added/updated automated tests - Changes file: not applicable — internal refactor with no user-visible behavior change <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Audit logging support is now available for premium license holders. * **Refactor** * Improved logging initialization and configuration management. * **Tests** * Added test coverage for audit logging enablement and configuration mapping. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 task
MagnusHJensen
pushed a commit
that referenced
this pull request
Jun 9, 2026
) Extracts the geoIP provider and mail service setup out of `runServeCmd` and into new `cmd/fleet/geoip.go` and `cmd/fleet/mail.go`. Same pattern as the prior extractions on this issue (#44929, #45343, #45583, #46166, #46421, #46517, #46742, #46830, #46893). Both are best-effort startup providers — they log and fall back rather than aborting boot — so they group naturally. Functions: - `initGeoIP` — returns the GeoIP provider. When no database path is configured, or the MaxMind database fails to load, it returns a no-op provider and logs rather than aborting startup. - `initMailService` — configures the mail service; a construction failure is logged and the (possibly nil) service is returned, matching the prior best-effort behavior. - `shouldForceSMTPBackend` — the SMTP-vs-custom-backend rule, pulled out so the decision is its own testable unit: SMTP and a custom email backend are mutually exclusive, and an already-enabled SMTP configuration wins. Behavior is preserved — `runServeCmd` calls these in the same place with the same arguments, and the full `cmd/fleet` suite passes against MySQL + Redis. The mail block's `config.Email.EmailBackend` reset is local to mail construction (nothing downstream reads it), so moving it into `initMailService` is behavior-identical. On test scope: `TestInitGeoIP` pins the not-fatal fallback for both the missing-path and invalid-path cases — GeoIP being best-effort is a real guarantee worth locking. `TestShouldForceSMTPBackend` covers the backend mutual-exclusion decision, including the nil app config / nil SMTP settings edges. I didn't add a full `initMailService` happy-path unit test: `mail.NewService` builds real SMTP/SES backends, so that path is exercised by booting the server. **Related issue:** Refs #33370 # Checklist for submitter - [x] Added/updated automated tests - Changes file: not applicable — internal refactor with no user-visible behavior change <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **Refactor** * Improved GeoIP initialization with automatic fallback when database configuration is unavailable * Enhanced mail service initialization with better error handling during startup * Refined SMTP backend precedence logic * **Tests** * Added comprehensive unit tests for GeoIP and mail service initialization scenarios <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 tasks
MagnusHJensen
pushed a commit
that referenced
this pull request
Jun 17, 2026
Extracts the cron schedule registration out of `runServeCmd` and into a new `cmd/fleet/cron_registration.go`. Same pattern as the prior extractions on this issue (#44929, #45343, #45583, #46166, #46421, #46517, #46742, #46830, #46893, #47151). This is the largest slice so far — `runServeCmd` drops from ~1300 to ~1000 lines, and `serve.go` from 1776 to 1472. The 33 `StartCronSchedule` registrations move into one `startCronSchedules` entry point backed by a `cronSchedulesDeps` struct (the dependencies the closures previously captured from `runServeCmd`). Registration is grouped by domain: - `registerCleanupAndMaintenanceCrons` — chart data collection, the `cron_stats` cleanup goroutine, software migrations, frequent cleanups, cleanups-then-aggregation, query results cleanup, upcoming activities, usage statistics, batch activities. - `registerVulnerabilityCrons` — the vulnerabilities schedule, or the remote-trigger proxy when processing is disabled on this instance. - `registerWorkerCrons` — automations and worker integrations. - `registerMDMCrons` — Apple MDM worker, DEP profile assigner, service discovery, the Apple/Windows/Android profile managers, the Android device reconciler, the Android policy migrations, and the APNs pusher. - `registerPremiumCrons` — iPhone/iPad refetcher and reviver, maintained apps, VPP app version refresh (and the one-shot VPP country backfill), recovery lock passwords, managed local account rotation, activities streaming, and the calendar schedule. - `registerMiscCrons` — host vitals label membership and the batch activity completion checker. Behavior is preserved — the schedules register in the same order with the same arguments, the same conditionals gate them (premium, audit log, env vars, software store presence), and the `config` is threaded as a pointer so the `&config` and `config.Calendar` mutations inside the calendar closure keep their original semantics. `cmd/fleet/cron.go` (the schedule definitions) is intentionally untouched; only the wiring moved. One unit test added: `TestVulnerabilityProcessingDisabled` covers the vuln enable/disable predicate extracted into `vulnerabilityProcessingDisabled`, including the legacy `current_instance_checks` `"0"` value. The rest of the file is dependency-wiring relocation with no further decision logic to unit-test — those paths construct real schedules, so they stay covered by the existing suite and integration tests. The full `cmd/fleet` suite passes against MySQL + Redis, and a local server boot confirms the same 30 cron schedules start as before (verified against the "started cron schedules" log line). **Related issue:** Refs #33370 # Checklist for submitter - [x] Added/updated automated tests - [x] QA'd all new/changed functionality manually (verified via local server boot — same 30 cron schedules start) - Changes file: not applicable — internal refactor with no user-visible behavior change <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Centralized background cron schedule startup and standardized job initialization sequencing for maintenance, vulnerability handling, integrations, MDM workflows, and premium tasks. * **New Features / Behavior** * Added config- and license-controlled enablement for vulnerability processing (local vs remote triggering), MDM automation (including APNs delivery and device reconciliation), and premium-only refresh/recovery behaviors. * Made chart data collection and optional activity streaming configurable, with safe fallbacks for scheduling periodicity. * **Tests** * Added coverage for vulnerability-schedule enable/disable decision logic. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 tasks
JordanMontgomery
pushed a commit
that referenced
this pull request
Jun 24, 2026
Extracts the `/api/` request timeout/body-size override middleware out of `runServeCmd` and into `apiTimeoutOverrideHandler` in a new `cmd/fleet/http_middleware.go`. Same pattern as the prior extractions on this issue (#44929, #45343, #45583, #46166, #46421, #46517, #46742, #46830, #46893, #47151, #47562). `runServeCmd` drops from ~1000 to ~900 lines, and `serve.go` from 1475 to 1373. The middleware is the `~100`-line `rootMux.HandleFunc("/api/", ...)` closure that applies per-route read/write deadline overrides for endpoints that legitimately run long — synchronous script runs, large software-installer and bootstrap-package uploads, the Android enterprise signup SSE stream, and large MDM profile batch operations — and, for package-upload routes, caps the request body and threads the configured max installer size through the request context. Behavior is preserved — the handler is moved verbatim and wired into `rootMux` via a single `apiTimeoutOverrideHandler(apiHandler, config, logger)` call, so the same routes get the same overrides and every request still falls through to `apiHandler.ServeHTTP`. The now-unused `scripts` and `installersize` imports drop out of `serve.go`. On test scope: `TestAPITimeoutOverrideHandler` verifies the real decision in this middleware — that package-upload paths thread the configured max installer size into the request context (and non-upload requests keep the default) — and that the wrapped API handler is always invoked. The deadline overrides themselves go through `http.ResponseController`, which a unit-test `ResponseRecorder` doesn't support (the handler logs and proceeds, as in production), so those are exercised by booting the server rather than asserted in a unit test. **Related issue:** Refs #33370 # Checklist for submitter - [x] Added/updated automated tests - [x] QA'd all new/changed functionality manually (verified via local server boot) - Changes file: not applicable — internal refactor with no user-visible behavior change <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Improved timeout handling for long-running operations across the API. Script execution, file uploads, Server-Sent Event streams, and batch operations now have optimized request timeouts and body size limits. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 tasks
juan-fdz-hawa
pushed a commit
that referenced
this pull request
Jun 30, 2026
) Adds an end-to-end boot test for `runServeCmd`, the main server entry point. This is the coverage milestone for #33370: `serve.go` goes from ~7% to ~64%, and `runServeCmd` itself from 0% to ~62%. The earlier PRs on this issue (#44929, #45343, #45583, #46166, #46421, #46517, #46742, #46830, #46893, #47151, #47562, #47891) extracted testable pieces out of `runServeCmd`, but the function itself stayed at 0% — it blocks on an OS signal and wires the entire server together, so the only way to cover it is to actually boot it. This PR does that. `TestRunServeCmd` (gated behind `MYSQL_TEST` + `REDIS_TEST`) boots the full server against a real migrated test MySQL and Redis, waits for `/healthz`, then cancels the command context to trigger a graceful shutdown. It covers two paths: - **Full boot with Apple MDM enabled** — a 32-byte server private key brings up the Apple MDM protocol services and the host-identity / conditional-access SCEP setup, so the boot exercises the MDM startup path as well as the core wiring, cron schedules, and HTTP server. - **Fail-fast on bad config** — an invalid Redis host-cache configuration (enabled with a non-positive TTL) aborts startup through `initFatal` and returns rather than serving, covering the Redis-init error path and the nil-pool guard. Beyond coverage, this doubles as a regression net for the ongoing `runServeCmd` slicing: a future change that breaks startup now fails this test instead of reaching a release. **One production change**, in `runServeCmd`'s shutdown `select`: it now also watches `cmd.Context().Done()`. This is inert in production — the root command runs via `Execute()` (not `ExecuteContext()`), so `cmd.Context()` is `context.Background()` and never cancels. Only the test runs the command with a cancelable context, which is how it shuts the server down without sending a real signal (a `SIGTERM` would kill the test binary). A couple of notes for reviewers: - The test uses `os.Setenv` (not `t.Setenv`) because the MySQL test helper marks the test parallel; the boot scenarios run as serial subtests so the process-global config env doesn't race. - `runServeCmd` registers metrics with the process-global Prometheus registry, which can only happen once per process, so there is a single full boot here; the error-path scenario fails before that registration. - The test DB is loaded from a schema dump that doesn't mark every data migration as applied, so the boot runs with `FLEET_UPGRADES_ALLOW_MISSING_MIGRATIONS=1`. It adds ~2s to the `cmd/fleet` (`main`) test bundle, which is well off the CI critical path. **Related issue:** Refs #33370 # Checklist for submitter - [x] Added/updated automated tests - [x] QA'd all new/changed functionality manually (verified locally: boots to /healthz, graceful shutdown, ~64% serve.go coverage) - Changes file: not applicable — internal test coverage with no user-visible behavior change <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved server shutdown handling to stop cleanly when the running command’s context is canceled, not only on OS signals. * Added stronger startup validation to fail fast for invalid Redis host-cache configuration (e.g., non-positive TTL). * **Tests** * Added an end-to-end test that boots the server against real MySQL/Redis, verifies graceful startup/shutdown, and confirms fast-fail behavior for misconfiguration. <!-- 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
2 participants
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.
Extracts the Apple MDM initialization out of
runServeCmdinto testable functions in a newcmd/fleet/mdm_apple.go. Continues the chain of extractions on this issue (#44929, #45343, #45583, #46166, #46421) toward theserve.go>60% coverage target discussed on #33370.Five functions come out of the inline block:
initAppleMDMStorages— constructs the MDM, DEP, and SCEP storages.initAppleMDMPushService— picks the no-op pusher underFLEET_DEV_MDM_APPLE_DISABLE_PUSH=1, otherwise the real APNs pusher.checkMDMAssetsExist— promotes the inlinecheckMDMAssetsclosure to a package function. It was already used at several call sites; they now all share this one.reconcileAppleMDMAPNsAndSCEPAssets/reconcileAppleMDMABMAssets— the APNs/SCEP and ABM asset reconciliation blocks.Behavior is preserved —
runServeCmdcalls these in the same order with the same arguments, and the fullcmd/fleetsuite passes unchanged against MySQL + Redis. Each function returns early afterinitFatalso it's also safe when the caller'sinitFataldoesn't terminate (the case in tests).On test scope: the new unit tests cover the dev-mode push gate, all four branches of
checkMDMAssetsExist, and the no-op and missing-private-key paths of both reconcilers. The storage construction and the actual asset-insert paths need a real datastore, so those stay covered by the existing integration tests rather than new unit tests — I didn't want to stand up a full datastore mock for paths that are already exercised end-to-end.Related issue: Refs #33370
Checklist for submitter
Summary by CodeRabbit
New Features
Tests
Refactor