<!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **Related issue:** Resolves #46656 `server/mdm/apple/vpp.do` retried transient Apple errors by **calling itself recursively**, with the rate-limit branch nesting `retry.Do` inside `retry.Do`. This change replaces the recursion with a single retry loop (respecting the prior 1 initial attempt + 3 retries), closes each response before retrying, honors Apple's `Retry-After` capped at 30s so that a multi-minute value can't block a synchronous request, and threads `context` through the VPP calls so the backoff is cancellable. The retry timings are otherwise unchanged from before. Following @sgress454 suggestion, I considered routing this through the shared `retry.Do` helper (a single attempt wrapped in `retry.Do` + an error filter) but figured out that: - retry.Do` owns its own wait schedule and its error filter returns an outcome enum rather than a duration, so it can't honor Apple's per-response `Retry-After` value. - also, I'd have to change the `retry` package to receive an extra `ctx` param so that the backoff is context-aware (which IMHO is more blast radius than this incident fix should carry). # Checklist for submitter - [x] Changes file added for user-visible changes in `changes/`. - [x] Timeouts are implemented and retries are limited to avoid infinite loops ## Testing - [x] Added/updated automated tests - [x] QA'd all new/changed functionality manually **What was verified.** The new automated test cannot run against `main` (the fix changes the VPP function signatures and adds the retry knobs), so to confirm the actual failure mode I checked out `main` and ran a small repro that drives the VPP client against an Apple endpoint that always returns the rate-limit error. On `main`, the call **never returns** — `do()` recurses without bound — and the repro times out: ``` --- FAIL: TestReproUnboundedRecursionOnMain (10.00s) zz_repro_main_test.go:30: AssociateAssets did NOT return within 10s — unbounded retry recursion in do() on main FAIL FAIL github.com/fleetdm/fleet/v4/server/mdm/apple/vpp 10.642s ``` On this branch the same scenario returns a bounded error promptly. That behavior is covered by the new `TestDoRetryIsBoundedAndNonRecursive` (bounded rate-limit retries, `Retry-After` honored-but-capped, and context cancellation), and the full `server/mdm/apple/vpp` package passes. **I did not perform an end-to-end QA against a live Apple endpoint**. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Fixed a server out-of-memory crash that occurred when Apple VPP API repeatedly returned transient errors during VPP operations, including app installs, user registration, and license seat releases. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
88 lines
3.5 KiB
Go
88 lines
3.5 KiB
Go
package service
|
|
|
|
import (
|
|
"context"
|
|
"errors"
|
|
"net/http"
|
|
|
|
"github.com/fleetdm/fleet/v4/server/contexts/ctxerr"
|
|
"github.com/fleetdm/fleet/v4/server/fleet"
|
|
"github.com/fleetdm/fleet/v4/server/mdm/apple/vpp"
|
|
"github.com/google/uuid"
|
|
)
|
|
|
|
// errMissingManagedAppleID is returned when ensureVPPClientUser is called for
|
|
// a user-enrolled host whose Managed Apple ID hasn't yet been surfaced from
|
|
// nanomdm's TokenUpdate hook. Callers should surface a retryable user-facing
|
|
// message — the value is normally available a few minutes after enrollment.
|
|
var errMissingManagedAppleID = fleet.NewUserMessageError(
|
|
errors.New("Couldn't install. Fleet hasn't received a Managed Apple ID for this host yet. Please wait a few minutes after enrollment and try again."),
|
|
http.StatusUnprocessableEntity,
|
|
)
|
|
|
|
// ensureVPPClientUser returns the Fleet-generated clientUserId for the host's
|
|
// Managed Apple ID at the given VPP token (location), creating the Apple-side
|
|
// VPP user via Apple's synchronous v1 registerVPPUserSrv endpoint on first
|
|
// call. Idempotent: subsequent calls return the cached clientUserId from
|
|
// vpp_client_users.
|
|
//
|
|
// Used by the user-scoped Associate Assets path for hosts enrolled via
|
|
// Account-Driven User Enrollment (BYOD).
|
|
func (svc *Service) ensureVPPClientUser(ctx context.Context, host *fleet.Host, token *fleet.VPPTokenDB) (string, error) {
|
|
if host == nil {
|
|
return "", ctxerr.New(ctx, "ensureVPPClientUser: nil host")
|
|
}
|
|
if token == nil {
|
|
return "", ctxerr.New(ctx, "ensureVPPClientUser: nil token")
|
|
}
|
|
|
|
managedAppleID, err := svc.ds.GetHostManagedAppleID(ctx, host.ID)
|
|
if err != nil {
|
|
return "", ctxerr.Wrapf(ctx, err, "looking up managed apple id for host %d", host.ID)
|
|
}
|
|
if managedAppleID == "" {
|
|
return "", errMissingManagedAppleID
|
|
}
|
|
|
|
// Cache hit on (vpp_token_id, managed_apple_id): a previous successful call
|
|
// already registered this user with Apple.
|
|
existing, err := svc.ds.GetVPPClientUser(ctx, token.ID, managedAppleID)
|
|
if err != nil && !fleet.IsNotFound(err) {
|
|
return "", ctxerr.Wrapf(ctx, err, "looking up vpp client user for token %d managed_apple_id %q", token.ID, managedAppleID)
|
|
}
|
|
if existing != nil && existing.Status == fleet.VPPClientUserStatusRegistered {
|
|
return existing.ClientUserID, nil
|
|
}
|
|
|
|
return svc.registerVPPClientUser(ctx, token.ID, managedAppleID, token.Token)
|
|
}
|
|
|
|
// registerVPPClientUser unconditionally registers a new VPP user via Apple's
|
|
// synchronous v1 endpoint and upserts the (vpp_token_id, managed_apple_id)
|
|
// row with the freshly-generated clientUserId, overwriting any prior cache
|
|
// entry. Called by ensureVPPClientUser on its first-call / cache-miss branch.
|
|
func (svc *Service) registerVPPClientUser(ctx context.Context, tokenID uint, managedAppleID, token string) (string, error) {
|
|
clientUserID := uuid.NewString()
|
|
|
|
// v1 registerVPPUserSrv is synchronous — a successful response means the
|
|
// user is registered and ready to receive license associations.
|
|
appleUserID, err := vpp.RegisterUser(ctx, token, clientUserID, managedAppleID)
|
|
if err != nil {
|
|
return "", ctxerr.Wrapf(ctx, err, "registering vpp user for managed apple id %q", managedAppleID)
|
|
}
|
|
|
|
row := &fleet.VPPClientUser{
|
|
VPPTokenID: tokenID,
|
|
ManagedAppleID: managedAppleID,
|
|
ClientUserID: clientUserID,
|
|
Status: fleet.VPPClientUserStatusRegistered,
|
|
}
|
|
if appleUserID != "" {
|
|
row.AppleUserID = &appleUserID
|
|
}
|
|
if err := svc.ds.InsertVPPClientUser(ctx, row); err != nil {
|
|
return "", ctxerr.Wrap(ctx, err, "persisting registered vpp client user")
|
|
}
|
|
return clientUserID, nil
|
|
}
|