From 0b5aa1d491c68bd917178beeb4cfcbc61922052b Mon Sep 17 00:00:00 2001 From: Rodrigo Campos Date: Wed, 30 Jul 2025 11:31:20 +0200 Subject: [PATCH] userns: Use better defaults if idsPerPod are used The first UID used for userns mappings needs to be at least the idsPerPod. When idsPerPod is extended to be 65536*2, for example, then the default UID of 65536 doesn't work. While this can be configured by the user, let's just improve the default first UID to be the same as idsPerPod. This makes the default first UID work out of the box if the user just wants to tune that. This also simplifies testing, as we don't need to create a system user and /etc/subuid and /etc/subgid files to test the idsPerPod setting. Signed-off-by: Rodrigo Campos --- pkg/kubelet/kubelet_getters.go | 4 ++-- pkg/kubelet/kubelet_pods.go | 12 ++++++++---- pkg/kubelet/userns/types.go | 2 +- pkg/kubelet/userns/userns_manager.go | 9 ++++----- pkg/kubelet/userns/userns_manager_test.go | 2 +- 5 files changed, 16 insertions(+), 13 deletions(-) diff --git a/pkg/kubelet/kubelet_getters.go b/pkg/kubelet/kubelet_getters.go index 7858fb443d0..f2286dc91ed 100644 --- a/pkg/kubelet/kubelet_getters.go +++ b/pkg/kubelet/kubelet_getters.go @@ -133,8 +133,8 @@ func (kl *Kubelet) HandlerSupportsUserNamespaces(rtHandler string) (bool, error) } // GetKubeletMappings gets the additional IDs allocated for the Kubelet. -func (kl *Kubelet) GetKubeletMappings() (uint32, uint32, error) { - return kl.getKubeletMappings() +func (kl *Kubelet) GetKubeletMappings(idsPerPod uint32) (uint32, uint32, error) { + return kl.getKubeletMappings(idsPerPod) } func (kl *Kubelet) GetMaxPods() int { diff --git a/pkg/kubelet/kubelet_pods.go b/pkg/kubelet/kubelet_pods.go index e72afc183a8..af1d205eebb 100644 --- a/pkg/kubelet/kubelet_pods.go +++ b/pkg/kubelet/kubelet_pods.go @@ -124,11 +124,15 @@ func parseGetSubIdsOutput(input string) (uint32, uint32, error) { // If subordinate user or group ID ranges are specified for the kubelet user and the getsubids tool // is installed, then the single mapping specified both for user and group IDs will be used. // If the tool is not installed, or there are no IDs configured, the default mapping is returned. -// The default mapping includes the entire IDs range except IDs below 65536. -func (kl *Kubelet) getKubeletMappings() (uint32, uint32, error) { +// The default mapping includes the entire IDs range except IDs below idsPerPod. +func (kl *Kubelet) getKubeletMappings(idsPerPod uint32) (uint32, uint32, error) { // default mappings to return if there is no specific configuration - const defaultFirstID = 1 << 16 - const defaultLen = 1<<32 - defaultFirstID + defaultFirstID := idsPerPod + // We cast defaultFirstID to 64 bits, as otherwise any operation (including subtraction) + // fires the overflow detection (go is not smart enough to realize that if we subtract a + // non-negative number, it fits in 32 bits). + // Then we cast it back to 32 bits, as this what the function returns. + defaultLen := uint32((1 << 32) - uint64(defaultFirstID)) if !utilfeature.DefaultFeatureGate.Enabled(features.UserNamespacesSupport) { return defaultFirstID, defaultLen, nil diff --git a/pkg/kubelet/userns/types.go b/pkg/kubelet/userns/types.go index a0422d0042c..e494ca212e6 100644 --- a/pkg/kubelet/userns/types.go +++ b/pkg/kubelet/userns/types.go @@ -24,6 +24,6 @@ type userNsPodsManager interface { HandlerSupportsUserNamespaces(runtimeHandler string) (bool, error) GetPodDir(podUID types.UID) string ListPodsFromDisk() ([]types.UID, error) - GetKubeletMappings() (uint32, uint32, error) + GetKubeletMappings(idsPerPod uint32) (uint32, uint32, error) GetMaxPods() int } diff --git a/pkg/kubelet/userns/userns_manager.go b/pkg/kubelet/userns/userns_manager.go index a0e00976f9b..ff46be225e9 100644 --- a/pkg/kubelet/userns/userns_manager.go +++ b/pkg/kubelet/userns/userns_manager.go @@ -131,17 +131,16 @@ func (m *UsernsManager) readMappingsFromFile(pod types.UID) ([]byte, error) { } func MakeUserNsManager(logger klog.Logger, kl userNsPodsManager, idsPerPod *int64) (*UsernsManager, error) { - kubeletMappingID, kubeletMappingLen, err := kl.GetKubeletMappings() - if err != nil { - return nil, fmt.Errorf("kubelet mappings: %w", err) - } - userNsLength := uint32(kubeletconfig.DefaultKubeletUserNamespacesIDsPerPod) if idsPerPod != nil { // The value is already validated as part of kubelet config validation, so we can safely // cast it. userNsLength = uint32(*idsPerPod) } + kubeletMappingID, kubeletMappingLen, err := kl.GetKubeletMappings(userNsLength) + if err != nil { + return nil, fmt.Errorf("kubelet mappings: %w", err) + } if userNsLength%userNsUnitLength != 0 { return nil, fmt.Errorf("kubelet user namespace length %v is not a multiple of %d", userNsLength, userNsUnitLength) diff --git a/pkg/kubelet/userns/userns_manager_test.go b/pkg/kubelet/userns/userns_manager_test.go index 6f4d9a9c677..90e56418c8d 100644 --- a/pkg/kubelet/userns/userns_manager_test.go +++ b/pkg/kubelet/userns/userns_manager_test.go @@ -78,7 +78,7 @@ func (m *testUserNsPodsManager) HandlerSupportsUserNamespaces(runtimeHandler str return m.userns, nil } -func (m *testUserNsPodsManager) GetKubeletMappings() (uint32, uint32, error) { +func (m *testUserNsPodsManager) GetKubeletMappings(idsPerPod uint32) (uint32, uint32, error) { if m.mappingFirstID != 0 { return m.mappingFirstID, m.mappingLen, nil }