Merge pull request #13 from crobby/migrationreview2

More updates based on review comments
rancher · Aug 9, 2023 · cb98c12 · cb98c12
2 parents e17d56f + 4d2f735
commit cb98c12
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 9 deletions.
diff --git a/pkg/agent/clean/active_directory.go b/pkg/agent/clean/active_directory.go
@@ -285,10 +285,11 @@ func prepareClientContexts(clientConfig *restclient.Config) (*config.ScaledConte
 // UnmigrateAdGUIDUsersOnce will ensure that the migration script will run only once.  cycle through all users, ctrb, ptrb, tokens and migrate them to an
 // appropriate DN-based PrincipalID.
 func UnmigrateAdGUIDUsersOnce(sc *config.ScaledContext) error {
-	migrationConfigMap, _ := sc.Core.ConfigMaps("cattle-system").GetNamespaced("cattle-system", "ad-guid-migration", metav1.GetOptions{})
+	migrationConfigMap, _ := sc.Core.ConfigMaps(activedirectory.StatusConfigMapNamespace).GetNamespaced(activedirectory.StatusConfigMapNamespace, activedirectory.StatusConfigMapName, metav1.GetOptions{})
 	if migrationConfigMap != nil {
-		migrationStatus := migrationConfigMap.Data["ad-guid-migration-status"]
-		if migrationStatus == activedirectory.StatusMigrationFinished {
+		migrationStatus := migrationConfigMap.Data[activedirectory.StatusMigrationField]
+		switch migrationStatus {
+		case activedirectory.StatusMigrationFinished:
 			logrus.Debugf("[%v] ad-guid migration has already been completed, refusing to run again at startup", migrateAdUserOperation)
 			return nil
 		}
@@ -311,6 +312,16 @@ func UnmigrateAdGUIDUsers(clientConfig *restclient.Config, dryRun bool, deleteMi
 		return err
 	}
 
+	migrationConfigMap, _ := sc.Core.ConfigMaps(activedirectory.StatusConfigMapNamespace).GetNamespaced(activedirectory.StatusConfigMapNamespace, activedirectory.StatusConfigMapName, metav1.GetOptions{})
+	if migrationConfigMap != nil {
+		migrationStatus := migrationConfigMap.Data[activedirectory.StatusMigrationField]
+		switch migrationStatus {
+		case activedirectory.StatusMigrationRunning:
+			logrus.Infof("[%v] ad-guid migration is currently running, refusing to run again concurrently", migrateAdUserOperation)
+			return nil
+		}
+	}
+
 	// We'll share this lConn for all lookups to hopefully speed things along
 	lConn, err := ldapConnection(adConfig)
 	if err != nil {

diff --git a/pkg/auth/providers/activedirectory/activedirectory_provider.go b/pkg/auth/providers/activedirectory/activedirectory_provider.go
@@ -6,6 +6,8 @@ import (
 	"fmt"
 	"strings"
 
+	"github.com/rancher/norman/httperror"
+
 	"github.com/mitchellh/mapstructure"
 	"github.com/pkg/errors"
 	"github.com/rancher/norman/types"
@@ -92,7 +94,7 @@ func (p *adProvider) AuthenticateUser(ctx context.Context, input interface{}) (v
 	} else {
 		migrationStatus := migrationConfigMap.Data[StatusMigrationField]
 		if migrationStatus == StatusMigrationRunning {
-			return v3.Principal{}, nil, "Unable to perform login while migration is running", LoginDisabledError{}
+			return v3.Principal{}, nil, "", httperror.WrapAPIError(err, httperror.ClusterUnavailable, StatusLoginDisabled)
 		}
 	}
 
@@ -275,5 +277,5 @@ func (p *adProvider) IsDisabledProvider() (bool, error) {
 
 // IsStatusLoginDisabledError will return true when the login is disabled due to a migration in process.
 func IsStatusLoginDisabledError(err error) bool {
-	return err.Error() == StatusLoginDisabled
+	return errors.Is(err, LoginDisabledError{})
 }
diff --git a/pkg/auth/providers/publicapi/login.go b/pkg/auth/providers/publicapi/login.go
@@ -76,9 +76,6 @@ func (h *loginHandler) login(actionName string, action *types.Action, request *t
 		if httperror.IsAPIError(err) {
 			return err
 		}
-		if activedirectory.IsStatusLoginDisabledError(err) {
-			return httperror.WrapAPIError(err, httperror.ClusterUnavailable, activedirectory.StatusLoginDisabled)
-		}
 		return httperror.WrapAPIError(err, httperror.ServerError, "Server error while authenticating")
 	}
 

diff --git a/pkg/multiclustermanager/app.go b/pkg/multiclustermanager/app.go
@@ -215,7 +215,7 @@ func (m *mcm) Start(ctx context.Context) error {
 			return errors.Wrap(err, "failed to telemetry")
 		}
 
-		clean.UnmigrateAdGUIDUsersOnce(m.ScaledContext)
+		go clean.UnmigrateAdGUIDUsersOnce(m.ScaledContext)
 		tokens.StartPurgeDaemon(ctx, management)
 		providerrefresh.StartRefreshDaemon(ctx, m.ScaledContext, management)
 		managementdata.CleanupOrphanedSystemUsers(ctx, management)