MCPRemoteProxy: Add remaining configuration validations

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

@@ -268,6 +268,21 @@ const (
 
 	// ConditionReasonOIDCIssuerInvalid indicates the OIDC issuer URL is malformed
 	ConditionReasonOIDCIssuerInvalid = "OIDCIssuerInvalid"
+
+	// ConditionReasonAuthzPolicySyntaxInvalid indicates an inline Cedar policy has a syntax error
+	ConditionReasonAuthzPolicySyntaxInvalid = "AuthzPolicySyntaxInvalid"
+
+	// ConditionReasonAuthzConfigMapNotFound indicates the referenced authz ConfigMap was not found
+	ConditionReasonAuthzConfigMapNotFound = "AuthzConfigMapNotFound"
+
+	// ConditionReasonHeaderSecretNotFound indicates a referenced header Secret was not found
+	ConditionReasonHeaderSecretNotFound = "HeaderSecretNotFound"
+
+	// ConditionReasonRemoteURLInvalid indicates the remoteURL is malformed or has an invalid scheme
+	ConditionReasonRemoteURLInvalid = "RemoteURLInvalid"
+
+	// ConditionReasonJWKSURLInvalid indicates the JWKS URL is malformed or has an invalid scheme
+	ConditionReasonJWKSURLInvalid = "JWKSURLInvalid"
 )
 
 //+kubebuilder:object:root=true

cmd/thv-operator/controllers/mcpremoteproxy_controller.go

@@ -7,6 +7,7 @@ package controllers
 
 import (
 	"context"
+	stderrors "errors"
 	"fmt"
 	"maps"
 	"reflect"
@@ -335,29 +336,49 @@ func (r *MCPRemoteProxyReconciler) ensureServiceURL(ctx context.Context, proxy *
 
 // validateSpec validates the MCPRemoteProxy spec
 func (r *MCPRemoteProxyReconciler) validateSpec(ctx context.Context, proxy *mcpv1alpha1.MCPRemoteProxy) error {
-	if proxy.Spec.RemoteURL == "" {
-		return fmt.Errorf("remoteURL is required")
-	}
-
 	// Validate external auth config if referenced
 	if proxy.Spec.ExternalAuthConfigRef != nil {
 		externalAuthConfig, err := ctrlutil.GetExternalAuthConfigForMCPRemoteProxy(ctx, r.Client, proxy)
 		if err != nil {
-			return fmt.Errorf("failed to validate external auth config: %w", err)
+			return r.failValidation(proxy,
+				mcpv1alpha1.ConditionReasonMCPRemoteProxyExternalAuthConfigFetchError,
+				fmt.Errorf("failed to validate external auth config: %w", err),
+			)
 		}
 		if externalAuthConfig == nil {
-			return fmt.Errorf("referenced MCPExternalAuthConfig %s not found", proxy.Spec.ExternalAuthConfigRef.Name)
+			return r.failValidation(proxy,
+				mcpv1alpha1.ConditionReasonMCPRemoteProxyExternalAuthConfigNotFound,
+				fmt.Errorf("referenced MCPExternalAuthConfig %s not found", proxy.Spec.ExternalAuthConfigRef.Name),
+			)
 		}
 	}
 
+	// Validate remote URL format (also rejects empty URLs)
+	if err := validation.ValidateRemoteURL(proxy.Spec.RemoteURL); err != nil {
+		return r.failValidation(proxy, mcpv1alpha1.ConditionReasonRemoteURLInvalid, err)
+	}
+
 	// Validate OIDC issuer URL scheme
 	if err := r.validateOIDCIssuerURL(proxy); err != nil {
 		reason := mcpv1alpha1.ConditionReasonOIDCIssuerInvalid
 		if strings.Contains(err.Error(), "HTTP scheme") {
 			reason = mcpv1alpha1.ConditionReasonOIDCIssuerInsecure
 		}
-		r.recordValidationEvent(proxy, reason, err.Error())
-		setConfigurationInvalidCondition(proxy, reason, err.Error())
+		return r.failValidation(proxy, reason, err)
+	}
+
+	// Validate JWKS URL format
+	if err := r.validateJWKSURL(proxy); err != nil {
+		return r.failValidation(proxy, mcpv1alpha1.ConditionReasonJWKSURLInvalid, err)
+	}
+
+	// Validate inline Cedar policy syntax
+	if err := r.validateAuthzPolicySyntax(proxy); err != nil {
+		return r.failValidation(proxy, mcpv1alpha1.ConditionReasonAuthzPolicySyntaxInvalid, err)
+	}
+
+	// Validate Kubernetes resource references (ConfigMaps, Secrets)
+	if err := r.validateK8sRefs(ctx, proxy); err != nil {
 		return err
 	}
 
@@ -373,6 +394,14 @@ func (r *MCPRemoteProxyReconciler) validateSpec(ctx context.Context, proxy *mcpv
 	return nil
 }
 
+// failValidation records a validation event, sets the ConfigurationValid condition to False,
+// and returns the error. This consolidates the repeated validate → event → condition → return pattern.
+func (r *MCPRemoteProxyReconciler) failValidation(proxy *mcpv1alpha1.MCPRemoteProxy, reason string, err error) error {
+	r.recordValidationEvent(proxy, reason, err.Error())
+	setConfigurationInvalidCondition(proxy, reason, err.Error())
+	return err
+}
+
 // recordValidationEvent emits a Warning event for a validation failure.
 func (r *MCPRemoteProxyReconciler) recordValidationEvent(proxy *mcpv1alpha1.MCPRemoteProxy, reason, message string) {
 	if r.Recorder != nil {
@@ -409,6 +438,117 @@ func (*MCPRemoteProxyReconciler) validateOIDCIssuerURL(proxy *mcpv1alpha1.MCPRem
 	return nil
 }
 
+// validateJWKSURL validates the JWKS URL scheme in the OIDC config.
+func (*MCPRemoteProxyReconciler) validateJWKSURL(proxy *mcpv1alpha1.MCPRemoteProxy) error {
+	oidcConfig := proxy.Spec.OIDCConfig
+
+	switch oidcConfig.Type {
+	case mcpv1alpha1.OIDCConfigTypeInline:
+		if oidcConfig.Inline != nil {
+			return validation.ValidateJWKSURL(oidcConfig.Inline.JWKSURL)
+		}
+	case mcpv1alpha1.OIDCConfigTypeKubernetes:
+		if oidcConfig.Kubernetes != nil {
+			return validation.ValidateJWKSURL(oidcConfig.Kubernetes.JWKSURL)
+		}
+	}
+	return nil
+}
+
+// validateAuthzPolicySyntax validates inline Cedar authorization policy syntax.
+func (*MCPRemoteProxyReconciler) validateAuthzPolicySyntax(
+	proxy *mcpv1alpha1.MCPRemoteProxy,
+) error {
+	if proxy.Spec.AuthzConfig == nil ||
+		proxy.Spec.AuthzConfig.Type != mcpv1alpha1.AuthzConfigTypeInline ||
+		proxy.Spec.AuthzConfig.Inline == nil {
+		return nil
+	}
+	return validation.ValidateCedarPolicies(proxy.Spec.AuthzConfig.Inline.Policies)
+}
+
+// validateK8sRefs validates that referenced ConfigMaps and Secrets exist.
+func (r *MCPRemoteProxyReconciler) validateK8sRefs(
+	ctx context.Context, proxy *mcpv1alpha1.MCPRemoteProxy,
+) error {
+	// Check authz ConfigMap reference
+	if proxy.Spec.AuthzConfig != nil &&
+		proxy.Spec.AuthzConfig.Type == mcpv1alpha1.AuthzConfigTypeConfigMap &&
+		proxy.Spec.AuthzConfig.ConfigMap != nil {
+		cm := &corev1.ConfigMap{}
+		cmName := proxy.Spec.AuthzConfig.ConfigMap.Name
+		err := r.Get(ctx, types.NamespacedName{
+			Name: cmName, Namespace: proxy.Namespace,
+		}, cm)
+		if err != nil {
+			if errors.IsNotFound(err) {
+				msg := fmt.Sprintf(
+					"authorization ConfigMap %q not found in namespace %q",
+					cmName, proxy.Namespace,
+				)
+				r.recordValidationEvent(
+					proxy,
+					mcpv1alpha1.ConditionReasonAuthzConfigMapNotFound,
+					msg,
+				)
+				setConfigurationInvalidCondition(
+					proxy,
+					mcpv1alpha1.ConditionReasonAuthzConfigMapNotFound,
+					msg,
+				)
+				return stderrors.New(msg)
+			}
+			ctxLogger := log.FromContext(ctx)
+			ctxLogger.Error(err, "Failed to fetch authorization ConfigMap", "name", cmName, "namespace", proxy.Namespace)
+			genericMsg := fmt.Sprintf("failed to fetch authorization ConfigMap %q", cmName)
+			r.recordValidationEvent(proxy, mcpv1alpha1.ConditionReasonAuthzConfigMapNotFound, genericMsg)
+			setConfigurationInvalidCondition(proxy, mcpv1alpha1.ConditionReasonAuthzConfigMapNotFound, genericMsg)
+			return stderrors.New(genericMsg)
+		}
+	}
+
+	// Check header Secret references
+	if proxy.Spec.HeaderForward != nil {
+		for _, headerRef := range proxy.Spec.HeaderForward.AddHeadersFromSecret {
+			if headerRef.ValueSecretRef == nil {
+				continue
+			}
+			secret := &corev1.Secret{}
+			secretName := headerRef.ValueSecretRef.Name
+			err := r.Get(ctx, types.NamespacedName{
+				Name: secretName, Namespace: proxy.Namespace,
+			}, secret)
+			if err != nil {
+				if errors.IsNotFound(err) {
+					msg := fmt.Sprintf(
+						"secret %q referenced for header %q not found in namespace %q",
+						secretName, headerRef.HeaderName, proxy.Namespace,
+					)
+					r.recordValidationEvent(
+						proxy,
+						mcpv1alpha1.ConditionReasonHeaderSecretNotFound,
+						msg,
+					)
+					setConfigurationInvalidCondition(
+						proxy,
+						mcpv1alpha1.ConditionReasonHeaderSecretNotFound,
+						msg,
+					)
+					return stderrors.New(msg)
+				}
+				ctxLogger := log.FromContext(ctx)
+				ctxLogger.Error(err, "Failed to fetch secret", "name", secretName, "namespace", proxy.Namespace)
+				genericMsg := fmt.Sprintf("failed to fetch secret %q for header %q", secretName, headerRef.HeaderName)
+				r.recordValidationEvent(proxy, mcpv1alpha1.ConditionReasonHeaderSecretNotFound, genericMsg)
+				setConfigurationInvalidCondition(proxy, mcpv1alpha1.ConditionReasonHeaderSecretNotFound, genericMsg)
+				return stderrors.New(genericMsg)
+			}
+		}
+	}
+
+	return nil
+}
+
 // handleToolConfig handles MCPToolConfig reference for an MCPRemoteProxy
 func (r *MCPRemoteProxyReconciler) handleToolConfig(ctx context.Context, proxy *mcpv1alpha1.MCPRemoteProxy) error {
 	ctxLogger := log.FromContext(ctx)

cmd/thv-operator/controllers/mcpremoteproxy_controller_test.go

@@ -87,7 +87,7 @@ func TestMCPRemoteProxyValidateSpec(t *testing.T) {
 				},
 			},
 			expectError: true,
-			errContains: "remoteURL is required",
+			errContains: "remote URL must not be empty",
 		},
 		// Note: "missing OIDC config" test removed - OIDCConfig is now a required value type
 		// with kubebuilder:validation:Required, so the API server prevents resources without it

cmd/thv-operator/controllers/mcpremoteproxy_reconciler_test.go

@@ -866,6 +866,130 @@ func TestValidateSpecConfigurationConditions(t *testing.T) {
 			expectCondition: mcpv1alpha1.ConditionReasonConfigurationValid,
 			conditionStatus: metav1.ConditionTrue,
 		},
+		{
+			name: "invalid Cedar policy syntax is rejected",
+			proxy: &mcpv1alpha1.MCPRemoteProxy{
+				ObjectMeta: metav1.ObjectMeta{Name: "invalid-cedar-proxy", Namespace: "default"},
+				Spec: mcpv1alpha1.MCPRemoteProxySpec{
+					RemoteURL: "https://mcp.example.com",
+					OIDCConfig: mcpv1alpha1.OIDCConfigRef{
+						Type: mcpv1alpha1.OIDCConfigTypeInline,
+						Inline: &mcpv1alpha1.InlineOIDCConfig{
+							Issuer:   "https://auth.example.com",
+							Audience: "test",
+						},
+					},
+					AuthzConfig: &mcpv1alpha1.AuthzConfigRef{
+						Type: mcpv1alpha1.AuthzConfigTypeInline,
+						Inline: &mcpv1alpha1.InlineAuthzConfig{
+							Policies: []string{"not valid cedar"},
+						},
+					},
+				},
+			},
+			expectError:     true,
+			errContains:     "invalid syntax",
+			expectCondition: mcpv1alpha1.ConditionReasonAuthzPolicySyntaxInvalid,
+			conditionStatus: metav1.ConditionFalse,
+		},
+		{
+			name: "referenced authz ConfigMap not found is rejected",
+			proxy: &mcpv1alpha1.MCPRemoteProxy{
+				ObjectMeta: metav1.ObjectMeta{Name: "missing-configmap-proxy", Namespace: "default"},
+				Spec: mcpv1alpha1.MCPRemoteProxySpec{
+					RemoteURL: "https://mcp.example.com",
+					OIDCConfig: mcpv1alpha1.OIDCConfigRef{
+						Type: mcpv1alpha1.OIDCConfigTypeInline,
+						Inline: &mcpv1alpha1.InlineOIDCConfig{
+							Issuer:   "https://auth.example.com",
+							Audience: "test",
+						},
+					},
+					AuthzConfig: &mcpv1alpha1.AuthzConfigRef{
+						Type: mcpv1alpha1.AuthzConfigTypeConfigMap,
+						ConfigMap: &mcpv1alpha1.ConfigMapAuthzRef{
+							Name: "does-not-exist",
+						},
+					},
+				},
+			},
+			expectError:     true,
+			errContains:     "not found",
+			expectCondition: mcpv1alpha1.ConditionReasonAuthzConfigMapNotFound,
+			conditionStatus: metav1.ConditionFalse,
+		},
+		{
+			name: "referenced header secret not found is rejected",
+			proxy: &mcpv1alpha1.MCPRemoteProxy{
+				ObjectMeta: metav1.ObjectMeta{Name: "missing-header-secret-proxy", Namespace: "default"},
+				Spec: mcpv1alpha1.MCPRemoteProxySpec{
+					RemoteURL: "https://mcp.example.com",
+					OIDCConfig: mcpv1alpha1.OIDCConfigRef{
+						Type: mcpv1alpha1.OIDCConfigTypeInline,
+						Inline: &mcpv1alpha1.InlineOIDCConfig{
+							Issuer:   "https://auth.example.com",
+							Audience: "test",
+						},
+					},
+					HeaderForward: &mcpv1alpha1.HeaderForwardConfig{
+						AddHeadersFromSecret: []mcpv1alpha1.HeaderFromSecret{
+							{
+								HeaderName: "X-API-Key",
+								ValueSecretRef: &mcpv1alpha1.SecretKeyRef{
+									Name: "missing-secret",
+									Key:  "api-key",
+								},
+							},
+						},
+					},
+				},
+			},
+			expectError:     true,
+			errContains:     "not found",
+			expectCondition: mcpv1alpha1.ConditionReasonHeaderSecretNotFound,
+			conditionStatus: metav1.ConditionFalse,
+		},
+		{
+			name: "malformed remote URL is rejected",
+			proxy: &mcpv1alpha1.MCPRemoteProxy{
+				ObjectMeta: metav1.ObjectMeta{Name: "bad-scheme-proxy", Namespace: "default"},
+				Spec: mcpv1alpha1.MCPRemoteProxySpec{
+					RemoteURL: "ftp://bad-scheme.example.com",
+					OIDCConfig: mcpv1alpha1.OIDCConfigRef{
+						Type: mcpv1alpha1.OIDCConfigTypeInline,
+						Inline: &mcpv1alpha1.InlineOIDCConfig{
+							Issuer:   "https://auth.example.com",
+							Audience: "test",
+						},
+					},
+				},
+			},
+			expectError:     true,
+			errContains:     "scheme",
+			expectCondition: mcpv1alpha1.ConditionReasonRemoteURLInvalid,
+			conditionStatus: metav1.ConditionFalse,
+		},
+		{
+			name: "HTTP JWKS URL is rejected",
+			proxy: &mcpv1alpha1.MCPRemoteProxy{
+				ObjectMeta: metav1.ObjectMeta{Name: "http-jwks-proxy", Namespace: "default"},
+				Spec: mcpv1alpha1.MCPRemoteProxySpec{
+					RemoteURL: "https://mcp.example.com",
+					OIDCConfig: mcpv1alpha1.OIDCConfigRef{
+						Type: mcpv1alpha1.OIDCConfigTypeInline,
+						Inline: &mcpv1alpha1.InlineOIDCConfig{
+							Issuer:   "https://auth.example.com",
+							Audience: "test",
+							JWKSURL:  "http://jwks.example.com",
+						},
+					},
+				},
+			},
+			expectError:     true,
+			errContains:     "HTTPS",
+			expectCondition: mcpv1alpha1.ConditionReasonJWKSURLInvalid,
+			conditionStatus: metav1.ConditionFalse,
+		},
 	}
 
 	for _, tt := range tests {

cmd/thv-operator/pkg/validation/cedar_validation.go

@@ -0,0 +1,23 @@
+// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+package validation
+
+import (
+	"fmt"
+
+	cedar "github.com/cedar-policy/cedar-go"
+)
+
+// ValidateCedarPolicies validates the syntax of each Cedar policy string in the
+// provided slice. It returns an error for the first policy that fails to parse,
+// or nil if all policies are valid (including when the slice is empty or nil).
+func ValidateCedarPolicies(policies []string) error {
+	for i, policy := range policies {
+		var p cedar.Policy
+		if err := p.UnmarshalCedar([]byte(policy)); err != nil {
+			return fmt.Errorf("cedar policy at index %d has invalid syntax: %w", i, err)
+		}
+	}
+	return nil
+}