From cfcca6a6ac52ca9c7f694b3f5271e3674dcfe16a Mon Sep 17 00:00:00 2001 From: Magnus Jensen Date: Fri, 5 Jun 2026 11:48:13 +0200 Subject: [PATCH] Handle not found bootstrap package in GitOps flows (#46802) **Related issue:** Resolves #45441 The issue is when hitting the `svc.DeleteMDMAppleBootstrapPackage` via the API/UI, it only clears the row in `mdm_apple_bootstrap_packages`. However when GitOps runs the next time, it compares the old team config, which has a stale `macos_setup.bootstrap_package` config value. Which forces it to call the same Delete method again. This PR adds the defensive approach to gracefully handle a not found bootstrap package when GitOps wants to delete it. The reason the second run works, is that we only attempt to delete the bootstrap package after we called SaveTeam with the new empty `bootstrap_package` value. So next run sees it as empty and avoid calling the Delete method. _One question is if we want to add a more active approach on the delete service method, which also handles updating the team config clearing out this value? That would have prevented the cause, I think either keeping only this layer, or doing both solutions is a good approach._ # Checklist for submitter If some of the following don't apply, delete the relevant line. - [x] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. - [x] 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. - [x] Timeouts are implemented and retries are limited to avoid infinite loops - [x] If paths of existing endpoints are modified without backwards compatibility, checked the frontend/CLI for any necessary changes ## Testing - [x] Added/updated automated tests - [x] QA'd all new/changed functionality manually ## Summary by CodeRabbit * **Bug Fixes** * GitOps automation no longer fails on its first run after a bootstrap package is deleted via the UI. * Clearing a macOS bootstrap package (team or app config) now succeeds even if the underlying package record is already missing. --- ...41-bootstrap-package-not-found-not-handled | 1 + ee/server/service/teams.go | 6 +- ee/server/service/teams_test.go | 77 +++++++++++++++++++ server/datastore/mysql/apple_mdm.go | 4 +- server/platform/mysql/errors.go | 9 +++ server/service/appconfig.go | 6 +- server/service/appconfig_test.go | 44 +++++++++++ 7 files changed, 143 insertions(+), 4 deletions(-) create mode 100644 changes/45441-bootstrap-package-not-found-not-handled diff --git a/changes/45441-bootstrap-package-not-found-not-handled b/changes/45441-bootstrap-package-not-found-not-handled new file mode 100644 index 0000000000..b94386afd2 --- /dev/null +++ b/changes/45441-bootstrap-package-not-found-not-handled @@ -0,0 +1 @@ +- Fixed an issue where GitOps would fail on the first run after deleting the bootstrap package in the UI. \ No newline at end of file diff --git a/ee/server/service/teams.go b/ee/server/service/teams.go index 1003ae01fe..a67dca88c1 100644 --- a/ee/server/service/teams.go +++ b/ee/server/service/teams.go @@ -1978,7 +1978,11 @@ func (svc *Service) editTeamFromSpec( spec.MDM.MacOSSetup.BootstrapPackage.Value == "" && oldMacOSSetup.BootstrapPackage.Value != "" { if err := svc.DeleteMDMAppleBootstrapPackage(ctx, &team.ID, opts.DryRun); err != nil { - return ctxerr.Wrapf(ctx, err, "clear bootstrap package for team %d", team.ID) + // The package may have already been deleted via the GUI while the + // team config JSON still had the stale URL; ignore not-found. + if !fleet.IsNotFound(err) { + return ctxerr.Wrapf(ctx, err, "clear bootstrap package for team %d", team.ID) + } } } diff --git a/ee/server/service/teams_test.go b/ee/server/service/teams_test.go index 7d7d86648a..30276e4612 100644 --- a/ee/server/service/teams_test.go +++ b/ee/server/service/teams_test.go @@ -1235,3 +1235,80 @@ func TestApplyTeamSpecsCustomSettingsWithoutMDMConfigured(t *testing.T) { require.Len(t, (*saved).Config.MDM.AndroidSettings.CustomSettings.Value, 1) }) } + +// TestApplyTeamSpecsClearBootstrapPackageAlreadyDeleted verifies that clearing +// a bootstrap package via GitOps succeeds even when the actual package row has +// already been deleted (e.g. via the GUI), leaving a stale URL in team config. +func TestApplyTeamSpecsClearBootstrapPackageAlreadyDeleted(t *testing.T) { + authorizer, err := authz.NewAuthorizer() + require.NoError(t, err) + adminUser := &fleet.User{ID: 1, GlobalRole: new(fleet.RoleAdmin)} + ctx := test.UserContext(context.Background(), adminUser) + + const teamName = "TestTeam" + const teamID = uint(42) + + ds := new(mock.Store) + ds.AppConfigFunc = func(ctx context.Context) (*fleet.AppConfig, error) { + return &fleet.AppConfig{MDM: fleet.MDM{EnabledAndConfigured: true}}, nil + } + ds.TeamByFilenameFunc = func(ctx context.Context, _ string) (*fleet.Team, error) { + return nil, ¬FoundError{} + } + ds.TeamByNameFunc = func(ctx context.Context, name string) (*fleet.Team, error) { + return &fleet.Team{ + ID: teamID, + Name: name, + Config: fleet.TeamConfig{ + MDM: fleet.TeamMDM{ + MacOSSetup: fleet.MacOSSetup{ + // Stale URL: the DB row is gone but team config still has it. + BootstrapPackage: optjson.SetString("https://example.com/bootstrap.pkg"), + }, + }, + }, + }, nil + } + ds.TeamConflictsWithNameFunc = func(ctx context.Context, name string, excludeID uint) (*fleet.Team, error) { + return nil, nil + } + ds.SaveTeamFunc = func(ctx context.Context, team *fleet.Team) (*fleet.Team, error) { + return team, nil + } + // The bootstrap package row was already deleted via the GUI. + ds.TeamWithExtrasFunc = func(ctx context.Context, tid uint) (*fleet.Team, error) { + return &fleet.Team{ID: tid, Name: teamName}, nil + } + ds.GetMDMAppleBootstrapPackageMetaFunc = func(ctx context.Context, teamID uint) (*fleet.MDMAppleBootstrapPackage, error) { + return nil, &bootstrapNotFoundError{msg: "bootstrap package not found"} + } + + mockSvc := &svcmock.Service{} + mockSvc.NewActivityFunc = func(ctx context.Context, _ *fleet.User, _ fleet.ActivityDetails) error { + return nil + } + + svc := &Service{ + Service: mockSvc, + ds: ds, + config: config.FleetConfig{ + Server: config.ServerConfig{PrivateKey: "something"}, + }, + authz: authorizer, + logger: slog.New(slog.DiscardHandler), + } + + spec := &fleet.TeamSpec{ + Name: teamName, + MDM: fleet.TeamSpecMDM{ + MacOSSetup: fleet.MacOSSetup{ + // Clearing the bootstrap package (Set=true, Value=""). + BootstrapPackage: optjson.SetString(""), + }, + }, + } + + _, err = svc.ApplyTeamSpecs(ctx, []*fleet.TeamSpec{spec}, fleet.ApplyTeamSpecOptions{}) + require.NoError(t, err) + require.True(t, ds.SaveTeamFuncInvoked) +} diff --git a/server/datastore/mysql/apple_mdm.go b/server/datastore/mysql/apple_mdm.go index d8e07da5ec..53d5234e3e 100644 --- a/server/datastore/mysql/apple_mdm.go +++ b/server/datastore/mysql/apple_mdm.go @@ -3451,7 +3451,7 @@ func (ds *Datastore) DeleteMDMAppleBootstrapPackage(ctx context.Context, teamID deleted, _ := res.RowsAffected() if deleted != 1 { - return ctxerr.Wrap(ctx, notFound("BootstrapPackage").WithID(teamID)) + return ctxerr.Wrap(ctx, notFound("BootstrapPackage").WithFleetID(teamID)) } return nil } @@ -3600,7 +3600,7 @@ func (ds *Datastore) GetMDMAppleBootstrapPackageMeta(ctx context.Context, teamID var bp fleet.MDMAppleBootstrapPackage if err := sqlx.GetContext(ctx, ds.reader(ctx), &bp, stmt, teamID); err != nil { if err == sql.ErrNoRows { - return nil, ctxerr.Wrap(ctx, notFound("BootstrapPackage").WithID(teamID)) + return nil, ctxerr.Wrap(ctx, notFound("BootstrapPackage").WithFleetID(teamID)) } return nil, ctxerr.Wrap(ctx, err, "get bootstrap package meta") } diff --git a/server/platform/mysql/errors.go b/server/platform/mysql/errors.go index bf85e297ee..1623cb272e 100644 --- a/server/platform/mysql/errors.go +++ b/server/platform/mysql/errors.go @@ -12,6 +12,7 @@ import ( type NotFoundError struct { ID uint + FleetID uint Name string Message string ResourceType string @@ -30,6 +31,9 @@ func (e *NotFoundError) Error() string { if e.ID != 0 { return fmt.Sprintf("%s %d was not found in the datastore", e.ResourceType, e.ID) } + if e.FleetID != 0 { + return fmt.Sprintf("%s for fleet %d was not found in the datastore", e.ResourceType, e.FleetID) + } if e.Name != "" { return fmt.Sprintf("%s %s was not found in the datastore", e.ResourceType, e.Name) } @@ -44,6 +48,11 @@ func (e *NotFoundError) WithID(id uint) error { return e } +func (e *NotFoundError) WithFleetID(fleetID uint) error { + e.FleetID = fleetID + return e +} + func (e *NotFoundError) WithName(name string) *NotFoundError { e.Name = name return e diff --git a/server/service/appconfig.go b/server/service/appconfig.go index f32200e489..491048f182 100644 --- a/server/service/appconfig.go +++ b/server/service/appconfig.go @@ -1133,7 +1133,11 @@ func (svc *Service) ModifyAppConfig(ctx context.Context, p []byte, applyOpts fle // current service implementation. We have to go through the Enterprise // extensions. if err := svc.EnterpriseOverrides.DeleteMDMAppleBootstrapPackage(ctx, nil, applyOpts.DryRun); err != nil { - return nil, ctxerr.Wrap(ctx, err, "delete Apple bootstrap package") + // The package may have already been deleted via the GUI while the + // appconfig JSON still had the stale URL; ignore not-found. + if !fleet.IsNotFound(err) { + return nil, ctxerr.Wrap(ctx, err, "delete Apple bootstrap package") + } } } diff --git a/server/service/appconfig_test.go b/server/service/appconfig_test.go index 56441fa349..769e62bf4d 100644 --- a/server/service/appconfig_test.go +++ b/server/service/appconfig_test.go @@ -2409,3 +2409,47 @@ func TestDiffStringSlices(t *testing.T) { }) } } + +// TestModifyAppConfigClearBootstrapPackageAlreadyDeleted verifies that clearing +// a bootstrap package via ModifyAppConfig succeeds even when the actual package +// row has already been deleted (e.g. via the GUI), leaving a stale URL in +// appconfig. +func TestModifyAppConfigClearBootstrapPackageAlreadyDeleted(t *testing.T) { + ds := new(mock.Store) + admin := &fleet.User{GlobalRole: new(fleet.RoleAdmin)} + svc, ctx := newTestService(t, ds, nil, nil, &TestServerOpts{License: &fleet.LicenseInfo{Tier: fleet.TierPremium}}) + ctx = viewer.NewContext(ctx, viewer.Viewer{User: admin}) + + dsAppConfig := &fleet.AppConfig{ + OrgInfo: fleet.OrgInfo{OrgName: "Test"}, + ServerSettings: fleet.ServerSettings{ServerURL: "https://example.org"}, + MDM: fleet.MDM{ + MacOSSetup: fleet.MacOSSetup{ + // Stale URL: the DB row is gone but appconfig still has it. + BootstrapPackage: optjson.SetString("https://example.com/bootstrap.pkg"), + }, + }, + } + + ds.AppConfigFunc = func(ctx context.Context) (*fleet.AppConfig, error) { + return dsAppConfig, nil + } + ds.SaveAppConfigFunc = func(ctx context.Context, conf *fleet.AppConfig) error { + *dsAppConfig = *conf + return nil + } + ds.ListABMTokensFunc = func(ctx context.Context) ([]*fleet.ABMToken, error) { + return []*fleet.ABMToken{}, nil + } + ds.ListVPPTokensFunc = func(ctx context.Context) ([]*fleet.VPPTokenDB, error) { + return []*fleet.VPPTokenDB{}, nil + } + // The bootstrap package row was already deleted via the GUI. + ds.GetMDMAppleBootstrapPackageMetaFunc = func(ctx context.Context, teamID uint) (*fleet.MDMAppleBootstrapPackage, error) { + return nil, newNotFoundError() + } + + raw := []byte(`{"mdm":{"macos_setup":{"bootstrap_package":""}}}`) + _, err := svc.ModifyAppConfig(ctx, raw, fleet.ApplySpecOptions{}) + require.NoError(t, err) +}