Revert "chore: Simplify admission webhook setup (#1633)"

This reverts commit 5032815.
kyma-project · Jan 14, 2025 · a03d0a0 · a03d0a0
1 parent aaef6ca
commit a03d0a0
Show file tree

Hide file tree

Showing 28 changed files with 162 additions and 214 deletions.
diff --git a/.testcoverage.yml b/.testcoverage.yml
@@ -45,7 +45,7 @@ override:
     path: ^internal/resourcelock$
   - threshold: 84
     path: ^internal/webhookcert$
-  - threshold: 85
+  - threshold: 87
     path: ^webhook/logpipeline/v1alpha1$
   - threshold: 84
     path: ^webhook/logpipeline/v1beta1$

diff --git a/main.go b/main.go
@@ -322,8 +322,15 @@ func run() error {
 		return fmt.Errorf("failed to enable webhook server: %w", err)
 	}
 
-	if err := setupAdmissionsWebhooks(mgr); err != nil {
-		return fmt.Errorf("failed to setup admission webhooks: %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)
 	}
 
 	mgr.GetWebhookServer().Register("/api/v2/alerts", selfmonitorwebhook.NewHandler(
@@ -340,39 +347,37 @@ func run() error {
 	return nil
 }
 
-func setupAdmissionsWebhooks(mgr manager.Manager) error {
-	if err := metricpipelinewebhookv1alpha1.SetupWithManager(mgr); err != nil {
+func setupMutatingWebhooks(mgr manager.Manager) error {
+	if err := metricpipelinewebhookv1alpha1.SetupMetricPipelineWebhookWithManager(mgr); err != nil {
 		return fmt.Errorf("failed to setup metric pipeline v1alpha1 webhook: %w", err)
 	}
 
 	if featureflags.IsEnabled(featureflags.V1Beta1) {
-		if err := metricpipelinewebhookv1beta1.SetupWithManager(mgr); err != nil {
+		if err := metricpipelinewebhookv1beta1.SetupMetricPipelineWebhookWithManager(mgr); err != nil {
 			return fmt.Errorf("failed to setup metric pipeline v1beta1 webhook: %w", err)
 		}
 	}
 
-	if err := tracepipelinewebhookv1alpha1.SetupWithManager(mgr); err != nil {
+	if err := tracepipelinewebhookv1alpha1.SetupTracePipelineWebhookWithManager(mgr); err != nil {
 		return fmt.Errorf("failed to setup trace pipeline v1alpha1 webhook: %w", err)
 	}
 
 	if featureflags.IsEnabled(featureflags.V1Beta1) {
-		if err := tracepipelinewebhookv1beta1.SetupWithManager(mgr); err != nil {
+		if err := tracepipelinewebhookv1beta1.SetupTracePipelineWebhookWithManager(mgr); err != nil {
 			return fmt.Errorf("failed to setup trace pipeline v1beta1 webhook: %w", err)
 		}
 	}
 
-	if err := logpipelinewebhookv1alpha1.SetupWithManager(mgr); err != nil {
+	if err := logpipelinewebhookv1alpha1.SetupLogPipelineWebhookWithManager(mgr); err != nil {
 		return fmt.Errorf("failed to setup log pipeline v1alpha1 webhook: %w", err)
 	}
 
 	if featureflags.IsEnabled(featureflags.V1Beta1) {
-		if err := logpipelinewebhookv1beta1.SetupWithManager(mgr); err != nil {
+		if err := logpipelinewebhookv1beta1.SetupLogPipelineWebhookWithManager(mgr); err != nil {
 			return fmt.Errorf("failed to setup log pipeline v1beta1 webhook: %w", err)
 		}
 	}
 
-	logparserwebhookv1alpha1.SetupWithManager(mgr)
-
 	return nil
 }
 

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

diff --git a/webhook/logpipeline/v1alpha1/defaulter.go b/webhook/logpipeline/v1alpha1/defaulter.go
@@ -5,20 +5,30 @@ 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 = &defaulter{}
+var _ webhook.CustomDefaulter = &LogPipelineDefaulter{}
 
-type defaulter struct {
+type LogPipelineDefaulter struct {
 	ApplicationInputEnabled          bool
 	ApplicationInputKeepOriginalBody bool
 }
 
-func (ld defaulter) Default(ctx context.Context, obj runtime.Object) error {
+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 {
 	pipeline, ok := obj.(*telemetryv1alpha1.LogPipeline)
 	if !ok {
 		return fmt.Errorf("expected an LogPipeline object but got %T", obj)
@@ -29,7 +39,7 @@ func (ld defaulter) Default(ctx context.Context, obj runtime.Object) error {
 	return nil
 }
 
-func (ld defaulter) applyDefaults(pipeline *telemetryv1alpha1.LogPipeline) {
+func (ld LogPipelineDefaulter) applyDefaults(pipeline *telemetryv1alpha1.LogPipeline) {
 	if pipeline.Spec.Input.Application != nil {
 		if pipeline.Spec.Input.Application.Enabled == nil {
 			pipeline.Spec.Input.Application.Enabled = &ld.ApplicationInputEnabled

diff --git a/webhook/logpipeline/v1alpha1/defaulter_test.go b/webhook/logpipeline/v1alpha1/defaulter_test.go
@@ -11,7 +11,7 @@ import (
 )
 
 func TestDefault(t *testing.T) {
-	sut := defaulter{
+	defaulter := LogPipelineDefaulter{
 		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 := sut.Default(context.Background(), tt.input)
+			err := defaulter.Default(context.Background(), tt.input)
 			assert.NoError(t, err)
 			assert.Equal(t, tt.expected, tt.input)
 		})

diff --git a/webhook/logpipeline/v1alpha1/files_validator_test.go b/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 := newValidateHandler(fakeClient, scheme)
+	sut := NewValidatingWebhookHandler(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 := newValidateHandler(fakeClient, scheme)
+	sut := NewValidatingWebhookHandler(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 := newValidateHandler(fakeClient, scheme)
+	sut := NewValidatingWebhookHandler(fakeClient, scheme)
 
 	newPipeline := testutils.NewLogPipelineBuilder().WithName("foo").WithFile("f1.json", "").Build()
 

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

diff --git a/...ipeline/v1alpha1/validate_handler_test.go → webhook/logpipeline/v1alpha1/handler_test.go b/...ipeline/v1alpha1/validate_handler_test.go → webhook/logpipeline/v1alpha1/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 := newValidateHandler(fakeClient, scheme)
+		sut := NewValidatingWebhookHandler(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 := newValidateHandler(fakeClient, scheme)
+				sut := NewValidatingWebhookHandler(fakeClient, scheme)
 
 				response := sut.Handle(context.Background(), admissionRequestFrom(t, logPipeline))
 				require.Equal(t, tt.allowed, response.Allowed)

diff --git a/webhook/logpipeline/v1alpha1/max_pipelines_validator_test.go b/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 := newValidateHandler(fakeClient, scheme)
+	sut := NewValidatingWebhookHandler(fakeClient, scheme)
 
 	newPipeline := testutils.NewLogPipelineBuilder().Build()
 
@@ -46,7 +46,7 @@ func TestValidateLimitNotExceeded(t *testing.T) {
 
 	fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(existingPipelines...).Build()
 
-	sut := newValidateHandler(fakeClient, scheme)
+	sut := NewValidatingWebhookHandler(fakeClient, scheme)
 
 	newPipeline := testutils.NewLogPipelineBuilder().Build()
 
@@ -69,7 +69,7 @@ func TestValidateLimitExceeded(t *testing.T) {
 
 	fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(existingPipelines...).Build()
 
-	sut := newValidateHandler(fakeClient, scheme)
+	sut := NewValidatingWebhookHandler(fakeClient, scheme)
 
 	newPipeline := testutils.NewLogPipelineBuilder().Build()
 
@@ -89,7 +89,7 @@ func TestValidateUpdate(t *testing.T) {
 
 	fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(&existingPipeline).Build()
 
-	sut := newValidateHandler(fakeClient, scheme)
+	sut := NewValidatingWebhookHandler(fakeClient, scheme)
 
 	response := sut.Handle(context.Background(), admissionRequestFrom(t, existingPipeline))
 

diff --git a/webhook/logpipeline/v1alpha1/setup.go b/webhook/logpipeline/v1alpha1/setup.go
diff --git a/webhook/logpipeline/v1alpha1/spec_validator_test.go b/webhook/logpipeline/v1alpha1/spec_validator_test.go
@@ -181,7 +181,7 @@ func TestValidateLogPipelineSpec(t *testing.T) {
 
 			fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects().Build()
 
-			sut := newValidateHandler(fakeClient, scheme)
+			sut := NewValidatingWebhookHandler(fakeClient, scheme)
 
 			response := sut.Handle(context.Background(), admissionRequestFrom(t, *tt.logPipeline))
 

diff --git a/webhook/logpipeline/v1alpha1/variable_validator_test.go b/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 := newValidateHandler(fakeClient, scheme)
+	sut := NewValidatingWebhookHandler(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 := newValidateHandler(fakeClient, scheme)
+	sut := NewValidatingWebhookHandler(fakeClient, scheme)
 
 	newPipeline := testutils.NewLogPipelineBuilder().
 		WithName("log-pipeline-2").

diff --git a/webhook/logpipeline/v1beta1/defaulter.go b/webhook/logpipeline/v1beta1/defaulter.go
@@ -5,21 +5,32 @@ 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 = &defaulter{}
+var _ webhook.CustomDefaulter = &LogPipelineDefaulter{}
 
-type defaulter struct {
+type LogPipelineDefaulter struct {
 	RuntimeInputEnabled          bool
 	RuntimeInputKeepOriginalBody bool
 	DefaultOTLPOutputProtocol    telemetryv1beta1.OTLPProtocol
 }
 
-func (ld defaulter) Default(ctx context.Context, obj runtime.Object) error {
+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 {
 	pipeline, ok := obj.(*telemetryv1beta1.LogPipeline)
 	if !ok {
 		return fmt.Errorf("expected an LogPipeline object but got %T", obj)
@@ -30,7 +41,7 @@ func (ld defaulter) Default(ctx context.Context, obj runtime.Object) error {
 	return nil
 }
 
-func (ld defaulter) applyDefaults(pipeline *telemetryv1beta1.LogPipeline) {
+func (ld LogPipelineDefaulter) applyDefaults(pipeline *telemetryv1beta1.LogPipeline) {
 	if pipeline.Spec.Input.Runtime != nil {
 		if pipeline.Spec.Input.Runtime.Enabled == nil {
 			pipeline.Spec.Input.Runtime.Enabled = &ld.RuntimeInputEnabled