Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions cmd/thv-operator/api/v1beta1/mcpserverentry_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,11 @@ const (
// ConditionTypeMCPServerEntryRemoteURLValidated indicates whether the RemoteURL passes
// format and SSRF safety checks.
ConditionTypeMCPServerEntryRemoteURLValidated = "RemoteURLValidated"

// ConditionTypeMCPServerEntryHeaderSecretRefsValidated indicates whether every Kubernetes
// Secret referenced by spec.headerForward.addHeadersFromSecret exists. Absent when the
// entry declares no header Secret refs.
ConditionTypeMCPServerEntryHeaderSecretRefsValidated = "HeaderSecretRefsValidated"
)

// Condition reasons for MCPServerEntry.
Expand Down Expand Up @@ -146,6 +151,14 @@ const (
// ConditionReasonMCPServerEntryRemoteURLInvalid indicates the RemoteURL is malformed or
// targets a blocked internal/metadata endpoint.
ConditionReasonMCPServerEntryRemoteURLInvalid = ConditionReasonRemoteURLInvalid

// ConditionReasonMCPServerEntryHeaderSecretsValid indicates every referenced header Secret exists.
ConditionReasonMCPServerEntryHeaderSecretsValid = "HeaderSecretsValid"

// ConditionReasonMCPServerEntryHeaderSecretNotFound indicates a Secret referenced by
// spec.headerForward.addHeadersFromSecret was not found in the entry's namespace.
// Reuses the string value used by MCPRemoteProxy for parity.
ConditionReasonMCPServerEntryHeaderSecretNotFound = ConditionReasonHeaderSecretNotFound
)

//+kubebuilder:object:root=true
Expand Down
3 changes: 2 additions & 1 deletion cmd/thv-operator/controllers/mcpremoteproxy_deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
ctrlutil "github.com/stacklok/toolhive/cmd/thv-operator/pkg/controllerutil"
"github.com/stacklok/toolhive/cmd/thv-operator/pkg/runconfig/configmap/checksum"
"github.com/stacklok/toolhive/pkg/container/kubernetes"
"github.com/stacklok/toolhive/pkg/vmcp/headerforward/wirefmt"
)

// deploymentForMCPRemoteProxy returns a MCPRemoteProxy Deployment object
Expand Down Expand Up @@ -278,7 +279,7 @@ func buildHeaderForwardSecretEnvVars(proxy *mcpv1beta1.MCPRemoteProxy) []corev1.
}

// Generate env var name following the TOOLHIVE_SECRET_ pattern
envVarName, _ := ctrlutil.GenerateHeaderForwardSecretEnvVarName(proxy.Name, headerSecret.HeaderName)
envVarName, _ := wirefmt.SecretEnvVarName(proxy.Name, headerSecret.HeaderName)

envVars = append(envVars, corev1.EnvVar{
Name: envVarName,
Expand Down
3 changes: 2 additions & 1 deletion cmd/thv-operator/controllers/mcpremoteproxy_runconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/stacklok/toolhive/cmd/thv-operator/pkg/runconfig/configmap/checksum"
"github.com/stacklok/toolhive/pkg/runner"
transporttypes "github.com/stacklok/toolhive/pkg/transport/types"
"github.com/stacklok/toolhive/pkg/vmcp/headerforward/wirefmt"
)

// ensureRunConfigConfigMap ensures the RunConfig ConfigMap exists and is up to date for MCPRemoteProxy
Expand Down Expand Up @@ -291,7 +292,7 @@ func addHeaderForwardConfigOptions(proxy *mcpv1beta1.MCPRemoteProxy, options *[]
continue
}
// Get the secret identifier (not the full env var name)
_, secretIdentifier := ctrlutil.GenerateHeaderForwardSecretEnvVarName(proxy.Name, headerSecret.HeaderName)
_, secretIdentifier := wirefmt.SecretEnvVarName(proxy.Name, headerSecret.HeaderName)
headerSecrets[headerSecret.HeaderName] = secretIdentifier
}
*options = append(*options, runner.WithHeaderForwardSecrets(headerSecrets))
Expand Down
161 changes: 161 additions & 0 deletions cmd/thv-operator/controllers/mcpserverentry_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package controllers
import (
"context"
"fmt"
"strings"
"time"

corev1 "k8s.io/api/core/v1"
Expand All @@ -32,6 +33,10 @@ const (

// mcpServerEntryCABundleRefField is the field index key for CABundleRef ConfigMap lookups.
mcpServerEntryCABundleRefField = "spec.caBundleRef.configMapRef.name"

// mcpServerEntryHeaderSecretRefField is the field index key for
// spec.headerForward.addHeadersFromSecret[*].valueSecretRef.name lookups.
mcpServerEntryHeaderSecretRefField = "spec.headerForward.addHeadersFromSecret.valueSecretRef.name"
)

// MCPServerEntryReconciler reconciles a MCPServerEntry object.
Expand All @@ -46,6 +51,7 @@ type MCPServerEntryReconciler struct {
// +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=mcpgroups,verbs=get;list;watch
// +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=mcpexternalauthconfigs,verbs=get;list;watch
// +kubebuilder:rbac:groups="",resources=configmaps,verbs=get;list;watch
// +kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch

// Reconcile validates referenced resources and updates status conditions.
func (r *MCPServerEntryReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
Expand Down Expand Up @@ -85,6 +91,12 @@ func (r *MCPServerEntryReconciler) Reconcile(ctx context.Context, req ctrl.Reque
}
allValid = valid && allValid

valid, err = r.validateHeaderForwardSecretRefs(ctx, entry)
if err != nil {
return ctrl.Result{}, err
}
allValid = valid && allValid

// Compute overall phase and Valid condition
r.updateOverallStatus(entry, allValid)

Expand Down Expand Up @@ -137,6 +149,38 @@ func (r *MCPServerEntryReconciler) SetupWithManager(mgr ctrl.Manager) error {
mcpServerEntryCABundleRefField, err)
}

// Set up field index for headerForward Secret refs. Used by the Secret
// watch below so a Secret create/update/delete reconciles every entry that
// references it — without this index we would have to list all entries on
// every Secret event.
if err := mgr.GetFieldIndexer().IndexField(
context.Background(),
&mcpv1beta1.MCPServerEntry{},
mcpServerEntryHeaderSecretRefField,
func(obj client.Object) []string {
entry := obj.(*mcpv1beta1.MCPServerEntry)
if entry.Spec.HeaderForward == nil {
return nil
}
seen := make(map[string]struct{}, len(entry.Spec.HeaderForward.AddHeadersFromSecret))
names := make([]string, 0, len(entry.Spec.HeaderForward.AddHeadersFromSecret))
for _, ref := range entry.Spec.HeaderForward.AddHeadersFromSecret {
if ref.ValueSecretRef == nil {
continue
}
if _, ok := seen[ref.ValueSecretRef.Name]; ok {
continue
}
seen[ref.ValueSecretRef.Name] = struct{}{}
names = append(names, ref.ValueSecretRef.Name)
}
return names
},
); err != nil {
return fmt.Errorf("unable to create field index for MCPServerEntry %s: %w",
mcpServerEntryHeaderSecretRefField, err)
}

return ctrl.NewControllerManagedBy(mgr).
For(&mcpv1beta1.MCPServerEntry{}).
Watches(
Expand All @@ -151,6 +195,10 @@ func (r *MCPServerEntryReconciler) SetupWithManager(mgr ctrl.Manager) error {
&corev1.ConfigMap{},
handler.EnqueueRequestsFromMapFunc(r.findEntriesForConfigMap),
).
Watches(
&corev1.Secret{},
handler.EnqueueRequestsFromMapFunc(r.findEntriesForHeaderSecret),
).
Complete(r)
}

Expand Down Expand Up @@ -300,6 +348,81 @@ func (r *MCPServerEntryReconciler) validateCABundleRef(
return true, nil
}

// validateHeaderForwardSecretRefs checks that every Secret referenced by
// spec.headerForward.addHeadersFromSecret exists AND that the named key
// inside it is present in the entry's namespace. Skipped (condition removed)
// when no Secret-backed headers are declared.
//
// All ref-level failures are aggregated into one condition message — a user
// fixing two missing secrets at once should see both surface together rather
// than one-per-reconcile.
//
// Returns (valid, error) where a non-nil error means a transient API
// failure that should be requeued. Key-not-found and Secret-not-found are
// surfaced via the condition (valid=false, err=nil).
func (r *MCPServerEntryReconciler) validateHeaderForwardSecretRefs(
ctx context.Context,
entry *mcpv1beta1.MCPServerEntry,
) (bool, error) {
ctxLogger := log.FromContext(ctx)

if entry.Spec.HeaderForward == nil || len(entry.Spec.HeaderForward.AddHeadersFromSecret) == 0 {
meta.RemoveStatusCondition(&entry.Status.Conditions,
mcpv1beta1.ConditionTypeMCPServerEntryHeaderSecretRefsValidated)
return true, nil
}

var failures []string
for _, ref := range entry.Spec.HeaderForward.AddHeadersFromSecret {
if ref.ValueSecretRef == nil {
continue
}
secret := &corev1.Secret{}
secretKey := types.NamespacedName{
Namespace: entry.Namespace,
Name: ref.ValueSecretRef.Name,
}
if err := r.Get(ctx, secretKey, secret); err != nil {
if errors.IsNotFound(err) {
failures = append(failures, fmt.Sprintf(
"Secret %q referenced for header %q not found",
ref.ValueSecretRef.Name, ref.HeaderName))
continue
}
// Transient API failure (rate-limit, network, etc.) — requeue
// rather than condition-flip. The next reconcile will re-validate.
ctxLogger.Error(err, "Failed to get referenced header Secret",
"secret", ref.ValueSecretRef.Name, "header", ref.HeaderName)
return false, err
}
if _, ok := secret.Data[ref.ValueSecretRef.Key]; !ok {
failures = append(failures, fmt.Sprintf(
"Secret %q has no key %q referenced for header %q",
ref.ValueSecretRef.Name, ref.ValueSecretRef.Key, ref.HeaderName))
}
}

if len(failures) > 0 {
meta.SetStatusCondition(&entry.Status.Conditions, metav1.Condition{
Type: mcpv1beta1.ConditionTypeMCPServerEntryHeaderSecretRefsValidated,
Status: metav1.ConditionFalse,
Reason: mcpv1beta1.ConditionReasonMCPServerEntryHeaderSecretNotFound,
Message: strings.Join(failures, "; "),
ObservedGeneration: entry.Generation,
})
return false, nil
}

meta.SetStatusCondition(&entry.Status.Conditions, metav1.Condition{
Type: mcpv1beta1.ConditionTypeMCPServerEntryHeaderSecretRefsValidated,
Status: metav1.ConditionTrue,
Reason: mcpv1beta1.ConditionReasonMCPServerEntryHeaderSecretsValid,
Message: "All referenced header Secrets exist with required keys",
ObservedGeneration: entry.Generation,
})
return true, nil
}

// validateRemoteURL checks that the RemoteURL is well-formed and does not target
// a blocked internal or metadata endpoint (SSRF protection).
func (*MCPServerEntryReconciler) validateRemoteURL(
Expand Down Expand Up @@ -421,6 +544,44 @@ func (r *MCPServerEntryReconciler) findEntriesForGroup(
return requests
}

// findEntriesForHeaderSecret maps Secret changes to MCPServerEntry reconcile
// requests for entries that reference the Secret in
// spec.headerForward.addHeadersFromSecret. This ensures that creating the
// referenced Secret (or rotating its contents) flips the validation condition
// from false → true within one reconcile, instead of waiting for the resync.
func (r *MCPServerEntryReconciler) findEntriesForHeaderSecret(
ctx context.Context,
obj client.Object,
) []reconcile.Request {
ctxLogger := log.FromContext(ctx)

secret, ok := obj.(*corev1.Secret)
if !ok {
ctxLogger.Error(nil, "Object is not a Secret", "object", obj.GetName())
return nil
}

entryList := &mcpv1beta1.MCPServerEntryList{}
if err := r.List(ctx, entryList,
client.InNamespace(secret.Namespace),
client.MatchingFields{mcpServerEntryHeaderSecretRefField: secret.Name},
); err != nil {
ctxLogger.Error(err, "Failed to list MCPServerEntries for header Secret change")
return nil
}

requests := make([]reconcile.Request, len(entryList.Items))
for i, entry := range entryList.Items {
requests[i] = reconcile.Request{
NamespacedName: types.NamespacedName{
Namespace: entry.Namespace,
Name: entry.Name,
},
}
}
return requests
}

// findEntriesForConfigMap maps ConfigMap changes to MCPServerEntry reconcile requests
// for entries that reference the ConfigMap as a CA bundle.
func (r *MCPServerEntryReconciler) findEntriesForConfigMap(
Expand Down
Loading
Loading