Skip to content

Add Android support for custom host vitals - #49696

Merged
nulmete merged 9 commits into
mainfrom
49421-android-support-custom-host-vitals
Jul 29, 2026
Merged

Add Android support for custom host vitals#49696
nulmete merged 9 commits into
mainfrom
49421-android-support-custom-host-vitals

Conversation

@nulmete

@nulmete nulmete commented Jul 21, 2026

Copy link
Copy Markdown
Member

Related issue: Resolves #49421

Custom host vitals ($FLEET_HOST_VITAL_<id>) already worked in scripts and Apple/Windows configuration profiles, but Android configuration profiles and managed app configuration explicitly rejected them at upload to keep parity with $FLEET_SECRET_*. This left admins unable to inject per-host vitals (e.g. an asset tag) into Android MDM configuration the same way they can for every other platform.

For more context, prior PRs:

Checklist for submitter

  • Changes file added for user-visible changes in changes/, orbit/changes/ or ee/fleetd-chrome/changes.
    See Changes files for more information.

Testing

  • Added/updated automated tests

  • QA'd all new/changed functionality manually

  • Created an "Asset tag" host vital.

  • Enrolled an Android device.

  • Initially the test profile showed as "Failed" because no value was set for the vital.

  • Set a value for the vital, saw that it went from Enforcing to Verified.

Screenshot 2026-07-24 at 8 57 46 AM Screenshot 2026-07-24 at 8 56 56 AM Screenshot 2026-07-24 at 8 57 30 AM

Also tested the rejection cases:

  • trying to upload a profile with an invalid custom host vital id (either a non-numeric value, a numeric but non-existent ID, and referencing a vital as a JSON key instead of a value)
  • deleting a vital referenced in a profile
Screen.Recording.2026-07-24.at.9.30.20.AM.mov

Summary by CodeRabbit

Summary by CodeRabbit

  • New Features

    • Android app configurations and profiles now support custom host vital placeholders ($FLEET_HOST_VITAL_<id>).
    • Custom host vital values are expanded per device during Android delivery.
    • Managed Android profiles/configurations are automatically resent when a referenced vital value changes.
  • Bug Fixes

    • Added validation for malformed, missing, or undefined vital references during Android app association and profile/config uploads.
    • Prevented deletion of vitals referenced by Android profiles.
    • Improved error handling and delivery failure details when a device lacks a required vital value.

Custom host vitals ($FLEET_HOST_VITAL_<id>) previously worked in scripts and
Apple/Windows profiles but were rejected at upload for Android configuration
profiles and managed app configuration, to keep parity with $FLEET_SECRET_.
This adds validation, per-host expansion at delivery, and resend-on-value-
change for Android, mirroring the existing Apple/Windows behavior. Also fixes
a pre-existing gap where Android profiles weren't covered by the vital
delete-protection check that Apple/Windows/scripts already had, and renames
ContainsCustomHostVitalIDs to FindCustomHostVitalIDs for clarity.
Copilot AI review requested due to automatic review settings July 21, 2026 18:54
@nulmete

nulmete commented Jul 21, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning

  • Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.

Pull request overview

Adds Android parity for custom host vitals ($FLEET_HOST_VITAL_<id>) by allowing references in Android configuration profiles and Android managed app configuration, validating referenced vitals at upload, and expanding them per-host at delivery time (including resend-on-change behavior for Android profiles).

Changes:

  • Add $FLEET_HOST_VITAL_<id> detection + per-host expansion for Android managed app config and Android profile delivery.
  • Validate referenced custom host vitals for Android profiles/app configs at upload (and adjust Android-side validation rules for token placement).
  • Extend “used-by” scanning and “resend on vital value change” behavior to include Android configuration profiles; add/extend tests across service, profiles, fleet validation, and datastore.

Reviewed changes

Copilot reviewed 20 out of 21 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
changes/49421-android-custom-host-vitals Release note entry (content excluded by policy).
server/worker/software_worker.go Detect host-vital tokens in Android app configs and pass HostID for per-host expansion.
server/service/microsoft_mdm.go Switch Windows per-host processing detection to FindCustomHostVitalIDs.
server/service/mdm.go Validate referenced custom host vitals for Android profiles at upload; include Android in batch validation.
server/service/mdm_test.go Add service-level tests for Android profile vital validation (contains a compile issue noted in comments).
server/service/custom_host_vitals_resolution_test.go Update tests to renamed FindCustomHostVitalIDs.
server/service/apple_mdm.go Update host-vital detection to FindCustomHostVitalIDs.
server/mdm/profiles/android_appconfig.go Expand custom host vitals in Android managed app configuration using host numeric ID.
server/mdm/profiles/android_appconfig_test.go Add tests for Android app-config host-vital substitution and missing-value behavior.
server/mdm/microsoft/profile_variables.go Update host-vital detection to FindCustomHostVitalIDs.
server/mdm/apple/profile_processor.go Update host-vital detection to FindCustomHostVitalIDs.
server/mdm/android/service/profiles.go Add Android profile per-host expansion detection for host vitals; surface missing-vital-value failures as profile failures.
server/mdm/android/service/profiles_test.go Add coverage for missing host-vital value failure path and new helper functions.
server/fleet/custom_host_vitals.go Rename token finder to FindCustomHostVitalIDs; add Android profile entity constant; adjust error strings.
server/fleet/custom_host_vitals_test.go Rename tests to match FindCustomHostVitalIDs.
server/fleet/android.go Allow host-vital tokens in Android profile validation; enforce “must be in JSON string value”; reject malformed host-vital refs in app config validation.
server/fleet/android_test.go Add/adjust Android validation tests for host-vital acceptance and malformed refs.
server/datastore/mysql/custom_host_vitals.go Include Android profiles in “used-by” scan and resend-on-vital-change logic; update to FindCustomHostVitalIDs.
server/datastore/mysql/custom_host_vitals_test.go Add tests ensuring Android profiles block deletion and resend-on-change resets Android profile status.
ee/server/service/vpp.go Validate referenced custom host vitals in Android app configuration during VPP batch association.
ee/server/service/vpp_test.go Add tests for malformed and unknown-vital references in Android app configuration.
Files excluded by content exclusion policy (1)
  • changes/49421-android-custom-host-vitals

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread server/service/mdm_test.go
Comment thread server/service/mdm_test.go
@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 544edc25-598c-4aa3-8b64-19e5c66a5415

📥 Commits

Reviewing files that changed from the base of the PR and between c6dface and 1d1ef77.

📒 Files selected for processing (6)
  • server/fleet/android.go
  • server/fleet/android_test.go
  • server/mdm/android/service/profiles.go
  • server/mdm/profiles/android_appconfig.go
  • server/mdm/profiles/android_appconfig_test.go
  • server/worker/software_worker.go
🚧 Files skipped from review as they are similar to previous changes (4)
  • server/fleet/android.go
  • server/mdm/android/service/profiles.go
  • server/mdm/profiles/android_appconfig_test.go
  • server/worker/software_worker.go

Walkthrough

Android configuration profiles and managed app configurations now accept custom host vital references, validate malformed, missing, and JSON-key usages, and expand values per host during delivery. Android substitution failures are classified for profile status reporting. Upload and batch paths validate referenced vitals. Deletion protection and resend logic now include Android profiles. Shared vital ID extraction was renamed to FindCustomHostVitalIDs, with corresponding updates across Apple, Windows, Android, service, worker, and test code.

Possibly related PRs

  • fleetdm/fleet#48821: Extends the existing custom host vital datastore and service flows that this PR updates for Android support.
  • fleetdm/fleet#49586: Modifies the same custom host vital usage scanning and resend mechanisms for another profile surface.
  • fleetdm/fleet#49640: Changes the related missing custom host vital error formatting.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 34.48% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately summarizes the main change: Android support for custom host vitals.
Description check ✅ Passed The description includes the related issue, checklist items, testing, and manual QA, so it matches the template well enough.
Linked Issues check ✅ Passed The code changes cover Android vitals validation, expansion, resend-on-change, deletion protection, and batch/single upload handling as requested.
Out of Scope Changes check ✅ Passed The changed files all support the Android custom host vital feature or its shared helpers, with no clear unrelated additions.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 49421-android-support-custom-host-vitals

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
server/mdm/android/service/profiles.go (1)

746-752: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

JSON-escape custom host vital values before Android substitution.

SubstituteFleetVarsInAndroidAppConfig uses JSON-safe replacement for Fleet variables but delegates vital expansion to the raw ExpandCustomHostVitals path. A value containing " or \ can corrupt the managed-configuration JSON, causing the later unmarshal to fail and aborting reconciliation for the host. Make Android vital expansion JSON-context-aware and add a special-character regression test.

🤖 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/mdm/android/service/profiles.go` around lines 746 - 752, Update the
Android profile substitution flow around SubstituteFleetVarsInAndroidAppConfig
so custom host vital expansion escapes values for JSON string context before
insertion, matching Fleet variable replacement behavior and preserving valid
managed-configuration JSON. Add a regression test covering vital values
containing quotes and backslashes, verifying substitution and subsequent
unmarshalling succeed.
🧹 Nitpick comments (1)
ee/server/service/vpp.go (1)

373-387: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

N+1 datastore round-trips for vitals validation in batch Android app association.

svc.ds.ValidateReferencedCustomHostVitals is invoked once per Android payload inside the per-payload loop, causing one DB round trip per app when validating referenced vitals. server/service/mdm.go's BatchSetMDMProfiles instead aggregates all profile documents and validates them in a single call before processing individual profiles. For GitOps/batch uploads with many Android apps referencing vitals, this diverges from that batched pattern and adds N round trips instead of 1.

Consider collecting payload.Configuration for all Android payloads up front and calling ValidateReferencedCustomHostVitals once, mirroring the mdm.go approach.

🤖 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 `@ee/server/service/vpp.go` around lines 373 - 387, Batch Android
custom-host-vitals validation instead of calling
ValidateReferencedCustomHostVitals inside the per-payload switch in the Android
association flow. Collect all non-nil payload.Configuration values up front,
invoke the datastore validation once before processing individual payloads, and
preserve the existing configuration and error handling behavior.
🤖 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/custom_host_vitals.go`:
- Around line 155-159: Update the documentation comment for the custom host
vital reference-checking function to enumerate every entity type it scans:
scripts, Apple declarations and profiles, Windows and Android configuration
profiles, software installers, setup-experience scripts, and labels. Keep the
existing return-value and database-error descriptions unchanged.

---

Outside diff comments:
In `@server/mdm/android/service/profiles.go`:
- Around line 746-752: Update the Android profile substitution flow around
SubstituteFleetVarsInAndroidAppConfig so custom host vital expansion escapes
values for JSON string context before insertion, matching Fleet variable
replacement behavior and preserving valid managed-configuration JSON. Add a
regression test covering vital values containing quotes and backslashes,
verifying substitution and subsequent unmarshalling succeed.

---

Nitpick comments:
In `@ee/server/service/vpp.go`:
- Around line 373-387: Batch Android custom-host-vitals validation instead of
calling ValidateReferencedCustomHostVitals inside the per-payload switch in the
Android association flow. Collect all non-nil payload.Configuration values up
front, invoke the datastore validation once before processing individual
payloads, and preserve the existing configuration and error handling behavior.
🪄 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: 86f0b3f7-1227-4015-8c02-2c8703920d19

📥 Commits

Reviewing files that changed from the base of the PR and between e41e9ea and abd167d.

📒 Files selected for processing (21)
  • changes/49421-android-custom-host-vitals
  • ee/server/service/vpp.go
  • ee/server/service/vpp_test.go
  • server/datastore/mysql/custom_host_vitals.go
  • server/datastore/mysql/custom_host_vitals_test.go
  • server/fleet/android.go
  • server/fleet/android_test.go
  • server/fleet/custom_host_vitals.go
  • server/fleet/custom_host_vitals_test.go
  • server/mdm/android/service/profiles.go
  • server/mdm/android/service/profiles_test.go
  • server/mdm/apple/profile_processor.go
  • server/mdm/microsoft/profile_variables.go
  • server/mdm/profiles/android_appconfig.go
  • server/mdm/profiles/android_appconfig_test.go
  • server/service/apple_mdm.go
  • server/service/custom_host_vitals_resolution_test.go
  • server/service/mdm.go
  • server/service/mdm_test.go
  • server/service/microsoft_mdm.go
  • server/worker/software_worker.go

Comment on lines +155 to +159
// Apple declarations, Windows configuration profiles, and Android configuration
// profiles for a $FLEET_HOST_VITAL_<id> (or ${FLEET_HOST_VITAL_<id>}) reference
// to the given vital id. It returns a *fleet.CustomHostVitalUsedInfo describing
// the first referencing entity found, or nil if unreferenced. Mirrors the scan
// structure of DeleteSecretVariable. The second return is a real DB error.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Keep the delete-protection comment aligned with the implementation.

The comment lists only Apple declarations, Windows profiles, and Android profiles, but this function also scans scripts, Apple profiles, software installers, setup-experience scripts, and labels. Update the comment so maintainers understand the complete protection surface.

🤖 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/datastore/mysql/custom_host_vitals.go` around lines 155 - 159, Update
the documentation comment for the custom host vital reference-checking function
to enumerate every entity type it scans: scripts, Apple declarations and
profiles, Windows and Android configuration profiles, software installers,
setup-experience scripts, and labels. Keep the existing return-value and
database-error descriptions unchanged.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — updated the comment to list all seven scanned entity types (scripts, Apple profiles/declarations, Windows/Android profiles, software installers, setup-experience scripts) plus the separately-checked host-vitals labels.

…-custom-host-vitals

# Conflicts:
#	server/service/mdm.go
Copilot AI review requested due to automatic review settings July 21, 2026 19:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning

  • Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.

Pull request overview

Copilot reviewed 20 out of 21 changed files in this pull request and generated 1 comment.

Files excluded by content exclusion policy (1)
  • changes/49421-android-custom-host-vitals

Comment thread server/fleet/android.go
Comment on lines +125 to +133
contents := string(rawJSON)

// Custom host vitals ($FLEET_HOST_VITAL_<id>) are validated for existence
// (and malformed refs are rejected) at the service layer via
// ds.ValidateReferencedCustomHostVitals, same as Apple/Windows profiles.
// Here we only enforce that the token sits inside a JSON string value,
// mirroring the $FLEET_VAR_* check below, since a token used as a JSON key
// would corrupt the profile structure once substituted at delivery time.
vitalIDs := FindCustomHostVitalIDs(contents)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch on the underlying issue, though the specific premise (fleetctl client-side validation) doesn't apply here — MDMAndroidConfigProfile.ValidateUserProvided is only ever called server-side, always paired with ds.ValidateReferencedCustomHostVitals in the same request, so there wasn't an actual client/server UX mismatch today. Still worth fixing for defense-in-depth: this function was silently treating a malformed ref as "not a vital reference" and relying entirely on the caller to catch it separately. Fixed by rejecting malformed refs directly here too, mirroring ValidateAndroidAppConfiguration.

@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.59829% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.01%. Comparing base (f7f0cfa) to head (1d1ef77).
⚠️ Report is 106 commits behind head on main.

Files with missing lines Patch % Lines
server/fleet/android.go 87.80% 3 Missing and 2 partials ⚠️
ee/server/service/vpp.go 69.23% 2 Missing and 2 partials ⚠️
server/mdm/android/service/profiles.go 87.50% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #49696      +/-   ##
==========================================
+ Coverage   67.90%   68.01%   +0.10%     
==========================================
  Files        3905     3897       -8     
  Lines      249645   249800     +155     
  Branches    13336    13118     -218     
==========================================
+ Hits       169532   169896     +364     
+ Misses      64838    64600     -238     
- Partials    15275    15304      +29     
Flag Coverage Δ
backend 69.41% <90.59%> (+0.10%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

… directly

validateAndroidProfileFleetVariables previously relied on callers pairing it
with ds.ValidateReferencedCustomHostVitals to catch a malformed reference
(e.g. $FLEET_HOST_VITAL_asset_tag); a malformed token was otherwise silently
treated as "not a vital reference" and passed through. Reject it directly
here instead, mirroring ValidateAndroidAppConfiguration, so this doesn't
depend on caller ordering.
Copilot AI review requested due to automatic review settings July 21, 2026 21:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning

  • Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.

Pull request overview

Copilot reviewed 20 out of 21 changed files in this pull request and generated no new comments.

Files excluded by content exclusion policy (1)
  • changes/49421-android-custom-host-vitals

…-custom-host-vitals

# Conflicts:
#	server/datastore/mysql/custom_host_vitals.go
Copilot AI review requested due to automatic review settings July 22, 2026 12:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning

  • Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.

Pull request overview

Copilot reviewed 21 out of 22 changed files in this pull request and generated no new comments.

Files excluded by content exclusion policy (1)
  • changes/49421-android-custom-host-vitals

…tion

The single-profile-upload (NewMDMAndroidConfigProfile) and Android VPP app
config (BatchAssociateVPPApps) call sites for ValidateReferencedCustomHostVitals
were reporting any error as a 422 invalid-argument, including a genuine
datastore failure. Use fleet.IsInvalidReferencedCustomHostVitalsError to only
convert the two known validation error types, matching the pattern already
used at every other ValidateReferencedCustomHostVitals call site (introduced
in #49586).
Copilot AI review requested due to automatic review settings July 22, 2026 13:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning

  • Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.

Pull request overview

Copilot reviewed 21 out of 22 changed files in this pull request and generated 1 comment.

Files excluded by content exclusion policy (1)
  • changes/49421-android-custom-host-vitals

Comment thread server/fleet/android.go Outdated
Comment on lines +185 to +187
// findCustomHostVitalTokens returns the full $FLEET_HOST_VITAL_<id> tokens
// (prefix included) found in s, in the same shape variables.Find returns
// $FLEET_VAR_* names, so both can drive walkJSONForVars.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems worth fixing the doc string to avoid confusion. They behave different.

Copilot AI review requested due to automatic review settings July 23, 2026 17:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning

  • Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.

Pull request overview

Copilot reviewed 21 out of 22 changed files in this pull request and generated 1 comment.

Files excluded by content exclusion policy (1)
  • changes/49421-android-custom-host-vitals

Comment on lines 208 to 214
}

subHost := profiles.AndroidAppConfigSubstitutionHost{
HostID: h.ID,
UUID: h.UUID,
HardwareSerial: h.HardwareSerial,
Platform: h.Platform,

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't like to extend the potential blast radius by touching the worker's code. This can be a follow-up if needed, IMO.

NewMDMAndroidConfigProfile wraps any error from ValidateUserProvided with a
hardcoded "Couldn't add. " prefix. The custom host vitals messages in
validateAndroidProfileFleetVariables already had their own "Couldn't edit
profile. " prefix baked in, so uploading e.g. $FLEET_HOST_VITAL_asset_tag
produced "Couldn't add. Couldn't edit profile. Invalid custom host vital
reference...". Drop the redundant prefix from the vitals-specific messages so
the caller's prefix applies cleanly. The existing $FLEET_VAR_* messages have
the same pre-existing issue but are left alone here to keep this PR scoped to
custom host vitals.
Copilot AI review requested due to automatic review settings July 23, 2026 18:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning

  • Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.

Pull request overview

Copilot reviewed 21 out of 22 changed files in this pull request and generated 1 comment.

Files excluded by content exclusion policy (1)
  • changes/49421-android-custom-host-vitals

Comment thread server/mdm/android/service/profiles.go Outdated
…/edit

AddAppStoreApp/UpdateAppStoreApp never called ds.ValidateReferencedCustomHostVitals,
so a managed app config referencing a non-existent $FLEET_HOST_VITAL_<id> was
silently accepted, unlike the GitOps batch path which already checked it.

Also addresses a review comment on PR #49696: the shared "does this content
reference a Fleet var or custom host vital" check now does a cheap bytes.Contains
prefix check before falling back to the string conversion FindCustomHostVitalIDs
needs, avoiding that allocation in the common case where neither is present.
Copilot AI review requested due to automatic review settings July 24, 2026 12:19
@nulmete
nulmete marked this pull request as ready for review July 24, 2026 12:20
@nulmete
nulmete requested a review from a team as a code owner July 24, 2026 12:20
Comment thread ee/server/service/vpp.go
Comment on lines +384 to +385
if err := svc.ds.ValidateReferencedCustomHostVitals(ctx, []string{string(payload.Configuration)}); err != nil {
if !fleet.IsInvalidReferencedCustomHostVitalsError(err) {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a common pattern when validating referenced custom host vitals across the codebase. I might look into abstracting this into a helper as a follow-up.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning

  • Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.

Pull request overview

Copilot reviewed 22 out of 23 changed files in this pull request and generated 2 comments.

Files excluded by content exclusion policy (1)
  • changes/49421-android-custom-host-vitals
Comments suppressed due to low confidence (2)

server/fleet/android.go:175

  • The custom host vital JSON-position error message is missing the "Couldn't edit profile." prefix used by the Fleet variable validation errors in this same function, making the returned errors inconsistent.
		for _, id := range vitalIDs {
			token := fmt.Sprintf("%s%d", CustomHostVitalPrefix, id)
			if _, inKey := vitalKeyVars[token]; inKey {
				return fmt.Errorf("Custom host vital $%s must be inside a JSON string value.", token)
			}

server/fleet/android.go:178

  • Same as above: the custom host vital JSON-position error should likely use the same "Couldn't edit profile." prefix as the other validation errors returned from this function.
			if _, inStr := vitalStringVars[token]; !inStr {
				return fmt.Errorf("Custom host vital $%s must be inside a JSON string value.", token)
			}

Comment thread server/fleet/android.go
Comment on lines +136 to +148
// ContainsFleetVarOrCustomHostVital reports whether content has a $FLEET_VAR_*
// token or a $FLEET_HOST_VITAL_<id> token. Checks bytes for the vital prefix
// before falling back to fleet.FindCustomHostVitalIDs, which needs a string, to
// avoid that conversion's allocation in the common case where content has neither.
func ContainsFleetVarOrCustomHostVital(content []byte) bool {
if variables.ContainsBytes(content) {
return true
}
if !bytes.Contains(content, []byte(fleet.CustomHostVitalPrefix)) {
return false
}
return len(fleet.FindCustomHostVitalIDs(string(content))) > 0
}

@lucasmrod lucasmrod left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Left some questions/comments.

Comment thread server/fleet/android.go Outdated
// at delivery time.
vitalIDs := FindCustomHostVitalIDs(contents)

found := variables.Find(contents)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit rename found to variables?

Comment thread server/fleet/android.go
Comment thread server/fleet/android.go
Comment on lines +174 to +177
return fmt.Errorf("Custom host vital $%s must be inside a JSON string value.", token)
}
if _, inStr := vitalStringVars[token]; !inStr {
return fmt.Errorf("Custom host vital $%s must be inside a JSON string value.", token)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have test coverage for this specific error? (E.g. seems this covers on a value but not a string?)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 1d1ef77 👍

Comment thread server/fleet/android.go Outdated
Comment on lines +185 to +187
// findCustomHostVitalTokens returns the full $FLEET_HOST_VITAL_<id> tokens
// (prefix included) found in s, in the same shape variables.Find returns
// $FLEET_VAR_* names, so both can drive walkJSONForVars.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems worth fixing the doc string to avoid confusion. They behave different.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we rename now that it's also replacing custom host vitals?

- Rename local var found -> varNames to avoid confusion with the
  variables package (kept distinct from it to avoid a name collision).
- Drop the redundant "Couldn't edit profile." prefix from the
  $FLEET_VAR_* messages in validateAndroidProfileFleetVariables too,
  matching the earlier fix for custom host vital messages, since the
  caller (parseAndValidateAndroidConfigProfile) already prefixes with
  "Couldn't add. "/"Couldn't edit profile. " itself.
- Add test coverage for a custom host vital token that ends up neither
  a decoded JSON key nor a decoded string value (shadowed by a
  duplicate JSON key), exercising that branch distinctly from the
  "used as a key" case.
- Fix the findCustomHostVitalTokens doc comment, which incorrectly
  implied its output has the same shape as variables.Find's.
- Rename SubstituteFleetVarsInAndroidAppConfig to
  SubstituteFleetVarsAndVitalsInAndroidAppConfig now that it also
  substitutes custom host vitals, not just $FLEET_VAR_*.
Copilot AI review requested due to automatic review settings July 28, 2026 21:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning

  • Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.

Pull request overview

Copilot reviewed 22 out of 23 changed files in this pull request and generated no new comments.

Files excluded by content exclusion policy (1)
  • changes/49421-android-custom-host-vitals
Comments suppressed due to low confidence (2)

server/mdm/profiles/android_appconfig.go:146

  • ContainsFleetVarOrCustomHostVital aims to avoid allocations, but []byte(fleet.CustomHostVitalPrefix) allocates (string→[]byte copy) on every call. Since this helper is used on hot paths, precompute the prefix bytes once at package scope and reuse it.
func ContainsFleetVarOrCustomHostVital(content []byte) bool {
	if variables.ContainsBytes(content) {
		return true
	}
	if !bytes.Contains(content, []byte(fleet.CustomHostVitalPrefix)) {

server/fleet/android.go:134

  • Returning errors.New((&InvalidCustomHostVitalRefError{...}).Error()) drops the concrete error type, preventing callers from detecting malformed vital refs via errors.As (e.g. fleet.IsInvalidReferencedCustomHostVitalsError). Return the typed error directly to keep it inspectable and avoid an extra string allocation.
	if malformed := ContainsMalformedCustomHostVitalRefs(contents); len(malformed) > 0 {
		return errors.New((&InvalidCustomHostVitalRefError{Refs: malformed}).Error())
	}

@nulmete
nulmete merged commit f5ca4b5 into main Jul 29, 2026
46 checks passed
@nulmete
nulmete deleted the 49421-android-support-custom-host-vitals branch July 29, 2026 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Android: support custom host vitals in configuration profiles

3 participants