feat: use admission review v1 (#5464)

* feat: use admission review v1

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

* fix

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

* nit

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

* logs

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

* patch type

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

* fix tests

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

* fix

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>
Signed-off-by: Charles-Edouard Brétéché <charled.breteche@gmail.com>
Co-authored-by: Prateek Pandey <prateek.pandey@nirmata.com>

.github/workflows/e2e.yaml

@@ -91,3 +91,4 @@ jobs:
           kubectl -n kyverno describe pod | grep -i events -A10
           kubectl -n kyverno logs deploy/kyverno -p || true
           kubectl -n kyverno logs deploy/kyverno
+          kubectl -n kube-system logs kube-apiserver-kind-control-plane

cmd/cleanup-controller/handlers.go

@@ -25,11 +25,11 @@ func (h *cleanupPolicyHandlers) Validate(ctx context.Context, logger logr.Logger
 	policy, _, err := admissionutils.GetCleanupPolicies(request)
 	if err != nil {
 		logger.Error(err, "failed to unmarshal policies from admission request")
-		return admissionutils.Response(err)
+		return admissionutils.Response(request.UID, err)
 	}
 	err = validate.ValidateCleanupPolicy(logger, policy, h.client, false)
 	if err != nil {
 		logger.Error(err, "policy validation errors")
 	}
-	return admissionutils.Response(err)
+	return admissionutils.Response(request.UID, err)
 }

pkg/controllers/webhook/controller.go

@@ -526,7 +526,7 @@ func (c *controller) buildVerifyMutatingWebhookConfiguration(caBundle []byte) (*
 				FailurePolicy:           &ignore,
 				SideEffects:             &noneOnDryRun,
 				ReinvocationPolicy:      &ifNeeded,
-				AdmissionReviewVersions: []string{"v1beta1"},
+				AdmissionReviewVersions: []string{"v1"},
 				ObjectSelector: &metav1.LabelSelector{
 					MatchLabels: map[string]string{
 						"app.kubernetes.io/name": kyvernov1.ValueKyvernoApp,
@@ -553,7 +553,7 @@ func (c *controller) buildPolicyMutatingWebhookConfiguration(caBundle []byte) (*
 				FailurePolicy:           &ignore,
 				SideEffects:             &noneOnDryRun,
 				ReinvocationPolicy:      &ifNeeded,
-				AdmissionReviewVersions: []string{"v1beta1"},
+				AdmissionReviewVersions: []string{"v1"},
 			}},
 		},
 		nil
@@ -574,7 +574,7 @@ func (c *controller) buildPolicyValidatingWebhookConfiguration(caBundle []byte)
 				}},
 				FailurePolicy:           &ignore,
 				SideEffects:             &none,
-				AdmissionReviewVersions: []string{"v1beta1"},
+				AdmissionReviewVersions: []string{"v1"},
 			}},
 		},
 		nil
@@ -599,7 +599,7 @@ func (c *controller) buildDefaultResourceMutatingWebhookConfiguration(caBundle [
 				}},
 				FailurePolicy:           &ignore,
 				SideEffects:             &noneOnDryRun,
-				AdmissionReviewVersions: []string{"v1beta1"},
+				AdmissionReviewVersions: []string{"v1"},
 				TimeoutSeconds:          &c.defaultTimeout,
 				ReinvocationPolicy:      &ifNeeded,
 			}},
@@ -651,7 +651,7 @@ func (c *controller) buildResourceMutatingWebhookConfiguration(caBundle []byte)
 					Rules:                   ignore.buildRulesWithOperations(admissionregistrationv1.Create, admissionregistrationv1.Update),
 					FailurePolicy:           &ignore.failurePolicy,
 					SideEffects:             &noneOnDryRun,
-					AdmissionReviewVersions: []string{"v1beta1"},
+					AdmissionReviewVersions: []string{"v1"},
 					NamespaceSelector:       webhookCfg.NamespaceSelector,
 					ObjectSelector:          webhookCfg.ObjectSelector,
 					TimeoutSeconds:          &ignore.maxWebhookTimeout,
@@ -668,7 +668,7 @@ func (c *controller) buildResourceMutatingWebhookConfiguration(caBundle []byte)
 					Rules:                   fail.buildRulesWithOperations(admissionregistrationv1.Create, admissionregistrationv1.Update),
 					FailurePolicy:           &fail.failurePolicy,
 					SideEffects:             &noneOnDryRun,
-					AdmissionReviewVersions: []string{"v1beta1"},
+					AdmissionReviewVersions: []string{"v1"},
 					NamespaceSelector:       webhookCfg.NamespaceSelector,
 					ObjectSelector:          webhookCfg.ObjectSelector,
 					TimeoutSeconds:          &fail.maxWebhookTimeout,
@@ -707,7 +707,7 @@ func (c *controller) buildDefaultResourceValidatingWebhookConfiguration(caBundle
 				}},
 				FailurePolicy:           &ignore,
 				SideEffects:             sideEffects,
-				AdmissionReviewVersions: []string{"v1beta1"},
+				AdmissionReviewVersions: []string{"v1"},
 				TimeoutSeconds:          &c.defaultTimeout,
 			}},
 		},
@@ -762,7 +762,7 @@ func (c *controller) buildResourceValidatingWebhookConfiguration(caBundle []byte
 					Rules:                   ignore.buildRulesWithOperations(admissionregistrationv1.Create, admissionregistrationv1.Update, admissionregistrationv1.Delete, admissionregistrationv1.Connect),
 					FailurePolicy:           &ignore.failurePolicy,
 					SideEffects:             sideEffects,
-					AdmissionReviewVersions: []string{"v1beta1"},
+					AdmissionReviewVersions: []string{"v1"},
 					NamespaceSelector:       webhookCfg.NamespaceSelector,
 					ObjectSelector:          webhookCfg.ObjectSelector,
 					TimeoutSeconds:          &ignore.maxWebhookTimeout,
@@ -778,7 +778,7 @@ func (c *controller) buildResourceValidatingWebhookConfiguration(caBundle []byte
 					Rules:                   fail.buildRulesWithOperations(admissionregistrationv1.Create, admissionregistrationv1.Update, admissionregistrationv1.Delete, admissionregistrationv1.Connect),
 					FailurePolicy:           &fail.failurePolicy,
 					SideEffects:             sideEffects,
-					AdmissionReviewVersions: []string{"v1beta1"},
+					AdmissionReviewVersions: []string{"v1"},
 					NamespaceSelector:       webhookCfg.NamespaceSelector,
 					ObjectSelector:          webhookCfg.ObjectSelector,
 					TimeoutSeconds:          &fail.maxWebhookTimeout,

pkg/utils/admission/response.go

@@ -3,11 +3,15 @@ package admission
 import (
 	admissionv1 "k8s.io/api/admission/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 )
 
-func Response(err error, warnings ...string) *admissionv1.AdmissionResponse {
+var patchTypeJSONPatch = admissionv1.PatchTypeJSONPatch
+
+func Response(uid types.UID, err error, warnings ...string) *admissionv1.AdmissionResponse {
 	response := &admissionv1.AdmissionResponse{
 		Allowed: err == nil,
+		UID:     uid,
 	}
 	if err != nil {
 		response.Result = &metav1.Status{
@@ -19,12 +23,15 @@ func Response(err error, warnings ...string) *admissionv1.AdmissionResponse {
 	return response
 }
 
-func ResponseSuccess(warnings ...string) *admissionv1.AdmissionResponse {
+func ResponseSuccess(uid types.UID, warnings ...string) *admissionv1.AdmissionResponse {
-	return Response(nil, warnings...)
+	return Response(uid, nil, warnings...)
 }
 
-func MutationResponse(patch []byte, warnings ...string) *admissionv1.AdmissionResponse {
+func MutationResponse(uid types.UID, patch []byte, warnings ...string) *admissionv1.AdmissionResponse {
-	response := ResponseSuccess(warnings...)
+	response := ResponseSuccess(uid, warnings...)
-	response.Patch = patch
+	if len(patch) != 0 {
+		response.Patch = patch
+		response.PatchType = &patchTypeJSONPatch
+	}
 	return response
 }

pkg/utils/admission/response_test.go

@@ -67,7 +67,7 @@ func TestResponse(t *testing.T) {
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if got := Response(tt.args.err, tt.args.warnings...); !reflect.DeepEqual(got, tt.want) {
+			if got := Response("", tt.args.err, tt.args.warnings...); !reflect.DeepEqual(got, tt.want) {
 				t.Errorf("Response() = %v, want %v", got, tt.want)
 			}
 		})
@@ -102,7 +102,7 @@ func TestResponseSuccess(t *testing.T) {
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if got := ResponseSuccess(tt.args.warnings...); !reflect.DeepEqual(got, tt.want) {
+			if got := ResponseSuccess("", tt.args.warnings...); !reflect.DeepEqual(got, tt.want) {
 				t.Errorf("ResponseSuccess() = %v, want %v", got, tt.want)
 			}
 		})
@@ -144,8 +144,9 @@ func TestMutationResponse(t *testing.T) {
 			warnings: nil,
 		},
 		want: &admissionv1.AdmissionResponse{
-			Allowed: true,
+			Allowed:   true,
-			Patch:   []byte{1, 2, 3, 4},
+			Patch:     []byte{1, 2, 3, 4},
+			PatchType: &patchTypeJSONPatch,
 		},
 	}, {
 		name: "patch, warnings",
@@ -154,14 +155,15 @@ func TestMutationResponse(t *testing.T) {
 			warnings: []string{"foo", "bar"},
 		},
 		want: &admissionv1.AdmissionResponse{
-			Allowed:  true,
+			Allowed:   true,
-			Patch:    []byte{1, 2, 3, 4},
+			Patch:     []byte{1, 2, 3, 4},
-			Warnings: []string{"foo", "bar"},
+			Warnings:  []string{"foo", "bar"},
+			PatchType: &patchTypeJSONPatch,
 		},
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if got := MutationResponse(tt.args.patch, tt.args.warnings...); !reflect.DeepEqual(got, tt.want) {
+			if got := MutationResponse("", tt.args.patch, tt.args.warnings...); !reflect.DeepEqual(got, tt.want) {
 				t.Errorf("MutationResponse() = %v, want %v", got, tt.want)
 			}
 		})

pkg/webhooks/handlers/admission.go

@@ -19,23 +19,23 @@ func (inner AdmissionHandler) withAdmission(logger logr.Logger) HttpHandler {
 	return func(writer http.ResponseWriter, request *http.Request) {
 		startTime := time.Now()
 		if request.Body == nil {
-			httpError(writer, request, logger, errors.New("empty body"), http.StatusBadRequest)
+			HttpError(request.Context(), writer, request, logger, errors.New("empty body"), http.StatusBadRequest)
 			return
 		}
 		defer request.Body.Close()
 		body, err := io.ReadAll(request.Body)
 		if err != nil {
-			httpError(writer, request, logger, err, http.StatusBadRequest)
+			HttpError(request.Context(), writer, request, logger, err, http.StatusBadRequest)
 			return
 		}
 		contentType := request.Header.Get("Content-Type")
 		if contentType != "application/json" {
-			httpError(writer, request, logger, errors.New("invalid Content-Type"), http.StatusUnsupportedMediaType)
+			HttpError(request.Context(), writer, request, logger, errors.New("invalid Content-Type"), http.StatusUnsupportedMediaType)
 			return
 		}
 		admissionReview := &admissionv1.AdmissionReview{}
 		if err := json.Unmarshal(body, &admissionReview); err != nil {
-			httpError(writer, request, logger, err, http.StatusExpectationFailed)
+			HttpError(request.Context(), writer, request, logger, err, http.StatusExpectationFailed)
 			return
 		}
 		logger := logger.WithValues(
@@ -56,12 +56,12 @@ func (inner AdmissionHandler) withAdmission(logger logr.Logger) HttpHandler {
 		}
 		responseJSON, err := json.Marshal(admissionReview)
 		if err != nil {
-			httpError(writer, request, logger, err, http.StatusInternalServerError)
+			HttpError(request.Context(), writer, request, logger, err, http.StatusInternalServerError)
 			return
 		}
 		writer.Header().Set("Content-Type", "application/json; charset=utf-8")
 		if _, err := writer.Write(responseJSON); err != nil {
-			httpError(writer, request, logger, err, http.StatusInternalServerError)
+			HttpError(request.Context(), writer, request, logger, err, http.StatusInternalServerError)
 			return
 		}
 		if admissionReview.Request.Kind.Kind == "Lease" {

pkg/webhooks/handlers/error.go

@@ -1,14 +1,15 @@
 package handlers
 
 import (
+	"context"
 	"net/http"
 
 	"github.com/go-logr/logr"
 	"github.com/kyverno/kyverno/pkg/tracing"
 )
 
-func httpError(writer http.ResponseWriter, request *http.Request, logger logr.Logger, err error, code int) {
+func HttpError(ctx context.Context, writer http.ResponseWriter, request *http.Request, logger logr.Logger, err error, code int) {
 	logger.Error(err, "an error has occurred", "url", request.URL.String())
-	tracing.SetHttpStatus(request.Context(), err, code)
+	tracing.SetHttpStatus(ctx, err, code)
 	http.Error(writer, err.Error(), code)
 }

pkg/webhooks/handlers/protect.go

@@ -27,14 +27,14 @@ func (inner AdmissionHandler) withProtection() AdmissionHandler {
 		newResource, oldResource, err := utils.ExtractResources(nil, request)
 		if err != nil {
 			logger.Error(err, "Failed to extract resources")
-			return admissionutils.Response(err)
+			return admissionutils.Response(request.UID, err)
 		}
 		for _, resource := range []unstructured.Unstructured{newResource, oldResource} {
 			resLabels := resource.GetLabels()
 			if resLabels[kyvernov1.LabelAppManagedBy] == kyvernov1.ValueKyvernoApp {
 				if request.UserInfo.Username != fmt.Sprintf("system:serviceaccount:%s:%s", config.KyvernoNamespace(), config.KyvernoServiceAccountName()) {
 					logger.Info("Access to the resource not authorized, this is a kyverno managed resource and should be altered only by kyverno")
-					return admissionutils.Response(errors.New("A kyverno managed resource can only be modified by kyverno"))
+					return admissionutils.Response(request.UID, errors.New("A kyverno managed resource can only be modified by kyverno"))
 				}
 			}
 		}

pkg/webhooks/handlers/verify.go

@@ -13,13 +13,13 @@ import (
 
 func Verify(ctx context.Context, logger logr.Logger, request *admissionv1.AdmissionRequest, startTime time.Time) *admissionv1.AdmissionResponse {
 	if request.Name != "kyverno-health" || request.Namespace != config.KyvernoNamespace() {
-		return admissionutils.ResponseSuccess()
+		return admissionutils.ResponseSuccess(request.UID)
 	}
 	patch := jsonutils.NewPatchOperation("/metadata/annotations/"+"kyverno.io~1last-request-time", "replace", time.Now().Format(time.RFC3339))
 	bytes, err := patch.ToPatchBytes()
 	if err != nil {
 		logger.Error(err, "failed to build patch bytes")
-		return admissionutils.Response(err)
+		return admissionutils.Response(request.UID, err)
 	}
-	return admissionutils.MutationResponse(bytes)
+	return admissionutils.MutationResponse(request.UID, bytes)
 }

pkg/webhooks/policy/handlers.go

@@ -29,15 +29,15 @@ func (h *handlers) Validate(ctx context.Context, logger logr.Logger, request *ad
 	policy, _, err := admissionutils.GetPolicies(request)
 	if err != nil {
 		logger.Error(err, "failed to unmarshal policies from admission request")
-		return admissionutils.Response(err)
+		return admissionutils.Response(request.UID, err)
 	}
 	warnings, err := policyvalidate.Validate(policy, h.client, false, h.openApiManager)
 	if err != nil {
 		logger.Error(err, "policy validation errors")
 	}
-	return admissionutils.Response(err, warnings...)
+	return admissionutils.Response(request.UID, err, warnings...)
 }
 
 func (h *handlers) Mutate(_ context.Context, _ logr.Logger, _ *admissionv1.AdmissionRequest, _ time.Time) *admissionv1.AdmissionResponse {
-	return admissionutils.ResponseSuccess()
+	return nil
 }

pkg/webhooks/resource/handlers.go

@@ -119,7 +119,7 @@ func (h *handlers) Validate(ctx context.Context, logger logr.Logger, request *ad
 
 	policyContext, err := h.pcBuilder.Build(request, generatePolicies...)
 	if err != nil {
-		return errorResponse(logger, err, "failed create policy context")
+		return errorResponse(logger, request.UID, err, "failed create policy context")
 	}
 
 	namespaceLabels := make(map[string]string)
@@ -132,13 +132,13 @@ func (h *handlers) Validate(ctx context.Context, logger logr.Logger, request *ad
 	ok, msg, warnings := vh.HandleValidation(h.metricsConfig, request, policies, policyContext, namespaceLabels, startTime)
 	if !ok {
 		logger.Info("admission request denied")
-		return admissionutils.Response(errors.New(msg), warnings...)
+		return admissionutils.Response(request.UID, errors.New(msg), warnings...)
 	}
 
 	defer h.handleDelete(logger, request)
 	go h.createUpdateRequests(logger, request, policyContext, generatePolicies, mutatePolicies, startTime)
 
-	return admissionutils.ResponseSuccess(warnings...)
+	return admissionutils.ResponseSuccess(request.UID, warnings...)
 }
 
 func (h *handlers) Mutate(ctx context.Context, logger logr.Logger, request *admissionv1.AdmissionRequest, failurePolicy string, startTime time.Time) *admissionv1.AdmissionResponse {
@@ -149,13 +149,13 @@ func (h *handlers) Mutate(ctx context.Context, logger logr.Logger, request *admi
 	verifyImagesPolicies := filterPolicies(failurePolicy, h.pCache.GetPolicies(policycache.VerifyImagesMutate, kind, request.Namespace)...)
 	if len(mutatePolicies) == 0 && len(verifyImagesPolicies) == 0 {
 		logger.V(4).Info("no policies matched mutate admission request")
-		return admissionutils.ResponseSuccess()
+		return admissionutils.ResponseSuccess(request.UID)
 	}
 	logger.V(4).Info("processing policies for mutate admission request", "mutatePolicies", len(mutatePolicies), "verifyImagesPolicies", len(verifyImagesPolicies))
 	policyContext, err := h.pcBuilder.Build(request, mutatePolicies...)
 	if err != nil {
 		logger.Error(err, "failed to build policy context")
-		return admissionutils.Response(err)
+		return admissionutils.Response(request.UID, err)
 	}
 	// update container images to a canonical form
 	if err := enginectx.MutateResourceWithImageInfo(request.Object.Raw, policyContext.JSONContext); err != nil {
@@ -165,26 +165,26 @@ func (h *handlers) Mutate(ctx context.Context, logger logr.Logger, request *admi
 	mutatePatches, mutateWarnings, err := mh.HandleMutation(h.metricsConfig, request, mutatePolicies, policyContext, startTime)
 	if err != nil {
 		logger.Error(err, "mutation failed")
-		return admissionutils.Response(err)
+		return admissionutils.Response(request.UID, err)
 	}
 	newRequest := patchRequest(mutatePatches, request, logger)
 	// rebuild context to process images updated via mutate policies
 	policyContext, err = h.pcBuilder.Build(newRequest, mutatePolicies...)
 	if err != nil {
 		logger.Error(err, "failed to build policy context")
-		return admissionutils.Response(err)
+		return admissionutils.Response(request.UID, err)
 	}
 	ivh := imageverification.NewImageVerificationHandler(logger, h.kyvernoClient, h.eventGen, h.admissionReports)
 	imagePatches, imageVerifyWarnings, err := ivh.Handle(h.metricsConfig, newRequest, verifyImagesPolicies, policyContext)
 	if err != nil {
 		logger.Error(err, "image verification failed")
-		return admissionutils.Response(err)
+		return admissionutils.Response(request.UID, err)
 	}
 	patch := jsonutils.JoinPatches(mutatePatches, imagePatches)
 	var warnings []string
 	warnings = append(warnings, mutateWarnings...)
 	warnings = append(warnings, imageVerifyWarnings...)
-	return admissionutils.MutationResponse(patch, warnings...)
+	return admissionutils.MutationResponse(request.UID, patch, warnings...)
 }
 
 func (h *handlers) handleDelete(logger logr.Logger, request *admissionv1.AdmissionRequest) {

pkg/webhooks/resource/utils.go

@@ -11,6 +11,7 @@ import (
 	admissionutils "github.com/kyverno/kyverno/pkg/utils/admission"
 	"github.com/kyverno/kyverno/pkg/webhooks/updaterequest"
 	admissionv1 "k8s.io/api/admission/v1"
+	"k8s.io/apimachinery/pkg/types"
 )
 
 type updateRequestResponse struct {
@@ -18,9 +19,9 @@ type updateRequestResponse struct {
 	err error
 }
 
-func errorResponse(logger logr.Logger, err error, message string) *admissionv1.AdmissionResponse {
+func errorResponse(logger logr.Logger, uid types.UID, err error, message string) *admissionv1.AdmissionResponse {
 	logger.Error(err, message)
-	return admissionutils.Response(errors.New(message + ": " + err.Error()))
+	return admissionutils.Response(uid, errors.New(message+": "+err.Error()))
 }
 
 func patchRequest(patches []byte, request *admissionv1.AdmissionRequest, logger logr.Logger) *admissionv1.AdmissionRequest {