fix(operator): resolve PR #69 review findings

Author: Agent-Hellboy

internal/operator/controller.go

@@ -119,6 +119,9 @@ func (r *MCPServerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		return ctrl.Result{Requeue: true}, nil
 	}
 
+	if err := r.validateMCPServerSpec(ctx, mcpServer, logger); err != nil {
+		return ctrl.Result{Requeue: false}, err
+	}
 	if err := r.validateIngressConfig(ctx, mcpServer, logger); err != nil {
 		return ctrl.Result{Requeue: false}, err
 	}
@@ -172,22 +175,30 @@ func (r *MCPServerReconciler) applyDefaultsIfNeeded(ctx context.Context, mcpServ
 	return true, nil
 }
 
-func (r *MCPServerReconciler) validateIngressConfig(ctx context.Context, mcpServer *mcpv1alpha1.MCPServer, logger logr.Logger) error {
+func (r *MCPServerReconciler) validateMCPServerSpec(ctx context.Context, mcpServer *mcpv1alpha1.MCPServer, logger logr.Logger) error {
 	if _, err := mcpServer.ValidateCreate(); err != nil {
 		r.updateStatus(ctx, mcpServer, "Error", err.Error(), resourceReadiness{})
-		logOperatorError(logger, err, "Invalid ingress configuration")
+		logOperatorError(logger, err, "Invalid MCPServer specification")
 		return err
 	}
 	return nil
 }
 
-func (r *MCPServerReconciler) validateGatewayConfig(ctx context.Context, mcpServer *mcpv1alpha1.MCPServer, logger logr.Logger) error {
-	if _, err := mcpServer.ValidateCreate(); err != nil {
-		r.updateStatus(ctx, mcpServer, "Error", err.Error(), resourceReadiness{})
-		logOperatorError(logger, err, "Invalid gateway configuration")
-		return err
+func (r *MCPServerReconciler) validateIngressConfig(ctx context.Context, mcpServer *mcpv1alpha1.MCPServer, logger logr.Logger) error {
+	if strings.TrimSpace(mcpServer.Spec.PublicPathPrefix) == "" {
+		if err := r.requireSpecField(ctx, mcpServer, logger, "ingress path", effectiveIngressPath(mcpServer),
+			"ingressPath is required; set spec.ingressPath or ensure metadata.name is set"); err != nil {
+			return err
+		}
+		if err := r.requireSpecField(ctx, mcpServer, logger, "ingress host", effectiveIngressHost(mcpServer),
+			"ingressHost is required; set spec.ingressHost, set MCP_DEFAULT_INGRESS_HOST on the operator, or use spec.publicPathPrefix for hostless routing"); err != nil {
+			return err
+		}
 	}
+	return nil
+}
 
+func (r *MCPServerReconciler) validateGatewayConfig(ctx context.Context, mcpServer *mcpv1alpha1.MCPServer, logger logr.Logger) error {
 	if gatewayEnabled(mcpServer) {
 		if _, err := r.resolveGatewayImage(mcpServer); err != nil {
 			r.updateStatus(ctx, mcpServer, "Error", err.Error(), resourceReadiness{})
@@ -314,9 +325,12 @@ func determinePhase(readiness resourceReadiness) (string, bool) {
 }
 
 func (r *MCPServerReconciler) setDefaults(mcpServer *mcpv1alpha1.MCPServer) {
+	ingressHostUnset := strings.TrimSpace(mcpServer.Spec.IngressHost) == ""
+	publicPathPrefixUnset := strings.TrimSpace(mcpServer.Spec.PublicPathPrefix) == ""
+
 	mcpServer.Default()
 
-	if strings.TrimSpace(mcpServer.Spec.IngressHost) == "" && strings.TrimSpace(mcpServer.Spec.PublicPathPrefix) == "" {
+	if ingressHostUnset && publicPathPrefixUnset {
 		mcpServer.Spec.IngressHost = strings.TrimSpace(r.DefaultIngressHost)
 	}
 

internal/operator/controller_test.go

@@ -157,7 +157,7 @@ func TestBuildGatewayContainerAppliesDefaultResources(t *testing.T) {
 	}
 }
 
-func TestValidateGatewayConfigRejectsInvalidRolloutValues(t *testing.T) {
+func TestValidateMCPServerSpecRejectsInvalidRolloutValues(t *testing.T) {
 	scheme := runtime.NewScheme()
 	if err := mcpv1alpha1.AddToScheme(scheme); err != nil {
 		t.Fatalf("AddToScheme() error = %v", err)
@@ -195,7 +195,7 @@ func TestValidateGatewayConfigRejectsInvalidRolloutValues(t *testing.T) {
 		Scheme: scheme,
 	}
 
-	err := reconciler.validateGatewayConfig(context.Background(), server, logr.Discard())
+	err := reconciler.validateMCPServerSpec(context.Background(), server, logr.Discard())
 	if err == nil {
 		t.Fatal("expected rollout validation error")
 	}
@@ -204,7 +204,7 @@ func TestValidateGatewayConfigRejectsInvalidRolloutValues(t *testing.T) {
 	}
 }
 
-func TestValidateGatewayConfigRequiresOAuthIssuer(t *testing.T) {
+func TestValidateMCPServerSpecRequiresOAuthIssuer(t *testing.T) {
 	scheme := runtime.NewScheme()
 	if err := mcpv1alpha1.AddToScheme(scheme); err != nil {
 		t.Fatalf("AddToScheme() error = %v", err)
@@ -240,7 +240,7 @@ func TestValidateGatewayConfigRequiresOAuthIssuer(t *testing.T) {
 		Scheme: scheme,
 	}
 
-	err := reconciler.validateGatewayConfig(context.Background(), server, logr.Discard())
+	err := reconciler.validateMCPServerSpec(context.Background(), server, logr.Discard())
 	if err == nil {
 		t.Fatal("expected oauth issuer validation error")
 	}