Skip to content

Commit 291104f

Browse files
committed
Report deprecated inbound grant fields
Operators need a visible migration signal before legacy grant fields can be removed safely. Record deprecated field paths during normalization and surface them as status conditions without changing the effective authorization policy. Refs #6200
1 parent b2adbba commit 291104f

9 files changed

Lines changed: 439 additions & 22 deletions

cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1814,6 +1814,16 @@ const (
18141814
// declaration so a missing identity source is visible in
18151815
// `kubectl describe` instead of only in proxyrunner logs.
18161816
ConditionTypeIdentitySynthesized = "IdentitySynthesized"
1817+
1818+
// ConditionTypeDeprecatedInboundGrantConfiguration reports whether released
1819+
// legacy inbound grant fields remain populated.
1820+
ConditionTypeDeprecatedInboundGrantConfiguration = "DeprecatedInboundGrantConfiguration"
1821+
)
1822+
1823+
// Condition reasons for the deprecated inbound grant advisory.
1824+
const (
1825+
ConditionReasonLegacyInboundGrantFields = "LegacyInboundGrantFields"
1826+
ConditionReasonCanonicalInboundGrantConfiguration = "CanonicalInboundGrantConfiguration"
18171827
)
18181828

18191829
// Condition reasons for ConditionTypeIdentitySynthesized.

cmd/thv-operator/api/v1beta1/virtualmcpserver_types.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,10 @@ const (
356356

357357
// ConditionTypeVirtualMCPServerTelemetryConfigRefValidated indicates whether the TelemetryConfigRef is valid
358358
ConditionTypeVirtualMCPServerTelemetryConfigRefValidated = "TelemetryConfigRefValidated"
359+
360+
// ConditionTypeVirtualMCPServerDeprecatedInboundGrantConfiguration reports
361+
// whether inline auth uses released legacy inbound grant fields.
362+
ConditionTypeVirtualMCPServerDeprecatedInboundGrantConfiguration = "DeprecatedInboundGrantConfiguration"
359363
)
360364

361365
// Condition reasons for VirtualMCPServer
@@ -423,6 +427,14 @@ const (
423427
// ConditionReasonAuthServerConfigInvalid indicates the AuthServerConfig is invalid
424428
ConditionReasonAuthServerConfigInvalid = "AuthServerConfigInvalid"
425429

430+
// ConditionReasonVirtualMCPServerLegacyInboundGrantFields indicates that
431+
// inline auth uses released legacy inbound grant fields.
432+
ConditionReasonVirtualMCPServerLegacyInboundGrantFields = "LegacyInboundGrantFields"
433+
434+
// ConditionReasonVirtualMCPServerCanonicalInboundGrantConfiguration indicates
435+
// that inline auth uses only canonical inbound grant configuration.
436+
ConditionReasonVirtualMCPServerCanonicalInboundGrantConfiguration = "CanonicalInboundGrantConfiguration"
437+
426438
// ConditionReasonAuthzRequiresUpstream indicates that authorization policies are
427439
// configured but no upstream IDP is available to source claims from. Without an
428440
// upstream, Cedar evaluates against the ToolHive-issued AS token, whose claim

cmd/thv-operator/controllers/mcpexternalauthconfig_controller.go

Lines changed: 131 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"context"
88
stderrors "errors"
99
"fmt"
10+
"strings"
1011
"time"
1112

1213
corev1 "k8s.io/api/core/v1"
@@ -36,8 +37,17 @@ const (
3637
// authServerRefKindMCPExternalAuthConfig is the Kind value on a TypedLocalObjectReference
3738
// that identifies the ref as pointing to an MCPExternalAuthConfig resource.
3839
authServerRefKindMCPExternalAuthConfig = "MCPExternalAuthConfig"
40+
41+
// inboundGrantDeprecationEventReason is emitted when released legacy grant
42+
// fields transition from absent to populated.
43+
inboundGrantDeprecationEventReason = "InboundGrantsLegacyFieldsDeprecated"
3944
)
4045

46+
type deprecatedInboundGrantField struct {
47+
path string
48+
replacement string
49+
}
50+
4151
// MCPExternalAuthConfigReconciler reconciles a MCPExternalAuthConfig object
4252
type MCPExternalAuthConfigReconciler struct {
4353
client.Client
@@ -108,9 +118,13 @@ func (r *MCPExternalAuthConfigReconciler) Reconcile(ctx context.Context, req ctr
108118
// in place, so the Warning fires only when entering the invalid state.
109119
wasInvalid := conditionStatusIs(externalAuthConfig.Status.Conditions,
110120
mcpv1beta1.ConditionTypeValid, metav1.ConditionFalse)
121+
wasDeprecated := conditionStatusIs(externalAuthConfig.Status.Conditions,
122+
mcpv1beta1.ConditionTypeDeprecatedInboundGrantConfiguration, metav1.ConditionTrue)
111123
updateErr := ctrlutil.MutateAndPatchStatus(ctx, r.Client, externalAuthConfig,
112124
func(c *mcpv1beta1.MCPExternalAuthConfig) {
113125
r.applyIdentitySynthesizedCondition(c)
126+
r.applyDeprecatedInboundGrantCondition(c)
127+
c.Status.ObservedGeneration = c.Generation
114128
meta.SetStatusCondition(&c.Status.Conditions, metav1.Condition{
115129
Type: mcpv1beta1.ConditionTypeValid,
116130
Status: metav1.ConditionFalse,
@@ -121,14 +135,18 @@ func (r *MCPExternalAuthConfigReconciler) Reconcile(ctx context.Context, req ctr
121135
})
122136
if updateErr != nil {
123137
logger.Error(updateErr, "Failed to update status after validation error")
138+
return ctrl.Result{}, updateErr
124139
}
125140
// Emit the Warning only on the transition into the invalid state, and
126141
// only once the condition persisted, so a failing status write does not
127142
// re-fire the event every reconcile.
128-
if !wasInvalid && updateErr == nil {
143+
if !wasInvalid {
129144
emitConfigEvent(r.Recorder, externalAuthConfig, corev1.EventTypeWarning,
130145
eventReasonConfigInvalid, eventActionValidate, "spec validation failed: %s", err.Error())
131146
}
147+
desiredDeprecated := conditionStatusIs(externalAuthConfig.Status.Conditions,
148+
mcpv1beta1.ConditionTypeDeprecatedInboundGrantConfiguration, metav1.ConditionTrue)
149+
emitInboundGrantDeprecationEvent(r.Recorder, externalAuthConfig, wasDeprecated, desiredDeprecated)
132150
return ctrl.Result{}, nil // Don't requeue on validation errors - user must fix spec
133151
}
134152

@@ -158,6 +176,99 @@ func (r *MCPExternalAuthConfigReconciler) Reconcile(ctx context.Context, req ctr
158176
return r.updateSteadyStateStatus(ctx, externalAuthConfig)
159177
}
160178

179+
func deprecatedInboundGrantFields(cfg *mcpv1beta1.EmbeddedAuthServerConfig, root string) []deprecatedInboundGrantField {
180+
if cfg == nil {
181+
return nil
182+
}
183+
fields := make([]deprecatedInboundGrantField, 0)
184+
if len(cfg.DelegateClients) > 0 {
185+
fields = append(fields, deprecatedInboundGrantField{
186+
path: root + ".delegateClients", replacement: root + ".inboundGrants.tokenExchange.delegateClients",
187+
})
188+
}
189+
for i, issuer := range cfg.TrustedIssuers {
190+
issuerPath := fmt.Sprintf("%s.trustedIssuers[%d]", root, i)
191+
tokenExchangeReplacement := root + ".inboundGrants.tokenExchange.issuerPolicies"
192+
legacyTokenExchangeFields := []struct {
193+
populated bool
194+
name string
195+
}{
196+
{issuer.ExpectedAudience != "", "expectedAudience"},
197+
{issuer.ActorClaim != "", "actorClaim"},
198+
{len(issuer.AllowedActors) > 0, "allowedActors"},
199+
{issuer.ActorMatcher != "", "actorMatcher"},
200+
{len(issuer.AllowedDelegateClients) > 0, "allowedDelegateClients"},
201+
{issuer.AllowMayAct, "allowMayAct"},
202+
}
203+
for _, field := range legacyTokenExchangeFields {
204+
if field.populated {
205+
fields = append(fields, deprecatedInboundGrantField{
206+
path: issuerPath + "." + field.name, replacement: tokenExchangeReplacement,
207+
})
208+
}
209+
}
210+
if issuer.JWTBearerGrant != nil {
211+
fields = append(fields, deprecatedInboundGrantField{
212+
path: issuerPath + ".jwtBearerGrant",
213+
replacement: root + ".inboundGrants.jwtBearer.issuerPolicies",
214+
})
215+
}
216+
}
217+
return fields
218+
}
219+
220+
func deprecatedInboundGrantMessage(fields []deprecatedInboundGrantField) string {
221+
paths := make([]string, len(fields))
222+
for i, field := range fields {
223+
paths[i] = field.path + " -> " + field.replacement
224+
}
225+
return "Deprecated inbound grant fields are configured: " + strings.Join(paths, ", ")
226+
}
227+
228+
func setDeprecatedInboundGrantCondition(
229+
conditions *[]metav1.Condition,
230+
generation int64,
231+
fields []deprecatedInboundGrantField,
232+
conditionType, trueReason, falseReason string,
233+
) {
234+
condition := metav1.Condition{
235+
Type: conditionType, ObservedGeneration: generation,
236+
Status: metav1.ConditionFalse, Reason: falseReason,
237+
Message: "Only canonical inbound grant configuration is populated",
238+
}
239+
if len(fields) > 0 {
240+
condition.Status = metav1.ConditionTrue
241+
condition.Reason = trueReason
242+
condition.Message = deprecatedInboundGrantMessage(fields)
243+
}
244+
meta.SetStatusCondition(conditions, condition)
245+
}
246+
247+
func (*MCPExternalAuthConfigReconciler) applyDeprecatedInboundGrantCondition(
248+
cfg *mcpv1beta1.MCPExternalAuthConfig,
249+
) {
250+
setDeprecatedInboundGrantCondition(
251+
&cfg.Status.Conditions,
252+
cfg.Generation,
253+
deprecatedInboundGrantFields(cfg.Spec.EmbeddedAuthServer, "spec.embeddedAuthServer"),
254+
mcpv1beta1.ConditionTypeDeprecatedInboundGrantConfiguration,
255+
mcpv1beta1.ConditionReasonLegacyInboundGrantFields,
256+
mcpv1beta1.ConditionReasonCanonicalInboundGrantConfiguration,
257+
)
258+
}
259+
260+
func emitInboundGrantDeprecationEvent(
261+
recorder events.EventRecorder,
262+
obj runtime.Object,
263+
wasDeprecated, desiredDeprecated bool,
264+
) {
265+
if recorder == nil || wasDeprecated || !desiredDeprecated {
266+
return
267+
}
268+
recorder.Eventf(obj, nil, corev1.EventTypeWarning, inboundGrantDeprecationEventReason, "MigrateInboundGrants",
269+
"Released legacy inbound grant fields are deprecated; see status condition for canonical replacement paths")
270+
}
271+
161272
// setValidTrueAndSynthesized stamps ConditionTypeValid=True and refreshes the
162273
// IdentitySynthesized advisory on the supplied object. It is callable inside a
163274
// MutateAndPatchStatus closure: applyIdentitySynthesizedCondition is idempotent
@@ -166,6 +277,8 @@ func (r *MCPExternalAuthConfigReconciler) Reconcile(ctx context.Context, req ctr
166277
// skips.
167278
func (r *MCPExternalAuthConfigReconciler) setValidTrueAndSynthesized(c *mcpv1beta1.MCPExternalAuthConfig) {
168279
r.applyIdentitySynthesizedCondition(c)
280+
r.applyDeprecatedInboundGrantCondition(c)
281+
c.Status.ObservedGeneration = c.Generation
169282
meta.SetStatusCondition(&c.Status.Conditions, metav1.Condition{
170283
Type: mcpv1beta1.ConditionTypeValid,
171284
Status: metav1.ConditionTrue,
@@ -292,13 +405,17 @@ func (r *MCPExternalAuthConfigReconciler) setInvalid(
292405
// the Warning fires only when entering the invalid state.
293406
wasInvalid := conditionStatusIs(fresh.Status.Conditions,
294407
mcpv1beta1.ConditionTypeValid, metav1.ConditionFalse)
408+
wasDeprecated := conditionStatusIs(fresh.Status.Conditions,
409+
mcpv1beta1.ConditionTypeDeprecatedInboundGrantConfiguration, metav1.ConditionTrue)
295410
if patchErr := ctrlutil.MutateAndPatchStatus(ctx, r.Client, fresh, func(c *mcpv1beta1.MCPExternalAuthConfig) {
296411
// applyIdentitySynthesizedCondition is idempotent on the same spec;
297412
// re-applying it inside the closure folds the advisory transition into
298413
// the same patch as the Valid=False write below. See
299414
// TestMCPExternalAuthConfigReconciler_IdentitySynthesizedTransitionsOnValidationFailure
300415
// for the related validation-path regression guard.
301416
r.applyIdentitySynthesizedCondition(c)
417+
r.applyDeprecatedInboundGrantCondition(c)
418+
c.Status.ObservedGeneration = c.Generation
302419
meta.SetStatusCondition(&c.Status.Conditions, metav1.Condition{
303420
Type: mcpv1beta1.ConditionTypeValid,
304421
Status: metav1.ConditionFalse,
@@ -313,6 +430,9 @@ func (r *MCPExternalAuthConfigReconciler) setInvalid(
313430
emitConfigEvent(r.Recorder, fresh, corev1.EventTypeWarning,
314431
eventReasonConfigInvalid, eventActionValidate, "spec validation failed: %s", err.Error())
315432
}
433+
desiredDeprecated := conditionStatusIs(fresh.Status.Conditions,
434+
mcpv1beta1.ConditionTypeDeprecatedInboundGrantConfiguration, metav1.ConditionTrue)
435+
emitInboundGrantDeprecationEvent(r.Recorder, fresh, wasDeprecated, desiredDeprecated)
316436
return nil
317437
}
318438

@@ -331,6 +451,8 @@ func (r *MCPExternalAuthConfigReconciler) handleConfigHashChange(
331451
// place, so a single Normal event fires on the False->True transition.
332452
wasInvalid := conditionStatusIs(externalAuthConfig.Status.Conditions,
333453
mcpv1beta1.ConditionTypeValid, metav1.ConditionFalse)
454+
wasDeprecated := conditionStatusIs(externalAuthConfig.Status.Conditions,
455+
mcpv1beta1.ConditionTypeDeprecatedInboundGrantConfiguration, metav1.ConditionTrue)
334456

335457
// Single status patch covering the hash-change success path: the new hash
336458
// and generation, and the Valid=True / IdentitySynthesized conditions. All
@@ -346,6 +468,9 @@ func (r *MCPExternalAuthConfigReconciler) handleConfigHashChange(
346468
return ctrl.Result{}, err
347469
}
348470
emitConfigRecoveryEvent(r.Recorder, externalAuthConfig, wasInvalid)
471+
desiredDeprecated := conditionStatusIs(externalAuthConfig.Status.Conditions,
472+
mcpv1beta1.ConditionTypeDeprecatedInboundGrantConfiguration, metav1.ConditionTrue)
473+
emitInboundGrantDeprecationEvent(r.Recorder, externalAuthConfig, wasDeprecated, desiredDeprecated)
349474

350475
return ctrl.Result{}, nil
351476
}
@@ -587,6 +712,8 @@ func (r *MCPExternalAuthConfigReconciler) updateSteadyStateStatus(
587712
// place, so a single Normal event fires on the False->True transition.
588713
wasInvalid := conditionStatusIs(externalAuthConfig.Status.Conditions,
589714
mcpv1beta1.ConditionTypeValid, metav1.ConditionFalse)
715+
wasDeprecated := conditionStatusIs(externalAuthConfig.Status.Conditions,
716+
mcpv1beta1.ConditionTypeDeprecatedInboundGrantConfiguration, metav1.ConditionTrue)
590717

591718
if err := ctrlutil.MutateAndPatchStatus(ctx, r.Client, externalAuthConfig,
592719
func(c *mcpv1beta1.MCPExternalAuthConfig) {
@@ -596,6 +723,9 @@ func (r *MCPExternalAuthConfigReconciler) updateSteadyStateStatus(
596723
return ctrl.Result{}, err
597724
}
598725
emitConfigRecoveryEvent(r.Recorder, externalAuthConfig, wasInvalid)
726+
desiredDeprecated := conditionStatusIs(externalAuthConfig.Status.Conditions,
727+
mcpv1beta1.ConditionTypeDeprecatedInboundGrantConfiguration, metav1.ConditionTrue)
728+
emitInboundGrantDeprecationEvent(r.Recorder, externalAuthConfig, wasDeprecated, desiredDeprecated)
599729

600730
return ctrl.Result{}, nil
601731
}

0 commit comments

Comments
 (0)