Skip to content

Patch when closed: migration, types, FMA ingestion - #49691

Merged
jkatz01 merged 12 commits into
feat/39962-patch-when-closedfrom
49415-patch-when-closed-migration-types-manifests
Jul 22, 2026
Merged

Patch when closed: migration, types, FMA ingestion#49691
jkatz01 merged 12 commits into
feat/39962-patch-when-closedfrom
49415-patch-when-closed-migration-types-manifests

Conversation

@jkatz01

@jkatz01 jkatz01 commented Jul 21, 2026

Copy link
Copy Markdown
Member

Related issue: Resolves #49415

Checklist for submitter

If some of the following don't apply, delete the relevant line.

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

  • Input data is properly validated, SELECT * is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters.

  • Timeouts are implemented and retries are limited to avoid infinite loops

  • If paths of existing endpoints are modified without backwards compatibility, checked the frontend/CLI for any necessary changes

Testing

  • Added/updated automated tests

  • Where appropriate, automated tests simulate multiple hosts and test for host isolation (updates to one hosts's records do not affect another)

  • QA'd all new/changed functionality manually

    • Tested with FLEET_DEV_MAINTAINED_APPS_BASE_URL pointed to a branch with the new queries. If there is an open query, adding the FMA adds it to app_open_query. If not, it remains empty.
    • Also tested a few of the SELECT 1 WHERE NOT EXISTS (SELECT 1 FROM apps a JOIN processes p ON p.path LIKE concat(a.path, '/%%') WHERE a.bundle_identifier = '%s'); queries locally on apps available in dogfood self service, looks like it will work reasonably well for a start.

Database migrations

  • Checked schema for all modified table for columns that will auto-update timestamps during migration.
  • N/A Confirmed that updating the timestamps is acceptable, and will not cause unwanted side effects.
    • No timestamp updates
  • Ensured the correct collation is explicitly set for character columns (COLLATE utf8mb4_unicode_ci).

Summary by CodeRabbit

  • New Features

    • Added a policy option to skip patch installation while the managed application is open.
    • Added support for detecting open applications on macOS and Windows.
    • Maintained applications now preserve and use app-open checks during installation and updates.
    • Policy and installer responses now include the related configuration fields.
  • Bug Fixes

    • Ensured app-open settings are retained when installers are created, updated, or retrieved.
    • Existing policies and installers receive safe default values for the new settings.

jkatz01 added 9 commits July 20, 2026 16:38
The manifest pipeline generates queries.open, but nothing copied it onto
the installer, so app_open_query was always stored empty. Map it onto
MaintainedApp and carry it into the install payload across the add,
auto-update, clone, and GitOps batch paths, mirroring patch_query.

Keep the field internal (json:"-") like patch_query so it stays out of
software title API responses.
…ed-migration-types-manifests

# Conflicts:
#	server/datastore/mysql/schema.sql
@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.30435% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.90%. Comparing base (d81902f) to head (b9a5107).
⚠️ Report is 1 commits behind head on feat/39962-patch-when-closed.

Files with missing lines Patch % Lines
...igrations/tables/20260721173820_PatchWhenClosed.go 68.42% 4 Missing and 2 partials ⚠️
Additional details and impacted files
@@                      Coverage Diff                      @@
##           feat/39962-patch-when-closed   #49691   +/-   ##
=============================================================
  Coverage                         67.89%   67.90%           
=============================================================
  Files                              3890     3891    +1     
  Lines                            248437   248495   +58     
  Branches                          13022    13022           
=============================================================
+ Hits                             168679   168743   +64     
  Misses                            64543    64543           
+ Partials                          15215    15209    -6     
Flag Coverage Δ
backend 69.28% <91.30%> (+0.01%) ⬆️

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.

@jkatz01
jkatz01 marked this pull request as ready for review July 21, 2026 20:01
@jkatz01
jkatz01 requested a review from a team as a code owner July 21, 2026 20:01
@jkatz01

jkatz01 commented Jul 21, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Adds the patch_when_closed policy field and the app_open_query installer field through migrations, schemas, and Go models. Maintained-app ingesters now generate platform-specific queries that detect whether an application is closed. The query is serialized in manifests, hydrated into maintained-app data, propagated through installer payloads and services, and persisted by MySQL. Tests cover query generation, migration defaults, installer round trips, ingester output, and updated policy response fixtures.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% 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 migration, type, and FMA ingestion changes.
Description check ✅ Passed The description matches the template with the related issue, checklist, testing, and migration sections filled in.
Linked Issues check ✅ Passed The changes cover the migration, type updates, open-query generation/hydration, and installer propagation required by #49415.
Out of Scope Changes check ✅ Passed The diff stays focused on patch-when-closed plumbing, tests, and fixture updates; no clear unrelated code was introduced.
✨ 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 49415-patch-when-closed-migration-types-manifests

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: 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 `@pkg/patch_policy/patch_policy.go`:
- Around line 165-186: Escape SQL string-literal values before interpolation in
defaultMacOSOpenQuery and defaultWindowsOpenQuery, including bundleIdentifier
and the derived executable from softwareTitle. Preserve the existing query
behavior while ensuring apostrophes and other literal content cannot break or
alter the SQL predicates.

In `@server/datastore/mysql/software_installers.go`:
- Around line 2988-2989: Update the wasUpdatedArgs construction in both
checkExistingInstaller and checkExistingActiveInstaller to include
installer.AppOpenQuery immediately after installer.PreInstallQuery. Ensure both
comparison query variants treat app-open-query-only changes as metadata
modifications so the existing pending-install side effects run.

In `@server/mdm/maintainedapps/sync.go`:
- Line 261: Update the cached-version hydration branch in the sync flow to copy
cached.AppOpenQuery onto the hydrated app before returning, matching the
manifest hydration assignment to app.AppOpenQuery. Ensure pinned cache hits
preserve the managed query and existing patch-when-closed 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: cfc6a55c-7fe9-4679-be31-449789f20ff8

📥 Commits

Reviewing files that changed from the base of the PR and between d81902f and a0050b6.

📒 Files selected for processing (23)
  • cmd/fleetctl/fleetctl/api_test.go
  • cmd/fleetctl/fleetctl/testdata/expectedHostDetailResponseJson.json
  • cmd/fleetctl/fleetctl/testdata/expectedHostDetailResponseYaml.yml
  • ee/maintained-apps/ingesters/homebrew/ingester.go
  • ee/maintained-apps/ingesters/homebrew/ingester_test.go
  • ee/maintained-apps/ingesters/winget/ingester.go
  • ee/maintained-apps/ingesters/winget/ingester_test.go
  • ee/maintained-apps/maintained_apps.go
  • ee/server/service/maintained_apps.go
  • ee/server/service/maintained_apps_auto_update.go
  • ee/server/service/software_installers.go
  • pkg/patch_policy/patch_policy.go
  • pkg/patch_policy/patch_policy_test.go
  • server/datastore/mysql/migrations/tables/20260721173820_PatchWhenClosed.go
  • server/datastore/mysql/migrations/tables/20260721173820_PatchWhenClosed_test.go
  • server/datastore/mysql/schema.sql
  • server/datastore/mysql/software_installers.go
  • server/datastore/mysql/software_installers_test.go
  • server/fleet/maintained_apps.go
  • server/fleet/policies.go
  • server/fleet/software_installer.go
  • server/mdm/maintainedapps/sync.go
  • server/webhooks/failing_policies_test.go

Comment thread pkg/patch_policy/patch_policy.go Outdated
Comment thread server/datastore/mysql/software_installers.go
Comment thread server/mdm/maintainedapps/sync.go
Comment thread pkg/patch_policy/patch_policy.go Outdated
Comment thread pkg/patch_policy/patch_policy.go
Comment thread ee/maintained-apps/ingesters/winget/ingester.go Outdated
Comment thread ee/maintained-apps/ingesters/homebrew/ingester.go Outdated
Comment thread pkg/patch_policy/patch_policy.go
@jkatz01
jkatz01 merged commit 61bae56 into feat/39962-patch-when-closed Jul 22, 2026
28 of 29 checks passed
@jkatz01
jkatz01 deleted the 49415-patch-when-closed-migration-types-manifests branch July 22, 2026 18:27
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.

Patch when closed: migration, types, and FMA manifest "is app open" query

2 participants