Reduce MCPServer CRD size by using runtime.RawExtension for PodTemplateSpec

The MCPServer CRD was too large (~9500 lines) to apply without server-side
apply due to the embedded PodTemplateSpec taking up ~8500 lines. This was
causing issues as reported in GitHub issue #2013.

Changed the PodTemplateSpec field from a strongly-typed corev1.PodTemplateSpec
to runtime.RawExtension, which stores the raw JSON without schema validation
at the CRD level. This reduces the CRD size from ~9500 lines to 651 lines
(93% reduction).

The solution maintains full backwards compatibility - users can still use
the same YAML structure. Validation now happens at runtime in the operator,
with proper error handling via Kubernetes events and status conditions to
notify users when invalid PodTemplateSpec data is provided.

Key changes:
- Modified MCPServer type to use runtime.RawExtension for PodTemplateSpec
- Updated PodTemplateSpecBuilder to unmarshal and validate at runtime
- Added event recording and status conditions for validation errors
- Added comprehensive tests for invalid PodTemplateSpec scenarios
- Fixed race conditions in parallel tests

Fixes #2013

Author: JAORMX

cmd/thv-operator/api/v1alpha1/mcpserver_types.go

@@ -1,8 +1,8 @@
 package v1alpha1
 
 import (
-	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 )
 
 // Condition types for MCPServer
@@ -85,8 +85,11 @@ type MCPServerSpec struct {
 	// This allows for customizing the pod configuration beyond what is provided by the other fields.
 	// Note that to modify the specific container the MCP server runs in, you must specify
 	// the `mcp` container name in the PodTemplateSpec.
+	// This field accepts a PodTemplateSpec object as JSON/YAML.
 	// +optional
-	PodTemplateSpec *corev1.PodTemplateSpec `json:"podTemplateSpec,omitempty"`
+	// +kubebuilder:pruning:PreserveUnknownFields
+	// +kubebuilder:validation:Type=object
+	PodTemplateSpec *runtime.RawExtension `json:"podTemplateSpec,omitempty"`
 
 	// ResourceOverrides allows overriding annotations and labels for resources created by the operator
 	// +optional

cmd/thv-operator/api/v1alpha1/zz_generated.deepcopy.go

@@ -0,0 +1,21 @@
+package controllers
+
+import (
+	"encoding/json"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+)
+
+// podTemplateSpecToRawExtension is a test helper to convert PodTemplateSpec to RawExtension
+func podTemplateSpecToRawExtension(t *testing.T, pts *corev1.PodTemplateSpec) *runtime.RawExtension {
+	t.Helper()
+	if pts == nil {
+		return nil
+	}
+	raw, err := json.Marshal(pts)
+	require.NoError(t, err, "Failed to marshal PodTemplateSpec")
+	return &runtime.RawExtension{Raw: raw}
+}

cmd/thv-operator/controllers/helpers_test.go

@@ -26,6 +26,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/client-go/rest"
+	"k8s.io/client-go/tools/record"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
@@ -40,6 +41,7 @@ import (
 type MCPServerReconciler struct {
 	client.Client
 	Scheme           *runtime.Scheme
+	Recorder         record.EventRecorder
 	platformDetector kubernetes.PlatformDetector
 	detectedPlatform kubernetes.Platform
 	platformOnce     sync.Once
@@ -186,6 +188,15 @@ func (r *MCPServerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		return ctrl.Result{Requeue: true}, nil
 	}
 
+	// Validate PodTemplateSpec early - before other validations
+	// This ensures we fail fast if the spec is invalid
+	if !r.validateAndUpdatePodTemplateStatus(ctx, mcpServer) {
+		// Invalid PodTemplateSpec - return without error to avoid infinite retries
+		// The user must fix the spec and the next reconciliation will retry
+		ctxLogger.Info("Skipping reconciliation due to invalid PodTemplateSpec")
+		return ctrl.Result{}, nil
+	}
+
 	// Check if MCPToolConfig is referenced and handle it
 	if err := r.handleToolConfig(ctx, mcpServer); err != nil {
 		ctxLogger.Error(err, "Failed to handle MCPToolConfig")
@@ -206,6 +217,10 @@ func (r *MCPServerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		setImageValidationCondition(mcpServer, metav1.ConditionTrue,
 			mcpv1alpha1.ConditionReasonImageValidationSkipped,
 			"Image validation was not performed (no enforcement configured)")
+		// Update status to persist the condition
+		if statusErr := r.Status().Update(ctx, mcpServer); statusErr != nil {
+			ctxLogger.Error(statusErr, "Failed to update MCPServer status after image validation")
+		}
 	} else if goerr.Is(err, validation.ErrImageInvalid) {
 		ctxLogger.Error(err, "MCPServer image validation failed", "image", mcpServer.Spec.Image)
 		// Update status to reflect validation failure
@@ -236,6 +251,10 @@ func (r *MCPServerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		setImageValidationCondition(mcpServer, metav1.ConditionTrue,
 			mcpv1alpha1.ConditionReasonImageValidationSuccess,
 			"Image validation passed - image found in enforced registries")
+		// Update status to persist the condition
+		if statusErr := r.Status().Update(ctx, mcpServer); statusErr != nil {
+			ctxLogger.Error(statusErr, "Failed to update MCPServer status after image validation")
+		}
 	}
 
 	// Check if the MCPServer instance is marked to be deleted
@@ -406,6 +425,64 @@ func setImageValidationCondition(mcpServer *mcpv1alpha1.MCPServer, status metav1
 	})
 }
 
+// validateAndUpdatePodTemplateStatus validates the PodTemplateSpec and updates the MCPServer status
+// with appropriate conditions and events
+func (r *MCPServerReconciler) validateAndUpdatePodTemplateStatus(ctx context.Context, mcpServer *mcpv1alpha1.MCPServer) bool {
+	ctxLogger := log.FromContext(ctx)
+
+	// Only validate if PodTemplateSpec is provided
+	if mcpServer.Spec.PodTemplateSpec == nil || mcpServer.Spec.PodTemplateSpec.Raw == nil {
+		// No PodTemplateSpec provided, validation passes
+		return true
+	}
+
+	_, err := NewMCPServerPodTemplateSpecBuilder(mcpServer.Spec.PodTemplateSpec)
+	if err != nil {
+		// Record event for invalid PodTemplateSpec
+		r.Recorder.Eventf(mcpServer, corev1.EventTypeWarning, "InvalidPodTemplateSpec",
+			"Failed to parse PodTemplateSpec: %v. Deployment blocked until PodTemplateSpec is fixed.", err)
+
+		// Set phase and message
+		mcpServer.Status.Phase = mcpv1alpha1.MCPServerPhaseFailed
+		mcpServer.Status.Message = fmt.Sprintf("Invalid PodTemplateSpec: %v", err)
+
+		// Set condition for invalid PodTemplateSpec
+		meta.SetStatusCondition(&mcpServer.Status.Conditions, metav1.Condition{
+			Type:               "PodTemplateValid",
+			Status:             metav1.ConditionFalse,
+			ObservedGeneration: mcpServer.Generation,
+			Reason:             "InvalidPodTemplateSpec",
+			Message:            fmt.Sprintf("Failed to parse PodTemplateSpec: %v. Deployment blocked until fixed.", err),
+		})
+
+		// Update status with the condition
+		if statusErr := r.Status().Update(ctx, mcpServer); statusErr != nil {
+			ctxLogger.Error(statusErr, "Failed to update MCPServer status with PodTemplateSpec validation")
+			return false
+		}
+
+		ctxLogger.Error(err, "PodTemplateSpec validation failed")
+		return false
+	}
+
+	// Set condition for valid PodTemplateSpec
+	meta.SetStatusCondition(&mcpServer.Status.Conditions, metav1.Condition{
+		Type:               "PodTemplateValid",
+		Status:             metav1.ConditionTrue,
+		ObservedGeneration: mcpServer.Generation,
+		Reason:             "ValidPodTemplateSpec",
+		Message:            "PodTemplateSpec is valid",
+	})
+
+	// Update status with the condition
+	if statusErr := r.Status().Update(ctx, mcpServer); statusErr != nil {
+		ctxLogger.Error(statusErr, "Failed to update MCPServer status with PodTemplateSpec validation")
+	}
+
+	ctxLogger.V(1).Info("PodTemplateSpec validation completed successfully")
+	return true
+}
+
 // handleRestartAnnotation checks if the restart annotation has been updated and triggers a restart if needed
 // Returns true if a restart was triggered and the reconciliation should be requeued
 func (r *MCPServerReconciler) handleRestartAnnotation(ctx context.Context, mcpServer *mcpv1alpha1.MCPServer) (bool, error) {
@@ -755,17 +832,22 @@ func (r *MCPServerReconciler) deploymentForMCPServer(ctx context.Context, m *mcp
 	useConfigMap := os.Getenv("TOOLHIVE_USE_CONFIGMAP") == trueValue
 	if useConfigMap {
 		// Also add pod template patch for secrets (same as regular flags approach)
-		finalPodTemplateSpec := NewMCPServerPodTemplateSpecBuilder(m.Spec.PodTemplateSpec).
-			WithSecrets(m.Spec.Secrets).
-			Build()
-		// Add pod template patch if we have one
-		if finalPodTemplateSpec != nil {
-			podTemplatePatch, err := json.Marshal(finalPodTemplateSpec)
-			if err != nil {
-				ctxLogger := log.FromContext(ctx)
-				ctxLogger.Error(err, "Failed to marshal pod template spec")
-			} else {
-				args = append(args, fmt.Sprintf("--k8s-pod-patch=%s", string(podTemplatePatch)))
+		builder, err := NewMCPServerPodTemplateSpecBuilder(m.Spec.PodTemplateSpec)
+		if err != nil {
+			ctxLogger := log.FromContext(ctx)
+			ctxLogger.Error(err, "Invalid PodTemplateSpec in MCPServer spec, continuing without customizations")
+			// Continue without pod template patch - the deployment will still work
+		} else {
+			finalPodTemplateSpec := builder.WithSecrets(m.Spec.Secrets).Build()
+			// Add pod template patch if we have one
+			if finalPodTemplateSpec != nil {
+				podTemplatePatch, err := json.Marshal(finalPodTemplateSpec)
+				if err != nil {
+					ctxLogger := log.FromContext(ctx)
+					ctxLogger.Error(err, "Failed to marshal pod template spec")
+				} else {
+					args = append(args, fmt.Sprintf("--k8s-pod-patch=%s", string(podTemplatePatch)))
+				}
 			}
 		}
 
@@ -812,18 +894,26 @@ func (r *MCPServerReconciler) deploymentForMCPServer(ctx context.Context, m *mcp
 			defaultSA := mcpServerServiceAccountName(m.Name)
 			serviceAccount = &defaultSA
 		}
-		finalPodTemplateSpec := NewMCPServerPodTemplateSpecBuilder(m.Spec.PodTemplateSpec).
-			WithServiceAccount(serviceAccount).
-			WithSecrets(m.Spec.Secrets).
-			Build()
-		// Add pod template patch if we have one
-		if finalPodTemplateSpec != nil {
-			podTemplatePatch, err := json.Marshal(finalPodTemplateSpec)
-			if err != nil {
-				ctxLogger := log.FromContext(ctx)
-				ctxLogger.Error(err, "Failed to marshal pod template spec")
-			} else {
-				args = append(args, fmt.Sprintf("--k8s-pod-patch=%s", string(podTemplatePatch)))
+
+		builder, err := NewMCPServerPodTemplateSpecBuilder(m.Spec.PodTemplateSpec)
+		if err != nil {
+			ctxLogger := log.FromContext(ctx)
+			ctxLogger.Error(err, "Invalid PodTemplateSpec in MCPServer spec, continuing without customizations")
+			// Continue without pod template patch - the deployment will still work
+		} else {
+			finalPodTemplateSpec := builder.
+				WithServiceAccount(serviceAccount).
+				WithSecrets(m.Spec.Secrets).
+				Build()
+			// Add pod template patch if we have one
+			if finalPodTemplateSpec != nil {
+				podTemplatePatch, err := json.Marshal(finalPodTemplateSpec)
+				if err != nil {
+					ctxLogger := log.FromContext(ctx)
+					ctxLogger.Error(err, "Failed to marshal pod template spec")
+				} else {
+					args = append(args, fmt.Sprintf("--k8s-pod-patch=%s", string(podTemplatePatch)))
+				}
 			}
 		}
 
@@ -1393,21 +1483,19 @@ func (r *MCPServerReconciler) deploymentNeedsUpdate(
 		// TODO: Add more comprehensive checks for PodTemplateSpec changes beyond just the args
 		// This would involve comparing the actual pod template spec fields with what would be
 		// generated by the operator, rather than just checking the command-line arguments.
-		if mcpServer.Spec.PodTemplateSpec != nil {
-			podTemplatePatch, err := json.Marshal(mcpServer.Spec.PodTemplateSpec)
-			if err == nil {
-				podTemplatePatchArg := fmt.Sprintf("--k8s-pod-patch=%s", string(podTemplatePatch))
-				found := false
-				for _, arg := range container.Args {
-					if arg == podTemplatePatchArg {
-						found = true
-						break
-					}
-				}
-				if !found {
-					return true
+		if mcpServer.Spec.PodTemplateSpec != nil && mcpServer.Spec.PodTemplateSpec.Raw != nil {
+			// Use the raw bytes directly since PodTemplateSpec is now a RawExtension
+			podTemplatePatchArg := fmt.Sprintf("--k8s-pod-patch=%s", string(mcpServer.Spec.PodTemplateSpec.Raw))
+			found := false
+			for _, arg := range container.Args {
+				if arg == podTemplatePatchArg {
+					found = true
+					break
 				}
 			}
+			if !found {
+				return true
+			}
 		}
 
 		// Check if the container port has changed
@@ -1461,7 +1549,14 @@ func (r *MCPServerReconciler) deploymentNeedsUpdate(
 			defaultSA := mcpServerServiceAccountName(mcpServer.Name)
 			serviceAccount = &defaultSA
 		}
-		expectedPodTemplateSpec := NewMCPServerPodTemplateSpecBuilder(mcpServer.Spec.PodTemplateSpec).
+
+		builder, err := NewMCPServerPodTemplateSpecBuilder(mcpServer.Spec.PodTemplateSpec)
+		if err != nil {
+			// If we can't parse the PodTemplateSpec, consider it as needing update
+			return true
+		}
+
+		expectedPodTemplateSpec := builder.
 			WithServiceAccount(serviceAccount).
 			WithSecrets(mcpServer.Spec.Secrets).
 			Build()
@@ -1527,7 +1622,6 @@ func (r *MCPServerReconciler) deploymentNeedsUpdate(
 		if !equalOpenTelemetryArgs(otelConfig, container.Args) {
 			return true
 		}
-
 	}
 
 	// Check if the service account name has changed