From 87ac54856371fedea7f03ff5d8c5532a7a964284 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?=
<charled.breteche@gmail.com>
Date: Fri, 13 May 2022 07:33:20 +0200
Subject: [PATCH] refactor: separate policy mutation/validation handlers from
server (#3905)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* refactor: webhooks server logger
Signed-off-by: Charles-Edouard Brétéché <charled.breteche@gmail.com>
* refactor: separate policy mutation/validation handlers from server
Signed-off-by: Charles-Edouard Brétéché <charled.breteche@gmail.com>
Co-authored-by: Vyankatesh Kudtarkar <vyankateshkd@gmail.com>
---
cmd/kyverno/main.go | 4 ++
pkg/webhooks/handlers.go | 72 -------------------------
pkg/webhooks/policy/handlers.go | 95 +++++++++++++++++++++++++++++++++
pkg/webhooks/server.go | 12 ++++-
4 files changed, 109 insertions(+), 74 deletions(-)
create mode 100644 pkg/webhooks/policy/handlers.go
diff --git a/cmd/kyverno/main.go b/cmd/kyverno/main.go
index c6bc7cd24f..2a52fcea6b 100755
--- a/cmd/kyverno/main.go
+++ b/cmd/kyverno/main.go
@@ -37,6 +37,7 @@ import (
"github.com/kyverno/kyverno/pkg/version"
"github.com/kyverno/kyverno/pkg/webhookconfig"
"github.com/kyverno/kyverno/pkg/webhooks"
+ webhookspolicy "github.com/kyverno/kyverno/pkg/webhooks/policy"
webhookgenerate "github.com/kyverno/kyverno/pkg/webhooks/updaterequest"
"github.com/prometheus/client_golang/prometheus/promhttp"
kubeinformers "k8s.io/client-go/informers"
@@ -405,7 +406,10 @@ func main() {
// -- annotations on resources with update details on mutation JSON patches
// -- generate policy violation resource
// -- generate events on policy and resource
+ policyHandlers := webhookspolicy.NewHandlers(dynamicClient, openAPIController)
+
server, err := webhooks.NewWebhookServer(
+ policyHandlers,
kyvernoClient,
dynamicClient,
certManager.GetTLSPemPair,
diff --git a/pkg/webhooks/handlers.go b/pkg/webhooks/handlers.go
index d00a61d0b0..814acca678 100644
--- a/pkg/webhooks/handlers.go
+++ b/pkg/webhooks/handlers.go
@@ -3,8 +3,6 @@ package webhooks
import (
"fmt"
"net/http"
- "reflect"
- "strings"
"time"
"github.com/go-logr/logr"
@@ -13,9 +11,7 @@ import (
"github.com/kyverno/kyverno/pkg/common"
"github.com/kyverno/kyverno/pkg/engine"
enginectx "github.com/kyverno/kyverno/pkg/engine/context"
- policyvalidate "github.com/kyverno/kyverno/pkg/policy"
"github.com/kyverno/kyverno/pkg/policycache"
- "github.com/kyverno/kyverno/pkg/policymutation"
"github.com/kyverno/kyverno/pkg/userinfo"
"github.com/kyverno/kyverno/pkg/utils"
admissionutils "github.com/kyverno/kyverno/pkg/utils/admission"
@@ -23,20 +19,6 @@ import (
admissionv1 "k8s.io/api/admission/v1"
)
-// TODO: use admission review sub resource ?
-func isStatusUpdate(old, new kyverno.PolicyInterface) bool {
- if !reflect.DeepEqual(old.GetAnnotations(), new.GetAnnotations()) {
- return false
- }
- if !reflect.DeepEqual(old.GetLabels(), new.GetLabels()) {
- return false
- }
- if !reflect.DeepEqual(old.GetSpec(), new.GetSpec()) {
- return false
- }
- return true
-}
-
func errorResponse(logger logr.Logger, err error, message string) *admissionv1.AdmissionResponse {
logger.Error(err, message)
return admissionutils.ResponseFailure(false, message+": "+err.Error())
@@ -49,60 +31,6 @@ func (ws *WebhookServer) admissionHandler(logger logr.Logger, filter bool, inner
return handlers.Monitor(ws.webhookMonitor, handlers.Admission(logger, inner))
}
-func (ws *WebhookServer) policyMutation(logger logr.Logger, request *admissionv1.AdmissionRequest) *admissionv1.AdmissionResponse {
- policy, oldPolicy, err := admissionutils.GetPolicies(request)
- if err != nil {
- logger.Error(err, "failed to unmarshal policies from admission request")
- return admissionutils.ResponseWithMessage(true, fmt.Sprintf("failed to default value, check kyverno controller logs for details: %v", err))
- }
-
- if oldPolicy != nil && isStatusUpdate(oldPolicy, policy) {
- logger.V(4).Info("skip policy mutation on status update")
- return admissionutils.Response(true)
- }
-
- startTime := time.Now()
- logger.V(3).Info("start policy change mutation")
- defer logger.V(3).Info("finished policy change mutation", "time", time.Since(startTime).String())
-
- // Generate JSON Patches for defaults
- if patches, updateMsgs := policymutation.GenerateJSONPatchesForDefaults(policy, logger); len(patches) != 0 {
- return admissionutils.ResponseWithMessageAndPatch(true, strings.Join(updateMsgs, "'"), patches)
- }
-
- return admissionutils.Response(true)
-}
-
-//policyValidation performs the validation check on policy resource
-func (ws *WebhookServer) policyValidation(logger logr.Logger, request *admissionv1.AdmissionRequest) *admissionv1.AdmissionResponse {
- policy, oldPolicy, err := admissionutils.GetPolicies(request)
- if err != nil {
- logger.Error(err, "failed to unmarshal policies from admission request")
- return admissionutils.ResponseWithMessage(true, fmt.Sprintf("failed to validate policy, check kyverno controller logs for details: %v", err))
- }
-
- if oldPolicy != nil && isStatusUpdate(oldPolicy, policy) {
- logger.V(4).Info("skip policy validation on status update")
- return admissionutils.Response(true)
- }
-
- startTime := time.Now()
- logger.V(3).Info("start policy change validation")
- defer logger.V(3).Info("finished policy change validation", "time", time.Since(startTime).String())
-
- response, err := policyvalidate.Validate(policy, ws.client, false, ws.openAPIController)
- if err != nil {
- logger.Error(err, "policy validation errors")
- return admissionutils.ResponseWithMessage(false, err.Error())
- }
-
- if response != nil && len(response.Warnings) != 0 {
- return response
- }
-
- return admissionutils.Response(true)
-}
-
// resourceMutation mutates resource
func (ws *WebhookServer) resourceMutation(logger logr.Logger, request *admissionv1.AdmissionRequest) *admissionv1.AdmissionResponse {
if excludeKyvernoResources(request.Kind.Kind) {
diff --git a/pkg/webhooks/policy/handlers.go b/pkg/webhooks/policy/handlers.go
new file mode 100644
index 0000000000..0c197c79cc
--- /dev/null
+++ b/pkg/webhooks/policy/handlers.go
@@ -0,0 +1,95 @@
+package policy
+
+import (
+ "fmt"
+ "reflect"
+ "strings"
+ "time"
+
+ "github.com/go-logr/logr"
+ kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
+ client "github.com/kyverno/kyverno/pkg/dclient"
+ "github.com/kyverno/kyverno/pkg/openapi"
+ policyvalidate "github.com/kyverno/kyverno/pkg/policy"
+ "github.com/kyverno/kyverno/pkg/policymutation"
+ admissionutils "github.com/kyverno/kyverno/pkg/utils/admission"
+ admissionv1 "k8s.io/api/admission/v1"
+)
+
+type Handlers interface {
+ // Mutate performs the mutation of policy resources
+ Mutate(logr.Logger, *admissionv1.AdmissionRequest) *admissionv1.AdmissionResponse
+ // Validate performs the validation check on policy resources
+ Validate(logr.Logger, *admissionv1.AdmissionRequest) *admissionv1.AdmissionResponse
+}
+
+type handlers struct {
+ client client.Interface
+ openAPIController *openapi.Controller
+}
+
+func NewHandlers(
+ client client.Interface,
+ openAPIController *openapi.Controller,
+) Handlers {
+ return &handlers{
+ client: client,
+ openAPIController: openAPIController,
+ }
+}
+
+func (h *handlers) Validate(logger logr.Logger, request *admissionv1.AdmissionRequest) *admissionv1.AdmissionResponse {
+ policy, oldPolicy, err := admissionutils.GetPolicies(request)
+ if err != nil {
+ logger.Error(err, "failed to unmarshal policies from admission request")
+ return admissionutils.ResponseWithMessage(true, fmt.Sprintf("failed to validate policy, check kyverno controller logs for details: %v", err))
+ }
+ if oldPolicy != nil && isStatusUpdate(oldPolicy, policy) {
+ logger.V(4).Info("skip policy validation on status update")
+ return admissionutils.Response(true)
+ }
+ startTime := time.Now()
+ logger.V(3).Info("start policy change validation")
+ defer logger.V(3).Info("finished policy change validation", "time", time.Since(startTime).String())
+ response, err := policyvalidate.Validate(policy, h.client, false, h.openAPIController)
+ if err != nil {
+ logger.Error(err, "policy validation errors")
+ return admissionutils.ResponseWithMessage(false, err.Error())
+ }
+ if response != nil && len(response.Warnings) != 0 {
+ return response
+ }
+ return admissionutils.Response(true)
+}
+
+func (h *handlers) Mutate(logger logr.Logger, request *admissionv1.AdmissionRequest) *admissionv1.AdmissionResponse {
+ policy, oldPolicy, err := admissionutils.GetPolicies(request)
+ if err != nil {
+ logger.Error(err, "failed to unmarshal policies from admission request")
+ return admissionutils.ResponseWithMessage(true, fmt.Sprintf("failed to default value, check kyverno controller logs for details: %v", err))
+ }
+ if oldPolicy != nil && isStatusUpdate(oldPolicy, policy) {
+ logger.V(4).Info("skip policy mutation on status update")
+ return admissionutils.Response(true)
+ }
+ startTime := time.Now()
+ logger.V(3).Info("start policy change mutation")
+ defer logger.V(3).Info("finished policy change mutation", "time", time.Since(startTime).String())
+ if patches, updateMsgs := policymutation.GenerateJSONPatchesForDefaults(policy, logger); len(patches) != 0 {
+ return admissionutils.ResponseWithMessageAndPatch(true, strings.Join(updateMsgs, "'"), patches)
+ }
+ return admissionutils.Response(true)
+}
+
+func isStatusUpdate(old, new kyvernov1.PolicyInterface) bool {
+ if !reflect.DeepEqual(old.GetAnnotations(), new.GetAnnotations()) {
+ return false
+ }
+ if !reflect.DeepEqual(old.GetLabels(), new.GetLabels()) {
+ return false
+ }
+ if !reflect.DeepEqual(old.GetSpec(), new.GetSpec()) {
+ return false
+ }
+ return true
+}
diff --git a/pkg/webhooks/server.go b/pkg/webhooks/server.go
index e2f6e2eaf8..fea0fbfe5a 100644
--- a/pkg/webhooks/server.go
+++ b/pkg/webhooks/server.go
@@ -38,6 +38,13 @@ import (
rbaclister "k8s.io/client-go/listers/rbac/v1"
)
+type Handlers interface {
+ // Mutate performs the mutation of policy resources
+ Mutate(logr.Logger, *admissionv1.AdmissionRequest) *admissionv1.AdmissionResponse
+ // Validate performs the validation check on policy resources
+ Validate(logr.Logger, *admissionv1.AdmissionRequest) *admissionv1.AdmissionResponse
+}
+
// WebhookServer contains configured TLS server with MutationWebhook.
type WebhookServer struct {
server *http.Server
@@ -94,6 +101,7 @@ type WebhookServer struct {
// NewWebhookServer creates new instance of WebhookServer accordingly to given configuration
// Policy Controller and Kubernetes Client should be initialized in configuration
func NewWebhookServer(
+ policyHandlers Handlers,
kyvernoClient kyvernoclient.Interface,
client client.Interface,
tlsPair func() ([]byte, []byte, error),
@@ -150,8 +158,8 @@ func NewWebhookServer(
verifyLogger := ws.log.WithName("verify")
mux.HandlerFunc("POST", config.MutatingWebhookServicePath, ws.admissionHandler(resourceLogger.WithName("mutate"), true, ws.resourceMutation))
mux.HandlerFunc("POST", config.ValidatingWebhookServicePath, ws.admissionHandler(resourceLogger.WithName("validate"), true, ws.resourceValidation))
- mux.HandlerFunc("POST", config.PolicyMutatingWebhookServicePath, ws.admissionHandler(policyLogger.WithName("mutate"), true, ws.policyMutation))
- mux.HandlerFunc("POST", config.PolicyValidatingWebhookServicePath, ws.admissionHandler(policyLogger.WithName("validate"), true, ws.policyValidation))
+ mux.HandlerFunc("POST", config.PolicyMutatingWebhookServicePath, ws.admissionHandler(policyLogger.WithName("mutate"), true, policyHandlers.Mutate))
+ mux.HandlerFunc("POST", config.PolicyValidatingWebhookServicePath, ws.admissionHandler(policyLogger.WithName("validate"), true, policyHandlers.Validate))
mux.HandlerFunc("POST", config.VerifyMutatingWebhookServicePath, ws.admissionHandler(verifyLogger.WithName("mutate"), false, handlers.Verify(ws.webhookMonitor, ws.log.WithName("verifyHandler"))))
mux.HandlerFunc("GET", config.LivenessServicePath, handlers.Probe(ws.webhookRegister.Check))
mux.HandlerFunc("GET", config.ReadinessServicePath, handlers.Probe(nil))