feat: enforce kind on admission review

[acpana]

Fixes #2504

This improves the UX of using gator verify with an AdmissionReview input. Prior, an err would occur deeper in the call stack at evaluation which actually occluded the real issue too:

unexpected number of violations: got 1 violations but want none: got messages [unable to match constraints: invalid request object: failed to unmarshal gkReview object {"spec" ... 

This somewhat ties in with #2005 but alas it only solves the problem with missing kind error.

[maxsmythe]

LGTM

[ritazh]

Do we want to emulate real admission requests for the DELETE operation such that

object is the new object being admitted.
It is null for DELETE operations.

https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#webhook-request-and-response

[ritazh]

When I print input.review.object in the rego after running kubectl delete I definitely see the object and it is not null. This was on a k8s v1.25 cluster. Not sure if the doc is old because the behavior is updated but this was definitely the case before. #144 (comment) very strange.

[acpana]

Hm, this is interesting. We specifically decided to overwrite the Object when I/ we worked on this: https://github.com/open-policy-agent/gatekeeper/blob/master/pkg/util/request_validation.go#L15-L32 which was what g8r was doing before too:

• gatekeeper/pkg/webhook/policy.go

  Lines 148 to 152 in 149fb90

  	if err := util.SetObjectOnDelete(&req); err != nil {
  	vResp := admission.Denied(err.Error())
  	vResp.Result.Code = http.StatusInternalServerError
  	return vResp
  	}

If we did want to change things around here, I'd be open to opening a new issue and tackling this in a new PR. Just want to keep this one's scope small to kind enforcement. Would that be ok?

[ritazh]

LOL looks like I added this in 2019 #146 so it is a GK specific behavior. So for gator, we can assume object is not null then?

[maxsmythe]

IMO the input should match what we expect to receive from the API server (meaning an appropriate test would show object as nil and gator should handle the rewriting to make object == oldObject).

[acpana]

an appropriate test would show object as nil and gator should handle the rewriting to make object == oldObject).

could I handle this in a followup PR?

[ritazh]

LGTM