Return error when credentials fail validation.

openshift · Dec 2, 2020 · c743702 · c743702
1 parent 58fe555
commit c743702
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 17 deletions.
diff --git a/pkg/controller/clusterdeployment/clusterdeployment_controller.go b/pkg/controller/clusterdeployment/clusterdeployment_controller.go
@@ -57,8 +57,8 @@ const (
 	defaultRequeueTime = 10 * time.Second
 	maxProvisions      = 3
 
-	platformAuthFailureResason = "PlatformAuthError"
-	platformAuthSuccessReason  = "PlatformAuthWorking"
+	platformAuthFailureReason = "PlatformAuthError"
+	platformAuthSuccessReason = "PlatformAuthSuccess"
 
 	clusterImageSetNotFoundReason = "ClusterImageSetNotFound"
 	clusterImageSetFoundReason    = "ClusterImageSetFound"
@@ -586,21 +586,22 @@ func (r *ReconcileClusterDeployment) reconcile(request reconcile.Request, cd *hi
 	// Sanity check the platform/cloud credentials.
 	validCreds, err := r.validatePlatformCreds(cd, cdLog)
 	if err != nil {
-		cdLog.WithError(err).Error("errored validating platform credentials")
+		cdLog.WithError(err).Error("unable to validate platform credentials")
 		return reconcile.Result{}, err
 	}
 	// Make sure the condition is set properly.
-	changed, err := r.setAuthenticationFailure(cd, validCreds, cdLog)
-	if changed || err != nil {
+	_, err = r.setAuthenticationFailure(cd, validCreds, cdLog)
+	if err != nil {
+		cdLog.WithError(err).Error("unable to update clusterdeployment")
 		return reconcile.Result{}, err
 	}
 
-	// If the platform credentials are no good, do not bother with ClusterProvision objects
+	// If the platform credentials are no good, return error and go into backoff
 	authCondition := controllerutils.FindClusterDeploymentCondition(cd.Status.Conditions, hivev1.AuthenticationFailureClusterDeploymentCondition)
 	if authCondition != nil && authCondition.Status == corev1.ConditionTrue {
-		cdLog.Info("Skipping provision while platform credentials authentication is failing.")
-		// Periodically retry???
-		return reconcile.Result{}, nil
+		authError := errors.New(authCondition.Message)
+		cdLog.WithError(authError).Error("cannot proceed with provision while platform credentials authentication is failing.")
+		return reconcile.Result{}, authError
 	}
 
 	imageSet, err := r.getClusterImageSet(cd, cdLog)
@@ -1293,18 +1294,18 @@ func (r *ReconcileClusterDeployment) setDNSNotReadyCondition(cd *hivev1.ClusterD
 	return r.Status().Update(context.TODO(), cd)
 }
 
-func (r *ReconcileClusterDeployment) setAuthenticationFailure(cd *hivev1.ClusterDeployment, authWorking bool, cdLog log.FieldLogger) (bool, error) {
+func (r *ReconcileClusterDeployment) setAuthenticationFailure(cd *hivev1.ClusterDeployment, authSuccessful bool, cdLog log.FieldLogger) (bool, error) {
 
 	var status corev1.ConditionStatus
 	var reason, message string
 
-	if authWorking {
+	if authSuccessful {
 		status = corev1.ConditionFalse
 		reason = platformAuthSuccessReason
-		message = "Platform credentails passed authentication check"
+		message = "Platform credentials passed authentication check"
 	} else {
 		status = corev1.ConditionTrue
-		reason = platformAuthFailureResason
+		reason = platformAuthFailureReason
 		message = "Platform credentials failed authentication check"
 	}
 

diff --git a/pkg/controller/clusterdeployment/clusterdeployment_controller_test.go b/pkg/controller/clusterdeployment/clusterdeployment_controller_test.go
@@ -1509,6 +1509,7 @@ func TestClusterDeploymentReconcile(t *testing.T) {
 			platformCredentialsValidation: func(client.Client, *hivev1.ClusterDeployment, log.FieldLogger) (bool, error) {
 				return false, nil
 			},
+			expectErr: true,
 			validate: func(c client.Client, t *testing.T) {
 				cd := getCD(c)
 				require.NotNil(t, cd, "could not get ClusterDeployment")
@@ -1525,7 +1526,7 @@ func TestClusterDeploymentReconcile(t *testing.T) {
 						{
 							Status:  corev1.ConditionTrue,
 							Type:    hivev1.AuthenticationFailureClusterDeploymentCondition,
-							Reason:  platformAuthFailureResason,
+							Reason:  platformAuthFailureReason,
 							Message: "Platform credentials failed authentication check",
 						},
 					}
@@ -1535,6 +1536,7 @@ func TestClusterDeploymentReconcile(t *testing.T) {
 			platformCredentialsValidation: func(client.Client, *hivev1.ClusterDeployment, log.FieldLogger) (bool, error) {
 				return false, nil
 			},
+			expectErr: true,
 			validate: func(c client.Client, t *testing.T) {
 				cd := getCD(c)
 				require.NotNil(t, cd, "could not get ClusterDeployment")

diff --git a/pkg/controller/utils/credentials.go b/pkg/controller/utils/credentials.go
@@ -32,7 +32,8 @@ func ValidateCredentialsForClusterDeployment(kubeClient client.Client, cd *hivev
 		}
 		return validateVSphereCredentials(cd.Spec.Platform.VSphere.VCenter,
 			string(secret.Data[constants.UsernameSecretKey]),
-			string(secret.Data[constants.PasswordSecretKey]))
+			string(secret.Data[constants.PasswordSecretKey]),
+			logger)
 	default:
 		// If we have no platform-specific credentials verification
 		// assume the creds are valid.
@@ -41,9 +42,9 @@ func ValidateCredentialsForClusterDeployment(kubeClient client.Client, cd *hivev
 	}
 }
 
-func validateVSphereCredentials(vcenter, username, password string) (bool, error) {
-
+func validateVSphereCredentials(vcenter, username, password string, logger log.FieldLogger) (bool, error) {
 	_, _, err := vsphere.CreateVSphereClients(context.TODO(), vcenter, username, password)
+	logger.WithError(err).Warn("failed to validate VSphere credentials")
 	return err == nil, nil
 }