Multiple packages: install-time precedence and setup experience#48708
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## feat/28108-multiple-custom-packages #48708 +/- ##
======================================================================
Coverage ? 68.04%
======================================================================
Files ? 3680
Lines ? 234292
Branches ? 12454
======================================================================
Hits ? 159427
Misses ? 60512
Partials ? 14353
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@claude review once. |
There was a problem hiding this comment.
Not approving auto — this touches install precedence, setup experience queuing, policy automation, and multi-package software display across service and datastore layers with new SQL (CTEs, window functions, subquery MIN() collapses), and it changes API surface (new software_installer_id field). Bug hunter found no issues, but the scope + criticality warrants a human eye.
Extended reasoning...
Overview
The PR implements the runtime side of Fleet's multi-package feature on top of the feat/28108-multiple-custom-packages branch. Roughly four behavior changes:
- Install precedence (
ee/server/service/software_installers.go): NewresolveFirstAddedInScopeInstallerpicks the first-added (smallest installer_id) in-scope, platform-compatible package. RewiresInstallSoftwareTitleandSelfServiceInstallSoftwareTitleto use it. Preserves the two distinct error messages (not-in-scope vs not-self-service). - Setup experience (
server/datastore/mysql/setup_experience.go): AddsROW_NUMBER() OVER (PARTITION BY software_title_id ...)around the combined installer/VPP UNION so a multi-package title queues only its first-added package. - Policy automation (
server/datastore/mysql/policies.go,server/service/team_policies.go,server/fleet/*policies.go): GitOpsApplyPolicySpecsnowORDER BY si_id IS NULL, si_id ASC LIMIT 1to pin to the first-added package. Adds a newsoftware_installer_idfield onTeamPolicyRequest/ModifyPolicyPayloadto override the default.DeleteSoftwareInstallerre-points policies to the first-added surviving package before deleting, or lets the FK 409 fire when there is no survivor. - Host software listing (
server/datastore/mysql/software.go):filterSoftwareInstallersByLabelreturns a resolved package map instead of a filter; a newresolveFirstAddedInstallersForHostruns the four label-scope CTEs across every active package of the titles.ListHostSoftwarepins the display join to those resolved installer IDs, andhostSoftwareInstalls/hostSoftwareUninstallsaddMIN(id)subqueries to collapse historical rows.
Security risks
No authn/authz code changes. The user-facing surface additions (software_installer_id on policy create/modify) route through the existing authz.ActionWrite check on policies, and the new field is validated against GetSoftwarePackagesByTeamAndTitleID for the same team, which rejects cross-team installer IDs with a 400. The new SQL uses sqlx.Named+sqlx.In with parameterized args (no interpolation), so no injection surface.
Level of scrutiny
High. This is production-critical multi-tenant code: (a) install execution path (chooses which package to install on a host), (b) policy automation on failure (which package a failing policy triggers), (c) setup experience queuing (what runs on new-host bootstrap), (d) inventory display for tens of thousands of hosts. A precedence bug here can queue the wrong package for enrollments-in-flight or leak availability across labels. The four candidate tiers (both/inScopeAny/compatAny/any) in resolveFirstAddedInstallersForHost are new and subtle — the fallback-any tier means a title is never hidden but the installed package can be non-compatible on the display side. Worth a human verifying that matches product intent.
Other factors
- Base branch is
feat/28108-multiple-custom-packages, notmain. This is a first-pass PR onto a feature branch, and the merge intomainwill presumably be reviewed again downstream — but this PR still lands on that feature branch and merits scrutiny now. - Test coverage is decent (integration tests for install precedence, policy default/override/reject, delete re-point; datastore tests for first-added policy resolution, setup-experience dedupe, list-host-software display). Codecov flags ~35 uncovered lines, mostly in fallback branches of the resolver and the modify-policy validation.
- PR title says "first pass" — the author signals more iteration to come.
- No prior human reviews on the timeline other than the request-for-review comment.
There was a problem hiding this comment.
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.
a5c85af
into
feat/28108-multiple-custom-packages
Related issue: Resolves #48398
Applies first-added-wins install precedence everywhere a host can match more than one package of a software title: manual install, self-service, policy auto-install, setup experience, and the host-software read all resolve the first-added package the host is in label scope for. Policy automations can now target a specific package (defaulting to first-added), and deleting a package re-points its automations to the first-added surviving package.
Checklist for submitter
SELECT *is avoided, SQL injection is prevented (using placeholders for values in statements).Testing