Add operator-level defaultImagePullSecrets across all controllers

cmd/thv-operator/controllers/embeddingserver_controller.go

@@ -29,6 +29,7 @@ import (
 
 	mcpv1beta1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1beta1"
 	ctrlutil "github.com/stacklok/toolhive/cmd/thv-operator/pkg/controllerutil"
+	"github.com/stacklok/toolhive/cmd/thv-operator/pkg/imagepullsecrets"
 )
 
 // EmbeddingServerReconciler reconciles a EmbeddingServer object
@@ -37,6 +38,13 @@ type EmbeddingServerReconciler struct {
 	Scheme           *runtime.Scheme
 	Recorder         events.EventRecorder
 	PlatformDetector *ctrlutil.SharedPlatformDetector
+	// ImagePullSecretsDefaults are cluster-wide defaults sourced from the
+	// operator chart, applied to the StatefulSet's PodSpec before the
+	// user-provided PodTemplateSpec strategic-merge patch runs. The strategic
+	// merge with the user PTS continues to additively merge the user's
+	// imagePullSecrets entries on top, with the user's entries winning on
+	// name collisions per Kubernetes' strategic-merge semantics.
+	ImagePullSecretsDefaults imagepullsecrets.Defaults
 }
 
 const (
@@ -644,7 +652,12 @@ func (*EmbeddingServerReconciler) applyResourceRequirements(embedding *mcpv1beta
 // buildPodTemplate builds the pod template for the statefulset.
 // User-provided PodTemplateSpec customizations are applied later in
 // statefulSetForEmbedding via strategic merge patch.
-func (*EmbeddingServerReconciler) buildPodTemplate(
+//
+// Cluster-wide chart defaults for imagePullSecrets are placed on the base
+// PodSpec here so that a subsequent strategic-merge with the user PTS
+// additively unions the lists (Kubernetes treats PodSpec.ImagePullSecrets
+// as a merge list keyed on Name; user entries win on name collisions).
+func (r *EmbeddingServerReconciler) buildPodTemplate(
 	labels map[string]string,
 	container corev1.Container,
 ) corev1.PodTemplateSpec {
@@ -655,7 +668,8 @@ func (*EmbeddingServerReconciler) buildPodTemplate(
 			Labels: labels,
 		},
 		Spec: corev1.PodSpec{
-			Containers: []corev1.Container{container},
+			ImagePullSecrets: r.ImagePullSecretsDefaults.List(),
+			Containers:       []corev1.Container{container},
 		},
 	}
 }

cmd/thv-operator/controllers/embeddingserver_default_imagepullsecrets_test.go

@@ -0,0 +1,72 @@
+// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+package controllers
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	corev1 "k8s.io/api/core/v1"
+
+	ctrlutil "github.com/stacklok/toolhive/cmd/thv-operator/pkg/controllerutil"
+	"github.com/stacklok/toolhive/cmd/thv-operator/pkg/imagepullsecrets"
+)
+
+// TestEmbeddingServer_DefaultImagePullSecrets verifies that cluster-wide
+// chart defaults reach the StatefulSet's PodSpec.ImagePullSecrets.
+//
+// EmbeddingServer has no per-CR imagePullSecrets field; users add their own
+// entries via spec.podTemplateSpec.spec.imagePullSecrets, which is
+// strategic-merged on top of this base list. The strategic-merge behavior
+// (additive union keyed by Name) is exercised by integration tests against a
+// real K8s API; here we only assert the chart defaults reach the base PodSpec.
+func TestEmbeddingServer_DefaultImagePullSecrets(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name        string
+		defaults    []string
+		wantSecrets []corev1.LocalObjectReference
+	}{
+		{
+			name:     "chart defaults reach base PodSpec",
+			defaults: []string{"chart-default", "second-default"},
+			wantSecrets: []corev1.LocalObjectReference{
+				{Name: "chart-default"},
+				{Name: "second-default"},
+			},
+		},
+		{
+			name:        "no defaults yields nil ImagePullSecrets",
+			defaults:    nil,
+			wantSecrets: nil,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			embedding := createTestEmbeddingServer(
+				"default-pullsecrets-embed",
+				testNamespaceDefault,
+				"image:latest",
+				"model",
+			)
+
+			scheme := createEmbeddingServerTestScheme()
+			reconciler := &EmbeddingServerReconciler{
+				Scheme:                   scheme,
+				PlatformDetector:         ctrlutil.NewSharedPlatformDetector(),
+				ImagePullSecretsDefaults: imagepullsecrets.NewDefaults(tt.defaults),
+			}
+
+			sts := reconciler.statefulSetForEmbedding(t.Context(), embedding)
+			require.NotNil(t, sts)
+			assert.Equal(t, tt.wantSecrets, sts.Spec.Template.Spec.ImagePullSecrets,
+				"StatefulSet PodSpec ImagePullSecrets must reflect chart defaults")
+		})
+	}
+}

cmd/thv-operator/controllers/mcpregistry_controller.go

@@ -22,6 +22,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/log"
 
 	mcpv1beta1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1beta1"
+	"github.com/stacklok/toolhive/cmd/thv-operator/pkg/imagepullsecrets"
 	"github.com/stacklok/toolhive/cmd/thv-operator/pkg/registryapi"
 	"github.com/stacklok/toolhive/cmd/thv-operator/pkg/registryapi/config"
 )
@@ -47,9 +48,16 @@ type MCPRegistryReconciler struct {
 	registryAPIManager registryapi.Manager
 }
 
-// NewMCPRegistryReconciler creates a new MCPRegistryReconciler with required dependencies
-func NewMCPRegistryReconciler(k8sClient client.Client, scheme *runtime.Scheme) *MCPRegistryReconciler {
-	registryAPIManager := registryapi.NewManager(k8sClient, scheme)
+// NewMCPRegistryReconciler creates a new MCPRegistryReconciler with required
+// dependencies. imagePullSecretsDefaults are cluster-wide pull-secret defaults
+// from the operator chart that are merged with the per-CR list at registry-api
+// workload-construction time.
+func NewMCPRegistryReconciler(
+	k8sClient client.Client,
+	scheme *runtime.Scheme,
+	imagePullSecretsDefaults imagepullsecrets.Defaults,
+) *MCPRegistryReconciler {
+	registryAPIManager := registryapi.NewManager(k8sClient, scheme, imagePullSecretsDefaults)
 	return &MCPRegistryReconciler{
 		Client:             k8sClient,
 		Scheme:             scheme,

cmd/thv-operator/controllers/mcpremoteproxy_controller.go

@@ -30,6 +30,7 @@ import (
 
 	mcpv1beta1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1beta1"
 	ctrlutil "github.com/stacklok/toolhive/cmd/thv-operator/pkg/controllerutil"
+	"github.com/stacklok/toolhive/cmd/thv-operator/pkg/imagepullsecrets"
 	"github.com/stacklok/toolhive/cmd/thv-operator/pkg/kubernetes/rbac"
 	"github.com/stacklok/toolhive/cmd/thv-operator/pkg/runconfig/configmap/checksum"
 	"github.com/stacklok/toolhive/cmd/thv-operator/pkg/validation"
@@ -41,6 +42,10 @@ type MCPRemoteProxyReconciler struct {
 	Scheme           *runtime.Scheme
 	Recorder         events.EventRecorder
 	PlatformDetector *ctrlutil.SharedPlatformDetector
+	// ImagePullSecretsDefaults are cluster-wide defaults sourced from the
+	// operator chart that are merged with the per-CR imagePullSecrets when
+	// constructing workloads. The zero value is a usable empty Defaults.
+	ImagePullSecretsDefaults imagepullsecrets.Defaults
 }
 
 // +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=mcpremoteproxies,verbs=get;list;watch;create;update;patch;delete
@@ -1092,18 +1097,25 @@ func (r *MCPRemoteProxyReconciler) ensureRBACResources(ctx context.Context, prox
 		Namespace:        proxy.Namespace,
 		Rules:            remoteProxyRBACRules,
 		Owner:            proxy,
-		ImagePullSecrets: imagePullSecretsForRemoteProxy(proxy),
+		ImagePullSecrets: r.imagePullSecretsForRemoteProxy(proxy),
 	})
 	return err
 }
 
-// imagePullSecretsForRemoteProxy returns the image pull secrets configured via
-// spec.resourceOverrides.proxyDeployment.imagePullSecrets, or nil if unset.
-func imagePullSecretsForRemoteProxy(proxy *mcpv1beta1.MCPRemoteProxy) []corev1.LocalObjectReference {
-	if proxy.Spec.ResourceOverrides == nil || proxy.Spec.ResourceOverrides.ProxyDeployment == nil {
-		return nil
+// imagePullSecretsForRemoteProxy returns the image pull secrets the operator
+// will set on the workload's PodSpec and ServiceAccount. The list is the merge
+// of cluster-wide chart defaults (from r.ImagePullSecretsDefaults) with the
+// per-CR list from spec.resourceOverrides.proxyDeployment.imagePullSecrets.
+// CR-level entries win on name collisions; chart-level entries are appended
+// additively. Returns nil when both inputs are empty.
+func (r *MCPRemoteProxyReconciler) imagePullSecretsForRemoteProxy(
+	proxy *mcpv1beta1.MCPRemoteProxy,
+) []corev1.LocalObjectReference {
+	var crLevel []corev1.LocalObjectReference
+	if proxy.Spec.ResourceOverrides != nil && proxy.Spec.ResourceOverrides.ProxyDeployment != nil {
+		crLevel = proxy.Spec.ResourceOverrides.ProxyDeployment.ImagePullSecrets
 	}
-	return proxy.Spec.ResourceOverrides.ProxyDeployment.ImagePullSecrets
+	return r.ImagePullSecretsDefaults.Merge(crLevel)
 }
 
 // updateMCPRemoteProxyStatus updates the status of the MCPRemoteProxy
@@ -1390,14 +1402,15 @@ func (r *MCPRemoteProxyReconciler) podTemplateMetadataNeedsUpdate(
 
 // podSpecNeedsUpdate checks if pod-level fields (not container fields) have drifted.
 //
-// Currently compares ImagePullSecrets sourced from spec.resourceOverrides.proxyDeployment.
-// Uses equality.Semantic.DeepEqual so nil and empty slices are treated as equal,
+// Currently compares ImagePullSecrets — the merge of cluster-wide chart
+// defaults with spec.resourceOverrides.proxyDeployment.imagePullSecrets. Uses
+// equality.Semantic.DeepEqual so nil and empty slices are treated as equal,
 // which matches Kubernetes' own serialization semantics.
-func (*MCPRemoteProxyReconciler) podSpecNeedsUpdate(
+func (r *MCPRemoteProxyReconciler) podSpecNeedsUpdate(
 	deployment *appsv1.Deployment,
 	proxy *mcpv1beta1.MCPRemoteProxy,
 ) bool {
-	expected := imagePullSecretsForRemoteProxy(proxy)
+	expected := r.imagePullSecretsForRemoteProxy(proxy)
 	current := deployment.Spec.Template.Spec.ImagePullSecrets
 	return !equality.Semantic.DeepEqual(current, expected)
 }

cmd/thv-operator/controllers/mcpremoteproxy_default_imagepullsecrets_test.go

@@ -0,0 +1,110 @@
+// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+package controllers
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	corev1 "k8s.io/api/core/v1"
+	rbacv1 "k8s.io/api/rbac/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+
+	mcpv1beta1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1beta1"
+	ctrlutil "github.com/stacklok/toolhive/cmd/thv-operator/pkg/controllerutil"
+	"github.com/stacklok/toolhive/cmd/thv-operator/pkg/imagepullsecrets"
+)
+
+// TestMCPRemoteProxy_DefaultImagePullSecrets verifies that the merge of
+// cluster-wide chart defaults with spec.resourceOverrides.proxyDeployment.imagePullSecrets
+// reaches both the proxy-runner ServiceAccount and the Deployment PodSpec.
+//
+// The Merge precedence rule is exhaustively covered in
+// imagepullsecrets/defaults_test.go::TestDefaultsMerge; the cases here exist
+// only to prove the wiring is correct end-to-end.
+func TestMCPRemoteProxy_DefaultImagePullSecrets(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name        string
+		defaults    []string
+		crSecrets   []corev1.LocalObjectReference
+		wantSecrets []corev1.LocalObjectReference
+	}{
+		{
+			name:     "merged defaults+CR with name collision reach SA and Deployment",
+			defaults: []string{"shared", "chart-only"},
+			crSecrets: []corev1.LocalObjectReference{
+				{Name: "shared"},
+			},
+			wantSecrets: []corev1.LocalObjectReference{
+				{Name: "shared"},
+				{Name: "chart-only"},
+			},
+		},
+		{
+			name:        "no defaults and no CR yields empty fields",
+			defaults:    nil,
+			crSecrets:   nil,
+			wantSecrets: nil,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			proxy := &mcpv1beta1.MCPRemoteProxy{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "default-pullsecrets-proxy",
+					Namespace: "default",
+				},
+				Spec: mcpv1beta1.MCPRemoteProxySpec{
+					RemoteURL: "https://mcp.example.com",
+					ProxyPort: 8080,
+				},
+			}
+			if tt.crSecrets != nil {
+				proxy.Spec.ResourceOverrides = &mcpv1beta1.ResourceOverrides{
+					ProxyDeployment: &mcpv1beta1.ProxyDeploymentOverrides{
+						ImagePullSecrets: tt.crSecrets,
+					},
+				}
+			}
+
+			scheme := createRunConfigTestScheme()
+			_ = rbacv1.AddToScheme(scheme)
+
+			fakeClient := fake.NewClientBuilder().
+				WithScheme(scheme).
+				WithRuntimeObjects(proxy).
+				Build()
+
+			reconciler := &MCPRemoteProxyReconciler{
+				Client:                   fakeClient,
+				Scheme:                   scheme,
+				PlatformDetector:         ctrlutil.NewSharedPlatformDetector(),
+				ImagePullSecretsDefaults: imagepullsecrets.NewDefaults(tt.defaults),
+			}
+
+			require.NoError(t, reconciler.ensureRBACResources(t.Context(), proxy))
+
+			sa := &corev1.ServiceAccount{}
+			require.NoError(t, fakeClient.Get(t.Context(), types.NamespacedName{
+				Name:      proxyRunnerServiceAccountNameForRemoteProxy(proxy.Name),
+				Namespace: proxy.Namespace,
+			}, sa))
+			assert.Equal(t, tt.wantSecrets, sa.ImagePullSecrets,
+				"proxy runner SA ImagePullSecrets must reflect merged defaults+CR")
+
+			dep := reconciler.deploymentForMCPRemoteProxy(t.Context(), proxy, "test-checksum")
+			require.NotNil(t, dep)
+			assert.Equal(t, tt.wantSecrets, dep.Spec.Template.Spec.ImagePullSecrets,
+				"proxy runner Deployment ImagePullSecrets must reflect merged defaults+CR")
+		})
+	}
+}

cmd/thv-operator/controllers/mcpremoteproxy_deployment.go

@@ -72,7 +72,7 @@ func (r *MCPRemoteProxyReconciler) deploymentForMCPRemoteProxy(
 				},
 				Spec: corev1.PodSpec{
 					ServiceAccountName: serviceAccountNameForRemoteProxy(proxy),
-					ImagePullSecrets:   imagePullSecretsForRemoteProxy(proxy),
+					ImagePullSecrets:   r.imagePullSecretsForRemoteProxy(proxy),
 					Containers: []corev1.Container{{
 						Image:           getToolhiveRunnerImage(),
 						Name:            "toolhive",