Skip to content

Commit

Permalink
Merge pull request #2326 from sbueringer/pr-cleanup-webhook-logging
Browse files Browse the repository at this point in the history
✨ Cleanup webhook logging
  • Loading branch information
k8s-ci-robot committed May 16, 2023
2 parents af986b8 + 0f47d28 commit ca1cf6f
Show file tree
Hide file tree
Showing 6 changed files with 11 additions and 8 deletions.
1 change: 1 addition & 0 deletions pkg/builder/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ func (blder *WebhookBuilder) setLogConstructor() {
blder.gvk.Kind, klog.KRef(req.Namespace, req.Name),
"namespace", req.Namespace, "name", req.Name,
"resource", req.Resource, "user", req.UserInfo.Username,
"requestID", req.UID,
)
}
return log
Expand Down
6 changes: 3 additions & 3 deletions pkg/webhook/admission/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func (wh *Webhook) ServeHTTP(w http.ResponseWriter, r *http.Request) {
// verify the content type is accurate
if contentType := r.Header.Get("Content-Type"); contentType != "application/json" {
err = fmt.Errorf("contentType=%s, expected application/json", contentType)
wh.getLogger(nil).Error(err, "unable to process a request with an unknown content type", "content type", contentType)
wh.getLogger(nil).Error(err, "unable to process a request with unknown content type")
reviewResponse = Errored(http.StatusBadRequest, err)
wh.writeResponse(w, reviewResponse)
return
Expand All @@ -93,7 +93,7 @@ func (wh *Webhook) ServeHTTP(w http.ResponseWriter, r *http.Request) {
wh.writeResponse(w, reviewResponse)
return
}
wh.getLogger(nil).V(1).Info("received request", "UID", req.UID, "kind", req.Kind, "resource", req.Resource)
wh.getLogger(&req).V(1).Info("received request")

reviewResponse = wh.Handle(ctx, req)
wh.writeResponseTyped(w, reviewResponse, actualAdmRevGVK)
Expand Down Expand Up @@ -140,7 +140,7 @@ func (wh *Webhook) writeAdmissionResponse(w io.Writer, ar v1.AdmissionReview) {
if res.Result != nil {
log = log.WithValues("code", res.Result.Code, "reason", res.Result.Reason, "message", res.Result.Message)
}
log.V(1).Info("wrote response", "UID", res.UID, "allowed", res.Allowed)
log.V(1).Info("wrote response", "requestID", res.UID, "allowed", res.Allowed)
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/webhook/admission/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"k8s.io/apimachinery/pkg/util/json"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/klog/v2"

logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/webhook/internal/metrics"
)
Expand Down Expand Up @@ -163,7 +164,6 @@ func (wh *Webhook) Handle(ctx context.Context, req Request) (response Response)
}

reqLog := wh.getLogger(&req)
reqLog = reqLog.WithValues("requestID", req.UID)
ctx = logf.IntoContext(ctx, reqLog)

resp := wh.Handler.Handle(ctx, req)
Expand Down Expand Up @@ -196,6 +196,7 @@ func DefaultLogConstructor(base logr.Logger, req *Request) logr.Logger {
return base.WithValues("object", klog.KRef(req.Namespace, req.Name),
"namespace", req.Namespace, "name", req.Name,
"resource", req.Resource, "user", req.UserInfo.Username,
"requestID", req.UID,
)
}
return base
Expand Down
2 changes: 1 addition & 1 deletion pkg/webhook/admission/webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ var _ = Describe("Admission Webhooks", func() {
}
}),
LogConstructor: func(base logr.Logger, req *Request) logr.Logger {
return base.WithValues("operation", req.Operation)
return base.WithValues("operation", req.Operation, "requestID", req.UID)
},
log: testLogger,
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/webhook/authentication/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func (wh *Webhook) ServeHTTP(w http.ResponseWriter, r *http.Request) {
// verify the content type is accurate
if contentType := r.Header.Get("Content-Type"); contentType != "application/json" {
err = fmt.Errorf("contentType=%s, expected application/json", contentType)
wh.getLogger(nil).Error(err, "unable to process a request with an unknown content type", "content type", contentType)
wh.getLogger(nil).Error(err, "unable to process a request with unknown content type")
reviewResponse = Errored(err)
wh.writeResponse(w, reviewResponse)
return
Expand All @@ -94,7 +94,7 @@ func (wh *Webhook) ServeHTTP(w http.ResponseWriter, r *http.Request) {
wh.writeResponse(w, reviewResponse)
return
}
wh.getLogger(&req).V(1).Info("received request", "UID", req.UID, "kind", req.Kind)
wh.getLogger(&req).V(1).Info("received request")

if req.Spec.Token == "" {
err = errors.New("token is empty")
Expand Down Expand Up @@ -136,7 +136,7 @@ func (wh *Webhook) writeTokenResponse(w io.Writer, ar authenticationv1.TokenRevi
}
res := ar
if wh.getLogger(nil).V(1).Enabled() {
wh.getLogger(nil).V(1).Info("wrote response", "UID", res.UID, "authenticated", res.Status.Authenticated)
wh.getLogger(nil).V(1).Info("wrote response", "requestID", res.UID, "authenticated", res.Status.Authenticated)
}
}

Expand Down
1 change: 1 addition & 0 deletions pkg/webhook/authentication/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ func logConstructor(base logr.Logger, req *Request) logr.Logger {
return base.WithValues("object", klog.KRef(req.Namespace, req.Name),
"namespace", req.Namespace, "name", req.Name,
"user", req.Status.User.Username,
"requestID", req.UID,
)
}
return base
Expand Down

0 comments on commit ca1cf6f

Please sign in to comment.