Skip to content

Commit b8892f0

Browse files
committed
Fix webhook validation for nested fields
1 parent 66e667d commit b8892f0

File tree

4 files changed

+54
-15
lines changed

4 files changed

+54
-15
lines changed

apis/flowmetrics/v1alpha1/flowmetric_webhook.go

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -80,17 +80,17 @@ func checkFlowMetricCartinality(fMetric *FlowMetric) admission.Warnings {
8080
}
8181

8282
func validateFlowMetric(_ context.Context, fMetric *FlowMetric) (admission.Warnings, error) {
83-
var str []string
83+
var fields []string
8484
var allErrs field.ErrorList
8585

8686
for _, f := range fMetric.Spec.Filters {
87-
str = append(str, f.Field)
87+
fields = append(fields, f.Field)
8888
}
8989

90-
if len(str) != 0 {
91-
if !helper.FindFields(str, false) {
92-
allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "filters"), str,
93-
fmt.Sprintf("invalid filter field: %s", str)))
90+
if len(fields) != 0 {
91+
if !helper.FindFields(fields, false) {
92+
allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "filters"), fields,
93+
fmt.Sprintf("invalid filter field: %s", fields)))
9494
}
9595
}
9696

@@ -119,17 +119,11 @@ func validateFlowMetric(_ context.Context, fMetric *FlowMetric) (admission.Warni
119119
}
120120
}
121121

122-
// Only fields defined as Labels are valid for flattening
122+
// Check for valid fields
123123
if len(fMetric.Spec.Flatten) != 0 {
124-
var invalidFlatten []string
125-
for _, toFlatten := range fMetric.Spec.Flatten {
126-
if _, ok := labelsMap[toFlatten]; !ok {
127-
invalidFlatten = append(invalidFlatten, toFlatten)
128-
}
129-
}
130-
if len(invalidFlatten) > 0 {
124+
if !helper.FindFields(fMetric.Spec.Flatten, false) {
131125
allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "flatten"), fMetric.Spec.Flatten,
132-
fmt.Sprintf("some fields defined for flattening are not defined as labels: %v", invalidFlatten)))
126+
fmt.Sprintf("invalid fields to flatten: %s", fMetric.Spec.Flatten)))
133127
}
134128
}
135129
}

apis/flowmetrics/v1alpha1/flowmetric_webhook_test.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,27 @@ func TestFlowMetric(t *testing.T) {
105105
},
106106
expectedError: "invalid value field",
107107
},
108+
{
109+
desc: "Valid nested fields",
110+
m: &FlowMetric{
111+
ObjectMeta: metav1.ObjectMeta{
112+
Name: "test1",
113+
Namespace: "test-namespace",
114+
},
115+
Spec: FlowMetricSpec{
116+
Labels: []string{"NetworkEvents>Name"},
117+
Flatten: []string{"NetworkEvents"},
118+
Filters: []MetricFilter{
119+
{
120+
Field: "NetworkEvents>Type",
121+
Value: "acl",
122+
},
123+
},
124+
Remap: map[string]string{"NetworkEvents>Name": "name"},
125+
},
126+
},
127+
expectedError: "",
128+
},
108129
}
109130

110131
for _, test := range tests {
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
# Count network policy events
2+
# More examples in https://github.com/netobserv/network-observability-operator/tree/main/config/samples/flowmetrics
3+
apiVersion: flows.netobserv.io/v1alpha1
4+
kind: FlowMetric
5+
metadata:
6+
name: network-policy-events
7+
namespace: netobserv
8+
spec:
9+
metricName: network_policy_events_total
10+
type: Counter
11+
labels: [NetworkEvents>Type, NetworkEvents>Namespace, NetworkEvents>Name, NetworkEvents>Action, NetworkEvents>Direction]
12+
filters:
13+
- field: NetworkEvents>Feature
14+
value: acl
15+
flatten: [NetworkEvents]
16+
remap:
17+
"NetworkEvents>Type": type
18+
"NetworkEvents>Namespace": namespace
19+
"NetworkEvents>Name": name
20+
"NetworkEvents>Action": action
21+
"NetworkEvents>Direction": direction

pkg/helper/helpers.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,9 @@ func FindFields(labels []string, isNumber bool) bool {
114114
}
115115

116116
for _, l := range labels {
117+
// Split field for nesting, e.g. "NetworkEvents>Name" (and we don't verify the nested part)
118+
parts := strings.Split(l, ">")
119+
l = parts[0]
117120
if ok := labelMap[l].exists; !ok {
118121
return false
119122
}

0 commit comments

Comments
 (0)