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.
This commit is contained in:
Dan Winship 2024-01-22 08:23:41 -05:00
parent a07b1aaa5b
commit b46455ddfe
2 changed files with 7 additions and 9 deletions

View File

@ -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
}

View File

@ -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",