Skip to content

Commit

Permalink
Merge pull request #622 from msherif1234/validation_webhook
Browse files Browse the repository at this point in the history
NETOBSERV-1614: Metrics API validation webhook
  • Loading branch information
msherif1234 authored Apr 24, 2024
2 parents 02a0f01 + e9c8717 commit bcf6017
Show file tree
Hide file tree
Showing 18 changed files with 382 additions and 26 deletions.
8 changes: 7 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions PROJECT
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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"
6 changes: 3 additions & 3 deletions apis/flowmetrics/v1alpha1/flowmetric_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`

Expand All @@ -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"`

Expand Down
99 changes: 99 additions & 0 deletions apis/flowmetrics/v1alpha1/flowmetric_webhook.go
Original file line number Diff line number Diff line change
@@ -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
}
125 changes: 125 additions & 0 deletions apis/flowmetrics/v1alpha1/flowmetric_webhook_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
}
}
18 changes: 17 additions & 1 deletion apis/flowmetrics/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 13 additions & 3 deletions bundle/manifests/flows.netobserv.io_flowmetrics.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
23 changes: 22 additions & 1 deletion bundle/manifests/netobserv-operator.clusterserviceversion.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Loading

0 comments on commit bcf6017

Please sign in to comment.