From f4c2f2420836eae61139da44cdd2f29b554b8c93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?= Date: Wed, 23 Nov 2022 07:50:55 +0100 Subject: [PATCH] feat: improve handlers tracing code (#5442) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: improve handlers tracing code Signed-off-by: Charles-Edouard Brétéché * fix Signed-off-by: Charles-Edouard Brétéché * fix cleanup Signed-off-by: Charles-Edouard Brétéché Signed-off-by: Charles-Edouard Brétéché --- cmd/cleanup-controller/server.go | 8 +- pkg/tracing/tracing.go | 60 ++++++++++++++ pkg/webhooks/handlers/admission.go | 36 +++------ pkg/webhooks/handlers/dump.go | 41 ++++------ pkg/webhooks/handlers/filter.go | 25 ++---- pkg/webhooks/handlers/metrics.go | 23 ++---- pkg/webhooks/handlers/protect.go | 46 +++++------ pkg/webhooks/handlers/trace.go | 69 ++++++++++++++-- pkg/webhooks/handlers/types.go | 8 ++ pkg/webhooks/handlers/utils.go | 18 ++--- pkg/webhooks/handlers/verify.go | 33 +++----- pkg/webhooks/server.go | 122 ++++++++++++++++------------- 12 files changed, 279 insertions(+), 210 deletions(-) diff --git a/cmd/cleanup-controller/server.go b/cmd/cleanup-controller/server.go index f70ea6d047..51247899c0 100644 --- a/cmd/cleanup-controller/server.go +++ b/cmd/cleanup-controller/server.go @@ -44,11 +44,9 @@ func NewServer( mux.HandlerFunc( "POST", ValidatingWebhookServicePath, - http.HandlerFunc( - handlers.AdmissionHandler(policyHandlers.Validate). - WithAdmission(logger.Logger.WithName("validate")). - WithTrace(), - ), + handlers.FromAdmissionFunc("VALIDATE", policyHandlers.Validate). + WithAdmission(logger.Logger.WithName("validate")). + ToHandlerFunc(), ) return &server{ server: &http.Server{ diff --git a/pkg/tracing/tracing.go b/pkg/tracing/tracing.go index b7981f994b..14dc74931c 100644 --- a/pkg/tracing/tracing.go +++ b/pkg/tracing/tracing.go @@ -2,12 +2,14 @@ package tracing import ( "context" + "net/http" "time" "github.com/go-logr/logr" "github.com/kyverno/kyverno/pkg/utils/kube" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/exporters/otlp/otlptrace" "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc" "go.opentelemetry.io/otel/propagation" @@ -18,6 +20,51 @@ import ( "k8s.io/client-go/kubernetes" ) +const ( + PolicyGroupKey = attribute.Key("kyverno.policy.group") + PolicyVersionKey = attribute.Key("kyverno.policy.version") + PolicyKindKey = attribute.Key("kyverno.policy.kind") + PolicyNameKey = attribute.Key("kyverno.policy.name") + PolicyNamespaceKey = attribute.Key("kyverno.policy.namespace") + RuleNameKey = attribute.Key("kyverno.rule.name") + // ResourceNameKey = attribute.Key("admission.resource.name") + // ResourceNamespaceKey = attribute.Key("admission.resource.namespace") + // ResourceGroupKey = attribute.Key("admission.resource.group") + // ResourceVersionKey = attribute.Key("admission.resource.version") + // ResourceKindKey = attribute.Key("admission.resource.kind") + // ResourceUidKey = attribute.Key("admission.resource.uid") + RequestNameKey = attribute.Key("admission.request.name") + RequestNamespaceKey = attribute.Key("admission.request.namespace") + RequestUidKey = attribute.Key("admission.request.uid") + RequestOperationKey = attribute.Key("admission.request.operation") + RequestDryRunKey = attribute.Key("admission.request.dryrun") + RequestKindGroupKey = attribute.Key("admission.request.kind.group") + RequestKindVersionKey = attribute.Key("admission.request.kind.version") + RequestKindKindKey = attribute.Key("admission.request.kind.kind") + RequestSubResourceKey = attribute.Key("admission.request.subresource") + RequestRequestKindGroupKey = attribute.Key("admission.request.requestkind.group") + RequestRequestKindVersionKey = attribute.Key("admission.request.requestkind.version") + RequestRequestKindKindKey = attribute.Key("admission.request.requestkind.kind") + RequestRequestSubResourceKey = attribute.Key("admission.request.requestsubresource") + RequestResourceGroupKey = attribute.Key("admission.request.resource.group") + RequestResourceVersionKey = attribute.Key("admission.request.resource.version") + RequestResourceResourceKey = attribute.Key("admission.request.resource.resource") + RequestRequestResourceGroupKey = attribute.Key("admission.request.requestresource.group") + RequestRequestResourceVersionKey = attribute.Key("admission.request.requestresource.version") + RequestRequestResourceResourceKey = attribute.Key("admission.request.requestresource.resource") + RequestUserNameKey = attribute.Key("admission.request.user.name") + RequestUserUidKey = attribute.Key("admission.request.user.uid") + RequestUserGroupsKey = attribute.Key("admission.request.user.groups") + ResponseUidKey = attribute.Key("admission.response.uid") + ResponseAllowedKey = attribute.Key("admission.response.allowed") + ResponseWarningsKey = attribute.Key("admission.response.warnings") + ResponseResultStatusKey = attribute.Key("admission.response.result.status") + ResponseResultMessageKey = attribute.Key("admission.response.result.message") + ResponseResultReasonKey = attribute.Key("admission.response.result.reason") + ResponseResultCodeKey = attribute.Key("admission.response.result.code") + ResponsePatchTypeKey = attribute.Key("admission.response.patchtype") +) + // NewTraceConfig generates the initial tracing configuration with 'address' as the endpoint to connect to the Opentelemetry Collector func NewTraceConfig(log logr.Logger, name, address, certs string, kubeClient kubernetes.Interface) (func(), error) { ctx := context.Background() @@ -104,3 +151,16 @@ func Span1[T any](ctx context.Context, tracerName string, operationName string, defer span.End() return doFn(newCtx, span) } + +func SetHttpStatus(ctx context.Context, err error, code int) { + span := trace.SpanFromContext(ctx) + if err != nil { + span.RecordError(err) + } + span.SetAttributes(semconv.HTTPStatusCodeKey.Int(code)) + if code >= 400 { + span.SetStatus(codes.Error, http.StatusText(code)) + } else { + span.SetStatus(codes.Ok, http.StatusText(code)) + } +} diff --git a/pkg/webhooks/handlers/admission.go b/pkg/webhooks/handlers/admission.go index 3859220af2..dec7e3ac5f 100644 --- a/pkg/webhooks/handlers/admission.go +++ b/pkg/webhooks/handlers/admission.go @@ -2,45 +2,40 @@ package handlers import ( "encoding/json" - "fmt" + "errors" "io" "net/http" "time" "github.com/go-logr/logr" - "github.com/kyverno/kyverno/pkg/tracing" admissionv1 "k8s.io/api/admission/v1" ) -func (h AdmissionHandler) WithAdmission(logger logr.Logger) HttpHandler { - return withAdmission(logger, h) +func (inner AdmissionHandler) WithAdmission(logger logr.Logger) HttpHandler { + return inner.withAdmission(logger).WithTrace("ADMISSION") } -func withAdmission(logger logr.Logger, inner AdmissionHandler) HttpHandler { +func (inner AdmissionHandler) withAdmission(logger logr.Logger) HttpHandler { return func(writer http.ResponseWriter, request *http.Request) { startTime := time.Now() if request.Body == nil { - logger.Info("empty body", "req", request.URL.String()) - http.Error(writer, "empty body", http.StatusBadRequest) + httpError(writer, request, logger, errors.New("empty body"), http.StatusBadRequest) return } defer request.Body.Close() body, err := io.ReadAll(request.Body) if err != nil { - logger.Info("failed to read HTTP body", "req", request.URL.String()) - http.Error(writer, "failed to read HTTP body", http.StatusBadRequest) + httpError(writer, request, logger, err, http.StatusBadRequest) return } contentType := request.Header.Get("Content-Type") if contentType != "application/json" { - logger.Info("invalid Content-Type", "contentType", contentType) - http.Error(writer, "invalid Content-Type, expect `application/json`", http.StatusUnsupportedMediaType) + httpError(writer, request, logger, errors.New("invalid Content-Type"), http.StatusUnsupportedMediaType) return } admissionReview := &admissionv1.AdmissionReview{} if err := json.Unmarshal(body, &admissionReview); err != nil { - logger.Error(err, "failed to decode request body to type 'AdmissionReview") - http.Error(writer, "Can't decode body as AdmissionReview", http.StatusExpectationFailed) + httpError(writer, request, logger, err, http.StatusExpectationFailed) return } logger := logger.WithValues( @@ -55,26 +50,19 @@ func withAdmission(logger logr.Logger, inner AdmissionHandler) HttpHandler { Allowed: true, UID: admissionReview.Request.UID, } - // start span from request context - ctx, span := tracing.StartSpan( - request.Context(), - "webhooks/handlers", - fmt.Sprintf("ADMISSION %s %s", admissionReview.Request.Operation, admissionReview.Request.Kind), - admissionRequestAttributes(admissionReview.Request)..., - ) - defer span.End() - admissionResponse := inner(ctx, logger, admissionReview.Request, startTime) + admissionResponse := inner(request.Context(), logger, admissionReview.Request, startTime) if admissionResponse != nil { admissionReview.Response = admissionResponse } responseJSON, err := json.Marshal(admissionReview) if err != nil { - http.Error(writer, fmt.Sprintf("Could not encode response: %v", err), http.StatusInternalServerError) + httpError(writer, request, logger, err, http.StatusInternalServerError) return } writer.Header().Set("Content-Type", "application/json; charset=utf-8") if _, err := writer.Write(responseJSON); err != nil { - http.Error(writer, fmt.Sprintf("could not write response: %v", err), http.StatusInternalServerError) + httpError(writer, request, logger, err, http.StatusInternalServerError) + return } if admissionReview.Request.Kind.Kind == "Lease" { logger.V(6).Info("admission review request processed", "time", time.Since(startTime).String()) diff --git a/pkg/webhooks/handlers/dump.go b/pkg/webhooks/handlers/dump.go index cf5dbe3c6b..308936c2ae 100644 --- a/pkg/webhooks/handlers/dump.go +++ b/pkg/webhooks/handlers/dump.go @@ -2,15 +2,12 @@ package handlers import ( "context" - "fmt" "strings" "time" "github.com/go-logr/logr" engineutils "github.com/kyverno/kyverno/pkg/engine/utils" - "github.com/kyverno/kyverno/pkg/tracing" "github.com/kyverno/kyverno/pkg/utils" - "go.opentelemetry.io/otel/trace" admissionv1 "k8s.io/api/admission/v1" authenticationv1 "k8s.io/api/authentication/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -18,6 +15,21 @@ import ( "k8s.io/apimachinery/pkg/types" ) +func (inner AdmissionHandler) WithDump(enabled bool) AdmissionHandler { + if !enabled { + return inner + } + return inner.withDump().WithTrace("DUMP") +} + +func (inner AdmissionHandler) withDump() AdmissionHandler { + return func(ctx context.Context, logger logr.Logger, request *admissionv1.AdmissionRequest, startTime time.Time) *admissionv1.AdmissionResponse { + response := inner(ctx, logger, request, startTime) + dumpPayload(logger, request, response) + return response + } +} + func dumpPayload(logger logr.Logger, request *admissionv1.AdmissionRequest, response *admissionv1.AdmissionResponse) { reqPayload, err := newAdmissionRequestPayload(request) if err != nil { @@ -96,26 +108,3 @@ func redactPayload(payload *admissionRequestPayload) (*admissionRequestPayload, } return payload, nil } - -func (h AdmissionHandler) WithDump(enabled bool) AdmissionHandler { - if !enabled { - return h - } - return withDump(h) -} - -func withDump(inner AdmissionHandler) AdmissionHandler { - return func(ctx context.Context, logger logr.Logger, request *admissionv1.AdmissionRequest, startTime time.Time) *admissionv1.AdmissionResponse { - return tracing.Span1( - ctx, - "webhooks/handlers", - fmt.Sprintf("DUMP %s %s", request.Operation, request.Kind), - func(ctx context.Context, span trace.Span) *admissionv1.AdmissionResponse { - response := inner(ctx, logger, request, startTime) - dumpPayload(logger, request, response) - return response - }, - trace.WithAttributes(admissionRequestAttributes(request)...), - ) - } -} diff --git a/pkg/webhooks/handlers/filter.go b/pkg/webhooks/handlers/filter.go index 784b706c66..dd30549cf0 100644 --- a/pkg/webhooks/handlers/filter.go +++ b/pkg/webhooks/handlers/filter.go @@ -2,33 +2,22 @@ package handlers import ( "context" - "fmt" "time" "github.com/go-logr/logr" "github.com/kyverno/kyverno/pkg/config" - "github.com/kyverno/kyverno/pkg/tracing" - "go.opentelemetry.io/otel/trace" admissionv1 "k8s.io/api/admission/v1" ) -func (h AdmissionHandler) WithFilter(configuration config.Configuration) AdmissionHandler { - return withFilter(configuration, h) +func (inner AdmissionHandler) WithFilter(configuration config.Configuration) AdmissionHandler { + return inner.withFilter(configuration).WithTrace("FILTER") } -func withFilter(c config.Configuration, inner AdmissionHandler) AdmissionHandler { +func (inner AdmissionHandler) withFilter(c config.Configuration) AdmissionHandler { return func(ctx context.Context, logger logr.Logger, request *admissionv1.AdmissionRequest, startTime time.Time) *admissionv1.AdmissionResponse { - return tracing.Span1( - ctx, - "webhooks/handlers", - fmt.Sprintf("FILTER %s %s", request.Operation, request.Kind), - func(ctx context.Context, span trace.Span) *admissionv1.AdmissionResponse { - if c.ToFilter(request.Kind.Kind, request.Namespace, request.Name) { - return nil - } - return inner(ctx, logger, request, startTime) - }, - trace.WithAttributes(admissionRequestAttributes(request)...), - ) + if c.ToFilter(request.Kind.Kind, request.Namespace, request.Name) { + return nil + } + return inner(ctx, logger, request, startTime) } } diff --git a/pkg/webhooks/handlers/metrics.go b/pkg/webhooks/handlers/metrics.go index 648cb88bb3..5aa34eb0b8 100644 --- a/pkg/webhooks/handlers/metrics.go +++ b/pkg/webhooks/handlers/metrics.go @@ -2,34 +2,23 @@ package handlers import ( "context" - "fmt" "time" "github.com/go-logr/logr" "github.com/kyverno/kyverno/pkg/metrics" admissionRequests "github.com/kyverno/kyverno/pkg/metrics/admissionrequests" admissionReviewDuration "github.com/kyverno/kyverno/pkg/metrics/admissionreviewduration" - "github.com/kyverno/kyverno/pkg/tracing" - "go.opentelemetry.io/otel/trace" admissionv1 "k8s.io/api/admission/v1" ) -func (h AdmissionHandler) WithMetrics(metricsConfig *metrics.MetricsConfig) AdmissionHandler { - return withMetrics(metricsConfig, h) +func (inner AdmissionHandler) WithMetrics(metricsConfig *metrics.MetricsConfig) AdmissionHandler { + return inner.withMetrics(metricsConfig).WithTrace("METRICS") } -func withMetrics(metricsConfig *metrics.MetricsConfig, inner AdmissionHandler) AdmissionHandler { +func (inner AdmissionHandler) withMetrics(metricsConfig *metrics.MetricsConfig) AdmissionHandler { return func(ctx context.Context, logger logr.Logger, request *admissionv1.AdmissionRequest, startTime time.Time) *admissionv1.AdmissionResponse { - return tracing.Span1( - ctx, - "webhooks/handlers", - fmt.Sprintf("METRICS %s %s", request.Operation, request.Kind), - func(ctx context.Context, span trace.Span) *admissionv1.AdmissionResponse { - defer admissionReviewDuration.Process(metricsConfig, request, int64(time.Since(startTime))) - admissionRequests.Process(metricsConfig, request) - return inner(ctx, logger, request, startTime) - }, - trace.WithAttributes(admissionRequestAttributes(request)...), - ) + defer admissionReviewDuration.Process(metricsConfig, request, int64(time.Since(startTime))) + admissionRequests.Process(metricsConfig, request) + return inner(ctx, logger, request, startTime) } } diff --git a/pkg/webhooks/handlers/protect.go b/pkg/webhooks/handlers/protect.go index c6cd3e7cb2..9c297fea70 100644 --- a/pkg/webhooks/handlers/protect.go +++ b/pkg/webhooks/handlers/protect.go @@ -9,45 +9,35 @@ import ( "github.com/go-logr/logr" kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" "github.com/kyverno/kyverno/pkg/config" - "github.com/kyverno/kyverno/pkg/tracing" "github.com/kyverno/kyverno/pkg/utils" admissionutils "github.com/kyverno/kyverno/pkg/utils/admission" - "go.opentelemetry.io/otel/trace" admissionv1 "k8s.io/api/admission/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) -func (h AdmissionHandler) WithProtection(enabled bool) AdmissionHandler { +func (inner AdmissionHandler) WithProtection(enabled bool) AdmissionHandler { if !enabled { - return h + return inner } - return withProtection(h) + return inner.withProtection().WithTrace("PROTECT") } -func withProtection(inner AdmissionHandler) AdmissionHandler { +func (inner AdmissionHandler) withProtection() AdmissionHandler { return func(ctx context.Context, logger logr.Logger, request *admissionv1.AdmissionRequest, startTime time.Time) *admissionv1.AdmissionResponse { - return tracing.Span1( - ctx, - "webhooks/handlers", - fmt.Sprintf("PROTECT %s %s", request.Operation, request.Kind), - func(ctx context.Context, span trace.Span) *admissionv1.AdmissionResponse { - newResource, oldResource, err := utils.ExtractResources(nil, request) - if err != nil { - logger.Error(err, "Failed to extract resources") - return admissionutils.Response(err) + newResource, oldResource, err := utils.ExtractResources(nil, request) + if err != nil { + logger.Error(err, "Failed to extract resources") + return admissionutils.Response(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")) } - 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 inner(ctx, logger, request, startTime) - }, - trace.WithAttributes(admissionRequestAttributes(request)...), - ) + } + } + return inner(ctx, logger, request, startTime) } } diff --git a/pkg/webhooks/handlers/trace.go b/pkg/webhooks/handlers/trace.go index 53b05ade78..472dd95c36 100644 --- a/pkg/webhooks/handlers/trace.go +++ b/pkg/webhooks/handlers/trace.go @@ -4,22 +4,21 @@ import ( "context" "fmt" "net/http" + "time" + "github.com/go-logr/logr" "github.com/kyverno/kyverno/pkg/tracing" semconv "go.opentelemetry.io/otel/semconv/v1.4.0" "go.opentelemetry.io/otel/trace" + admissionv1 "k8s.io/api/admission/v1" ) -func (h HttpHandler) WithTrace() HttpHandler { - return withTrace(h) -} - -func withTrace(inner HttpHandler) HttpHandler { +func (inner HttpHandler) WithTrace(name string) HttpHandler { return func(writer http.ResponseWriter, request *http.Request) { tracing.Span( request.Context(), "webhooks/handlers", - fmt.Sprintf("HTTP %s %s", request.Method, request.URL.Path), + fmt.Sprintf("%s %s %s", name, request.Method, request.URL.Path), func(ctx context.Context, span trace.Span) { inner(writer, request.WithContext(ctx)) }, @@ -33,3 +32,61 @@ func withTrace(inner HttpHandler) HttpHandler { ) } } + +func (inner AdmissionHandler) WithTrace(name string) AdmissionHandler { + return func(ctx context.Context, logger logr.Logger, request *admissionv1.AdmissionRequest, startTime time.Time) *admissionv1.AdmissionResponse { + return tracing.Span1( + ctx, + "webhooks/handlers", + fmt.Sprintf("%s %s %s", name, request.Operation, request.Kind), + func(ctx context.Context, span trace.Span) *admissionv1.AdmissionResponse { + response := inner(ctx, logger, request, startTime) + if response != nil { + span.SetAttributes( + tracing.ResponseUidKey.String(string(response.UID)), + tracing.ResponseAllowedKey.Bool(response.Allowed), + tracing.ResponseWarningsKey.StringSlice(response.Warnings), + ) + if response.Result != nil { + span.SetAttributes( + tracing.ResponseResultStatusKey.String(response.Result.Status), + tracing.ResponseResultMessageKey.String(response.Result.Message), + tracing.ResponseResultReasonKey.String(string(response.Result.Reason)), + tracing.ResponseResultCodeKey.Int(int(response.Result.Code)), + ) + } + if response.PatchType != nil { + span.SetAttributes( + tracing.ResponsePatchTypeKey.String(string(*response.PatchType)), + ) + } + } + return response + }, + trace.WithAttributes( + tracing.RequestNameKey.String(request.Name), + tracing.RequestNamespaceKey.String(request.Namespace), + tracing.RequestUidKey.String(string(request.UID)), + tracing.RequestOperationKey.String(string(request.Operation)), + tracing.RequestDryRunKey.Bool(request.DryRun != nil && *request.DryRun), + tracing.RequestKindGroupKey.String(request.Kind.Group), + tracing.RequestKindVersionKey.String(request.Kind.Version), + tracing.RequestKindKindKey.String(request.Kind.Kind), + tracing.RequestSubResourceKey.String(request.SubResource), + tracing.RequestRequestKindGroupKey.String(request.RequestKind.Group), + tracing.RequestRequestKindVersionKey.String(request.RequestKind.Version), + tracing.RequestRequestKindKindKey.String(request.RequestKind.Kind), + tracing.RequestRequestSubResourceKey.String(request.RequestSubResource), + tracing.RequestResourceGroupKey.String(request.Resource.Group), + tracing.RequestResourceVersionKey.String(request.Resource.Version), + tracing.RequestResourceResourceKey.String(request.Resource.Resource), + tracing.RequestRequestResourceGroupKey.String(request.RequestResource.Group), + tracing.RequestRequestResourceVersionKey.String(request.RequestResource.Version), + tracing.RequestRequestResourceResourceKey.String(request.RequestResource.Resource), + tracing.RequestUserNameKey.String(request.UserInfo.Username), + tracing.RequestUserUidKey.String(request.UserInfo.UID), + tracing.RequestUserGroupsKey.StringSlice(request.UserInfo.Groups), + ), + ) + } +} diff --git a/pkg/webhooks/handlers/types.go b/pkg/webhooks/handlers/types.go index 003aa3cd7c..9489dc2663 100644 --- a/pkg/webhooks/handlers/types.go +++ b/pkg/webhooks/handlers/types.go @@ -13,3 +13,11 @@ type ( AdmissionHandler func(context.Context, logr.Logger, *admissionv1.AdmissionRequest, time.Time) *admissionv1.AdmissionResponse HttpHandler func(http.ResponseWriter, *http.Request) ) + +func FromAdmissionFunc(name string, h AdmissionHandler) AdmissionHandler { + return h.WithTrace(name) +} + +func (h HttpHandler) ToHandlerFunc() http.HandlerFunc { + return http.HandlerFunc(h) +} diff --git a/pkg/webhooks/handlers/utils.go b/pkg/webhooks/handlers/utils.go index 4121e0a239..0abfa98c5a 100644 --- a/pkg/webhooks/handlers/utils.go +++ b/pkg/webhooks/handlers/utils.go @@ -1,16 +1,14 @@ package handlers import ( - "go.opentelemetry.io/otel/attribute" - admissionv1 "k8s.io/api/admission/v1" + "net/http" + + "github.com/go-logr/logr" + "github.com/kyverno/kyverno/pkg/tracing" ) -func admissionRequestAttributes(request *admissionv1.AdmissionRequest) []attribute.KeyValue { - return []attribute.KeyValue{ - attribute.String("kind", request.Kind.Kind), - attribute.String("namespace", request.Namespace), - attribute.String("name", request.Name), - attribute.String("operation", string(request.Operation)), - attribute.String("uid", string(request.UID)), - } +func httpError(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) + http.Error(writer, err.Error(), code) } diff --git a/pkg/webhooks/handlers/verify.go b/pkg/webhooks/handlers/verify.go index 7dfdd0a27b..d07821d7fa 100644 --- a/pkg/webhooks/handlers/verify.go +++ b/pkg/webhooks/handlers/verify.go @@ -2,37 +2,24 @@ package handlers import ( "context" - "fmt" "time" "github.com/go-logr/logr" "github.com/kyverno/kyverno/pkg/config" - "github.com/kyverno/kyverno/pkg/tracing" admissionutils "github.com/kyverno/kyverno/pkg/utils/admission" jsonutils "github.com/kyverno/kyverno/pkg/utils/json" - "go.opentelemetry.io/otel/trace" admissionv1 "k8s.io/api/admission/v1" ) -func Verify() AdmissionHandler { - return func(ctx context.Context, logger logr.Logger, request *admissionv1.AdmissionRequest, startTime time.Time) *admissionv1.AdmissionResponse { - return tracing.Span1( - ctx, - "webhooks/handlers", - fmt.Sprintf("VERIFY %s %s", request.Operation, request.Kind), - func(ctx context.Context, span trace.Span) *admissionv1.AdmissionResponse { - if request.Name != "kyverno-health" || request.Namespace != config.KyvernoNamespace() { - return admissionutils.ResponseSuccess() - } - 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.MutationResponse(bytes) - }, - trace.WithAttributes(admissionRequestAttributes(request)...), - ) +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() } + 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.MutationResponse(bytes) } diff --git a/pkg/webhooks/server.go b/pkg/webhooks/server.go index ed8513cdb6..0d50541f2f 100644 --- a/pkg/webhooks/server.go +++ b/pkg/webhooks/server.go @@ -79,40 +79,52 @@ func NewServer( resourceLogger := logger.WithName("resource") policyLogger := logger.WithName("policy") verifyLogger := logger.WithName("verify") - registerWebhookHandlers(resourceLogger.WithName("mutate"), mux, config.MutatingWebhookServicePath, configuration, metricsConfig, resourceHandlers.Mutate, debugModeOpts) - registerWebhookHandlers(resourceLogger.WithName("validate"), mux, config.ValidatingWebhookServicePath, configuration, metricsConfig, resourceHandlers.Validate, debugModeOpts) + registerWebhookHandlers( + resourceLogger.WithName("mutate"), + mux, + "MUTATE", + config.MutatingWebhookServicePath, + configuration, + metricsConfig, + resourceHandlers.Mutate, + debugModeOpts, + ) + registerWebhookHandlers( + resourceLogger.WithName("validate"), + mux, + "VALIDATE", + config.ValidatingWebhookServicePath, + configuration, + metricsConfig, + resourceHandlers.Validate, + debugModeOpts, + ) mux.HandlerFunc( "POST", config.PolicyMutatingWebhookServicePath, - http.HandlerFunc( - handlers.AdmissionHandler(policyHandlers.Mutate). - WithFilter(configuration). - WithDump(debugModeOpts.DumpPayload). - WithMetrics(metricsConfig). - WithAdmission(policyLogger.WithName("mutate")). - WithTrace(), - ), + handlers.FromAdmissionFunc("MUTATE", policyHandlers.Mutate). + WithFilter(configuration). + WithDump(debugModeOpts.DumpPayload). + WithMetrics(metricsConfig). + WithAdmission(policyLogger.WithName("mutate")). + ToHandlerFunc(), ) mux.HandlerFunc( "POST", config.PolicyValidatingWebhookServicePath, - http.HandlerFunc( - handlers.AdmissionHandler(policyHandlers.Validate). - WithFilter(configuration). - WithDump(debugModeOpts.DumpPayload). - WithMetrics(metricsConfig). - WithAdmission(policyLogger.WithName("validate")). - WithTrace(), - ), + handlers.FromAdmissionFunc("VALIDATE", policyHandlers.Validate). + WithFilter(configuration). + WithDump(debugModeOpts.DumpPayload). + WithMetrics(metricsConfig). + WithAdmission(policyLogger.WithName("validate")). + ToHandlerFunc(), ) mux.HandlerFunc( "POST", config.VerifyMutatingWebhookServicePath, - http.HandlerFunc( - handlers.Verify(). - WithAdmission(verifyLogger.WithName("mutate")). - WithTrace(), - ), + handlers.FromAdmissionFunc("VERIFY", handlers.Verify). + WithAdmission(verifyLogger.WithName("mutate")). + ToHandlerFunc(), ) mux.HandlerFunc("GET", config.LivenessServicePath, handlers.Probe(runtime.IsLive)) mux.HandlerFunc("GET", config.ReadinessServicePath, handlers.Probe(runtime.IsReady)) @@ -205,6 +217,7 @@ func (s *server) cleanup(ctx context.Context) { func registerWebhookHandlers( logger logr.Logger, mux *httprouter.Router, + name string, basePath string, configuration config.Configuration, metricsConfig *metrics.MetricsConfig, @@ -214,46 +227,49 @@ func registerWebhookHandlers( mux.HandlerFunc( "POST", basePath, - http.HandlerFunc( - handlers.AdmissionHandler(func(ctx context.Context, logger logr.Logger, request *admissionv1.AdmissionRequest, startTime time.Time) *admissionv1.AdmissionResponse { + handlers.FromAdmissionFunc( + name, + func(ctx context.Context, logger logr.Logger, request *admissionv1.AdmissionRequest, startTime time.Time) *admissionv1.AdmissionResponse { return handlerFunc(ctx, logger, request, "all", startTime) - }). - WithFilter(configuration). - WithProtection(toggle.ProtectManagedResources.Enabled()). - WithDump(debugModeOpts.DumpPayload). - WithMetrics(metricsConfig). - WithAdmission(logger). - WithTrace(), - ), + }, + ). + WithFilter(configuration). + WithProtection(toggle.ProtectManagedResources.Enabled()). + WithDump(debugModeOpts.DumpPayload). + WithMetrics(metricsConfig). + WithAdmission(logger). + ToHandlerFunc(), ) mux.HandlerFunc( "POST", basePath+"/fail", - http.HandlerFunc( - handlers.AdmissionHandler(func(ctx context.Context, logger logr.Logger, request *admissionv1.AdmissionRequest, startTime time.Time) *admissionv1.AdmissionResponse { + handlers.FromAdmissionFunc( + name, + func(ctx context.Context, logger logr.Logger, request *admissionv1.AdmissionRequest, startTime time.Time) *admissionv1.AdmissionResponse { return handlerFunc(ctx, logger, request, "fail", startTime) - }). - WithFilter(configuration). - WithProtection(toggle.ProtectManagedResources.Enabled()). - WithDump(debugModeOpts.DumpPayload). - WithMetrics(metricsConfig). - WithAdmission(logger). - WithTrace(), - ), + }, + ). + WithFilter(configuration). + WithProtection(toggle.ProtectManagedResources.Enabled()). + WithDump(debugModeOpts.DumpPayload). + WithMetrics(metricsConfig). + WithAdmission(logger). + ToHandlerFunc(), ) mux.HandlerFunc( "POST", basePath+"/ignore", - http.HandlerFunc( - handlers.AdmissionHandler(func(ctx context.Context, logger logr.Logger, request *admissionv1.AdmissionRequest, startTime time.Time) *admissionv1.AdmissionResponse { + handlers.FromAdmissionFunc( + name, + func(ctx context.Context, logger logr.Logger, request *admissionv1.AdmissionRequest, startTime time.Time) *admissionv1.AdmissionResponse { return handlerFunc(ctx, logger, request, "ignore", startTime) - }). - WithFilter(configuration). - WithProtection(toggle.ProtectManagedResources.Enabled()). - WithDump(debugModeOpts.DumpPayload). - WithMetrics(metricsConfig). - WithAdmission(logger). - WithTrace(), - ), + }, + ). + WithFilter(configuration). + WithProtection(toggle.ProtectManagedResources.Enabled()). + WithDump(debugModeOpts.DumpPayload). + WithMetrics(metricsConfig). + WithAdmission(logger). + ToHandlerFunc(), ) }