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") }