From aabe60111f9cb817e47f79fcc42f8149bb5c8589 Mon Sep 17 00:00:00 2001 From: Juan Antonio Osorio Date: Thu, 16 Apr 2026 09:13:30 +0000 Subject: [PATCH 1/3] Fix embedded auth deployment drift checks --- .../controllers/mcpremoteproxy_controller.go | 9 + .../mcpremoteproxy_deployment_test.go | 180 ++++++++++++++++ .../controllers/mcpserver_controller.go | 14 ++ .../mcpserver_resource_overrides_test.go | 194 +++++++++++++++++- pkg/container/kubernetes/client_test.go | 119 ++++++++++- 5 files changed, 503 insertions(+), 13 deletions(-) diff --git a/cmd/thv-operator/controllers/mcpremoteproxy_controller.go b/cmd/thv-operator/controllers/mcpremoteproxy_controller.go index c73e902f41..5724239c12 100644 --- a/cmd/thv-operator/controllers/mcpremoteproxy_controller.go +++ b/cmd/thv-operator/controllers/mcpremoteproxy_controller.go @@ -1274,6 +1274,15 @@ func (r *MCPRemoteProxyReconciler) containerNeedsUpdate( // Check if environment variables have changed expectedEnv := r.buildEnvVarsForProxy(ctx, proxy) + if configName := ctrlutil.EmbeddedAuthServerConfigName(proxy.Spec.ExternalAuthConfigRef, proxy.Spec.AuthServerRef); configName != "" { + _, _, authServerEnvVars, err := ctrlutil.GenerateAuthServerConfigByName( + ctx, r.Client, proxy.Namespace, configName, + ) + if err != nil { + return true + } + expectedEnv = append(expectedEnv, authServerEnvVars...) + } if !reflect.DeepEqual(container.Env, expectedEnv) { return true } diff --git a/cmd/thv-operator/controllers/mcpremoteproxy_deployment_test.go b/cmd/thv-operator/controllers/mcpremoteproxy_deployment_test.go index fdbf86583b..6686f30d76 100644 --- a/cmd/thv-operator/controllers/mcpremoteproxy_deployment_test.go +++ b/cmd/thv-operator/controllers/mcpremoteproxy_deployment_test.go @@ -700,6 +700,186 @@ func TestEnsureService(t *testing.T) { } } +func TestMCPRemoteProxyDeploymentNeedsUpdate_EmbeddedAuthLegacyEnvStable(t *testing.T) { + t.Parallel() + + scheme := createRunConfigTestScheme() + + authConfig := &mcpv1alpha1.MCPExternalAuthConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "embedded-auth", + Namespace: "default", + }, + Spec: mcpv1alpha1.MCPExternalAuthConfigSpec{ + Type: mcpv1alpha1.ExternalAuthTypeEmbeddedAuthServer, + EmbeddedAuthServer: &mcpv1alpha1.EmbeddedAuthServerConfig{ + UpstreamProviders: []mcpv1alpha1.UpstreamProviderConfig{ + { + Name: "google", + Type: mcpv1alpha1.UpstreamProviderTypeOIDC, + OIDCConfig: &mcpv1alpha1.OIDCUpstreamConfig{ + IssuerURL: "https://accounts.google.com", + ClientID: "client-id", + ClientSecretRef: &mcpv1alpha1.SecretKeyRef{ + Name: "upstream-secret", + Key: "client-secret", + }, + }, + }, + }, + }, + }, + } + + proxy := &mcpv1alpha1.MCPRemoteProxy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-proxy", + Namespace: "default", + }, + Spec: mcpv1alpha1.MCPRemoteProxySpec{ + RemoteURL: "https://mcp.example.com", + ProxyPort: 8080, + ExternalAuthConfigRef: &mcpv1alpha1.ExternalAuthConfigRef{ + Name: authConfig.Name, + }, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithRuntimeObjects(authConfig). + Build() + + reconciler := &MCPRemoteProxyReconciler{ + Client: fakeClient, + Scheme: scheme, + PlatformDetector: ctrlutil.NewSharedPlatformDetector(), + } + + deployment := reconciler.deploymentForMCPRemoteProxy(t.Context(), proxy, "test-checksum") + require.NotNil(t, deployment) + + assert.False(t, reconciler.deploymentNeedsUpdate(t.Context(), deployment, proxy, "test-checksum")) +} + +func TestMCPRemoteProxyDeploymentNeedsUpdate_EmbeddedAuthAuthServerRefEnvStable(t *testing.T) { + t.Parallel() + + scheme := createRunConfigTestScheme() + + authConfig := &mcpv1alpha1.MCPExternalAuthConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "embedded-auth", + Namespace: "default", + }, + Spec: mcpv1alpha1.MCPExternalAuthConfigSpec{ + Type: mcpv1alpha1.ExternalAuthTypeEmbeddedAuthServer, + EmbeddedAuthServer: &mcpv1alpha1.EmbeddedAuthServerConfig{ + UpstreamProviders: []mcpv1alpha1.UpstreamProviderConfig{ + { + Name: "google", + Type: mcpv1alpha1.UpstreamProviderTypeOIDC, + OIDCConfig: &mcpv1alpha1.OIDCUpstreamConfig{ + IssuerURL: "https://accounts.google.com", + ClientID: "client-id", + ClientSecretRef: &mcpv1alpha1.SecretKeyRef{ + Name: "upstream-secret", + Key: "client-secret", + }, + }, + }, + }, + }, + }, + } + + proxy := &mcpv1alpha1.MCPRemoteProxy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-proxy", + Namespace: "default", + }, + Spec: mcpv1alpha1.MCPRemoteProxySpec{ + RemoteURL: "https://mcp.example.com", + ProxyPort: 8080, + AuthServerRef: &mcpv1alpha1.AuthServerRef{ + Kind: "MCPExternalAuthConfig", + Name: authConfig.Name, + }, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithRuntimeObjects(authConfig). + Build() + + reconciler := &MCPRemoteProxyReconciler{ + Client: fakeClient, + Scheme: scheme, + PlatformDetector: ctrlutil.NewSharedPlatformDetector(), + } + + deployment := reconciler.deploymentForMCPRemoteProxy(t.Context(), proxy, "test-checksum") + require.NotNil(t, deployment) + + assert.False(t, reconciler.deploymentNeedsUpdate(t.Context(), deployment, proxy, "test-checksum")) +} + +func TestMCPRemoteProxyDeploymentNeedsUpdate_TokenExchangeDoesNotDrift(t *testing.T) { + t.Parallel() + + scheme := createRunConfigTestScheme() + + authConfig := &mcpv1alpha1.MCPExternalAuthConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "exchange-config", + Namespace: "default", + }, + Spec: mcpv1alpha1.MCPExternalAuthConfigSpec{ + Type: mcpv1alpha1.ExternalAuthTypeTokenExchange, + TokenExchange: &mcpv1alpha1.TokenExchangeConfig{ + TokenURL: "https://oauth.example.com/token", + ClientID: "client-id", + ClientSecretRef: &mcpv1alpha1.SecretKeyRef{ + Name: "token-secret", + Key: "client-secret", + }, + Audience: "api", + }, + }, + } + + proxy := &mcpv1alpha1.MCPRemoteProxy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-proxy", + Namespace: "default", + }, + Spec: mcpv1alpha1.MCPRemoteProxySpec{ + RemoteURL: "https://mcp.example.com", + ProxyPort: 8080, + ExternalAuthConfigRef: &mcpv1alpha1.ExternalAuthConfigRef{ + Name: authConfig.Name, + }, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithRuntimeObjects(authConfig). + Build() + + reconciler := &MCPRemoteProxyReconciler{ + Client: fakeClient, + Scheme: scheme, + PlatformDetector: ctrlutil.NewSharedPlatformDetector(), + } + + deployment := reconciler.deploymentForMCPRemoteProxy(t.Context(), proxy, "test-checksum") + require.NotNil(t, deployment) + + assert.False(t, reconciler.deploymentNeedsUpdate(t.Context(), deployment, proxy, "test-checksum")) +} + // TestBuildEnvVarsForProxy tests environment variable building func TestBuildEnvVarsForProxy(t *testing.T) { t.Parallel() diff --git a/cmd/thv-operator/controllers/mcpserver_controller.go b/cmd/thv-operator/controllers/mcpserver_controller.go index 44068ccf43..a3f955ada8 100644 --- a/cmd/thv-operator/controllers/mcpserver_controller.go +++ b/cmd/thv-operator/controllers/mcpserver_controller.go @@ -1654,6 +1654,20 @@ func (r *MCPServerReconciler) deploymentNeedsUpdate( }) } } + + // Add embedded auth server environment variables. AuthServerRef takes precedence; + // externalAuthConfigRef is used as a fallback (legacy path). + if configName := ctrlutil.EmbeddedAuthServerConfigName( + mcpServer.Spec.ExternalAuthConfigRef, mcpServer.Spec.AuthServerRef, + ); configName != "" { + _, _, authServerEnvVars, err := ctrlutil.GenerateAuthServerConfigByName( + ctx, r.Client, mcpServer.Namespace, configName, + ) + if err != nil { + return true + } + expectedProxyEnv = append(expectedProxyEnv, authServerEnvVars...) + } // Add default environment variables that are always injected expectedProxyEnv = ctrlutil.EnsureRequiredEnvVars(ctx, expectedProxyEnv) if !equality.Semantic.DeepEqual(container.Env, expectedProxyEnv) { diff --git a/cmd/thv-operator/controllers/mcpserver_resource_overrides_test.go b/cmd/thv-operator/controllers/mcpserver_resource_overrides_test.go index ba6e516752..9adfedfb9f 100644 --- a/cmd/thv-operator/controllers/mcpserver_resource_overrides_test.go +++ b/cmd/thv-operator/controllers/mcpserver_resource_overrides_test.go @@ -15,7 +15,6 @@ package controllers import ( - "context" "testing" "github.com/stretchr/testify/assert" @@ -30,6 +29,189 @@ import ( "github.com/stacklok/toolhive/pkg/container/kubernetes" ) +func TestMCPServerDeploymentNeedsUpdate_EmbeddedAuthLegacyEnvStable(t *testing.T) { + t.Parallel() + + scheme := runtime.NewScheme() + require.NoError(t, corev1.AddToScheme(scheme)) + require.NoError(t, mcpv1alpha1.AddToScheme(scheme)) + + externalAuthConfig := &mcpv1alpha1.MCPExternalAuthConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "embedded-auth", + Namespace: "default", + }, + Spec: mcpv1alpha1.MCPExternalAuthConfigSpec{ + Type: mcpv1alpha1.ExternalAuthTypeEmbeddedAuthServer, + EmbeddedAuthServer: &mcpv1alpha1.EmbeddedAuthServerConfig{ + UpstreamProviders: []mcpv1alpha1.UpstreamProviderConfig{ + { + Name: "google", + Type: mcpv1alpha1.UpstreamProviderTypeOIDC, + OIDCConfig: &mcpv1alpha1.OIDCUpstreamConfig{ + IssuerURL: "https://accounts.google.com", + ClientID: "client-id", + ClientSecretRef: &mcpv1alpha1.SecretKeyRef{ + Name: "upstream-secret", + Key: "client-secret", + }, + }, + }, + }, + }, + }, + } + + mcpServer := &mcpv1alpha1.MCPServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-server", + Namespace: "default", + }, + Spec: mcpv1alpha1.MCPServerSpec{ + Image: "test-image", + ProxyPort: 8080, + ExternalAuthConfigRef: &mcpv1alpha1.ExternalAuthConfigRef{ + Name: externalAuthConfig.Name, + }, + }, + } + + client := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(externalAuthConfig). + Build() + r := newTestMCPServerReconciler(client, scheme, kubernetes.PlatformKubernetes) + + deployment := r.deploymentForMCPServer(t.Context(), mcpServer, "test-checksum") + require.NotNil(t, deployment) + require.Len(t, deployment.Spec.Template.Spec.Containers, 1) + require.Contains(t, deployment.Spec.Template.Spec.Containers[0].Env, corev1.EnvVar{ + Name: "TOOLHIVE_UPSTREAM_CLIENT_SECRET_GOOGLE", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "upstream-secret"}, + Key: "client-secret", + }, + }, + }) + + assert.False(t, r.deploymentNeedsUpdate(t.Context(), deployment, mcpServer, "test-checksum")) +} + +func TestMCPServerDeploymentNeedsUpdate_EmbeddedAuthAuthServerRefEnvStable(t *testing.T) { + t.Parallel() + + scheme := runtime.NewScheme() + require.NoError(t, corev1.AddToScheme(scheme)) + require.NoError(t, mcpv1alpha1.AddToScheme(scheme)) + + authConfig := &mcpv1alpha1.MCPExternalAuthConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "embedded-auth", + Namespace: "default", + }, + Spec: mcpv1alpha1.MCPExternalAuthConfigSpec{ + Type: mcpv1alpha1.ExternalAuthTypeEmbeddedAuthServer, + EmbeddedAuthServer: &mcpv1alpha1.EmbeddedAuthServerConfig{ + UpstreamProviders: []mcpv1alpha1.UpstreamProviderConfig{ + { + Name: "google", + Type: mcpv1alpha1.UpstreamProviderTypeOIDC, + OIDCConfig: &mcpv1alpha1.OIDCUpstreamConfig{ + IssuerURL: "https://accounts.google.com", + ClientID: "client-id", + ClientSecretRef: &mcpv1alpha1.SecretKeyRef{ + Name: "upstream-secret", + Key: "client-secret", + }, + }, + }, + }, + }, + }, + } + + mcpServer := &mcpv1alpha1.MCPServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-server", + Namespace: "default", + }, + Spec: mcpv1alpha1.MCPServerSpec{ + Image: "test-image", + ProxyPort: 8080, + AuthServerRef: &mcpv1alpha1.AuthServerRef{ + Kind: "MCPExternalAuthConfig", + Name: authConfig.Name, + }, + }, + } + + client := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(authConfig). + Build() + r := newTestMCPServerReconciler(client, scheme, kubernetes.PlatformKubernetes) + + deployment := r.deploymentForMCPServer(t.Context(), mcpServer, "test-checksum") + require.NotNil(t, deployment) + require.Len(t, deployment.Spec.Template.Spec.Containers, 1) + + assert.False(t, r.deploymentNeedsUpdate(t.Context(), deployment, mcpServer, "test-checksum")) +} + +func TestMCPServerDeploymentNeedsUpdate_TokenExchangeDoesNotDrift(t *testing.T) { + t.Parallel() + + scheme := runtime.NewScheme() + require.NoError(t, corev1.AddToScheme(scheme)) + require.NoError(t, mcpv1alpha1.AddToScheme(scheme)) + + authConfig := &mcpv1alpha1.MCPExternalAuthConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "exchange-config", + Namespace: "default", + }, + Spec: mcpv1alpha1.MCPExternalAuthConfigSpec{ + Type: mcpv1alpha1.ExternalAuthTypeTokenExchange, + TokenExchange: &mcpv1alpha1.TokenExchangeConfig{ + TokenURL: "https://oauth.example.com/token", + ClientID: "client-id", + ClientSecretRef: &mcpv1alpha1.SecretKeyRef{ + Name: "token-secret", + Key: "client-secret", + }, + Audience: "api", + }, + }, + } + + mcpServer := &mcpv1alpha1.MCPServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-server", + Namespace: "default", + }, + Spec: mcpv1alpha1.MCPServerSpec{ + Image: "test-image", + ProxyPort: 8080, + ExternalAuthConfigRef: &mcpv1alpha1.ExternalAuthConfigRef{ + Name: authConfig.Name, + }, + }, + } + + client := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(authConfig). + Build() + r := newTestMCPServerReconciler(client, scheme, kubernetes.PlatformKubernetes) + + deployment := r.deploymentForMCPServer(t.Context(), mcpServer, "test-checksum") + require.NotNil(t, deployment) + require.Len(t, deployment.Spec.Template.Spec.Containers, 1) + + assert.False(t, r.deploymentNeedsUpdate(t.Context(), deployment, mcpServer, "test-checksum")) +} + func TestResourceOverrides(t *testing.T) { t.Parallel() @@ -341,7 +523,7 @@ func TestResourceOverrides(t *testing.T) { r := newTestMCPServerReconciler(client, scheme, kubernetes.PlatformKubernetes) // Test deployment creation - ctx := context.Background() + ctx := t.Context() deployment := r.deploymentForMCPServer(ctx, tt.mcpServer, "test-checksum") require.NotNil(t, deployment) @@ -349,7 +531,7 @@ func TestResourceOverrides(t *testing.T) { assert.Equal(t, tt.expectedDeploymentAnns, deployment.Annotations) // Test service creation - service := r.serviceForMCPServer(context.Background(), tt.mcpServer) + service := r.serviceForMCPServer(t.Context(), tt.mcpServer) require.NotNil(t, service) assert.Equal(t, tt.expectedServiceLabels, service.Labels) @@ -615,7 +797,7 @@ func TestDeploymentNeedsUpdateProxyEnv(t *testing.T) { t.Parallel() // Create a deployment and manually set up its state to isolate proxy env testing - ctx := context.Background() + ctx := t.Context() deployment := r.deploymentForMCPServer(ctx, tt.mcpServer, "test-checksum") require.NotNil(t, deployment) require.Len(t, deployment.Spec.Template.Spec.Containers, 1) @@ -627,7 +809,7 @@ func TestDeploymentNeedsUpdateProxyEnv(t *testing.T) { deployment.Spec.Template.Spec.Containers[0].Image = getToolhiveRunnerImage() // Test if deployment needs update - should correlate with env change expectation - needsUpdate := r.deploymentNeedsUpdate(context.Background(), deployment, tt.mcpServer, "test-checksum") + needsUpdate := r.deploymentNeedsUpdate(t.Context(), deployment, tt.mcpServer, "test-checksum") if tt.expectEnvChange { assert.True(t, needsUpdate, "Expected deployment update due to proxy env change") @@ -663,7 +845,7 @@ func TestMCPServerSessionAffinityNone(t *testing.T) { client := fake.NewClientBuilder().WithScheme(scheme).Build() r := newTestMCPServerReconciler(client, scheme, kubernetes.PlatformKubernetes) - service := r.serviceForMCPServer(context.Background(), mcpServer) + service := r.serviceForMCPServer(t.Context(), mcpServer) require.NotNil(t, service) assert.Equal(t, corev1.ServiceAffinityNone, service.Spec.SessionAffinity) } diff --git a/pkg/container/kubernetes/client_test.go b/pkg/container/kubernetes/client_test.go index e68a65812f..861374ea26 100644 --- a/pkg/container/kubernetes/client_test.go +++ b/pkg/container/kubernetes/client_test.go @@ -6,6 +6,7 @@ package kubernetes import ( "context" "encoding/json" + "fmt" "strings" "testing" "time" @@ -16,7 +17,10 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" k8stypes "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/intstr" + appsv1apply "k8s.io/client-go/applyconfigurations/apps/v1" corev1apply "k8s.io/client-go/applyconfigurations/core/v1" + metav1apply "k8s.io/client-go/applyconfigurations/meta/v1" "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/fake" "k8s.io/client-go/rest" @@ -40,6 +44,107 @@ func (m *mockPlatformDetector) DetectPlatform(_ *rest.Config) (Platform, error) return m.platform, m.err } +func TestStatefulSetApplyAssociativeListReorderingDoesNotChangeGeneration(t *testing.T) { + t.Parallel() + + ctx := t.Context() + clientset := fake.NewClientset() + + buildApply := func(envOrder []string, portOrder []int32) *appsv1apply.StatefulSetApplyConfiguration { + envValues := map[string]string{ + "ALPHA": "1", + "BETA": "2", + } + + envVars := make([]*corev1apply.EnvVarApplyConfiguration, 0, len(envOrder)) + for _, name := range envOrder { + envVars = append(envVars, corev1apply.EnvVar().WithName(name).WithValue(envValues[name])) + } + + ports := make([]*corev1apply.ContainerPortApplyConfiguration, 0, len(portOrder)) + for _, port := range portOrder { + ports = append(ports, corev1apply.ContainerPort(). + WithContainerPort(port). + WithProtocol(corev1.ProtocolTCP)) + } + + return appsv1apply.StatefulSet("reordered", defaultNamespace). + WithSpec(appsv1apply.StatefulSetSpec(). + WithServiceName("reordered"). + WithReplicas(1). + WithSelector(metav1apply.LabelSelector(). + WithMatchLabels(map[string]string{"app": "reordered"})). + WithTemplate(corev1apply.PodTemplateSpec(). + WithLabels(map[string]string{"app": "reordered"}). + WithSpec(corev1apply.PodSpec(). + WithContainers(corev1apply.Container(). + WithName("mcp"). + WithImage("test-image"). + WithEnv(envVars...). + WithPorts(ports...))))) + } + + first, err := clientset.AppsV1().StatefulSets(defaultNamespace).Apply( + ctx, + buildApply([]string{"ALPHA", "BETA"}, []int32{8080, 9090}), + metav1.ApplyOptions{FieldManager: "test-manager", Force: true}, + ) + require.NoError(t, err) + + second, err := clientset.AppsV1().StatefulSets(defaultNamespace).Apply( + ctx, + buildApply([]string{"BETA", "ALPHA"}, []int32{9090, 8080}), + metav1.ApplyOptions{FieldManager: "test-manager", Force: true}, + ) + require.NoError(t, err) + + assert.Equal(t, first.Generation, second.Generation) + assert.Len(t, second.Spec.Template.Spec.Containers, 1) + assert.Len(t, second.Spec.Template.Spec.Containers[0].Env, 2) + assert.Len(t, second.Spec.Template.Spec.Containers[0].Ports, 2) +} + +func TestServiceApplyAssociativeListReorderingDoesNotChangeGeneration(t *testing.T) { + t.Parallel() + + ctx := t.Context() + clientset := fake.NewClientset() + + buildApply := func(portOrder []int32) *corev1apply.ServiceApplyConfiguration { + ports := make([]*corev1apply.ServicePortApplyConfiguration, 0, len(portOrder)) + for _, port := range portOrder { + ports = append(ports, corev1apply.ServicePort(). + WithName(fmt.Sprintf("port-%d", port)). + WithPort(port). + WithTargetPort(intstr.FromInt32(port)). + WithProtocol(corev1.ProtocolTCP)) + } + + return corev1apply.Service("reordered-svc", defaultNamespace). + WithSpec(corev1apply.ServiceSpec(). + WithSelector(map[string]string{"app": "reordered"}). + WithPorts(ports...). + WithType(corev1.ServiceTypeClusterIP)) + } + + first, err := clientset.CoreV1().Services(defaultNamespace).Apply( + ctx, + buildApply([]int32{8080, 9090}), + metav1.ApplyOptions{FieldManager: "test-manager", Force: true}, + ) + require.NoError(t, err) + + second, err := clientset.CoreV1().Services(defaultNamespace).Apply( + ctx, + buildApply([]int32{9090, 8080}), + metav1.ApplyOptions{FieldManager: "test-manager", Force: true}, + ) + require.NoError(t, err) + + assert.Equal(t, first.Generation, second.Generation) + assert.Len(t, second.Spec.Ports, 2) +} + // TestCreateContainerWithPodTemplatePatch tests that the pod template patch is correctly applied func TestCreateContainerWithPodTemplatePatch(t *testing.T) { t.Parallel() @@ -193,7 +298,7 @@ func TestCreateContainerWithPodTemplatePatch(t *testing.T) { // Deploy the workload _, err := client.DeployWorkload( - context.Background(), + t.Context(), "test-image", "test-container", []string{"test-command"}, @@ -215,7 +320,7 @@ func TestCreateContainerWithPodTemplatePatch(t *testing.T) { // Get the created StatefulSet statefulSet, err := clientset.AppsV1().StatefulSets("default").Get( - context.Background(), + t.Context(), "test-container", metav1.GetOptions{}, ) @@ -729,7 +834,7 @@ func TestCreateContainerWithMCP(t *testing.T) { // Deploy the workload _, err := client.DeployWorkload( - context.Background(), + t.Context(), tc.image, "test-container", tc.command, @@ -751,7 +856,7 @@ func TestCreateContainerWithMCP(t *testing.T) { // Get the created StatefulSet statefulSet, err := clientset.AppsV1().StatefulSets("default").Get( - context.Background(), + t.Context(), "test-container", metav1.GetOptions{}, ) @@ -959,7 +1064,7 @@ func TestAttachToWorkloadNoPodFound(t *testing.T) { client.namespaceFunc = func() string { return defaultNamespace } // Call AttachToWorkload with a workload that has no pods - _, _, err := client.AttachToWorkload(context.Background(), "nonexistent-workload") + _, _, err := client.AttachToWorkload(t.Context(), "nonexistent-workload") // Should return error immediately (no pods found) require.Error(t, err) @@ -1302,7 +1407,7 @@ func TestDeployWorkloadBackendReplicas(t *testing.T) { client.namespaceFunc = func() string { return defaultNamespace } _, err := client.DeployWorkload( - context.Background(), + t.Context(), "test-image", "test-container", nil, map[string]string{}, map[string]string{}, nil, "streamable-http", options, false, @@ -1310,7 +1415,7 @@ func TestDeployWorkloadBackendReplicas(t *testing.T) { require.NoError(t, err) sts, err := clientset.AppsV1().StatefulSets(defaultNamespace).Get( - context.Background(), "test-container", metav1.GetOptions{}, + t.Context(), "test-container", metav1.GetOptions{}, ) require.NoError(t, err) return sts From 938f6ad3454803dc09c46d2dcc416c0fba622ec1 Mon Sep 17 00:00:00 2001 From: Juan Antonio Osorio Date: Thu, 16 Apr 2026 12:20:29 +0000 Subject: [PATCH 2/3] Break long line to satisfy lll linter Co-Authored-By: Claude Opus 4.6 (1M context) --- cmd/thv-operator/controllers/mcpremoteproxy_controller.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cmd/thv-operator/controllers/mcpremoteproxy_controller.go b/cmd/thv-operator/controllers/mcpremoteproxy_controller.go index 5724239c12..84b503678f 100644 --- a/cmd/thv-operator/controllers/mcpremoteproxy_controller.go +++ b/cmd/thv-operator/controllers/mcpremoteproxy_controller.go @@ -1274,7 +1274,10 @@ func (r *MCPRemoteProxyReconciler) containerNeedsUpdate( // Check if environment variables have changed expectedEnv := r.buildEnvVarsForProxy(ctx, proxy) - if configName := ctrlutil.EmbeddedAuthServerConfigName(proxy.Spec.ExternalAuthConfigRef, proxy.Spec.AuthServerRef); configName != "" { + configName := ctrlutil.EmbeddedAuthServerConfigName( + proxy.Spec.ExternalAuthConfigRef, proxy.Spec.AuthServerRef, + ) + if configName != "" { _, _, authServerEnvVars, err := ctrlutil.GenerateAuthServerConfigByName( ctx, r.Client, proxy.Namespace, configName, ) From c0660ca25d28e383da5e82f2bc6afba394c7096e Mon Sep 17 00:00:00 2001 From: Juan Antonio Osorio Date: Thu, 16 Apr 2026 12:31:28 +0000 Subject: [PATCH 3/3] Remove SSA reordering tests that only exercise fake client These tests validated behavior of fake.NewClientset(), not any production code. The fake client does not implement real SSA merge semantics, so the assertions were meaningless. The actual drift check regression is covered by the controller-level tests. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/container/kubernetes/client_test.go | 105 ------------------------ 1 file changed, 105 deletions(-) diff --git a/pkg/container/kubernetes/client_test.go b/pkg/container/kubernetes/client_test.go index 861374ea26..6d12be60ee 100644 --- a/pkg/container/kubernetes/client_test.go +++ b/pkg/container/kubernetes/client_test.go @@ -6,7 +6,6 @@ package kubernetes import ( "context" "encoding/json" - "fmt" "strings" "testing" "time" @@ -17,10 +16,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" k8stypes "k8s.io/apimachinery/pkg/types" - "k8s.io/apimachinery/pkg/util/intstr" - appsv1apply "k8s.io/client-go/applyconfigurations/apps/v1" corev1apply "k8s.io/client-go/applyconfigurations/core/v1" - metav1apply "k8s.io/client-go/applyconfigurations/meta/v1" "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/fake" "k8s.io/client-go/rest" @@ -44,107 +40,6 @@ func (m *mockPlatformDetector) DetectPlatform(_ *rest.Config) (Platform, error) return m.platform, m.err } -func TestStatefulSetApplyAssociativeListReorderingDoesNotChangeGeneration(t *testing.T) { - t.Parallel() - - ctx := t.Context() - clientset := fake.NewClientset() - - buildApply := func(envOrder []string, portOrder []int32) *appsv1apply.StatefulSetApplyConfiguration { - envValues := map[string]string{ - "ALPHA": "1", - "BETA": "2", - } - - envVars := make([]*corev1apply.EnvVarApplyConfiguration, 0, len(envOrder)) - for _, name := range envOrder { - envVars = append(envVars, corev1apply.EnvVar().WithName(name).WithValue(envValues[name])) - } - - ports := make([]*corev1apply.ContainerPortApplyConfiguration, 0, len(portOrder)) - for _, port := range portOrder { - ports = append(ports, corev1apply.ContainerPort(). - WithContainerPort(port). - WithProtocol(corev1.ProtocolTCP)) - } - - return appsv1apply.StatefulSet("reordered", defaultNamespace). - WithSpec(appsv1apply.StatefulSetSpec(). - WithServiceName("reordered"). - WithReplicas(1). - WithSelector(metav1apply.LabelSelector(). - WithMatchLabels(map[string]string{"app": "reordered"})). - WithTemplate(corev1apply.PodTemplateSpec(). - WithLabels(map[string]string{"app": "reordered"}). - WithSpec(corev1apply.PodSpec(). - WithContainers(corev1apply.Container(). - WithName("mcp"). - WithImage("test-image"). - WithEnv(envVars...). - WithPorts(ports...))))) - } - - first, err := clientset.AppsV1().StatefulSets(defaultNamespace).Apply( - ctx, - buildApply([]string{"ALPHA", "BETA"}, []int32{8080, 9090}), - metav1.ApplyOptions{FieldManager: "test-manager", Force: true}, - ) - require.NoError(t, err) - - second, err := clientset.AppsV1().StatefulSets(defaultNamespace).Apply( - ctx, - buildApply([]string{"BETA", "ALPHA"}, []int32{9090, 8080}), - metav1.ApplyOptions{FieldManager: "test-manager", Force: true}, - ) - require.NoError(t, err) - - assert.Equal(t, first.Generation, second.Generation) - assert.Len(t, second.Spec.Template.Spec.Containers, 1) - assert.Len(t, second.Spec.Template.Spec.Containers[0].Env, 2) - assert.Len(t, second.Spec.Template.Spec.Containers[0].Ports, 2) -} - -func TestServiceApplyAssociativeListReorderingDoesNotChangeGeneration(t *testing.T) { - t.Parallel() - - ctx := t.Context() - clientset := fake.NewClientset() - - buildApply := func(portOrder []int32) *corev1apply.ServiceApplyConfiguration { - ports := make([]*corev1apply.ServicePortApplyConfiguration, 0, len(portOrder)) - for _, port := range portOrder { - ports = append(ports, corev1apply.ServicePort(). - WithName(fmt.Sprintf("port-%d", port)). - WithPort(port). - WithTargetPort(intstr.FromInt32(port)). - WithProtocol(corev1.ProtocolTCP)) - } - - return corev1apply.Service("reordered-svc", defaultNamespace). - WithSpec(corev1apply.ServiceSpec(). - WithSelector(map[string]string{"app": "reordered"}). - WithPorts(ports...). - WithType(corev1.ServiceTypeClusterIP)) - } - - first, err := clientset.CoreV1().Services(defaultNamespace).Apply( - ctx, - buildApply([]int32{8080, 9090}), - metav1.ApplyOptions{FieldManager: "test-manager", Force: true}, - ) - require.NoError(t, err) - - second, err := clientset.CoreV1().Services(defaultNamespace).Apply( - ctx, - buildApply([]int32{9090, 8080}), - metav1.ApplyOptions{FieldManager: "test-manager", Force: true}, - ) - require.NoError(t, err) - - assert.Equal(t, first.Generation, second.Generation) - assert.Len(t, second.Spec.Ports, 2) -} - // TestCreateContainerWithPodTemplatePatch tests that the pod template patch is correctly applied func TestCreateContainerWithPodTemplatePatch(t *testing.T) { t.Parallel()