NO-JIRA: test: add RBAC e2e tests for create and delete endpoints

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,28 +77,37 @@ 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.
+// Kubernetes auth errors are checked first to prevent information leakage;
+// domain errors are then mapped to 4xx codes.
 func parseError(err error) (int, string) {
-	var nf *management.NotFoundError
-	if errors.As(err, &nf) {
+	var (
+		notFound   *management.NotFoundError
+		validation *management.ValidationError
+		notAllowed *management.NotAllowedError
+		conflict   *management.ConflictError
+	)
+	switch {
+	case apierrors.IsUnauthorized(err):
+		return http.StatusUnauthorized, "authentication failed"
+	case apierrors.IsForbidden(err):
+		return http.StatusForbidden, "insufficient permissions"
+	case errors.As(err, &notFound):
 		return http.StatusNotFound, err.Error()
-	}
-	var ve *management.ValidationError
-	if errors.As(err, &ve) {
+	case errors.As(err, &validation):
 		return http.StatusBadRequest, err.Error()
-	}
-	var na *management.NotAllowedError
-	if errors.As(err, &na) {
+	case errors.As(err, &notAllowed):
 		return http.StatusMethodNotAllowed, err.Error()
-	}
-	var ce *management.ConflictError
-	if errors.As(err, &ce) {
+	case errors.As(err, &conflict):
 		return http.StatusConflict, err.Error()
+	default:
+		log.WithError(err).Error("unexpected management API error")
+		return http.StatusInternalServerError, "An unexpected error occurred"
 	}
-	log.WithError(err).Error("unexpected management API error")
-	return http.StatusInternalServerError, "An unexpected error occurred"
 }

internal/managementrouter/router_test.go

@@ -0,0 +1,88 @@
+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:           "NotFoundError wrapped",
+			err:            fmt.Errorf("lookup failed: %w", &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,71 @@
+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",
+		Impersonate: rest.ImpersonationConfig{
+			UserName: "system:admin",
+			Groups:   []string{"system:masters"},
+		},
+		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.Impersonate.UserName != "" || len(cfg.Impersonate.Groups) != 0 {
+		t.Errorf("derived Impersonate = %+v, want empty", cfg.Impersonate)
+	}
+	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)
+	}
+}