Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions cmd/cluster-kube-apiserver-operator-tests-ext/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import (
"k8s.io/klog/v2"

"github.com/openshift/cluster-kube-apiserver-operator/pkg/version"
// Import test packages to register Ginkgo tests
_ "github.com/openshift/cluster-kube-apiserver-operator/test/e2e"
)

func main() {
Expand Down
83 changes: 83 additions & 0 deletions test/e2e/bound_sa_token.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
package e2e

import (
"context"

g "github.com/onsi/ginkgo/v2"
"github.com/stretchr/testify/require"

authenticationv1 "k8s.io/api/authentication/v1"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes"

testlibrary "github.com/openshift/cluster-kube-apiserver-operator/test/library"
)

// TestingT is the minimal common interface that both testing.TB and
// ginkgo.GinkgoTInterface implement. This allows test functions to be
// called from both standard Go tests and Ginkgo tests.
type TestingT interface {
Errorf(format string, args ...interface{})
FailNow()
Helper()
}

var _ = g.Describe("[sig-api-machinery] kube-apiserver operator", func() {
g.It("[Operator][Serial] TestTokenRequestAndReview", func() {
testTokenRequestAndReview(g.GinkgoT())
})
})

// testTokenRequestAndReview checks that bound sa tokens are correctly
// configured. A token is requested via the TokenRequest API and
// validated via the TokenReview API.
func testTokenRequestAndReview(t TestingT) {
kubeConfig, err := testlibrary.NewClientConfigForTest()
require.NoError(t, err)
kubeClient, err := kubernetes.NewForConfig(kubeConfig)
require.NoError(t, err)
corev1client := kubeClient.CoreV1()

// Create all test resources in a temp namespace that will be
// removed at the end of the test to avoid requiring explicit
// cleanup.
ns, err := corev1client.Namespaces().Create(context.TODO(), &v1.Namespace{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "e2e-token-request-",
},
}, metav1.CreateOptions{})
require.NoError(t, err)
defer func() {
err := corev1client.Namespaces().Delete(context.TODO(), ns.Name, metav1.DeleteOptions{})
require.NoError(t, err)
}()
Comment on lines +51 to +54
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Avoid require in defer for cleanup.

Using require.NoError (which calls FailNow()) inside a defer can cause issues—if the test already failed, it may produce confusing output, and FailNow() in a defer can behave unexpectedly in some contexts.

Consider logging the error instead or using assert.NoError which doesn't abort:

 	defer func() {
 		err := corev1client.Namespaces().Delete(context.TODO(), ns.Name, metav1.DeleteOptions{})
-		require.NoError(t, err)
+		if err != nil {
+			t.Errorf("failed to delete namespace %s: %v", ns.Name, err)
+		}
 	}()
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
defer func() {
err := corev1client.Namespaces().Delete(context.TODO(), ns.Name, metav1.DeleteOptions{})
require.NoError(t, err)
}()
defer func() {
err := corev1client.Namespaces().Delete(context.TODO(), ns.Name, metav1.DeleteOptions{})
if err != nil {
t.Errorf("failed to delete namespace %s: %v", ns.Name, err)
}
}()
🤖 Prompt for AI Agents
In test/e2e/bound_sa_token.go around lines 51 to 54, the deferred cleanup
currently calls require.NoError which invokes FailNow inside a defer—replace it
so deferred cleanup does not abort the test: capture the delete error and either
call assert.NoError(t, err) (non-fatal) or log it with t.Logf/t.Errorf if
non-nil; alternatively move the namespace deletion into t.Cleanup and use
require.NoError there (since t.Cleanup runs outside a defer), ensuring the
cleanup path never calls FailNow from within a defer.

namespace := ns.Name

sa, err := corev1client.ServiceAccounts(namespace).Create(context.TODO(), &v1.ServiceAccount{
ObjectMeta: metav1.ObjectMeta{
Name: "test-service-account",
},
}, metav1.CreateOptions{})
require.NoError(t, err)

treq, err := corev1client.ServiceAccounts(sa.Namespace).CreateToken(context.TODO(),
sa.Name,
&authenticationv1.TokenRequest{
Spec: authenticationv1.TokenRequestSpec{
// Avoid specifying any audiences so that the token will be
// issued for the default audience of the issuer.
},
},
metav1.CreateOptions{})
require.NoError(t, err)

trev, err := kubeClient.AuthenticationV1().TokenReviews().Create(context.TODO(), &authenticationv1.TokenReview{
Spec: authenticationv1.TokenReviewSpec{
Token: treq.Status.Token,
},
}, metav1.CreateOptions{})
require.NoError(t, err)
require.Empty(t, trev.Status.Error)
require.True(t, trev.Status.Authenticated)
}
55 changes: 5 additions & 50 deletions test/e2e/bound_sa_token_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,11 @@ import (

"github.com/stretchr/testify/require"

authenticationv1 "k8s.io/api/authentication/v1"
corev1 "k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/client-go/kubernetes"

clientcorev1 "k8s.io/client-go/kubernetes/typed/core/v1"

tokenctl "github.com/openshift/cluster-kube-apiserver-operator/pkg/operator/boundsatokensignercontroller"
Expand Down Expand Up @@ -171,52 +169,9 @@ func checkCertConfigMap(t *testing.T, kubeClient *clientcorev1.CoreV1Client, exp
// TestTokenRequestAndReview checks that bound sa tokens are correctly
// configured. A token is requested via the TokenRequest API and
// validated via the TokenReview API.
//
// This test calls the shared testTokenRequestAndReview function which
// can be called from both standard Go tests and Ginkgo tests.
func TestTokenRequestAndReview(t *testing.T) {
kubeConfig, err := testlibrary.NewClientConfigForTest()
require.NoError(t, err)
kubeClient, err := kubernetes.NewForConfig(kubeConfig)
require.NoError(t, err)
corev1client := kubeClient.CoreV1()

// Create all test resources in a temp namespace that will be
// removed at the end of the test to avoid requiring explicit
// cleanup.
ns, err := corev1client.Namespaces().Create(context.TODO(), &v1.Namespace{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "e2e-token-request-",
},
}, metav1.CreateOptions{})
require.NoError(t, err)
defer func() {
err := corev1client.Namespaces().Delete(context.TODO(), ns.Name, metav1.DeleteOptions{})
require.NoError(t, err)
}()
namespace := ns.Name

sa, err := corev1client.ServiceAccounts(namespace).Create(context.TODO(), &v1.ServiceAccount{
ObjectMeta: metav1.ObjectMeta{
Name: "test-service-account",
},
}, metav1.CreateOptions{})
require.NoError(t, err)

treq, err := corev1client.ServiceAccounts(sa.Namespace).CreateToken(context.TODO(),
sa.Name,
&authenticationv1.TokenRequest{
Spec: authenticationv1.TokenRequestSpec{
// Avoid specifying any audiences so that the token will be
// issued for the default audience of the issuer.
},
},
metav1.CreateOptions{})
require.NoError(t, err)

trev, err := kubeClient.AuthenticationV1().TokenReviews().Create(context.TODO(), &authenticationv1.TokenReview{
Spec: authenticationv1.TokenReviewSpec{
Token: treq.Status.Token,
},
}, metav1.CreateOptions{})
require.NoError(t, err)
require.Empty(t, trev.Status.Error)
require.True(t, trev.Status.Authenticated)
testTokenRequestAndReview(t)
}