Handle edge case of adding new fleet + vpp at the same time (#46533)
<!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## 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. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -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,
|
||||
},
|
||||
|
||||
@@ -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 <existing> 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(`
|
||||
|
||||
Reference in New Issue
Block a user