From cbbe7d1bb91d3e59a1f51e4f729a059b4564e009 Mon Sep 17 00:00:00 2001 From: Lee Verberne Date: Thu, 31 Oct 2019 17:15:23 +0000 Subject: [PATCH] Remove checks for PodShareProcessNamespace feature gate --- pkg/api/pod/util.go | 16 ---- pkg/api/pod/util_test.go | 100 ------------------------ pkg/features/kube_features.go | 2 +- pkg/kubelet/kuberuntime/helpers.go | 4 +- pkg/kubelet/kuberuntime/helpers_test.go | 49 ------------ test/e2e_node/BUILD | 1 - test/e2e_node/security_context_test.go | 6 -- 7 files changed, 2 insertions(+), 176 deletions(-) diff --git a/pkg/api/pod/util.go b/pkg/api/pod/util.go index 30bdcd8e83c..4e9fc774a08 100644 --- a/pkg/api/pod/util.go +++ b/pkg/api/pod/util.go @@ -340,12 +340,6 @@ func dropDisabledFields( } } - if !utilfeature.DefaultFeatureGate.Enabled(features.PodShareProcessNamespace) && !shareProcessNamespaceInUse(oldPodSpec) { - if podSpec.SecurityContext != nil { - podSpec.SecurityContext.ShareProcessNamespace = nil - } - } - if !utilfeature.DefaultFeatureGate.Enabled(features.Sysctls) && !sysctlsInUse(oldPodSpec) { if podSpec.SecurityContext != nil { podSpec.SecurityContext.Sysctls = nil @@ -633,16 +627,6 @@ func appArmorInUse(podAnnotations map[string]string) bool { return false } -func shareProcessNamespaceInUse(podSpec *api.PodSpec) bool { - if podSpec == nil { - return false - } - if podSpec.SecurityContext != nil && podSpec.SecurityContext.ShareProcessNamespace != nil { - return true - } - return false -} - func tokenRequestProjectionInUse(podSpec *api.PodSpec) bool { if podSpec == nil { return false diff --git a/pkg/api/pod/util_test.go b/pkg/api/pod/util_test.go index 93952681008..12485e681f2 100644 --- a/pkg/api/pod/util_test.go +++ b/pkg/api/pod/util_test.go @@ -980,106 +980,6 @@ func TestDropEmptyDirSizeLimit(t *testing.T) { } } -func TestDropPodShareProcessNamespace(t *testing.T) { - podWithShareProcessNamespace := func() *api.Pod { - return &api.Pod{ - Spec: api.PodSpec{ - SecurityContext: &api.PodSecurityContext{ - ShareProcessNamespace: &[]bool{true}[0], - }, - }, - } - } - podWithoutShareProcessNamespace := func() *api.Pod { - return &api.Pod{ - Spec: api.PodSpec{ - SecurityContext: &api.PodSecurityContext{}, - }, - } - } - podWithoutSecurityContext := func() *api.Pod { - return &api.Pod{ - Spec: api.PodSpec{}, - } - } - - podInfo := []struct { - description string - hasShareProcessNamespace bool - pod func() *api.Pod - }{ - { - description: "has ShareProcessNamespace", - hasShareProcessNamespace: true, - pod: podWithShareProcessNamespace, - }, - { - description: "does not have ShareProcessNamespace", - hasShareProcessNamespace: false, - pod: podWithoutShareProcessNamespace, - }, - { - description: "does not have SecurityContext", - hasShareProcessNamespace: false, - pod: podWithoutSecurityContext, - }, - { - description: "is nil", - hasShareProcessNamespace: false, - pod: func() *api.Pod { return nil }, - }, - } - - for _, enabled := range []bool{true, false} { - for _, oldPodInfo := range podInfo { - for _, newPodInfo := range podInfo { - oldPodHasShareProcessNamespace, oldPod := oldPodInfo.hasShareProcessNamespace, oldPodInfo.pod() - newPodHasShareProcessNamespace, newPod := newPodInfo.hasShareProcessNamespace, newPodInfo.pod() - if newPod == nil { - continue - } - - 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.PodShareProcessNamespace, enabled)() - - var oldPodSpec *api.PodSpec - if oldPod != nil { - oldPodSpec = &oldPod.Spec - } - dropDisabledFields(&newPod.Spec, nil, oldPodSpec, nil) - - // old pod should never be changed - if !reflect.DeepEqual(oldPod, oldPodInfo.pod()) { - t.Errorf("old pod changed: %v", diff.ObjectReflectDiff(oldPod, oldPodInfo.pod())) - } - - switch { - case enabled || oldPodHasShareProcessNamespace: - // new pod should not be changed if the feature is enabled, or if the old pod had ShareProcessNamespace set - if !reflect.DeepEqual(newPod, newPodInfo.pod()) { - t.Errorf("new pod changed: %v", diff.ObjectReflectDiff(newPod, newPodInfo.pod())) - } - case newPodHasShareProcessNamespace: - // new pod should be changed - if reflect.DeepEqual(newPod, newPodInfo.pod()) { - t.Errorf("new pod was not changed") - } - // new pod should not have ShareProcessNamespace - if !reflect.DeepEqual(newPod, podWithoutShareProcessNamespace()) { - t.Errorf("new pod had ShareProcessNamespace: %v", diff.ObjectReflectDiff(newPod, podWithoutShareProcessNamespace())) - } - default: - // new pod should not need to be changed - if !reflect.DeepEqual(newPod, newPodInfo.pod()) { - t.Errorf("new pod changed: %v", diff.ObjectReflectDiff(newPod, newPodInfo.pod())) - } - } - }) - } - } - } -} - func TestDropAppArmor(t *testing.T) { podWithAppArmor := func() *api.Pod { return &api.Pod{ diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 7b089a124ac..a17ef453840 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -521,7 +521,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS LocalStorageCapacityIsolation: {Default: true, PreRelease: featuregate.Beta}, Sysctls: {Default: true, PreRelease: featuregate.Beta}, EphemeralContainers: {Default: false, PreRelease: featuregate.Alpha}, - PodShareProcessNamespace: {Default: true, PreRelease: featuregate.GA}, // remove in 1.19 + PodShareProcessNamespace: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.19 PodPriority: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.18 TaintNodesByCondition: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.18 QOSReserved: {Default: false, PreRelease: featuregate.Alpha}, diff --git a/pkg/kubelet/kuberuntime/helpers.go b/pkg/kubelet/kuberuntime/helpers.go index 1d7c4e2c12d..2d970333b3b 100644 --- a/pkg/kubelet/kuberuntime/helpers.go +++ b/pkg/kubelet/kuberuntime/helpers.go @@ -24,10 +24,8 @@ import ( "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" - utilfeature "k8s.io/apiserver/pkg/util/feature" runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1alpha2" "k8s.io/klog" - "k8s.io/kubernetes/pkg/features" kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" ) @@ -250,7 +248,7 @@ func pidNamespaceForPod(pod *v1.Pod) runtimeapi.NamespaceMode { if pod.Spec.HostPID { return runtimeapi.NamespaceMode_NODE } - if utilfeature.DefaultFeatureGate.Enabled(features.PodShareProcessNamespace) && pod.Spec.ShareProcessNamespace != nil && *pod.Spec.ShareProcessNamespace { + if pod.Spec.ShareProcessNamespace != nil && *pod.Spec.ShareProcessNamespace { return runtimeapi.NamespaceMode_POD } } diff --git a/pkg/kubelet/kuberuntime/helpers_test.go b/pkg/kubelet/kuberuntime/helpers_test.go index a48b9591291..e8f26d99bd7 100644 --- a/pkg/kubelet/kuberuntime/helpers_test.go +++ b/pkg/kubelet/kuberuntime/helpers_test.go @@ -25,11 +25,8 @@ import ( "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - utilfeature "k8s.io/apiserver/pkg/util/feature" - featuregatetesting "k8s.io/component-base/featuregate/testing" runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1alpha2" runtimetesting "k8s.io/cri-api/pkg/apis/testing" - "k8s.io/kubernetes/pkg/features" kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" ) @@ -285,8 +282,6 @@ func TestGetSeccompProfileFromAnnotations(t *testing.T) { } func TestNamespacesForPod(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.PodShareProcessNamespace, true)() - for desc, test := range map[string]struct { input *v1.Pod expected *runtimeapi.NamespaceOption @@ -350,48 +345,4 @@ func TestNamespacesForPod(t *testing.T) { actual := namespacesForPod(test.input) assert.Equal(t, test.expected, actual) } - - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.PodShareProcessNamespace, false)() - - for desc, test := range map[string]struct { - input *v1.Pod - expected *runtimeapi.NamespaceOption - }{ - "v1.Pod default namespaces": { - &v1.Pod{}, - &runtimeapi.NamespaceOption{ - Ipc: runtimeapi.NamespaceMode_POD, - Network: runtimeapi.NamespaceMode_POD, - Pid: runtimeapi.NamespaceMode_CONTAINER, - }, - }, - "Shared Process Namespace (feature disabled)": { - &v1.Pod{ - Spec: v1.PodSpec{ - ShareProcessNamespace: &[]bool{true}[0], - }, - }, - &runtimeapi.NamespaceOption{ - Ipc: runtimeapi.NamespaceMode_POD, - Network: runtimeapi.NamespaceMode_POD, - Pid: runtimeapi.NamespaceMode_CONTAINER, - }, - }, - "Shared Process Namespace, redundant flag (feature disabled)": { - &v1.Pod{ - Spec: v1.PodSpec{ - ShareProcessNamespace: &[]bool{false}[0], - }, - }, - &runtimeapi.NamespaceOption{ - Ipc: runtimeapi.NamespaceMode_POD, - Network: runtimeapi.NamespaceMode_POD, - Pid: runtimeapi.NamespaceMode_CONTAINER, - }, - }, - } { - t.Logf("TestCase: %s", desc) - actual := namespacesForPod(test.input) - assert.Equal(t, test.expected, actual) - } } diff --git a/test/e2e_node/BUILD b/test/e2e_node/BUILD index 92f21edf97f..c6d2c2a05bf 100644 --- a/test/e2e_node/BUILD +++ b/test/e2e_node/BUILD @@ -161,7 +161,6 @@ go_test( "//staging/src/k8s.io/apimachinery/pkg/util/uuid:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/watch:go_default_library", - "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", "//staging/src/k8s.io/client-go/kubernetes:go_default_library", "//staging/src/k8s.io/client-go/tools/watch:go_default_library", "//staging/src/k8s.io/cri-api/pkg/apis:go_default_library", diff --git a/test/e2e_node/security_context_test.go b/test/e2e_node/security_context_test.go index 9b823f80d62..b87f5047462 100644 --- a/test/e2e_node/security_context_test.go +++ b/test/e2e_node/security_context_test.go @@ -26,8 +26,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/uuid" - utilfeature "k8s.io/apiserver/pkg/util/feature" - "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/test/e2e/framework" e2epod "k8s.io/kubernetes/test/e2e/framework/pod" imageutils "k8s.io/kubernetes/test/utils/image" @@ -79,10 +77,6 @@ var _ = framework.KubeDescribe("Security Context", func() { if !isEnabled { framework.Skipf("Skipped because shared PID namespace is not supported by this docker version.") } - // It's not enough to set this flag in the kubelet because the apiserver needs it too - if !utilfeature.DefaultFeatureGate.Enabled(features.PodShareProcessNamespace) { - framework.Skipf("run test with --feature-gates=PodShareProcessNamespace=true to test PID namespace sharing") - } ginkgo.By("Create a pod with shared PID namespace.") f.PodClient().CreateSync(&v1.Pod{