From b6c9c2d6fa6cd812131d19567a86ae7126dcd2dd Mon Sep 17 00:00:00 2001 From: carlory Date: Tue, 5 Nov 2024 16:35:59 +0800 Subject: [PATCH] 1. When the kubelet constructs the cri mounts for the container which references an `image` volume source type, It passes the missing mount attributes to the CRI implementation, including `readOnly`, `propagation`, and `recursiveReadOnly`. When the readOnly field of the containerMount is explicitly set to false, the kubelet will take the `readOnly`as true to the CRI implementation because the image volume plugin requires the mount to be read-only. 2. Fix a bug where the pod is unexpectedly running when the `image` volume source type is used and mounted to `/etc/hosts` in the container. --- pkg/kubelet/kubelet_pods.go | 124 ++++++++++++++++++------------------ pkg/volume/image/image.go | 4 +- 2 files changed, 65 insertions(+), 63 deletions(-) diff --git a/pkg/kubelet/kubelet_pods.go b/pkg/kubelet/kubelet_pods.go index 6072a48027c..9e016b2f77c 100644 --- a/pkg/kubelet/kubelet_pods.go +++ b/pkg/kubelet/kubelet_pods.go @@ -277,18 +277,6 @@ func makeMounts(pod *v1.Pod, podDir string, container *v1.Container, hostName, h mounts := []kubecontainer.Mount{} var cleanupAction func() for i, mount := range container.VolumeMounts { - // Check if the mount is referencing an OCI volume - if imageVolumes != nil && utilfeature.DefaultFeatureGate.Enabled(features.ImageVolume) { - if image, ok := imageVolumes[mount.Name]; ok { - mounts = append(mounts, kubecontainer.Mount{ - Name: mount.Name, - ContainerPath: mount.MountPath, - Image: image, - }) - continue - } - } - // do not mount /etc/hosts if container is already mounting on the path mountEtcHostsFile = mountEtcHostsFile && (mount.MountPath != etcHostsPath) vol, ok := podVolumes[mount.Name] @@ -306,69 +294,82 @@ func makeMounts(pod *v1.Pod, podDir string, container *v1.Container, hostName, h vol.SELinuxLabeled = true relabelVolume = true } - hostPath, err := volumeutil.GetPath(vol.Mounter) - if err != nil { - return nil, cleanupAction, err + + var ( + hostPath string + image *runtimeapi.ImageSpec + err error + ) + + if imageVolumes != nil && utilfeature.DefaultFeatureGate.Enabled(features.ImageVolume) { + image = imageVolumes[mount.Name] } - subPath := mount.SubPath - if mount.SubPathExpr != "" { - subPath, err = kubecontainer.ExpandContainerVolumeMounts(mount, expandEnvs) - + if image == nil { + hostPath, err = volumeutil.GetPath(vol.Mounter) if err != nil { return nil, cleanupAction, err } - } - if subPath != "" { - if utilfs.IsAbs(subPath) { - return nil, cleanupAction, fmt.Errorf("error SubPath `%s` must not be an absolute path", subPath) - } + subPath := mount.SubPath + if mount.SubPathExpr != "" { + subPath, err = kubecontainer.ExpandContainerVolumeMounts(mount, expandEnvs) - err = volumevalidation.ValidatePathNoBacksteps(subPath) - if err != nil { - return nil, cleanupAction, fmt.Errorf("unable to provision SubPath `%s`: %v", subPath, err) - } - - volumePath := hostPath - hostPath = filepath.Join(volumePath, subPath) - - if subPathExists, err := hu.PathExists(hostPath); err != nil { - klog.ErrorS(nil, "Could not determine if subPath exists, will not attempt to change its permissions", "path", hostPath) - } else if !subPathExists { - // Create the sub path now because if it's auto-created later when referenced, it may have an - // incorrect ownership and mode. For example, the sub path directory must have at least g+rwx - // when the pod specifies an fsGroup, and if the directory is not created here, Docker will - // later auto-create it with the incorrect mode 0750 - // Make extra care not to escape the volume! - perm, err := hu.GetMode(volumePath) if err != nil { return nil, cleanupAction, err } - if err := subpather.SafeMakeDir(subPath, volumePath, perm); err != nil { + } + + if subPath != "" { + if utilfs.IsAbs(subPath) { + return nil, cleanupAction, fmt.Errorf("error SubPath `%s` must not be an absolute path", subPath) + } + + err = volumevalidation.ValidatePathNoBacksteps(subPath) + if err != nil { + return nil, cleanupAction, fmt.Errorf("unable to provision SubPath `%s`: %v", subPath, err) + } + + volumePath := hostPath + hostPath = filepath.Join(volumePath, subPath) + + if subPathExists, err := hu.PathExists(hostPath); err != nil { + klog.ErrorS(nil, "Could not determine if subPath exists, will not attempt to change its permissions", "path", hostPath) + } else if !subPathExists { + // Create the sub path now because if it's auto-created later when referenced, it may have an + // incorrect ownership and mode. For example, the sub path directory must have at least g+rwx + // when the pod specifies an fsGroup, and if the directory is not created here, Docker will + // later auto-create it with the incorrect mode 0750 + // Make extra care not to escape the volume! + perm, err := hu.GetMode(volumePath) + if err != nil { + return nil, cleanupAction, err + } + if err := subpather.SafeMakeDir(subPath, volumePath, perm); err != nil { + // Don't pass detailed error back to the user because it could give information about host filesystem + klog.ErrorS(err, "Failed to create subPath directory for volumeMount of the container", "containerName", container.Name, "volumeMountName", mount.Name) + return nil, cleanupAction, fmt.Errorf("failed to create subPath directory for volumeMount %q of container %q", mount.Name, container.Name) + } + } + hostPath, cleanupAction, err = subpather.PrepareSafeSubpath(subpath.Subpath{ + VolumeMountIndex: i, + Path: hostPath, + VolumeName: vol.InnerVolumeSpecName, + VolumePath: volumePath, + PodDir: podDir, + ContainerName: container.Name, + }) + if err != nil { // Don't pass detailed error back to the user because it could give information about host filesystem - klog.ErrorS(err, "Failed to create subPath directory for volumeMount of the container", "containerName", container.Name, "volumeMountName", mount.Name) - return nil, cleanupAction, fmt.Errorf("failed to create subPath directory for volumeMount %q of container %q", mount.Name, container.Name) + klog.ErrorS(err, "Failed to prepare subPath for volumeMount of the container", "containerName", container.Name, "volumeMountName", mount.Name) + return nil, cleanupAction, fmt.Errorf("failed to prepare subPath for volumeMount %q of container %q", mount.Name, container.Name) } } - hostPath, cleanupAction, err = subpather.PrepareSafeSubpath(subpath.Subpath{ - VolumeMountIndex: i, - Path: hostPath, - VolumeName: vol.InnerVolumeSpecName, - VolumePath: volumePath, - PodDir: podDir, - ContainerName: container.Name, - }) - if err != nil { - // Don't pass detailed error back to the user because it could give information about host filesystem - klog.ErrorS(err, "Failed to prepare subPath for volumeMount of the container", "containerName", container.Name, "volumeMountName", mount.Name) - return nil, cleanupAction, fmt.Errorf("failed to prepare subPath for volumeMount %q of container %q", mount.Name, container.Name) - } - } - // Docker Volume Mounts fail on Windows if it is not of the form C:/ - if volumeutil.IsWindowsLocalPath(runtime.GOOS, hostPath) { - hostPath = volumeutil.MakeAbsolutePath(runtime.GOOS, hostPath) + // Docker Volume Mounts fail on Windows if it is not of the form C:/ + if hostPath != "" && volumeutil.IsWindowsLocalPath(runtime.GOOS, hostPath) { + hostPath = volumeutil.MakeAbsolutePath(runtime.GOOS, hostPath) + } } containerPath := mount.MountPath @@ -396,6 +397,7 @@ func makeMounts(pod *v1.Pod, podDir string, container *v1.Container, hostName, h Name: mount.Name, ContainerPath: containerPath, HostPath: hostPath, + Image: image, ReadOnly: mount.ReadOnly || mustMountRO, RecursiveReadOnly: rro, SELinuxRelabel: relabelVolume, diff --git a/pkg/volume/image/image.go b/pkg/volume/image/image.go index fcadcb903cf..ac742e7cafc 100644 --- a/pkg/volume/image/image.go +++ b/pkg/volume/image/image.go @@ -67,8 +67,8 @@ func (o *imagePlugin) ConstructVolumeSpec(volumeName, mountPath string) (volume. func (o *imagePlugin) GetAttributes() volume.Attributes { return volume.Attributes{ ReadOnly: true, - Managed: true, - SELinuxRelabel: true, + Managed: false, + SELinuxRelabel: false, } }