fix(kuma-cp): validate the bandwidth strictly (#10371)

* fix(kuma-cp): validate the bandwidth strictly Signed-off-by: spacewander <[email protected]> * Update UPGRADE.md Signed-off-by: Krzysztof Słonka <[email protected]> --------- Signed-off-by: spacewander <[email protected]> Signed-off-by: Krzysztof Słonka <[email protected]> Co-authored-by: Krzysztof Słonka <[email protected]>
kumahq · Jun 19, 2024 · ba31b94 · ba31b94
1 parent d0fd4e0
commit ba31b94
Show file tree

Hide file tree

Showing 5 changed files with 76 additions and 4 deletions.
diff --git a/UPGRADE.md b/UPGRADE.md
@@ -8,6 +8,10 @@ does not have any particular instructions.
 
 ## Upgrade to `2.8.x`
 
+### MeshFaultInjection responseBandwidth.limit
+
+With [#10371](https://github.com/kumahq/kuma/pull/10371) we have tightened the validation of the `responseBandwidth.limit` field in `MeshFaultInjection` policy. Policies with invalid values, such as `-10kbps`, will be rejected.
+
 ### MeshRetry tcp.MaxConnectAttempt
 
 With [#10250](https://github.com/kumahq/kuma/pull/10250) `MeshRetry` policies with `spec.tcp.MaxConnectAttempt=0` will be rejected.

diff --git a/pkg/core/validators/common_validators.go b/pkg/core/validators/common_validators.go
@@ -223,7 +223,7 @@ func ValidateIntegerGreaterThan(path PathBuilder, value uint32, minValue uint32)
 	return err
 }
 
-var BandwidthRegex = regexp.MustCompile(`(\d*)\s?([GMk]?bps)`)
+var BandwidthRegex = regexp.MustCompile(`^(\d*)\s?([GMk]+bps)$`)
 
 func ValidateBandwidth(path PathBuilder, value string) ValidationError {
 	var err ValidationError

diff --git a/pkg/core/validators/common_validators_test.go b/pkg/core/validators/common_validators_test.go
@@ -0,0 +1,68 @@
+package validators
+
+import "testing"
+
+func TestValidateBandwidth(t *testing.T) {
+	path := []string{"path"}
+
+	tests := []struct {
+		name  string
+		input string
+		err   string
+	}{
+		{
+			name:  "sanity",
+			input: "1kbps",
+		},
+		{
+			name:  "without number",
+			input: "Mbps",
+		},
+		{
+			name:  "not exact match",
+			input: "1bpsp",
+			err: func() string {
+				e := &ValidationError{}
+				e.AddViolationAt(path, MustHaveBPSUnit)
+				return e.Error()
+			}(),
+		},
+		{
+			name:  "bps is not allowed",
+			input: "1bps",
+			err: func() string {
+				e := &ValidationError{}
+				e.AddViolationAt(path, MustHaveBPSUnit)
+				return e.Error()
+			}(),
+		},
+		{
+			name:  "float point number is not supported",
+			input: "0.1kbps",
+			err: func() string {
+				e := &ValidationError{}
+				e.AddViolationAt(path, MustHaveBPSUnit)
+				return e.Error()
+			}(),
+		},
+		{
+			name:  "not defined",
+			input: "",
+			err: func() string {
+				e := &ValidationError{}
+				e.AddViolationAt(path, MustBeDefined)
+				return e.Error()
+			}(),
+		},
+	}
+
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			actual := ValidateBandwidth(path, tt.input)
+			if actual.Error() != tt.err {
+				t.Errorf("ValidateBandwidth(%s): expected %s, actual %s", tt.input, tt.err, actual)
+			}
+		})
+	}
+}
diff --git a/pkg/plugins/policies/meshfaultinjection/api/v1alpha1/validator_test.go b/pkg/plugins/policies/meshfaultinjection/api/v1alpha1/validator_test.go
@@ -34,7 +34,7 @@ from:
             value: 5s
             percentage: 5
         - responseBandwidth:
-            limit: 100mbps
+            limit: 100Mbps
             percentage: 5
         - abort:
             httpStatus: 500

diff --git a/test/e2e_env/kubernetes/meshfaultinjection/api.go b/test/e2e_env/kubernetes/meshfaultinjection/api.go
@@ -67,7 +67,7 @@ spec:
                 value: 5s
                 percentage: 3
               responseBandwidth:
-                limit: 10mbps
+                limit: 10Mbps
                 percentage: 1
             - delay:
                 value: 11s
@@ -84,7 +84,7 @@ spec:
                 value: 5s
                 percentage: "3.2"
             - responseBandwidth:
-                limit: 10mbps
+                limit: 10Mbps
                 percentage: 1
 `, Config.KumaNamespace, meshName))(kubernetes.Cluster)).To(Succeed())