test: add RBAC e2e tests for create and delete endpoints

Add e2e tests verifying that the management API enforces Kubernetes
RBAC for create and delete alert rule operations. Three user profiles
are tested: unprivileged (expects 403), namespace-scoped (succeeds in
own namespace, denied elsewhere), and cluster-admin (succeeds everywhere).

Also fixes a critical bug in newUserScopedClientsets: when the base
rest.Config uses client certificates (common in CI kubeconfigs),
CopyConfig preserved them. Since Kubernetes authenticates via client
certs when both certs and bearer token are present, user RBAC was
bypassed entirely. Clear CertData/CertFile/KeyData/KeyFile so the
API server authenticates exclusively via the user's bearer token.

Co-authored-by: Cursor <cursoragent@cursor.com>

Author: sradco

internal/managementrouter/router.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/gorilla/mux"
 	"github.com/sirupsen/logrus"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 
 	"github.com/openshift/monitoring-plugin/pkg/k8s"
 	"github.com/openshift/monitoring-plugin/pkg/management"
@@ -62,6 +63,7 @@ func authMiddleware(next http.Handler) http.Handler {
 	})
 }
 
+// writeError sends a JSON {"error": message} response with the given status code.
 func writeError(w http.ResponseWriter, statusCode int, message string) {
 	w.Header().Set("Content-Type", "application/json")
 	w.WriteHeader(statusCode)
@@ -75,11 +77,15 @@ func writeError(w http.ResponseWriter, statusCode int, message string) {
 	}
 }
 
+// handleError maps err to an HTTP status via parseError and writes the response.
 func handleError(w http.ResponseWriter, err error) {
 	status, message := parseError(err)
 	writeError(w, status, message)
 }
 
+// parseError inspects err and returns a (statusCode, userMessage) pair.
+// Domain errors are mapped to 4xx; Kubernetes Unauthorized/Forbidden are mapped
+// to 401/403 with generic messages to avoid leaking API server details.
 func parseError(err error) (int, string) {
 	var nf *management.NotFoundError
 	if errors.As(err, &nf) {
@@ -97,6 +103,12 @@ func parseError(err error) (int, string) {
 	if errors.As(err, &ce) {
 		return http.StatusConflict, err.Error()
 	}
+	if apierrors.IsUnauthorized(err) {
+		return http.StatusUnauthorized, "authentication failed"
+	}
+	if apierrors.IsForbidden(err) {
+		return http.StatusForbidden, "insufficient permissions"
+	}
 	log.WithError(err).Error("unexpected management API error")
 	return http.StatusInternalServerError, "An unexpected error occurred"
 }

internal/managementrouter/router_test.go

@@ -0,0 +1,83 @@
+package managementrouter
+
+import (
+	"fmt"
+	"net/http"
+	"testing"
+
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+
+	"github.com/openshift/monitoring-plugin/pkg/management"
+)
+
+func TestParseError(t *testing.T) {
+	tests := []struct {
+		name           string
+		err            error
+		expectedStatus int
+		expectedMsg    string
+	}{
+		{
+			name:           "NotFoundError",
+			err:            &management.NotFoundError{Resource: "AlertRule", Id: "abc"},
+			expectedStatus: http.StatusNotFound,
+		},
+		{
+			name:           "ValidationError",
+			err:            &management.ValidationError{Message: "bad input"},
+			expectedStatus: http.StatusBadRequest,
+		},
+		{
+			name:           "NotAllowedError",
+			err:            &management.NotAllowedError{Message: "not allowed"},
+			expectedStatus: http.StatusMethodNotAllowed,
+		},
+		{
+			name:           "ConflictError",
+			err:            &management.ConflictError{Message: "conflict"},
+			expectedStatus: http.StatusConflict,
+		},
+		{
+			name: "Kubernetes Forbidden",
+			err: apierrors.NewForbidden(schema.GroupResource{
+				Group: "monitoring.coreos.com", Resource: "prometheusrules",
+			}, "test-pr", fmt.Errorf("access denied")),
+			expectedStatus: http.StatusForbidden,
+			expectedMsg:    "insufficient permissions",
+		},
+		{
+			name: "Kubernetes Forbidden wrapped",
+			err: fmt.Errorf("failed to get PrometheusRule: %w",
+				apierrors.NewForbidden(schema.GroupResource{
+					Group: "monitoring.coreos.com", Resource: "prometheusrules",
+				}, "test-pr", fmt.Errorf("access denied"))),
+			expectedStatus: http.StatusForbidden,
+			expectedMsg:    "insufficient permissions",
+		},
+		{
+			name:           "Kubernetes Unauthorized",
+			err:            apierrors.NewUnauthorized("token expired"),
+			expectedStatus: http.StatusUnauthorized,
+			expectedMsg:    "authentication failed",
+		},
+		{
+			name:           "unknown error",
+			err:            fmt.Errorf("something unexpected"),
+			expectedStatus: http.StatusInternalServerError,
+			expectedMsg:    "An unexpected error occurred",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			status, msg := parseError(tt.err)
+			if status != tt.expectedStatus {
+				t.Errorf("expected status %d, got %d", tt.expectedStatus, status)
+			}
+			if tt.expectedMsg != "" && msg != tt.expectedMsg {
+				t.Errorf("expected message %q, got %q", tt.expectedMsg, msg)
+			}
+		})
+	}
+}

pkg/k8s/user_scoped_client.go

@@ -13,14 +13,21 @@ type userScopedClientsets struct {
 	osmV1        *osmv1client.Clientset
 }
 
+// buildUserScopedConfig creates a rest.Config that authenticates exclusively
+// with the given bearer token. It uses AnonymousClientConfig to strip all
+// existing auth (certs, basic auth, auth/exec providers, impersonation) while
+// preserving the server connection settings (host, TLS CA, proxy).
+func buildUserScopedConfig(baseConfig *rest.Config, userToken string) *rest.Config {
+	cfg := rest.AnonymousClientConfig(baseConfig)
+	cfg.BearerToken = userToken
+	return cfg
+}
+
 // newUserScopedClientsets creates clientsets that carry the supplied bearer
 // token so that Kubernetes RBAC is enforced for the requesting user on all
 // mutating API calls.
 func newUserScopedClientsets(baseConfig *rest.Config, userToken string) (*userScopedClientsets, error) {
-	cfg := rest.CopyConfig(baseConfig)
-	// Override any SA token loaded from the file system with the user's token.
-	cfg.BearerToken = userToken
-	cfg.BearerTokenFile = ""
+	cfg := buildUserScopedConfig(baseConfig, userToken)
 
 	monClient, err := monitoringv1client.NewForConfig(cfg)
 	if err != nil {

pkg/k8s/user_scoped_client_test.go

@@ -0,0 +1,64 @@
+package k8s
+
+import (
+	"testing"
+
+	"k8s.io/client-go/rest"
+)
+
+func TestBuildUserScopedConfig(t *testing.T) {
+	base := &rest.Config{
+		Host:            "https://api.example.com:6443",
+		BearerToken:     "sa-token",
+		BearerTokenFile: "/var/run/secrets/kubernetes.io/serviceaccount/token",
+		TLSClientConfig: rest.TLSClientConfig{
+			Insecure: true,
+			CertData: []byte("admin-cert"),
+			KeyData:  []byte("admin-key"),
+			CertFile: "/path/to/cert",
+			KeyFile:  "/path/to/key",
+		},
+	}
+
+	cfg := buildUserScopedConfig(base, "user-token")
+
+	// Derived config uses the user token exclusively.
+	if cfg.BearerToken != "user-token" {
+		t.Errorf("derived BearerToken = %q, want %q", cfg.BearerToken, "user-token")
+	}
+	if cfg.BearerTokenFile != "" {
+		t.Errorf("derived BearerTokenFile = %q, want empty", cfg.BearerTokenFile)
+	}
+	if cfg.CertData != nil {
+		t.Error("derived CertData should be nil")
+	}
+	if cfg.KeyData != nil {
+		t.Error("derived KeyData should be nil")
+	}
+	if cfg.CertFile != "" {
+		t.Errorf("derived CertFile = %q, want empty", cfg.CertFile)
+	}
+	if cfg.KeyFile != "" {
+		t.Errorf("derived KeyFile = %q, want empty", cfg.KeyFile)
+	}
+	if !cfg.Insecure {
+		t.Error("derived Insecure should be preserved as true")
+	}
+	if cfg.Host != base.Host {
+		t.Errorf("derived Host = %q, want %q", cfg.Host, base.Host)
+	}
+
+	// Base config must not be mutated.
+	if base.CertData == nil {
+		t.Error("base CertData was mutated")
+	}
+	if base.KeyData == nil {
+		t.Error("base KeyData was mutated")
+	}
+	if base.BearerToken != "sa-token" {
+		t.Errorf("base BearerToken = %q, want %q", base.BearerToken, "sa-token")
+	}
+	if base.BearerTokenFile != "/var/run/secrets/kubernetes.io/serviceaccount/token" {
+		t.Errorf("base BearerTokenFile = %q, was mutated", base.BearerTokenFile)
+	}
+}

test/e2e/framework/framework.go

@@ -113,6 +113,8 @@ func (f *Framework) HTTPClient() *http.Client {
 	return f.httpClient
 }
 
+// CreateNamespace creates a uniquely-named namespace (prefixed with name and
+// a unix timestamp) and returns the actual name plus a cleanup function.
 func (f *Framework) CreateNamespace(ctx context.Context, name string, isClusterMonitoringNamespace bool) (string, CleanupFunc, error) {
 	testNamespace := fmt.Sprintf("%s-%d", name, time.Now().Unix())
 	namespace := &corev1.Namespace{
@@ -212,3 +214,102 @@ func createServiceAccountToken(clientset *kubernetes.Clientset) (string, error)
 	}
 	return resp.Status.Token, nil
 }
+// ScopedUser represents a ServiceAccount with specific RBAC permissions for testing.
+type ScopedUser struct {
+	Token   string
+	Cleanup CleanupFunc
+}
+
+// requestServiceAccountToken creates a short-lived (1 hour) bearer token for
+// the named ServiceAccount via the TokenRequest API.
+func (f *Framework) requestServiceAccountToken(ctx context.Context, namespace, name string) (string, error) {
+	expSeconds := int64(3600)
+	treq := &authv1.TokenRequest{
+		Spec: authv1.TokenRequestSpec{ExpirationSeconds: &expSeconds},
+	}
+	tokenResp, err := f.Clientset.CoreV1().ServiceAccounts(namespace).CreateToken(ctx, name, treq, metav1.CreateOptions{})
+	if err != nil {
+		return "", fmt.Errorf("requesting token for %s/%s: %w", namespace, name, err)
+	}
+	return tokenResp.Status.Token, nil
+}
+
+// CreateScopedUser creates a ServiceAccount in the given namespace with a Role
+// granting the specified verbs on the specified resources. Returns a bearer token
+// and a cleanup function. The apiGroup should be e.g. "monitoring.coreos.com".
+func (f *Framework) CreateScopedUser(ctx context.Context, name, namespace, apiGroup string, resources, verbs []string) (*ScopedUser, error) {
+	rollback := func() {
+		_ = f.Clientset.RbacV1().RoleBindings(namespace).Delete(ctx, name, metav1.DeleteOptions{})
+		_ = f.Clientset.RbacV1().Roles(namespace).Delete(ctx, name, metav1.DeleteOptions{})
+		_ = f.Clientset.CoreV1().ServiceAccounts(namespace).Delete(ctx, name, metav1.DeleteOptions{})
+	}
+
+	sa := &corev1.ServiceAccount{
+		ObjectMeta: metav1.ObjectMeta{Name: name},
+	}
+	if _, err := f.Clientset.CoreV1().ServiceAccounts(namespace).Create(ctx, sa, metav1.CreateOptions{}); err != nil && !apierrors.IsAlreadyExists(err) {
+		return nil, fmt.Errorf("creating service account %s/%s: %w", namespace, name, err)
+	}
+
+	role := &rbacv1.Role{
+		ObjectMeta: metav1.ObjectMeta{Name: name},
+		Rules: []rbacv1.PolicyRule{{
+			APIGroups: []string{apiGroup},
+			Resources: resources,
+			Verbs:     verbs,
+		}},
+	}
+	if _, err := f.Clientset.RbacV1().Roles(namespace).Create(ctx, role, metav1.CreateOptions{}); err != nil && !apierrors.IsAlreadyExists(err) {
+		rollback()
+		return nil, fmt.Errorf("creating role %s/%s: %w", namespace, name, err)
+	}
+
+	rb := &rbacv1.RoleBinding{
+		ObjectMeta: metav1.ObjectMeta{Name: name},
+		Subjects: []rbacv1.Subject{{
+			Kind:      rbacv1.ServiceAccountKind,
+			Name:      name,
+			Namespace: namespace,
+		}},
+		RoleRef: rbacv1.RoleRef{
+			APIGroup: rbacv1.GroupName,
+			Kind:     "Role",
+			Name:     name,
+		},
+	}
+	if _, err := f.Clientset.RbacV1().RoleBindings(namespace).Create(ctx, rb, metav1.CreateOptions{}); err != nil && !apierrors.IsAlreadyExists(err) {
+		rollback()
+		return nil, fmt.Errorf("creating role binding %s/%s: %w", namespace, name, err)
+	}
+
+	token, err := f.requestServiceAccountToken(ctx, namespace, name)
+	if err != nil {
+		rollback()
+		return nil, err
+	}
+
+	return &ScopedUser{Token: token, Cleanup: func() error { rollback(); return nil }}, nil
+}
+
+// CreateUnprivilegedUser creates a ServiceAccount with no RBAC permissions.
+func (f *Framework) CreateUnprivilegedUser(ctx context.Context, name, namespace string) (*ScopedUser, error) {
+	sa := &corev1.ServiceAccount{
+		ObjectMeta: metav1.ObjectMeta{Name: name},
+	}
+	if _, err := f.Clientset.CoreV1().ServiceAccounts(namespace).Create(ctx, sa, metav1.CreateOptions{}); err != nil && !apierrors.IsAlreadyExists(err) {
+		return nil, fmt.Errorf("creating service account %s/%s: %w", namespace, name, err)
+	}
+
+	token, err := f.requestServiceAccountToken(ctx, namespace, name)
+	if err != nil {
+		_ = f.Clientset.CoreV1().ServiceAccounts(namespace).Delete(ctx, name, metav1.DeleteOptions{})
+		return nil, err
+	}
+
+	cleanup := func() error {
+		_ = f.Clientset.CoreV1().ServiceAccounts(namespace).Delete(ctx, name, metav1.DeleteOptions{})
+		return nil
+	}
+
+	return &ScopedUser{Token: token, Cleanup: cleanup}, nil
+}