Skip to content

Commit 95da812

Browse files
RoddieKieleyCursor o3
RoddieKieley
and
Cursor o3
committed
Deploy ToolHive Operator into OpenShift (#1063)
 * Update helm chart resources and seccompProfile type values for OKD environment. * Update MCPServer Deployment with check for XDG_CONFIG_HOME and HOME env vars being set. * Add PlatformDetector interface with detection implementation for Kubernetes and OpenShift. * Update behaviour of tests now requiring in cluster to skip locally. Signed-off-by: Roddie Kieley <[email protected]> Co-authored-by: Cursor o3 <[email protected]> Co-authored-by: Cursor claude-4 <[email protected]>
1 parent e1eccad commit 95da812

File tree

7 files changed

+416
-33
lines changed

7 files changed

+416
-33
lines changed

‎cmd/thv-operator/controllers/mcpserver_controller.go

Lines changed: 75 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -556,6 +556,8 @@ func (r *MCPServerReconciler) deploymentForMCPServer(m *mcpv1alpha1.MCPServer) *
556556
}
557557
}
558558

559+
env = ensureRequiredEnvVars(env)
560+
559561
dep := &appsv1.Deployment{
560562
ObjectMeta: metav1.ObjectMeta{
561563
Name: m.Name,
@@ -625,6 +627,36 @@ func (r *MCPServerReconciler) deploymentForMCPServer(m *mcpv1alpha1.MCPServer) *
625627
return dep
626628
}
627629

630+
func ensureRequiredEnvVars(env []corev1.EnvVar) []corev1.EnvVar {
631+
// Check for the existence of the XDG_CONFIG_HOME and HOME environment variables
632+
// and set them to /tmp if they don't exist
633+
xdgConfigHomeFound := false
634+
homeFound := false
635+
for _, envVar := range env {
636+
if envVar.Name == "XDG_CONFIG_HOME" {
637+
xdgConfigHomeFound = true
638+
}
639+
if envVar.Name == "HOME" {
640+
homeFound = true
641+
}
642+
}
643+
if !xdgConfigHomeFound {
644+
logger.Debugf("XDG_CONFIG_HOME not found, setting to /tmp")
645+
env = append(env, corev1.EnvVar{
646+
Name: "XDG_CONFIG_HOME",
647+
Value: "/tmp",
648+
})
649+
}
650+
if !homeFound {
651+
logger.Debugf("HOME not found, setting to /tmp")
652+
env = append(env, corev1.EnvVar{
653+
Name: "HOME",
654+
Value: "/tmp",
655+
})
656+
}
657+
return env
658+
}
659+
628660
func createServiceName(mcpServerName string) string {
629661
return fmt.Sprintf("mcp-%s-proxy", mcpServerName)
630662
}
@@ -822,20 +854,9 @@ func deploymentNeedsUpdate(deployment *appsv1.Deployment, mcpServer *mcpv1alpha1
822854
return true
823855
}
824856

825-
// Check if the tools filter has changed
826-
if mcpServer.Spec.ToolsFilter == nil {
827-
for _, arg := range container.Args {
828-
if strings.HasPrefix(arg, "--tools=") {
829-
return true
830-
}
831-
}
832-
} else {
833-
slices.Sort(mcpServer.Spec.ToolsFilter)
834-
toolsFilterArg := fmt.Sprintf("--tools=%s", strings.Join(mcpServer.Spec.ToolsFilter, ","))
835-
found = slices.Contains(container.Args, toolsFilterArg)
836-
if !found {
837-
return true
838-
}
857+
// Check if the tools filter has changed (order-independent)
858+
if !equalToolsFilter(mcpServer.Spec.ToolsFilter, container.Args) {
859+
return true
839860
}
840861

841862
// Check if the pod template spec has changed
@@ -890,6 +911,8 @@ func deploymentNeedsUpdate(deployment *appsv1.Deployment, mcpServer *mcpv1alpha1
890911
})
891912
}
892913
}
914+
// Add default environment variables that are always injected
915+
expectedProxyEnv = ensureRequiredEnvVars(expectedProxyEnv)
893916
if !reflect.DeepEqual(container.Env, expectedProxyEnv) {
894917
return true
895918
}
@@ -952,8 +975,10 @@ func deploymentNeedsUpdate(deployment *appsv1.Deployment, mcpServer *mcpv1alpha1
952975
}
953976

954977
// Check if the service account name has changed
978+
// ServiceAccountName: treat empty (not yet set) as equal to the expected default
955979
expectedServiceAccountName := proxyRunnerServiceAccountName(mcpServer.Name)
956-
if deployment.Spec.Template.Spec.ServiceAccountName != expectedServiceAccountName {
980+
currentServiceAccountName := deployment.Spec.Template.Spec.ServiceAccountName
981+
if currentServiceAccountName != "" && currentServiceAccountName != expectedServiceAccountName {
957982
return true
958983
}
959984

@@ -1568,3 +1593,38 @@ func (r *MCPServerReconciler) SetupWithManager(mgr ctrl.Manager) error {
15681593
Owns(&corev1.Service{}).
15691594
Complete(r)
15701595
}
1596+
1597+
// equalToolsFilter returns true when the desired toolsFilter slice and the
1598+
// currently-applied `--tools=` argument in the container args represent the
1599+
// same unordered set of tools.
1600+
func equalToolsFilter(spec []string, args []string) bool {
1601+
// Build canonical form for spec
1602+
specCanon := canonicalToolsList(spec)
1603+
1604+
// Extract current tools argument (if any) from args
1605+
var currentArg string
1606+
for _, a := range args {
1607+
if strings.HasPrefix(a, "--tools=") {
1608+
currentArg = strings.TrimPrefix(a, "--tools=")
1609+
break
1610+
}
1611+
}
1612+
1613+
if specCanon == "" && currentArg == "" {
1614+
return true // both unset/empty
1615+
}
1616+
1617+
// Canonicalise current list
1618+
currentCanon := canonicalToolsList(strings.Split(strings.TrimSpace(currentArg), ","))
1619+
return specCanon == currentCanon
1620+
}
1621+
1622+
// canonicalToolsList sorts a slice and joins it with commas; empty slice yields "".
1623+
func canonicalToolsList(list []string) string {
1624+
if len(list) == 0 || (len(list) == 1 && list[0] == "") {
1625+
return ""
1626+
}
1627+
cp := slices.Clone(list)
1628+
slices.Sort(cp)
1629+
return strings.Join(cp, ",")
1630+
}

‎cmd/thv-operator/controllers/mcpserver_resource_overrides_test.go

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -288,14 +288,18 @@ func TestResourceOverrides(t *testing.T) {
288288
var expectedEnvVars map[string]string
289289
if tt.name == "with proxy environment variables" {
290290
expectedEnvVars = map[string]string{
291-
"HTTP_PROXY": "http://proxy.example.com:8080",
292-
"NO_PROXY": "localhost,127.0.0.1",
293-
"CUSTOM_ENV": "custom-value",
291+
"HTTP_PROXY": "http://proxy.example.com:8080",
292+
"NO_PROXY": "localhost,127.0.0.1",
293+
"CUSTOM_ENV": "custom-value",
294+
"XDG_CONFIG_HOME": "/tmp",
295+
"HOME": "/tmp",
294296
}
295297
} else {
296298
expectedEnvVars = map[string]string{
297299
"LOG_LEVEL": "debug",
298300
"METRICS_ENABLED": "true",
301+
"XDG_CONFIG_HOME": "/tmp",
302+
"HOME": "/tmp",
299303
}
300304
}
301305

@@ -374,8 +378,9 @@ func TestDeploymentNeedsUpdateServiceAccount(t *testing.T) {
374378
Namespace: "default",
375379
},
376380
Spec: mcpv1alpha1.MCPServerSpec{
377-
Image: "test-image",
378-
Port: 8080,
381+
Image: "test-image",
382+
Port: 8080,
383+
Transport: "stdio",
379384
},
380385
}
381386

@@ -634,6 +639,7 @@ func TestDeploymentNeedsUpdateToolsFilter(t *testing.T) {
634639
Spec: mcpv1alpha1.MCPServerSpec{
635640
Image: "test-image",
636641
Port: 8080,
642+
Transport: "stdio",
637643
ToolsFilter: tt.initialToolsFilter,
638644
},
639645
}

‎deploy/charts/operator/values.yaml

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,14 @@ operator:
4242
# -- Pod security context settings
4343
podSecurityContext:
4444
runAsNonRoot: true
45+
seccompProfile:
46+
type: RuntimeDefault
4547

4648
# -- Container security context settings for the operator
4749
containerSecurityContext:
4850
allowPrivilegeEscalation: false
4951
readOnlyRootFilesystem: true
5052
runAsNonRoot: true
51-
runAsUser: 1000
5253
capabilities:
5354
drop:
5455
- ALL
@@ -85,10 +86,10 @@ operator:
8586
resources:
8687
limits:
8788
cpu: 500m
88-
memory: 128Mi
89+
memory: 384Mi
8990
requests:
9091
cpu: 10m
91-
memory: 64Mi
92+
memory: 192Mi
9293

9394
# -- RBAC configuration for the operator
9495
rbac:

‎pkg/container/kubernetes/client.go

Lines changed: 84 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -246,17 +246,31 @@ func (c *Client) DeployWorkload(ctx context.Context,
246246
}
247247

248248
// Ensure the pod template has required configuration (labels, etc.)
249-
podTemplateSpec = ensurePodTemplateConfig(podTemplateSpec, containerLabels)
249+
// Get a config to talk to the apiserver
250+
cfg, err := rest.InClusterConfig()
251+
if err != nil {
252+
return 0, fmt.Errorf("error getting in-cluster config for APIServer: %w", err)
253+
}
254+
255+
// Detect platform type
256+
platformDetector := NewDefaultPlatformDetector()
257+
platform, err := platformDetector.DetectPlatform(cfg)
258+
if err != nil {
259+
return 0, fmt.Errorf("can't determine api server type: %w", err)
260+
}
261+
262+
podTemplateSpec = ensurePodTemplateConfig(podTemplateSpec, containerLabels, platform)
250263

251264
// Configure the MCP container
252-
err := configureMCPContainer(
265+
err = configureMCPContainer(
253266
podTemplateSpec,
254267
image,
255268
command,
256269
attachStdio,
257270
envVarList,
258271
transportType,
259272
options,
273+
platform,
260274
)
261275
if err != nil {
262276
return 0, err
@@ -891,6 +905,7 @@ func createPodTemplateFromPatch(patchJSON string) (*corev1apply.PodTemplateSpecA
891905
func ensurePodTemplateConfig(
892906
podTemplateSpec *corev1apply.PodTemplateSpecApplyConfiguration,
893907
containerLabels map[string]string,
908+
platform Platform,
894909
) *corev1apply.PodTemplateSpecApplyConfiguration {
895910
podTemplateSpec = ensureObjectMetaApplyConfigurationExists(podTemplateSpec)
896911
// Ensure the pod template has labels
@@ -940,6 +955,31 @@ func ensurePodTemplateConfig(
940955
podTemplateSpec.Spec.SecurityContext = podTemplateSpec.Spec.SecurityContext.WithRunAsGroup(int64(1000))
941956
}
942957
}
958+
959+
if platform == PlatformOpenShift {
960+
if podTemplateSpec.Spec.SecurityContext.RunAsUser != nil {
961+
podTemplateSpec.Spec.SecurityContext.RunAsUser = nil
962+
}
963+
964+
if podTemplateSpec.Spec.SecurityContext.RunAsGroup != nil {
965+
podTemplateSpec.Spec.SecurityContext.RunAsGroup = nil
966+
}
967+
968+
if podTemplateSpec.Spec.SecurityContext.FSGroup != nil {
969+
podTemplateSpec.Spec.SecurityContext.FSGroup = nil
970+
}
971+
972+
if podTemplateSpec.Spec.SecurityContext.SeccompProfile == nil {
973+
podTemplateSpec.Spec.SecurityContext.SeccompProfile =
974+
corev1apply.SeccompProfile().WithType(
975+
corev1.SeccompProfileTypeRuntimeDefault)
976+
} else {
977+
podTemplateSpec.Spec.SecurityContext.SeccompProfile =
978+
podTemplateSpec.Spec.SecurityContext.SeccompProfile.WithType(
979+
corev1.SeccompProfileTypeRuntimeDefault)
980+
}
981+
}
982+
943983
return podTemplateSpec
944984
}
945985

@@ -985,7 +1025,18 @@ func configureContainer(
9851025
command []string,
9861026
attachStdio bool,
9871027
envVars []*corev1apply.EnvVarApplyConfiguration,
1028+
platform Platform,
9881029
) {
1030+
logger.Infof("Configuring container %s with image %s", *container.Name, image)
1031+
logger.Infof("Command: ")
1032+
for _, arg := range command {
1033+
logger.Infof("Arg: %s", arg)
1034+
}
1035+
logger.Infof("AttachStdio: %v", attachStdio)
1036+
for _, envVar := range envVars {
1037+
logger.Infof("EnvVar: %s=%s", *envVar.Name, *envVar.Value)
1038+
}
1039+
9891040
container.WithImage(image).
9901041
WithArgs(command...).
9911042
WithStdin(attachStdio).
@@ -1029,6 +1080,34 @@ func configureContainer(
10291080
container.SecurityContext = container.SecurityContext.WithAllowPrivilegeEscalation(false)
10301081
}
10311082
}
1083+
1084+
if platform == PlatformOpenShift {
1085+
logger.Infof("Setting OpenShift security context requirements to container %s", *container.Name)
1086+
1087+
if container.SecurityContext.RunAsUser != nil {
1088+
container.SecurityContext.RunAsUser = nil
1089+
}
1090+
1091+
if container.SecurityContext.RunAsGroup != nil {
1092+
container.SecurityContext.RunAsGroup = nil
1093+
}
1094+
1095+
if container.SecurityContext.SeccompProfile == nil {
1096+
container.SecurityContext.SeccompProfile =
1097+
corev1apply.SeccompProfile().WithType(
1098+
corev1.SeccompProfileTypeRuntimeDefault)
1099+
} else {
1100+
container.SecurityContext.SeccompProfile =
1101+
container.SecurityContext.SeccompProfile.WithType(
1102+
corev1.SeccompProfileTypeRuntimeDefault)
1103+
}
1104+
1105+
if container.SecurityContext.Capabilities == nil {
1106+
container.SecurityContext.Capabilities = &corev1apply.CapabilitiesApplyConfiguration{
1107+
Drop: []corev1.Capability{"ALL"},
1108+
}
1109+
}
1110+
}
10321111
}
10331112

10341113
// configureMCPContainer configures the MCP container in the pod template
@@ -1040,6 +1119,7 @@ func configureMCPContainer(
10401119
envVarList []*corev1apply.EnvVarApplyConfiguration,
10411120
transportType string,
10421121
options *runtime.DeployWorkloadOptions,
1122+
platform Platform,
10431123
) error {
10441124
// Get the "mcp" container if it exists
10451125
mcpContainer := getMCPContainer(podTemplateSpec)
@@ -1049,7 +1129,7 @@ func configureMCPContainer(
10491129
mcpContainer = corev1apply.Container().WithName("mcp")
10501130

10511131
// Configure the container
1052-
configureContainer(mcpContainer, image, command, attachStdio, envVarList)
1132+
configureContainer(mcpContainer, image, command, attachStdio, envVarList, platform)
10531133

10541134
// Configure ports if needed
10551135
if options != nil && transportType == string(transtypes.TransportTypeSSE) {
@@ -1064,7 +1144,7 @@ func configureMCPContainer(
10641144
podTemplateSpec.Spec.WithContainers(mcpContainer)
10651145
} else {
10661146
// Configure the existing container
1067-
configureContainer(mcpContainer, image, command, attachStdio, envVarList)
1147+
configureContainer(mcpContainer, image, command, attachStdio, envVarList, platform)
10681148

10691149
// Configure ports if needed
10701150
if options != nil && transportType == string(transtypes.TransportTypeSSE) {

0 commit comments

Comments
 (0)