From e87016cf9416041fcec182bbd6fdc491beb8b2a6 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Wed, 30 Jun 2021 11:24:31 -0400 Subject: [PATCH 1/2] PodSecurity: add ability to skip failure cases if relevant features are disabled --- staging/publishing/import-restrictions.yaml | 1 + .../src/k8s.io/pod-security-admission/go.mod | 1 + .../pod-security-admission/test/fixtures.go | 15 +++++++++++++++ .../k8s.io/pod-security-admission/test/run.go | 17 +++++++++++++++++ 4 files changed, 34 insertions(+) diff --git a/staging/publishing/import-restrictions.yaml b/staging/publishing/import-restrictions.yaml index b64fd49a6cc..003ffa2f32b 100644 --- a/staging/publishing/import-restrictions.yaml +++ b/staging/publishing/import-restrictions.yaml @@ -271,4 +271,5 @@ - k8s.io/client-go - k8s.io/klog - k8s.io/pod-security-admission + - k8s.io/component-base/featuregate - k8s.io/utils diff --git a/staging/src/k8s.io/pod-security-admission/go.mod b/staging/src/k8s.io/pod-security-admission/go.mod index d785f67723e..f65ee084445 100644 --- a/staging/src/k8s.io/pod-security-admission/go.mod +++ b/staging/src/k8s.io/pod-security-admission/go.mod @@ -11,6 +11,7 @@ require ( k8s.io/apimachinery v0.0.0 k8s.io/apiserver v0.0.0 k8s.io/client-go v0.0.0 + k8s.io/component-base v0.0.0 k8s.io/klog/v2 v2.9.0 k8s.io/utils v0.0.0-20210521133846-da695404a2bc sigs.k8s.io/yaml v1.2.0 diff --git a/staging/src/k8s.io/pod-security-admission/test/fixtures.go b/staging/src/k8s.io/pod-security-admission/test/fixtures.go index 2d32d4324a6..8ba2062fcb5 100644 --- a/staging/src/k8s.io/pod-security-admission/test/fixtures.go +++ b/staging/src/k8s.io/pod-security-admission/test/fixtures.go @@ -20,6 +20,7 @@ import ( "fmt" corev1 "k8s.io/api/core/v1" + "k8s.io/component-base/featuregate" "k8s.io/pod-security-admission/api" "k8s.io/pod-security-admission/policy" "k8s.io/utils/pointer" @@ -90,6 +91,13 @@ type fixtureGenerator struct { // expectErrorSubstring is a substring to expect in the error message for failed pods. // if empty, the check ID is used. expectErrorSubstring string + + // failRequiresFeatures lists feature gates that must all be enabled for failure cases to fail properly. + // This allows failure cases depending on rejecting data populated in alpha or beta fields to be skipped when those features are not enabled. + // If empty, failure test cases are always run. + // Pass cases are not allowed to be feature-gated (pass cases must only depend on data existing in GA fields). + failRequiresFeatures []featuregate.Feature + // generatePass transforms a minimum valid pod into one or more valid pods. // pods do not need to populate metadata.name. generatePass func(*corev1.Pod) []*corev1.Pod @@ -102,6 +110,12 @@ type fixtureGenerator struct { type fixtureData struct { expectErrorSubstring string + // failRequiresFeatures lists feature gates that must all be enabled for failure cases to fail properly. + // This allows failure cases depending on rejecting data populated in alpha or beta fields to be skipped when those features are not enabled. + // If empty, failure test cases are always run. + // Pass cases are not allowed to be feature-gated (pass cases must only depend on data existing in GA fields). + failRequiresFeatures []featuregate.Feature + pass []*corev1.Pod fail []*corev1.Pod } @@ -148,6 +162,7 @@ func getFixtures(key fixtureKey) (fixtureData, error) { if generator, exists := fixtureGenerators[key]; exists { data := fixtureData{ expectErrorSubstring: generator.expectErrorSubstring, + failRequiresFeatures: generator.failRequiresFeatures, pass: generator.generatePass(validPodForLevel.DeepCopy()), fail: generator.generateFail(validPodForLevel.DeepCopy()), diff --git a/staging/src/k8s.io/pod-security-admission/test/run.go b/staging/src/k8s.io/pod-security-admission/test/run.go index 61e6d28e36a..3a3ccb48870 100644 --- a/staging/src/k8s.io/pod-security-admission/test/run.go +++ b/staging/src/k8s.io/pod-security-admission/test/run.go @@ -30,6 +30,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" + "k8s.io/component-base/featuregate" "k8s.io/pod-security-admission/api" "k8s.io/pod-security-admission/policy" ) @@ -41,6 +42,11 @@ type Options struct { // Required. ClientConfig *rest.Config + // Features optionally provides information about which feature gates are enabled. + // This is used to skip failure cases for negative tests of data in alpha/beta fields. + // If unset, all testcases are run. + Features featuregate.FeatureGate + // CreateNamespace is an optional stub for creating a namespace with the given name and labels. // Returning an error fails the test. // If nil, DefaultCreateNamespace is used. @@ -278,7 +284,18 @@ func Run(t *testing.T, opts Options) { createController(t, i, pod, true, "") } }) + + // see if any features required for failure cases are disabled + var disabledRequiredFeatures []featuregate.Feature + for _, f := range checkData.failRequiresFeatures { + if opts.Features != nil && !opts.Features.Enabled(f) { + disabledRequiredFeatures = append(disabledRequiredFeatures, f) + } + } t.Run(ns+"_fail_"+checkID, func(t *testing.T) { + if len(disabledRequiredFeatures) > 0 { + t.Skipf("features required for failure cases are disabled: %v", disabledRequiredFeatures) + } for i, pod := range checkData.fail { createPod(t, i, pod, false, checkData.expectErrorSubstring) createController(t, i, pod, false, checkData.expectErrorSubstring) From ba6b4c5a18c455867f11036a7208962e84012a86 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Wed, 30 Jun 2021 11:25:13 -0400 Subject: [PATCH 2/2] PodSecurity: test GA-only cases and alpha/beta fields separately --- .../featuregate/feature_gate.go | 11 ++++ test/integration/auth/podsecurity_test.go | 54 +++++++++++++++---- 2 files changed, 54 insertions(+), 11 deletions(-) diff --git a/staging/src/k8s.io/component-base/featuregate/feature_gate.go b/staging/src/k8s.io/component-base/featuregate/feature_gate.go index c805ffb01b5..c7166d80b84 100644 --- a/staging/src/k8s.io/component-base/featuregate/feature_gate.go +++ b/staging/src/k8s.io/component-base/featuregate/feature_gate.go @@ -109,6 +109,8 @@ type MutableFeatureGate interface { SetFromMap(m map[string]bool) error // Add adds features to the featureGate. Add(features map[Feature]FeatureSpec) error + // GetAll returns a copy of the map of known feature names to feature specs. + GetAll() map[Feature]FeatureSpec } // featureGate implements FeatureGate as well as pflag.Value for flag parsing. @@ -290,6 +292,15 @@ func (f *featureGate) Add(features map[Feature]FeatureSpec) error { return nil } +// GetAll returns a copy of the map of known feature names to feature specs. +func (f *featureGate) GetAll() map[Feature]FeatureSpec { + retval := map[Feature]FeatureSpec{} + for k, v := range f.known.Load().(map[Feature]FeatureSpec) { + retval[k] = v + } + return retval +} + // Enabled returns true if the key is enabled. If the key is not known, this call will panic. func (f *featureGate) Enabled(key Feature) bool { if v, ok := f.enabled.Load().(map[Feature]bool)[key]; ok { diff --git a/test/integration/auth/podsecurity_test.go b/test/integration/auth/podsecurity_test.go index bea7e700067..baea7c03a63 100644 --- a/test/integration/auth/podsecurity_test.go +++ b/test/integration/auth/podsecurity_test.go @@ -20,6 +20,7 @@ import ( "testing" utilfeature "k8s.io/apiserver/pkg/util/feature" + "k8s.io/component-base/featuregate" featuregatetesting "k8s.io/component-base/featuregate/testing" kubeapiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing" "k8s.io/kubernetes/pkg/capabilities" @@ -29,21 +30,17 @@ import ( ) func TestPodSecurity(t *testing.T) { + // Enable all feature gates needed to allow all fields to be exercised + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ProcMountType, true)() + // Ensure the PodSecurity feature is enabled defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.PodSecurity, true)() - server := kubeapiservertesting.StartTestServerOrDie(t, kubeapiservertesting.NewDefaultTestServerOptions(), []string{ - "--anonymous-auth=false", - "--enable-admission-plugins=PodSecurity", - "--allow-privileged=true", - // TODO: "--admission-control-config-file=" + admissionConfigFile.Name(), - }, framework.SharedEtcd()) - defer server.TearDownFn() - - // ensure the global is set to allow privileged containers - capabilities.SetForTests(capabilities.Capabilities{AllowPrivileged: true}) - + // Start server + server := startPodSecurityServer(t) opts := podsecuritytest.Options{ ClientConfig: server.ClientConfig, + // Don't pass in feature-gate info, so all testcases run + // TODO ExemptClient: nil, ExemptNamespaces: []string{}, @@ -51,3 +48,38 @@ func TestPodSecurity(t *testing.T) { } podsecuritytest.Run(t, opts) } + +// TestPodSecurityGAOnly ensures policies pass with only GA features enabled +func TestPodSecurityGAOnly(t *testing.T) { + // Disable all alpha and beta features + for k, v := range utilfeature.DefaultFeatureGate.DeepCopy().GetAll() { + if v.PreRelease == featuregate.Alpha || v.PreRelease == featuregate.Beta { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, k, false)() + } + } + // Ensure PodSecurity feature is enabled + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.PodSecurity, true)() + // Start server + server := startPodSecurityServer(t) + + opts := podsecuritytest.Options{ + ClientConfig: server.ClientConfig, + // Pass in feature gate info so negative test cases depending on alpha or beta features can be skipped + Features: utilfeature.DefaultFeatureGate, + } + podsecuritytest.Run(t, opts) +} + +func startPodSecurityServer(t *testing.T) *kubeapiservertesting.TestServer { + // ensure the global is set to allow privileged containers + capabilities.SetForTests(capabilities.Capabilities{AllowPrivileged: true}) + + server := kubeapiservertesting.StartTestServerOrDie(t, kubeapiservertesting.NewDefaultTestServerOptions(), []string{ + "--anonymous-auth=false", + "--enable-admission-plugins=PodSecurity", + "--allow-privileged=true", + // TODO: "--admission-control-config-file=" + admissionConfigFile.Name(), + }, framework.SharedEtcd()) + t.Cleanup(server.TearDownFn) + return server +}