From 2bd7fec8a70e0dcb201be2de2623edfc0063f601 Mon Sep 17 00:00:00 2001 From: Scott Gress Date: Tue, 2 Jun 2026 14:47:54 -0500 Subject: [PATCH] Handle edge case of adding new fleet + vpp at the same time (#46533) **Related issue:** Resolves #44444 # Details This PR fixes the following edge case when running `fleetctl gitops`: 1. A new fleet is added 2. That fleet is declared as an ABM default fleet and/or a fleet in a VPP token location 3. The _other_ fleets declared as ABM defaults or VPP fleets are _not_ all provided in the GitOps run In that case, the GitOps run would fail with an error that one of the previously-existing fleets could not be found. This PR fixes the bug by making sure that GitOps looks at both the currently-persisted fleets (via the API) and any fleets that are being created in the current run. # 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. ## Testing - [X] Added/updated automated tests - [X] QA'd all new/changed functionality manually - Reproduced the issue on `main` by attempting to create a new fleet _and_ add it both as an ABM default fleet and to the set of VPP token users in a single run, and getting an error about one of the existing fleets not being found - Verified that I was able to complete a gitops run successfully on this branch with a new fleet as a VPP token user and a default ABM fleet ## Summary by CodeRabbit ## Release Notes * **Bug Fixes** * Improved validation for Apple Business Manager and Volume Purchasing Program token team assignments with clearer error messages when referenced teams aren't found in Fleet. * Enhanced team name matching to properly handle Unicode characters, ensuring consistent team identification across GitOps configurations. --- cmd/fleetctl/fleetctl/gitops.go | 48 ++++++- .../integrationtest/gitops/software_test.go | 117 ++++++++++++++++++ 2 files changed, 159 insertions(+), 6 deletions(-) diff --git a/cmd/fleetctl/fleetctl/gitops.go b/cmd/fleetctl/fleetctl/gitops.go index 9a1e37a08b..676aa041f6 100644 --- a/cmd/fleetctl/fleetctl/gitops.go +++ b/cmd/fleetctl/fleetctl/gitops.go @@ -1069,6 +1069,26 @@ func checkABMTeamAssignments(config *spec.GitOps, fleetClient *service.Client) ( return abmTeams, missingTeam, usesLegacyConfig, nil } +// knownTeamNamesForTokenAssignment returns the set of team names that count as +// "existing" when validating deferred ABM/VPP token assignments. A team is known +// if it already exists in Fleet (returned by ListTeams, which by this point +// includes any teams created earlier in this gitops run) or if it was processed +// during this run (teamNames). +func knownTeamNamesForTokenAssignment(teamNames []string, fleetClient *service.Client) (map[string]struct{}, error) { + teams, err := fleetClient.ListTeams("") + if err != nil { + return nil, err + } + known := make(map[string]struct{}, len(teams)+len(teamNames)) + for _, tm := range teams { + known[norm.NFC.String(tm.Name)] = struct{}{} + } + for _, name := range teamNames { + known[norm.NFC.String(name)] = struct{}{} + } + return known, nil +} + func applyABMTokenAssignmentIfNeeded( ctx *cli.Context, teamNames []string, @@ -1086,11 +1106,18 @@ func applyABMTokenAssignmentIfNeeded( return errors.New("using legacy config without any ABM teams defined") } + knownTeams, err := knownTeamNamesForTokenAssignment(teamNames, fleetClient) + if err != nil { + return err + } + var appConfigUpdate map[string]map[string]any if usesLegacyConfig { appleBMDefaultTeam := abmTeamNames[0] - if !slices.Contains(teamNames, appleBMDefaultTeam) { - return fmt.Errorf("apple_bm_default_team team %q not found in team configs", appleBMDefaultTeam) + if !fleet.IsReservedTeamName(appleBMDefaultTeam) { + if _, ok := knownTeams[norm.NFC.String(appleBMDefaultTeam)]; !ok { + return fmt.Errorf("apple_bm_default_team team %q not found in team configs", appleBMDefaultTeam) + } } appConfigUpdate = map[string]map[string]any{ "mdm": { @@ -1099,7 +1126,10 @@ func applyABMTokenAssignmentIfNeeded( } } else { for _, abmTeam := range abmTeamNames { - if !slices.Contains(teamNames, abmTeam) { + if fleet.IsReservedTeamName(abmTeam) { + continue + } + if _, ok := knownTeams[norm.NFC.String(abmTeam)]; !ok { return fmt.Errorf("apple_business_manager team %q not found in team configs", abmTeam) } } @@ -1171,14 +1201,20 @@ func applyVPPTokenAssignmentIfNeeded( flDryRun bool, fleetClient *service.Client, ) error { - var appConfigUpdate map[string]map[string]any + knownTeams, err := knownTeamNamesForTokenAssignment(teamNames, fleetClient) + if err != nil { + return err + } for _, vppTeam := range vppTeamNames { - if !fleet.IsReservedTeamName(vppTeam) && !slices.Contains(teamNames, vppTeam) { + if fleet.IsReservedTeamName(vppTeam) { + continue + } + if _, ok := knownTeams[norm.NFC.String(vppTeam)]; !ok { return fmt.Errorf("volume_purchasing_program team %s not found in team configs", vppTeam) } } - appConfigUpdate = map[string]map[string]any{ + appConfigUpdate := map[string]map[string]any{ "mdm": { "volume_purchasing_program": originalVPPConfig, }, diff --git a/cmd/fleetctl/integrationtest/gitops/software_test.go b/cmd/fleetctl/integrationtest/gitops/software_test.go index b4085c4e4a..fdc1ec9479 100644 --- a/cmd/fleetctl/integrationtest/gitops/software_test.go +++ b/cmd/fleetctl/integrationtest/gitops/software_test.go @@ -884,6 +884,123 @@ software: assert.Contains(t, buf.String(), fmt.Sprintf(fleetctl.ReapplyingTeamForVPPAppsMsg, newTeamName)) } +// TestGitOpsNewTeamVPPSharedWithUnsuppliedExistingTeam covers issue #44444: adding +// a NEW team that shares a VPP token with an EXISTING team must succeed even when +// the existing team's config file is NOT supplied in the same run. Detecting a +// missing (new) team strips the VPP config and re-applies it after teams are +// created; that re-apply must validate referenced teams against teams that exist in +// Fleet, not only against teams whose files were supplied this run. Previously this +// errored with "volume_purchasing_program team not found in team configs". +func TestGitOpsNewTeamVPPSharedWithUnsuppliedExistingTeam(t *testing.T) { + testing_utils.StartAndServeVPPServer(t) + ds, _, savedTeams := testing_utils.SetupFullGitOpsPremiumServer(t) + renewDate := time.Now().Add(24 * time.Hour) + token, err := test.CreateVPPTokenEncoded(renewDate, "fleet", "ca") + require.NoError(t, err) + + existingTeamName := "Existing Team" + newTeamName := "New Team" + + // Pre-populate the existing team so it exists in Fleet, but deliberately do NOT + // supply its config file in the gitops run below. + existingTeam := &fleet.Team{ID: 42, Name: existingTeamName} + savedTeams[existingTeamName] = &existingTeam + + ds.GetLabelSpecsFunc = func(ctx context.Context, filter fleet.TeamFilter) ([]*fleet.LabelSpec, error) { + return nil, nil + } + ds.GetVPPAppsFunc = func(ctx context.Context, teamID *uint) ([]fleet.VPPAppResponse, error) { + return []fleet.VPPAppResponse{}, nil + } + ds.GetABMTokenCountFunc = func(ctx context.Context) (int, error) { return 0, nil } + ds.GetCertificateTemplatesByTeamIDFunc = func(ctx context.Context, teamID uint, options fleet.ListOptions) ([]*fleet.CertificateTemplateResponseSummary, *fleet.PaginationMetadata, error) { + return []*fleet.CertificateTemplateResponseSummary{}, &fleet.PaginationMetadata{}, nil + } + ds.ListCertificateAuthoritiesFunc = func(ctx context.Context) ([]*fleet.CertificateAuthoritySummary, error) { + return nil, nil + } + + vppToken := &fleet.VPPTokenDB{ + ID: 1, OrgName: "Fleet", Location: "Earth", RenewDate: renewDate, + Token: string(token), Teams: nil, CountryCode: "us", + } + tokensByTeams := make(map[uint]*fleet.VPPTokenDB) + ds.UpdateVPPTokenTeamsFunc = func(ctx context.Context, id uint, teams []uint) (*fleet.VPPTokenDB, error) { + for _, teamID := range teams { + tokensByTeams[teamID] = vppToken + } + return vppToken, nil + } + ds.ListVPPTokensFunc = func(ctx context.Context) ([]*fleet.VPPTokenDB, error) { + return []*fleet.VPPTokenDB{vppToken}, nil + } + ds.GetVPPTokenByTeamIDFunc = func(ctx context.Context, teamID *uint) (*fleet.VPPTokenDB, error) { + if teamID == nil { + return vppToken, nil + } + tok, ok := tokensByTeams[*teamID] + if !ok { + return nil, sql.ErrNoRows + } + return tok, nil + } + ds.GetSoftwareCategoryIDsFunc = func(ctx context.Context, names []string) ([]uint, error) { + return []uint{}, nil + } + ds.InsertOrReplaceMDMConfigAssetFunc = func(ctx context.Context, asset fleet.MDMConfigAsset) error { return nil } + ds.HardDeleteMDMConfigAssetFunc = func(ctx context.Context, assetName fleet.MDMAssetName) error { return nil } + ds.TeamLiteFunc = func(ctx context.Context, id uint) (*fleet.TeamLite, error) { return &fleet.TeamLite{}, nil } + + globalCfg := fmt.Sprintf(` +policies: +queries: +agent_options: +controls: +org_settings: + mdm: + volume_purchasing_program: + - location: Earth + fleets: + - %q + - %q + server_settings: + server_url: https://example.com + org_info: + org_name: Fleet + secrets: + - secret: "FLEET_GLOBAL_ENROLL_SECRET" +`, existingTeamName, newTeamName) + + newTeamCfg := fmt.Sprintf(` +name: %q +team_settings: + secrets: + - secret: "new-secret" +agent_options: +controls: +policies: +queries: +software: + app_store_apps: + - app_store_id: "1" +`, newTeamName) + + tmpDir := t.TempDir() + globalFile := filepath.Join(tmpDir, "default.yml") + require.NoError(t, os.WriteFile(globalFile, []byte(globalCfg), 0o600)) + newTeamFile := filepath.Join(tmpDir, "new-team.yml") + require.NoError(t, os.WriteFile(newTeamFile, []byte(newTeamCfg), 0o600)) + + // Only default.yml + the new team's file — the existing team's file is omitted. + buf, err := fleetctltest.RunAppNoChecks([]string{ + "gitops", "-f", globalFile, "-f", newTeamFile, + }) + require.NoError(t, err) + assert.True(t, ds.UpdateVPPTokenTeamsFuncInvoked) + assert.True(t, ds.SetTeamVPPAppsFuncInvoked) + assert.Contains(t, buf.String(), fmt.Sprintf(fleetctl.ReapplyingTeamForVPPAppsMsg, newTeamName)) +} + func TestGitOpsVPP(t *testing.T) { global := func(mdm string) string { return fmt.Sprintf(`