From 0eec47e5744c26fa22f4cfc5de44947bb085ed0c Mon Sep 17 00:00:00 2001 From: chlins Date: Tue, 4 Aug 2026 13:26:47 +0800 Subject: [PATCH 1/2] 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) From bfda68ca3a061ee7f250ea493a0f2abd467f232c Mon Sep 17 00:00:00 2001 From: chlins Date: Tue, 4 Aug 2026 14:22:01 +0800 Subject: [PATCH 2/2] Address review comments on host path exclusion Detect the host path volumes by their source instead of their name, so the customized ones are excluded as well. Drop the container capability changes since the data mover needs them to access the data, and fix the copyright headers. Signed-off-by: chlins --- pkg/exposer/csi_snapshot.go | 30 ++++-------- pkg/exposer/generic_restore.go | 30 ++++-------- pkg/exposer/image.go | 68 +++++++++++++-------------- pkg/exposer/image_daemonset_test.go | 20 +++++++- pkg/exposer/image_test.go | 72 ++++++++++------------------- pkg/exposer/pod_volume.go | 4 +- 6 files changed, 92 insertions(+), 132 deletions(-) diff --git a/pkg/exposer/csi_snapshot.go b/pkg/exposer/csi_snapshot.go index 3fd78cb9b..2e8e08889 100644 --- a/pkg/exposer/csi_snapshot.go +++ b/pkg/exposer/csi_snapshot.go @@ -1,5 +1,5 @@ /* -Copyright The Velero Contributors. +Copyright the Velero contributors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -686,7 +686,7 @@ func (e *csiSnapshotExposer) createBackupPod( // 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...) + podInfo, err := getInheritedPodInfo(ctx, e.kubeClient, ownerObject.Namespace, nodeOS, excludeHostPathVolumes) if err != nil { return nil, errors.Wrap(err, "error to get inherited pod info from node-agent") } @@ -752,7 +752,6 @@ func (e *csiSnapshotExposer) createBackupPod( } var securityCtx *corev1api.PodSecurityContext - var containerSecurityCtx *corev1api.SecurityContext nodeSelector := map[string]string{} podOS := corev1api.PodOS{} if nodeOS == kube.NodeOSWindows { @@ -791,18 +790,6 @@ 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", @@ -874,13 +861,12 @@ func (e *csiSnapshotExposer) createBackupPod( "data-mover", "backup", }, - Args: args, - VolumeMounts: volumeMounts, - VolumeDevices: volumeDevices, - Env: podInfo.env, - EnvFrom: podInfo.envFrom, - Resources: resources, - SecurityContext: containerSecurityCtx, + Args: args, + VolumeMounts: volumeMounts, + VolumeDevices: volumeDevices, + Env: podInfo.env, + EnvFrom: podInfo.envFrom, + Resources: resources, }, }, PriorityClassName: priorityClassName, diff --git a/pkg/exposer/generic_restore.go b/pkg/exposer/generic_restore.go index 46851803c..16a114e64 100644 --- a/pkg/exposer/generic_restore.go +++ b/pkg/exposer/generic_restore.go @@ -1,5 +1,5 @@ /* -Copyright The Velero Contributors. +Copyright the Velero contributors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -630,7 +630,7 @@ func (e *genericRestoreExposer) createRestorePod( // 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...) + podInfo, err := getInheritedPodInfo(ctx, e.kubeClient, ownerObject.Namespace, nodeOS, excludeHostPathVolumes) if err != nil { return nil, errors.Wrap(err, "error to get inherited pod info from node-agent") } @@ -694,7 +694,6 @@ 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" @@ -732,18 +731,6 @@ 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{ @@ -796,13 +783,12 @@ func (e *genericRestoreExposer) createRestorePod( "data-mover", "restore", }, - Args: args, - VolumeMounts: volumeMounts, - VolumeDevices: volumeDevices, - Env: podInfo.env, - EnvFrom: podInfo.envFrom, - Resources: resources, - SecurityContext: containerSecurityCtx, + Args: args, + VolumeMounts: volumeMounts, + VolumeDevices: volumeDevices, + Env: podInfo.env, + EnvFrom: podInfo.envFrom, + Resources: resources, }, }, PriorityClassName: priorityClassName, diff --git a/pkg/exposer/image.go b/pkg/exposer/image.go index aa774221b..396303d4f 100644 --- a/pkg/exposer/image.go +++ b/pkg/exposer/image.go @@ -1,5 +1,5 @@ /* -Copyright The Velero Contributors. +Copyright the Velero contributors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -28,17 +28,16 @@ import ( ) const ( - // hostPluginsVolumeName is the name of the node-agent volume that mounts the kubelet - // plugins directory from the host. - hostPluginsVolumeName = "host-plugins" -) + // excludeHostPathVolumes indicates that the volumes backed by a host path are not + // inherited from the node-agent. The exposers accessing data through PVCs use it so + // that the hosting pods don't get unnecessary access to the host file system. + excludeHostPathVolumes = true -// 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} + // inheritHostPathVolumes indicates that the volumes backed by a host path are + // inherited from the node-agent. fs-backup uses it because it resolves and accesses + // the pod volume data through the kubelet pod directory on the host. + inheritHostPathVolumes = false +) type inheritedPodInfo struct { image string @@ -55,10 +54,11 @@ type inheritedPodInfo struct { } // 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) { +// node-agent pod template. When excludeHostPath is true, the volumes backed by a host path, +// together with their volume mounts, are dropped from the result. The volumes are detected +// by their source instead of their name, so the ones customized in the node-agent daemonset +// are covered as well. +func getInheritedPodInfo(ctx context.Context, client kubernetes.Interface, veleroNamespace string, osType string, excludeHostPath bool) (inheritedPodInfo, error) { podInfo := inheritedPodInfo{} podSpec, err := nodeagent.GetPodSpec(ctx, client, veleroNamespace, osType) @@ -75,7 +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, podInfo.volumes = excludeVolumes(podSpec.Containers[0].VolumeMounts, podSpec.Volumes, excludedVolumes) + podInfo.volumeMounts, podInfo.volumes = filterVolumes(podSpec.Containers[0].VolumeMounts, podSpec.Volumes, excludeHostPath) podInfo.dnsPolicy = podSpec.DNSPolicy podInfo.dnsConfig = podSpec.DNSConfig @@ -98,22 +98,27 @@ 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 { +// filterVolumes removes the volumes backed by a host path, as well as the volume mounts +// referring to them, when excludeHostPath is true. The volumes are recognized by their +// source, so the host path volumes customized in the node-agent daemonset are removed as +// well. The other volumes, including the ones customized by users, are kept as is. +func filterVolumes(volumeMounts []corev1api.VolumeMount, volumes []corev1api.Volume, excludeHostPath bool) ([]corev1api.VolumeMount, []corev1api.Volume) { + if !excludeHostPath { return volumeMounts, volumes } - excluded := make(map[string]struct{}, len(excludedVolumes)) - for _, name := range excludedVolumes { - excluded[name] = struct{}{} + excluded := make(map[string]struct{}) + retainedVolumes := make([]corev1api.Volume, 0, len(volumes)) + for _, volume := range volumes { + if volume.HostPath != nil { + excluded[volume.Name] = struct{}{} + continue + } + + retainedVolumes = append(retainedVolumes, volume) } - var retainedMounts []corev1api.VolumeMount + retainedMounts := make([]corev1api.VolumeMount, 0, len(volumeMounts)) for _, volumeMount := range volumeMounts { if _, found := excluded[volumeMount.Name]; found { continue @@ -122,14 +127,5 @@ func excludeVolumes(volumeMounts []corev1api.VolumeMount, volumes []corev1api.Vo 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 index 0941d44f7..21ae104df 100644 --- a/pkg/exposer/image_daemonset_test.go +++ b/pkg/exposer/image_daemonset_test.go @@ -1,3 +1,19 @@ +/* +Copyright the Velero contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package exposer import ( @@ -37,7 +53,7 @@ func TestInheritedPodInfoAgainstRealDaemonSet(t *testing.T) { } // fs-backup resolves pod volume data through the kubelet pod directory, so it keeps them. - fsBackupInfo, err := getInheritedPodInfo(context.Background(), client, "velero", "linux") + fsBackupInfo, err := getInheritedPodInfo(context.Background(), client, "velero", "linux", inheritHostPathVolumes) if err != nil { t.Fatalf("error to get inherited pod info for fs-backup: %v", err) } @@ -47,7 +63,7 @@ func TestInheritedPodInfoAgainstRealDaemonSet(t *testing.T) { } // 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...) + dataMoverInfo, err := getInheritedPodInfo(context.Background(), client, "velero", "linux", excludeHostPathVolumes) if err != nil { t.Fatalf("error to get inherited pod info for data mover: %v", err) } diff --git a/pkg/exposer/image_test.go b/pkg/exposer/image_test.go index d93a667ba..a7672b344 100644 --- a/pkg/exposer/image_test.go +++ b/pkg/exposer/image_test.go @@ -1,5 +1,5 @@ /* -Copyright The Velero Contributors. +Copyright the Velero contributors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -209,9 +209,13 @@ func TestGetInheritedPodInfo(t *testing.T) { MountPath: "/host_pods", }, { - Name: hostPluginsVolumeName, + Name: "host-plugins", MountPath: "/var/lib/kubelet/plugins", }, + { + Name: "customized-host-path", + MountPath: "/customized", + }, { Name: "scratch", MountPath: "/scratch", @@ -233,13 +237,23 @@ func TestGetInheritedPodInfo(t *testing.T) { }, }, { - Name: hostPluginsVolumeName, + Name: "host-plugins", VolumeSource: corev1api.VolumeSource{ HostPath: &corev1api.HostPathVolumeSource{ Path: "/var/lib/kubelet/plugins", }, }, }, + { + // A host path volume added by users. It's not named after any + // well-known volume, so it can only be recognized by its source. + Name: "customized-host-path", + VolumeSource: corev1api.VolumeSource{ + HostPath: &corev1api.HostPathVolumeSource{ + Path: "/mnt/customized", + }, + }, + }, { Name: "scratch", VolumeSource: corev1api.VolumeSource{ @@ -297,7 +311,7 @@ func TestGetInheritedPodInfo(t *testing.T) { namespace string client kubernetes.Interface kubeClientObj []runtime.Object - excludedVolumes []string + excludeHostPath bool result inheritedPodInfo expectErr string }{ @@ -433,7 +447,7 @@ func TestGetInheritedPodInfo(t *testing.T) { }, }, { - name: "no excluded volume, host path volumes are inherited", + name: "host path volumes are inherited by default", namespace: "fake-ns", kubeClientObj: []runtime.Object{ daemonSetWithHostPath, @@ -446,12 +460,12 @@ func TestGetInheritedPodInfo(t *testing.T) { }, }, { - name: "host path volumes and their mounts are excluded", + name: "host path volumes and their mounts are excluded, no matter how they are named", namespace: "fake-ns", kubeClientObj: []runtime.Object{ daemonSetWithHostPath, }, - excludedVolumes: hostPathVolumesOfNodeAgent, + excludeHostPath: true, result: inheritedPodInfo{ image: "image-1", serviceAccount: "sa-1", @@ -460,12 +474,12 @@ func TestGetInheritedPodInfo(t *testing.T) { }, }, { - name: "excluding a volume that doesn't exist doesn't affect the others", + name: "excluding host path volumes keeps the others when there is none", namespace: "fake-ns", kubeClientObj: []runtime.Object{ daemonSetWithNoLog, }, - excludedVolumes: hostPathVolumesOfNodeAgent, + excludeHostPath: true, result: inheritedPodInfo{ image: "image-1", serviceAccount: "sa-1", @@ -513,50 +527,12 @@ func TestGetInheritedPodInfo(t *testing.T) { }, }, }, - { - 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, test.excludedVolumes...) + info, err := getInheritedPodInfo(t.Context(), fakeKubeClient, test.namespace, kube.NodeOSLinux, test.excludeHostPath) if test.expectErr == "" { require.NoError(t, err) diff --git a/pkg/exposer/pod_volume.go b/pkg/exposer/pod_volume.go index 0526b2c5e..5d6de1831 100644 --- a/pkg/exposer/pod_volume.go +++ b/pkg/exposer/pod_volume.go @@ -1,5 +1,5 @@ /* -Copyright The Velero Contributors. +Copyright the Velero contributors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -365,7 +365,7 @@ func (e *podVolumeExposer) createHostingPod( clientVolumeName := string(ownerObject.UID) clientVolumePath := "/" + clientVolumeName - podInfo, err := getInheritedPodInfo(ctx, e.kubeClient, ownerObject.Namespace, nodeOS) + podInfo, err := getInheritedPodInfo(ctx, e.kubeClient, ownerObject.Namespace, nodeOS, inheritHostPathVolumes) if err != nil { return nil, errors.Wrap(err, "error to get inherited pod info from node-agent") }