From ef652f2b96db726f249d58456e8908a9a7559b19 Mon Sep 17 00:00:00 2001 From: Martin Angers Date: Mon, 8 Apr 2024 16:33:10 -0400 Subject: [PATCH] Test batch-set with declarations --- server/datastore/mysql/apple_mdm.go | 48 ++++++++++++-------------- server/service/integration_mdm_test.go | 33 ++++++++++++++---- 2 files changed, 49 insertions(+), 32 deletions(-) diff --git a/server/datastore/mysql/apple_mdm.go b/server/datastore/mysql/apple_mdm.go index 30812cab03..e9e9699b33 100644 --- a/server/datastore/mysql/apple_mdm.go +++ b/server/datastore/mysql/apple_mdm.go @@ -3419,6 +3419,7 @@ ON DUPLICATE KEY UPDATE uploaded_at = IF(checksum = VALUES(checksum) AND name = VALUES(name), uploaded_at, CURRENT_TIMESTAMP()), checksum = VALUES(checksum), name = VALUES(name), + identifier = VALUES(identifier), raw_json = VALUES(raw_json) ` @@ -3428,18 +3429,18 @@ DELETE FROM WHERE team_id = ? AND %s ` - andIdentNotInList := "identifier NOT IN (?)" // added to fmtDeleteStmt if needed + andNameNotInList := "name NOT IN (?)" // added to fmtDeleteStmt if needed const loadExistingDecls = ` SELECT - identifier, + name, declaration_uuid, raw_json FROM mdm_apple_declarations WHERE team_id = ? AND - identifier IN (?) + name IN (?) ` var declTeamID uint @@ -3447,22 +3448,22 @@ WHERE declTeamID = *tmID } - // build a list of identifiers for the incoming declarations, will keep the + // build a list of names for the incoming declarations, will keep the // existing ones if there's a match and no change - incomingIdents := make([]string, len(incomingDeclarations)) - // at the same time, index the incoming declarations keyed by identifier for ease + incomingNames := make([]string, len(incomingDeclarations)) + // at the same time, index the incoming declarations keyed by name for ease // or processing incomingDecls := make(map[string]*fleet.MDMAppleDeclaration, len(incomingDeclarations)) for i, p := range incomingDeclarations { - incomingIdents[i] = p.Identifier - incomingDecls[p.Identifier] = p + incomingNames[i] = p.Name + incomingDecls[p.Name] = p } var existingDecls []*fleet.MDMAppleDeclaration - if len(incomingIdents) > 0 { - // load existing declarations that match the incoming declarations by identifiers - stmt, args, err := sqlx.In(loadExistingDecls, declTeamID, incomingIdents) + if len(incomingNames) > 0 { + // load existing declarations that match the incoming declarations by names + stmt, args, err := sqlx.In(loadExistingDecls, declTeamID, incomingNames) if err != nil || strings.HasPrefix(ds.testBatchSetMDMAppleProfilesErr, "inselect") { // TODO(JVE): do we need to create similar errors for testing decls? if err == nil { err = errors.New(ds.testBatchSetMDMAppleProfilesErr) @@ -3478,28 +3479,23 @@ WHERE } // figure out if we need to delete any declarations - keepIdents := make([]string, 0, len(incomingIdents)) + keepNames := make([]string, 0, len(incomingNames)) for _, p := range existingDecls { - if newP := incomingDecls[p.Identifier]; newP != nil { - keepIdents = append(keepIdents, p.Identifier) + if newP := incomingDecls[p.Name]; newP != nil { + keepNames = append(keepNames, p.Name) } } + keepNames = append(keepNames, fleetmdm.ListFleetReservedMacOSDeclarationNames()...) var delArgs []any var delStmt string - if len(keepIdents) == 0 { + if len(keepNames) == 0 { // delete all declarations for the team delStmt = fmt.Sprintf(fmtDeleteStmt, "TRUE") delArgs = []any{declTeamID} } else { // delete the obsolete declarations (all those that are not in keepIdents) - stmt, args, err := sqlx.In(fmt.Sprintf(fmtDeleteStmt, andIdentNotInList), declTeamID, append(keepIdents, fleetmdm.ListFleetReservedMacOSDeclarationNames()...)) - // if err != nil || strings.HasPrefix(ds.testBatchSetMDMAppleProfilesErr, "inselect") { // TODO(JVE): do we need to create similar errors for testing decls? - // if err == nil { - // err = errors.New(ds.testBatchSetMDMAppleProfilesErr) - // } - // return nil, ctxerr.Wrap(ctx, err, "build query to load existing declarations") - // } + stmt, args, err := sqlx.In(fmt.Sprintf(fmtDeleteStmt, andNameNotInList), declTeamID, keepNames) if err != nil { return nil, ctxerr.Wrap(ctx, err, "build query to delete obsolete profiles") } @@ -3532,7 +3528,7 @@ WHERE } incomingLabels := []fleet.ConfigurationProfileLabel{} - if len(incomingIdents) > 0 { + if len(incomingNames) > 0 { var newlyInsertedDecls []*fleet.MDMAppleDeclaration // load current declarations (again) that match the incoming declarations by name to grab their uuids // this is an easy way to grab the identifiers for both the existing declarations and the new ones we generated. @@ -3541,7 +3537,7 @@ WHERE // information without this extra request in the previous DB // calls. Due to time constraints, I'm leaving that // optimization for a later iteration. - stmt, args, err := sqlx.In(loadExistingDecls, declTeamID, incomingIdents) + stmt, args, err := sqlx.In(loadExistingDecls, declTeamID, incomingNames) if err != nil { return nil, ctxerr.Wrap(ctx, err, "build query to load newly inserted declarations") } @@ -3550,9 +3546,9 @@ WHERE } for _, newlyInsertedDecl := range newlyInsertedDecls { - incomingDecl, ok := incomingDecls[newlyInsertedDecl.Identifier] + incomingDecl, ok := incomingDecls[newlyInsertedDecl.Name] if !ok { - return nil, ctxerr.Wrapf(ctx, err, "declaration %q is in the database but was not incoming", newlyInsertedDecl.Identifier) + return nil, ctxerr.Wrapf(ctx, err, "declaration %q is in the database but was not incoming", newlyInsertedDecl.Name) } for _, label := range incomingDecl.Labels { diff --git a/server/service/integration_mdm_test.go b/server/service/integration_mdm_test.go index 73553bb47d..cc9731a2ac 100644 --- a/server/service/integration_mdm_test.go +++ b/server/service/integration_mdm_test.go @@ -12334,6 +12334,7 @@ func (s *integrationMDMTestSuite) TestMDMBatchSetProfilesKeepsReservedNames() { } return sqlx.GetContext(ctx, q, &count, `SELECT COUNT(*) FROM mdm_windows_configuration_profiles WHERE team_id = ?`, tid) }) + require.Equal(t, len(names), count) for _, n := range names { s.assertWindowsConfigProfilesByName(teamID, n, true) } @@ -12361,8 +12362,8 @@ func (s *integrationMDMTestSuite) TestMDMBatchSetProfilesKeepsReservedNames() { "grace_period_days": 1 }, "macos_updates": { - "deadline": "2023-12-31", - "minimum_version": "13.3.7" + "deadline": "2023-11-30", + "minimum_version": "13.3.8" } } }`), http.StatusOK, &acResp) @@ -12379,6 +12380,7 @@ func (s *integrationMDMTestSuite) TestMDMBatchSetProfilesKeepsReservedNames() { }) s.Do("POST", "/api/v1/fleet/mdm/profiles/batch", batchSetMDMProfilesRequest{Profiles: testProfiles}, http.StatusNoContent) checkMacProfs(nil, servermdm.ListFleetReservedMacOSProfileNames()...) + checkMacDecls(nil, servermdm.ListFleetReservedMacOSDeclarationNames()...) checkWinProfs(nil, append(servermdm.ListFleetReservedWindowsProfileNames(), "n1")...) // batch set windows and mac profiles doesn't remove the reserved names @@ -12389,15 +12391,27 @@ func (s *integrationMDMTestSuite) TestMDMBatchSetProfilesKeepsReservedNames() { }) s.Do("POST", "/api/v1/fleet/mdm/profiles/batch", batchSetMDMProfilesRequest{Profiles: testProfiles}, http.StatusNoContent) checkMacProfs(nil, append(servermdm.ListFleetReservedMacOSProfileNames(), "n2")...) + checkMacDecls(nil, servermdm.ListFleetReservedMacOSDeclarationNames()...) checkWinProfs(nil, append(servermdm.ListFleetReservedWindowsProfileNames(), "n1")...) - // batch set only mac profiles doesn't remove the reserved names + // batch set only mac profiles and declaration doesn't remove the reserved names + newMacDecl := []byte(fmt.Sprintf(`{ + "Type": "com.apple.configuration.foo", + "Payload": { + "Echo": "f1337" + }, + "Identifier": "%s" +}`, uuid.NewString())) testProfiles = []fleet.MDMProfileBatchPayload{{ Name: "n2", Contents: newMacProfile, + }, { + Name: "n3", + Contents: newMacDecl, }} s.Do("POST", "/api/v1/fleet/mdm/profiles/batch", batchSetMDMProfilesRequest{Profiles: testProfiles}, http.StatusNoContent) checkMacProfs(nil, append(servermdm.ListFleetReservedMacOSProfileNames(), "n2")...) + checkMacDecls(nil, append(servermdm.ListFleetReservedMacOSDeclarationNames(), "n3")...) checkWinProfs(nil, servermdm.ListFleetReservedWindowsProfileNames()...) // create a team @@ -12416,7 +12430,7 @@ func (s *integrationMDMTestSuite) TestMDMBatchSetProfilesKeepsReservedNames() { }, MacOSUpdates: &fleet.MacOSUpdates{ Deadline: optjson.SetString("2023-12-31"), - MinimumVersion: optjson.SetString("13.3.8"), + MinimumVersion: optjson.SetString("13.3.9"), }, }, }, @@ -12427,11 +12441,12 @@ func (s *integrationMDMTestSuite) TestMDMBatchSetProfilesKeepsReservedNames() { require.Equal(t, 4, tmResp.Team.Config.MDM.WindowsUpdates.DeadlineDays.Value) require.Equal(t, 1, tmResp.Team.Config.MDM.WindowsUpdates.GracePeriodDays.Value) require.Equal(t, "2023-12-31", tmResp.Team.Config.MDM.MacOSUpdates.Deadline.Value) - require.Equal(t, "13.3.8", tmResp.Team.Config.MDM.MacOSUpdates.MinimumVersion.Value) + require.Equal(t, "13.3.9", tmResp.Team.Config.MDM.MacOSUpdates.MinimumVersion.Value) require.NoError(t, ReconcileAppleProfiles(ctx, s.ds, s.mdmCommander, s.logger)) checkMacProfs(&tmResp.Team.ID, servermdm.ListFleetReservedMacOSProfileNames()...) + checkMacDecls(&tmResp.Team.ID, servermdm.ListFleetReservedMacOSDeclarationNames()...) checkWinProfs(&tmResp.Team.ID, servermdm.ListFleetReservedWindowsProfileNames()...) // batch set only windows profiles doesn't remove the reserved names @@ -12442,6 +12457,7 @@ func (s *integrationMDMTestSuite) TestMDMBatchSetProfilesKeepsReservedNames() { }) s.Do("POST", "/api/v1/fleet/mdm/profiles/batch", batchSetMDMProfilesRequest{Profiles: testTeamProfiles}, http.StatusNoContent, "team_id", strconv.Itoa(int(tmResp.Team.ID))) checkMacProfs(&tmResp.Team.ID, servermdm.ListFleetReservedMacOSProfileNames()...) + checkMacDecls(&tmResp.Team.ID, servermdm.ListFleetReservedMacOSDeclarationNames()...) checkWinProfs(&tmResp.Team.ID, append(servermdm.ListFleetReservedWindowsProfileNames(), "n1")...) // batch set windows and mac profiles doesn't remove the reserved names @@ -12451,14 +12467,19 @@ func (s *integrationMDMTestSuite) TestMDMBatchSetProfilesKeepsReservedNames() { }) s.Do("POST", "/api/v1/fleet/mdm/profiles/batch", batchSetMDMProfilesRequest{Profiles: testTeamProfiles}, http.StatusNoContent, "team_id", strconv.Itoa(int(tmResp.Team.ID))) checkMacProfs(&tmResp.Team.ID, append(servermdm.ListFleetReservedMacOSProfileNames(), "n2")...) + checkMacDecls(&tmResp.Team.ID, servermdm.ListFleetReservedMacOSDeclarationNames()...) checkWinProfs(&tmResp.Team.ID, append(servermdm.ListFleetReservedWindowsProfileNames(), "n1")...) - // batch set only mac profiles doesn't remove the reserved names + // batch set only mac profiles and declaration doesn't remove the reserved names testTeamProfiles = []fleet.MDMProfileBatchPayload{{ Name: "n2", Contents: newMacProfile, + }, { + Name: "n3", + Contents: newMacDecl, }} s.Do("POST", "/api/v1/fleet/mdm/profiles/batch", batchSetMDMProfilesRequest{Profiles: testTeamProfiles}, http.StatusNoContent, "team_id", strconv.Itoa(int(tmResp.Team.ID))) checkMacProfs(&tmResp.Team.ID, append(servermdm.ListFleetReservedMacOSProfileNames(), "n2")...) + checkMacDecls(&tmResp.Team.ID, append(servermdm.ListFleetReservedMacOSDeclarationNames(), "n3")...) checkWinProfs(&tmResp.Team.ID, servermdm.ListFleetReservedWindowsProfileNames()...) }