[MM-51554] Make LDAP sync job more resilient against errors (#25496)

server/channels/app/syncables.go

@@ -8,6 +8,7 @@ import (
 	"net/http"
 	"strings"
 
+	"github.com/hashicorp/go-multierror"
 	"github.com/mattermost/mattermost/server/public/model"
 	"github.com/mattermost/mattermost/server/public/shared/mlog"
 	"github.com/mattermost/mattermost/server/public/shared/request"
@@ -18,67 +19,68 @@ import (
 // only that channel's members are created. If channelID is nil all channel memberships are created.
 // If includeRemovedMembers is true, then channel members who left or were removed from the channel will
 // be re-added; otherwise, they will not be re-added.
-func (a *App) createDefaultChannelMemberships(c request.CTX, params model.CreateDefaultMembershipParams) error {
+func (a *App) createDefaultChannelMemberships(rctx request.CTX, params model.CreateDefaultMembershipParams) error {
 	channelMembers, appErr := a.ChannelMembersToAdd(params.Since, params.ScopedChannelID, params.ReAddRemovedMembers)
 	if appErr != nil {
 		return appErr
 	}
 
+	var multiErr *multierror.Error
 	for _, userChannel := range channelMembers {
 		if params.ScopedUserID != nil && *params.ScopedUserID != userChannel.UserID {
 			continue
 		}
-		channel, err := a.GetChannel(c, userChannel.ChannelID)
+
+		logger := rctx.Logger().With(
+			mlog.String("user_id", userChannel.UserID),
+			mlog.String("channel_id", userChannel.ChannelID),
+		)
+
+		channel, err := a.GetChannel(rctx, userChannel.ChannelID)
 		if err != nil {
-			return err
+			multiErr = multierror.Append(multiErr, fmt.Errorf("failed to get channel for default channel membership: %w", err))
+			continue
 		}
 
-		tmem, err := a.GetTeamMember(c, channel.TeamId, userChannel.UserID)
+		tmem, err := a.GetTeamMember(rctx, channel.TeamId, userChannel.UserID)
 		if err != nil && err.Id != "app.team.get_member.missing.app_error" {
-			return err
+			multiErr = multierror.Append(multiErr, fmt.Errorf("failed to get member for default channel membership: %w", err))
+			continue
 		}
 
 		// First add user to team
 		if tmem == nil {
-			_, err = a.AddTeamMember(c, channel.TeamId, userChannel.UserID)
+			_, err = a.AddTeamMember(rctx, channel.TeamId, userChannel.UserID)
 			if err != nil {
 				if err.Id == "api.team.join_user_to_team.allowed_domains.app_error" {
-					c.Logger().Info("User not added to channel - the domain associated with the user is not in the list of allowed team domains",
-						mlog.String("user_id", userChannel.UserID),
-						mlog.String("channel_id", userChannel.ChannelID),
+					logger.Info(
+						"User not added to channel - the domain associated with the user is not in the list of allowed team domains",
 						mlog.String("team_id", channel.TeamId),
 					)
-					continue
+				} else {
+					multiErr = multierror.Append(multiErr, fmt.Errorf("failed to add team member for default channel membership: %w", err))
 				}
-				return err
+				continue
 			}
-			c.Logger().Info("added teammember",
-				mlog.String("user_id", userChannel.UserID),
-				mlog.String("team_id", channel.TeamId),
-			)
+			logger.Info("Added channel member for default channel membership")
 		}
 
-		_, err = a.AddChannelMember(c, userChannel.UserID, channel, ChannelMemberOpts{
+		_, err = a.AddChannelMember(rctx, userChannel.UserID, channel, ChannelMemberOpts{
 			SkipTeamMemberIntegrityCheck: true,
 		})
 		if err != nil {
 			if err.Id == "api.channel.add_user.to.channel.failed.deleted.app_error" {
-				c.Logger().Info("Not adding user to channel because they have already left the team",
-					mlog.String("user_id", userChannel.UserID),
-					mlog.String("channel_id", userChannel.ChannelID),
-				)
+				logger.Info("Not adding user to channel because they have already left the team")
 			} else {
-				return err
+				multiErr = multierror.Append(multiErr, fmt.Errorf("failed to add channel member for default channel membership: %w", err))
 			}
+			continue
 		}
 
-		c.Logger().Info("added channelmember",
-			mlog.String("user_id", userChannel.UserID),
-			mlog.String("channel_id", userChannel.ChannelID),
-		)
+		logger.Info("Added channel member for default channel membership")
 	}
 
-	return nil
+	return multiErr.ErrorOrNil()
 }
 
 // createDefaultTeamMemberships adds users to teams based on their group memberships and how those groups are
@@ -86,48 +88,50 @@ func (a *App) createDefaultChannelMemberships(c request.CTX, params model.Create
 // only that team's members are created. If teamID is nil all team memberships are created.
 // If includeRemovedMembers is true, then team members who left or were removed from the team will
 // be re-added; otherwise, they will not be re-added.
-func (a *App) createDefaultTeamMemberships(c request.CTX, params model.CreateDefaultMembershipParams) error {
+func (a *App) createDefaultTeamMemberships(rctx request.CTX, params model.CreateDefaultMembershipParams) error {
 	teamMembers, appErr := a.TeamMembersToAdd(params.Since, params.ScopedTeamID, params.ReAddRemovedMembers)
 	if appErr != nil {
 		return appErr
 	}
 
+	var multiErr *multierror.Error
 	for _, userTeam := range teamMembers {
 		if params.ScopedUserID != nil && *params.ScopedUserID != userTeam.UserID {
 			continue
 		}
-		_, err := a.AddTeamMember(c, userTeam.TeamID, userTeam.UserID)
-		if err != nil {
-			if err.Id == "api.team.join_user_to_team.allowed_domains.app_error" {
-				c.Logger().Info("User not added to team - the domain associated with the user is not in the list of allowed team domains",
-					mlog.String("user_id", userTeam.UserID),
-					mlog.String("team_id", userTeam.TeamID),
-				)
-				continue
-			}
-			return err
-		}
 
-		c.Logger().Info("added teammember",
+		logger := rctx.Logger().With(
 			mlog.String("user_id", userTeam.UserID),
 			mlog.String("team_id", userTeam.TeamID),
 		)
+
+		_, err := a.AddTeamMember(rctx, userTeam.TeamID, userTeam.UserID)
+		if err != nil {
+			if err.Id == "api.team.join_user_to_team.allowed_domains.app_error" {
+				logger.Info("User not added to team - the domain associated with the user is not in the list of allowed team domains")
+			} else {
+				multiErr = multierror.Append(multiErr, fmt.Errorf("failed to add team member for default team membership: %w", err))
+			}
+			continue
+		}
+
+		logger.Info("Added team member for default team membership")
 	}
 
-	return nil
+	return multiErr.ErrorOrNil()
 }
 
 // CreateDefaultMemberships adds users to teams and channels based on their group memberships and how those groups
 // are configured to sync with teams and channels for group members on or after the given timestamp.
 // If includeRemovedMembers is true, then members who left or were removed from a team/channel will
 // be re-added; otherwise, they will not be re-added.
-func (a *App) CreateDefaultMemberships(c request.CTX, params model.CreateDefaultMembershipParams) error {
-	err := a.createDefaultTeamMemberships(c, params)
+func (a *App) CreateDefaultMemberships(rctx request.CTX, params model.CreateDefaultMembershipParams) error {
+	err := a.createDefaultTeamMemberships(rctx, params)
 	if err != nil {
 		return err
 	}
 
-	err = a.createDefaultChannelMemberships(c, params)
+	err = a.createDefaultChannelMemberships(rctx, params)
 	if err != nil {
 		return err
 	}
@@ -137,13 +141,13 @@ func (a *App) CreateDefaultMemberships(c request.CTX, params model.CreateDefault
 
 // DeleteGroupConstrainedMemberships deletes team and channel memberships of users who aren't members of the allowed
 // groups of all group-constrained teams and channels.
-func (a *App) DeleteGroupConstrainedMemberships(c request.CTX) error {
-	err := a.deleteGroupConstrainedChannelMemberships(c, nil)
+func (a *App) DeleteGroupConstrainedMemberships(rctx request.CTX) error {
+	err := a.deleteGroupConstrainedChannelMemberships(rctx, nil)
 	if err != nil {
 		return err
 	}
 
-	err = a.deleteGroupConstrainedTeamMemberships(c, nil)
+	err = a.deleteGroupConstrainedTeamMemberships(rctx, nil)
 	if err != nil {
 		return err
 	}
@@ -154,66 +158,75 @@ func (a *App) DeleteGroupConstrainedMemberships(c request.CTX) error {
 // deleteGroupConstrainedTeamMemberships deletes team memberships of users who aren't members of the allowed
 // groups of the given group-constrained team. If a teamID is given then the procedure is scoped to the given team,
 // if teamID is nil then the procedure affects all teams.
-func (a *App) deleteGroupConstrainedTeamMemberships(c request.CTX, teamID *string) error {
+func (a *App) deleteGroupConstrainedTeamMemberships(rctx request.CTX, teamID *string) error {
 	teamMembers, appErr := a.TeamMembersToRemove(teamID)
 	if appErr != nil {
 		return appErr
 	}
 
+	var multiErr *multierror.Error
 	for _, userTeam := range teamMembers {
-		err := a.RemoveUserFromTeam(c, userTeam.TeamId, userTeam.UserId, "")
-		if err != nil {
-			return err
-		}
-
-		c.Logger().Info("removed teammember",
+		logger := rctx.Logger().With(
 			mlog.String("user_id", userTeam.UserId),
 			mlog.String("team_id", userTeam.TeamId),
 		)
+
+		err := a.RemoveUserFromTeam(rctx, userTeam.TeamId, userTeam.UserId, "")
+		if err != nil {
+			multiErr = multierror.Append(multiErr, fmt.Errorf("failed to remove team member for default team membership: %w", err))
+			continue
+		}
+
+		logger.Info("Removed team member for group constrained team membership")
 	}
 
-	return nil
+	return multiErr.ErrorOrNil()
 }
 
 // deleteGroupConstrainedChannelMemberships deletes channel memberships of users who aren't members of the allowed
 // groups of the given group-constrained channel. If a channelID is given then the procedure is scoped to the given team,
 // if channelID is nil then the procedure affects all teams.
-func (a *App) deleteGroupConstrainedChannelMemberships(c request.CTX, channelID *string) error {
+func (a *App) deleteGroupConstrainedChannelMemberships(rctx request.CTX, channelID *string) error {
 	channelMembers, appErr := a.ChannelMembersToRemove(channelID)
 	if appErr != nil {
 		return appErr
 	}
 
+	var multiErr *multierror.Error
 	for _, userChannel := range channelMembers {
-		channel, err := a.GetChannel(c, userChannel.ChannelId)
-		if err != nil {
-			return err
-		}
-
-		err = a.RemoveUserFromChannel(c, userChannel.UserId, "", channel)
-		if err != nil {
-			return err
-		}
-
-		a.Log().Info("removed channelmember",
+		logger := rctx.Logger().With(
 			mlog.String("user_id", userChannel.UserId),
-			mlog.String("channel_id", channel.Id),
+			mlog.String("channel_id", userChannel.ChannelId),
 		)
+
+		channel, err := a.GetChannel(rctx, userChannel.ChannelId)
+		if err != nil {
+			multiErr = multierror.Append(multiErr, fmt.Errorf("failed to get channel for group constrained channel membership: %w", err))
+			continue
+		}
+
+		err = a.RemoveUserFromChannel(rctx, userChannel.UserId, "", channel)
+		if err != nil {
+			multiErr = multierror.Append(multiErr, fmt.Errorf("failed to remove channel member for group constrained channel membership: %w", err))
+			continue
+		}
+
+		logger.Info("Removed channel member for group constrained channel membership")
 	}
 
-	return nil
+	return multiErr.ErrorOrNil()
 }
 
 // SyncSyncableRoles updates the SchemeAdmin field value of the given syncable's members based on the configuration of
 // the member's group memberships and the configuration of those groups to the syncable. This method should only
 // be invoked on group-synced (aka group-constrained) syncables.
-func (a *App) SyncSyncableRoles(syncableID string, syncableType model.GroupSyncableType) *model.AppError {
+func (a *App) SyncSyncableRoles(rctx request.CTX, syncableID string, syncableType model.GroupSyncableType) *model.AppError {
 	permittedAdmins, err := a.Srv().Store().Group().PermittedSyncableAdmins(syncableID, syncableType)
 	if err != nil {
 		return model.NewAppError("SyncSyncableRoles", "app.select_error", nil, "", http.StatusInternalServerError).Wrap(err)
 	}
 
-	a.Log().Info(
+	rctx.Logger().Info(
 		fmt.Sprintf("Permitted admins for %s", syncableType),
 		mlog.String(strings.ToLower(fmt.Sprintf("%s_id", syncableType)), syncableID),
 		mlog.Array("permitted_admins", permittedAdmins),
@@ -239,8 +252,8 @@ func (a *App) SyncSyncableRoles(syncableID string, syncableType model.GroupSynca
 
 // SyncRolesAndMembership updates the SchemeAdmin status and membership of all of the members of the given
 // syncable.
-func (a *App) SyncRolesAndMembership(c request.CTX, syncableID string, syncableType model.GroupSyncableType, includeRemovedMembers bool) {
-	a.SyncSyncableRoles(syncableID, syncableType)
+func (a *App) SyncRolesAndMembership(rctx request.CTX, syncableID string, syncableType model.GroupSyncableType, includeRemovedMembers bool) {
+	a.SyncSyncableRoles(rctx, syncableID, syncableType)
 
 	lastJob, _ := a.Srv().Store().Job().GetNewestJobByStatusAndType(model.JobStatusSuccess, model.JobTypeLdapSync)
 	var since int64
@@ -253,17 +266,25 @@ func (a *App) SyncRolesAndMembership(c request.CTX, syncableID string, syncableT
 	switch syncableType {
 	case model.GroupSyncableTypeTeam:
 		params.ScopedTeamID = &syncableID
-		a.createDefaultTeamMemberships(c, params)
-		a.deleteGroupConstrainedTeamMemberships(c, &syncableID)
+		if err := a.createDefaultTeamMemberships(rctx, params); err != nil {
+			rctx.Logger().Warn("Error creating default team memberships", mlog.Err(err))
+		}
+		if err := a.deleteGroupConstrainedTeamMemberships(rctx, &syncableID); err != nil {
+			rctx.Logger().Warn("Error deleting group constrained team memberships", mlog.Err(err))
+		}
 		if err := a.ClearTeamMembersCache(syncableID); err != nil {
-			c.Logger().Warn("Error clearing team members cache", mlog.Err(err))
+			rctx.Logger().Warn("Error clearing team members cache", mlog.Err(err))
 		}
 	case model.GroupSyncableTypeChannel:
 		params.ScopedChannelID = &syncableID
-		a.createDefaultChannelMemberships(c, params)
-		a.deleteGroupConstrainedChannelMemberships(c, &syncableID)
-		if err := a.ClearChannelMembersCache(c, syncableID); err != nil {
-			c.Logger().Warn("Error clearing channel members cache", mlog.Err(err))
+		if err := a.createDefaultChannelMemberships(rctx, params); err != nil {
+			rctx.Logger().Warn("Error creating default channel memberships", mlog.Err(err))
+		}
+		if err := a.deleteGroupConstrainedChannelMemberships(rctx, &syncableID); err != nil {
+			rctx.Logger().Warn("Error deleting group constrained team memberships", mlog.Err(err))
+		}
+		if err := a.ClearChannelMembersCache(rctx, syncableID); err != nil {
+			rctx.Logger().Warn("Error clearing channel members cache", mlog.Err(err))
 		}
 	}
 }