Skip to content

iOS/iPadOS managed config: datastore (#43964) - #44931

Merged
cdcme merged 20 commits into
mainfrom
iosmac/43964-datastore-methods
May 12, 2026
Merged

iOS/iPadOS managed config: datastore (#43964)#44931
cdcme merged 20 commits into
mainfrom
iosmac/43964-datastore-methods

Conversation

@cdcme

@cdcme cdcme commented May 7, 2026

Copy link
Copy Markdown
Member

Part of #38790. Stacked on top of #44930.

Closes #43964.

Adds VPP and in-house datastore methods (GetVPPAppConfiguration, BulkGetVPPAppConfigurations, DeleteVPPAppConfiguration, HasVPPAppConfigurationChanged, plus in-house equivalents) keyed on the merged vpp_app_configurations and in_house_app_configurations tables. Wires them into InsertVPPAppWithTeam, SaveInHouseAppUpdates, and the team / app removal paths.

Two follow-up bug fixes folded into this branch:

  1. iPadOS in-house apps received no configuration: a single .ipa upload creates two in_house_apps rows but config was only stored against the iOS row's id, so iPadOS lookups returned NotFound. Now writes to both sibling rows on insert and propagates updates / clears via installerIDsForInHouseAppSibling.
  2. Single-app PATCH with "configuration": null was inserting empty bytes instead of deleting; aligned the iOS/iPadOS branch in InsertVPPAppWithTeam with the batch path's len > 0 upsert / len == 0 delete semantics.

Summary by CodeRabbit

  • New Features

    • Added support for managing in-house app configurations including storage, updates, and deletion.
    • Expanded VPP app configuration support for iOS/iPadOS platforms with configuration storage and change detection.
  • Refactor

    • Updated Android app configuration handling to improve internal data consistency and streamline configuration management workflows across all app types.

Review Change Stack

jkatz01 and others added 15 commits May 4, 2026 14:42
…o []byte

Replaces the stub from the datastore-methods branch (#43964) with a real
plist validator. Lives in server/fleet/vpp.go alongside the rest of VPP
because validation is part of the InstallApplication command flow.

- ValidateAppleAppConfiguration parses the payload via howett.net/plist
  into a map[string]any (which naturally rejects non-dict roots), then
  walks string-typed leaves checking Fleet variable tokens against the
  app-config allow-list.
- FleetVarsSupportedInAppleAppConfig: host-scoped subset of the variables
  permitted in Apple configuration profiles. Excludes credential / SCEP /
  NDES variables that don't fit the InstallApplication shape.
- Configuration field type: json.RawMessage -> []byte across VPPAppTeam,
  VPPAppStoreApp, AppStoreAppUpdatePayload. The Apple payload is plist
  XML, not JSON, so the field is just opaque bytes. Existing Android
  callers pass and receive []byte transparently (json.RawMessage is
  []byte-underlying, assignable in both directions).
- Updated comments on the three Configuration fields to reflect Apple
  support.
- No size cap in the validator — caps belong at the API layer.
- Empty input is allowed: callers (e.g. the gitops change-detector
  in vpp.go) decide whether to store or clear.

Issue: #43963
Replaces the ad-hoc walker over the parsed plist tree with a raw-bytes scan
using server/variables.Find — the same helper validateConfigProfileFleetVariables
in apple_mdm.go uses for profile validation. Drops the local
fleetVarTokenRegexp, the per-call allowed-set map, the walkAppleAppConfigStrings
recursive walker, and the higher-order-function callback style.

The plist.Unmarshal step stays as the structural check (must be valid plist
with a <dict> root). Token allow-list check is now a flat loop with
slices.Contains.
…figuration

Mirrors the Android datastore methods for the new vpp_app_configurations
and in_house_app_configurations tables (subtask 01). Wires VPP methods
into the existing add/update/delete code paths in vpp.go alongside the
Android branches. In-house methods land here but aren't wired yet —
fleet.InHouseAppPayload doesn't carry a Configuration field until
subtask 02.

ValidateAppleAppConfiguration is a temporary stub (TODO #43963)
replaced by the real plist validator in subtask 02.

Issue: #43964
Migration #44435 dropped team_id from in_house_app_configurations (the
parent in_house_apps row already pins team and platform). Updates the
five in-house datastore methods, the four interface signatures, and the
regenerated mocks to match.
- Test helpers use real datastore APIs (insertInHouseApp, InsertVPPAppWithTeam,
  DeleteInHouseApp) instead of raw SQL where a public function exists.
- Replace fleetdm.com sample with example.com in test plist.
- HasChanged methods early-return false when incoming config is empty —
  null/empty incoming is a no-op; clearing requires explicit Delete*Configuration.
  Destructive-on-omit semantics for gitops will live in #43969 (TODO added).
- HasVPPAppConfigurationChanged / HasInHouseAppConfigurationChanged: empty
  incoming against existing config now returns true (= delete intent), matching
  Android's semantic. Removing a config from gitops YAML clears it.
- SetTeamVPPApps batch upsert path: when iOS/iPadOS Configuration is empty,
  DELETE the existing config row instead of skipping the write. Implements
  the destructive-on-omit behavior at the wiring layer.
- Get*AppConfiguration return type changed from *[]byte to []byte (nil = not
  found, returned alongside notFound error). Drops a non-idiomatic pointer.
- Updated stale comment on VPPAppTeam.Configuration / VPPAppStoreApp.Configuration
  to reflect that Apple platforms now use the field (plist payload).
- Added documentation on Datastore interface that in-house config methods do
  not enforce team-scoped auth — callers must validate.
- Added cross-team isolation assertions to the VPP CRUDFlow test.
…byte

Now that VPPAppTeam.Configuration is []byte (subtask 02), align the Android
datastore surface to match. Drops the now-redundant pointer wrapping on Get*
returns at the same time.

Method signatures:
- GetAndroidAppConfiguration -> ([]byte, error) (was *json.RawMessage)
- GetAndroidAppConfigurationByAppTeamID -> ([]byte, error)
- BulkGetAndroidAppConfigurations -> (map[string][]byte, error)
- HasAndroidAppConfigurationChanged -> takes []byte
- updateAndroidAppConfigurationTx -> takes []byte

Callers updated:
- vpp.go app-store fetch (drops *config deref)
- worker/software_worker.go (configByAppID maps + helper signature)
- Tests in android_test.go, vpp_test.go, software_worker_test.go

Mocks regenerated.

Also fixes a side-effect in TestAndroid/AddDeleteAndroidAppWithConfiguration:
the test previously sent JSON-shaped Configuration to an iOS app (vestigial
"ios shouldn't have configuration" wording). The real iOS validator from
subtask 02 now rejects non-plist input, so the test seeds a valid plist
fragment instead.
Adds Configuration on InHouseAppPayload and the SoftwareInstaller
upload/update payloads, and threads it through MatchOrCreateSoftwareInstaller,
insertInHouseApp, and SaveInHouseAppUpdates inside their existing transactions.
GetInHouseAppMetadataByTeamAndTitleID now hydrates the Configuration field.
@codecov

codecov Bot commented May 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 74.26160% with 61 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.87%. Comparing base (3517ff6) to head (310e027).
⚠️ Report is 22 commits behind head on main.

Files with missing lines Patch % Lines
server/datastore/mysql/vpp.go 70.00% 23 Missing and 16 partials ⚠️
server/datastore/mysql/in_house_apps.go 75.55% 12 Missing and 10 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #44931      +/-   ##
==========================================
+ Coverage   66.86%   66.87%   +0.01%     
==========================================
  Files        2724     2665      -59     
  Lines      219083   216717    -2366     
  Branches    10590     9647     -943     
==========================================
- Hits       146489   144940    -1549     
+ Misses      59429    58590     -839     
- Partials    13165    13187      +22     
Flag Coverage Δ
backend 68.74% <74.26%> (+<0.01%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 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.

@cdcme
cdcme marked this pull request as ready for review May 9, 2026 00:20
@cdcme
cdcme requested a review from a team as a code owner May 9, 2026 00:20

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

@cdcme

cdcme commented May 11, 2026

Copy link
Copy Markdown
Member Author

@claude review once

Comment thread server/datastore/mysql/in_house_apps.go
Comment thread server/datastore/mysql/vpp.go
jkatz01 added 2 commits May 12, 2026 10:41
Verify the three-state contract on SaveInHouseAppUpdates (nil =
leave unchanged, empty = clear, non-empty = set) and that each
case applies to both the iOS row and its iPadOS sibling.

Also reflow the VPPAppTeam/VPPAppStoreApp Configuration comments
so the JSON-vs-XML note sits on its own line.
The singular PATCH /software/titles/{id}/package endpoint targets
one software title. iOS and iPadOS rows for the same .ipa live
under different title_ids, so silently mirroring the configuration
across siblings on update would mutate a title the user didn't edit.

Drop installerIDsForInHouseAppSibling and write only to the
installer ID in the payload. The insert path keeps the dual-row
write because a single .ipa upload genuinely owns both rows.

Tests updated to match: the CRUD three-state semantics now assert
against the targeted row only, and testInHouseAppConfigSiblingRows
keeps the insert-time propagation check but drops the obsolete
update-time propagation assertion.
Comment thread server/datastore/mysql/in_house_apps.go
@cdcme
cdcme requested a review from jkatz01 May 12, 2026 16:34
return ctxerr.Wrap(ctx, err, "setting configuration for android app")
switch toAdd.Platform {
case fleet.AndroidPlatform:
if toAdd.Configuration != nil {

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.

This is a separate bug that I will file later. The GitOps behaviour for this should be that if the config is not set, it should be deleted.

jkatz01
jkatz01 previously approved these changes May 12, 2026
Base automatically changed from iosmac/43963-validation to main May 12, 2026 17:23
@cdcme
cdcme dismissed jkatz01’s stale review May 12, 2026 17:23

The base branch was changed.

Copilot AI review requested due to automatic review settings May 12, 2026 17:28

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.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds managed app configuration support for iOS/iPadOS VPP and in-house apps via new datastore CRUD/“has changed” APIs, and wires those configurations into existing install/update/removal flows.

Changes:

  • Introduces VPP and in-house app configuration datastore methods (get/bulk-get/delete/has-changed) and updates mocks/interfaces accordingly.
  • Fixes iPadOS in-house configuration not being persisted for sibling rows and aligns single vs batch configuration “clear” semantics.
  • Updates Android app configuration plumbing to use []byte instead of *json.RawMessage, plus adds datastore tests for new config flows.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
server/worker/software_worker_test.go Updates Android config mock signatures to []byte.
server/worker/software_worker.go Updates Android config handling types to []byte.
server/mock/datastore_mock.go Updates Android config method signatures and adds VPP/in-house config mocks.
server/fleet/software_installer.go Adds in-house app configuration fields to installer structs/payloads.
server/fleet/in_house_apps.go Adds configuration to in-house app payload.
server/fleet/datastore.go Extends datastore interface for VPP/in-house config + adjusts Android signatures.
server/datastore/mysql/vpp_test.go Adds CRUD and has-changed tests for VPP app configs; adjusts Android config assertions.
server/datastore/mysql/vpp.go Wires VPP+iOS/iPadOS config fetching/upserting/deleting and adds VPP config datastore methods.
server/datastore/mysql/software_installers.go Persists in-house configuration on insert path.
server/datastore/mysql/in_house_apps_test.go Adds CRUD/sibling/has-changed tests for in-house configs; expands update test coverage.
server/datastore/mysql/in_house_apps.go Writes config to both iOS/iPadOS sibling rows, adds config CRUD/has-changed methods, updates read/update flows.
server/datastore/mysql/android_test.go Updates Android config tests for []byte return type + adjusts a config payload in a VPP-related test.
server/datastore/mysql/android.go Changes Android config datastore methods to return/accept []byte instead of json.RawMessage.

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

@@ -2685,6 +2742,11 @@ func (ds *Datastore) hasAppStoreAppChanged(ctx context.Context, teamID *uint, in
if configurationChanged && len(incomingApp.Configuration) == 0 {
incomingApp.Configuration = json.RawMessage("{}")
require.Equal(t, iosCfg, bulk[adamID])

// Cross-team isolation: same (adamID, platform) on a different team is independent.
otherTeamID := teamID + 1
Comment on lines +144 to +145
// Configuration is the in-house app's managed app configuration (iOS / iPadOS only) as returned in API responses: a JSON string of XML.
Configuration json.RawMessage `json:"configuration,omitempty" db:"-"`
@coderabbitai

coderabbitai Bot commented May 12, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

This PR migrates Android app configuration datastore methods to use []byte instead of json.RawMessage and adds comprehensive configuration CRUD support for iOS/iPadOS VPP and in-house apps. The changes update method signatures across the datastore interface, implementation files, and all call sites; wire configuration persistence into app creation, update, and deletion flows; and add extensive test coverage for the new functionality.

Possibly related PRs

  • fleetdm/fleet#37740: Earlier Android app configuration datastore updates (GetAndroidAppConfiguration, HasAndroidAppConfigurationChanged, BulkGetAndroidAppConfigurations); this PR follows up with type changes and extends to VPP/in-house apps.
  • fleetdm/fleet#44930: Adds ValidateAppleAppConfiguration for iOS/iPadOS managed app config validation; this PR consumes that validator in the upsert helper methods for VPP and in-house apps.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 18.18% 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
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly and specifically describes the main change: adding datastore support for iOS/iPadOS managed app configuration.
Linked Issues check ✅ Passed The PR comprehensively fulfills issue #43964 objectives: adds platform-parameterized VPP and in-house configuration methods (Get, BulkGet, Delete, HasChanged, updateTx), integrates them into VPP add/update/delete flows, wires deletions on team/app removal, updates Datastore interface and mocks, and includes unit tests.
Out of Scope Changes check ✅ Passed All changes are scoped to requirements: Android configuration refactored to use []byte (aligning with new iOS/iPadOS types), in-house app configuration added (mentioned in PR objectives as proper extension), software installer and worker changes support the configuration feature, and tests validate the full integration.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 iosmac/43964-datastore-methods

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 and usage tips.

@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: 3

🤖 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/in_house_apps_test.go`:
- Around line 1873-1874: The SELECT used to find the sibling row via
sqlx.GetContext (populating ipadID) is only filtered by filename and platform
which can be ambiguous; tighten the WHERE clause on the in_house_apps query used
in ds.reader/test by adding a stable parent key (for example include the same
bundle, title or global_or_team_id used in the fixture) so the lookup targets
the exact sibling row—update the query in the sqlx.GetContext call that selects
id FROM in_house_apps (and the analogous query at the other occurrence) to
include that additional AND condition referencing the chosen parent column(s).

In `@server/datastore/mysql/in_house_apps.go`:
- Around line 1709-1727: Team deletion currently omits in_house_apps, leaving
orphaned in_house_apps and their configurations; update the team deletion path
(the Datastore method that deletes teams—e.g., DeleteTeam/DeleteTeamByID) to
also remove rows from the in_house_apps table (DELETE FROM in_house_apps WHERE
team_id = ?) before or within the same transaction, or alternatively add a
DB-level foreign key on in_house_apps.team_id -> teams.id WITH ON DELETE CASCADE
so in_house_apps (and transitively in_house_app_configurations) are removed
automatically; ensure this change is performed within the same transaction and
reference existing helpers like DeleteInHouseAppConfiguration or
ds.writer(ctx).ExecContext for consistency.

In `@server/datastore/mysql/vpp_test.go`:
- Around line 3152-3155: The test currently only inserts global (no-team) VPP
rows and then asserts behavior using teamID and a synthetic teamID+1, which
doesn't exercise real team-scoped app rows; modify the test around
test.CreateInsertGlobalVPPToken, setupTestVPPApp and setupTestTeam so that for
each team used in assertions you create or assign real app rows scoped to that
team (e.g., call setupTestVPPApp or the equivalent app-creation helper with the
actual teamID and with the other teamID used in assertions) before performing
the team-isolation assertions; ensure the same change is applied to the other
test blocks referenced (around the sections corresponding to lines 3188-3203 and
3228-3230) so each assertion uses actual team-scoped data rather than synthetic
IDs.
🪄 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: dbea7c3f-c3ec-4e09-b2de-474eeccd3fcb

📥 Commits

Reviewing files that changed from the base of the PR and between 3517ff6 and 310e027.

📒 Files selected for processing (13)
  • server/datastore/mysql/android.go
  • server/datastore/mysql/android_test.go
  • server/datastore/mysql/in_house_apps.go
  • server/datastore/mysql/in_house_apps_test.go
  • server/datastore/mysql/software_installers.go
  • server/datastore/mysql/vpp.go
  • server/datastore/mysql/vpp_test.go
  • server/fleet/datastore.go
  • server/fleet/in_house_apps.go
  • server/fleet/software_installer.go
  • server/mock/datastore_mock.go
  • server/worker/software_worker.go
  • server/worker/software_worker_test.go

Comment on lines +1873 to +1874
err = sqlx.GetContext(testCtx(), ds.reader(testCtx()), &ipadID,
`SELECT id FROM in_house_apps WHERE filename = ? AND platform = 'ipados'`, filename)

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.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Tighten sibling-row lookup scoping in test SQL.

These lookups filter only by filename + platform, which can become non-deterministic if multiple rows share filename in broader fixture evolution. Prefer scoping via a stable parent key (e.g., same bundle/title/global_or_team_id) when selecting the sibling row.

As per coding guidelines: “ensure that appropriate filtering criteria are applied… check for missing WHERE clauses or incorrect filtering that could lead to incorrect or non-deterministic results.”

Also applies to: 1954-1955

🤖 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/in_house_apps_test.go` around lines 1873 - 1874, The
SELECT used to find the sibling row via sqlx.GetContext (populating ipadID) is
only filtered by filename and platform which can be ambiguous; tighten the WHERE
clause on the in_house_apps query used in ds.reader/test by adding a stable
parent key (for example include the same bundle, title or global_or_team_id used
in the fixture) so the lookup targets the exact sibling row—update the query in
the sqlx.GetContext call that selects id FROM in_house_apps (and the analogous
query at the other occurrence) to include that additional AND condition
referencing the chosen parent column(s).

Comment on lines +1709 to +1727
func (ds *Datastore) DeleteInHouseAppConfiguration(ctx context.Context, inHouseAppID uint) error {
const stmt = `DELETE FROM in_house_app_configurations WHERE in_house_app_id = ?`

result, err := ds.writer(ctx).ExecContext(ctx, stmt, inHouseAppID)
if err != nil {
return ctxerr.Wrap(ctx, err, "delete in-house app configuration")
}

rows, err := result.RowsAffected()
if err != nil {
return ctxerr.Wrap(ctx, err, "delete in-house app configuration rows affected")
}

if rows == 0 {
return ctxerr.Wrap(ctx, notFound("InHouseAppConfiguration"))
}

return nil
}

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.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Schema / migrations mentioning in_house_app_configurations =="
rg -n -C4 'in_house_app_configurations|FOREIGN KEY|ON DELETE CASCADE' server

echo
echo "== In-house app delete paths and config delete call sites =="
rg -n -C3 '\bDeleteInHouseApp\(|DELETE FROM in_house_apps|deleteAllInHouseInstallersInTeam|deleteInHouseInstallersNotInList|DeleteInHouseAppConfiguration\(' server/datastore/mysql

Repository: fleetdm/fleet

Length of output: 50370


🏁 Script executed:

fd -type f -name "*.go" -path "*migrations*" | xargs rg -l "in_house_app_configurations"

Repository: fleetdm/fleet

Length of output: 501


🏁 Script executed:

rg -n "CREATE TABLE.*in_house_app_configurations|in_house_app_configurations.*FOREIGN KEY" server

Repository: fleetdm/fleet

Length of output: 449


🏁 Script executed:

rg -n -A10 "func.*DeleteInHouseApp" server/datastore/mysql/in_house_apps.go | head -60

Repository: fleetdm/fleet

Length of output: 1237


🏁 Script executed:

rg -n -B2 -A8 "DELETE FROM in_house_apps|DeleteInHouseApp\(" server/datastore/mysql/in_house_apps.go | head -100

Repository: fleetdm/fleet

Length of output: 858


🏁 Script executed:

rg -n -B5 -A20 "func.*DeleteTeam" server/datastore/mysql/teams.go | head -80

Repository: fleetdm/fleet

Length of output: 1562


🏁 Script executed:

rg -n -B2 -A2 "DeleteInHouseApp\|in_house_apps" server/datastore/mysql/teams.go

Repository: fleetdm/fleet

Length of output: 39


🏁 Script executed:

rg -n "in_house_apps" server/datastore/mysql/teams.go

Repository: fleetdm/fleet

Length of output: 39


🏁 Script executed:

rg -n -B20 "var teamRefs" server/datastore/mysql/teams.go | head -50

Repository: fleetdm/fleet

Length of output: 604


🏁 Script executed:

rg -n -A30 "var teamRefs = \[\]string" server/datastore/mysql/teams.go

Repository: fleetdm/fleet

Length of output: 1524


🏁 Script executed:

rg -n -A15 "CREATE TABLE.*in_house_apps" server/datastore/mysql/schema.sql

Repository: fleetdm/fleet

Length of output: 1155


🏁 Script executed:

rg -n -A20 "func (ds \*Datastore) DeleteTeam" server/datastore/mysql/teams.go | grep -A50 "DELETE FROM"

Repository: fleetdm/fleet

Length of output: 39


🏁 Script executed:

rg -n -A25 "CREATE TABLE.*in_house_apps" server/datastore/mysql/schema.sql

Repository: fleetdm/fleet

Length of output: 1821


🏁 Script executed:

rg -n "DELETE FROM in_house_apps\|team_id.*REFERENCES teams" server/datastore/mysql/schema.sql

Repository: fleetdm/fleet

Length of output: 39


🏁 Script executed:

cat -n server/datastore/mysql/teams.go | sed -n '163,220p'

Repository: fleetdm/fleet

Length of output: 3058


🏁 Script executed:

rg -n "in_house_apps.*team_id\|FOREIGN KEY.*team_id.*in_house" server/datastore/mysql/schema.sql

Repository: fleetdm/fleet

Length of output: 39


🏁 Script executed:

rg -n "in_house_apps" server/datastore/mysql/migrations/tables/ --type-list

Repository: fleetdm/fleet

Length of output: 447


🏁 Script executed:

cat server/datastore/mysql/migrations/tables/20260429180725_CreateTableAppConfigurations.go

Repository: fleetdm/fleet

Length of output: 1959


🏁 Script executed:

rg -n -A40 "CREATE TABLE.*in_house_apps" server/datastore/mysql/schema.sql | grep -A35 "^1449"

Repository: fleetdm/fleet

Length of output: 2495


🏁 Script executed:

rg -n "FOREIGN KEY.*team_id.*in_house_apps\|in_house_apps.*team_id.*FOREIGN" server/datastore/mysql/schema.sql

Repository: fleetdm/fleet

Length of output: 39


Add in_house_apps deletion to DeleteTeam to prevent orphaned configurations.

The in_house_app_configurations table correctly has ON DELETE CASCADE on the in_house_app_id foreign key. However, in_house_apps.team_id has no foreign key constraint to teams.id, and in_house_apps is not included in the team deletion cleanup. When a team is deleted, orphaned in_house_apps rows remain, which indirectly orphans their associated configurations since the configs rely on the parent app existing.

Add in_house_apps to the team deletion path (either via explicit deletion or by adding a FOREIGN KEY (team_id) REFERENCES teams (id) ON DELETE CASCADE constraint to the in_house_apps table).

🤖 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/in_house_apps.go` around lines 1709 - 1727, Team
deletion currently omits in_house_apps, leaving orphaned in_house_apps and their
configurations; update the team deletion path (the Datastore method that deletes
teams—e.g., DeleteTeam/DeleteTeamByID) to also remove rows from the
in_house_apps table (DELETE FROM in_house_apps WHERE team_id = ?) before or
within the same transaction, or alternatively add a DB-level foreign key on
in_house_apps.team_id -> teams.id WITH ON DELETE CASCADE so in_house_apps (and
transitively in_house_app_configurations) are removed automatically; ensure this
change is performed within the same transaction and reference existing helpers
like DeleteInHouseAppConfiguration or ds.writer(ctx).ExecContext for
consistency.

Comment on lines +3152 to +3155
test.CreateInsertGlobalVPPToken(t, ds)
setupTestVPPApp(t, ds, adamID, fleet.IOSPlatform)
setupTestVPPApp(t, ds, adamID, fleet.IPadOSPlatform)
teamID := setupTestTeam(t, ds)

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.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Use real team-scoped app rows in team-isolation assertions.

This flow currently inserts no-team VPP rows, then validates config behavior using teamID and a synthetic teamID + 1. That can pass without exercising the real team-association path. Create/assign app rows for each team used in the assertions to make this test accurately guard team scoping behavior.

Also applies to: 3188-3203, 3228-3230

🤖 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/vpp_test.go` around lines 3152 - 3155, The test
currently only inserts global (no-team) VPP rows and then asserts behavior using
teamID and a synthetic teamID+1, which doesn't exercise real team-scoped app
rows; modify the test around test.CreateInsertGlobalVPPToken, setupTestVPPApp
and setupTestTeam so that for each team used in assertions you create or assign
real app rows scoped to that team (e.g., call setupTestVPPApp or the equivalent
app-creation helper with the actual teamID and with the other teamID used in
assertions) before performing the team-isolation assertions; ensure the same
change is applied to the other test blocks referenced (around the sections
corresponding to lines 3188-3203 and 3228-3230) so each assertion uses actual
team-scoped data rather than synthetic IDs.

@cdcme
cdcme merged commit 36de1a6 into main May 12, 2026
40 of 42 checks passed
@cdcme
cdcme deleted the iosmac/43964-datastore-methods branch May 12, 2026 17:39
cdcme added a commit that referenced this pull request May 12, 2026
Part of #38790. Stacked on top of #44931.

Closes #43965.

Wires managed configuration through the existing REST endpoints (`POST
/api/.../app_store_apps`, `PATCH
/api/.../software/titles/:id/app_store_app`, in-house `.ipa` upload /
update). Validation runs at the service layer for iOS / iPadOS; macOS
VPP installs silently drop the field. Wire format: a JSON-encoded string
of the XML plist on POST/PATCH and on GET single-title responses (not
base64).

Includes `server/service/integration_apple_vpp_config_test.go` with
end-to-end coverage: add / update with valid plist, allowed
`$FLEET_VAR_HOST_UUID`, omit-field-no-change, **`configuration: null` →
row deleted** (regression test for the clear-on-null fix in #43964),
malformed XML → 422, disallowed Fleet variable → 422, and macOS
silent-drop pre- / post-validation.

Also drops a stray `fmt.Println("auth")` in `SoftwareTitleByID`'s
authorization-failure branch.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
* Added support for managed app configuration on iOS/iPadOS devices
through VPP and in-house installers.
* Configuration now validates plist format and detects disallowed Fleet
variables.
  
* **Bug Fixes**
  * macOS apps now correctly ignore configuration settings as expected.

* **Tests**
* Added comprehensive integration tests for Apple VPP and in-house
installer configuration workflows.

[![Review Change
Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/fleetdm/fleet/pull/44932)

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: jkatz01 <yehonatankatz@gmail.com>
Co-authored-by: Jonathan Katz <44128041+jkatz01@users.noreply.github.com>
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.

IOSMAC: Datastore methods for iOS/iPadOS managed app config

3 participants