feat: expose AdmissionReview for gator verify (#2348)

* feat: expose AdmissionReview for gator verify Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com> * review: error out on lack of objects Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com> * review: mimick g8r webhook behavior on DELETE Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com> * move arr logic into helper func Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com> * Update pkg/gator/errors.go Co-authored-by: Rita Zhang <rita.z.zhang@gmail.com> Signed-off-by: alex <8968914+acpana@users.noreply.github.com> * review: no code duplication on check Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com> * review: func name Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com> Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com> Signed-off-by: alex <8968914+acpana@users.noreply.github.com> Co-authored-by: Rita Zhang <rita.z.zhang@gmail.com> Co-authored-by: Sertaç Özercan <852750+sozercan@users.noreply.github.com> Co-authored-by: Max Smythe <smythe@google.com>
open-policy-agent · Oct 31, 2022 · 4ec4f34 · 4ec4f34
1 parent 66539ed
commit 4ec4f34
Show file tree

Hide file tree

Showing 8 changed files with 375 additions and 15 deletions.
diff --git a/pkg/gator/errors.go b/pkg/gator/errors.go
@@ -41,4 +41,17 @@ var (
 	// ErrValidConstraint occurs when a test's configuration signals an expectation
 	// that a constraint should fail validation but no validation error is raised.
 	ErrValidConstraint = errors.New("constraint should have failed schema validation")
+	// ErrInvalidK8sAdmissionReview occurs when a test attempts to pass in an AdmissionReview
+	// object but we fail to convert the unstructured object into a typed AdmissionReview one.
+	ErrInvalidK8sAdmissionReview = errors.New("not a valid AdmissionReview object")
+	// ErrMissingK8sAdmissionRequest occurs when a test attempts to pass in an AdmissionReview
+	// object but it does not actually pass in an AdmissionRequest object.
+	ErrMissingK8sAdmissionRequest = errors.New("missing an AdmissionRequest object")
+	// ErrReviewObject occurs when a test attempts to pass in an AdmissionRequest with no
+	// object or oldObject for the underlying framework to review.
+	// This mimicks the k8s api server behvaior.
+	ErrNoObjectForReview = errors.New("no object or oldObject found to review")
+	// ErrNilOldObject indicates that the AdmissionRequest did not provide an oldObject.
+	// Gatekeeper expects oldObject to be non nil on DELETE operations.
+	ErrNilOldObject = errors.New("oldObject is nil")
 )
diff --git a/pkg/gator/fixtures/fixtures.go b/pkg/gator/fixtures/fixtures.go
@@ -1,6 +1,29 @@
+// fixtures package contains commonly used ConstraintTemplates, Constraints, Objects and other k8s resources
+// mostly used for testing.
 package fixtures
 
 const (
+	TemplateValidateUserInfo = `
+kind: ConstraintTemplate
+apiVersion: templates.gatekeeper.sh/v1beta1
+metadata:
+  name: validateuserinfo
+spec:
+  crd:
+    spec:
+      names:
+        kind: ValidateUserInfo
+  targets:
+    - target: admission.k8s.gatekeeper.sh
+      rego: |
+        package k8svalidateuserinfo
+        violation[{"msg": msg}] {
+          username := input.review.userInfo.username
+          not startswith(username, "system:")
+          msg := sprintf("username is not allowed to perform this operation: %v", [username])
+        }
+`
+
 	TemplateAlwaysValidate = `
 kind: ConstraintTemplate
 apiVersion: templates.gatekeeper.sh/v1beta1
@@ -446,4 +469,76 @@ spec:
   parameters:
     labels: ["abc"]
 `
+
+	// AdmissionReviewMissingRequest makes sure that our code can handle nil requests.
+	AdmissionReviewMissingRequest = `
+kind: AdmissionReview
+apiVersion: admission.k8s.io/v1beta1
+`
+	// AdmissionReviewMissingObjectAndOldObject makes sure we enforce having an object to review.
+	AdmissionReviewMissingObjectAndOldObject = `
+kind: AdmissionReview
+apiVersion: admission.k8s.io/v1beta1
+request:
+  name:
+`
+
+	// AdmissionReviewWithOldObject proves that our code handles submitting a request with an oldObject for review.
+	AdmissionReviewWithOldObject = `
+kind: AdmissionReview
+apiVersion: admission.k8s.io/v1beta1
+request:
+  oldObject:
+    labels: 
+      - app: "bar"
+`
+
+	// DeleteAdmissionReviewWithNoOldObject enforces the AdmissionRequest behavior for k8s v1.15.0+ for DELETE operations.
+	DeleteAdmissionReviewWithNoOldObject = `
+kind: AdmissionReview
+apiVersion: admission.k8s.io/v1beta1
+request:
+  operation: "DELETE"
+  object:
+    labels:
+      - app: "bar"
+`
+	// SystemAdmissionReview holds a request coming from a system user name.
+	SystemAdmissionReview = `
+kind: AdmissionReview
+apiVersion: admission.k8s.io/v1beta1
+request:
+  userInfo:
+    username: "system:foo"
+  object:
+    labels: 
+      - app: "bar"
+`
+
+	// NonSystemAdmissionReview holds a request coming from a non system user name.
+	NonSystemAdmissionReview = `
+kind: AdmissionReview
+apiVersion: admission.k8s.io/v1
+request:
+  userInfo:
+    username: "foo"
+  object:
+    labels: 
+      - app: "bar"
+`
+
+	// InvalidAdmissionReview cannot be converted into a typed AdmissionReview.
+	InvalidAdmissionReview = `
+kind: AdmissionReview
+apiVersion: admission.k8s.io/v1
+request:
+key_that_does_not_exist_in_spec: "some_value"
+`
+
+	ConstraintAlwaysValidateUserInfo = `
+kind: ValidateUserInfo
+apiVersion: constraints.gatekeeper.sh/v1beta1
+metadata:
+  name: always-pass
+`
 )
diff --git a/pkg/gator/runner.go b/pkg/gator/runner.go
@@ -13,8 +13,11 @@ import (
 	"github.com/open-policy-agent/gatekeeper/apis"
 	mutationtypes "github.com/open-policy-agent/gatekeeper/pkg/mutation/types"
 	"github.com/open-policy-agent/gatekeeper/pkg/target"
+	"github.com/open-policy-agent/gatekeeper/pkg/util"
+	admissionv1 "k8s.io/api/admission/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 )
 
 // Runner defines logic independent of how tests are run and the results are
@@ -282,13 +285,49 @@ func (r *Runner) runReview(ctx context.Context, newClient func() (Client, error)
 		}
 	}
 
+	// check to see if obj is an AdmissionReview kind
+	if toReview.GetKind() == "AdmissionReview" && toReview.GroupVersionKind().Group == admissionv1.SchemeGroupVersion.Group {
+		return r.validateAndReviewAdmissionReviewRequest(ctx, c, toReview)
+	}
+
+	// otherwise our object is some other k8s object
 	au := target.AugmentedUnstructured{
 		Object: *toReview,
 		Source: mutationtypes.SourceTypeOriginal,
 	}
 	return c.Review(ctx, au)
 }
 
+func (r *Runner) validateAndReviewAdmissionReviewRequest(ctx context.Context, c Client, toReview *unstructured.Unstructured) (*types.Responses, error) {
+	// convert unstructured into AdmissionReview, don't allow unknown fields
+	var ar admissionv1.AdmissionReview
+	if err := runtime.DefaultUnstructuredConverter.FromUnstructuredWithValidation(toReview.UnstructuredContent(), &ar, true); err != nil {
+		return nil, fmt.Errorf("%w: unable to convert to an AdmissionReview object, error: %v", ErrInvalidK8sAdmissionReview, err)
+	}
+
+	if ar.Request == nil { // then this admission review did not actually pass in an AdmissionRequest
+		return nil, fmt.Errorf("%w: request did not actually pass in an AdmissionRequest", ErrMissingK8sAdmissionRequest)
+	}
+
+	// validate the AdmissionReview to match k8s api server behavior
+	if ar.Request.Object.Raw == nil && ar.Request.OldObject.Raw == nil {
+		return nil, fmt.Errorf("%w: AdmissionRequest does not contain an \"object\" or \"oldObject\" to review", ErrNoObjectForReview)
+	}
+
+	// parse into webhook/admission type
+	req := &admission.Request{AdmissionRequest: *ar.Request}
+	if err := util.SetObjectOnDelete(req); err != nil {
+		return nil, fmt.Errorf("%w: %v", ErrNilOldObject, err.Error())
+	}
+
+	arr := target.AugmentedReview{
+		AdmissionRequest: &req.AdmissionRequest,
+		Source:           mutationtypes.SourceTypeOriginal,
+	}
+
+	return c.Review(ctx, arr)
+}
+
 func (r *Runner) addInventory(ctx context.Context, c Client, suiteDir, inventoryPath string) error {
 	p := path.Join(suiteDir, inventoryPath)
 

diff --git a/pkg/gator/runner_test.go b/pkg/gator/runner_test.go
@@ -1016,6 +1016,124 @@ func TestRunner_Run(t *testing.T) {
 				}},
 			},
 		},
+		{
+			name: "admission review object",
+			suite: Suite{
+				Tests: []Test{
+					{
+						Name:       "userInfo admission review object", // this test checks that the user name start with "system:"
+						Template:   "template.yaml",
+						Constraint: "constraint.yaml",
+						Cases: []*Case{
+							{
+								Name:       "user begins with \"system:\"",
+								Object:     "system-ar.yaml",
+								Assertions: []Assertion{{Violations: intStrFromStr("no")}},
+							},
+							{
+								Name:       "user doesn't begin with \"system:\"",
+								Object:     "non-system-ar.yaml",
+								Assertions: []Assertion{{Violations: intStrFromStr("yes"), Message: pointer.StringPtr("username is not allowed to perform this operation")}},
+							},
+						},
+					},
+					{
+						Name:       "AdmissionReview with oldObject", // this test makes sure that we are submitting an old object for review
+						Template:   "template.yaml",
+						Constraint: "constraint.yaml",
+						Cases: []*Case{
+							{
+								Name:       "oldObject submits for review",
+								Object:     "oldObject-ar.yaml",
+								Assertions: []Assertion{{Violations: intStrFromStr("no")}},
+							},
+						},
+					},
+					{
+						Name:       "invalid admission review usage", // this test covers error handling for invalid admission review objects
+						Template:   "template.yaml",
+						Constraint: "constraint.yaml",
+						Cases: []*Case{
+							{
+								Name:       "invalid admission review object", // this is an AdmissionReview with unknown fields
+								Object:     "invalid-ar.yaml",
+								Assertions: []Assertion{{}},
+							},
+							{
+								Name:       "missing admission request object", // this is a blank AdmissionReview test
+								Object:     "missing-ar-ar.yaml",
+								Assertions: []Assertion{{}},
+							},
+							{
+								Name:       "no objects to review", // this is an AdmissionRequest with no objects to review
+								Object:     "no-objects-ar.yaml",
+								Assertions: []Assertion{{}},
+							},
+							{
+								Name:       "no oldObject on delete", // this is an AdmissionRequest w a DELETE operation but no oldObject provided
+								Object:     "no-oldObject-ar.yaml",
+								Assertions: []Assertion{{}},
+							},
+						},
+					},
+				},
+			},
+			f: fstest.MapFS{
+				"template.yaml": &fstest.MapFile{
+					Data: []byte(fixtures.TemplateValidateUserInfo),
+				},
+				"constraint.yaml": &fstest.MapFile{
+					Data: []byte(fixtures.ConstraintAlwaysValidateUserInfo),
+				},
+				"no-objects-ar.yaml": &fstest.MapFile{
+					Data: []byte(fixtures.AdmissionReviewMissingObjectAndOldObject),
+				},
+				"system-ar.yaml": &fstest.MapFile{
+					Data: []byte(fixtures.SystemAdmissionReview),
+				},
+				"non-system-ar.yaml": &fstest.MapFile{
+					Data: []byte(fixtures.NonSystemAdmissionReview),
+				},
+				"invalid-ar.yaml": &fstest.MapFile{
+					Data: []byte(fixtures.InvalidAdmissionReview),
+				},
+				"missing-ar-ar.yaml": &fstest.MapFile{
+					Data: []byte(fixtures.AdmissionReviewMissingRequest),
+				},
+				"oldObject-ar.yaml": &fstest.MapFile{
+					Data: []byte(fixtures.AdmissionReviewWithOldObject),
+				},
+				"no-oldObject-ar.yaml": &fstest.MapFile{
+					Data: []byte(fixtures.DeleteAdmissionReviewWithNoOldObject),
+				},
+			},
+			want: SuiteResult{
+				TestResults: []TestResult{
+					{
+						Name: "userInfo admission review object",
+						CaseResults: []CaseResult{
+							{Name: "user begins with \"system:\""},
+							{Name: "user doesn't begin with \"system:\""},
+						},
+					},
+					{
+						Name: "AdmissionReview with oldObject",
+						CaseResults: []CaseResult{
+							{Name: "oldObject submits for review"},
+						},
+					},
+					{
+						Name: "invalid admission review usage",
+						CaseResults: []CaseResult{
+							{Name: "invalid admission review object", Error: ErrInvalidK8sAdmissionReview},
+							{Name: "missing admission request object", Error: ErrMissingK8sAdmissionRequest},
+							{Name: "no objects to review", Error: ErrNoObjectForReview},
+							{Name: "no oldObject on delete", Error: ErrNilOldObject},
+						},
+					},
+				},
+			},
+		},
 	}
 
 	for _, tc := range testCases {

diff --git a/pkg/gator/suite.go b/pkg/gator/suite.go
@@ -51,6 +51,9 @@ type Case struct {
 	Name string `json:"name"`
 
 	// Object is the path to the file containing a Kubernetes object to test.
+	// NOTE: Object can be an AdmissionReview resource as well to facilitate open box testing.
+	// To convert the object to an an AdmissionReview, the group, kind pair needs to be set
+	// as "AdmissionReview, admission.k8s.io"
 	Object string `json:"object"`
 
 	// Inventory is a list of paths to files containing Kubernetes objects to put

diff --git a/pkg/util/request_validation.go b/pkg/util/request_validation.go
@@ -0,0 +1,33 @@
+package util
+
+import (
+	"errors"
+
+	admissionv1 "k8s.io/api/admission/v1"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+)
+
+// nolint: revive // Moved error out of pkg/webhook/admission; needs capitalization for backwards compat.
+var ErrOldObjectIsNil = errors.New("For admission webhooks registered for DELETE operations, please use Kubernetes v1.15.0+.")
+
+// SetObjectOnDelete enforces that we use at least K8s API v1.15.0+ on DELETE operations
+// and copies over the oldObject into the Object field for the given AdmissionRequest.
+func SetObjectOnDelete(req *admission.Request) error {
+	if req.AdmissionRequest.Operation == admissionv1.Delete {
+		// oldObject is the existing object.
+		// It is null for DELETE operations in API servers prior to v1.15.0.
+		// https://github.com/kubernetes/website/pull/14671
+		if req.AdmissionRequest.OldObject.Raw == nil {
+			return ErrOldObjectIsNil
+		}
+
+		// For admission webhooks registered for DELETE operations on k8s built APIs or CRDs,
+		// the apiserver now sends the existing object as admissionRequest.Request.OldObject to the webhook
+		// object is the new object being admitted.
+		// It is null for DELETE operations.
+		// https://github.com/kubernetes/kubernetes/pull/76346
+		req.AdmissionRequest.Object = req.AdmissionRequest.OldObject
+	}
+
+	return nil
+}