chore: Simplify admission webhook setup (#1633)

.testcoverage.yml

@@ -45,7 +45,7 @@ override:
     path: ^internal/resourcelock$
   - threshold: 84
     path: ^internal/webhookcert$
-  - threshold: 87
+  - threshold: 85
     path: ^webhook/logpipeline/v1alpha1$
   - threshold: 84
     path: ^webhook/logpipeline/v1beta1$

main.go

@@ -322,15 +322,8 @@ func run() error {
 		return fmt.Errorf("failed to enable webhook server: %w", err)
 	}
 
-	mgr.GetWebhookServer().Register("/validate-logpipeline", &webhook.Admission{
-		Handler: logpipelinewebhookv1alpha1.NewValidatingWebhookHandler(mgr.GetClient(), scheme),
-	})
-	mgr.GetWebhookServer().Register("/validate-logparser", &webhook.Admission{
-		Handler: logparserwebhookv1alpha1.NewValidatingWebhookHandler(scheme),
-	})
-
-	if err := setupMutatingWebhooks(mgr); err != nil {
-		return fmt.Errorf("failed to setup mutating webhooks: %w", err)
+	if err := setupAdmissionsWebhooks(mgr); err != nil {
+		return fmt.Errorf("failed to setup admission webhooks: %w", err)
 	}
 
 	mgr.GetWebhookServer().Register("/api/v2/alerts", selfmonitorwebhook.NewHandler(
@@ -347,37 +340,39 @@ func run() error {
 	return nil
 }
 
-func setupMutatingWebhooks(mgr manager.Manager) error {
-	if err := metricpipelinewebhookv1alpha1.SetupMetricPipelineWebhookWithManager(mgr); err != nil {
+func setupAdmissionsWebhooks(mgr manager.Manager) error {
+	if err := metricpipelinewebhookv1alpha1.SetupWithManager(mgr); err != nil {
 		return fmt.Errorf("failed to setup metric pipeline v1alpha1 webhook: %w", err)
 	}
 
 	if featureflags.IsEnabled(featureflags.V1Beta1) {
-		if err := metricpipelinewebhookv1beta1.SetupMetricPipelineWebhookWithManager(mgr); err != nil {
+		if err := metricpipelinewebhookv1beta1.SetupWithManager(mgr); err != nil {
 			return fmt.Errorf("failed to setup metric pipeline v1beta1 webhook: %w", err)
 		}
 	}
 
-	if err := tracepipelinewebhookv1alpha1.SetupTracePipelineWebhookWithManager(mgr); err != nil {
+	if err := tracepipelinewebhookv1alpha1.SetupWithManager(mgr); err != nil {
 		return fmt.Errorf("failed to setup trace pipeline v1alpha1 webhook: %w", err)
 	}
 
 	if featureflags.IsEnabled(featureflags.V1Beta1) {
-		if err := tracepipelinewebhookv1beta1.SetupTracePipelineWebhookWithManager(mgr); err != nil {
+		if err := tracepipelinewebhookv1beta1.SetupWithManager(mgr); err != nil {
 			return fmt.Errorf("failed to setup trace pipeline v1beta1 webhook: %w", err)
 		}
 	}
 
-	if err := logpipelinewebhookv1alpha1.SetupLogPipelineWebhookWithManager(mgr); err != nil {
+	if err := logpipelinewebhookv1alpha1.SetupWithManager(mgr); err != nil {
 		return fmt.Errorf("failed to setup log pipeline v1alpha1 webhook: %w", err)
 	}
 
 	if featureflags.IsEnabled(featureflags.V1Beta1) {
-		if err := logpipelinewebhookv1beta1.SetupLogPipelineWebhookWithManager(mgr); err != nil {
+		if err := logpipelinewebhookv1beta1.SetupWithManager(mgr); err != nil {
 			return fmt.Errorf("failed to setup log pipeline v1beta1 webhook: %w", err)
 		}
 	}
 
+	logparserwebhookv1alpha1.SetupWithManager(mgr)
+
 	return nil
 }
 

webhook/logparser/v1alpha1/setup.go

@@ -0,0 +1,12 @@
+package v1alpha1
+
+import (
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/webhook"
+)
+
+func SetupWithManager(mgr ctrl.Manager) {
+	mgr.GetWebhookServer().Register("/validate-logparser", &webhook.Admission{
+		Handler: newValidateHandler(mgr.GetScheme()),
+	})
+}

webhook/logparser/v1alpha1/webhook.go → webhook/logparser/v1alpha1/validate_handler.go

@@ -11,17 +11,17 @@ import (
 	telemetryv1alpha1 "github.com/kyma-project/telemetry-manager/apis/telemetry/v1alpha1"
 )
 
-type ValidatingWebhookHandler struct {
+type validateHandler struct {
 	decoder admission.Decoder
 }
 
-func NewValidatingWebhookHandler(scheme *runtime.Scheme) *ValidatingWebhookHandler {
-	return &ValidatingWebhookHandler{
+func newValidateHandler(scheme *runtime.Scheme) *validateHandler {
+	return &validateHandler{
 		decoder: admission.NewDecoder(scheme),
 	}
 }
 
-func (v *ValidatingWebhookHandler) Handle(ctx context.Context, req admission.Request) admission.Response {
+func (v *validateHandler) Handle(ctx context.Context, req admission.Request) admission.Response {
 	log := logf.FromContext(ctx)
 
 	logParser := &telemetryv1alpha1.LogParser{}

webhook/logpipeline/v1alpha1/defaulter.go

@@ -5,30 +5,20 @@ import (
 	"fmt"
 
 	"k8s.io/apimachinery/pkg/runtime"
-	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 
 	telemetryv1alpha1 "github.com/kyma-project/telemetry-manager/apis/telemetry/v1alpha1"
 )
 
 // +kubebuilder:object:generate=false
-var _ webhook.CustomDefaulter = &LogPipelineDefaulter{}
+var _ webhook.CustomDefaulter = &defaulter{}
 
-type LogPipelineDefaulter struct {
+type defaulter struct {
 	ApplicationInputEnabled          bool
 	ApplicationInputKeepOriginalBody bool
 }
 
-func SetupLogPipelineWebhookWithManager(mgr ctrl.Manager) error {
-	return ctrl.NewWebhookManagedBy(mgr).For(&telemetryv1alpha1.LogPipeline{}).
-		WithDefaulter(&LogPipelineDefaulter{
-			ApplicationInputEnabled:          true,
-			ApplicationInputKeepOriginalBody: true,
-		}).
-		Complete()
-}
-
-func (ld LogPipelineDefaulter) Default(ctx context.Context, obj runtime.Object) error {
+func (ld defaulter) Default(ctx context.Context, obj runtime.Object) error {
 	pipeline, ok := obj.(*telemetryv1alpha1.LogPipeline)
 	if !ok {
 		return fmt.Errorf("expected an LogPipeline object but got %T", obj)
@@ -39,7 +29,7 @@ func (ld LogPipelineDefaulter) Default(ctx context.Context, obj runtime.Object)
 	return nil
 }
 
-func (ld LogPipelineDefaulter) applyDefaults(pipeline *telemetryv1alpha1.LogPipeline) {
+func (ld defaulter) applyDefaults(pipeline *telemetryv1alpha1.LogPipeline) {
 	if pipeline.Spec.Input.Application != nil {
 		if pipeline.Spec.Input.Application.Enabled == nil {
 			pipeline.Spec.Input.Application.Enabled = &ld.ApplicationInputEnabled

webhook/logpipeline/v1alpha1/defaulter_test.go

@@ -11,7 +11,7 @@ import (
 )
 
 func TestDefault(t *testing.T) {
-	defaulter := LogPipelineDefaulter{
+	sut := defaulter{
 		ApplicationInputEnabled:          true,
 		ApplicationInputKeepOriginalBody: true,
 	}
@@ -67,7 +67,7 @@ func TestDefault(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			err := defaulter.Default(context.Background(), tt.input)
+			err := sut.Default(context.Background(), tt.input)
 			assert.NoError(t, err)
 			assert.Equal(t, tt.expected, tt.input)
 		})

webhook/logpipeline/v1alpha1/files_validator_test.go

@@ -23,7 +23,7 @@ func TestDuplicateFileName(t *testing.T) {
 
 	fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(&existingPipeline).Build()
 
-	sut := NewValidatingWebhookHandler(fakeClient, scheme)
+	sut := newValidateHandler(fakeClient, scheme)
 
 	newPipeline := testutils.NewLogPipelineBuilder().WithName("bar").WithFile("f1.json", "").Build()
 
@@ -41,7 +41,7 @@ func TestDuplicateFileNameInSamePipeline(t *testing.T) {
 
 	fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects().Build()
 
-	sut := NewValidatingWebhookHandler(fakeClient, scheme)
+	sut := newValidateHandler(fakeClient, scheme)
 
 	newPipeline := testutils.NewLogPipelineBuilder().WithName("foo").WithFile("f1.json", "").WithFile("f1.json", "").Build()
 
@@ -61,7 +61,7 @@ func TestValidateUpdatePipeline(t *testing.T) {
 
 	fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(&existingPipeline).Build()
 
-	sut := NewValidatingWebhookHandler(fakeClient, scheme)
+	sut := newValidateHandler(fakeClient, scheme)
 
 	newPipeline := testutils.NewLogPipelineBuilder().WithName("foo").WithFile("f1.json", "").Build()
 

webhook/logpipeline/v1alpha1/max_pipelines_validator_test.go

@@ -23,7 +23,7 @@ func TestValidateFirstPipeline(t *testing.T) {
 
 	fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects().Build()
 
-	sut := NewValidatingWebhookHandler(fakeClient, scheme)
+	sut := newValidateHandler(fakeClient, scheme)
 
 	newPipeline := testutils.NewLogPipelineBuilder().Build()
 
@@ -46,7 +46,7 @@ func TestValidateLimitNotExceeded(t *testing.T) {
 
 	fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(existingPipelines...).Build()
 
-	sut := NewValidatingWebhookHandler(fakeClient, scheme)
+	sut := newValidateHandler(fakeClient, scheme)
 
 	newPipeline := testutils.NewLogPipelineBuilder().Build()
 
@@ -69,7 +69,7 @@ func TestValidateLimitExceeded(t *testing.T) {
 
 	fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(existingPipelines...).Build()
 
-	sut := NewValidatingWebhookHandler(fakeClient, scheme)
+	sut := newValidateHandler(fakeClient, scheme)
 
 	newPipeline := testutils.NewLogPipelineBuilder().Build()
 
@@ -89,7 +89,7 @@ func TestValidateUpdate(t *testing.T) {
 
 	fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(&existingPipeline).Build()
 
-	sut := NewValidatingWebhookHandler(fakeClient, scheme)
+	sut := newValidateHandler(fakeClient, scheme)
 
 	response := sut.Handle(context.Background(), admissionRequestFrom(t, existingPipeline))
 

webhook/logpipeline/v1alpha1/setup.go

@@ -0,0 +1,25 @@
+package v1alpha1
+
+import (
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/webhook"
+
+	telemetryv1alpha1 "github.com/kyma-project/telemetry-manager/apis/telemetry/v1alpha1"
+)
+
+func SetupWithManager(mgr ctrl.Manager) error {
+	if err := ctrl.NewWebhookManagedBy(mgr).For(&telemetryv1alpha1.LogPipeline{}).
+		WithDefaulter(&defaulter{
+			ApplicationInputEnabled:          true,
+			ApplicationInputKeepOriginalBody: true,
+		}).
+		Complete(); err != nil {
+		return err
+	}
+
+	mgr.GetWebhookServer().Register("/validate-logpipeline", &webhook.Admission{
+		Handler: newValidateHandler(mgr.GetClient(), mgr.GetScheme()),
+	})
+
+	return nil
+}

webhook/logpipeline/v1alpha1/spec_validator_test.go

@@ -181,7 +181,7 @@ func TestValidateLogPipelineSpec(t *testing.T) {
 
 			fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects().Build()
 
-			sut := NewValidatingWebhookHandler(fakeClient, scheme)
+			sut := newValidateHandler(fakeClient, scheme)
 
 			response := sut.Handle(context.Background(), admissionRequestFrom(t, *tt.logPipeline))
 

webhook/logpipeline/v1alpha1/handler.go → webhook/logpipeline/v1alpha1/validate_handler.go

@@ -15,22 +15,22 @@ import (
 	logpipelineutils "github.com/kyma-project/telemetry-manager/internal/utils/logpipeline"
 )
 
-type ValidatingWebhookHandler struct {
+type validateHandler struct {
 	client  client.Client
 	decoder admission.Decoder
 }
 
-func NewValidatingWebhookHandler(
+func newValidateHandler(
 	client client.Client,
 	scheme *runtime.Scheme,
-) *ValidatingWebhookHandler {
-	return &ValidatingWebhookHandler{
+) *validateHandler {
+	return &validateHandler{
 		client:  client,
 		decoder: admission.NewDecoder(scheme),
 	}
 }
 
-func (h *ValidatingWebhookHandler) Handle(ctx context.Context, req admission.Request) admission.Response {
+func (h *validateHandler) Handle(ctx context.Context, req admission.Request) admission.Response {
 	log := logf.FromContext(ctx)
 
 	logPipeline := &telemetryv1alpha1.LogPipeline{}
@@ -64,7 +64,7 @@ func (h *ValidatingWebhookHandler) Handle(ctx context.Context, req admission.Req
 	return admission.Allowed("LogPipeline validation successful")
 }
 
-func (h *ValidatingWebhookHandler) validateLogPipeline(ctx context.Context, logPipeline *telemetryv1alpha1.LogPipeline) error {
+func (h *validateHandler) validateLogPipeline(ctx context.Context, logPipeline *telemetryv1alpha1.LogPipeline) error {
 	log := logf.FromContext(ctx)
 
 	var logPipelines telemetryv1alpha1.LogPipelineList

webhook/logpipeline/v1alpha1/handler_test.go → webhook/logpipeline/v1alpha1/validate_handler_test.go

@@ -27,7 +27,7 @@ func TestHandle(t *testing.T) {
 		logPipeline := testutils.NewLogPipelineBuilder().WithName("custom-output").WithCustomOutput("Name stdout").Build()
 		fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects().Build()
 
-		sut := NewValidatingWebhookHandler(fakeClient, scheme)
+		sut := newValidateHandler(fakeClient, scheme)
 
 		response := sut.Handle(context.Background(), admissionRequestFrom(t, logPipeline))
 
@@ -97,7 +97,7 @@ func TestHandle(t *testing.T) {
 
 				fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects().Build()
 
-				sut := NewValidatingWebhookHandler(fakeClient, scheme)
+				sut := newValidateHandler(fakeClient, scheme)
 
 				response := sut.Handle(context.Background(), admissionRequestFrom(t, logPipeline))
 				require.Equal(t, tt.allowed, response.Allowed)

webhook/logpipeline/v1alpha1/variable_validator_test.go

@@ -27,7 +27,7 @@ func TestVariableNotGloballyUnique(t *testing.T) {
 
 	fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(&existingPipeline).Build()
 
-	sut := NewValidatingWebhookHandler(fakeClient, scheme)
+	sut := newValidateHandler(fakeClient, scheme)
 
 	newPipeline := testutils.NewLogPipelineBuilder().
 		WithName("log-pipeline-2").
@@ -48,7 +48,7 @@ func TestVariableValidator(t *testing.T) {
 
 	fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects().Build()
 
-	sut := NewValidatingWebhookHandler(fakeClient, scheme)
+	sut := newValidateHandler(fakeClient, scheme)
 
 	newPipeline := testutils.NewLogPipelineBuilder().
 		WithName("log-pipeline-2").

webhook/logpipeline/v1beta1/defaulter.go

@@ -5,32 +5,21 @@ import (
 	"fmt"
 
 	"k8s.io/apimachinery/pkg/runtime"
-	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 
 	telemetryv1beta1 "github.com/kyma-project/telemetry-manager/apis/telemetry/v1beta1"
 )
 
 // +kubebuilder:object:generate=false
-var _ webhook.CustomDefaulter = &LogPipelineDefaulter{}
+var _ webhook.CustomDefaulter = &defaulter{}
 
-type LogPipelineDefaulter struct {
+type defaulter struct {
 	RuntimeInputEnabled          bool
 	RuntimeInputKeepOriginalBody bool
 	DefaultOTLPOutputProtocol    telemetryv1beta1.OTLPProtocol
 }
 
-func SetupLogPipelineWebhookWithManager(mgr ctrl.Manager) error {
-	return ctrl.NewWebhookManagedBy(mgr).For(&telemetryv1beta1.LogPipeline{}).
-		WithDefaulter(&LogPipelineDefaulter{
-			RuntimeInputEnabled:          true,
-			RuntimeInputKeepOriginalBody: true,
-			DefaultOTLPOutputProtocol:    telemetryv1beta1.OTLPProtocolGRPC,
-		}).
-		Complete()
-}
-
-func (ld LogPipelineDefaulter) Default(ctx context.Context, obj runtime.Object) error {
+func (ld defaulter) Default(ctx context.Context, obj runtime.Object) error {
 	pipeline, ok := obj.(*telemetryv1beta1.LogPipeline)
 	if !ok {
 		return fmt.Errorf("expected an LogPipeline object but got %T", obj)
@@ -41,7 +30,7 @@ func (ld LogPipelineDefaulter) Default(ctx context.Context, obj runtime.Object)
 	return nil
 }
 
-func (ld LogPipelineDefaulter) applyDefaults(pipeline *telemetryv1beta1.LogPipeline) {
+func (ld defaulter) applyDefaults(pipeline *telemetryv1beta1.LogPipeline) {
 	if pipeline.Spec.Input.Runtime != nil {
 		if pipeline.Spec.Input.Runtime.Enabled == nil {
 			pipeline.Spec.Input.Runtime.Enabled = &ld.RuntimeInputEnabled