Fix imagePullSecrets propagation to ProxyRunner

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

@@ -232,6 +232,11 @@ type ProxyDeploymentOverrides struct {
 	// Use TOOLHIVE_DEBUG=true to enable debug logging in the proxy
 	// +optional
 	Env []EnvVar `json:"env,omitempty"`
+
+	// ImagePullSecrets allows specifying image pull secrets for the proxy runner
+	// These are applied to both the Deployment and the ServiceAccount
+	// +optional
+	ImagePullSecrets []corev1.LocalObjectReference `json:"imagePullSecrets,omitempty"`
 }
 
 // ResourceMetadataOverrides defines metadata overrides for a resource

cmd/thv-operator/controllers/mcpserver_controller.go

@@ -847,12 +847,20 @@ func (r *MCPServerReconciler) ensureRBACResources(ctx context.Context, mcpServer
 	rbacClient := rbac.NewClient(r.Client, r.Scheme)
 	proxyRunnerNameForRBAC := ctrlutil.ProxyRunnerServiceAccountName(mcpServer.Name)
 
+	// Extract ImagePullSecrets from ResourceOverrides if present
+	var imagePullSecrets []corev1.LocalObjectReference
+	if mcpServer.Spec.ResourceOverrides != nil &&
+		mcpServer.Spec.ResourceOverrides.ProxyDeployment != nil {
+		imagePullSecrets = mcpServer.Spec.ResourceOverrides.ProxyDeployment.ImagePullSecrets
+	}
+
 	// Ensure RBAC resources for proxy runner
 	if _, err := rbacClient.EnsureRBACResources(ctx, rbac.EnsureRBACResourcesParams{
-		Name:      proxyRunnerNameForRBAC,
-		Namespace: mcpServer.Namespace,
-		Rules:     defaultRBACRules,
-		Owner:     mcpServer,
+		Name:             proxyRunnerNameForRBAC,
+		Namespace:        mcpServer.Namespace,
+		Rules:            defaultRBACRules,
+		Owner:            mcpServer,
+		ImagePullSecrets: imagePullSecrets,
 	}); err != nil {
 		return err
 	}
@@ -869,6 +877,7 @@ func (r *MCPServerReconciler) ensureRBACResources(ctx context.Context, mcpServer
 			Name:      mcpServerSAName,
 			Namespace: mcpServer.Namespace,
 		},
+		ImagePullSecrets: imagePullSecrets,
 	}
 	_, err := rbacClient.UpsertServiceAccountWithOwnerReference(ctx, mcpServerSA, mcpServer)
 	return err
@@ -1132,6 +1141,12 @@ func (r *MCPServerReconciler) deploymentForMCPServer(
 
 	env = ctrlutil.EnsureRequiredEnvVars(ctx, env)
 
+	// Extract ImagePullSecrets from ResourceOverrides if present
+	var imagePullSecrets []corev1.LocalObjectReference
+	if m.Spec.ResourceOverrides != nil && m.Spec.ResourceOverrides.ProxyDeployment != nil {
+		imagePullSecrets = m.Spec.ResourceOverrides.ProxyDeployment.ImagePullSecrets
+	}
+
 	dep := &appsv1.Deployment{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:        m.Name,
@@ -1151,6 +1166,7 @@ func (r *MCPServerReconciler) deploymentForMCPServer(
 				},
 				Spec: corev1.PodSpec{
 					ServiceAccountName: ctrlutil.ProxyRunnerServiceAccountName(m.Name),
+					ImagePullSecrets:   imagePullSecrets,
 					Containers: []corev1.Container{{
 						Image:        getToolhiveRunnerImage(),
 						Name:         "toolhive",

cmd/thv-operator/controllers/mcpserver_rbac_test.go

@@ -381,6 +381,49 @@ func TestEnsureRBACResources_CustomServiceAccount(t *testing.T) {
 	assert.Error(t, err, "MCP server ServiceAccount should not be created when custom ServiceAccount is provided")
 }
 
+func TestEnsureRBACResources_ImagePullSecrets(t *testing.T) {
+	t.Parallel()
+	tc := setupTest("test-server-pull-secrets", "default")
+
+	// Set ImagePullSecrets via ResourceOverrides
+	tc.mcpServer.Spec.ResourceOverrides = &mcpv1alpha1.ResourceOverrides{
+		ProxyDeployment: &mcpv1alpha1.ProxyDeploymentOverrides{
+			ImagePullSecrets: []corev1.LocalObjectReference{
+				{Name: "my-secret"},
+			},
+		},
+	}
+
+	err := tc.ensureRBACResources()
+	require.NoError(t, err)
+
+	tc.assertServiceAccountExists(t)
+
+	// Verify ImagePullSecrets are present on the Proxy Runner ServiceAccount
+	sa := &corev1.ServiceAccount{}
+	// Re-get the client from fake client to ensure we have the created object
+	err = tc.client.Get(context.TODO(), types.NamespacedName{
+		Name:      tc.proxyRunnerNameForRBAC,
+		Namespace: tc.mcpServer.Namespace,
+	}, sa)
+	require.NoError(t, err)
+
+	expectedSecrets := []corev1.LocalObjectReference{
+		{Name: "my-secret"},
+	}
+	assert.Equal(t, expectedSecrets, sa.ImagePullSecrets)
+
+	// Verify ImagePullSecrets are present on the MCP Server ServiceAccount (since we didn't specify a custom one)
+	mcpServerSAName := mcpServerServiceAccountName(tc.mcpServer.Name)
+	mcpServerSA := &corev1.ServiceAccount{}
+	err = tc.client.Get(context.TODO(), types.NamespacedName{
+		Name:      mcpServerSAName,
+		Namespace: tc.mcpServer.Namespace,
+	}, mcpServerSA)
+	require.NoError(t, err)
+	assert.Equal(t, expectedSecrets, mcpServerSA.ImagePullSecrets)
+}
+
 func createTestMCPServer(name, namespace string) *mcpv1alpha1.MCPServer {
 	return &mcpv1alpha1.MCPServer{
 		ObjectMeta: metav1.ObjectMeta{

cmd/thv-operator/pkg/kubernetes/rbac/rbac.go

@@ -329,6 +329,8 @@ type EnsureRBACResourcesParams struct {
 	Owner client.Object
 	// Labels are optional labels to apply to all RBAC resources
 	Labels map[string]string
+	// ImagePullSecrets are optional image pull secrets to apply to the ServiceAccount
+	ImagePullSecrets []corev1.LocalObjectReference
 }
 
 // OperationResults contains the operation results for each RBAC resource.
@@ -367,6 +369,7 @@ func (c *Client) EnsureRBACResources(ctx context.Context, params EnsureRBACResou
 			Namespace: params.Namespace,
 			Labels:    params.Labels,
 		},
+		ImagePullSecrets: params.ImagePullSecrets,
 	}
 	saResult, err := c.UpsertServiceAccountWithOwnerReference(ctx, serviceAccount, params.Owner)
 	if err != nil {

deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpremoteproxies.yaml

@@ -461,6 +461,27 @@ spec:
                           - value
                           type: object
                         type: array
+                      imagePullSecrets:
+                        description: |-
+                          ImagePullSecrets allows specifying image pull secrets for the proxy runner
+                          These are applied to both the Deployment and the ServiceAccount
+                        items:
+                          description: |-
+                            LocalObjectReference contains enough information to let you locate the
+                            referenced object inside the same namespace.
+                          properties:
+                            name:
+                              default: ""
+                              description: |-
+                                Name of the referent.
+                                This field is effectively required, but due to backwards compatibility is
+                                allowed to be empty. Instances of this type with an empty value here are
+                                almost certainly wrong.
+                                More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
+                              type: string
+                          type: object
+                          x-kubernetes-map-type: atomic
+                        type: array
                       labels:
                         additionalProperties:
                           type: string

deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpservers.yaml

@@ -485,6 +485,27 @@ spec:
                           - value
                           type: object
                         type: array
+                      imagePullSecrets:
+                        description: |-
+                          ImagePullSecrets allows specifying image pull secrets for the proxy runner
+                          These are applied to both the Deployment and the ServiceAccount
+                        items:
+                          description: |-
+                            LocalObjectReference contains enough information to let you locate the
+                            referenced object inside the same namespace.
+                          properties:
+                            name:
+                              default: ""
+                              description: |-
+                                Name of the referent.
+                                This field is effectively required, but due to backwards compatibility is
+                                allowed to be empty. Instances of this type with an empty value here are
+                                almost certainly wrong.
+                                More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
+                              type: string
+                          type: object
+                          x-kubernetes-map-type: atomic
+                        type: array
                       labels:
                         additionalProperties:
                           type: string

deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpremoteproxies.yaml

@@ -464,6 +464,27 @@ spec:
                           - value
                           type: object
                         type: array
+                      imagePullSecrets:
+                        description: |-
+                          ImagePullSecrets allows specifying image pull secrets for the proxy runner
+                          These are applied to both the Deployment and the ServiceAccount
+                        items:
+                          description: |-
+                            LocalObjectReference contains enough information to let you locate the
+                            referenced object inside the same namespace.
+                          properties:
+                            name:
+                              default: ""
+                              description: |-
+                                Name of the referent.
+                                This field is effectively required, but due to backwards compatibility is
+                                allowed to be empty. Instances of this type with an empty value here are
+                                almost certainly wrong.
+                                More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
+                              type: string
+                          type: object
+                          x-kubernetes-map-type: atomic
+                        type: array
                       labels:
                         additionalProperties:
                           type: string

deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpservers.yaml

@@ -488,6 +488,27 @@ spec:
                           - value
                           type: object
                         type: array
+                      imagePullSecrets:
+                        description: |-
+                          ImagePullSecrets allows specifying image pull secrets for the proxy runner
+                          These are applied to both the Deployment and the ServiceAccount
+                        items:
+                          description: |-
+                            LocalObjectReference contains enough information to let you locate the
+                            referenced object inside the same namespace.
+                          properties:
+                            name:
+                              default: ""
+                              description: |-
+                                Name of the referent.
+                                This field is effectively required, but due to backwards compatibility is
+                                allowed to be empty. Instances of this type with an empty value here are
+                                almost certainly wrong.
+                                More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
+                              type: string
+                          type: object
+                          x-kubernetes-map-type: atomic
+                        type: array
                       labels:
                         additionalProperties:
                           type: string