Skip to content

Commit 57e9006

Browse files
JAORMXclaude
andcommitted
Add disableWorkloadRBAC flag to skip per-workload RBAC creation
Some K8s platform teams reject the operator because its ClusterRole includes roles/rolebindings permissions for dynamic RBAC creation. This adds an opt-in DISABLE_WORKLOAD_RBAC env var (exposed via operator.rbac.disableWorkloadRBAC Helm value) so the operator skips per-workload ServiceAccount, Role, and RoleBinding creation. When enabled: - All controller ensureRBACResources() methods return nil immediately - ClusterRole omits roles/rolebindings rules and SA write verbs - Registry API ClusterRole/ClusterRoleBinding are not rendered - Users must pre-create RBAC resources externally Default behavior (false) is unchanged. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent e469d48 commit 57e9006

21 files changed

+250
-45
lines changed

cmd/thv-operator/controllers/mcpregistry_controller.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,8 @@ type MCPRegistryReconciler struct {
4747
}
4848

4949
// NewMCPRegistryReconciler creates a new MCPRegistryReconciler with required dependencies
50-
func NewMCPRegistryReconciler(k8sClient client.Client, scheme *runtime.Scheme) *MCPRegistryReconciler {
51-
registryAPIManager := registryapi.NewManager(k8sClient, scheme)
50+
func NewMCPRegistryReconciler(k8sClient client.Client, scheme *runtime.Scheme, disableWorkloadRBAC bool) *MCPRegistryReconciler {
51+
registryAPIManager := registryapi.NewManager(k8sClient, scheme, disableWorkloadRBAC)
5252
return &MCPRegistryReconciler{
5353
Client: k8sClient,
5454
Scheme: scheme,

cmd/thv-operator/controllers/mcpremoteproxy_controller.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,10 @@ import (
3737
// MCPRemoteProxyReconciler reconciles a MCPRemoteProxy object
3838
type MCPRemoteProxyReconciler struct {
3939
client.Client
40-
Scheme *runtime.Scheme
41-
Recorder record.EventRecorder
42-
PlatformDetector *ctrlutil.SharedPlatformDetector
40+
Scheme *runtime.Scheme
41+
Recorder record.EventRecorder
42+
PlatformDetector *ctrlutil.SharedPlatformDetector
43+
DisableWorkloadRBAC bool
4344
}
4445

4546
// +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=mcpremoteproxies,verbs=get;list;watch;create;update;patch;delete
@@ -584,6 +585,10 @@ func (r *MCPRemoteProxyReconciler) validateGroupRef(ctx context.Context, proxy *
584585
// Uses the RBAC client (pkg/kubernetes/rbac) which creates or updates RBAC resources
585586
// automatically during operator upgrades.
586587
func (r *MCPRemoteProxyReconciler) ensureRBACResources(ctx context.Context, proxy *mcpv1alpha1.MCPRemoteProxy) error {
588+
if r.DisableWorkloadRBAC {
589+
return nil
590+
}
591+
587592
// If a service account is specified, we don't need to create one
588593
if proxy.Spec.ServiceAccount != nil {
589594
return nil

cmd/thv-operator/controllers/mcpremoteproxy_controller_test.go

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1099,6 +1099,50 @@ func TestMCPRemoteProxyEnsureRBACResources_CustomServiceAccount(t *testing.T) {
10991099
assert.Error(t, err, "RoleBinding should not be created when custom ServiceAccount is provided")
11001100
}
11011101

1102+
// TestEnsureRBACResources_DisableWorkloadRBAC_MCPRemoteProxy tests that RBAC creation is skipped
1103+
// when DisableWorkloadRBAC is true.
1104+
func TestEnsureRBACResources_DisableWorkloadRBAC_MCPRemoteProxy(t *testing.T) {
1105+
t.Parallel()
1106+
1107+
proxy := &mcpv1alpha1.MCPRemoteProxy{
1108+
ObjectMeta: metav1.ObjectMeta{
1109+
Name: "test-proxy-disable-rbac",
1110+
Namespace: "default",
1111+
},
1112+
Spec: mcpv1alpha1.MCPRemoteProxySpec{
1113+
RemoteURL: "https://mcp.example.com",
1114+
Port: 8080,
1115+
},
1116+
}
1117+
1118+
testScheme := createRunConfigTestScheme()
1119+
_ = rbacv1.AddToScheme(testScheme)
1120+
1121+
fakeClient := fake.NewClientBuilder().
1122+
WithScheme(testScheme).
1123+
WithRuntimeObjects(proxy).
1124+
Build()
1125+
1126+
reconciler := &MCPRemoteProxyReconciler{
1127+
Client: fakeClient,
1128+
Scheme: testScheme,
1129+
DisableWorkloadRBAC: true,
1130+
}
1131+
1132+
err := reconciler.ensureRBACResources(t.Context(), proxy)
1133+
require.NoError(t, err)
1134+
1135+
// Verify NO RBAC resources were created
1136+
generatedSAName := proxyRunnerServiceAccountNameForRemoteProxy(proxy.Name)
1137+
1138+
sa := &corev1.ServiceAccount{}
1139+
err = fakeClient.Get(t.Context(), types.NamespacedName{
1140+
Name: generatedSAName,
1141+
Namespace: proxy.Namespace,
1142+
}, sa)
1143+
assert.Error(t, err, "ServiceAccount should not be created when workload RBAC is disabled")
1144+
}
1145+
11021146
// TestUpdateMCPRemoteProxyStatus tests status update logic
11031147
func TestUpdateMCPRemoteProxyStatus(t *testing.T) {
11041148
t.Parallel()

cmd/thv-operator/controllers/mcpserver_controller.go

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,11 @@ import (
4646
// MCPServerReconciler reconciles a MCPServer object
4747
type MCPServerReconciler struct {
4848
client.Client
49-
Scheme *runtime.Scheme
50-
Recorder record.EventRecorder
51-
PlatformDetector *ctrlutil.SharedPlatformDetector
52-
ImageValidation validation.ImageValidation
49+
Scheme *runtime.Scheme
50+
Recorder record.EventRecorder
51+
PlatformDetector *ctrlutil.SharedPlatformDetector
52+
ImageValidation validation.ImageValidation
53+
DisableWorkloadRBAC bool
5354
}
5455

5556
// defaultRBACRules are the default RBAC rules that the
@@ -884,7 +885,12 @@ func (r *MCPServerReconciler) handleToolConfig(ctx context.Context, m *mcpv1alph
884885

885886
return nil
886887
}
888+
887889
func (r *MCPServerReconciler) ensureRBACResources(ctx context.Context, mcpServer *mcpv1alpha1.MCPServer) error {
890+
if r.DisableWorkloadRBAC {
891+
return nil
892+
}
893+
888894
rbacClient := rbac.NewClient(r.Client, r.Scheme)
889895
proxyRunnerNameForRBAC := ctrlutil.ProxyRunnerServiceAccountName(mcpServer.Name)
890896

cmd/thv-operator/controllers/mcpserver_rbac_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -424,6 +424,23 @@ func TestEnsureRBACResources_ImagePullSecrets(t *testing.T) {
424424
assert.Equal(t, expectedSecrets, mcpServerSA.ImagePullSecrets)
425425
}
426426

427+
func TestEnsureRBACResources_DisableWorkloadRBAC(t *testing.T) {
428+
t.Parallel()
429+
tc := setupTest("test-server-disable-rbac", "default")
430+
tc.reconciler.DisableWorkloadRBAC = true
431+
432+
err := tc.ensureRBACResources()
433+
require.NoError(t, err)
434+
435+
// Verify NO RBAC resources were created
436+
sa := &corev1.ServiceAccount{}
437+
err = tc.client.Get(t.Context(), types.NamespacedName{
438+
Name: tc.proxyRunnerNameForRBAC,
439+
Namespace: tc.mcpServer.Namespace,
440+
}, sa)
441+
assert.Error(t, err, "ServiceAccount should not be created when workload RBAC is disabled")
442+
}
443+
427444
func createTestMCPServer(name, namespace string) *mcpv1alpha1.MCPServer {
428445
return &mcpv1alpha1.MCPServer{
429446
ObjectMeta: metav1.ObjectMeta{

cmd/thv-operator/controllers/virtualmcpserver_controller.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,10 @@ type AuthConfigError struct {
8383
// may not have owner references (StatefulSet, headless Service, RunConfig ConfigMap).
8484
type VirtualMCPServerReconciler struct {
8585
client.Client
86-
Scheme *runtime.Scheme
87-
Recorder record.EventRecorder
88-
PlatformDetector *ctrlutil.SharedPlatformDetector
86+
Scheme *runtime.Scheme
87+
Recorder record.EventRecorder
88+
PlatformDetector *ctrlutil.SharedPlatformDetector
89+
DisableWorkloadRBAC bool
8990
}
9091

9192
// +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=virtualmcpservers,verbs=get;list;watch;create;update;patch;delete
@@ -630,6 +631,10 @@ func (r *VirtualMCPServerReconciler) ensureRBACResources(
630631
ctx context.Context,
631632
vmcp *mcpv1alpha1.VirtualMCPServer,
632633
) error {
634+
if r.DisableWorkloadRBAC {
635+
return nil
636+
}
637+
633638
// If a service account is specified, we don't need to create one
634639
if vmcp.Spec.ServiceAccount != nil {
635640
return nil

cmd/thv-operator/controllers/virtualmcpserver_controller_test.go

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -630,6 +630,52 @@ func TestVirtualMCPServerEnsureRBACResources_CustomServiceAccount(t *testing.T)
630630
assert.Error(t, err, "RoleBinding should not be created when custom ServiceAccount is provided")
631631
}
632632

633+
// TestVirtualMCPServerEnsureRBACResources_DisableWorkloadRBAC tests that RBAC creation is skipped
634+
// when DisableWorkloadRBAC is true.
635+
func TestVirtualMCPServerEnsureRBACResources_DisableWorkloadRBAC(t *testing.T) {
636+
t.Parallel()
637+
638+
vmcp := &mcpv1alpha1.VirtualMCPServer{
639+
ObjectMeta: metav1.ObjectMeta{
640+
Name: "test-vmcp-disable-rbac",
641+
Namespace: "default",
642+
UID: "test-uid",
643+
},
644+
Spec: mcpv1alpha1.VirtualMCPServerSpec{
645+
Config: vmcpconfig.Config{Group: testGroupName},
646+
},
647+
}
648+
649+
scheme := runtime.NewScheme()
650+
_ = mcpv1alpha1.AddToScheme(scheme)
651+
_ = corev1.AddToScheme(scheme)
652+
_ = rbacv1.AddToScheme(scheme)
653+
654+
fakeClient := fake.NewClientBuilder().
655+
WithScheme(scheme).
656+
WithObjects(vmcp).
657+
Build()
658+
659+
r := &VirtualMCPServerReconciler{
660+
Client: fakeClient,
661+
Scheme: scheme,
662+
DisableWorkloadRBAC: true,
663+
}
664+
665+
err := r.ensureRBACResources(t.Context(), vmcp)
666+
require.NoError(t, err)
667+
668+
// Verify NO RBAC resources were created
669+
generatedSAName := vmcpServiceAccountName(vmcp.Name)
670+
671+
sa := &corev1.ServiceAccount{}
672+
err = fakeClient.Get(t.Context(), types.NamespacedName{
673+
Name: generatedSAName,
674+
Namespace: vmcp.Namespace,
675+
}, sa)
676+
assert.Error(t, err, "ServiceAccount should not be created when workload RBAC is disabled")
677+
}
678+
633679
// TestVirtualMCPServerEnsureDeployment tests Deployment creation
634680
func TestVirtualMCPServerEnsureDeployment(t *testing.T) {
635681
t.Parallel()

cmd/thv-operator/main.go

Lines changed: 35 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,10 @@ const (
4646
featureServer = "ENABLE_SERVER"
4747
featureRegistry = "ENABLE_REGISTRY"
4848
featureVMCP = "ENABLE_VMCP"
49+
// disableWorkloadRBAC disables per-workload RBAC management (ServiceAccount, Role, RoleBinding).
50+
// When enabled, the operator will not create RBAC resources for workloads,
51+
// allowing them to be managed externally (e.g., via per-workload Helm charts).
52+
disableWorkloadRBAC = "DISABLE_WORKLOAD_RBAC"
4953
)
5054

5155
// controllerDependencies maps each controller group to its required dependencies
@@ -131,6 +135,11 @@ func setupControllersAndWebhooks(mgr ctrl.Manager) error {
131135
enableServer := isFeatureEnabled(featureServer, true)
132136
enableRegistry := isFeatureEnabled(featureRegistry, true)
133137
enableVMCP := isFeatureEnabled(featureVMCP, true)
138+
workloadRBACDisabled := isFeatureEnabled(disableWorkloadRBAC, false)
139+
140+
if workloadRBACDisabled {
141+
setupLog.Info("DISABLE_WORKLOAD_RBAC is enabled, operator will not create per-workload RBAC resources")
142+
}
134143

135144
// Track enabled features for dependency checking
136145
enabledFeatures := map[string]bool{
@@ -159,7 +168,7 @@ func setupControllersAndWebhooks(mgr ctrl.Manager) error {
159168

160169
// Set up server-related controllers
161170
if enabledFeatures[featureServer] {
162-
if err := setupServerControllers(mgr, enableRegistry); err != nil {
171+
if err := setupServerControllers(mgr, enableRegistry, workloadRBACDisabled); err != nil {
163172
return err
164173
}
165174
} else {
@@ -168,7 +177,7 @@ func setupControllersAndWebhooks(mgr ctrl.Manager) error {
168177

169178
// Set up registry controller
170179
if enabledFeatures[featureRegistry] {
171-
if err := setupRegistryController(mgr); err != nil {
180+
if err := setupRegistryController(mgr, workloadRBACDisabled); err != nil {
172181
return err
173182
}
174183
} else {
@@ -177,7 +186,7 @@ func setupControllersAndWebhooks(mgr ctrl.Manager) error {
177186

178187
// Set up Virtual MCP controllers and webhooks
179188
if enabledFeatures[featureVMCP] {
180-
if err := setupAggregationControllers(mgr); err != nil {
189+
if err := setupAggregationControllers(mgr, workloadRBACDisabled); err != nil {
181190
return err
182191
}
183192
} else {
@@ -189,7 +198,7 @@ func setupControllersAndWebhooks(mgr ctrl.Manager) error {
189198
}
190199

191200
// setupServerControllers sets up server-related controllers (MCPServer, MCPExternalAuthConfig, MCPRemoteProxy, ToolConfig)
192-
func setupServerControllers(mgr ctrl.Manager, enableRegistry bool) error {
201+
func setupServerControllers(mgr ctrl.Manager, enableRegistry, disableWorkloadRBAC bool) error {
193202
// Set up field indexing for MCPServer.Spec.GroupRef
194203
if err := mgr.GetFieldIndexer().IndexField(
195204
context.Background(),
@@ -232,11 +241,12 @@ func setupServerControllers(mgr ctrl.Manager, enableRegistry bool) error {
232241

233242
// Set up MCPServer controller
234243
rec := &controllers.MCPServerReconciler{
235-
Client: mgr.GetClient(),
236-
Scheme: mgr.GetScheme(),
237-
Recorder: mgr.GetEventRecorderFor("mcpserver-controller"),
238-
PlatformDetector: ctrlutil.NewSharedPlatformDetector(),
239-
ImageValidation: imageValidation,
244+
Client: mgr.GetClient(),
245+
Scheme: mgr.GetScheme(),
246+
Recorder: mgr.GetEventRecorderFor("mcpserver-controller"),
247+
PlatformDetector: ctrlutil.NewSharedPlatformDetector(),
248+
ImageValidation: imageValidation,
249+
DisableWorkloadRBAC: disableWorkloadRBAC,
240250
}
241251
if err := rec.SetupWithManager(mgr); err != nil {
242252
return fmt.Errorf("unable to create controller MCPServer: %w", err)
@@ -260,10 +270,11 @@ func setupServerControllers(mgr ctrl.Manager, enableRegistry bool) error {
260270

261271
// Set up MCPRemoteProxy controller
262272
if err := (&controllers.MCPRemoteProxyReconciler{
263-
Client: mgr.GetClient(),
264-
Scheme: mgr.GetScheme(),
265-
Recorder: mgr.GetEventRecorderFor("mcpremoteproxy-controller"),
266-
PlatformDetector: ctrlutil.NewSharedPlatformDetector(),
273+
Client: mgr.GetClient(),
274+
Scheme: mgr.GetScheme(),
275+
Recorder: mgr.GetEventRecorderFor("mcpremoteproxy-controller"),
276+
PlatformDetector: ctrlutil.NewSharedPlatformDetector(),
277+
DisableWorkloadRBAC: disableWorkloadRBAC,
267278
}).SetupWithManager(mgr); err != nil {
268279
return fmt.Errorf("unable to create controller MCPRemoteProxy: %w", err)
269280
}
@@ -283,8 +294,11 @@ func setupServerControllers(mgr ctrl.Manager, enableRegistry bool) error {
283294
}
284295

285296
// setupRegistryController sets up the MCPRegistry controller
286-
func setupRegistryController(mgr ctrl.Manager) error {
287-
if err := (controllers.NewMCPRegistryReconciler(mgr.GetClient(), mgr.GetScheme())).SetupWithManager(mgr); err != nil {
297+
func setupRegistryController(mgr ctrl.Manager, disableWorkloadRBAC bool) error {
298+
reconciler := controllers.NewMCPRegistryReconciler(
299+
mgr.GetClient(), mgr.GetScheme(), disableWorkloadRBAC,
300+
)
301+
if err := reconciler.SetupWithManager(mgr); err != nil {
288302
return fmt.Errorf("unable to create controller MCPRegistry: %w", err)
289303
}
290304
return nil
@@ -294,7 +308,7 @@ func setupRegistryController(mgr ctrl.Manager) error {
294308
// (MCPGroup, VirtualMCPServer, and their webhooks)
295309
// Note: This function assumes server controllers are enabled (enforced by dependency check)
296310
// The field index for MCPServer.Spec.GroupRef is created in setupServerControllers
297-
func setupAggregationControllers(mgr ctrl.Manager) error {
311+
func setupAggregationControllers(mgr ctrl.Manager, disableWorkloadRBAC bool) error {
298312
// Set up MCPGroup controller
299313
if err := (&controllers.MCPGroupReconciler{
300314
Client: mgr.GetClient(),
@@ -304,10 +318,11 @@ func setupAggregationControllers(mgr ctrl.Manager) error {
304318

305319
// Set up VirtualMCPServer controller
306320
if err := (&controllers.VirtualMCPServerReconciler{
307-
Client: mgr.GetClient(),
308-
Scheme: mgr.GetScheme(),
309-
Recorder: mgr.GetEventRecorderFor("virtualmcpserver-controller"),
310-
PlatformDetector: ctrlutil.NewSharedPlatformDetector(),
321+
Client: mgr.GetClient(),
322+
Scheme: mgr.GetScheme(),
323+
Recorder: mgr.GetEventRecorderFor("virtualmcpserver-controller"),
324+
PlatformDetector: ctrlutil.NewSharedPlatformDetector(),
325+
DisableWorkloadRBAC: disableWorkloadRBAC,
311326
}).SetupWithManager(mgr); err != nil {
312327
return fmt.Errorf("unable to create controller VirtualMCPServer: %w", err)
313328
}

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

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,20 +21,23 @@ import (
2121

2222
// manager implements the Manager interface
2323
type manager struct {
24-
client client.Client
25-
scheme *runtime.Scheme
26-
kubeHelper *kubernetes.Client
24+
client client.Client
25+
scheme *runtime.Scheme
26+
kubeHelper *kubernetes.Client
27+
disableWorkloadRBAC bool
2728
}
2829

2930
// NewManager creates a new registry API manager
3031
func NewManager(
3132
k8sClient client.Client,
3233
scheme *runtime.Scheme,
34+
disableWorkloadRBAC bool,
3335
) Manager {
3436
return &manager{
35-
client: k8sClient,
36-
scheme: scheme,
37-
kubeHelper: kubernetes.NewClient(k8sClient, scheme),
37+
client: k8sClient,
38+
scheme: scheme,
39+
kubeHelper: kubernetes.NewClient(k8sClient, scheme),
40+
disableWorkloadRBAC: disableWorkloadRBAC,
3841
}
3942
}
4043

0 commit comments

Comments
 (0)