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
@@ -295,6 +297,9 @@ func (r *MCPServerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 	deployment := &appsv1.Deployment{}
 	err = r.Get(ctx, types.NamespacedName{Name: mcpServer.Name, Namespace: mcpServer.Namespace}, deployment)
 	if err != nil && errors.IsNotFound(err) {
+		// Validate PodTemplateSpec and update status
+		r.validateAndUpdatePodTemplateStatus(ctx, mcpServer)
+
 		// Define a new deployment
 		dep := r.deploymentForMCPServer(ctx, mcpServer)
 		if dep == nil {
@@ -364,6 +369,9 @@ func (r *MCPServerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 
 	// Check if the deployment spec changed
 	if r.deploymentNeedsUpdate(ctx, deployment, mcpServer) {
+		// Validate PodTemplateSpec and update status
+		r.validateAndUpdatePodTemplateStatus(ctx, mcpServer)
+
 		// Update the deployment
 		newDeployment := r.deploymentForMCPServer(ctx, mcpServer)
 		deployment.Spec = newDeployment.Spec
@@ -406,6 +414,47 @@ 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) {
+	ctxLogger := log.FromContext(ctx)
+
+	// Only validate if PodTemplateSpec is provided
+	if mcpServer.Spec.PodTemplateSpec == nil || mcpServer.Spec.PodTemplateSpec.Raw == nil {
+		return
+	}
+
+	_, 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 will continue without pod customizations.", 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 continues without customizations.", err),
+		})
+	} else {
+		// 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")
+	}
+}
+
 // 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) {
@@ -756,17 +805,22 @@ func (r *MCPServerReconciler) deploymentForMCPServer(ctx context.Context, m *mcp
 		args = append(args, fmt.Sprintf("--from-configmap=%s", configMapRef))
 
 		// 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)))
+				}
 			}
 		}
 	} else {
@@ -794,18 +848,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)))
+				}
 			}
 		}
 
@@ -1379,21 +1441,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
@@ -1447,7 +1507,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()
@@ -1513,7 +1580,6 @@ func (r *MCPServerReconciler) deploymentNeedsUpdate(
 		if !equalOpenTelemetryArgs(otelConfig, container.Args) {
 			return true
 		}
-
 	}
 
 	// Check if the service account name has changed