Skip to content

Commit

Permalink
webhook: switch to contextual logger
Browse files Browse the repository at this point in the history
Use k8s contextual logger instead of own. Comparing to other parts
of the operator webhook is a bit special since it serves own
endpoints and has context independent from controllers.

Include more info (operation), just to get more details. Do not add
kind since it's clear from "resource" field.

Since controller-framework adds "admission" to the name, use own
LogConstructor with own base logger for the framework's
DefaultLogConstructor.

Add name locally to distinguish from framework's messages.

Add Name field to the structures to avoid copying string literal for
both WithName() and WithValues().

The output changes and it looks like

```
{"level":"info","ts":"2024-10-11T05:17:20Z","logger":"ValidatingWebhook","msg":"Validation request","object":{"name":"default-dsci"},"namespace":"","name":"default-dsci","resource":{"group":"dscinitialization.opendatahub.io","version":"v1","resource":"dscinitializations"},"user":"kube:admin","requestID":"e5bf3768-6faa-4e14-9004-e54ee84ad8b7","webhook":"ValidatingWebhook","operation":"CREATE"}
```

or for the defaulter:

```
{"level":"info","ts":"2024-10-11T04:50:48Z","logger":"DSCDefaulter","msg":"Defaulting DSC","object":{"name":"default-dsc"},"namespace":"","name":"default-dsc","resource":{"group":"datasciencecluster.opendatahub.io","version":"v1","resource":"datascienceclusters"},"user":"kube:admin","requestID":"c9213ff3-80ee-40c0-9f15-12188dece68e","webhook":"DSCDefaulter"}
```

(the messages are not from the current codebase, was added for demo only)

Signed-off-by: Yauheni Kaliuta <[email protected]>
  • Loading branch information
ykaliuta committed Oct 11, 2024
1 parent e8e266f commit 7943dc0
Showing 1 changed file with 31 additions and 5 deletions.
36 changes: 31 additions & 5 deletions controllers/webhook/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,15 @@ import (
"fmt"
"net/http"

"github.com/go-logr/logr"
operatorv1 "github.com/openshift/api/operator/v1"
admissionv1 "k8s.io/api/admission/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/webhook"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"

Expand All @@ -38,29 +40,44 @@ import (
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk"
)

var log = ctrl.Log.WithName("odh-controller-webhook")

//+kubebuilder:webhook:path=/validate-opendatahub-io-v1,mutating=false,failurePolicy=fail,sideEffects=None,groups=datasciencecluster.opendatahub.io;dscinitialization.opendatahub.io,resources=datascienceclusters;dscinitializations,verbs=create;delete,versions=v1,name=operator.opendatahub.io,admissionReviewVersions=v1
//nolint:lll

// TODO: Get rid of platform in name, rename to ValidatingWebhook.
type OpenDataHubValidatingWebhook struct {
Client client.Client
Decoder *admission.Decoder
Name string
}

func Init(mgr ctrl.Manager) {
(&OpenDataHubValidatingWebhook{
Client: mgr.GetClient(),
Decoder: admission.NewDecoder(mgr.GetScheme()),
Name: "ValidatingWebhook",
}).SetupWithManager(mgr)

(&DSCDefaulter{
Name: "DefaultingWebhook",
}).SetupWithManager(mgr)
}

(&DSCDefaulter{}).SetupWithManager(mgr)
// newLogConstructor creates a new logger constructor for a webhook.
// It is based on the root controller-runtime logger witch is set in main.go
// The purpose of it is to remove "admission" from the log name.
func newLogConstructor(name string) func(logr.Logger, *admission.Request) logr.Logger {
return func(_ logr.Logger, req *admission.Request) logr.Logger {
base := ctrl.Log
l := admission.DefaultLogConstructor(base, req)
return l.WithValues("webhook", name)
}
}

func (w *OpenDataHubValidatingWebhook) SetupWithManager(mgr ctrl.Manager) {
hookServer := mgr.GetWebhookServer()
odhWebhook := &webhook.Admission{
Handler: w,
Handler: w,
LogConstructor: newLogConstructor(w.Name),
}
hookServer.Register("/validate-opendatahub-io-v1", odhWebhook)
}
Expand Down Expand Up @@ -90,6 +107,8 @@ func denyCountGtZero(ctx context.Context, cli client.Client, gvk schema.GroupVer
}

func (w *OpenDataHubValidatingWebhook) checkDupCreation(ctx context.Context, req admission.Request) admission.Response {
log := logf.FromContext(ctx)

switch req.Kind.Kind {
case "DataScienceCluster", "DSCInitialization":
default:
Expand Down Expand Up @@ -119,6 +138,9 @@ func (w *OpenDataHubValidatingWebhook) checkDeletion(ctx context.Context, req ad
}

func (w *OpenDataHubValidatingWebhook) Handle(ctx context.Context, req admission.Request) admission.Response {
log := logf.FromContext(ctx).WithName(w.Name).WithValues("operation", req.Operation)
ctx = logf.IntoContext(ctx, log)

var resp admission.Response
resp.Allowed = true // initialize Allowed to be true in case Operation falls into "default" case

Expand All @@ -141,19 +163,23 @@ func (w *OpenDataHubValidatingWebhook) Handle(ctx context.Context, req admission
//+kubebuilder:webhook:path=/mutate-opendatahub-io-v1,mutating=true,failurePolicy=fail,sideEffects=None,groups=datasciencecluster.opendatahub.io,resources=datascienceclusters,verbs=create;update,versions=v1,name=mutate.operator.opendatahub.io,admissionReviewVersions=v1
//nolint:lll

type DSCDefaulter struct{}
type DSCDefaulter struct {
Name string
}

// just assert that DSCDefaulter implements webhook.CustomDefaulter.
var _ webhook.CustomDefaulter = &DSCDefaulter{}

func (m *DSCDefaulter) SetupWithManager(mgr ctrl.Manager) {
mutateWebhook := admission.WithCustomDefaulter(mgr.GetScheme(), &dscv1.DataScienceCluster{}, m)
mutateWebhook.LogConstructor = newLogConstructor(m.Name)
mgr.GetWebhookServer().Register("/mutate-opendatahub-io-v1", mutateWebhook)
}

// Implement admission.CustomDefaulter interface.
// It currently only sets defaults for modelregiestry in datascienceclusters.
func (m *DSCDefaulter) Default(_ context.Context, obj runtime.Object) error {
// TODO: add debug logging, log := logf.FromContext(ctx).WithName(m.Name)
dsc, isDSC := obj.(*dscv1.DataScienceCluster)
if !isDSC {
return fmt.Errorf("expected DataScienceCluster but got a different type: %T", obj)
Expand Down

0 comments on commit 7943dc0

Please sign in to comment.