From 556d713a4ad3a1f34d9eac8590468f33f3ec0cb2 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Thu, 15 Jun 2023 17:24:57 +0200 Subject: [PATCH 1/2] apis: drop check for volumes with user namespaces The second phase of user namespaces support was related to supporting only stateless pods. Since the changes were accepted for the KEP, now the scope is extended to support stateful pods as well. Remove the check that blocks creating PODs with volumes when using user namespaces. Signed-off-by: Giuseppe Scrivano --- pkg/apis/core/validation/validation.go | 19 ------------------- pkg/apis/core/validation/validation_test.go | 5 ++--- 2 files changed, 2 insertions(+), 22 deletions(-) diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 6502d9c210a..96f6be0839c 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -3256,25 +3256,6 @@ func validateHostUsers(spec *core.PodSpec, fldPath *field.Path) field.ErrorList return allErrs } - // For now only these volumes are supported: - // - configmap - // - secret - // - downwardAPI - // - emptyDir - // - projected - // So reject anything else. - for i, vol := range spec.Volumes { - switch { - case vol.EmptyDir != nil: - case vol.Secret != nil: - case vol.DownwardAPI != nil: - case vol.ConfigMap != nil: - case vol.Projected != nil: - default: - allErrs = append(allErrs, field.Forbidden(fldPath.Child("volumes").Index(i), "volume type not supported when `pod.Spec.HostUsers` is false")) - } - } - // We decided to restrict the usage of userns with other host namespaces: // https://github.com/kubernetes/kubernetes/pull/111090#discussion_r935994282 // The tl;dr is: you can easily run into permission issues that seem unexpected, we don't diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 7474473f816..46167e32f49 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -21780,8 +21780,8 @@ func TestValidateHostUsers(t *testing.T) { }}, }, }, { - name: "hostUsers=false - unsupported volume", - success: false, + name: "hostUsers=false - stateful volume", + success: true, spec: &core.PodSpec{ SecurityContext: &core.PodSecurityContext{ HostUsers: &falseVar, @@ -21794,7 +21794,6 @@ func TestValidateHostUsers(t *testing.T) { }}, }, }, { - // It should ignore unsupported volumes with hostUsers=true. name: "hostUsers=true - unsupported volume", success: true, spec: &core.PodSpec{ From 531d38e323c54378acd8ea664f5752d31e8ee27a Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Thu, 22 Jun 2023 15:18:22 +0200 Subject: [PATCH 2/2] features: rename UserNamespacesStatelessPodsSupport now it is called UserNamespacesSupport since all kind of volumes are supported. Signed-off-by: Giuseppe Scrivano --- pkg/api/pod/util.go | 2 +- pkg/api/pod/util_test.go | 2 +- pkg/features/kube_features.go | 4 ++-- pkg/kubelet/kuberuntime/kuberuntime_container_linux.go | 2 +- pkg/kubelet/userns/userns_manager.go | 8 ++++---- pkg/kubelet/userns/userns_manager_test.go | 4 ++-- test/e2e/common/node/security_context.go | 8 ++++---- 7 files changed, 15 insertions(+), 15 deletions(-) diff --git a/pkg/api/pod/util.go b/pkg/api/pod/util.go index b063a21e135..1ca2645bf1e 100644 --- a/pkg/api/pod/util.go +++ b/pkg/api/pod/util.go @@ -477,7 +477,7 @@ func dropDisabledFields( } // If the feature is disabled and not in use, drop the hostUsers field. - if !utilfeature.DefaultFeatureGate.Enabled(features.UserNamespacesStatelessPodsSupport) && !hostUsersInUse(oldPodSpec) { + if !utilfeature.DefaultFeatureGate.Enabled(features.UserNamespacesSupport) && !hostUsersInUse(oldPodSpec) { // Drop the field in podSpec only if SecurityContext is not nil. // If it is nil, there is no need to set hostUsers=nil (it will be nil too). if podSpec.SecurityContext != nil { diff --git a/pkg/api/pod/util_test.go b/pkg/api/pod/util_test.go index f399eab4285..18dfe1f7942 100644 --- a/pkg/api/pod/util_test.go +++ b/pkg/api/pod/util_test.go @@ -1700,7 +1700,7 @@ func TestDropHostUsers(t *testing.T) { } t.Run(fmt.Sprintf("feature enabled=%v, old pod %v, new pod %v", enabled, oldPodInfo.description, newPodInfo.description), func(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.UserNamespacesStatelessPodsSupport, enabled)() + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.UserNamespacesSupport, enabled)() DropDisabledPodFields(newPod, oldPod) diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 3827a55e388..547b49c7dd6 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -793,7 +793,7 @@ const ( // alpha: v1.25 // // Enables user namespace support for stateless pods. - UserNamespacesStatelessPodsSupport featuregate.Feature = "UserNamespacesStatelessPodsSupport" + UserNamespacesSupport featuregate.Feature = "UserNamespacesSupport" // owner: @cofyc // alpha: v1.21 @@ -1058,7 +1058,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS VolumeCapacityPriority: {Default: false, PreRelease: featuregate.Alpha}, - UserNamespacesStatelessPodsSupport: {Default: false, PreRelease: featuregate.Alpha}, + UserNamespacesSupport: {Default: false, PreRelease: featuregate.Alpha}, WinDSR: {Default: false, PreRelease: featuregate.Alpha}, diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container_linux.go b/pkg/kubelet/kuberuntime/kuberuntime_container_linux.go index 6db16bd03d1..466378deda3 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_container_linux.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_container_linux.go @@ -55,7 +55,7 @@ func (m *kubeGenericRuntimeManager) applyPlatformSpecificContainerConfig(config } config.Linux = cl - if utilfeature.DefaultFeatureGate.Enabled(kubefeatures.UserNamespacesStatelessPodsSupport) { + if utilfeature.DefaultFeatureGate.Enabled(kubefeatures.UserNamespacesSupport) { if cl.SecurityContext.NamespaceOptions.UsernsOptions != nil { for _, mount := range config.Mounts { mount.UidMappings = cl.SecurityContext.NamespaceOptions.UsernsOptions.Uids diff --git a/pkg/kubelet/userns/userns_manager.go b/pkg/kubelet/userns/userns_manager.go index 7d23f215adc..ffd23630f13 100644 --- a/pkg/kubelet/userns/userns_manager.go +++ b/pkg/kubelet/userns/userns_manager.go @@ -142,7 +142,7 @@ func MakeUserNsManager(kl userNsPodsManager) (*UsernsManager, error) { } // do not bother reading the list of pods if user namespaces are not enabled. - if !utilfeature.DefaultFeatureGate.Enabled(features.UserNamespacesStatelessPodsSupport) { + if !utilfeature.DefaultFeatureGate.Enabled(features.UserNamespacesSupport) { return &m, nil } @@ -258,7 +258,7 @@ func (m *UsernsManager) record(pod types.UID, from, length uint32) (err error) { // Release releases the user namespace allocated to the specified pod. func (m *UsernsManager) Release(podUID types.UID) { - if !utilfeature.DefaultFeatureGate.Enabled(features.UserNamespacesStatelessPodsSupport) { + if !utilfeature.DefaultFeatureGate.Enabled(features.UserNamespacesSupport) { return } @@ -367,7 +367,7 @@ func (m *UsernsManager) createUserNs(pod *v1.Pod) (userNs userNamespace, err err // GetOrCreateUserNamespaceMappings returns the configuration for the sandbox user namespace func (m *UsernsManager) GetOrCreateUserNamespaceMappings(pod *v1.Pod) (*runtimeapi.UserNamespace, error) { - if !utilfeature.DefaultFeatureGate.Enabled(features.UserNamespacesStatelessPodsSupport) { + if !utilfeature.DefaultFeatureGate.Enabled(features.UserNamespacesSupport) { return nil, nil } @@ -427,7 +427,7 @@ func (m *UsernsManager) GetOrCreateUserNamespaceMappings(pod *v1.Pod) (*runtimea // allocations with the pods actually running. It frees any user namespace // allocation for orphaned pods. func (m *UsernsManager) CleanupOrphanedPodUsernsAllocations(pods []*v1.Pod, runningPods []*kubecontainer.Pod) error { - if !utilfeature.DefaultFeatureGate.Enabled(features.UserNamespacesStatelessPodsSupport) { + if !utilfeature.DefaultFeatureGate.Enabled(features.UserNamespacesSupport) { return nil } diff --git a/pkg/kubelet/userns/userns_manager_test.go b/pkg/kubelet/userns/userns_manager_test.go index 5555296f58d..fc74025d75d 100644 --- a/pkg/kubelet/userns/userns_manager_test.go +++ b/pkg/kubelet/userns/userns_manager_test.go @@ -40,7 +40,7 @@ func (m *testUserNsPodsManager) ListPodsFromDisk() ([]types.UID, error) { } func TestUserNsManagerAllocate(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, pkgfeatures.UserNamespacesStatelessPodsSupport, true)() + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, pkgfeatures.UserNamespacesSupport, true)() testUserNsPodsManager := &testUserNsPodsManager{} m, err := MakeUserNsManager(testUserNsPodsManager) @@ -90,7 +90,7 @@ func TestUserNsManagerAllocate(t *testing.T) { } func TestUserNsManagerParseUserNsFile(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, pkgfeatures.UserNamespacesStatelessPodsSupport, true)() + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, pkgfeatures.UserNamespacesSupport, true)() cases := []struct { name string diff --git a/test/e2e/common/node/security_context.go b/test/e2e/common/node/security_context.go index 4997a96b534..0f014cf0c07 100644 --- a/test/e2e/common/node/security_context.go +++ b/test/e2e/common/node/security_context.go @@ -72,7 +72,7 @@ var _ = SIGDescribe("Security Context", func() { } } - ginkgo.It("must create the user namespace if set to false [LinuxOnly] [Feature:UserNamespacesStatelessPodsSupport]", func(ctx context.Context) { + ginkgo.It("must create the user namespace if set to false [LinuxOnly] [Feature:UserNamespacesSupport]", func(ctx context.Context) { // with hostUsers=false the pod must use a new user namespace podClient := e2epod.PodClientNS(f, f.Namespace.Name) @@ -110,7 +110,7 @@ var _ = SIGDescribe("Security Context", func() { } }) - ginkgo.It("must not create the user namespace if set to true [LinuxOnly] [Feature:UserNamespacesStatelessPodsSupport]", func(ctx context.Context) { + ginkgo.It("must not create the user namespace if set to true [LinuxOnly] [Feature:UserNamespacesSupport]", func(ctx context.Context) { // with hostUsers=true the pod must use the host user namespace pod := makePod(true) // When running in the host's user namespace, the /proc/self/uid_map file content looks like: @@ -121,7 +121,7 @@ var _ = SIGDescribe("Security Context", func() { }) }) - ginkgo.It("should mount all volumes with proper permissions with hostUsers=false [LinuxOnly] [Feature:UserNamespacesStatelessPodsSupport]", func(ctx context.Context) { + ginkgo.It("should mount all volumes with proper permissions with hostUsers=false [LinuxOnly] [Feature:UserNamespacesSupport]", func(ctx context.Context) { // Create all volume types supported: configmap, secret, downwardAPI, projected. // Create configmap. @@ -245,7 +245,7 @@ var _ = SIGDescribe("Security Context", func() { }) }) - ginkgo.It("should set FSGroup to user inside the container with hostUsers=false [LinuxOnly] [Feature:UserNamespacesStatelessPodsSupport]", func(ctx context.Context) { + ginkgo.It("should set FSGroup to user inside the container with hostUsers=false [LinuxOnly] [Feature:UserNamespacesSupport]", func(ctx context.Context) { // Create configmap. name := "userns-volumes-test-" + string(uuid.NewUUID()) configMap := newConfigMap(f, name)