Skip to content
Draft
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
2 changes: 1 addition & 1 deletion cmd/thv-operator/Taskfile.yml
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ tasks:
platforms: [windows]
ignore_error: true # Windows has no mkdir -p, so just ignore error if it exists
- go install sigs.k8s.io/controller-tools/cmd/controller-gen@v0.17.3
- $(go env GOPATH)/bin/controller-gen object:headerFile="hack/boilerplate.go.txt" paths="./cmd/thv-operator/..." paths="./pkg/json/..." paths="./pkg/vmcp/config/..." paths="./pkg/vmcp/auth/types/..." paths="./pkg/telemetry/..." paths="./pkg/audit/..."
- $(go env GOPATH)/bin/controller-gen object:headerFile="hack/boilerplate.go.txt" paths="./cmd/thv-operator/..." paths="./pkg/json/..." paths="./pkg/vmcp" paths="./pkg/vmcp/config/..." paths="./pkg/vmcp/auth/types/..." paths="./pkg/telemetry/..." paths="./pkg/audit/..."

operator-manifests:
desc: Generate WebhookConfiguration, ClusterRole and CustomResourceDefinition objects
Expand Down
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
60 changes: 60 additions & 0 deletions cmd/thv-operator/controllers/mcpserverentry_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,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 @@ -300,6 +306,60 @@ func (r *MCPServerEntryReconciler) validateCABundleRef(
return true, nil
}

// validateHeaderForwardSecretRefs checks that every Secret referenced by
// spec.headerForward.addHeadersFromSecret exists in the entry's namespace.
// Skipped (condition removed) when no Secret-backed headers are declared.
// Returns (valid, error) where a non-nil error means a transient API failure
// that should be requeued.
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
}

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) {
meta.SetStatusCondition(&entry.Status.Conditions, metav1.Condition{
Type: mcpv1beta1.ConditionTypeMCPServerEntryHeaderSecretRefsValidated,
Status: metav1.ConditionFalse,
Reason: mcpv1beta1.ConditionReasonMCPServerEntryHeaderSecretNotFound,
Message: fmt.Sprintf("Secret %q referenced for header %q not found",
ref.ValueSecretRef.Name, ref.HeaderName),
ObservedGeneration: entry.Generation,
})
return false, nil
}
ctxLogger.Error(err, "Failed to get referenced header Secret",
"secret", ref.ValueSecretRef.Name, "header", ref.HeaderName)
return false, err
}
}

meta.SetStatusCondition(&entry.Status.Conditions, metav1.Condition{
Type: mcpv1beta1.ConditionTypeMCPServerEntryHeaderSecretRefsValidated,
Status: metav1.ConditionTrue,
Reason: mcpv1beta1.ConditionReasonMCPServerEntryHeaderSecretsValid,
Message: "All referenced header Secrets exist",
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
75 changes: 75 additions & 0 deletions cmd/thv-operator/controllers/mcpserverentry_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,81 @@ func TestMCPServerEntryReconciler_Reconcile(t *testing.T) {
{mcpv1beta1.ConditionTypeMCPServerEntryValid, metav1.ConditionTrue, mcpv1beta1.ConditionReasonMCPServerEntryValid},
},
},
{
name: "headerForward plaintext only - no secret validation needed",
entry: func() *mcpv1beta1.MCPServerEntry {
e := newMCPServerEntry(testEntryGroupRef, nil, nil)
e.Spec.HeaderForward = &mcpv1beta1.HeaderForwardConfig{
AddPlaintextHeaders: map[string]string{"X-MCP-Toolsets": "projects,issues"},
}
return e
}(),
objects: []client.Object{newMCPGroup(mcpv1beta1.MCPGroupPhaseReady)},
wantPhase: mcpv1beta1.MCPServerEntryPhaseValid,
conditions: []struct {
condType string
status metav1.ConditionStatus
reason string
}{
{mcpv1beta1.ConditionTypeMCPServerEntryValid, metav1.ConditionTrue, mcpv1beta1.ConditionReasonMCPServerEntryValid},
},
},
{
name: "headerForward secret ref exists",
entry: func() *mcpv1beta1.MCPServerEntry {
e := newMCPServerEntry(testEntryGroupRef, nil, nil)
e.Spec.HeaderForward = &mcpv1beta1.HeaderForwardConfig{
AddHeadersFromSecret: []mcpv1beta1.HeaderFromSecret{
{
HeaderName: "X-API-Key",
ValueSecretRef: &mcpv1beta1.SecretKeyRef{Name: "api-key-secret", Key: "token"},
},
},
}
return e
}(),
objects: []client.Object{
newMCPGroup(mcpv1beta1.MCPGroupPhaseReady),
&corev1.Secret{
ObjectMeta: metav1.ObjectMeta{Name: "api-key-secret", Namespace: testEntryNS},
Data: map[string][]byte{"token": []byte("secret-value")},
},
},
wantPhase: mcpv1beta1.MCPServerEntryPhaseValid,
conditions: []struct {
condType string
status metav1.ConditionStatus
reason string
}{
{mcpv1beta1.ConditionTypeMCPServerEntryHeaderSecretRefsValidated, metav1.ConditionTrue, mcpv1beta1.ConditionReasonMCPServerEntryHeaderSecretsValid},
{mcpv1beta1.ConditionTypeMCPServerEntryValid, metav1.ConditionTrue, mcpv1beta1.ConditionReasonMCPServerEntryValid},
},
},
{
name: "headerForward secret ref missing flips entry to Failed",
entry: func() *mcpv1beta1.MCPServerEntry {
e := newMCPServerEntry(testEntryGroupRef, nil, nil)
e.Spec.HeaderForward = &mcpv1beta1.HeaderForwardConfig{
AddHeadersFromSecret: []mcpv1beta1.HeaderFromSecret{
{
HeaderName: "X-API-Key",
ValueSecretRef: &mcpv1beta1.SecretKeyRef{Name: "missing-secret", Key: "token"},
},
},
}
return e
}(),
objects: []client.Object{newMCPGroup(mcpv1beta1.MCPGroupPhaseReady)},
wantPhase: mcpv1beta1.MCPServerEntryPhaseFailed,
conditions: []struct {
condType string
status metav1.ConditionStatus
reason string
}{
{mcpv1beta1.ConditionTypeMCPServerEntryHeaderSecretRefsValidated, metav1.ConditionFalse, mcpv1beta1.ConditionReasonMCPServerEntryHeaderSecretNotFound},
{mcpv1beta1.ConditionTypeMCPServerEntryValid, metav1.ConditionFalse, mcpv1beta1.ConditionReasonMCPServerEntryInvalid},
},
},
}

for _, tt := range tests {
Expand Down
13 changes: 12 additions & 1 deletion cmd/thv-operator/controllers/virtualmcpserver_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -2112,12 +2112,19 @@ func injectSubjectProviderIfNeeded(
// convertBackendsToStaticBackends converts Backend objects to StaticBackendConfig for ConfigMap embedding.
// Preserves metadata and uses transport types from workload Specs.
// Logs warnings when backends are skipped due to missing URL or transport information.
// caBundlePathMap maps backend names to their CA bundle mount paths (populated for MCPServerEntry backends).
//
// caBundlePathMap maps backend names to their CA bundle mount paths (populated for
// MCPServerEntry backends with caBundleRef).
// headerForwardMap maps backend names to per-backend HeaderForwardConfig (populated
// for MCPServerEntry backends with headerForward). Plaintext values are embedded
// as-is; secret-backed headers carry only env-var identifiers that the vMCP runtime
// resolves via secrets.EnvironmentProvider (TOOLHIVE_SECRET_<ident>).
func convertBackendsToStaticBackends(
ctx context.Context,
backends []vmcptypes.Backend,
transportMap map[string]string,
caBundlePathMap map[string]string,
headerForwardMap map[string]*vmcptypes.HeaderForwardConfig,
) []vmcpconfig.StaticBackendConfig {
logger := log.FromContext(ctx)
static := make([]vmcpconfig.StaticBackendConfig, 0, len(backends))
Expand Down Expand Up @@ -2146,6 +2153,10 @@ func convertBackendsToStaticBackends(
cfg.CABundlePath = caBundlePath
}

if hf, ok := headerForwardMap[backend.Name]; ok && hf != nil {
cfg.HeaderForward = hf
}

static = append(static, cfg)
}
return static
Expand Down
75 changes: 75 additions & 0 deletions cmd/thv-operator/controllers/virtualmcpserver_deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,16 @@ func (r *VirtualMCPServerReconciler) buildEnvVarsForVmcp(
// Mount outgoing auth secrets
env = append(env, r.buildOutgoingAuthEnvVars(ctx, vmcp, typedWorkloads)...)

// Mount per-backend header-forward secrets for MCPServerEntry backends.
// Each Secret referenced by MCPServerEntry.spec.headerForward.addHeadersFromSecret
// is exposed as TOOLHIVE_SECRET_<IDENT> via valueFrom.secretKeyRef. The vMCP
// runtime reads these through secrets.EnvironmentProvider at request time.
headerEnv, err := r.buildHeaderForwardEnvVarsForEntries(ctx, vmcp.Namespace, typedWorkloads)
if err != nil {
return nil, fmt.Errorf("failed to build header forward env vars: %w", err)
}
env = append(env, headerEnv...)

// Always mount HMAC secret for session token binding.
env = append(env, r.buildHMACSecretEnvVar(vmcp))

Expand Down Expand Up @@ -482,6 +492,71 @@ func (r *VirtualMCPServerReconciler) buildOutgoingAuthEnvVars(
return env
}

// buildHeaderForwardEnvVarsForEntries iterates entry-type workloads and emits one
// env var per (entry, header) declared in spec.headerForward.addHeadersFromSecret.
// Each env var uses valueFrom.secretKeyRef so the Secret value is injected by
// Kubernetes at pod start and never stored in the vMCP ConfigMap.
//
// The env var name is TOOLHIVE_SECRET_<IDENT> where <IDENT> is generated by
// ctrlutil.GenerateHeaderForwardSecretEnvVarName(entry.Name, headerName). Scoping
// by entry name prevents collisions when multiple entries in the group declare
// the same header.
//
// Returns (nil, nil) when no entries declare Secret-backed headers — this is the
// common case and produces no env var churn on the Deployment spec.
func (r *VirtualMCPServerReconciler) buildHeaderForwardEnvVarsForEntries(
ctx context.Context,
namespace string,
typedWorkloads []workloads.TypedWorkload,
) ([]corev1.EnvVar, error) {
// Early return if no MCPServerEntry workloads to avoid unnecessary API calls.
hasEntries := false
for _, workload := range typedWorkloads {
if workload.Type == workloads.WorkloadTypeMCPServerEntry {
hasEntries = true
break
}
}
if !hasEntries {
return nil, nil
}

entryMap, err := r.listMCPServerEntriesAsMap(ctx, namespace)
if err != nil {
return nil, fmt.Errorf("failed to list MCPServerEntries: %w", err)
}

var envVars []corev1.EnvVar
for _, workload := range typedWorkloads {
if workload.Type != workloads.WorkloadTypeMCPServerEntry {
continue
}
entry, found := entryMap[workload.Name]
if !found || entry.Spec.HeaderForward == nil {
continue
}
for _, ref := range entry.Spec.HeaderForward.AddHeadersFromSecret {
if ref.ValueSecretRef == nil {
continue
}
envVarName, _ := ctrlutil.GenerateHeaderForwardSecretEnvVarName(entry.Name, ref.HeaderName)
envVars = append(envVars, corev1.EnvVar{
Name: envVarName,
ValueFrom: &corev1.EnvVarSource{
SecretKeyRef: &corev1.SecretKeySelector{
LocalObjectReference: corev1.LocalObjectReference{
Name: ref.ValueSecretRef.Name,
},
Key: ref.ValueSecretRef.Key,
},
},
})
}
}

return envVars, nil
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

buildHeaderForwardEnvVarsForEntries returns envVars without sorting, but the two sibling functions in this same file (discoverExternalAuthConfigSecrets at line 622, discoverInlineExternalAuthConfigSecrets at line 668) both sort by name and include a comment explaining why: containerNeedsUpdate uses reflect.DeepEqual on the env list, and unsorted env vars produce a continuous deployment update loop. PR #4783 fixed exactly this bug for the sibling functions.

With ≥2 entries declaring headerForward.addHeadersFromSecret, the order that listMCPServerEntriesAsMap + inner slice iteration produces isn't guaranteed stable across reconciles, so this function can trigger the same rollout loop.

Suggest adding the same sort before return envVars, nil, mirroring the existing comment:

sort.Slice(envVars, func(i, j int) bool {
    return envVars[i].Name < envVars[j].Name
})

}

// discoverExternalAuthConfigSecrets discovers ExternalAuthConfigs from workloads in the group
// and returns environment variables for their client secrets. This is used for discovered mode.
func (r *VirtualMCPServerReconciler) discoverExternalAuthConfigSecrets(
Expand Down
Loading
Loading