Skip to content

Commit 836d2e7

Browse files
authored
Add an enforce flag to MCPRegistry (#1997)
* Extend the CRD * Optional enforcement of images in registries * Rename enforce to enforceServers * Clarify the documentation of the EnforceServers parameter * Allow instantiating MCPServer from a specific registry * Reduce the code duplication somewhat * Filter the list of non-enforcing registries * Bump charts * fix lint
1 parent 8b9ac40 commit 836d2e7

File tree

11 files changed

+1628
-6
lines changed

11 files changed

+1628
-6
lines changed

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,15 @@ type MCPRegistrySpec struct {
4444
// Filter defines include/exclude patterns for registry content
4545
// +optional
4646
Filter *RegistryFilter `json:"filter,omitempty"`
47+
48+
// EnforceServers indicates whether MCPServers in this namespace must have their images
49+
// present in at least one registry in the namespace. When any registry in the namespace
50+
// has this field set to true, enforcement is enabled for the entire namespace.
51+
// MCPServers with images not found in any registry will be rejected.
52+
// When false (default), MCPServers can be deployed regardless of registry presence.
53+
// +kubebuilder:default=false
54+
// +optional
55+
EnforceServers bool `json:"enforceServers,omitempty"`
4756
}
4857

4958
// MCPRegistrySource defines the source configuration for registry data

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,23 @@ import (
55
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
66
)
77

8+
// Condition types for MCPServer
9+
const (
10+
// ConditionImageValidated indicates whether this image is fine to be used
11+
ConditionImageValidated = "ImageValidated"
12+
)
13+
14+
const (
15+
// ConditionReasonImageValidationFailed indicates image validation failed
16+
ConditionReasonImageValidationFailed = "ImageValidationFailed"
17+
// ConditionReasonImageValidationSuccess indicates image validation succeeded
18+
ConditionReasonImageValidationSuccess = "ImageValidationSuccess"
19+
// ConditionReasonImageValidationError indicates an error occurred during validation
20+
ConditionReasonImageValidationError = "ImageValidationError"
21+
// ConditionReasonImageValidationSkipped indicates image validation was skipped
22+
ConditionReasonImageValidationSkipped = "ImageValidationSkipped"
23+
)
24+
825
// MCPServerSpec defines the desired state of MCPServer
926
type MCPServerSpec struct {
1027
// Image is the container image for the MCP server

cmd/thv-operator/controllers/mcpserver_controller.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package controllers
55
import (
66
"context"
77
"encoding/json"
8+
goerr "errors"
89
"fmt"
910
"maps"
1011
"os"
@@ -18,6 +19,7 @@ import (
1819
corev1 "k8s.io/api/core/v1"
1920
rbacv1 "k8s.io/api/rbac/v1"
2021
"k8s.io/apimachinery/pkg/api/errors"
22+
"k8s.io/apimachinery/pkg/api/meta"
2123
"k8s.io/apimachinery/pkg/api/resource"
2224
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2325
"k8s.io/apimachinery/pkg/runtime"
@@ -30,6 +32,7 @@ import (
3032
"sigs.k8s.io/controller-runtime/pkg/log"
3133

3234
mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1"
35+
"github.com/stacklok/toolhive/cmd/thv-operator/pkg/validation"
3336
"github.com/stacklok/toolhive/pkg/container/kubernetes"
3437
)
3538

@@ -40,6 +43,7 @@ type MCPServerReconciler struct {
4043
platformDetector kubernetes.PlatformDetector
4144
detectedPlatform kubernetes.Platform
4245
platformOnce sync.Once
46+
ImageValidation validation.ImageValidation
4347
}
4448

4549
// defaultRBACRules are the default RBAC rules that the
@@ -193,6 +197,47 @@ func (r *MCPServerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
193197
return ctrl.Result{}, err
194198
}
195199

200+
// Validate MCPServer image against enforcing registries
201+
imageValidator := validation.NewImageValidator(r.Client, mcpServer.Namespace, r.ImageValidation)
202+
err = imageValidator.ValidateImage(ctx, mcpServer.Spec.Image, mcpServer.ObjectMeta)
203+
if goerr.Is(err, validation.ErrImageNotChecked) {
204+
ctxLogger.Info("Image validation skipped - no enforcement configured")
205+
// Set condition to indicate validation was skipped
206+
setImageValidationCondition(mcpServer, metav1.ConditionTrue,
207+
mcpv1alpha1.ConditionReasonImageValidationSkipped,
208+
"Image validation was not performed (no enforcement configured)")
209+
} else if goerr.Is(err, validation.ErrImageInvalid) {
210+
ctxLogger.Error(err, "MCPServer image validation failed", "image", mcpServer.Spec.Image)
211+
// Update status to reflect validation failure
212+
mcpServer.Status.Phase = mcpv1alpha1.MCPServerPhaseFailed
213+
mcpServer.Status.Message = err.Error() // Gets the specific validation failure reason
214+
setImageValidationCondition(mcpServer, metav1.ConditionFalse,
215+
mcpv1alpha1.ConditionReasonImageValidationFailed,
216+
err.Error()) // This will include the wrapped error context with specific reason
217+
if statusErr := r.Status().Update(ctx, mcpServer); statusErr != nil {
218+
ctxLogger.Error(statusErr, "Failed to update MCPServer status after validation error")
219+
}
220+
// Requeue after 5 minutes to retry validation
221+
return ctrl.Result{RequeueAfter: 5 * time.Minute}, nil
222+
} else if err != nil {
223+
// Other system/infrastructure errors
224+
ctxLogger.Error(err, "MCPServer image validation system error", "image", mcpServer.Spec.Image)
225+
setImageValidationCondition(mcpServer, metav1.ConditionFalse,
226+
mcpv1alpha1.ConditionReasonImageValidationError,
227+
fmt.Sprintf("Error checking image validity: %v", err))
228+
if statusErr := r.Status().Update(ctx, mcpServer); statusErr != nil {
229+
ctxLogger.Error(statusErr, "Failed to update MCPServer status after validation error")
230+
}
231+
// Requeue after 5 minutes to retry validation
232+
return ctrl.Result{RequeueAfter: 5 * time.Minute}, nil
233+
} else {
234+
// Validation passed
235+
ctxLogger.Info("Image validation passed", "image", mcpServer.Spec.Image)
236+
setImageValidationCondition(mcpServer, metav1.ConditionTrue,
237+
mcpv1alpha1.ConditionReasonImageValidationSuccess,
238+
"Image validation passed - image found in enforced registries")
239+
}
240+
196241
// Check if the MCPServer instance is marked to be deleted
197242
if mcpServer.GetDeletionTimestamp() != nil {
198243
// The object is being deleted
@@ -350,6 +395,17 @@ func (r *MCPServerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
350395
return ctrl.Result{}, nil
351396
}
352397

398+
// setImageValidationCondition is a helper function to set the image validation status condition
399+
// This reduces code duplication in the image validation logic
400+
func setImageValidationCondition(mcpServer *mcpv1alpha1.MCPServer, status metav1.ConditionStatus, reason, message string) {
401+
meta.SetStatusCondition(&mcpServer.Status.Conditions, metav1.Condition{
402+
Type: mcpv1alpha1.ConditionImageValidated,
403+
Status: status,
404+
Reason: reason,
405+
Message: message,
406+
})
407+
}
408+
353409
// handleRestartAnnotation checks if the restart annotation has been updated and triggers a restart if needed
354410
// Returns true if a restart was triggered and the reconciliation should be requeued
355411
func (r *MCPServerReconciler) handleRestartAnnotation(ctx context.Context, mcpServer *mcpv1alpha1.MCPServer) (bool, error) {

cmd/thv-operator/main.go

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222

2323
mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1"
2424
"github.com/stacklok/toolhive/cmd/thv-operator/controllers"
25+
"github.com/stacklok/toolhive/cmd/thv-operator/pkg/validation"
2526
"github.com/stacklok/toolhive/pkg/logger"
2627
"github.com/stacklok/toolhive/pkg/operator/telemetry"
2728
)
@@ -74,16 +75,21 @@ func main() {
7475
os.Exit(1)
7576
}
7677

77-
if err = (&controllers.MCPServerReconciler{
78-
Client: mgr.GetClient(),
79-
Scheme: mgr.GetScheme(),
80-
}).SetupWithManager(mgr); err != nil {
78+
rec := &controllers.MCPServerReconciler{
79+
Client: mgr.GetClient(),
80+
Scheme: mgr.GetScheme(),
81+
ImageValidation: validation.ImageValidationAlwaysAllow,
82+
}
83+
84+
if err = rec.SetupWithManager(mgr); err != nil {
8185
setupLog.Error(err, "unable to create controller", "controller", "MCPServer")
8286
os.Exit(1)
8387
}
8488

8589
// Only register MCPRegistry controller if feature flag is enabled
8690
if os.Getenv("ENABLE_EXPERIMENTAL_FEATURES") == "true" {
91+
rec.ImageValidation = validation.ImageValidationRegistryEnforcing
92+
8793
if err = (controllers.NewMCPRegistryReconciler(mgr.GetClient(), mgr.GetScheme())).SetupWithManager(mgr); err != nil {
8894
setupLog.Error(err, "unable to create controller", "controller", "MCPRegistry")
8995
os.Exit(1)

0 commit comments

Comments
 (0)