From 8af3cce7fec15a65a9c42823880569e36b9546db Mon Sep 17 00:00:00 2001 From: Rodrigo Campos Date: Tue, 7 Mar 2023 17:12:02 +0100 Subject: [PATCH] kubelet: remove GetHostIDsForPod() Now KEP-127 relies on idmap mounts to do the ID translation and we won't do any chowns in the kubelet. This patch just removes the usage of GetHostIDsForPod() in operationexecutor to do the chown, and also removes the GetHostIDsForPod() method from the kubelet volume interface. Signed-off-by: Rodrigo Campos --- .../attachdetach/attach_detach_controller.go | 4 - .../volume/expand/expand_controller.go | 4 - .../volume/persistentvolume/volume_host.go | 4 - pkg/kubelet/kubelet_pods.go | 4 - pkg/kubelet/userns_manager.go | 65 --------- pkg/kubelet/userns_manager_test.go | 131 ------------------ pkg/kubelet/volume_host.go | 10 -- pkg/volume/plugins.go | 7 - pkg/volume/testing/volume_host.go | 4 - .../operationexecutor/operation_generator.go | 29 +--- 10 files changed, 2 insertions(+), 260 deletions(-) diff --git a/pkg/controller/volume/attachdetach/attach_detach_controller.go b/pkg/controller/volume/attachdetach/attach_detach_controller.go index 967f40f172b..e323bcb6759 100644 --- a/pkg/controller/volume/attachdetach/attach_detach_controller.go +++ b/pkg/controller/volume/attachdetach/attach_detach_controller.go @@ -819,10 +819,6 @@ func (adc *attachDetachController) GetPodVolumeDir(podUID types.UID, pluginName, return "" } -func (adc *attachDetachController) GetHostIDsForPod(pod *v1.Pod, containerUID, containerGID *int64) (hostUID, hostGID *int64, err error) { - return nil, nil, nil -} - func (adc *attachDetachController) GetPodPluginDir(podUID types.UID, pluginName string) string { return "" } diff --git a/pkg/controller/volume/expand/expand_controller.go b/pkg/controller/volume/expand/expand_controller.go index 31a01ae56c4..37495504de8 100644 --- a/pkg/controller/volume/expand/expand_controller.go +++ b/pkg/controller/volume/expand/expand_controller.go @@ -393,10 +393,6 @@ func (expc *expandController) GetPodsDir() string { return "" } -func (expc *expandController) GetHostIDsForPod(pod *v1.Pod, containerUID, containerGID *int64) (hostUID, hostGID *int64, err error) { - return nil, nil, nil -} - func (expc *expandController) GetPodVolumeDir(podUID types.UID, pluginName string, volumeName string) string { return "" } diff --git a/pkg/controller/volume/persistentvolume/volume_host.go b/pkg/controller/volume/persistentvolume/volume_host.go index b47f183a61d..c064823c9c2 100644 --- a/pkg/controller/volume/persistentvolume/volume_host.go +++ b/pkg/controller/volume/persistentvolume/volume_host.go @@ -55,10 +55,6 @@ func (ctrl *PersistentVolumeController) GetPodVolumeDir(podUID types.UID, plugin return "" } -func (ctrl *PersistentVolumeController) GetHostIDsForPod(pod *v1.Pod, containerUID, containerGID *int64) (hostUID, hostGID *int64, err error) { - return nil, nil, nil -} - func (ctrl *PersistentVolumeController) GetPodPluginDir(podUID types.UID, pluginName string) string { return "" } diff --git a/pkg/kubelet/kubelet_pods.go b/pkg/kubelet/kubelet_pods.go index 1f0080d6b19..3a77baa2464 100644 --- a/pkg/kubelet/kubelet_pods.go +++ b/pkg/kubelet/kubelet_pods.go @@ -426,10 +426,6 @@ func (kl *Kubelet) GetOrCreateUserNamespaceMappings(pod *v1.Pod) (*runtimeapi.Us return kl.usernsManager.GetOrCreateUserNamespaceMappings(pod) } -func (kl *Kubelet) getHostIDsForPod(pod *v1.Pod, containerUID, containerGID *int64) (hostUID, hostGID *int64, err error) { - return kl.usernsManager.getHostIDsForPod(pod, containerUID, containerGID) -} - // GeneratePodHostNameAndDomain creates a hostname and domain name for a pod, // given that pod's spec and annotations or returns an error. func (kl *Kubelet) GeneratePodHostNameAndDomain(pod *v1.Pod) (string, string, error) { diff --git a/pkg/kubelet/userns_manager.go b/pkg/kubelet/userns_manager.go index 6b03a976824..c1e05931743 100644 --- a/pkg/kubelet/userns_manager.go +++ b/pkg/kubelet/userns_manager.go @@ -479,68 +479,3 @@ func (m *usernsManager) CleanupOrphanedPodUsernsAllocations(pods []*v1.Pod, runn return nil } - -// getHostIDsForPod if the pod uses user namespaces, takes the uid and gid -// inside the container and returns the host UID and GID those are mapped to on -// the host. If containerUID/containerGID is nil, then it returns the host -// UID/GID for ID 0 inside the container. -// If the pod is not using user namespaces, as there is no mapping needed, the -// same containerUID and containerGID params are returned. -func (m *usernsManager) getHostIDsForPod(pod *v1.Pod, containerUID, containerGID *int64) (hostUID, hostGID *int64, err error) { - if !utilfeature.DefaultFeatureGate.Enabled(features.UserNamespacesStatelessPodsSupport) { - return containerUID, containerGID, nil - } - - if pod == nil || pod.Spec.HostUsers == nil || *pod.Spec.HostUsers == true { - return containerUID, containerGID, nil - } - - mapping, err := m.GetOrCreateUserNamespaceMappings(pod) - if err != nil { - err = fmt.Errorf("Error getting pod user namespace mapping: %w", err) - return - } - - uid, err := hostIDFromMapping(mapping.Uids, containerUID) - if err != nil { - err = fmt.Errorf("Error getting host UID: %w", err) - return - } - - gid, err := hostIDFromMapping(mapping.Gids, containerGID) - if err != nil { - err = fmt.Errorf("Error getting host GID: %w", err) - return - } - - return &uid, &gid, nil -} - -func hostIDFromMapping(mapping []*runtimeapi.IDMapping, containerId *int64) (int64, error) { - if len(mapping) == 0 { - return 0, fmt.Errorf("can't use empty user namespace mapping") - } - - // If none is requested, root inside the container is used - id := int64(0) - if containerId != nil { - id = *containerId - } - - for _, m := range mapping { - if m == nil { - continue - } - - firstId := int64(m.ContainerId) - lastId := firstId + int64(m.Length) - 1 - - // The id we are looking for is in the range - if id >= firstId && id <= lastId { - // Return the host id for this container id - return int64(m.HostId) + id - firstId, nil - } - } - - return 0, fmt.Errorf("ID: %v not present in pod user namespace", id) -} diff --git a/pkg/kubelet/userns_manager_test.go b/pkg/kubelet/userns_manager_test.go index d61d6d733ff..594de3f59f1 100644 --- a/pkg/kubelet/userns_manager_test.go +++ b/pkg/kubelet/userns_manager_test.go @@ -25,7 +25,6 @@ import ( "k8s.io/apimachinery/pkg/types" utilfeature "k8s.io/apiserver/pkg/util/feature" featuregatetesting "k8s.io/component-base/featuregate/testing" - runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1" pkgfeatures "k8s.io/kubernetes/pkg/features" ) @@ -173,133 +172,3 @@ func TestUserNsManagerParseUserNsFile(t *testing.T) { }) } } - -func TestUserNsManagerHostIDFromMapping(t *testing.T) { - // mapping []*runtimeapi.IDMapping, containerId *int64 - - cases := []struct { - name string - success bool - containerId int64 // -1 means a nil ptr will be used. - expHostId int64 - m []*runtimeapi.IDMapping - }{ - { - name: "one basic mapping", - success: true, - containerId: -1, - expHostId: 0, - m: []*runtimeapi.IDMapping{ - { - HostId: 0, - ContainerId: 0, - Length: userNsLength, - }, - }, - }, - { - name: "one unprivileged mapping", - success: true, - containerId: -1, - expHostId: userNsLength * 2, - m: []*runtimeapi.IDMapping{ - { - HostId: userNsLength * 2, - ContainerId: 0, - Length: userNsLength, - }, - }, - }, - { - name: "one unprivileged mapping random id", - success: true, - containerId: 3, - expHostId: userNsLength*2 + 3, - m: []*runtimeapi.IDMapping{ - { - HostId: userNsLength * 2, - ContainerId: 0, - Length: userNsLength, - }, - }, - }, - { - name: "two unprivileged mapping", - success: true, - containerId: 0, - expHostId: userNsLength*2 + 0, - m: []*runtimeapi.IDMapping{ - { - HostId: userNsLength * 2, - ContainerId: 0, - Length: 1, - }, - { - HostId: userNsLength*2 + 10, - ContainerId: 1, - Length: 1, - }, - }, - }, - { - name: "two unprivileged mapping - random id", - success: true, - containerId: 1, - expHostId: userNsLength*2 + 10, - m: []*runtimeapi.IDMapping{ - { - HostId: userNsLength * 2, - ContainerId: 0, - Length: 1, - }, - { - HostId: userNsLength*2 + 10, - ContainerId: 1, - Length: 1, - }, - }, - }, - { - name: "two unprivileged mapping - not mapped user", - success: false, - containerId: 3, - m: []*runtimeapi.IDMapping{ - { - HostId: userNsLength * 2, - ContainerId: 0, - Length: 1, - }, - { - HostId: userNsLength*2 + 1, - ContainerId: 1, - Length: 1, - }, - }, - }, - { - name: "no mappings", - success: false, - }, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - var containerId *int64 - if tc.containerId != -1 { - containerId = &tc.containerId - } - - id, err := hostIDFromMapping(tc.m, containerId) - if (tc.success && err != nil) || (!tc.success && err == nil) { - t.Fatalf("%v: expected success: %v - got error: %v", tc.name, tc.success, err) - } - if !tc.success && err != nil { - return - } - - if id != tc.expHostId { - t.Errorf("expected: %v - got: %v", tc.expHostId, id) - } - }) - } -} diff --git a/pkg/kubelet/volume_host.go b/pkg/kubelet/volume_host.go index 1a79460680a..ec321b4066e 100644 --- a/pkg/kubelet/volume_host.go +++ b/pkg/kubelet/volume_host.go @@ -128,16 +128,6 @@ func (kvh *kubeletVolumeHost) GetPodsDir() string { return kvh.kubelet.getPodsDir() } -// GetHostIDsForPod if the pod uses user namespaces, takes the uid and gid -// inside the container and returns the host UID and GID those are mapped to on -// the host. If containerUID/containerGID is nil, then it returns the host -// UID/GID for ID 0 inside the container. -// If the pod is not using user namespaces, as there is no mapping needed, the -// same containerUID and containerGID params are returned. -func (kvh *kubeletVolumeHost) GetHostIDsForPod(pod *v1.Pod, containerUID, containerGID *int64) (hostUID, hostGID *int64, err error) { - return kvh.kubelet.getHostIDsForPod(pod, containerUID, containerGID) -} - func (kvh *kubeletVolumeHost) GetPodVolumeDir(podUID types.UID, pluginName string, volumeName string) string { dir := kvh.kubelet.getPodVolumeDir(podUID, pluginName, volumeName) if runtime.GOOS == "windows" { diff --git a/pkg/volume/plugins.go b/pkg/volume/plugins.go index c0ec12f0c0f..82013449f1f 100644 --- a/pkg/volume/plugins.go +++ b/pkg/volume/plugins.go @@ -334,13 +334,6 @@ type KubeletVolumeHost interface { WaitForCacheSync() error // Returns hostutil.HostUtils GetHostUtil() hostutil.HostUtils - // GetHostIDsForPod if the pod uses user namespaces, takes the uid and - // gid inside the container and returns the host UID and GID those are - // mapped to on the host. If containerUID/containerGID is nil, then it - // returns the host UID/GID for ID 0 inside the container. - // If the pod is not using user namespaces, as there is no mapping needed, the - // same containerUID and containerGID params are returned. - GetHostIDsForPod(pod *v1.Pod, containerUID, containerGID *int64) (hostUID, hostGID *int64, err error) } // AttachDetachVolumeHost is a AttachDetach Controller specific interface that plugins can use diff --git a/pkg/volume/testing/volume_host.go b/pkg/volume/testing/volume_host.go index 457ab3ce995..78c4a70566a 100644 --- a/pkg/volume/testing/volume_host.go +++ b/pkg/volume/testing/volume_host.go @@ -123,10 +123,6 @@ func (f *fakeVolumeHost) GetPodsDir() string { return filepath.Join(f.rootDir, "pods") } -func (f *fakeVolumeHost) GetHostIDsForPod(pod *v1.Pod, containerUID, containerGID *int64) (hostUID, hostGID *int64, err error) { - return containerUID, containerGID, nil -} - func (f *fakeVolumeHost) GetPodVolumeDir(podUID types.UID, pluginName, volumeName string) string { return filepath.Join(f.rootDir, "pods", string(podUID), "volumes", pluginName, volumeName) } diff --git a/pkg/volume/util/operationexecutor/operation_generator.go b/pkg/volume/util/operationexecutor/operation_generator.go index 269c8b51636..1effdf4acdb 100644 --- a/pkg/volume/util/operationexecutor/operation_generator.go +++ b/pkg/volume/util/operationexecutor/operation_generator.go @@ -679,35 +679,10 @@ func (og *operationGenerator) GenerateMountVolumeFunc( resizeOptions.DeviceStagePath = deviceStagePath } - // No mapping is needed for hostUID/hostGID if userns is not used. - // Therefore, just assign the container users to host UID/GID. - hostUID := util.FsUserFrom(volumeToMount.Pod) - hostGID := fsGroup - if utilfeature.DefaultFeatureGate.Enabled(features.UserNamespacesStatelessPodsSupport) { - // Without userns hostUID/GID was the user inside the container too. - containerUID, containerGID := hostUID, hostGID - - kvh, ok := og.GetVolumePluginMgr().Host.(volume.KubeletVolumeHost) - if !ok { - msg := fmt.Errorf("volume host does not implement KubeletVolumeHost interface") - eventErr, detailedErr := volumeToMount.GenerateError("MountVolume type assertion error", msg) - return volumetypes.NewOperationContext(eventErr, detailedErr, migrated) - } - - // This pod _might_ use userns. GetHostIDsForPod() will give us the right - // UID/GID to use for this pod (no matter if the pod uses userns or not). - hostUID, hostGID, err = kvh.GetHostIDsForPod(volumeToMount.Pod, containerUID, containerGID) - if err != nil { - msg := fmt.Sprintf("MountVolume.GetHostIDsForPod failed to find host ID in user namespace (UID: %v GID: %v)", containerUID, containerGID) - eventErr, detailedErr := volumeToMount.GenerateError(msg, err) - return volumetypes.NewOperationContext(eventErr, detailedErr, migrated) - } - } - // Execute mount mountErr := volumeMounter.SetUp(volume.MounterArgs{ - FsUser: hostUID, - FsGroup: hostGID, + FsUser: util.FsUserFrom(volumeToMount.Pod), + FsGroup: fsGroup, DesiredSize: volumeToMount.DesiredSizeLimit, FSGroupChangePolicy: fsGroupChangePolicy, SELinuxLabel: volumeToMount.SELinuxLabel,