chore: Flatten validating webhook packages (#1607)

.golangci.yaml

@@ -72,8 +72,6 @@ linters-settings:
         alias: logpipelinewebhook
       - pkg: github.com/kyma-project/telemetry-manager/internal/reconciler/logpipeline/fluentbit
         alias: logpipelinefluentbit
-      - pkg: github.com/kyma-project/telemetry-manager/webhook/logpipeline/validation/mocks
-        alias: logpipelinevalidationmocks
       - pkg: github.com/prometheus/client_golang/api/prometheus/v1
         alias: promv1
       - pkg: github.com/prometheus/client_model/go

.mockery.yaml

@@ -38,8 +38,3 @@ packages:
   github.com/kyma-project/telemetry-manager/internal/selfmonitor/prober:
     interfaces:
       alertGetter:
-  github.com/kyma-project/telemetry-manager/webhook/logpipeline/validation:
-    interfaces:
-      FilesValidator:
-      MaxPipelinesValidator:
-      VariablesValidator:

internal/utils/test/log_pipeline_builder.go

@@ -20,14 +20,13 @@ type LogPipelineBuilder struct {
 	finalizers        []string
 	deletionTimeStamp metav1.Time
 
-	input telemetryv1alpha1.LogPipelineInput
-
-	filters []telemetryv1alpha1.LogPipelineFilter
-
-	httpOutput   *telemetryv1alpha1.LogPipelineHTTPOutput
-	otlpOutput   *telemetryv1alpha1.OTLPOutput
-	customOutput string
-
+	input            telemetryv1alpha1.LogPipelineInput
+	filters          []telemetryv1alpha1.LogPipelineFilter
+	httpOutput       *telemetryv1alpha1.LogPipelineHTTPOutput
+	otlpOutput       *telemetryv1alpha1.OTLPOutput
+	customOutput     string
+	files            []telemetryv1alpha1.LogPipelineFileMount
+	variables        []telemetryv1alpha1.LogPipelineVariableRef
 	statusConditions []metav1.Condition
 }
 
@@ -155,6 +154,30 @@ func (b *LogPipelineBuilder) WithCustomFilter(filter string) *LogPipelineBuilder
 	return b
 }
 
+func (b *LogPipelineBuilder) WithFile(name, content string) *LogPipelineBuilder {
+	b.files = append(b.files, telemetryv1alpha1.LogPipelineFileMount{
+		Name:    name,
+		Content: content,
+	})
+
+	return b
+}
+
+func (b *LogPipelineBuilder) WithVariable(name, secretName, secretNamespace, secretKey string) *LogPipelineBuilder {
+	b.variables = append(b.variables, telemetryv1alpha1.LogPipelineVariableRef{
+		Name: name,
+		ValueFrom: telemetryv1alpha1.ValueFromSource{
+			SecretKeyRef: &telemetryv1alpha1.SecretKeyRef{
+				Name:      secretName,
+				Namespace: secretNamespace,
+				Key:       secretKey,
+			},
+		},
+	})
+
+	return b
+}
+
 func (b *LogPipelineBuilder) WithHTTPOutput(opts ...HTTPOutputOption) *LogPipelineBuilder {
 	b.httpOutput = defaultHTTPOutput()
 	for _, opt := range opts {
@@ -216,6 +239,8 @@ func (b *LogPipelineBuilder) Build() telemetryv1alpha1.LogPipeline {
 				Custom: b.customOutput,
 				OTLP:   b.otlpOutput,
 			},
+			Files:     b.files,
+			Variables: b.variables,
 		},
 		Status: telemetryv1alpha1.LogPipelineStatus{
 			Conditions: b.statusConditions,

main.go

@@ -43,7 +43,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
-	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
 	operatorv1alpha1 "github.com/kyma-project/telemetry-manager/apis/operator/v1alpha1"
 	telemetryv1alpha1 "github.com/kyma-project/telemetry-manager/apis/telemetry/v1alpha1"
@@ -59,7 +58,6 @@ import (
 	"github.com/kyma-project/telemetry-manager/internal/webhookcert"
 	logparserwebhook "github.com/kyma-project/telemetry-manager/webhook/logparser"
 	logpipelinewebhook "github.com/kyma-project/telemetry-manager/webhook/logpipeline"
-	"github.com/kyma-project/telemetry-manager/webhook/logpipeline/validation"
 
 	// Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.)
 	// to ensure that exec-entrypoint and run can make use of them.
@@ -318,10 +316,10 @@ func run() error {
 	}
 
 	mgr.GetWebhookServer().Register("/validate-logpipeline", &webhook.Admission{
-		Handler: createLogPipelineValidator(mgr.GetClient()),
+		Handler: logpipelinewebhook.NewValidatingWebhookHandler(mgr.GetClient(), scheme),
 	})
 	mgr.GetWebhookServer().Register("/validate-logparser", &webhook.Admission{
-		Handler: createLogParserValidator(mgr.GetClient()),
+		Handler: logparserwebhook.NewValidatingWebhookHandler(scheme),
 	})
 	mgr.GetWebhookServer().Register("/api/v2/alerts", selfmonitorwebhook.NewHandler(
 		mgr.GetClient(),
@@ -502,26 +500,6 @@ func setNamespaceFieldSelector() fields.Selector {
 	return fields.SelectorFromSet(fields.Set{"metadata.namespace": telemetryNamespace})
 }
 
-func createLogPipelineValidator(client client.Client) *logpipelinewebhook.ValidatingWebhookHandler {
-	// TODO: Align max log pipeline enforcement with the method used in the TracePipeline/MetricPipeline controllers,
-	// replacing the current validating webhook approach.
-	const maxLogPipelines = 5
-
-	return logpipelinewebhook.NewValidatingWebhookHandler(
-		client,
-		validation.NewVariablesValidator(client),
-		validation.NewMaxPipelinesValidator(maxLogPipelines),
-		validation.NewFilesValidator(),
-		admission.NewDecoder(scheme),
-	)
-}
-
-func createLogParserValidator(client client.Client) *logparserwebhook.ValidatingWebhookHandler {
-	return logparserwebhook.NewValidatingWebhookHandler(
-		client,
-		admission.NewDecoder(scheme))
-}
-
 func createSelfMonitoringConfig() telemetry.SelfMonitorConfig {
 	return telemetry.SelfMonitorConfig{
 		Config: selfmonitor.Config{

webhook/logparser/validation/spec_validator.go → webhook/logparser/spec_validator.go

@@ -1,4 +1,4 @@
-package validation
+package logparser
 
 import (
 	"fmt"
@@ -7,7 +7,7 @@ import (
 	"github.com/kyma-project/telemetry-manager/internal/fluentbit/config"
 )
 
-func ValidateSpec(lp *telemetryv1alpha1.LogParser) error {
+func validateSpec(lp *telemetryv1alpha1.LogParser) error {
 	if len(lp.Spec.Parser) == 0 {
 		return fmt.Errorf("log parser '%s' has no parser defined", lp.Name)
 	}

webhook/logparser/webhook.go

@@ -1,47 +1,23 @@
-/*
-Copyright 2021.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
 package logparser
 
 import (
 	"context"
 	"net/http"
 
-	admissionv1 "k8s.io/api/admission/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"sigs.k8s.io/controller-runtime/pkg/client"
+	"k8s.io/apimachinery/pkg/runtime"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
 	telemetryv1alpha1 "github.com/kyma-project/telemetry-manager/apis/telemetry/v1alpha1"
-	"github.com/kyma-project/telemetry-manager/webhook/logparser/validation"
-	logpipelinewebhook "github.com/kyma-project/telemetry-manager/webhook/logpipeline"
 )
 
-// +kubebuilder:webhook:path=/validate-logparser,mutating=false,failurePolicy=fail,sideEffects=None,groups=telemetry.kyma-project.io,resources=logparsers,verbs=create;update,versions=v1alpha1,name=vlogparser.kb.io,admissionReviewVersions=v1
 type ValidatingWebhookHandler struct {
-	client.Client
 	decoder admission.Decoder
 }
 
-// TODO: Merge validation package with the webhook package, avoid useless dependency injection
-func NewValidatingWebhookHandler(client client.Client, decoder admission.Decoder) *ValidatingWebhookHandler {
+func NewValidatingWebhookHandler(scheme *runtime.Scheme) *ValidatingWebhookHandler {
 	return &ValidatingWebhookHandler{
-		Client:  client,
-		decoder: decoder,
+		decoder: admission.NewDecoder(scheme),
 	}
 }
 
@@ -54,29 +30,10 @@ func (v *ValidatingWebhookHandler) Handle(ctx context.Context, req admission.Req
 		return admission.Errored(http.StatusBadRequest, err)
 	}
 
-	if err := v.validateLogParser(logParser); err != nil {
+	if err := validateSpec(logParser); err != nil {
 		log.Error(err, "LogParser rejected")
-
-		return admission.Response{
-			AdmissionResponse: admissionv1.AdmissionResponse{
-				Allowed: false,
-				Result: &metav1.Status{
-					Code:    int32(http.StatusForbidden),
-					Reason:  logpipelinewebhook.StatusReasonConfigurationError,
-					Message: err.Error(),
-				},
-			},
-		}
+		return admission.Errored(http.StatusBadRequest, err)
 	}
 
 	return admission.Allowed("LogParser validation successful")
 }
-
-func (v *ValidatingWebhookHandler) validateLogParser(logParser *telemetryv1alpha1.LogParser) error {
-	err := validation.ValidateSpec(logParser)
-	if err != nil {
-		return err
-	}
-
-	return nil
-}

webhook/logpipeline/validation/files_validator.go → webhook/logpipeline/files_validator.go

@@ -1,23 +1,12 @@
-package validation
+package logpipeline
 
 import (
 	"fmt"
 
 	telemetryv1alpha1 "github.com/kyma-project/telemetry-manager/apis/telemetry/v1alpha1"
 )
 
-type FilesValidator interface {
-	Validate(logPipeline *telemetryv1alpha1.LogPipeline, logPipelines *telemetryv1alpha1.LogPipelineList) error
-}
-
-type filesValidator struct {
-}
-
-func NewFilesValidator() FilesValidator {
-	return &filesValidator{}
-}
-
-func (f *filesValidator) Validate(logPipeline *telemetryv1alpha1.LogPipeline, logPipelines *telemetryv1alpha1.LogPipelineList) error {
+func validateFiles(logPipeline *telemetryv1alpha1.LogPipeline, logPipelines *telemetryv1alpha1.LogPipelineList) error {
 	err := validateUniqueFileName(logPipeline, logPipelines)
 	if err != nil {
 		return err

webhook/logpipeline/files_validator_test.go

@@ -0,0 +1,71 @@
+package logpipeline
+
+import (
+	"context"
+	"net/http"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"k8s.io/apimachinery/pkg/runtime"
+	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+
+	telemetryv1alpha1 "github.com/kyma-project/telemetry-manager/apis/telemetry/v1alpha1"
+	testutils "github.com/kyma-project/telemetry-manager/internal/utils/test"
+)
+
+func TestDuplicateFileName(t *testing.T) {
+	scheme := runtime.NewScheme()
+	_ = clientgoscheme.AddToScheme(scheme)
+	_ = telemetryv1alpha1.AddToScheme(scheme)
+
+	existingPipeline := testutils.NewLogPipelineBuilder().WithName("foo").WithFile("f1.json", "").Build()
+
+	fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(&existingPipeline).Build()
+
+	sut := NewValidatingWebhookHandler(fakeClient, scheme)
+
+	newPipeline := testutils.NewLogPipelineBuilder().WithName("bar").WithFile("f1.json", "").Build()
+
+	response := sut.Handle(context.Background(), admissionRequestFrom(t, newPipeline))
+
+	require.False(t, response.Allowed)
+	require.EqualValues(t, response.Result.Code, http.StatusBadRequest)
+	require.Equal(t, response.Result.Message, "filename 'f1.json' is already being used in the logPipeline 'foo'")
+}
+
+func TestDuplicateFileNameInSamePipeline(t *testing.T) {
+	scheme := runtime.NewScheme()
+	_ = clientgoscheme.AddToScheme(scheme)
+	_ = telemetryv1alpha1.AddToScheme(scheme)
+
+	fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects().Build()
+
+	sut := NewValidatingWebhookHandler(fakeClient, scheme)
+
+	newPipeline := testutils.NewLogPipelineBuilder().WithName("foo").WithFile("f1.json", "").WithFile("f1.json", "").Build()
+
+	response := sut.Handle(context.Background(), admissionRequestFrom(t, newPipeline))
+
+	require.False(t, response.Allowed)
+	require.EqualValues(t, response.Result.Code, http.StatusBadRequest)
+	require.Equal(t, response.Result.Message, "duplicate file names detected please review your pipeline")
+}
+
+func TestValidateUpdatePipeline(t *testing.T) {
+	scheme := runtime.NewScheme()
+	_ = clientgoscheme.AddToScheme(scheme)
+	_ = telemetryv1alpha1.AddToScheme(scheme)
+
+	existingPipeline := testutils.NewLogPipelineBuilder().WithName("foo").WithFile("f1.json", "").Build()
+
+	fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(&existingPipeline).Build()
+
+	sut := NewValidatingWebhookHandler(fakeClient, scheme)
+
+	newPipeline := testutils.NewLogPipelineBuilder().WithName("foo").WithFile("f1.json", "").Build()
+
+	response := sut.Handle(context.Background(), admissionRequestFrom(t, newPipeline))
+
+	require.True(t, response.Allowed)
+}