From 87a057d4170f762c4cd2c96fefb766e3d35b7b0b Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Tue, 9 Jan 2024 17:43:33 +0100 Subject: [PATCH 1/7] KEP-127: kubelet: honor kubelet user mappings allow to specify what IDs must be used by the kubelet to create user namespaces. If no additional UIDs/GIDs are not allocated to the "kubelet" user, then the kubelet assumes it can use any ID on the system. Signed-off-by: Giuseppe Scrivano --- pkg/kubelet/kubelet_getters.go | 5 ++ pkg/kubelet/kubelet_pods.go | 86 +++++++++++++++++++++++ pkg/kubelet/kubelet_pods_test.go | 74 +++++++++++++++++++ pkg/kubelet/userns/userns_manager.go | 53 ++++++++++---- pkg/kubelet/userns/userns_manager_test.go | 14 ++++ 5 files changed, 219 insertions(+), 13 deletions(-) diff --git a/pkg/kubelet/kubelet_getters.go b/pkg/kubelet/kubelet_getters.go index 4ef51b9791f..e1d00e3ed12 100644 --- a/pkg/kubelet/kubelet_getters.go +++ b/pkg/kubelet/kubelet_getters.go @@ -123,6 +123,11 @@ func (kl *Kubelet) HandlerSupportsUserNamespaces(rtHandler string) (bool, error) return h.SupportsUserNamespaces, nil } +// GetKubeletMappings gets the additional IDs allocated for the Kubelet. +func (kl *Kubelet) GetKubeletMappings() (uint32, uint32, error) { + return kl.getKubeletMappings() +} + // getPodDir returns the full path to the per-pod directory for the pod with // the given UID. func (kl *Kubelet) getPodDir(podUID types.UID) string { diff --git a/pkg/kubelet/kubelet_pods.go b/pkg/kubelet/kubelet_pods.go index fd6dedca2bb..7ad7a68808a 100644 --- a/pkg/kubelet/kubelet_pods.go +++ b/pkg/kubelet/kubelet_pods.go @@ -19,14 +19,18 @@ package kubelet import ( "bytes" "context" + goerrors "errors" "fmt" "io" "net/http" "net/url" "os" + "os/exec" + "os/user" "path/filepath" "runtime" "sort" + "strconv" "strings" "github.com/google/go-cmp/cmp" @@ -76,8 +80,90 @@ const ( const ( PodInitializing = "PodInitializing" ContainerCreating = "ContainerCreating" + + kubeletUser = "kubelet" ) +// parseGetSubIdsOutput parses the output from the `getsubids` tool, which is used to query subordinate user or group ID ranges for +// a given user or group. getsubids produces a line for each mapping configured. +// Here we expect that there is a single mapping, and the same values are used for the subordinate user and group ID ranges. +// The output is something like: +// $ getsubids kubelet +// 0: kubelet 65536 2147483648 +// $ getsubids -g kubelet +// 0: kubelet 65536 2147483648 +func parseGetSubIdsOutput(input string) (uint32, uint32, error) { + lines := strings.Split(strings.Trim(input, "\n"), "\n") + if len(lines) != 1 { + return 0, 0, fmt.Errorf("error parsing line %q: it must contain only one line", input) + } + + parts := strings.Fields(lines[0]) + if len(parts) != 4 { + return 0, 0, fmt.Errorf("invalid line %q", input) + } + + // Parsing the numbers + num1, err := strconv.ParseUint(parts[2], 10, 32) + if err != nil { + return 0, 0, fmt.Errorf("error parsing line %q: %w", input, err) + } + + num2, err := strconv.ParseUint(parts[3], 10, 32) + if err != nil { + return 0, 0, fmt.Errorf("error parsing line %q: %w", input, err) + } + + return uint32(num1), uint32(num2), nil +} + +// getKubeletMappings returns the range of IDs that can be used to configure user namespaces. +// 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) { + // default mappings to return if there is no specific configuration + const defaultFirstID = 1 << 16 + const defaultLen = 1<<32 - defaultFirstID + + if !utilfeature.DefaultFeatureGate.Enabled(features.UserNamespacesSupport) { + return defaultFirstID, defaultLen, nil + } + + _, err := user.Lookup(kubeletUser) + if err != nil { + var unknownUserErr user.UnknownUserError + if goerrors.As(err, &unknownUserErr) { + // if the user is not found, we assume that the user is not configured + return defaultFirstID, defaultLen, nil + } + return 0, 0, err + } + + execName := "getsubids" + cmd, err := exec.LookPath(execName) + if err != nil { + if os.IsNotExist(err) { + klog.V(2).InfoS("Could not find executable, default mappings will be used for the user namespaces", "executable", execName, "err", err) + return defaultFirstID, defaultLen, nil + } + return 0, 0, err + } + outUids, err := exec.Command(cmd, kubeletUser).Output() + if err != nil { + return 0, 0, fmt.Errorf("error retrieving additional ids for user %q", kubeletUser) + } + outGids, err := exec.Command(cmd, "-g", kubeletUser).Output() + if err != nil { + return 0, 0, fmt.Errorf("error retrieving additional gids for user %q", kubeletUser) + } + if string(outUids) != string(outGids) { + return 0, 0, fmt.Errorf("mismatched subuids and subgids for user %q", kubeletUser) + } + return parseGetSubIdsOutput(string(outUids)) +} + // Get a list of pods that have data directories. func (kl *Kubelet) listPodsFromDisk() ([]types.UID, error) { podInfos, err := os.ReadDir(kl.getPodsDir()) diff --git a/pkg/kubelet/kubelet_pods_test.go b/pkg/kubelet/kubelet_pods_test.go index 90c42dde60f..ae6710c50c6 100644 --- a/pkg/kubelet/kubelet_pods_test.go +++ b/pkg/kubelet/kubelet_pods_test.go @@ -6013,3 +6013,77 @@ func TestGetNonExistentImagePullSecret(t *testing.T) { event := <-fakeRecorder.Events assert.Equal(t, event, expectedEvent) } + +func TestParseGetSubIdsOutput(t *testing.T) { + tests := []struct { + name string + input string + wantFirstID uint32 + wantRangeLen uint32 + wantErr bool + }{ + { + name: "valid", + input: "0: kubelet 65536 2147483648", + wantFirstID: 65536, + wantRangeLen: 2147483648, + }, + { + name: "multiple lines", + input: "0: kubelet 1 2\n1: kubelet 3 4\n", + wantErr: true, + }, + { + name: "wrong format", + input: "0: kubelet 65536", + wantErr: true, + }, + { + name: "non numeric 1", + input: "0: kubelet Foo 65536", + wantErr: true, + }, + { + name: "non numeric 2", + input: "0: kubelet 0 Bar", + wantErr: true, + }, + { + name: "overflow 1", + input: "0: kubelet 4294967296 2147483648", + wantErr: true, + }, + { + name: "overflow 2", + input: "0: kubelet 65536 4294967296", + wantErr: true, + }, + { + name: "negative value 1", + input: "0: kubelet -1 2147483648", + wantErr: true, + }, + { + name: "negative value 2", + input: "0: kubelet 65536 -1", + wantErr: true, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + gotFirstID, gotRangeLen, err := parseGetSubIdsOutput(tc.input) + if tc.wantErr { + if err == nil { + t.Errorf("%s: expected error, got nil", tc.name) + } + } else { + if err != nil { + t.Errorf("%s: unexpected error: %v", tc.name, err) + } + if gotFirstID != tc.wantFirstID || gotRangeLen != tc.wantRangeLen { + t.Errorf("%s: got (%d, %d), want (%d, %d)", tc.name, gotFirstID, gotRangeLen, tc.wantFirstID, tc.wantRangeLen) + } + } + }) + } +} diff --git a/pkg/kubelet/userns/userns_manager.go b/pkg/kubelet/userns/userns_manager.go index 603dd053906..56a9a8e8ade 100644 --- a/pkg/kubelet/userns/userns_manager.go +++ b/pkg/kubelet/userns/userns_manager.go @@ -19,7 +19,6 @@ package userns import ( "encoding/json" "fmt" - "math" "os" "path/filepath" "sync" @@ -52,6 +51,7 @@ type userNsPodsManager interface { HandlerSupportsUserNamespaces(runtimeHandler string) (bool, error) GetPodDir(podUID types.UID) string ListPodsFromDisk() ([]types.UID, error) + GetKubeletMappings() (uint32, uint32, error) } type UsernsManager struct { @@ -59,7 +59,11 @@ type UsernsManager struct { usedBy map[types.UID]uint32 // Map pod.UID to range used removed int numAllocated int - kl userNsPodsManager + + off int + len int + + kl userNsPodsManager // This protects all members except for kl.anager lock sync.Mutex } @@ -130,16 +134,33 @@ func (m *UsernsManager) readMappingsFromFile(pod types.UID) ([]byte, error) { } func MakeUserNsManager(kl userNsPodsManager) (*UsernsManager, error) { + kubeletMappingID, kubeletMappingLen, err := kl.GetKubeletMappings() + if err != nil { + return nil, err + } + + if kubeletMappingID%userNsLength != 0 { + return nil, fmt.Errorf("kubelet user assigned ID %v is not a multiple of %v", kubeletMappingID, userNsLength) + } + if kubeletMappingID < userNsLength { + // We don't allow to map 0, as security is circumvented. + return nil, fmt.Errorf("kubelet user assigned ID %v must be greater or equal to %v", kubeletMappingID, userNsLength) + } + if kubeletMappingLen%userNsLength != 0 { + return nil, fmt.Errorf("kubelet user assigned IDs length %v is not a multiple of %v", kubeletMappingLen, userNsLength) + } + if kubeletMappingLen/userNsLength < maxPods { + return nil, fmt.Errorf("kubelet user assigned IDs are not enough to support %v pods", maxPods) + } + off := int(kubeletMappingID / userNsLength) + len := int(kubeletMappingLen / userNsLength) + m := UsernsManager{ - // Create a bitArray for all the UID space (2^32). - // As a by product of that, no index param to bitArray can be out of bounds (index is uint32). - used: allocator.NewAllocationMap((math.MaxUint32+1)/userNsLength, "user namespaces"), + used: allocator.NewAllocationMap(len, "user namespaces"), usedBy: make(map[types.UID]uint32), kl: kl, - } - // First block is reserved for the host. - if _, err := m.used.Allocate(0); err != nil { - return nil, err + off: off, + len: len, } // do not bother reading the list of pods if user namespaces are not enabled. @@ -184,7 +205,10 @@ func (m *UsernsManager) recordPodMappings(pod types.UID) error { // isSet checks if the specified index is already set. func (m *UsernsManager) isSet(v uint32) bool { - index := int(v / userNsLength) + index := int(v/userNsLength) - m.off + if index < 0 || index >= m.len { + return true + } return m.used.Has(index) } @@ -212,7 +236,7 @@ func (m *UsernsManager) allocateOne(pod types.UID) (firstID uint32, length uint3 klog.V(5).InfoS("new pod user namespace allocation", "podUID", pod) - firstID = uint32(firstZero * userNsLength) + firstID = uint32((firstZero + m.off) * userNsLength) m.usedBy[pod] = firstID return firstID, userNsLength, nil } @@ -229,7 +253,10 @@ func (m *UsernsManager) record(pod types.UID, from, length uint32) (err error) { if found && prevFrom != from { return fmt.Errorf("different user namespace range already used by pod %q", pod) } - index := int(from / userNsLength) + index := int(from/userNsLength) - m.off + if index < 0 || index >= m.len { + return fmt.Errorf("id %v is out of range", from) + } // if the pod wasn't found then verify the range is free. if !found && m.used.Has(index) { return fmt.Errorf("range picked for pod %q already taken", pod) @@ -304,7 +331,7 @@ func (m *UsernsManager) releaseWithLock(pod types.UID) { m.usedBy = n m.removed = 0 } - m.used.Release(int(v / userNsLength)) + _ = m.used.Release(int(v/userNsLength) - m.off) } func (m *UsernsManager) parseUserNsFileAndRecord(pod types.UID, content []byte) (userNs userNamespace, err error) { diff --git a/pkg/kubelet/userns/userns_manager_test.go b/pkg/kubelet/userns/userns_manager_test.go index 8c498955b78..626f743b46d 100644 --- a/pkg/kubelet/userns/userns_manager_test.go +++ b/pkg/kubelet/userns/userns_manager_test.go @@ -34,6 +34,13 @@ import ( kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" ) +const ( + // skip the first block + minimumMappingUID = userNsLength + // allocate enough space for 2000 user namespaces + mappingLen = userNsLength * 2000 +) + type testUserNsPodsManager struct { podDir string podList []types.UID @@ -61,6 +68,10 @@ func (m *testUserNsPodsManager) HandlerSupportsUserNamespaces(runtimeHandler str return m.userns, nil } +func (m *testUserNsPodsManager) GetKubeletMappings() (uint32, uint32, error) { + return minimumMappingUID, mappingLen, nil +} + func TestUserNsManagerAllocate(t *testing.T) { defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, pkgfeatures.UserNamespacesSupport, true)() @@ -97,6 +108,9 @@ func TestUserNsManagerAllocate(t *testing.T) { allocated, length, err = m.allocateOne(types.UID(fmt.Sprintf("%d", i))) assert.Equal(t, userNsLength, int(length), "length is not the expected. iter: %v", i) assert.NoError(t, err) + assert.True(t, allocated >= minimumMappingUID) + // The last ID of the userns range (allocated+userNsLength) should be within bounds. + assert.True(t, allocated <= minimumMappingUID+mappingLen-userNsLength) allocs = append(allocs, allocated) } for i, v := range allocs { From 4c81e5c9dc6796e5f288bf19ac7ae671a2286e11 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Fri, 1 Mar 2024 16:17:04 +0100 Subject: [PATCH 2/7] features: promote UserNamespacesSupport to beta Signed-off-by: Giuseppe Scrivano --- pkg/features/kube_features.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 44850082292..13c9f0eb9fa 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -822,6 +822,7 @@ const ( // owner: @rata, @giuseppe // kep: https://kep.k8s.io/127 // alpha: v1.25 + // beta: v1.30 // // Enables user namespace support for stateless pods. UserNamespacesSupport featuregate.Feature = "UserNamespacesSupport" @@ -1153,7 +1154,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS VolumeCapacityPriority: {Default: false, PreRelease: featuregate.Alpha}, - UserNamespacesSupport: {Default: false, PreRelease: featuregate.Alpha}, + UserNamespacesSupport: {Default: false, PreRelease: featuregate.Beta}, WinDSR: {Default: false, PreRelease: featuregate.Alpha}, From 4180284dc9736f8998da28b8645f9ae20a413b1c Mon Sep 17 00:00:00 2001 From: Rodrigo Campos Date: Fri, 1 Mar 2024 13:04:28 -0300 Subject: [PATCH 3/7] pkg/kubelet/userns: Remove outdated test When we were alocating the whole UID space, the first range was reserved to the host. Now we don't allocate the whole UID space, but just the range configured, so the first range doesn't point to [0;65535] anymore, so no need to test it is always set. Signed-off-by: Rodrigo Campos --- pkg/kubelet/userns/userns_manager_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/kubelet/userns/userns_manager_test.go b/pkg/kubelet/userns/userns_manager_test.go index 626f743b46d..56cb17bda60 100644 --- a/pkg/kubelet/userns/userns_manager_test.go +++ b/pkg/kubelet/userns/userns_manager_test.go @@ -79,8 +79,6 @@ func TestUserNsManagerAllocate(t *testing.T) { m, err := MakeUserNsManager(testUserNsPodsManager) require.NoError(t, err) - assert.Equal(t, true, m.isSet(0*65536), "m.isSet(0) should be true") - allocated, length, err := m.allocateOne("one") assert.NoError(t, err) assert.Equal(t, userNsLength, int(length), "m.isSet(%d).length=%v", allocated, length) From 39c68156766c9d4cf22d11e3e64feef385b08c69 Mon Sep 17 00:00:00 2001 From: Rodrigo Campos Date: Fri, 1 Mar 2024 13:16:17 -0300 Subject: [PATCH 4/7] kubelet/userns: Remove alpha maxPods limitation Signed-off-by: Rodrigo Campos --- pkg/kubelet/userns/userns_manager.go | 27 ++--------------- pkg/kubelet/userns/userns_manager_test.go | 36 ----------------------- 2 files changed, 3 insertions(+), 60 deletions(-) diff --git a/pkg/kubelet/userns/userns_manager.go b/pkg/kubelet/userns/userns_manager.go index 56a9a8e8ade..2024c071715 100644 --- a/pkg/kubelet/userns/userns_manager.go +++ b/pkg/kubelet/userns/userns_manager.go @@ -55,10 +55,9 @@ type userNsPodsManager interface { } type UsernsManager struct { - used *allocator.AllocationBitmap - usedBy map[types.UID]uint32 // Map pod.UID to range used - removed int - numAllocated int + used *allocator.AllocationBitmap + usedBy map[types.UID]uint32 // Map pod.UID to range used + removed int off int len int @@ -216,16 +215,6 @@ func (m *UsernsManager) isSet(v uint32) bool { // The first return value is the first ID in the user namespace, the second returns // the length for the user namespace range. func (m *UsernsManager) allocateOne(pod types.UID) (firstID uint32, length uint32, err error) { - if m.numAllocated >= maxPods { - return 0, 0, fmt.Errorf("limit on count of pods with user namespaces exceeded (limit is %v, current pods with userns: %v)", maxPods, m.numAllocated) - } - m.numAllocated++ - defer func() { - if err != nil { - m.numAllocated-- - } - }() - firstZero, found, err := m.used.AllocateNext() if err != nil { return 0, 0, err @@ -265,15 +254,6 @@ func (m *UsernsManager) record(pod types.UID, from, length uint32) (err error) { if found && prevFrom == from { return nil } - if m.numAllocated >= maxPods { - return fmt.Errorf("limit on count of pods with user namespaces exceeded (limit is %v, current pods with userns: %v)", maxPods, m.numAllocated) - } - m.numAllocated++ - defer func() { - if err != nil { - m.numAllocated-- - } - }() klog.V(5).InfoS("new pod user namespace allocation", "podUID", pod) @@ -318,7 +298,6 @@ func (m *UsernsManager) releaseWithLock(pod types.UID) { delete(m.usedBy, pod) klog.V(5).InfoS("releasing pod user namespace allocation", "podUID", pod) - m.numAllocated-- m.removed++ _ = os.Remove(filepath.Join(m.kl.GetPodDir(pod), mappingsFile)) diff --git a/pkg/kubelet/userns/userns_manager_test.go b/pkg/kubelet/userns/userns_manager_test.go index 56cb17bda60..db325a2c1e5 100644 --- a/pkg/kubelet/userns/userns_manager_test.go +++ b/pkg/kubelet/userns/userns_manager_test.go @@ -378,42 +378,6 @@ func TestCleanupOrphanedPodUsernsAllocations(t *testing.T) { } } -func TestAllocateMaxPods(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, pkgfeatures.UserNamespacesSupport, true)() - - testUserNsPodsManager := &testUserNsPodsManager{} - m, err := MakeUserNsManager(testUserNsPodsManager) - require.NoError(t, err) - - // The first maxPods allocations should succeed. - for i := 0; i < maxPods; i++ { - _, _, err = m.allocateOne(types.UID(fmt.Sprintf("%d", i))) - require.NoError(t, err) - } - - // The next allocation should fail, hitting maxPods. - _, _, err = m.allocateOne(types.UID(fmt.Sprintf("%d", maxPods+1))) - assert.Error(t, err) -} - -func TestRecordMaxPods(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, pkgfeatures.UserNamespacesSupport, true)() - - testUserNsPodsManager := &testUserNsPodsManager{} - m, err := MakeUserNsManager(testUserNsPodsManager) - require.NoError(t, err) - - // The first maxPods allocations should succeed. - for i := 0; i < maxPods; i++ { - err = m.record(types.UID(fmt.Sprintf("%d", i)), uint32((i+1)*65536), 65536) - require.NoError(t, err) - } - - // The next allocation should fail, hitting maxPods. - err = m.record(types.UID(fmt.Sprintf("%d", maxPods+1)), uint32((maxPods+1)*65536), 65536) - assert.Error(t, err) -} - type failingUserNsPodsManager struct { testUserNsPodsManager } From 0b69c2bc81d32a37b0cc136ae8540dfe2fa14000 Mon Sep 17 00:00:00 2001 From: Rodrigo Campos Date: Fri, 1 Mar 2024 13:26:57 -0300 Subject: [PATCH 5/7] kubelet/userns: Use kubelet maxPods We don't have the alpha limitation anymore, let's just use the kubelet maxPods instead of our hardcoded 1024 max. Signed-off-by: Rodrigo Campos --- pkg/kubelet/kubelet_getters.go | 4 ++++ pkg/kubelet/userns/userns_manager.go | 9 +++------ pkg/kubelet/userns/userns_manager_test.go | 12 +++++++++++- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/pkg/kubelet/kubelet_getters.go b/pkg/kubelet/kubelet_getters.go index e1d00e3ed12..39ff7fe9f2d 100644 --- a/pkg/kubelet/kubelet_getters.go +++ b/pkg/kubelet/kubelet_getters.go @@ -128,6 +128,10 @@ func (kl *Kubelet) GetKubeletMappings() (uint32, uint32, error) { return kl.getKubeletMappings() } +func (kl *Kubelet) GetMaxPods() int { + return kl.maxPods +} + // getPodDir returns the full path to the per-pod directory for the pod with // the given UID. func (kl *Kubelet) getPodDir(podUID types.UID) string { diff --git a/pkg/kubelet/userns/userns_manager.go b/pkg/kubelet/userns/userns_manager.go index 2024c071715..c431e0511af 100644 --- a/pkg/kubelet/userns/userns_manager.go +++ b/pkg/kubelet/userns/userns_manager.go @@ -39,10 +39,6 @@ import ( // length for the user namespace to create (65536). const userNsLength = (1 << 16) -// Limit the total number of pods using userns in this node to this value. -// This is an alpha limitation that will probably be lifted later. -const maxPods = 1024 - // Create a new map when we removed enough pods to avoid memory leaks // since Go maps never free memory. const mapReInitializeThreshold = 1000 @@ -52,6 +48,7 @@ type userNsPodsManager interface { GetPodDir(podUID types.UID) string ListPodsFromDisk() ([]types.UID, error) GetKubeletMappings() (uint32, uint32, error) + GetMaxPods() int } type UsernsManager struct { @@ -148,8 +145,8 @@ func MakeUserNsManager(kl userNsPodsManager) (*UsernsManager, error) { if kubeletMappingLen%userNsLength != 0 { return nil, fmt.Errorf("kubelet user assigned IDs length %v is not a multiple of %v", kubeletMappingLen, userNsLength) } - if kubeletMappingLen/userNsLength < maxPods { - return nil, fmt.Errorf("kubelet user assigned IDs are not enough to support %v pods", maxPods) + if kubeletMappingLen/userNsLength < uint32(kl.GetMaxPods()) { + return nil, fmt.Errorf("kubelet user assigned IDs are not enough to support %v pods", kl.GetMaxPods()) } off := int(kubeletMappingID / userNsLength) len := int(kubeletMappingLen / userNsLength) diff --git a/pkg/kubelet/userns/userns_manager_test.go b/pkg/kubelet/userns/userns_manager_test.go index db325a2c1e5..b2f6fc1eabc 100644 --- a/pkg/kubelet/userns/userns_manager_test.go +++ b/pkg/kubelet/userns/userns_manager_test.go @@ -38,13 +38,15 @@ const ( // skip the first block minimumMappingUID = userNsLength // allocate enough space for 2000 user namespaces - mappingLen = userNsLength * 2000 + mappingLen = userNsLength * 2000 + testMaxPods = 110 ) type testUserNsPodsManager struct { podDir string podList []types.UID userns bool + maxPods int } func (m *testUserNsPodsManager) GetPodDir(podUID types.UID) string { @@ -72,6 +74,14 @@ func (m *testUserNsPodsManager) GetKubeletMappings() (uint32, uint32, error) { return minimumMappingUID, mappingLen, nil } +func (m *testUserNsPodsManager) GetMaxPods() int { + if m.maxPods != 0 { + return m.maxPods + } + + return testMaxPods +} + func TestUserNsManagerAllocate(t *testing.T) { defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, pkgfeatures.UserNamespacesSupport, true)() From 4bb508dd3036dcc0fb8418e2617d9a2b90594f98 Mon Sep 17 00:00:00 2001 From: Rodrigo Campos Date: Fri, 1 Mar 2024 16:21:55 -0300 Subject: [PATCH 6/7] kubelet/userns: Add unit test Signed-off-by: Rodrigo Campos --- pkg/kubelet/userns/userns_manager_test.go | 89 ++++++++++++++++++++++- 1 file changed, 85 insertions(+), 4 deletions(-) diff --git a/pkg/kubelet/userns/userns_manager_test.go b/pkg/kubelet/userns/userns_manager_test.go index b2f6fc1eabc..251eee94bb1 100644 --- a/pkg/kubelet/userns/userns_manager_test.go +++ b/pkg/kubelet/userns/userns_manager_test.go @@ -43,10 +43,12 @@ const ( ) type testUserNsPodsManager struct { - podDir string - podList []types.UID - userns bool - maxPods int + podDir string + podList []types.UID + userns bool + maxPods int + mappingFirstID uint32 + mappingLen uint32 } func (m *testUserNsPodsManager) GetPodDir(podUID types.UID) string { @@ -71,6 +73,9 @@ func (m *testUserNsPodsManager) HandlerSupportsUserNamespaces(runtimeHandler str } func (m *testUserNsPodsManager) GetKubeletMappings() (uint32, uint32, error) { + if m.mappingFirstID != 0 { + return m.mappingFirstID, m.mappingLen, nil + } return minimumMappingUID, mappingLen, nil } @@ -133,6 +138,60 @@ func TestUserNsManagerAllocate(t *testing.T) { } } +func TestMakeUserNsManager(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, pkgfeatures.UserNamespacesSupport, true)() + + cases := []struct { + name string + mappingFirstID uint32 + mappingLen uint32 + maxPods int + success bool + }{ + { + name: "default", + success: true, + }, + { + name: "firstID not multiple", + mappingFirstID: 65536 + 1, + }, + { + name: "firstID is less than 65535", + mappingFirstID: 1, + }, + { + name: "mappingLen not multiple", + mappingFirstID: 65536, + mappingLen: 65536 + 1, + }, + { + name: "range can't fit maxPods", + mappingFirstID: 65536, + mappingLen: 65536, + maxPods: 2, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + testUserNsPodsManager := &testUserNsPodsManager{ + podDir: t.TempDir(), + mappingFirstID: tc.mappingFirstID, + mappingLen: tc.mappingLen, + maxPods: tc.maxPods, + } + _, err := MakeUserNsManager(testUserNsPodsManager) + + if tc.success { + assert.NoError(t, err) + } else { + assert.Error(t, err) + } + }) + } +} + func TestUserNsManagerParseUserNsFile(t *testing.T) { defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, pkgfeatures.UserNamespacesSupport, true)() @@ -404,3 +463,25 @@ func TestMakeUserNsManagerFailsListPod(t *testing.T) { assert.Error(t, err) assert.ErrorContains(t, err, "read pods from disk") } + +func TestRecordBounds(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, pkgfeatures.UserNamespacesSupport, true)() + + // Allow exactly for 1 pod + testUserNsPodsManager := &testUserNsPodsManager{ + mappingFirstID: 65536, + mappingLen: 65536, + maxPods: 1, + } + m, err := MakeUserNsManager(testUserNsPodsManager) + require.NoError(t, err) + + // The first pod allocation should succeed. + err = m.record(types.UID(fmt.Sprintf("%d", 0)), 65536, 65536) + require.NoError(t, err) + + // The next allocation should fail, as there is no space left. + err = m.record(types.UID(fmt.Sprintf("%d", 2)), uint32(2*65536), 65536) + assert.Error(t, err) + assert.ErrorContains(t, err, "out of range") +} From 6174f199dfb5753279c7209e3b2840a142b19fd4 Mon Sep 17 00:00:00 2001 From: Rodrigo Campos Date: Fri, 16 Feb 2024 18:03:59 -0300 Subject: [PATCH 7/7] kublet/userns: Add test switching feature gate off/on Signed-off-by: Rodrigo Campos --- .../userns/userns_manager_switch_test.go | 137 ++++++++++++++++++ 1 file changed, 137 insertions(+) create mode 100644 pkg/kubelet/userns/userns_manager_switch_test.go diff --git a/pkg/kubelet/userns/userns_manager_switch_test.go b/pkg/kubelet/userns/userns_manager_switch_test.go new file mode 100644 index 00000000000..e5aff5c0578 --- /dev/null +++ b/pkg/kubelet/userns/userns_manager_switch_test.go @@ -0,0 +1,137 @@ +/* +Copyright 2024 The Kubernetes Authors. + +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 userns + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + utilfeature "k8s.io/apiserver/pkg/util/feature" + featuregatetesting "k8s.io/component-base/featuregate/testing" + pkgfeatures "k8s.io/kubernetes/pkg/features" +) + +func TestMakeUserNsManagerSwitch(t *testing.T) { + // Create the manager with the feature gate enabled, to record some pods on disk. + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, pkgfeatures.UserNamespacesSupport, true)() + + pods := []types.UID{"pod-1", "pod-2"} + + testUserNsPodsManager := &testUserNsPodsManager{ + podDir: t.TempDir(), + // List the same pods we will record, so the second time we create the userns + // manager, it will find these pods on disk with userns data. + podList: pods, + } + m, err := MakeUserNsManager(testUserNsPodsManager) + require.NoError(t, err) + + // Record the pods on disk. + for _, podUID := range pods { + pod := v1.Pod{ObjectMeta: metav1.ObjectMeta{UID: podUID}} + _, err := m.GetOrCreateUserNamespaceMappings(&pod, "") + require.NoError(t, err, "failed to record userns range for pod %v", podUID) + } + + // Test re-init works when the feature gate is disabled and there were some + // pods written on disk. + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, pkgfeatures.UserNamespacesSupport, false)() + m2, err := MakeUserNsManager(testUserNsPodsManager) + require.NoError(t, err) + + // The feature gate is off, no pods should be allocated. + for _, pod := range pods { + ok := m2.podAllocated(pod) + assert.False(t, ok, "pod %q should not be allocated", pod) + } +} + +func TestGetOrCreateUserNamespaceMappingsSwitch(t *testing.T) { + // Enable the feature gate to create some pods on disk. + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, pkgfeatures.UserNamespacesSupport, true)() + + pods := []types.UID{"pod-1", "pod-2"} + + testUserNsPodsManager := &testUserNsPodsManager{ + podDir: t.TempDir(), + // List the same pods we will record, so the second time we create the userns + // manager, it will find these pods on disk with userns data. + podList: pods, + } + m, err := MakeUserNsManager(testUserNsPodsManager) + require.NoError(t, err) + + // Record the pods on disk. + for _, podUID := range pods { + pod := v1.Pod{ObjectMeta: metav1.ObjectMeta{UID: podUID}} + _, err := m.GetOrCreateUserNamespaceMappings(&pod, "") + require.NoError(t, err, "failed to record userns range for pod %v", podUID) + } + + // Test no-op when the feature gate is disabled and there were some + // pods registered on disk. + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, pkgfeatures.UserNamespacesSupport, false)() + // Create a new manager with the feature gate off and verify the userns range is nil. + m2, err := MakeUserNsManager(testUserNsPodsManager) + require.NoError(t, err) + + for _, podUID := range pods { + pod := v1.Pod{ObjectMeta: metav1.ObjectMeta{UID: podUID}} + userns, err := m2.GetOrCreateUserNamespaceMappings(&pod, "") + + assert.NoError(t, err, "failed to record userns range for pod %v", podUID) + assert.Nil(t, userns, "userns range should be nil for pod %v", podUID) + } +} + +func TestCleanupOrphanedPodUsernsAllocationsSwitch(t *testing.T) { + // Enable the feature gate to create some pods on disk. + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, pkgfeatures.UserNamespacesSupport, true)() + + listPods := []types.UID{"pod-1", "pod-2"} + pods := []types.UID{"pod-3", "pod-4"} + testUserNsPodsManager := &testUserNsPodsManager{ + podDir: t.TempDir(), + podList: listPods, + } + + m, err := MakeUserNsManager(testUserNsPodsManager) + require.NoError(t, err) + + // Record the pods on disk. + for _, podUID := range pods { + pod := v1.Pod{ObjectMeta: metav1.ObjectMeta{UID: podUID}} + _, err := m.GetOrCreateUserNamespaceMappings(&pod, "") + require.NoError(t, err, "failed to record userns range for pod %v", podUID) + } + + // Test cleanup works when the feature gate is disabled and there were some + // pods registered. + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, pkgfeatures.UserNamespacesSupport, false)() + err = m.CleanupOrphanedPodUsernsAllocations(nil, nil) + require.NoError(t, err) + + // The feature gate is off, no pods should be allocated. + for _, pod := range append(listPods, pods...) { + ok := m.podAllocated(pod) + assert.False(t, ok, "pod %q should not be allocated", pod) + } +}