From 0eec47e5744c26fa22f4cfc5de44947bb085ed0c Mon Sep 17 00:00:00 2001 From: chlins Date: Tue, 4 Aug 2026 13:26:47 +0800 Subject: [PATCH] Drop node-agent host path mounts from data mover pods The CSI snapshot and generic restore exposers access data through PVCs, so they no longer inherit the node-agent host path volumes. Also drop all capabilities on the data mover container. Signed-off-by: chlins --- changelogs/unreleased/10150-chlins | 1 + pkg/exposer/csi_snapshot.go | 30 +++- pkg/exposer/generic_restore.go | 30 +++- pkg/exposer/image.go | 58 ++++++- pkg/exposer/image_daemonset_test.go | 75 +++++++++ pkg/exposer/image_test.go | 236 +++++++++++++++++++++++++++- 6 files changed, 406 insertions(+), 24 deletions(-) create mode 100644 changelogs/unreleased/10150-chlins create mode 100644 pkg/exposer/image_daemonset_test.go diff --git a/changelogs/unreleased/10150-chlins b/changelogs/unreleased/10150-chlins new file mode 100644 index 000000000..158a3f43c --- /dev/null +++ b/changelogs/unreleased/10150-chlins @@ -0,0 +1 @@ +Drop node-agent host path mounts from data mover pods diff --git a/pkg/exposer/csi_snapshot.go b/pkg/exposer/csi_snapshot.go index ed510c798..3fd78cb9b 100644 --- a/pkg/exposer/csi_snapshot.go +++ b/pkg/exposer/csi_snapshot.go @@ -684,7 +684,9 @@ func (e *csiSnapshotExposer) createBackupPod( containerName := string(ownerObject.UID) volumeName := string(ownerObject.UID) - podInfo, err := getInheritedPodInfo(ctx, e.kubeClient, ownerObject.Namespace, nodeOS) + // The backup pod reads the data through the backup PVC only, so the node-agent's host + // path volumes to the kubelet root directory are not inherited. + podInfo, err := getInheritedPodInfo(ctx, e.kubeClient, ownerObject.Namespace, nodeOS, hostPathVolumesOfNodeAgent...) if err != nil { return nil, errors.Wrap(err, "error to get inherited pod info from node-agent") } @@ -750,6 +752,7 @@ func (e *csiSnapshotExposer) createBackupPod( } var securityCtx *corev1api.PodSecurityContext + var containerSecurityCtx *corev1api.SecurityContext nodeSelector := map[string]string{} podOS := corev1api.PodOS{} if nodeOS == kube.NodeOSWindows { @@ -788,6 +791,18 @@ func (e *csiSnapshotExposer) createBackupPod( RunAsUser: &userID, } + // The backup pod runs as root so that it can read the backup data regardless of the + // ownership, but it doesn't need any capability beyond that. + containerSecurityCtx = &corev1api.SecurityContext{ + AllowPrivilegeEscalation: boolptr.False(), + Capabilities: &corev1api.Capabilities{ + Drop: []corev1api.Capability{"ALL"}, + }, + SeccompProfile: &corev1api.SeccompProfile{ + Type: corev1api.SeccompProfileTypeRuntimeDefault, + }, + } + if spcNoRelabeling { securityCtx.SELinuxOptions = &corev1api.SELinuxOptions{ Type: "spc_t", @@ -859,12 +874,13 @@ func (e *csiSnapshotExposer) createBackupPod( "data-mover", "backup", }, - Args: args, - VolumeMounts: volumeMounts, - VolumeDevices: volumeDevices, - Env: podInfo.env, - EnvFrom: podInfo.envFrom, - Resources: resources, + Args: args, + VolumeMounts: volumeMounts, + VolumeDevices: volumeDevices, + Env: podInfo.env, + EnvFrom: podInfo.envFrom, + Resources: resources, + SecurityContext: containerSecurityCtx, }, }, PriorityClassName: priorityClassName, diff --git a/pkg/exposer/generic_restore.go b/pkg/exposer/generic_restore.go index 0f4b9c5b4..46851803c 100644 --- a/pkg/exposer/generic_restore.go +++ b/pkg/exposer/generic_restore.go @@ -628,7 +628,9 @@ func (e *genericRestoreExposer) createRestorePod( affinity = &kube.LoadAffinity{} } - podInfo, err := getInheritedPodInfo(ctx, e.kubeClient, ownerObject.Namespace, nodeOS) + // The restore pod writes the data through the restore PVC only, so the node-agent's host + // path volumes to the kubelet root directory are not inherited. + podInfo, err := getInheritedPodInfo(ctx, e.kubeClient, ownerObject.Namespace, nodeOS, hostPathVolumesOfNodeAgent...) if err != nil { return nil, errors.Wrap(err, "error to get inherited pod info from node-agent") } @@ -692,6 +694,7 @@ func (e *genericRestoreExposer) createRestorePod( args = append(args, podInfo.logLevelArgs...) var securityCtx *corev1api.PodSecurityContext + var containerSecurityCtx *corev1api.SecurityContext podOS := corev1api.PodOS{} if nodeOS == kube.NodeOSWindows { userID := "ContainerAdministrator" @@ -729,6 +732,18 @@ func (e *genericRestoreExposer) createRestorePod( RunAsUser: &userID, } + // The restore pod runs as root so that it can restore the data with the original + // ownership, but it doesn't need any capability beyond that. + containerSecurityCtx = &corev1api.SecurityContext{ + AllowPrivilegeEscalation: boolptr.False(), + Capabilities: &corev1api.Capabilities{ + Drop: []corev1api.Capability{"ALL"}, + }, + SeccompProfile: &corev1api.SeccompProfile{ + Type: corev1api.SeccompProfileTypeRuntimeDefault, + }, + } + podOS.Name = kube.NodeOSLinux affinity.NodeSelector.MatchExpressions = append(affinity.NodeSelector.MatchExpressions, metav1.LabelSelectorRequirement{ @@ -781,12 +796,13 @@ func (e *genericRestoreExposer) createRestorePod( "data-mover", "restore", }, - Args: args, - VolumeMounts: volumeMounts, - VolumeDevices: volumeDevices, - Env: podInfo.env, - EnvFrom: podInfo.envFrom, - Resources: resources, + Args: args, + VolumeMounts: volumeMounts, + VolumeDevices: volumeDevices, + Env: podInfo.env, + EnvFrom: podInfo.envFrom, + Resources: resources, + SecurityContext: containerSecurityCtx, }, }, PriorityClassName: priorityClassName, diff --git a/pkg/exposer/image.go b/pkg/exposer/image.go index 2157d8175..aa774221b 100644 --- a/pkg/exposer/image.go +++ b/pkg/exposer/image.go @@ -27,6 +27,19 @@ import ( "github.com/vmware-tanzu/velero/pkg/nodeagent" ) +const ( + // hostPluginsVolumeName is the name of the node-agent volume that mounts the kubelet + // plugins directory from the host. + hostPluginsVolumeName = "host-plugins" +) + +// hostPathVolumesOfNodeAgent lists the node-agent volumes that expose the kubelet root +// directory of the host. They are only required by fs-backup, which resolves and accesses +// pod volume data through the kubelet pod directory. Other exposers access data through +// PVCs only, so they must exclude these volumes from the inherited pod info to avoid +// granting data mover pods unnecessary access to the host file system. +var hostPathVolumesOfNodeAgent = []string{nodeagent.HostPodVolumeMount, hostPluginsVolumeName} + type inheritedPodInfo struct { image string serviceAccount string @@ -41,7 +54,11 @@ type inheritedPodInfo struct { imagePullSecrets []corev1api.LocalObjectReference } -func getInheritedPodInfo(ctx context.Context, client kubernetes.Interface, veleroNamespace string, osType string) (inheritedPodInfo, error) { +// getInheritedPodInfo collects the pod info to be inherited by the hosting pods from the +// node-agent pod template. Volumes whose name is listed in excludedVolumes, together with +// their volume mounts, are dropped from the result. Names that are not found in the +// node-agent pod template are ignored. +func getInheritedPodInfo(ctx context.Context, client kubernetes.Interface, veleroNamespace string, osType string, excludedVolumes ...string) (inheritedPodInfo, error) { podInfo := inheritedPodInfo{} podSpec, err := nodeagent.GetPodSpec(ctx, client, veleroNamespace, osType) @@ -58,8 +75,7 @@ func getInheritedPodInfo(ctx context.Context, client kubernetes.Interface, veler podInfo.env = podSpec.Containers[0].Env podInfo.envFrom = podSpec.Containers[0].EnvFrom - podInfo.volumeMounts = podSpec.Containers[0].VolumeMounts - podInfo.volumes = podSpec.Volumes + podInfo.volumeMounts, podInfo.volumes = excludeVolumes(podSpec.Containers[0].VolumeMounts, podSpec.Volumes, excludedVolumes) podInfo.dnsPolicy = podSpec.DNSPolicy podInfo.dnsConfig = podSpec.DNSConfig @@ -81,3 +97,39 @@ func getInheritedPodInfo(ctx context.Context, client kubernetes.Interface, veler return podInfo, nil } + +// excludeVolumes removes the volumes matching the given names, as well as the volume mounts +// referring to them, from the given volumes and volume mounts. An excluded name that doesn't +// match any volume is a no-op, so callers don't need to know how the node-agent daemonset is +// configured. Volumes that are not excluded, including the ones customized by users, are kept +// as is. +func excludeVolumes(volumeMounts []corev1api.VolumeMount, volumes []corev1api.Volume, excludedVolumes []string) ([]corev1api.VolumeMount, []corev1api.Volume) { + if len(excludedVolumes) == 0 { + return volumeMounts, volumes + } + + excluded := make(map[string]struct{}, len(excludedVolumes)) + for _, name := range excludedVolumes { + excluded[name] = struct{}{} + } + + var retainedMounts []corev1api.VolumeMount + for _, volumeMount := range volumeMounts { + if _, found := excluded[volumeMount.Name]; found { + continue + } + + retainedMounts = append(retainedMounts, volumeMount) + } + + var retainedVolumes []corev1api.Volume + for _, volume := range volumes { + if _, found := excluded[volume.Name]; found { + continue + } + + retainedVolumes = append(retainedVolumes, volume) + } + + return retainedMounts, retainedVolumes +} diff --git a/pkg/exposer/image_daemonset_test.go b/pkg/exposer/image_daemonset_test.go new file mode 100644 index 000000000..0941d44f7 --- /dev/null +++ b/pkg/exposer/image_daemonset_test.go @@ -0,0 +1,75 @@ +package exposer + +import ( + "context" + "testing" + + appsv1api "k8s.io/api/apps/v1" + corev1api "k8s.io/api/core/v1" + "k8s.io/client-go/kubernetes/fake" + + "github.com/vmware-tanzu/velero/pkg/install" +) + +// TestInheritedPodInfoAgainstRealDaemonSet guards the exclusion against the node-agent +// daemonset that is actually installed, so that a host path volume added to the daemonset +// later is not silently inherited by the data mover pods. +func TestInheritedPodInfoAgainstRealDaemonSet(t *testing.T) { + nodeAgent := install.DaemonSet("velero") + client := fake.NewSimpleClientset(&appsv1api.DaemonSet{ + ObjectMeta: nodeAgent.ObjectMeta, + Spec: nodeAgent.Spec, + }) + + hostPathVolumes := func(volumes []corev1api.Volume) []string { + names := []string{} + for _, volume := range volumes { + if volume.HostPath != nil { + names = append(names, volume.Name) + } + } + return names + } + + // The installed daemonset must carry host path volumes, otherwise this test is vacuous. + if len(hostPathVolumes(nodeAgent.Spec.Template.Spec.Volumes)) == 0 { + t.Fatal("the installed node-agent daemonset is expected to have host path volumes") + } + + // fs-backup resolves pod volume data through the kubelet pod directory, so it keeps them. + fsBackupInfo, err := getInheritedPodInfo(context.Background(), client, "velero", "linux") + if err != nil { + t.Fatalf("error to get inherited pod info for fs-backup: %v", err) + } + + if len(hostPathVolumes(fsBackupInfo.volumes)) == 0 { + t.Error("fs-backup is expected to inherit the host path volumes") + } + + // The data mover pods access data through PVCs, so they must not get any host path. + dataMoverInfo, err := getInheritedPodInfo(context.Background(), client, "velero", "linux", hostPathVolumesOfNodeAgent...) + if err != nil { + t.Fatalf("error to get inherited pod info for data mover: %v", err) + } + + if inherited := hostPathVolumes(dataMoverInfo.volumes); len(inherited) > 0 { + t.Errorf("data mover pods are not expected to inherit host path volumes, but got %v", inherited) + } + + // The other volumes, e.g., the scratch volume, are still required. + if len(dataMoverInfo.volumes) == 0 { + t.Error("data mover pods are expected to inherit the volumes other than the host path ones") + } + + // Every remaining mount must still have its backing volume. + volumeNames := map[string]struct{}{} + for _, volume := range dataMoverInfo.volumes { + volumeNames[volume.Name] = struct{}{} + } + + for _, volumeMount := range dataMoverInfo.volumeMounts { + if _, exist := volumeNames[volumeMount.Name]; !exist { + t.Errorf("volume mount %q doesn't have a backing volume", volumeMount.Name) + } + } +} diff --git a/pkg/exposer/image_test.go b/pkg/exposer/image_test.go index 5c47f5c04..d93a667ba 100644 --- a/pkg/exposer/image_test.go +++ b/pkg/exposer/image_test.go @@ -26,6 +26,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes" + "github.com/vmware-tanzu/velero/pkg/nodeagent" "github.com/vmware-tanzu/velero/pkg/util/kube" appsv1api "k8s.io/api/apps/v1" @@ -187,16 +188,118 @@ func TestGetInheritedPodInfo(t *testing.T) { }, } + daemonSetWithHostPath := &appsv1api.DaemonSet{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "fake-ns", + Name: "node-agent", + }, + TypeMeta: metav1.TypeMeta{ + Kind: "DaemonSet", + }, + Spec: appsv1api.DaemonSetSpec{ + Template: corev1api.PodTemplateSpec{ + Spec: corev1api.PodSpec{ + Containers: []corev1api.Container{ + { + Name: "container-1", + Image: "image-1", + VolumeMounts: []corev1api.VolumeMount{ + { + Name: nodeagent.HostPodVolumeMount, + MountPath: "/host_pods", + }, + { + Name: hostPluginsVolumeName, + MountPath: "/var/lib/kubelet/plugins", + }, + { + Name: "scratch", + MountPath: "/scratch", + }, + { + Name: "user-credentials", + MountPath: "/credentials", + }, + }, + }, + }, + Volumes: []corev1api.Volume{ + { + Name: nodeagent.HostPodVolumeMount, + VolumeSource: corev1api.VolumeSource{ + HostPath: &corev1api.HostPathVolumeSource{ + Path: "/var/lib/kubelet/pods", + }, + }, + }, + { + Name: hostPluginsVolumeName, + VolumeSource: corev1api.VolumeSource{ + HostPath: &corev1api.HostPathVolumeSource{ + Path: "/var/lib/kubelet/plugins", + }, + }, + }, + { + Name: "scratch", + VolumeSource: corev1api.VolumeSource{ + EmptyDir: new(corev1api.EmptyDirVolumeSource), + }, + }, + { + Name: "user-credentials", + VolumeSource: corev1api.VolumeSource{ + Secret: &corev1api.SecretVolumeSource{ + SecretName: "user-credentials", + }, + }, + }, + }, + ServiceAccountName: "sa-1", + }, + }, + }, + } + + scratchAndCredentialMounts := []corev1api.VolumeMount{ + { + Name: "scratch", + MountPath: "/scratch", + }, + { + Name: "user-credentials", + MountPath: "/credentials", + }, + } + + scratchAndCredentialVolumes := []corev1api.Volume{ + { + Name: "scratch", + VolumeSource: corev1api.VolumeSource{ + EmptyDir: new(corev1api.EmptyDirVolumeSource), + }, + }, + { + Name: "user-credentials", + VolumeSource: corev1api.VolumeSource{ + Secret: &corev1api.SecretVolumeSource{ + SecretName: "user-credentials", + }, + }, + }, + } + scheme := runtime.NewScheme() appsv1api.AddToScheme(scheme) tests := []struct { - name string - namespace string - client kubernetes.Interface - kubeClientObj []runtime.Object - result inheritedPodInfo - expectErr string + name string + namespace string + client kubernetes.Interface + kubeClientObj []runtime.Object + excludedVolumes []string + result inheritedPodInfo + expectErr string }{ { name: "ds is not found", @@ -329,12 +432,131 @@ func TestGetInheritedPodInfo(t *testing.T) { }, }, }, + { + name: "no excluded volume, host path volumes are inherited", + namespace: "fake-ns", + kubeClientObj: []runtime.Object{ + daemonSetWithHostPath, + }, + result: inheritedPodInfo{ + image: "image-1", + serviceAccount: "sa-1", + volumeMounts: daemonSetWithHostPath.Spec.Template.Spec.Containers[0].VolumeMounts, + volumes: daemonSetWithHostPath.Spec.Template.Spec.Volumes, + }, + }, + { + name: "host path volumes and their mounts are excluded", + namespace: "fake-ns", + kubeClientObj: []runtime.Object{ + daemonSetWithHostPath, + }, + excludedVolumes: hostPathVolumesOfNodeAgent, + result: inheritedPodInfo{ + image: "image-1", + serviceAccount: "sa-1", + volumeMounts: scratchAndCredentialMounts, + volumes: scratchAndCredentialVolumes, + }, + }, + { + name: "excluding a volume that doesn't exist doesn't affect the others", + namespace: "fake-ns", + kubeClientObj: []runtime.Object{ + daemonSetWithNoLog, + }, + excludedVolumes: hostPathVolumesOfNodeAgent, + result: inheritedPodInfo{ + image: "image-1", + serviceAccount: "sa-1", + env: []corev1api.EnvVar{ + { + Name: "env-1", + Value: "value-1", + }, + { + Name: "env-2", + Value: "value-2", + }, + }, + envFrom: []corev1api.EnvFromSource{ + { + ConfigMapRef: &corev1api.ConfigMapEnvSource{ + LocalObjectReference: corev1api.LocalObjectReference{ + Name: "test-configmap", + }, + }, + }, + { + SecretRef: &corev1api.SecretEnvSource{ + LocalObjectReference: corev1api.LocalObjectReference{ + Name: "test-secret", + }, + }, + }, + }, + volumeMounts: []corev1api.VolumeMount{ + { + Name: "volume-1", + }, + { + Name: "volume-2", + }, + }, + volumes: []corev1api.Volume{ + { + Name: "volume-1", + }, + { + Name: "volume-2", + }, + }, + }, + }, + { + name: "excluding all volumes results in empty volumes and mounts", + namespace: "fake-ns", + kubeClientObj: []runtime.Object{ + daemonSetWithNoLog, + }, + excludedVolumes: []string{"volume-1", "volume-2"}, + result: inheritedPodInfo{ + image: "image-1", + serviceAccount: "sa-1", + env: []corev1api.EnvVar{ + { + Name: "env-1", + Value: "value-1", + }, + { + Name: "env-2", + Value: "value-2", + }, + }, + envFrom: []corev1api.EnvFromSource{ + { + ConfigMapRef: &corev1api.ConfigMapEnvSource{ + LocalObjectReference: corev1api.LocalObjectReference{ + Name: "test-configmap", + }, + }, + }, + { + SecretRef: &corev1api.SecretEnvSource{ + LocalObjectReference: corev1api.LocalObjectReference{ + Name: "test-secret", + }, + }, + }, + }, + }, + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { fakeKubeClient := fake.NewSimpleClientset(test.kubeClientObj...) - info, err := getInheritedPodInfo(t.Context(), fakeKubeClient, test.namespace, kube.NodeOSLinux) + info, err := getInheritedPodInfo(t.Context(), fakeKubeClient, test.namespace, kube.NodeOSLinux, test.excludedVolumes...) if test.expectErr == "" { require.NoError(t, err)