From 04ea1ce7d83f32abda962a10ccbcc80b64cb4ada Mon Sep 17 00:00:00 2001 From: Chad Roberts Date: Tue, 1 Aug 2023 18:02:19 -0400 Subject: [PATCH] Fixing status function and using copies of users in workUnit slices --- cleanup/ad-guid-unmigration.yaml | 3 ++- pkg/agent/clean/active_directory.go | 37 +++++++++++++++-------------- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/cleanup/ad-guid-unmigration.yaml b/cleanup/ad-guid-unmigration.yaml index 74e925d71b0..0806aefa656 100644 --- a/cleanup/ad-guid-unmigration.yaml +++ b/cleanup/ad-guid-unmigration.yaml @@ -80,6 +80,7 @@ rules: - authconfigs - clusterroletemplatebindings - projectroletemplatebindings + - tokens - users verbs: - '*' @@ -99,4 +100,4 @@ rules: verbs: - list - get - - delete \ No newline at end of file + - delete diff --git a/pkg/agent/clean/active_directory.go b/pkg/agent/clean/active_directory.go index 205147dc6c9..79ecfb730e8 100644 --- a/pkg/agent/clean/active_directory.go +++ b/pkg/agent/clean/active_directory.go @@ -12,12 +12,13 @@ import ( "bytes" "crypto/x509" "fmt" - corev1 "k8s.io/api/core/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" "os" "strings" "time" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + ldapv3 "github.com/go-ldap/ldap/v3" "github.com/mitchellh/mapstructure" "github.com/pkg/errors" @@ -290,10 +291,10 @@ func ListAdUsers(clientConfig *restclient.Config) error { } defer lConn.Close() - //err = updateMigrationStatus(sc, "ad-guid-migration-status", "Running") - //if err != nil { - // return fmt.Errorf("unable to update migration status configmap: %v", err) - //} + err = updateMigrationStatus(sc, "ad-guid-migration-status", "Running") + if err != nil { + return fmt.Errorf("unable to update migration status configmap: %v", err) + } users, err := sc.Management.Users("").List(metav1.ListOptions{}) if err != nil { @@ -379,10 +380,10 @@ func ListAdUsers(clientConfig *restclient.Config) error { } } - //err = updateMigrationStatus(sc, "ad-guid-migration-status", "Finished") - //if err != nil { - // return fmt.Errorf("unable to update migration status configmap: %v", err) - //} + err = updateMigrationStatus(sc, "ad-guid-migration-status", "Finished") + if err != nil { + return fmt.Errorf("unable to update migration status configmap: %v", err) + } return nil } @@ -434,7 +435,7 @@ func identifyMigrationWorkUnits(users *v3.UserList, lConn *ldapv3.Conn, adConfig } // If our LDAP connection has gone sour, we still need to log this user for reporting if ldapPermanentlyFailed { - skippedUsers = append(skippedUsers, skippedUserWorkUnit{guid: guid, originalUser: &user}) + skippedUsers = append(skippedUsers, skippedUserWorkUnit{guid: guid, originalUser: user.DeepCopy()}) } else { // Check for guid-based duplicates here. If we find one, we don't need to perform an other LDAP lookup. if i, exists := knownGuidWorkUnits[guid]; exists { @@ -443,9 +444,9 @@ func identifyMigrationWorkUnits(users *v3.UserList, lConn *ldapv3.Conn, adConfig // Make sure the oldest duplicate user is selected as the original if usersToMigrate[i].originalUser.CreationTimestamp.Time.After(user.CreationTimestamp.Time) { usersToMigrate[i].duplicateUsers = append(usersToMigrate[i].duplicateUsers, usersToMigrate[i].originalUser) - usersToMigrate[i].originalUser = &user + usersToMigrate[i].originalUser = user.DeepCopy() } else { - usersToMigrate[i].duplicateUsers = append(usersToMigrate[i].duplicateUsers, &user) + usersToMigrate[i].duplicateUsers = append(usersToMigrate[i].duplicateUsers, user.DeepCopy()) } continue } @@ -453,24 +454,24 @@ func identifyMigrationWorkUnits(users *v3.UserList, lConn *ldapv3.Conn, adConfig logrus.Debugf("[%v] User %v is GUID-based (%v) and a duplicate of %v which is known to be missing", listAdUsersOperation, user.Name, guid, missingUsers[i].originalUser.Name) // We're less picky about the age of the oldest user here, because we aren't going to deduplicate these - missingUsers[i].duplicateUsers = append(missingUsers[i].duplicateUsers, &user) + missingUsers[i].duplicateUsers = append(missingUsers[i].duplicateUsers, user.DeepCopy()) continue } dn, err := findDistinguishedNameWithRetries(guid, lConn, adConfig) if errors.Is(err, LdapConnectionPermanentlyFailed{}) { logrus.Warnf("[%v] LDAP connection has permanently failed! Will proceed to migrate the users we were able to identify up to this point.", listAdUsersOperation) - skippedUsers = append(skippedUsers, skippedUserWorkUnit{guid: guid, originalUser: &user}) + skippedUsers = append(skippedUsers, skippedUserWorkUnit{guid: guid, originalUser: user.DeepCopy()}) ldapPermanentlyFailed = true } else if errors.Is(err, LdapErrorNotFound{}) { logrus.Debugf("[%v] User %v is GUID-based (%v) and the Active Directory server doesn't know about it. Marking it as missing!", listAdUsersOperation, user.Name, guid) knownGuidMissingUnits[guid] = len(missingUsers) - missingUsers = append(missingUsers, missingUserWorkUnit{guid: guid, originalUser: &user}) + missingUsers = append(missingUsers, missingUserWorkUnit{guid: guid, originalUser: user.DeepCopy()}) } else { logrus.Debugf("[%v] User %v is GUID-based (%v) and the Active Directory server knows it by the Distinguished Name '%v'", listAdUsersOperation, user.Name, guid, dn) knownGuidWorkUnits[guid] = len(usersToMigrate) knownDnWorkUnits[dn] = len(usersToMigrate) var emptyDuplicateList []*v3.User - usersToMigrate = append(usersToMigrate, migrateUserWorkUnit{guid: guid, distinguishedName: dn, originalUser: &user, duplicateUsers: emptyDuplicateList}) + usersToMigrate = append(usersToMigrate, migrateUserWorkUnit{guid: guid, distinguishedName: dn, originalUser: user.DeepCopy(), duplicateUsers: emptyDuplicateList}) } } } @@ -696,5 +697,5 @@ func updateMigrationStatus(sc *config.ScaledContext, status string, value string } } - return err + return nil }