From b52bdf1be39686ea072e8f74c74d77d191e49322 Mon Sep 17 00:00:00 2001 From: King Star Date: Thu, 13 Aug 2026 04:29:59 +0800 Subject: [PATCH 1/3] fix(operator): detect vMCP volume drift Hash the complete generated volume and volume-mount state so changes to referenced Secrets, ConfigMaps, and CA bundles roll the Deployment. Signed-off-by: King Star --- .../virtualmcpserver_controller.go | 11 +- .../virtualmcpserver_controller_test.go | 18 ++- .../virtualmcpserver_deployment.go | 114 ++++++++++++++---- .../virtualmcpserver_deployment_test.go | 47 ++++++++ 4 files changed, 160 insertions(+), 30 deletions(-) diff --git a/cmd/thv-operator/controllers/virtualmcpserver_controller.go b/cmd/thv-operator/controllers/virtualmcpserver_controller.go index 20745f5beb..7de643e7fe 100644 --- a/cmd/thv-operator/controllers/virtualmcpserver_controller.go +++ b/cmd/thv-operator/controllers/virtualmcpserver_controller.go @@ -1667,6 +1667,15 @@ func (r *VirtualMCPServerReconciler) deploymentNeedsUpdate( return true } + _, _, expectedVolumesHash, err := r.buildPodVolumesForVmcp(ctx, vmcp, telemetryCfg, typedWorkloads) + if err != nil { + log.FromContext(ctx).Error(err, "Failed to build volumes, assuming update needed") + return true + } + if deployment.Annotations[podVolumesHashAnnotation] != expectedVolumesHash { + return true + } + // Check if spec.replicas has changed. Only compare when spec.replicas is non-nil; // nil means hands-off mode (HPA or external controller manages replicas) and the live count is authoritative. if vmcp.Spec.Replicas != nil { @@ -1816,7 +1825,7 @@ func (*VirtualMCPServerReconciler) podTemplateSpecNeedsUpdate( // MergeAnnotations otherwise preserves them forever once their source field goes empty (#5817, #5818). func mergeDeploymentAnnotations(desired, live map[string]string) map[string]string { merged := ctrlutil.MergeAnnotations(desired, live) - for _, key := range []string{imagePullRefsHashAnnotation, podTemplateSpecHashAnnotation} { + for _, key := range []string{imagePullRefsHashAnnotation, podTemplateSpecHashAnnotation, podVolumesHashAnnotation} { if _, want := desired[key]; !want { delete(merged, key) } diff --git a/cmd/thv-operator/controllers/virtualmcpserver_controller_test.go b/cmd/thv-operator/controllers/virtualmcpserver_controller_test.go index 4f08951923..4c14dfc794 100644 --- a/cmd/thv-operator/controllers/virtualmcpserver_controller_test.go +++ b/cmd/thv-operator/controllers/virtualmcpserver_controller_test.go @@ -1859,6 +1859,10 @@ func TestVirtualMCPServerDeploymentNeedsUpdate(t *testing.T) { expectedLabels, expectedAnnotations := reconciler.buildPodTemplateMetadata( labelsForVirtualMCPServer(vmcp.Name), vmcp, vmcpConfigChecksum, ) + desiredVolumeMounts, desiredVolumes, desiredVolumesHash, err := reconciler.buildPodVolumesForVmcp( + context.Background(), vmcp, nil, nil, + ) + require.NoError(t, err) tests := []struct { name string @@ -2032,7 +2036,7 @@ func TestVirtualMCPServerDeploymentNeedsUpdate(t *testing.T) { deployment: &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ Labels: labelsForVirtualMCPServer(vmcp.Name), - Annotations: make(map[string]string), + Annotations: map[string]string{podVolumesHashAnnotation: desiredVolumesHash}, }, Spec: appsv1.DeploymentSpec{ Template: corev1.PodTemplateSpec{ @@ -2048,10 +2052,12 @@ func TestVirtualMCPServerDeploymentNeedsUpdate(t *testing.T) { Ports: []corev1.ContainerPort{ {ContainerPort: 4483}, }, - Args: reconciler.buildContainerArgsForVmcp(vmcp), - Env: mustBuildEnvVarsForVmcp(reconciler, vmcp), + Args: reconciler.buildContainerArgsForVmcp(vmcp), + Env: mustBuildEnvVarsForVmcp(reconciler, vmcp), + VolumeMounts: desiredVolumeMounts, }, }, + Volumes: desiredVolumes, ServiceAccountName: vmcpServiceAccountName(vmcp.Name), }, }, @@ -2097,6 +2103,12 @@ func TestMergeDeploymentAnnotations(t *testing.T) { live: map[string]string{podTemplateSpecHashAnnotation: "stale-hash"}, expected: map[string]string{}, }, + { + name: "prunes stale pod volumes hash annotation when desired no longer wants it", + desired: map[string]string{}, + live: map[string]string{podVolumesHashAnnotation: "stale-hash"}, + expected: map[string]string{}, + }, { name: "keeps hash annotations desired still wants", desired: map[string]string{imagePullRefsHashAnnotation: "new-hash", podTemplateSpecHashAnnotation: "new-hash"}, diff --git a/cmd/thv-operator/controllers/virtualmcpserver_deployment.go b/cmd/thv-operator/controllers/virtualmcpserver_deployment.go index 9b6df3c02f..7651fa9383 100644 --- a/cmd/thv-operator/controllers/virtualmcpserver_deployment.go +++ b/cmd/thv-operator/controllers/virtualmcpserver_deployment.go @@ -46,6 +46,12 @@ const ( // detect every input that influences the deployed PodSpec.ImagePullSecrets. imagePullRefsHashAnnotation = "toolhive.stacklok.io/imagepullsecrets-hash" + // podVolumesHashAnnotation tracks the SHA256 hash of the desired vMCP + // container volume mounts and PodSpec volumes. The hash is stored on the + // Deployment so changes to referenced Secrets or ConfigMaps trigger a + // rollout without comparing API-server-defaulted live PodSpec fields. + podVolumesHashAnnotation = "toolhive.stacklok.io/podvolumes-hash" + // Log level configuration logLevelDebug = "debug" // Debug log level value @@ -143,7 +149,7 @@ func (r *VirtualMCPServerReconciler) deploymentForVirtualMCPServer( // Build deployment components using helper functions args := r.buildContainerArgsForVmcp(vmcp) - volumeMounts, volumes, err := r.buildVolumesForVmcp(ctx, vmcp) + volumeMounts, volumes, volumesHash, err := r.buildPodVolumesForVmcp(ctx, vmcp, telemetryCfg, typedWorkloads) if err != nil { log.FromContext(ctx).Error(err, "Failed to build volumes for VirtualMCPServer") return nil @@ -154,31 +160,7 @@ func (r *VirtualMCPServerReconciler) deploymentForVirtualMCPServer( return nil } - // Add CA bundle volumes for MCPServerEntry backends with caBundleRef - caVolumes, caMounts, err := r.buildCABundleVolumesForEntries(ctx, vmcp.Namespace, typedWorkloads) - if err != nil { - log.FromContext(ctx).Error(err, "Failed to build CA bundle volumes for MCPServerEntries") - return nil - } - volumes = append(volumes, caVolumes...) - volumeMounts = append(volumeMounts, caMounts...) - - // Add telemetry CA bundle volumes from the pre-fetched MCPTelemetryConfig - if telemetryCfg != nil { - telVolumes, telMounts := ctrlutil.AddTelemetryCABundleVolumes(telemetryCfg) - volumes = append(volumes, telVolumes...) - volumeMounts = append(volumeMounts, telMounts...) - } - - // Add embedded auth server volumes if configured (inline config). The matching - // env vars are injected by buildEnvVarsForVmcp above so the drift check stays - // symmetric with what is built here (see #5616). - if vmcp.Spec.AuthServerConfig != nil { - authServerVolumes, authServerMounts := ctrlutil.GenerateAuthServerVolumes(vmcp.Spec.AuthServerConfig) - volumes = append(volumes, authServerVolumes...) - volumeMounts = append(volumeMounts, authServerMounts...) - } - deploymentLabels, deploymentAnnotations := r.buildDeploymentMetadataForVmcp(ls, vmcp) + deploymentLabels, deploymentAnnotations := r.buildDeploymentMetadataForVmcp(ls, vmcp, volumesHash) deploymentTemplateLabels, deploymentTemplateAnnotations := r.buildPodTemplateMetadata(ls, vmcp, vmcpConfigChecksum) podSecurityContext, containerSecurityContext := r.buildSecurityContextsForVmcp(ctx, vmcp) serviceAccountName := r.serviceAccountNameForVmcp(vmcp) @@ -257,6 +239,81 @@ func (r *VirtualMCPServerReconciler) deploymentForVirtualMCPServer( return dep } +// buildPodVolumesForVmcp builds the complete desired volume state for the vmcp +// container and computes its stable hash in the same pass. Keeping all volume +// sources here ensures the PodSpec and Deployment annotation use one consistent +// snapshot of referenced Kubernetes objects. +func (r *VirtualMCPServerReconciler) buildPodVolumesForVmcp( + ctx context.Context, + vmcp *mcpv1beta1.VirtualMCPServer, + telemetryCfg *mcpv1beta1.MCPTelemetryConfig, + typedWorkloads []workloads.TypedWorkload, +) ([]corev1.VolumeMount, []corev1.Volume, string, error) { + volumeMounts, volumes, err := r.buildVolumesForVmcp(ctx, vmcp) + if err != nil { + return nil, nil, "", err + } + + caVolumes, caMounts, err := r.buildCABundleVolumesForEntries(ctx, vmcp.Namespace, typedWorkloads) + if err != nil { + return nil, nil, "", fmt.Errorf("failed to build CA bundle volumes for MCPServerEntries: %w", err) + } + volumes = append(volumes, caVolumes...) + volumeMounts = append(volumeMounts, caMounts...) + + if telemetryCfg != nil { + telVolumes, telMounts := ctrlutil.AddTelemetryCABundleVolumes(telemetryCfg) + volumes = append(volumes, telVolumes...) + volumeMounts = append(volumeMounts, telMounts...) + } + + if vmcp.Spec.AuthServerConfig != nil { + authServerVolumes, authServerMounts := ctrlutil.GenerateAuthServerVolumes(vmcp.Spec.AuthServerConfig) + volumes = append(volumes, authServerVolumes...) + volumeMounts = append(volumeMounts, authServerMounts...) + } + + hash, err := podVolumesHash(volumes, volumeMounts) + if err != nil { + return nil, nil, "", err + } + return volumeMounts, volumes, hash, nil +} + +// podVolumesHash returns a deterministic hash of the complete desired volume +// and volume-mount objects. Sorting by identity makes ordering-only changes a +// no-op while hashing the full Kubernetes structs preserves all drift-relevant +// source fields, including future VolumeSource additions. +func podVolumesHash(volumes []corev1.Volume, volumeMounts []corev1.VolumeMount) (string, error) { + normalizedVolumes := append([]corev1.Volume(nil), volumes...) + sort.SliceStable(normalizedVolumes, func(i, j int) bool { + return normalizedVolumes[i].Name < normalizedVolumes[j].Name + }) + normalizedMounts := append([]corev1.VolumeMount(nil), volumeMounts...) + sort.SliceStable(normalizedMounts, func(i, j int) bool { + if normalizedMounts[i].Name != normalizedMounts[j].Name { + return normalizedMounts[i].Name < normalizedMounts[j].Name + } + if normalizedMounts[i].MountPath != normalizedMounts[j].MountPath { + return normalizedMounts[i].MountPath < normalizedMounts[j].MountPath + } + return normalizedMounts[i].SubPath < normalizedMounts[j].SubPath + }) + + canonical, err := json.Marshal(struct { + Volumes []corev1.Volume + VolumeMounts []corev1.VolumeMount + }{ + Volumes: normalizedVolumes, + VolumeMounts: normalizedMounts, + }) + if err != nil { + return "", fmt.Errorf("failed to marshal pod volumes for hashing: %w", err) + } + hash := sha256.Sum256(canonical) + return hex.EncodeToString(hash[:]), nil +} + // buildContainerArgsForVmcp builds the container arguments for vmcp func (*VirtualMCPServerReconciler) buildContainerArgsForVmcp( vmcp *mcpv1beta1.VirtualMCPServer, @@ -924,6 +981,7 @@ func xaaSecretEnvVars(externalAuthConfig *mcpv1beta1.MCPExternalAuthConfig, conf func (r *VirtualMCPServerReconciler) buildDeploymentMetadataForVmcp( baseLabels map[string]string, vmcp *mcpv1beta1.VirtualMCPServer, + podVolumesHash ...string, ) (map[string]string, map[string]string) { deploymentLabels := baseLabels deploymentAnnotations := make(map[string]string) @@ -949,6 +1007,10 @@ func (r *VirtualMCPServerReconciler) buildDeploymentMetadataForVmcp( deploymentAnnotations[imagePullRefsHashAnnotation] = hash } + if len(podVolumesHash) > 0 && podVolumesHash[0] != "" { + deploymentAnnotations[podVolumesHashAnnotation] = podVolumesHash[0] + } + // TODO: Add support for ResourceOverrides if needed in the future return deploymentLabels, deploymentAnnotations diff --git a/cmd/thv-operator/controllers/virtualmcpserver_deployment_test.go b/cmd/thv-operator/controllers/virtualmcpserver_deployment_test.go index ef3ee1482d..ef846718fc 100644 --- a/cmd/thv-operator/controllers/virtualmcpserver_deployment_test.go +++ b/cmd/thv-operator/controllers/virtualmcpserver_deployment_test.go @@ -1103,6 +1103,53 @@ func TestDeploymentForVirtualMCPServer_AuthServerConfig_NoUpdateLoop(t *testing. "deploymentNeedsUpdate must not loop on a vMCP with AuthServerConfig (regression #5616)") } +// TestDeploymentForVirtualMCPServer_AuthServerSigningKeyVolumeDrift verifies that +// changing an embedded auth-server signing key Secret reference rolls the vMCP +// Deployment. The mounted Secret is selected by the PodSpec rather than an env +// var, so container drift checks alone cannot observe this change. +func TestDeploymentForVirtualMCPServer_AuthServerSigningKeyVolumeDrift(t *testing.T) { + t.Parallel() + + scheme := testutil.NewScheme(t) + r := &VirtualMCPServerReconciler{ + Scheme: scheme, + PlatformDetector: ctrlutil.NewSharedPlatformDetector(), + } + + vmcp := v1beta1test.NewVirtualMCPServer("test-vmcp", "default", + v1beta1test.WithVMCPGroupRef("test-group"), + v1beta1test.WithVMCPAuthServerConfig(&mcpv1beta1.EmbeddedAuthServerConfig{ + SigningKeySecretRefs: []mcpv1beta1.SecretKeyRef{{Name: "keys-v1", Key: "signing-key.pem"}}, + }), + ) + + const cfgChecksum = "test-checksum" + initialDeployment := r.deploymentForVirtualMCPServer(t.Context(), vmcp, cfgChecksum, nil, nil) + require.NotNil(t, initialDeployment) + require.NotEmpty(t, initialDeployment.Annotations["toolhive.stacklok.io/podvolumes-hash"]) + + updatedVMCP := vmcp.DeepCopy() + updatedVMCP.Spec.AuthServerConfig.SigningKeySecretRefs[0].Name = "keys-v2" + updatedDeployment := r.deploymentForVirtualMCPServer(t.Context(), updatedVMCP, cfgChecksum, nil, nil) + require.NotNil(t, updatedDeployment) + + assert.NotEqual(t, + initialDeployment.Annotations["toolhive.stacklok.io/podvolumes-hash"], + updatedDeployment.Annotations["toolhive.stacklok.io/podvolumes-hash"], + "changing the signing key Secret reference must change the pod volume hash") + assert.True(t, r.deploymentNeedsUpdate(t.Context(), initialDeployment, updatedVMCP, cfgChecksum, nil, nil)) + assert.False(t, r.deploymentNeedsUpdate(t.Context(), updatedDeployment, updatedVMCP, cfgChecksum, nil, nil)) + + var signingKeySecretName string + for _, volume := range updatedDeployment.Spec.Template.Spec.Volumes { + if volume.Name == ctrlutil.AuthServerKeysVolumePrefix+"0" && volume.Secret != nil { + signingKeySecretName = volume.Secret.SecretName + break + } + } + assert.Equal(t, "keys-v2", signingKeySecretName) +} + // TestImagePullSecretsHash verifies the hash helper normalizes order, treats an // empty list as the sentinel "" hash, and produces stable hashes across calls. func TestImagePullSecretsHash(t *testing.T) { From a42ccfe0c8c711f78c52e9bf7aaf0ba3cb0fae63 Mon Sep 17 00:00:00 2001 From: King Star Date: Sat, 29 Aug 2026 15:17:41 +0800 Subject: [PATCH 2/3] test(operator): pass ca bundle checksum in volume drift regressions Signed-off-by: King Star --- .../controllers/virtualmcpserver_deployment_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/thv-operator/controllers/virtualmcpserver_deployment_test.go b/cmd/thv-operator/controllers/virtualmcpserver_deployment_test.go index cdc7cb607b..408b77bef7 100644 --- a/cmd/thv-operator/controllers/virtualmcpserver_deployment_test.go +++ b/cmd/thv-operator/controllers/virtualmcpserver_deployment_test.go @@ -1162,21 +1162,21 @@ func TestDeploymentForVirtualMCPServer_AuthServerSigningKeyVolumeDrift(t *testin ) const cfgChecksum = "test-checksum" - initialDeployment := r.deploymentForVirtualMCPServer(t.Context(), vmcp, cfgChecksum, nil, nil) + initialDeployment := r.deploymentForVirtualMCPServer(t.Context(), vmcp, cfgChecksum, "", nil, nil) require.NotNil(t, initialDeployment) require.NotEmpty(t, initialDeployment.Annotations["toolhive.stacklok.io/podvolumes-hash"]) updatedVMCP := vmcp.DeepCopy() updatedVMCP.Spec.AuthServerConfig.SigningKeySecretRefs[0].Name = "keys-v2" - updatedDeployment := r.deploymentForVirtualMCPServer(t.Context(), updatedVMCP, cfgChecksum, nil, nil) + updatedDeployment := r.deploymentForVirtualMCPServer(t.Context(), updatedVMCP, cfgChecksum, "", nil, nil) require.NotNil(t, updatedDeployment) assert.NotEqual(t, initialDeployment.Annotations["toolhive.stacklok.io/podvolumes-hash"], updatedDeployment.Annotations["toolhive.stacklok.io/podvolumes-hash"], "changing the signing key Secret reference must change the pod volume hash") - assert.True(t, r.deploymentNeedsUpdate(t.Context(), initialDeployment, updatedVMCP, cfgChecksum, nil, nil)) - assert.False(t, r.deploymentNeedsUpdate(t.Context(), updatedDeployment, updatedVMCP, cfgChecksum, nil, nil)) + assert.True(t, r.deploymentNeedsUpdate(t.Context(), initialDeployment, updatedVMCP, cfgChecksum, "", nil, nil)) + assert.False(t, r.deploymentNeedsUpdate(t.Context(), updatedDeployment, updatedVMCP, cfgChecksum, "", nil, nil)) var signingKeySecretName string for _, volume := range updatedDeployment.Spec.Template.Spec.Volumes { From f7143a68ffd00c8c0940aee55dad886607157d8f Mon Sep 17 00:00:00 2001 From: King Star Date: Sun, 30 Aug 2026 23:17:55 +0800 Subject: [PATCH 3/3] Strengthen volume drift coverage Make the volume hash a required deployment metadata input and pin its ordering and full-field contract. Exercise ConfigMap-backed CA drift and the legacy missing-annotation transition through fake clients. Signed-off-by: King Star --- .../virtualmcpserver_controller_test.go | 159 ++++++++++++++++++ .../virtualmcpserver_deployment.go | 6 +- .../virtualmcpserver_deployment_test.go | 139 ++++++++++++++- 3 files changed, 298 insertions(+), 6 deletions(-) diff --git a/cmd/thv-operator/controllers/virtualmcpserver_controller_test.go b/cmd/thv-operator/controllers/virtualmcpserver_controller_test.go index b35299f9d5..a62e501879 100644 --- a/cmd/thv-operator/controllers/virtualmcpserver_controller_test.go +++ b/cmd/thv-operator/controllers/virtualmcpserver_controller_test.go @@ -2649,6 +2649,165 @@ func TestVirtualMCPServerEnsureDeployment_NoUpdateNeeded(t *testing.T) { assert.Equal(t, ctrl.Result{}, result) } +func TestVirtualMCPServerEnsureDeployment_BackfillsPodVolumesHashOnce(t *testing.T) { + t.Parallel() + + ctx := t.Context() + scheme := testutil.NewScheme(t) + vmcp := v1beta1test.NewVirtualMCPServer(testVmcpName, "default", + v1beta1test.WithVMCPGroupRef(testGroupName), + ) + configMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: vmcpConfigMapName(vmcp.Name), + Namespace: vmcp.Namespace, + Annotations: map[string]string{ + checksum.ContentChecksumAnnotation: "test-checksum", + }, + }, + Data: map[string]string{"config.yaml": "test-config"}, + } + + deploymentUpdates := 0 + k8sClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(vmcp, configMap). + WithInterceptorFuncs(interceptor.Funcs{ + Update: func(ctx context.Context, c client.WithWatch, obj client.Object, opts ...client.UpdateOption) error { + if _, ok := obj.(*appsv1.Deployment); ok { + deploymentUpdates++ + } + return c.Update(ctx, obj, opts...) + }, + }). + Build() + reconciler := &VirtualMCPServerReconciler{ + Client: k8sClient, + Scheme: scheme, + PlatformDetector: ctrlutil.NewSharedPlatformDetector(), + } + + deployment := reconciler.deploymentForVirtualMCPServer(ctx, vmcp, "test-checksum", "", nil, nil) + require.NotNil(t, deployment) + require.NotEmpty(t, deployment.Annotations[podVolumesHashAnnotation]) + delete(deployment.Annotations, podVolumesHashAnnotation) + require.NoError(t, k8sClient.Create(ctx, deployment)) + + result, err := reconciler.ensureDeployment(ctx, vmcp, nil, nil) + require.NoError(t, err) + assert.Equal(t, ctrl.Result{}, result) + assert.Equal(t, 1, deploymentUpdates, "a missing hash annotation must cause exactly one update") + + updated := &appsv1.Deployment{} + require.NoError(t, k8sClient.Get(ctx, types.NamespacedName{Name: vmcp.Name, Namespace: vmcp.Namespace}, updated)) + require.NotEmpty(t, updated.Annotations[podVolumesHashAnnotation]) + resourceVersion := updated.ResourceVersion + + result, err = reconciler.ensureDeployment(ctx, vmcp, nil, nil) + require.NoError(t, err) + assert.Equal(t, ctrl.Result{}, result) + assert.Equal(t, 1, deploymentUpdates, "the rebuilt deployment must be steady state") + + updated = &appsv1.Deployment{} + require.NoError(t, k8sClient.Get(ctx, types.NamespacedName{Name: vmcp.Name, Namespace: vmcp.Namespace}, updated)) + assert.Equal(t, resourceVersion, updated.ResourceVersion, "steady-state reconcile must not write again") +} + +func TestVirtualMCPServerEnsureDeployment_UpdatesMCPServerEntryCABundleVolume(t *testing.T) { + t.Parallel() + + ctx := t.Context() + scheme := testutil.NewScheme(t) + vmcp := v1beta1test.NewVirtualMCPServer(testVmcpName, "default", + v1beta1test.WithVMCPGroupRef(testGroupName), + ) + configMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: vmcpConfigMapName(vmcp.Name), + Namespace: vmcp.Namespace, + Annotations: map[string]string{ + checksum.ContentChecksumAnnotation: "test-checksum", + }, + }, + Data: map[string]string{"config.yaml": "test-config"}, + } + entry := &mcpv1beta1.MCPServerEntry{ + ObjectMeta: metav1.ObjectMeta{Name: "remote-entry", Namespace: vmcp.Namespace}, + Spec: mcpv1beta1.MCPServerEntrySpec{ + RemoteURL: "https://mcp.example.com", + Transport: "streamable-http", + GroupRef: &mcpv1beta1.MCPGroupRef{Name: testGroupName}, + CABundleRef: &mcpv1beta1.CABundleSource{ConfigMapRef: &corev1.ConfigMapKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "ca-bundle-v1"}, + Key: "ca.crt", + }}, + }, + } + typedWorkloads := []workloads.TypedWorkload{{ + Name: entry.Name, + Type: workloads.WorkloadTypeMCPServerEntry, + }} + + deploymentUpdates := 0 + k8sClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(vmcp, configMap, entry). + WithInterceptorFuncs(interceptor.Funcs{ + Update: func(ctx context.Context, c client.WithWatch, obj client.Object, opts ...client.UpdateOption) error { + if _, ok := obj.(*appsv1.Deployment); ok { + deploymentUpdates++ + } + return c.Update(ctx, obj, opts...) + }, + }). + Build() + reconciler := &VirtualMCPServerReconciler{ + Client: k8sClient, + Scheme: scheme, + PlatformDetector: ctrlutil.NewSharedPlatformDetector(), + } + + deployment := reconciler.deploymentForVirtualMCPServer( + ctx, vmcp, "test-checksum", "", nil, typedWorkloads, + ) + require.NotNil(t, deployment) + initialHash := deployment.Annotations[podVolumesHashAnnotation] + require.NotEmpty(t, initialHash) + require.NoError(t, k8sClient.Create(ctx, deployment)) + + updatedEntry := &mcpv1beta1.MCPServerEntry{} + require.NoError(t, k8sClient.Get(ctx, types.NamespacedName{Name: entry.Name, Namespace: entry.Namespace}, updatedEntry)) + updatedEntry.Spec.CABundleRef.ConfigMapRef.Name = "ca-bundle-v2" + require.NoError(t, k8sClient.Update(ctx, updatedEntry)) + + result, err := reconciler.ensureDeployment(ctx, vmcp, nil, typedWorkloads) + require.NoError(t, err) + assert.Equal(t, ctrl.Result{}, result) + assert.Equal(t, 1, deploymentUpdates, "changing only the ConfigMap volume source must update the deployment") + + updated := &appsv1.Deployment{} + require.NoError(t, k8sClient.Get(ctx, types.NamespacedName{Name: vmcp.Name, Namespace: vmcp.Namespace}, updated)) + assert.NotEqual(t, initialHash, updated.Annotations[podVolumesHashAnnotation]) + var caBundleConfigMapName string + for _, volume := range updated.Spec.Template.Spec.Volumes { + if volume.Name == caBundleVolumeName(entry.Name) && volume.ConfigMap != nil { + caBundleConfigMapName = volume.ConfigMap.Name + break + } + } + assert.Equal(t, "ca-bundle-v2", caBundleConfigMapName) + resourceVersion := updated.ResourceVersion + + result, err = reconciler.ensureDeployment(ctx, vmcp, nil, typedWorkloads) + require.NoError(t, err) + assert.Equal(t, ctrl.Result{}, result) + assert.Equal(t, 1, deploymentUpdates, "the updated CA bundle reference must reach steady state") + + updated = &appsv1.Deployment{} + require.NoError(t, k8sClient.Get(ctx, types.NamespacedName{Name: vmcp.Name, Namespace: vmcp.Namespace}, updated)) + assert.Equal(t, resourceVersion, updated.ResourceVersion, "steady-state reconcile must not write again") +} + // TestVirtualMCPServerEnsureDeployment_RemovesStaleHashAnnotation is a regression test // for #5817/#5818: a stale operator-owned hash annotation left over from a prior // reconcile (when the corresponding field was non-empty) must be removed once that diff --git a/cmd/thv-operator/controllers/virtualmcpserver_deployment.go b/cmd/thv-operator/controllers/virtualmcpserver_deployment.go index 54dd92a562..f0782a553f 100644 --- a/cmd/thv-operator/controllers/virtualmcpserver_deployment.go +++ b/cmd/thv-operator/controllers/virtualmcpserver_deployment.go @@ -986,7 +986,7 @@ func xaaSecretEnvVars(externalAuthConfig *mcpv1beta1.MCPExternalAuthConfig, conf func (r *VirtualMCPServerReconciler) buildDeploymentMetadataForVmcp( baseLabels map[string]string, vmcp *mcpv1beta1.VirtualMCPServer, - podVolumesHash ...string, + podVolumesHash string, ) (map[string]string, map[string]string) { deploymentLabels := baseLabels deploymentAnnotations := make(map[string]string) @@ -1012,9 +1012,7 @@ func (r *VirtualMCPServerReconciler) buildDeploymentMetadataForVmcp( deploymentAnnotations[imagePullRefsHashAnnotation] = hash } - if len(podVolumesHash) > 0 && podVolumesHash[0] != "" { - deploymentAnnotations[podVolumesHashAnnotation] = podVolumesHash[0] - } + deploymentAnnotations[podVolumesHashAnnotation] = podVolumesHash // TODO: Add support for ResourceOverrides if needed in the future diff --git a/cmd/thv-operator/controllers/virtualmcpserver_deployment_test.go b/cmd/thv-operator/controllers/virtualmcpserver_deployment_test.go index 408b77bef7..e06b9a58cd 100644 --- a/cmd/thv-operator/controllers/virtualmcpserver_deployment_test.go +++ b/cmd/thv-operator/controllers/virtualmcpserver_deployment_test.go @@ -371,10 +371,10 @@ func TestBuildDeploymentMetadataForVmcp(t *testing.T) { vmcp := v1beta1test.NewVirtualMCPServer("test-vmcp", "default") r := &VirtualMCPServerReconciler{} - labels, annotations := r.buildDeploymentMetadataForVmcp(baseLabels, vmcp) + labels, annotations := r.buildDeploymentMetadataForVmcp(baseLabels, vmcp, "pod-volumes-hash") assert.Equal(t, baseLabels, labels) - assert.NotNil(t, annotations) + assert.Equal(t, "pod-volumes-hash", annotations[podVolumesHashAnnotation]) } // TestBuildPodTemplateMetadata tests pod template metadata generation @@ -1188,6 +1188,141 @@ func TestDeploymentForVirtualMCPServer_AuthServerSigningKeyVolumeDrift(t *testin assert.Equal(t, "keys-v2", signingKeySecretName) } +func TestPodVolumesHash(t *testing.T) { + t.Parallel() + + defaultMode := int32(0o444) + mountPropagation := corev1.MountPropagationHostToContainer + baseVolumes := []corev1.Volume{ + { + Name: "ca-bundle", + VolumeSource: corev1.VolumeSource{ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{Name: "bundle-a"}, + Items: []corev1.KeyToPath{{Key: "ca.crt", Path: "ca.crt", Mode: &defaultMode}}, + }}, + }, + { + Name: "credentials", + VolumeSource: corev1.VolumeSource{Secret: &corev1.SecretVolumeSource{ + SecretName: "credentials-a", + }}, + }, + } + baseMounts := []corev1.VolumeMount{ + { + Name: "ca-bundle", + MountPath: "/etc/ca", + SubPath: "ca.crt", + ReadOnly: true, + MountPropagation: &mountPropagation, + }, + {Name: "credentials", MountPath: "/etc/credentials", ReadOnly: true}, + } + + cloneInputs := func() ([]corev1.Volume, []corev1.VolumeMount) { + volumes := make([]corev1.Volume, len(baseVolumes)) + for i := range baseVolumes { + volumes[i] = *baseVolumes[i].DeepCopy() + } + mounts := make([]corev1.VolumeMount, len(baseMounts)) + for i := range baseMounts { + mounts[i] = *baseMounts[i].DeepCopy() + } + return volumes, mounts + } + + baseHash, err := podVolumesHash(baseVolumes, baseMounts) + require.NoError(t, err) + require.NotEmpty(t, baseHash) + + reorderedVolumes, reorderedMounts := cloneInputs() + reorderedVolumes[0], reorderedVolumes[1] = reorderedVolumes[1], reorderedVolumes[0] + reorderedMounts[0], reorderedMounts[1] = reorderedMounts[1], reorderedMounts[0] + reorderedHash, err := podVolumesHash(reorderedVolumes, reorderedMounts) + require.NoError(t, err) + assert.Equal(t, baseHash, reorderedHash, "slice ordering must not affect the hash") + + tests := []struct { + name string + mutate func([]corev1.Volume, []corev1.VolumeMount) + }{ + { + name: "config map name", + mutate: func(volumes []corev1.Volume, _ []corev1.VolumeMount) { + volumes[0].ConfigMap.Name = "bundle-b" + }, + }, + { + name: "config map item key", + mutate: func(volumes []corev1.Volume, _ []corev1.VolumeMount) { + volumes[0].ConfigMap.Items[0].Key = "root.pem" + }, + }, + { + name: "config map item path", + mutate: func(volumes []corev1.Volume, _ []corev1.VolumeMount) { + volumes[0].ConfigMap.Items[0].Path = "root.pem" + }, + }, + { + name: "config map item mode", + mutate: func(volumes []corev1.Volume, _ []corev1.VolumeMount) { + mode := int32(0o400) + volumes[0].ConfigMap.Items[0].Mode = &mode + }, + }, + { + name: "secret name", + mutate: func(volumes []corev1.Volume, _ []corev1.VolumeMount) { + volumes[1].Secret.SecretName = "credentials-b" + }, + }, + { + name: "mount path", + mutate: func(_ []corev1.Volume, mounts []corev1.VolumeMount) { + mounts[0].MountPath = "/etc/root-ca" + }, + }, + { + name: "read only", + mutate: func(_ []corev1.Volume, mounts []corev1.VolumeMount) { + mounts[0].ReadOnly = false + }, + }, + { + name: "sub path", + mutate: func(_ []corev1.Volume, mounts []corev1.VolumeMount) { + mounts[0].SubPath = "root.pem" + }, + }, + { + name: "sub path expression", + mutate: func(_ []corev1.Volume, mounts []corev1.VolumeMount) { + mounts[0].SubPathExpr = "$(POD_NAME).pem" + }, + }, + { + name: "mount propagation", + mutate: func(_ []corev1.Volume, mounts []corev1.VolumeMount) { + propagation := corev1.MountPropagationBidirectional + mounts[0].MountPropagation = &propagation + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + volumes, mounts := cloneInputs() + tt.mutate(volumes, mounts) + got, err := podVolumesHash(volumes, mounts) + require.NoError(t, err) + assert.NotEqual(t, baseHash, got, "changing a volume or mount field must change the hash") + }) + } +} + // TestImagePullSecretsHash verifies the hash helper normalizes order, treats an // empty list as the sentinel "" hash, and produces stable hashes across calls. func TestImagePullSecretsHash(t *testing.T) {