Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
3 changes: 2 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ require (
sigs.k8s.io/kube-storage-version-migrator v0.0.6-0.20230721195810-5c8923c5ff96
)

require github.com/onsi/ginkgo/v2 v2.21.0

require (
cel.dev/expr v0.24.0 // indirect
github.com/NYTimes/gziphandler v1.1.1 // indirect
Expand Down Expand Up @@ -78,7 +80,6 @@ require (
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
github.com/modern-go/reflect2 v1.0.3-0.20250322232337-35a7c28c31ee // indirect
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
github.com/onsi/ginkgo/v2 v2.21.0 // indirect
github.com/onsi/gomega v1.35.1 // indirect
github.com/pkg/errors v0.9.1 // indirect
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
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)
}