Skip to content

Commit 26f6e1e

Browse files
ChrisJBurnsclaude
andauthored
Rename MCPOIDCConfig condition type from Valid to Ready per RFC-0023 (#4490)
RFC-0023 specifies `type: Ready` with reasons `ConfigValid`/`ConfigInvalid` for shared config resources. The initial MCPOIDCConfig controller used `type: Valid` with `ValidationSucceeded`/`ValidationFailed` instead. Define `ConditionTypeOIDCConfigReady`, `ConditionReasonOIDCConfigValid`, and `ConditionReasonOIDCConfigInvalid` constants in mcpoidcconfig_types.go and replace all raw string usages in the writer (mcpoidcconfig_controller), readers (mcpserver_controller), and tests. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent f43e29b commit 26f6e1e

7 files changed

Lines changed: 46 additions & 34 deletions

File tree

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,18 @@ const (
1818
MCPOIDCConfigTypeInline MCPOIDCConfigSourceType = "inline"
1919
)
2020

21+
// Condition type and reasons for MCPOIDCConfig status (RFC-0023)
22+
const (
23+
// ConditionTypeOIDCConfigReady indicates whether the MCPOIDCConfig is ready for use
24+
ConditionTypeOIDCConfigReady = "Ready"
25+
26+
// ConditionReasonOIDCConfigValid indicates spec validation passed
27+
ConditionReasonOIDCConfigValid = "ConfigValid"
28+
29+
// ConditionReasonOIDCConfigInvalid indicates spec validation failed
30+
ConditionReasonOIDCConfigInvalid = "ConfigInvalid"
31+
)
32+
2133
// MCPOIDCConfigSourceType represents the type of OIDC configuration source for MCPOIDCConfig
2234
type MCPOIDCConfigSourceType string
2335

cmd/thv-operator/controllers/mcpoidcconfig_controller.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,9 @@ func (r *MCPOIDCConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reques
8383
if err := oidcConfig.Validate(); err != nil {
8484
logger.Error(err, "MCPOIDCConfig spec validation failed")
8585
meta.SetStatusCondition(&oidcConfig.Status.Conditions, metav1.Condition{
86-
Type: "Valid",
86+
Type: mcpv1alpha1.ConditionTypeOIDCConfigReady,
8787
Status: metav1.ConditionFalse,
88-
Reason: "ValidationFailed",
88+
Reason: mcpv1alpha1.ConditionReasonOIDCConfigInvalid,
8989
Message: err.Error(),
9090
ObservedGeneration: oidcConfig.Generation,
9191
})
@@ -95,11 +95,11 @@ func (r *MCPOIDCConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reques
9595
return ctrl.Result{}, nil // Don't requeue on validation errors - user must fix spec
9696
}
9797

98-
// Validation succeeded - set Valid=True condition
98+
// Validation succeeded - set Ready=True condition
9999
conditionChanged := meta.SetStatusCondition(&oidcConfig.Status.Conditions, metav1.Condition{
100-
Type: "Valid",
100+
Type: mcpv1alpha1.ConditionTypeOIDCConfigReady,
101101
Status: metav1.ConditionTrue,
102-
Reason: "ValidationSucceeded",
102+
Reason: mcpv1alpha1.ConditionReasonOIDCConfigValid,
103103
Message: "Spec validation passed",
104104
ObservedGeneration: oidcConfig.Generation,
105105
})

cmd/thv-operator/controllers/mcpoidcconfig_controller_test.go

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ func TestMCPOIDCConfigReconciler_ValidationRecovery(t *testing.T) {
227227
},
228228
}
229229

230-
// Reconcile invalid config — should set Valid=False
230+
// Reconcile invalid config — should set Ready=False
231231
_, err := r.Reconcile(ctx, req)
232232
require.NoError(t, err)
233233

@@ -237,12 +237,12 @@ func TestMCPOIDCConfigReconciler_ValidationRecovery(t *testing.T) {
237237

238238
var foundFalse bool
239239
for _, cond := range invalidConfig.Status.Conditions {
240-
if cond.Type == conditionTypeValid {
240+
if cond.Type == mcpv1alpha1.ConditionTypeOIDCConfigReady {
241241
assert.Equal(t, metav1.ConditionFalse, cond.Status)
242242
foundFalse = true
243243
}
244244
}
245-
require.True(t, foundFalse, "Should have Valid=False condition")
245+
require.True(t, foundFalse, "Should have Ready=False condition")
246246
assert.Empty(t, invalidConfig.Status.ConfigHash, "Hash should not be set for invalid config")
247247

248248
// Fix the config by adding the inline spec
@@ -254,7 +254,7 @@ func TestMCPOIDCConfigReconciler_ValidationRecovery(t *testing.T) {
254254
err = fakeClient.Update(ctx, &invalidConfig)
255255
require.NoError(t, err)
256256

257-
// Reconcile again — should set Valid=True and compute hash
257+
// Reconcile again — should set Ready=True and compute hash
258258
_, err = r.Reconcile(ctx, req)
259259
require.NoError(t, err)
260260

@@ -264,13 +264,13 @@ func TestMCPOIDCConfigReconciler_ValidationRecovery(t *testing.T) {
264264

265265
var foundTrue bool
266266
for _, cond := range recoveredConfig.Status.Conditions {
267-
if cond.Type == conditionTypeValid {
268-
assert.Equal(t, metav1.ConditionTrue, cond.Status, "Valid condition should recover to True")
269-
assert.Equal(t, "ValidationSucceeded", cond.Reason)
267+
if cond.Type == mcpv1alpha1.ConditionTypeOIDCConfigReady {
268+
assert.Equal(t, metav1.ConditionTrue, cond.Status, "Ready condition should recover to True")
269+
assert.Equal(t, mcpv1alpha1.ConditionReasonOIDCConfigValid, cond.Reason)
270270
foundTrue = true
271271
}
272272
}
273-
assert.True(t, foundTrue, "Should have Valid=True condition after fix")
273+
assert.True(t, foundTrue, "Should have Ready=True condition after fix")
274274
assert.NotEmpty(t, recoveredConfig.Status.ConfigHash, "Hash should be set after recovery")
275275
}
276276

@@ -461,21 +461,21 @@ func TestMCPOIDCConfigReconciler_ValidationFailureSetsCondition(t *testing.T) {
461461
_, err := r.Reconcile(ctx, req)
462462
require.NoError(t, err)
463463

464-
// Check that the Valid condition is set to False
464+
// Check that the Ready condition is set to False
465465
var updatedConfig mcpv1alpha1.MCPOIDCConfig
466466
err = fakeClient.Get(ctx, req.NamespacedName, &updatedConfig)
467467
require.NoError(t, err)
468468

469469
var foundCondition bool
470470
for _, cond := range updatedConfig.Status.Conditions {
471-
if cond.Type == conditionTypeValid {
471+
if cond.Type == mcpv1alpha1.ConditionTypeOIDCConfigReady {
472472
foundCondition = true
473-
assert.Equal(t, metav1.ConditionFalse, cond.Status, "Valid condition should be False")
474-
assert.Equal(t, "ValidationFailed", cond.Reason)
473+
assert.Equal(t, metav1.ConditionFalse, cond.Status, "Ready condition should be False")
474+
assert.Equal(t, mcpv1alpha1.ConditionReasonOIDCConfigInvalid, cond.Reason)
475475
break
476476
}
477477
}
478-
assert.True(t, foundCondition, "Should have a Valid condition")
478+
assert.True(t, foundCondition, "Should have a Ready condition")
479479
}
480480

481481
func TestMCPOIDCConfig_Validate(t *testing.T) {

cmd/thv-operator/controllers/mcpserver_controller.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2070,12 +2070,12 @@ func (r *MCPServerReconciler) handleOIDCConfig(ctx context.Context, m *mcpv1alph
20702070
return fmt.Errorf("MCPOIDCConfig %s not found", m.Spec.OIDCConfigRef.Name)
20712071
}
20722072

2073-
// Check that the MCPOIDCConfig is valid
2074-
validCondition := meta.FindStatusCondition(oidcConfig.Status.Conditions, "Valid")
2075-
if validCondition == nil || validCondition.Status != metav1.ConditionTrue {
2076-
msg := fmt.Sprintf("MCPOIDCConfig %s is not valid", m.Spec.OIDCConfigRef.Name)
2077-
if validCondition != nil {
2078-
msg = fmt.Sprintf("MCPOIDCConfig %s is not valid: %s", m.Spec.OIDCConfigRef.Name, validCondition.Message)
2073+
// Check that the MCPOIDCConfig is ready
2074+
readyCondition := meta.FindStatusCondition(oidcConfig.Status.Conditions, mcpv1alpha1.ConditionTypeOIDCConfigReady)
2075+
if readyCondition == nil || readyCondition.Status != metav1.ConditionTrue {
2076+
msg := fmt.Sprintf("MCPOIDCConfig %s is not ready", m.Spec.OIDCConfigRef.Name)
2077+
if readyCondition != nil {
2078+
msg = fmt.Sprintf("MCPOIDCConfig %s is not ready: %s", m.Spec.OIDCConfigRef.Name, readyCondition.Message)
20792079
}
20802080
setOIDCConfigRefCondition(m, metav1.ConditionFalse,
20812081
mcpv1alpha1.ConditionReasonOIDCConfigRefNotValid, msg)

cmd/thv-operator/controllers/mcpserver_oidcconfig_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,9 @@ import (
2222
func TestMCPServerReconciler_handleOIDCConfig(t *testing.T) {
2323
t.Parallel()
2424

25-
// validOIDCCondition is a helper to build a Valid=True condition slice.
25+
// validOIDCCondition is a helper to build a Ready=True condition slice.
2626
validOIDCCondition := []metav1.Condition{{
27-
Type: "Valid", Status: metav1.ConditionTrue, Reason: "ValidationSucceeded",
27+
Type: mcpv1alpha1.ConditionTypeOIDCConfigReady, Status: metav1.ConditionTrue, Reason: mcpv1alpha1.ConditionReasonOIDCConfigValid,
2828
}}
2929

3030
tests := []struct {
@@ -62,7 +62,7 @@ func TestMCPServerReconciler_handleOIDCConfig(t *testing.T) {
6262
expectConditionReason: mcpv1alpha1.ConditionReasonOIDCConfigRefNotFound,
6363
},
6464
{
65-
name: "config with Valid=False sets NotValid condition",
65+
name: "config with Ready=False sets NotValid condition",
6666
mcpServer: &mcpv1alpha1.MCPServer{
6767
ObjectMeta: metav1.ObjectMeta{Name: "s", Namespace: "default"},
6868
Spec: mcpv1alpha1.MCPServerSpec{
@@ -78,13 +78,13 @@ func TestMCPServerReconciler_handleOIDCConfig(t *testing.T) {
7878
},
7979
Status: mcpv1alpha1.MCPOIDCConfigStatus{
8080
Conditions: []metav1.Condition{{
81-
Type: "Valid", Status: metav1.ConditionFalse, Reason: "ValidationFailed",
81+
Type: mcpv1alpha1.ConditionTypeOIDCConfigReady, Status: metav1.ConditionFalse, Reason: mcpv1alpha1.ConditionReasonOIDCConfigInvalid,
8282
Message: "missing fields",
8383
}},
8484
},
8585
},
8686
expectError: true,
87-
expectErrorContains: "not valid",
87+
expectErrorContains: "not ready",
8888
expectConditionStatus: conditionStatusPtr(metav1.ConditionFalse),
8989
expectConditionReason: mcpv1alpha1.ConditionReasonOIDCConfigRefNotValid,
9090
},

cmd/thv-operator/test-integration/mcp-oidc-config/mcpoidcconfig_controller_integration_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ const (
2020
)
2121

2222
var _ = Describe("MCPOIDCConfig Controller", func() {
23-
It("should set Valid condition and config hash on creation", func() {
23+
It("should set Ready condition and config hash on creation", func() {
2424
oidcConfig := &mcpv1alpha1.MCPOIDCConfig{
2525
ObjectMeta: metav1.ObjectMeta{
2626
Name: "test-oidc-creation",
@@ -50,7 +50,7 @@ var _ = Describe("MCPOIDCConfig Controller", func() {
5050
return fetched.Status.ConfigHash != ""
5151
}, timeout, interval).Should(BeTrue())
5252

53-
// Verify Valid condition is set to True
53+
// Verify Ready condition is set to True
5454
Eventually(func() bool {
5555
fetched := &mcpv1alpha1.MCPOIDCConfig{}
5656
err := k8sClient.Get(ctx, types.NamespacedName{
@@ -61,7 +61,7 @@ var _ = Describe("MCPOIDCConfig Controller", func() {
6161
return false
6262
}
6363
for _, cond := range fetched.Status.Conditions {
64-
if cond.Type == "Valid" && cond.Status == metav1.ConditionTrue {
64+
if cond.Type == mcpv1alpha1.ConditionTypeOIDCConfigReady && cond.Status == metav1.ConditionTrue {
6565
return true
6666
}
6767
}

cmd/thv-operator/test-integration/mcp-oidc-config/mcpoidcconfig_mcpserver_integration_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ var _ = Describe("MCPOIDCConfig and MCPServer Cross-Resource Integration Tests",
6161
}
6262
Expect(k8sClient.Create(ctx, oidcConfig)).Should(Succeed())
6363

64-
// Wait for Valid condition and ConfigHash to be set
64+
// Wait for Ready condition and ConfigHash to be set
6565
Eventually(func() bool {
6666
updated := &mcpv1alpha1.MCPOIDCConfig{}
6767
err := k8sClient.Get(ctx, types.NamespacedName{
@@ -75,7 +75,7 @@ var _ = Describe("MCPOIDCConfig and MCPServer Cross-Resource Integration Tests",
7575
return false
7676
}
7777
for _, cond := range updated.Status.Conditions {
78-
if cond.Type == "Valid" && cond.Status == metav1.ConditionTrue {
78+
if cond.Type == mcpv1alpha1.ConditionTypeOIDCConfigReady && cond.Status == metav1.ConditionTrue {
7979
return true
8080
}
8181
}

0 commit comments

Comments
 (0)