From b46455ddfe9469ebe390a917348070165bf61750 Mon Sep 17 00:00:00 2001
From: Dan Winship <danwinship@redhat.com>
Date: Mon, 22 Jan 2024 08:23:41 -0500
Subject: [PATCH] Fix to previous EnsureAdminClusterRoleBindingImpl fix

The previous fix changed the behavior of
EnsureAdminClusterRoleBindingImpl under the assumption that the unit
test was correct and the real-world behavior was wrong, but in fact,
the real-world behavior was already correct, and the unit test was
expecting the wrong result because of the difference in behavior
between real and fake clients.
---
 cmd/kubeadm/app/phases/kubeconfig/kubeconfig.go    | 14 ++++++--------
 .../app/phases/kubeconfig/kubeconfig_test.go       |  2 +-
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/cmd/kubeadm/app/phases/kubeconfig/kubeconfig.go b/cmd/kubeadm/app/phases/kubeconfig/kubeconfig.go
index 75215e71e2b..4e4007ca704 100644
--- a/cmd/kubeadm/app/phases/kubeconfig/kubeconfig.go
+++ b/cmd/kubeadm/app/phases/kubeconfig/kubeconfig.go
@@ -610,7 +610,7 @@ func EnsureAdminClusterRoleBindingImpl(ctx context.Context, adminClient, superAd
 
 	var (
 		err, lastError     error
-		crbResult          *rbac.ClusterRoleBinding
+		crbExists          bool
 		clusterRoleBinding = &rbac.ClusterRoleBinding{
 			ObjectMeta: metav1.ObjectMeta{
 				Name: kubeadmconstants.ClusterAdminsGroupAndClusterRoleBinding,
@@ -637,15 +637,11 @@ func EnsureAdminClusterRoleBindingImpl(ctx context.Context, adminClient, superAd
 		retryInterval,
 		retryTimeout,
 		true, func(ctx context.Context) (bool, error) {
-			if crbResult, err = adminClient.RbacV1().ClusterRoleBindings().Create(
+			if _, err := adminClient.RbacV1().ClusterRoleBindings().Create(
 				ctx,
 				clusterRoleBinding,
 				metav1.CreateOptions{},
 			); err != nil {
-				// (Create returns a non-nil object even on error, but the
-				// code after the poll uses `crbResult != nil` to
-				// determine success.)
-				crbResult = nil
 				if apierrors.IsForbidden(err) {
 					// If it encounters a forbidden error this means that the API server was reached
 					// but the CRB is missing - i.e. the admin.conf user does not have permissions
@@ -654,6 +650,7 @@ func EnsureAdminClusterRoleBindingImpl(ctx context.Context, adminClient, superAd
 				} else if apierrors.IsAlreadyExists(err) {
 					// If the CRB exists it means the admin.conf already has the right
 					// permissions; return.
+					crbExists = true
 					return true, nil
 				} else {
 					// Retry on any other error type.
@@ -661,14 +658,15 @@ func EnsureAdminClusterRoleBindingImpl(ctx context.Context, adminClient, superAd
 					return false, nil
 				}
 			}
+			crbExists = true
 			return true, nil
 		})
 	if err != nil {
 		return nil, lastError
 	}
 
-	// The CRB exists; return the admin.conf client.
-	if crbResult != nil {
+	// The CRB was created or already existed; return the admin.conf client.
+	if crbExists {
 		return adminClient, nil
 	}
 
diff --git a/cmd/kubeadm/app/phases/kubeconfig/kubeconfig_test.go b/cmd/kubeadm/app/phases/kubeconfig/kubeconfig_test.go
index bc87cb2c004..1c686df522f 100644
--- a/cmd/kubeadm/app/phases/kubeconfig/kubeconfig_test.go
+++ b/cmd/kubeadm/app/phases/kubeconfig/kubeconfig_test.go
@@ -850,7 +850,7 @@ func TestEnsureAdminClusterRoleBindingImpl(t *testing.T) {
 						schema.GroupResource{}, "name")
 				})
 			},
-			expectedError: true,
+			expectedError: false,
 		},
 		{
 			name: "admin.conf: handle other errors, such as a server timeout",