From a5e623f9392a9635d0ab297a91f5c95615562c26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?= Date: Thu, 12 May 2022 17:47:44 +0200 Subject: [PATCH] refactor: webhooks server logger (#3904) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Charles-Edouard Brétéché Co-authored-by: shuting --- pkg/webhooks/handlers.go | 35 +++++++++--------------------- pkg/webhooks/handlers/admission.go | 21 +++++------------- pkg/webhooks/server.go | 23 +++++++++++--------- 3 files changed, 29 insertions(+), 50 deletions(-) diff --git a/pkg/webhooks/handlers.go b/pkg/webhooks/handlers.go index 7c80a25245..d00a61d0b0 100644 --- a/pkg/webhooks/handlers.go +++ b/pkg/webhooks/handlers.go @@ -42,26 +42,14 @@ func errorResponse(logger logr.Logger, err error, message string) *admissionv1.A return admissionutils.ResponseFailure(false, message+": "+err.Error()) } -func setupLogger(logger logr.Logger, name string, request *admissionv1.AdmissionRequest) logr.Logger { - return logger.WithName(name).WithValues( - "uid", request.UID, - "kind", request.Kind, - "namespace", request.Namespace, - "name", request.Name, - "operation", request.Operation, - "gvk", request.Kind.String(), - ) -} - -func (ws *WebhookServer) admissionHandler(filter bool, inner handlers.AdmissionHandler) http.HandlerFunc { +func (ws *WebhookServer) admissionHandler(logger logr.Logger, filter bool, inner handlers.AdmissionHandler) http.HandlerFunc { if filter { - inner = handlers.Filter(ws.configHandler, inner) + inner = handlers.Filter(ws.configuration, inner) } - return handlers.Monitor(ws.webhookMonitor, handlers.Admission(ws.log.WithName("handlerFunc"), inner)) + return handlers.Monitor(ws.webhookMonitor, handlers.Admission(logger, inner)) } -func (ws *WebhookServer) policyMutation(request *admissionv1.AdmissionRequest) *admissionv1.AdmissionResponse { - logger := setupLogger(ws.log, "PolicyMutationWebhook", request) +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") @@ -86,8 +74,7 @@ func (ws *WebhookServer) policyMutation(request *admissionv1.AdmissionRequest) * } //policyValidation performs the validation check on policy resource -func (ws *WebhookServer) policyValidation(request *admissionv1.AdmissionRequest) *admissionv1.AdmissionResponse { - logger := setupLogger(ws.log, "PolicyValidationWebhook", request) +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") @@ -117,8 +104,7 @@ func (ws *WebhookServer) policyValidation(request *admissionv1.AdmissionRequest) } // resourceMutation mutates resource -func (ws *WebhookServer) resourceMutation(request *admissionv1.AdmissionRequest) *admissionv1.AdmissionResponse { - logger := setupLogger(ws.log, "ResourceMutationWebhook", request) +func (ws *WebhookServer) resourceMutation(logger logr.Logger, request *admissionv1.AdmissionRequest) *admissionv1.AdmissionResponse { if excludeKyvernoResources(request.Kind.Kind) { return admissionutils.ResponseSuccess(true, "") } @@ -174,8 +160,7 @@ func (ws *WebhookServer) resourceMutation(request *admissionv1.AdmissionRequest) return admissionutils.ResponseSuccessWithPatch(true, "", patches) } -func (ws *WebhookServer) resourceValidation(request *admissionv1.AdmissionRequest) *admissionv1.AdmissionResponse { - logger := setupLogger(ws.log, "ResourceValidationWebhook", request) +func (ws *WebhookServer) resourceValidation(logger logr.Logger, request *admissionv1.AdmissionRequest) *admissionv1.AdmissionResponse { if request.Operation == admissionv1.Delete { ws.handleDelete(request) } @@ -210,7 +195,7 @@ func (ws *WebhookServer) resourceValidation(request *admissionv1.AdmissionReques var roles, clusterRoles []string if containsRBACInfo(policies, generatePolicies) { var err error - roles, clusterRoles, err = userinfo.GetRoleRef(ws.rbLister, ws.crbLister, request, ws.configHandler) + roles, clusterRoles, err = userinfo.GetRoleRef(ws.rbLister, ws.crbLister, request, ws.configuration) if err != nil { return errorResponse(logger, err, "failed to fetch RBAC data") } @@ -245,8 +230,8 @@ func (ws *WebhookServer) resourceValidation(request *admissionv1.AdmissionReques NewResource: newResource, OldResource: oldResource, AdmissionInfo: userRequestInfo, - ExcludeGroupRole: ws.configHandler.GetExcludeGroupRole(), - ExcludeResourceFunc: ws.configHandler.ToFilter, + ExcludeGroupRole: ws.configuration.GetExcludeGroupRole(), + ExcludeResourceFunc: ws.configuration.ToFilter, JSONContext: ctx, Client: ws.client, AdmissionOperation: true, diff --git a/pkg/webhooks/handlers/admission.go b/pkg/webhooks/handlers/admission.go index 3a18b51031..417d32a931 100644 --- a/pkg/webhooks/handlers/admission.go +++ b/pkg/webhooks/handlers/admission.go @@ -14,7 +14,7 @@ import ( admissionv1 "k8s.io/api/admission/v1" ) -type AdmissionHandler func(*admissionv1.AdmissionRequest) *admissionv1.AdmissionResponse +type AdmissionHandler func(logr.Logger, *admissionv1.AdmissionRequest) *admissionv1.AdmissionResponse func Admission(logger logr.Logger, inner AdmissionHandler) http.HandlerFunc { return func(writer http.ResponseWriter, request *http.Request) { @@ -43,7 +43,7 @@ func Admission(logger logr.Logger, inner AdmissionHandler) http.HandlerFunc { http.Error(writer, "Can't decode body as AdmissionReview", http.StatusExpectationFailed) return } - logger = logger.WithValues( + logger := logger.WithValues( "kind", admissionReview.Request.Kind, "namespace", admissionReview.Request.Namespace, "name", admissionReview.Request.Name, @@ -54,7 +54,7 @@ func Admission(logger logr.Logger, inner AdmissionHandler) http.HandlerFunc { Allowed: true, UID: admissionReview.Request.UID, } - adminssionResponse := inner(admissionReview.Request) + adminssionResponse := inner(logger, admissionReview.Request) if adminssionResponse != nil { admissionReview.Response = adminssionResponse } @@ -67,7 +67,6 @@ func Admission(logger logr.Logger, inner AdmissionHandler) http.HandlerFunc { if _, err := writer.Write(responseJSON); err != nil { http.Error(writer, fmt.Sprintf("could not write response: %v", err), http.StatusInternalServerError) } - if admissionReview.Request.Kind.Kind == "Lease" { logger.V(6).Info("admission review request processed", "time", time.Since(startTime).String()) } else { @@ -77,24 +76,16 @@ func Admission(logger logr.Logger, inner AdmissionHandler) http.HandlerFunc { } func Filter(c config.Configuration, inner AdmissionHandler) AdmissionHandler { - return func(request *admissionv1.AdmissionRequest) *admissionv1.AdmissionResponse { + return func(logger logr.Logger, request *admissionv1.AdmissionRequest) *admissionv1.AdmissionResponse { if c.ToFilter(request.Kind.Kind, request.Namespace, request.Name) { return nil } - return inner(request) + return inner(logger, request) } } func Verify(m *webhookconfig.Monitor, logger logr.Logger) AdmissionHandler { - return func(request *admissionv1.AdmissionRequest) *admissionv1.AdmissionResponse { - logger = logger.WithValues( - "action", "verify", - "kind", request.Kind, - "namespace", request.Namespace, - "name", request.Name, - "operation", request.Operation, - "gvk", request.Kind.String(), - ) + return func(logger logr.Logger, request *admissionv1.AdmissionRequest) *admissionv1.AdmissionResponse { logger.V(6).Info("incoming request", "last admission request timestamp", m.Time()) return admissionutils.Response(true) } diff --git a/pkg/webhooks/server.go b/pkg/webhooks/server.go index 2c1804e4f1..e2f6e2eaf8 100644 --- a/pkg/webhooks/server.go +++ b/pkg/webhooks/server.go @@ -64,7 +64,7 @@ type WebhookServer struct { webhookRegister *webhookconfig.Register // helpers to validate against current loaded configuration - configHandler config.Configuration + configuration config.Configuration // channel for cleanup notification cleanUp chan<- struct{} @@ -133,7 +133,7 @@ func NewWebhookServer( eventGen: eventGen, pCache: pCache, webhookRegister: webhookRegistrationClient, - configHandler: configHandler, + configuration: configHandler, cleanUp: cleanUp, webhookMonitor: webhookMonitor, prGenerator: prGenerator, @@ -145,11 +145,14 @@ func NewWebhookServer( promConfig: promConfig, } mux := httprouter.New() - mux.HandlerFunc("POST", config.MutatingWebhookServicePath, ws.admissionHandler(true, ws.resourceMutation)) - mux.HandlerFunc("POST", config.ValidatingWebhookServicePath, ws.admissionHandler(true, ws.resourceValidation)) - mux.HandlerFunc("POST", config.PolicyMutatingWebhookServicePath, ws.admissionHandler(true, ws.policyMutation)) - mux.HandlerFunc("POST", config.PolicyValidatingWebhookServicePath, ws.admissionHandler(true, ws.policyValidation)) - mux.HandlerFunc("POST", config.VerifyMutatingWebhookServicePath, ws.admissionHandler(false, handlers.Verify(ws.webhookMonitor, ws.log.WithName("verifyHandler")))) + resourceLogger := ws.log.WithName("resource") + policyLogger := ws.log.WithName("policy") + 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.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)) ws.server = &http.Server{ @@ -182,7 +185,7 @@ func (ws *WebhookServer) buildPolicyContext(request *admissionv1.AdmissionReques if addRoles { var err error - userRequestInfo.Roles, userRequestInfo.ClusterRoles, err = userinfo.GetRoleRef(ws.rbLister, ws.crbLister, request, ws.configHandler) + userRequestInfo.Roles, userRequestInfo.ClusterRoles, err = userinfo.GetRoleRef(ws.rbLister, ws.crbLister, request, ws.configuration) if err != nil { return nil, errors.Wrap(err, "failed to fetch RBAC information for request") } @@ -205,8 +208,8 @@ func (ws *WebhookServer) buildPolicyContext(request *admissionv1.AdmissionReques policyContext := &engine.PolicyContext{ NewResource: resource, AdmissionInfo: userRequestInfo, - ExcludeGroupRole: ws.configHandler.GetExcludeGroupRole(), - ExcludeResourceFunc: ws.configHandler.ToFilter, + ExcludeGroupRole: ws.configuration.GetExcludeGroupRole(), + ExcludeResourceFunc: ws.configuration.ToFilter, JSONContext: ctx, Client: ws.client, AdmissionOperation: true,