From e1c4e85b52d92f0b85774140130d87ef0d87de69 Mon Sep 17 00:00:00 2001 From: Tim Allclair Date: Fri, 29 Oct 2021 18:05:57 -0700 Subject: [PATCH] [PodSecurity] Add ValidatePod unit test --- .../admission/admission.go | 5 +- .../admission/admission_test.go | 370 +++++++++++++++++- .../pod-security-admission/api/helpers.go | 3 + .../pod-security-admission/test/fixtures.go | 8 +- .../test/fixtures_test.go | 2 +- .../k8s.io/pod-security-admission/test/run.go | 2 +- 6 files changed, 373 insertions(+), 17 deletions(-) diff --git a/staging/src/k8s.io/pod-security-admission/admission/admission.go b/staging/src/k8s.io/pod-security-admission/admission/admission.go index 805761d76f4..ce8064064ec 100644 --- a/staging/src/k8s.io/pod-security-admission/admission/admission.go +++ b/staging/src/k8s.io/pod-security-admission/admission/admission.go @@ -149,7 +149,7 @@ func extractPodSpecFromTemplate(template *corev1.PodTemplateSpec) (*metav1.Objec return &template.ObjectMeta, &template.Spec, nil } -// CompleteConfiguration() sets up default or derived configuration. +// CompleteConfiguration sets up default or derived configuration. func (a *Admission) CompleteConfiguration() error { if a.Configuration != nil { if p, err := admissionapi.ToPolicy(a.Configuration.Defaults); err != nil { @@ -168,7 +168,7 @@ func (a *Admission) CompleteConfiguration() error { return nil } -// ValidateConfiguration() ensures all required fields are set with valid values. +// ValidateConfiguration ensures all required fields are set with valid values. func (a *Admission) ValidateConfiguration() error { if a.Configuration == nil { return fmt.Errorf("configuration required") @@ -675,6 +675,7 @@ func internalErrorResponse(msg string) *admissionv1.AdmissionResponse { // Relevant mutable pod fields as of 1.21 are image and seccomp annotations: // * https://github.com/kubernetes/kubernetes/blob/release-1.21/pkg/apis/core/validation/validation.go#L3947-L3949 func isSignificantPodUpdate(pod, oldPod *corev1.Pod) bool { + // TODO: invert this logic to only allow specific update types. if pod.Annotations[corev1.SeccompPodAnnotationKey] != oldPod.Annotations[corev1.SeccompPodAnnotationKey] { return true } diff --git a/staging/src/k8s.io/pod-security-admission/admission/admission_test.go b/staging/src/k8s.io/pod-security-admission/admission/admission_test.go index c3c9b0b12cb..81033c50fcb 100644 --- a/staging/src/k8s.io/pod-security-admission/admission/admission_test.go +++ b/staging/src/k8s.io/pod-security-admission/admission/admission_test.go @@ -18,6 +18,7 @@ package admission import ( "context" + "fmt" "math/rand" "reflect" "strings" @@ -25,20 +26,24 @@ import ( "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" admissionv1 "k8s.io/api/admission/v1" appsv1 "k8s.io/api/apps/v1" batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/uuid" admissionapi "k8s.io/pod-security-admission/admission/api" + "k8s.io/pod-security-admission/admission/api/load" "k8s.io/pod-security-admission/api" "k8s.io/pod-security-admission/metrics" "k8s.io/pod-security-admission/policy" + "k8s.io/pod-security-admission/test" "k8s.io/utils/pointer" ) @@ -194,12 +199,14 @@ func (t *testEvaluator) EvaluatePod(lv api.LevelVersion, meta *metav1.ObjectMeta } } -type testNamespaceGetter struct { - ns *corev1.Namespace -} +type testNamespaceGetter map[string]*corev1.Namespace -func (t *testNamespaceGetter) GetNamespace(ctx context.Context, name string) (*corev1.Namespace, error) { - return t.ns, nil +func (t testNamespaceGetter) GetNamespace(ctx context.Context, name string) (*corev1.Namespace, error) { + if ns, ok := t[name]; ok { + return ns.DeepCopy(), nil + } else { + return nil, apierrors.NewNotFound(corev1.Resource("namespaces"), name) + } } type testPodLister struct { @@ -734,11 +741,10 @@ func TestValidatePodController(t *testing.T) { evaluator, err := policy.NewEvaluator(policy.DefaultChecks()) assert.NoError(t, err) nsGetter := &testNamespaceGetter{ - ns: &corev1.Namespace{ + testNamespace: &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ - Name: testName, - Namespace: testNamespace, - Labels: nsLabels}}, + Name: testName, + Labels: nsLabels}}, } PodSpecExtractor := &DefaultPodSpecExtractor{} recorder := &FakeRecorder{} @@ -782,6 +788,329 @@ func TestValidatePodController(t *testing.T) { } } +func TestValidatePod(t *testing.T) { + const ( + exemptNs = "exempt-ns" + implicitNs = "implicit-ns" + privilegedNs = "privileged-ns" + baselineNs = "baseline-ns" + baselineWarnNs = "baseline-warn-ns" + baselineAuditNs = "baseline-audit-ns" + restrictedNs = "restricted-ns" + invalidNs = "invalid-ns" + + exemptUser = "exempt-user" + exemptRuntimeClass = "exempt-runtimeclass" + ) + + objMetadata := metav1.ObjectMeta{Name: "test-pod", Labels: map[string]string{"foo": "bar"}} + + restrictedPod, err := test.GetMinimalValidPod(api.LevelRestricted, api.MajorMinorVersion(1, 23)) + require.NoError(t, err) + restrictedPod.ObjectMeta = *objMetadata.DeepCopy() + + baselinePod, err := test.GetMinimalValidPod(api.LevelBaseline, api.MajorMinorVersion(1, 23)) + require.NoError(t, err) + baselinePod.ObjectMeta = *objMetadata.DeepCopy() + + privilegedPod := *baselinePod.DeepCopy() + privilegedPod.Spec.Containers[0].SecurityContext = &corev1.SecurityContext{ + Privileged: pointer.Bool(true), + } + + exemptRCPod := *privilegedPod.DeepCopy() + exemptRCPod.Spec.RuntimeClassName = pointer.String(exemptRuntimeClass) + + tolerantPod := *privilegedPod.DeepCopy() + tolerantPod.Spec.Tolerations = []corev1.Toleration{{ + Operator: corev1.TolerationOpExists, + }} + + differentPrivilegedPod := *privilegedPod.DeepCopy() + differentPrivilegedPod.Spec.Containers[0].Image = "https://example.com/a-different-image" + + differentRestrictedPod := *restrictedPod.DeepCopy() + differentRestrictedPod.Spec.Containers[0].Image = "https://example.com/a-different-image" + + deployment := appsv1.Deployment{ + ObjectMeta: *objMetadata.DeepCopy(), + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + ObjectMeta: *objMetadata.DeepCopy(), + Spec: *restrictedPod.Spec.DeepCopy(), + }, + }, + } + + makeNs := func(enforceLevel, warnLevel, auditLevel api.Level) *corev1.Namespace { + ns := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{}, + }, + } + if enforceLevel != "" { + ns.Labels[api.EnforceLevelLabel] = string(enforceLevel) + } + if warnLevel != "" { + ns.Labels[api.WarnLevelLabel] = string(warnLevel) + } + if auditLevel != "" { + ns.Labels[api.AuditLevelLabel] = string(auditLevel) + } + return ns + } + nsGetter := testNamespaceGetter{ + exemptNs: makeNs(api.LevelRestricted, api.LevelRestricted, api.LevelRestricted), + implicitNs: makeNs("", "", ""), + privilegedNs: makeNs(api.LevelPrivileged, api.LevelPrivileged, api.LevelPrivileged), + baselineNs: makeNs(api.LevelBaseline, "", ""), + baselineWarnNs: makeNs("", api.LevelBaseline, ""), + baselineAuditNs: makeNs("", "", api.LevelBaseline), + restrictedNs: makeNs(api.LevelRestricted, api.LevelRestricted, api.LevelRestricted), + invalidNs: makeNs("not-a-valid-level", "", ""), + } + + config, err := load.LoadFromData(nil) // Start with the default config. + require.NoError(t, err, "loading default config") + config.Exemptions.Namespaces = []string{exemptNs} + config.Exemptions.RuntimeClasses = []string{exemptRuntimeClass} + config.Exemptions.Usernames = []string{exemptUser} + + evaluator, err := policy.NewEvaluator(policy.DefaultChecks()) + assert.NoError(t, err) + + testCases := []struct { + desc string + + namespace string + username string + runtimeClass string + + operation admissionv1.Operation + obj runtime.Object + objErr error // Error to return instead of obj by attrs.GetObject() + oldObj runtime.Object + oldObjErr error // Error to return instead of oldObj by attrs.GetOldObject() + subresource string + + expectAllowed bool + expectReason metav1.StatusReason + expectWarning bool + expectedAuditAnnotationKeys []string + }{ + { + desc: "ignored subresource", + namespace: restrictedNs, + obj: privilegedPod.DeepCopy(), + subresource: "status", + expectAllowed: true, + }, + { + desc: "exempt namespace", + namespace: exemptNs, + obj: privilegedPod.DeepCopy(), + expectAllowed: true, + expectedAuditAnnotationKeys: []string{"exempt"}, + }, + { + desc: "exempt user", + namespace: restrictedNs, + username: exemptUser, + obj: privilegedPod.DeepCopy(), + expectAllowed: true, + expectedAuditAnnotationKeys: []string{"exempt"}, + }, + { + desc: "exempt runtimeClass", + namespace: restrictedNs, + obj: exemptRCPod.DeepCopy(), + expectAllowed: true, + expectedAuditAnnotationKeys: []string{"exempt"}, + }, + { + desc: "namespace not found", + namespace: "missing-ns", + obj: restrictedPod.DeepCopy(), + expectAllowed: false, + expectReason: metav1.StatusReasonInternalError, + }, + { + desc: "short-circuit privileged:latest (implicit)", + namespace: implicitNs, + obj: privilegedPod.DeepCopy(), + expectAllowed: true, + }, + { + desc: "short-circuit privileged:latest (explicit)", + namespace: privilegedNs, + obj: privilegedPod.DeepCopy(), + expectAllowed: true, + }, + { + desc: "failed decode", + namespace: baselineNs, + objErr: fmt.Errorf("expected (failed decode)"), + expectAllowed: false, + expectReason: metav1.StatusReasonBadRequest, + }, + { + desc: "invalid object", + namespace: baselineNs, + obj: deployment.DeepCopy(), + expectAllowed: false, + expectReason: metav1.StatusReasonBadRequest, + }, + { + desc: "failed decode old object", + namespace: baselineNs, + operation: admissionv1.Update, + obj: restrictedPod.DeepCopy(), + oldObjErr: fmt.Errorf("expected (failed decode)"), + expectAllowed: false, + expectReason: metav1.StatusReasonBadRequest, + }, + { + desc: "invalid old object", + namespace: baselineNs, + operation: admissionv1.Update, + obj: restrictedPod.DeepCopy(), + oldObj: deployment.DeepCopy(), + expectAllowed: false, + expectReason: metav1.StatusReasonBadRequest, + }, + { + desc: "insignificant pod update", + namespace: restrictedNs, + operation: admissionv1.Update, + obj: tolerantPod.DeepCopy(), + oldObj: privilegedPod.DeepCopy(), + expectAllowed: true, + }, + { + desc: "significant pod update - denied", + namespace: baselineNs, + operation: admissionv1.Update, + obj: differentPrivilegedPod.DeepCopy(), + oldObj: privilegedPod.DeepCopy(), + expectAllowed: false, + expectReason: metav1.StatusReasonForbidden, + expectedAuditAnnotationKeys: []string{"enforce-policy"}, + }, + { + desc: "significant pod update - allowed", + namespace: restrictedNs, + operation: admissionv1.Update, + obj: differentRestrictedPod.DeepCopy(), + oldObj: restrictedPod, + expectAllowed: true, + expectedAuditAnnotationKeys: []string{"enforce-policy"}, + }, + { + desc: "invalid namespace labels", + namespace: invalidNs, + obj: baselinePod.DeepCopy(), + expectAllowed: false, + expectReason: metav1.StatusReasonForbidden, + expectedAuditAnnotationKeys: []string{"enforce-policy", "error"}, + }, + { + desc: "enforce deny", + namespace: baselineNs, + obj: privilegedPod.DeepCopy(), + expectAllowed: false, + expectReason: metav1.StatusReasonForbidden, + expectedAuditAnnotationKeys: []string{"enforce-policy"}, + }, + { + desc: "enforce allow", + namespace: baselineNs, + obj: baselinePod.DeepCopy(), + expectAllowed: true, + expectedAuditAnnotationKeys: []string{"enforce-policy"}, + }, + { + desc: "warn deny", + namespace: baselineWarnNs, + obj: privilegedPod.DeepCopy(), + expectAllowed: true, + expectWarning: true, + expectedAuditAnnotationKeys: []string{"enforce-policy"}, + }, + { + desc: "audit deny", + namespace: baselineAuditNs, + obj: privilegedPod.DeepCopy(), + expectAllowed: true, + expectedAuditAnnotationKeys: []string{"enforce-policy", "audit-violations"}, + }, + } + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + if tc.obj != nil { + tc.obj.(metav1.ObjectMetaAccessor).GetObjectMeta().SetNamespace(tc.namespace) + } + if tc.oldObj != nil { + tc.oldObj.(metav1.ObjectMetaAccessor).GetObjectMeta().SetNamespace(tc.namespace) + } + attrs := &testAttributes{ + AttributesRecord: api.AttributesRecord{ + Name: "test-pod", + Namespace: tc.namespace, + Kind: schema.GroupVersionKind{Version: "v1", Kind: "Pod"}, + Resource: schema.GroupVersionResource{Version: "v1", Resource: "pods"}, + Subresource: tc.subresource, + Operation: admissionv1.Create, + Object: tc.obj, + OldObject: tc.oldObj, + Username: "test-user", + }, + objectErr: tc.objErr, + oldObjectErr: tc.oldObjErr, + } + if tc.operation != "" { + attrs.Operation = tc.operation + } + if tc.username != "" { + attrs.Username = tc.username + } + + a := &Admission{ + PodLister: &testPodLister{}, + Evaluator: evaluator, + Configuration: config, + Metrics: &FakeRecorder{}, + NamespaceGetter: nsGetter, + } + require.NoError(t, a.CompleteConfiguration(), "CompleteConfiguration()") + require.NoError(t, a.ValidateConfiguration(), "ValidateConfiguration()") + + response := a.Validate(context.TODO(), attrs) + + if tc.expectAllowed { + assert.True(t, response.Allowed, "Allowed") + assert.Nil(t, response.Result) + } else { + assert.False(t, response.Allowed) + if assert.NotNil(t, response.Result, "Result") { + assert.Equal(t, tc.expectReason, response.Result.Reason, "Reason") + } + } + + if tc.expectWarning { + assert.NotEmpty(t, response.Warnings, "Warnings") + } else { + assert.Empty(t, response.Warnings, "Warnings") + } + + assert.Len(t, response.AuditAnnotations, len(tc.expectedAuditAnnotationKeys), "AuditAnnotations") + for _, key := range tc.expectedAuditAnnotationKeys { + assert.Contains(t, response.AuditAnnotations, key, "AuditAnnotations") + } + }) + } +} + type FakeRecorder struct { evaluations []EvaluationRecord } @@ -862,3 +1191,26 @@ func TestPrioritizePods(t *testing.T) { assert.Fail(t, "Pod count is not the same after prioritization") } } + +type testAttributes struct { + api.AttributesRecord + + objectErr error + oldObjectErr error +} + +func (a *testAttributes) GetObject() (runtime.Object, error) { + if a.objectErr != nil { + return nil, a.objectErr + } else { + return a.AttributesRecord.GetObject() + } +} + +func (a *testAttributes) GetOldObject() (runtime.Object, error) { + if a.oldObjectErr != nil { + return nil, a.oldObjectErr + } else { + return a.AttributesRecord.GetOldObject() + } +} diff --git a/staging/src/k8s.io/pod-security-admission/api/helpers.go b/staging/src/k8s.io/pod-security-admission/api/helpers.go index 5e309fddc2c..d46670de6c5 100644 --- a/staging/src/k8s.io/pod-security-admission/api/helpers.go +++ b/staging/src/k8s.io/pod-security-admission/api/helpers.go @@ -161,6 +161,9 @@ func PolicyToEvaluate(labels map[string]string, defaults Policy) (Policy, field. p = defaults ) + if len(labels) == 0 { + return p, nil + } if level, ok := labels[EnforceLevelLabel]; ok { p.Enforce.Level, err = ParseLevel(level) errs = appendErr(errs, err, EnforceLevelLabel, level) 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 33d801c1c9d..cd11f0d380a 100644 --- a/staging/src/k8s.io/pod-security-admission/test/fixtures.go +++ b/staging/src/k8s.io/pod-security-admission/test/fixtures.go @@ -51,7 +51,7 @@ func init() { }) minimalValidPods[api.LevelRestricted][api.MajorMinorVersion(1, 0)] = restricted_1_0 - // 1.8+: runAsNonRoot=true + // 1.8+: allowPrivilegeEscalation=false restricted_1_8 := tweak(restricted_1_0, func(p *corev1.Pod) { p.Spec.Containers[0].SecurityContext = &corev1.SecurityContext{AllowPrivilegeEscalation: pointer.BoolPtr(false)} p.Spec.InitContainers[0].SecurityContext = &corev1.SecurityContext{AllowPrivilegeEscalation: pointer.BoolPtr(false)} @@ -75,8 +75,8 @@ func init() { minimalValidPods[api.LevelRestricted][api.MajorMinorVersion(1, 22)] = restricted_1_22 } -// getValidPod returns a minimal valid pod for the specified level and version. -func getMinimalValidPod(level api.Level, version api.Version) (*corev1.Pod, error) { +// GetMinimalValidPod returns a minimal valid pod for the specified level and version. +func GetMinimalValidPod(level api.Level, version api.Version) (*corev1.Pod, error) { originalVersion := version for { pod, exists := minimalValidPods[level][version] @@ -169,7 +169,7 @@ func getFixtures(key fixtureKey) (fixtureData, error) { return fixtureData{}, err } - validPodForLevel, err := getMinimalValidPod(key.level, key.version) + validPodForLevel, err := GetMinimalValidPod(key.level, key.version) if err != nil { return fixtureData{}, err } diff --git a/staging/src/k8s.io/pod-security-admission/test/fixtures_test.go b/staging/src/k8s.io/pod-security-admission/test/fixtures_test.go index 86ded471653..b6a17b89b3d 100644 --- a/staging/src/k8s.io/pod-security-admission/test/fixtures_test.go +++ b/staging/src/k8s.io/pod-security-admission/test/fixtures_test.go @@ -61,7 +61,7 @@ func TestFixtures(t *testing.T) { failDir := filepath.Join("testdata", string(level), fmt.Sprintf("v1.%d", version), "fail") // render the minimal valid pod fixture - validPod, err := getMinimalValidPod(level, api.MajorMinorVersion(1, version)) + validPod, err := GetMinimalValidPod(level, api.MajorMinorVersion(1, version)) if err != nil { t.Fatal(err) } 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 311203d60f1..030fe808c5b 100644 --- a/staging/src/k8s.io/pod-security-admission/test/run.go +++ b/staging/src/k8s.io/pod-security-admission/test/run.go @@ -296,7 +296,7 @@ func Run(t *testing.T, opts Options) { } } - minimalValidPod, err := getMinimalValidPod(level, version) + minimalValidPod, err := GetMinimalValidPod(level, version) if err != nil { t.Fatal(err) }