NETOBSERV-1614: Metrics API validation webhook

Makefile

@@ -255,7 +255,13 @@ endif
 
 ##@ Code / files generation
 manifests: YQ controller-gen ## Generate WebhookConfiguration, ClusterRole and CustomResourceDefinition objects.
-	$(CONTROLLER_GEN) rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases
+	$(CONTROLLER_GEN) \
+	rbac:roleName=manager-role \
+	crd:crdVersions=v1 \
+	paths="./apis/..." \
+	output:crd:artifacts:config=config/crd/bases \
+	output:webhook:dir=./config/webhook \
+	webhook
 	$(YQ) -i 'del(.spec.versions[].schema.openAPIV3Schema.properties.spec.properties.processor.properties.kafkaConsumerAutoscaler.properties.metrics.items | .. | select(has("description")) | .description)' config/crd/bases/flows.netobserv.io_flowcollectors.yaml
 	$(YQ) -i 'del(.spec.versions[].schema.openAPIV3Schema.properties.spec.properties.consolePlugin.properties.autoscaler.properties.metrics.items | .. | select(has("description")) | .description)' config/crd/bases/flows.netobserv.io_flowcollectors.yaml
 	$(YQ) -i 'del(.spec.versions[].schema.openAPIV3Schema.properties.spec.properties.agent.properties.ebpf.properties.advanced.properties.affinity.properties | .. | select(has("description")) | .description)' config/crd/bases/flows.netobserv.io_flowcollectors.yaml

PROJECT

@@ -1,3 +1,7 @@
+# Code generated by tool. DO NOT EDIT.
+# This file is used to track the info used to scaffold your project
+# and allow the plugins properly work.
+# More info: https://book.kubebuilder.io/reference/project-config.html
 domain: netobserv.io
 layout:
 - go.kubebuilder.io/v3
@@ -30,4 +34,7 @@ resources:
   kind: FlowMetric
   path: github.com/netobserv/network-observability-operator/apis/flowmetrics/v1alpha1
   version: v1alpha1
+  webhooks:
+    validation: true
+    webhookVersion: v1
 version: "3"

apis/flowmetrics/v1alpha1/flowmetric_types.go

@@ -74,13 +74,13 @@ type FlowMetricSpec struct {
 
 	// `valueField` is the flow field that must be used as a value for this metric. This field must hold numeric values.
 	// Leave empty to count flows rather than a specific value per flow.
-	// Refer to the documentation for the list of available fields: https://docs.openshift.com/container-platform/latest/networking/network_observability/json-flows-format-reference.html.
+	// Refer to the documentation for the list of available fields: https://docs.openshift.com/container-platform/latest/observability/network_observability/json-flows-format-reference.html.
 	// +optional
 	ValueField string `json:"valueField,omitempty"`
 
 	// `filters` is a list of fields and values used to restrict which flows are taken into account. Oftentimes, these filters must
 	// be used to eliminate duplicates: `Duplicate != "true"` and `FlowDirection = "0"`.
-	// Refer to the documentation for the list of available fields: https://docs.openshift.com/container-platform/latest/networking/network_observability/json-flows-format-reference.html.
+	// Refer to the documentation for the list of available fields: https://docs.openshift.com/container-platform/latest/observability/network_observability/json-flows-format-reference.html.
 	// +optional
 	Filters []MetricFilter `json:"filters"`
 
@@ -89,7 +89,7 @@ type FlowMetricSpec struct {
 	// It must be done carefully as it impacts the metric cardinality (cf https://rhobs-handbook.netlify.app/products/openshiftmonitoring/telemetry.md/#what-is-the-cardinality-of-a-metric).
 	// In general, avoid setting very high cardinality labels such as IP or MAC addresses.
 	// "SrcK8S_OwnerName" or "DstK8S_OwnerName" should be preferred over "SrcK8S_Name" or "DstK8S_Name" as much as possible.
-	// Refer to the documentation for the list of available fields: https://docs.openshift.com/container-platform/latest/network_observability/json-flows-format-reference.html.
+	// Refer to the documentation for the list of available fields: https://docs.openshift.com/container-platform/latest/observability/network_observability/json-flows-format-reference.html.
 	// +optional
 	Labels []string `json:"labels"`
 

apis/flowmetrics/v1alpha1/flowmetric_webhook.go

@@ -0,0 +1,99 @@
+package v1alpha1
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/netobserv/network-observability-operator/pkg/helper"
+
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/util/validation/field"
+	ctrl "sigs.k8s.io/controller-runtime"
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
+	"sigs.k8s.io/controller-runtime/pkg/webhook"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+)
+
+// log is for logging in this package.
+var flowmetriclog = logf.Log.WithName("flowmetric-resource")
+
+type FlowMetricWebhook struct {
+	FlowMetric
+}
+
+// +kubebuilder:webhook:verbs=create;update,path=/validate-flows-netobserv-io-v1alpha1-flowmetric,mutating=false,failurePolicy=fail,sideEffects=None,groups=flows.netobserv.io,resources=flowmetrics,versions=v1alpha1,name=flowmetricvalidationwebhook.netobserv.io,admissionReviewVersions=v1
+var (
+	_ webhook.CustomValidator = &FlowMetricWebhook{FlowMetric{}}
+)
+
+func (r *FlowMetricWebhook) SetupWebhookWithManager(mgr ctrl.Manager) error {
+	return ctrl.NewWebhookManagedBy(mgr).
+		For(&FlowMetric{}).
+		WithValidator(&FlowMetricWebhook{}).
+		Complete()
+}
+
+// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
+func (r *FlowMetricWebhook) ValidateCreate(ctx context.Context, newObj runtime.Object) (warnings admission.Warnings, err error) {
+	flowmetriclog.Info("validate create", "name", r.Name)
+	newFlowMetric, ok := newObj.(*FlowMetric)
+	if !ok {
+		return nil, apierrors.NewBadRequest(fmt.Sprintf("expected an FlowMetric but got a %T", newObj))
+	}
+	return nil, validateFlowMetric(ctx, newFlowMetric)
+}
+
+// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
+func (r *FlowMetricWebhook) ValidateUpdate(ctx context.Context, _, newObj runtime.Object) (warnings admission.Warnings, err error) {
+	flowmetriclog.Info("validate update", "name", r.Name)
+	newFlowMetric, ok := newObj.(*FlowMetric)
+	if !ok {
+		return nil, apierrors.NewBadRequest(fmt.Sprintf("expected an FlowMetric but got a %T", newObj))
+	}
+	return nil, validateFlowMetric(ctx, newFlowMetric)
+}
+
+// ValidateDelete implements webhook.Validator so a webhook will be registered for the type
+func (r *FlowMetricWebhook) ValidateDelete(_ context.Context, _ runtime.Object) (warnings admission.Warnings, err error) {
+	flowmetriclog.Info("validate delete", "name", r.Name)
+	return nil, nil
+}
+
+func validateFlowMetric(_ context.Context, fMetric *FlowMetric) error {
+	var str []string
+	var allErrs field.ErrorList
+
+	for _, f := range fMetric.Spec.Filters {
+		str = append(str, f.Field)
+	}
+
+	if len(str) != 0 {
+		if !helper.FindFilter(str, false) {
+			allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "filters"), str,
+				fmt.Sprintf("invalid filter field: %s", str)))
+		}
+	}
+
+	if len(fMetric.Spec.Labels) != 0 {
+		if !helper.FindFilter(fMetric.Spec.Labels, false) {
+			allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "labels"), fMetric.Spec.Labels,
+				fmt.Sprintf("invalid label name: %s", fMetric.Spec.Labels)))
+		}
+	}
+
+	if fMetric.Spec.ValueField != "" {
+		if !helper.FindFilter([]string{fMetric.Spec.ValueField}, true) {
+			allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "valueField"), fMetric.Spec.ValueField,
+				fmt.Sprintf("invalid value field: %s", fMetric.Spec.ValueField)))
+		}
+	}
+
+	if len(allErrs) != 0 {
+		return apierrors.NewInvalid(
+			schema.GroupKind{Group: GroupVersion.Group, Kind: FlowMetric{}.Kind},
+			fMetric.Name, allErrs)
+	}
+	return nil
+}

apis/flowmetrics/v1alpha1/flowmetric_webhook_test.go

@@ -0,0 +1,125 @@
+package v1alpha1
+
+import (
+	"context"
+	"fmt"
+	"strings"
+	"testing"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func TestFlowMetric(t *testing.T) {
+	tests := []struct {
+		desc          string
+		m             *FlowMetric
+		expectedError string
+	}{
+		{
+			desc: "Invalid FlowMetric Filter",
+			m: &FlowMetric{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test1",
+					Namespace: "test-namespace",
+				},
+				Spec: FlowMetricSpec{
+					Filters: []MetricFilter{
+						{
+							Field: "test",
+						},
+					},
+				},
+			},
+			expectedError: "invalid filter field",
+		},
+		{
+			desc: "Valid FlowMetric Filter",
+			m: &FlowMetric{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test1",
+					Namespace: "test-namespace",
+				},
+				Spec: FlowMetricSpec{
+					Filters: []MetricFilter{
+						{
+							Field: "DstK8S_Zone",
+						},
+					},
+				},
+			},
+			expectedError: "",
+		},
+		{
+			desc: "Invalid FlowMetric Label",
+			m: &FlowMetric{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test1",
+					Namespace: "test-namespace",
+				},
+				Spec: FlowMetricSpec{
+					Labels: []string{
+						"test",
+					},
+				},
+			},
+			expectedError: "invalid label name",
+		},
+		{
+			desc: "Valid FlowMetric Label",
+			m: &FlowMetric{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test1",
+					Namespace: "test-namespace",
+				},
+				Spec: FlowMetricSpec{
+					Labels: []string{
+						"DstK8S_Zone",
+					},
+				},
+			},
+			expectedError: "",
+		},
+		{
+			desc: "Valid valueField",
+			m: &FlowMetric{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test1",
+					Namespace: "test-namespace",
+				},
+				Spec: FlowMetricSpec{
+					ValueField: "Bytes",
+				},
+			},
+			expectedError: "",
+		},
+		{
+			desc: "Invalid valueField",
+			m: &FlowMetric{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test1",
+					Namespace: "test-namespace",
+				},
+				Spec: FlowMetricSpec{
+					ValueField: "DstAddr",
+				},
+			},
+			expectedError: "invalid value field",
+		},
+	}
+
+	for _, test := range tests {
+		err := validateFlowMetric(context.TODO(), test.m)
+		if err == nil {
+			if test.expectedError != "" {
+				t.Errorf("%s: ValidateFlowMetric failed, no error found while expected: \"%s\"", test.desc, test.expectedError)
+			}
+		} else {
+			if len(test.expectedError) == 0 {
+				t.Errorf("%s: ValidateFlowMetric failed, unexpected error: \"%s\"", test.desc, err)
+			}
+			if !strings.Contains(fmt.Sprint(err), test.expectedError) {
+				t.Errorf("%s: ValidateFlowMetric failed, expected error: \"%s\" to contain: \"%s\"", test.desc, err, test.expectedError)
+			}
+		}
+	}
+}

bundle/manifests/flows.netobserv.io_flowmetrics.yaml

@@ -6,6 +6,16 @@ metadata:
   creationTimestamp: null
   name: flowmetrics.flows.netobserv.io
 spec:
+  conversion:
+    strategy: Webhook
+    webhook:
+      clientConfig:
+        service:
+          name: netobserv-webhook-service
+          namespace: netobserv
+          path: /convert
+      conversionReviewVersions:
+      - v1
   group: flows.netobserv.io
   names:
     kind: FlowMetric
@@ -66,7 +76,7 @@ spec:
                 description: |-
                   `filters` is a list of fields and values used to restrict which flows are taken into account. Oftentimes, these filters must
                   be used to eliminate duplicates: `Duplicate != "true"` and `FlowDirection = "0"`.
-                  Refer to the documentation for the list of available fields: https://docs.openshift.com/container-platform/latest/networking/network_observability/json-flows-format-reference.html.
+                  Refer to the documentation for the list of available fields: https://docs.openshift.com/container-platform/latest/observability/network_observability/json-flows-format-reference.html.
                 items:
                   properties:
                     field:
@@ -103,7 +113,7 @@ spec:
                   It must be done carefully as it impacts the metric cardinality (cf https://rhobs-handbook.netlify.app/products/openshiftmonitoring/telemetry.md/#what-is-the-cardinality-of-a-metric).
                   In general, avoid setting very high cardinality labels such as IP or MAC addresses.
                   "SrcK8S_OwnerName" or "DstK8S_OwnerName" should be preferred over "SrcK8S_Name" or "DstK8S_Name" as much as possible.
-                  Refer to the documentation for the list of available fields: https://docs.openshift.com/container-platform/latest/network_observability/json-flows-format-reference.html.
+                  Refer to the documentation for the list of available fields: https://docs.openshift.com/container-platform/latest/observability/network_observability/json-flows-format-reference.html.
                 items:
                   type: string
                 type: array
@@ -124,7 +134,7 @@ spec:
                 description: |-
                   `valueField` is the flow field that must be used as a value for this metric. This field must hold numeric values.
                   Leave empty to count flows rather than a specific value per flow.
-                  Refer to the documentation for the list of available fields: https://docs.openshift.com/container-platform/latest/networking/network_observability/json-flows-format-reference.html.
+                  Refer to the documentation for the list of available fields: https://docs.openshift.com/container-platform/latest/observability/network_observability/json-flows-format-reference.html.
                 type: string
             required:
             - metricName

bundle/manifests/netobserv-operator.clusterserviceversion.yaml

@@ -1300,8 +1300,9 @@ spec:
     containerPort: 443
     conversionCRDs:
     - flowcollectors.flows.netobserv.io
+    - flowmetrics.flows.netobserv.io
     deploymentName: netobserv-controller-manager
-    generateName: cflowcollectors.kb.io
+    generateName: cflowcollectorsflowmetrics.kb.io
     sideEffects: None
     targetPort: 9443
     type: ConversionWebhook
@@ -1326,3 +1327,23 @@ spec:
     targetPort: 9443
     type: ValidatingAdmissionWebhook
     webhookPath: /validate-netobserv-io-v1beta2-flowcollector
+  - admissionReviewVersions:
+    - v1
+    containerPort: 443
+    deploymentName: netobserv-controller-manager
+    failurePolicy: Fail
+    generateName: flowmetricvalidationwebhook.netobserv.io
+    rules:
+    - apiGroups:
+      - flows.netobserv.io
+      apiVersions:
+      - v1alpha1
+      operations:
+      - CREATE
+      - UPDATE
+      resources:
+      - flowmetrics
+    sideEffects: None
+    targetPort: 9443
+    type: ValidatingAdmissionWebhook
+    webhookPath: /validate-flows-netobserv-io-v1alpha1-flowmetric