Skip to content

Commit 5524d25

Browse files
authored
adds podTemplateSpec field to MCPRegistry CRD that allows user to customise registry api deployment (#2777)
* adds `podTemplateSpec` to MCPRegistry CRD users will want to override the templatespec for the registry server and adding a podTemplateSpec override enables this. Signed-off-by: Chris Burns <[email protected]> * bumps crds chart Signed-off-by: Chris Burns <[email protected]> --------- Signed-off-by: Chris Burns <[email protected]>
1 parent fda9a56 commit 5524d25

File tree

13 files changed

+1368
-340
lines changed

13 files changed

+1368
-340
lines changed

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55

66
corev1 "k8s.io/api/core/v1"
77
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
8+
"k8s.io/apimachinery/pkg/runtime"
89
)
910

1011
const (
@@ -43,6 +44,16 @@ type MCPRegistrySpec struct {
4344
// +kubebuilder:default=false
4445
// +optional
4546
EnforceServers bool `json:"enforceServers,omitempty"`
47+
48+
// PodTemplateSpec defines the pod template to use for the registry API server
49+
// This allows for customizing the pod configuration beyond what is provided by the other fields.
50+
// Note that to modify the specific container the registry API server runs in, you must specify
51+
// the `registry-api` container name in the PodTemplateSpec.
52+
// This field accepts a PodTemplateSpec object as JSON/YAML.
53+
// +optional
54+
// +kubebuilder:pruning:PreserveUnknownFields
55+
// +kubebuilder:validation:Type=object
56+
PodTemplateSpec *runtime.RawExtension `json:"podTemplateSpec,omitempty"`
4657
}
4758

4859
// MCPRegistryConfig defines the configuration for a registry data source
@@ -350,6 +361,18 @@ const (
350361

351362
// ConditionAPIReady indicates whether the registry API is ready
352363
ConditionAPIReady = "APIReady"
364+
365+
// ConditionRegistryPodTemplateValid indicates whether the PodTemplateSpec is valid
366+
ConditionRegistryPodTemplateValid = "PodTemplateValid"
367+
)
368+
369+
// Condition reasons for MCPRegistry PodTemplateSpec validation
370+
const (
371+
// ConditionReasonRegistryPodTemplateValid indicates PodTemplateSpec validation succeeded
372+
ConditionReasonRegistryPodTemplateValid = "ValidPodTemplateSpec"
373+
374+
// ConditionReasonRegistryPodTemplateInvalid indicates PodTemplateSpec validation failed
375+
ConditionReasonRegistryPodTemplateInvalid = "InvalidPodTemplateSpec"
353376
)
354377

355378
//+kubebuilder:object:root=true
@@ -437,3 +460,13 @@ func (r *MCPRegistry) DeriveOverallPhase() MCPRegistryPhase {
437460
func init() {
438461
SchemeBuilder.Register(&MCPRegistry{}, &MCPRegistryList{})
439462
}
463+
464+
// HasPodTemplateSpec returns true if the MCPRegistry has a PodTemplateSpec
465+
func (r *MCPRegistry) HasPodTemplateSpec() bool {
466+
return r.Spec.PodTemplateSpec != nil
467+
}
468+
469+
// GetPodTemplateSpecRaw returns the raw PodTemplateSpec
470+
func (r *MCPRegistry) GetPodTemplateSpecRaw() *runtime.RawExtension {
471+
return r.Spec.PodTemplateSpec
472+
}

cmd/thv-operator/api/v1alpha1/zz_generated.deepcopy.go

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

cmd/thv-operator/controllers/mcpregistry_controller.go

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
appsv1 "k8s.io/api/apps/v1"
99
corev1 "k8s.io/api/core/v1"
1010
kerrors "k8s.io/apimachinery/pkg/api/errors"
11+
"k8s.io/apimachinery/pkg/api/meta"
1112
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1213
"k8s.io/apimachinery/pkg/runtime"
1314
ctrl "sigs.k8s.io/controller-runtime"
@@ -95,6 +96,17 @@ func (r *MCPRegistryReconciler) Reconcile(ctx context.Context, req ctrl.Request)
9596
ctxLogger.Info("Reconciling MCPRegistry", "MCPRegistry.Name", mcpRegistry.Name, "phase", mcpRegistry.Status.Phase,
9697
"apiPhase", apiPhase, "apiEndpoint", apiEndpoint)
9798

99+
// Validate PodTemplateSpec early - before other operations
100+
if mcpRegistry.HasPodTemplateSpec() {
101+
// Validate PodTemplateSpec early - before other operations
102+
// This ensures we fail fast if the spec is invalid
103+
if !r.validateAndUpdatePodTemplateStatus(ctx, mcpRegistry) {
104+
// Invalid PodTemplateSpec - return without error to avoid infinite retries
105+
// The user must fix the spec and the next reconciliation will retry
106+
return ctrl.Result{}, nil
107+
}
108+
}
109+
98110
// 2. Handle deletion if DeletionTimestamp is set
99111
if mcpRegistry.GetDeletionTimestamp() != nil {
100112
// The object is being deleted
@@ -301,3 +313,53 @@ func (*MCPRegistryReconciler) applyStatusUpdates(
301313

302314
return nil
303315
}
316+
317+
// validateAndUpdatePodTemplateStatus validates the PodTemplateSpec and updates the MCPRegistry status
318+
// with appropriate conditions. Returns true if validation passes, false otherwise.
319+
func (r *MCPRegistryReconciler) validateAndUpdatePodTemplateStatus(
320+
ctx context.Context, mcpRegistry *mcpv1alpha1.MCPRegistry,
321+
) bool {
322+
ctxLogger := log.FromContext(ctx)
323+
324+
// Validate the PodTemplateSpec by attempting to parse it
325+
err := registryapi.ValidatePodTemplateSpec(mcpRegistry.GetPodTemplateSpecRaw())
326+
if err != nil {
327+
// Set phase and message
328+
mcpRegistry.Status.Phase = mcpv1alpha1.MCPRegistryPhaseFailed
329+
mcpRegistry.Status.Message = fmt.Sprintf("Invalid PodTemplateSpec: %v", err)
330+
331+
// Set condition for invalid PodTemplateSpec
332+
meta.SetStatusCondition(&mcpRegistry.Status.Conditions, metav1.Condition{
333+
Type: mcpv1alpha1.ConditionRegistryPodTemplateValid,
334+
Status: metav1.ConditionFalse,
335+
ObservedGeneration: mcpRegistry.Generation,
336+
Reason: mcpv1alpha1.ConditionReasonRegistryPodTemplateInvalid,
337+
Message: fmt.Sprintf("Failed to parse PodTemplateSpec: %v. Deployment blocked until fixed.", err),
338+
})
339+
340+
// Update status with the condition
341+
if statusErr := r.Status().Update(ctx, mcpRegistry); statusErr != nil {
342+
ctxLogger.Error(statusErr, "Failed to update MCPRegistry status with PodTemplateSpec validation")
343+
return false
344+
}
345+
346+
ctxLogger.Error(err, "PodTemplateSpec validation failed")
347+
return false
348+
}
349+
350+
// Set condition for valid PodTemplateSpec
351+
meta.SetStatusCondition(&mcpRegistry.Status.Conditions, metav1.Condition{
352+
Type: mcpv1alpha1.ConditionRegistryPodTemplateValid,
353+
Status: metav1.ConditionTrue,
354+
ObservedGeneration: mcpRegistry.Generation,
355+
Reason: mcpv1alpha1.ConditionReasonRegistryPodTemplateValid,
356+
Message: "PodTemplateSpec is valid",
357+
})
358+
359+
// Update status with the condition
360+
if statusErr := r.Status().Update(ctx, mcpRegistry); statusErr != nil {
361+
ctxLogger.Error(statusErr, "Failed to update MCPRegistry status with PodTemplateSpec validation")
362+
}
363+
364+
return true
365+
}

cmd/thv-operator/pkg/registryapi/deployment.go

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ func (m *manager) ensureDeployment(
7777
ctxLogger := log.FromContext(ctx).WithValues("mcpregistry", mcpRegistry.Name)
7878

7979
// Build the desired deployment configuration
80-
deployment := m.buildRegistryAPIDeployment(mcpRegistry, configManager)
80+
deployment := m.buildRegistryAPIDeployment(ctx, mcpRegistry, configManager)
8181
deploymentName := deployment.Name
8282

8383
// Set owner reference for automatic garbage collection
@@ -119,18 +119,30 @@ func (m *manager) ensureDeployment(
119119
// This function handles all deployment configuration including labels, container specs, probes,
120120
// and storage manager integration. It returns a fully configured deployment ready for Kubernetes API operations.
121121
func (*manager) buildRegistryAPIDeployment(
122+
ctx context.Context,
122123
mcpRegistry *mcpv1alpha1.MCPRegistry,
123124
configManager config.ConfigManager,
124125
) *appsv1.Deployment {
126+
ctxLogger := log.FromContext(ctx).WithValues("mcpregistry", mcpRegistry.Name)
125127
// Generate deployment name using the established pattern
126128
deploymentName := mcpRegistry.GetAPIResourceName()
127129

128130
// Define labels using common function
129131
labels := labelsForRegistryAPI(mcpRegistry, deploymentName)
130132

131-
// Build the PodTemplateSpec using the functional options pattern
132-
// This includes all mount configurations via the builder pattern
133-
builder := NewPodTemplateSpecBuilder()
133+
// Parse user-provided PodTemplateSpec if present
134+
var userPTS *corev1.PodTemplateSpec
135+
if mcpRegistry.HasPodTemplateSpec() {
136+
var err error
137+
userPTS, err = ParsePodTemplateSpec(mcpRegistry.GetPodTemplateSpecRaw())
138+
if err != nil {
139+
ctxLogger.Error(err, "Failed to parse PodTemplateSpec")
140+
return nil
141+
}
142+
}
143+
144+
// Build PodTemplateSpec with defaults and user customizations merged
145+
builder := NewPodTemplateSpecBuilderFrom(userPTS)
134146
podTemplateSpec := builder.Apply(
135147
WithLabels(labels),
136148
WithAnnotations(map[string]string{

cmd/thv-operator/pkg/registryapi/deployment_test.go

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package registryapi
22

33
import (
4+
"context"
45
"testing"
56

67
"github.com/stretchr/testify/assert"
@@ -10,6 +11,7 @@ import (
1011
corev1 "k8s.io/api/core/v1"
1112
"k8s.io/apimachinery/pkg/api/resource"
1213
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
14+
"k8s.io/apimachinery/pkg/runtime"
1315
"k8s.io/apimachinery/pkg/util/intstr"
1416

1517
mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1"
@@ -121,6 +123,47 @@ func TestManagerBuildRegistryAPIDeployment(t *testing.T) {
121123

122124
},
123125
},
126+
{
127+
name: "user PodTemplateSpec merged correctly",
128+
mcpRegistry: &mcpv1alpha1.MCPRegistry{
129+
ObjectMeta: metav1.ObjectMeta{
130+
Name: "test-registry",
131+
Namespace: "test-namespace",
132+
},
133+
Spec: mcpv1alpha1.MCPRegistrySpec{
134+
Registries: []mcpv1alpha1.MCPRegistryConfig{
135+
{
136+
Name: "default",
137+
Format: mcpv1alpha1.RegistryFormatToolHive,
138+
ConfigMapRef: &corev1.ConfigMapKeySelector{
139+
LocalObjectReference: corev1.LocalObjectReference{
140+
Name: "test-configmap",
141+
},
142+
Key: "registry.json",
143+
},
144+
},
145+
},
146+
PodTemplateSpec: &runtime.RawExtension{
147+
Raw: []byte(`{"spec":{"serviceAccountName":"custom-sa"}}`),
148+
},
149+
},
150+
},
151+
setupMocks: func() {
152+
},
153+
validateResult: func(t *testing.T, deployment *appsv1.Deployment) {
154+
t.Helper()
155+
require.NotNil(t, deployment)
156+
157+
// User-provided service account name should take precedence
158+
assert.Equal(t, "custom-sa", deployment.Spec.Template.Spec.ServiceAccountName)
159+
160+
// Default volumes and mounts should still be present
161+
volumes := deployment.Spec.Template.Spec.Volumes
162+
assert.True(t, hasVolume(volumes, RegistryServerConfigVolumeName))
163+
assert.True(t, hasVolume(volumes, "storage-data"))
164+
assert.True(t, hasVolume(volumes, "registry-data-source-default"))
165+
},
166+
},
124167
}
125168

126169
for _, tt := range tests {
@@ -134,7 +177,7 @@ func TestManagerBuildRegistryAPIDeployment(t *testing.T) {
134177
manager := &manager{}
135178

136179
configManager := config.NewConfigManagerForTesting(tt.mcpRegistry)
137-
deployment := manager.buildRegistryAPIDeployment(tt.mcpRegistry, configManager)
180+
deployment := manager.buildRegistryAPIDeployment(context.Background(), tt.mcpRegistry, configManager)
138181
tt.validateResult(t, deployment)
139182
})
140183
}

cmd/thv-operator/pkg/registryapi/podtemplatespec.go

Lines changed: 55 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,13 @@
22
package registryapi
33

44
import (
5+
"encoding/json"
56
"fmt"
67
"path/filepath"
78

89
corev1 "k8s.io/api/core/v1"
910
"k8s.io/apimachinery/pkg/api/resource"
11+
"k8s.io/apimachinery/pkg/runtime"
1012
"k8s.io/apimachinery/pkg/util/intstr"
1113

1214
mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1"
@@ -17,15 +19,45 @@ import (
1719
type PodTemplateSpecOption func(*corev1.PodTemplateSpec)
1820

1921
// PodTemplateSpecBuilder builds a PodTemplateSpec using the functional options pattern.
22+
// When created with NewPodTemplateSpecBuilderFrom, the builder stores the user's template
23+
// and applies options as defaults. Build() merges them with user values taking precedence.
2024
type PodTemplateSpecBuilder struct {
21-
podTemplateSpec *corev1.PodTemplateSpec
25+
// userTemplate is the user-provided PodTemplateSpec (if any)
26+
userTemplate *corev1.PodTemplateSpec
27+
// defaultSpec is built up via Apply() with options acting as defaults
28+
defaultSpec *corev1.PodTemplateSpec
2229
}
2330

24-
// NewPodTemplateSpecBuilder creates a new PodTemplateSpecBuilder with default values.
25-
func NewPodTemplateSpecBuilder() *PodTemplateSpecBuilder {
31+
// NewPodTemplateSpecBuilderFrom creates a new PodTemplateSpecBuilder with a user-provided template.
32+
// The user template is deep-copied to avoid mutating the original.
33+
// Options applied via Apply() act as defaults - Build() will merge them with user values,
34+
// where user values take precedence over defaults.
35+
func NewPodTemplateSpecBuilderFrom(userTemplate *corev1.PodTemplateSpec) *PodTemplateSpecBuilder {
36+
var userCopy *corev1.PodTemplateSpec
37+
if userTemplate != nil {
38+
userCopy = userTemplate.DeepCopy()
39+
}
2640
return &PodTemplateSpecBuilder{
27-
podTemplateSpec: &corev1.PodTemplateSpec{},
41+
userTemplate: userCopy,
42+
defaultSpec: &corev1.PodTemplateSpec{},
43+
}
44+
}
45+
46+
// Apply applies the given options to build up the default PodTemplateSpec.
47+
func (b *PodTemplateSpecBuilder) Apply(opts ...PodTemplateSpecOption) *PodTemplateSpecBuilder {
48+
for _, opt := range opts {
49+
opt(b.defaultSpec)
50+
}
51+
return b
52+
}
53+
54+
// Build returns the final PodTemplateSpec.
55+
// If a user template was provided, merges defaults with user values (user takes precedence).
56+
func (b *PodTemplateSpecBuilder) Build() corev1.PodTemplateSpec {
57+
if b.userTemplate == nil {
58+
return *b.defaultSpec
2859
}
60+
return MergePodTemplateSpecs(b.defaultSpec, b.userTemplate)
2961
}
3062

3163
// WithLabels sets the labels on the PodTemplateSpec.
@@ -196,17 +228,28 @@ func WithRegistrySourceMounts(containerName string, registries []mcpv1alpha1.MCP
196228
}
197229
}
198230

199-
// Apply applies the given options to the PodTemplateSpecBuilder.
200-
func (b *PodTemplateSpecBuilder) Apply(opts ...PodTemplateSpecOption) *PodTemplateSpecBuilder {
201-
for _, opt := range opts {
202-
opt(b.podTemplateSpec)
231+
// ParsePodTemplateSpec parses a runtime.RawExtension into a PodTemplateSpec.
232+
// Returns nil if the raw extension is nil or empty.
233+
// Returns an error if the raw extension contains invalid PodTemplateSpec data.
234+
func ParsePodTemplateSpec(raw *runtime.RawExtension) (*corev1.PodTemplateSpec, error) {
235+
if raw == nil || raw.Raw == nil || len(raw.Raw) == 0 {
236+
return nil, nil
203237
}
204-
return b
238+
239+
var pts corev1.PodTemplateSpec
240+
if err := json.Unmarshal(raw.Raw, &pts); err != nil {
241+
return nil, fmt.Errorf("failed to unmarshal PodTemplateSpec: %w", err)
242+
}
243+
244+
return &pts, nil
205245
}
206246

207-
// Build returns the configured PodTemplateSpec.
208-
func (b *PodTemplateSpecBuilder) Build() corev1.PodTemplateSpec {
209-
return *b.podTemplateSpec
247+
// ValidatePodTemplateSpec validates a runtime.RawExtension contains valid PodTemplateSpec data.
248+
// Returns nil if the raw extension is nil, empty, or contains valid data.
249+
// Returns an error if the raw extension contains invalid PodTemplateSpec data.
250+
func ValidatePodTemplateSpec(raw *runtime.RawExtension) error {
251+
_, err := ParsePodTemplateSpec(raw)
252+
return err
210253
}
211254

212255
// BuildRegistryAPIContainer creates the registry-api container with default configuration.
@@ -257,19 +300,6 @@ func BuildRegistryAPIContainer(image string) corev1.Container {
257300
}
258301
}
259302

260-
// DefaultRegistryAPIPodTemplateSpec creates a default PodTemplateSpec for the registry-api.
261-
func DefaultRegistryAPIPodTemplateSpec(labels map[string]string, configHash string) corev1.PodTemplateSpec {
262-
builder := NewPodTemplateSpecBuilder()
263-
return builder.Apply(
264-
WithLabels(labels),
265-
WithAnnotations(map[string]string{
266-
"toolhive.stacklok.dev/config-hash": configHash,
267-
}),
268-
WithServiceAccountName(DefaultServiceAccountName),
269-
WithContainer(BuildRegistryAPIContainer(getRegistryAPIImage())),
270-
).Build()
271-
}
272-
273303
// MergePodTemplateSpecs merges a default PodTemplateSpec with a user-provided one.
274304
// User-provided values take precedence over defaults. This allows users to customize
275305
// infrastructure concerns while ensuring sensible defaults are applied where values

0 commit comments

Comments
 (0)