diff --git a/UPGRADE.md b/UPGRADE.md index aaa2a46d6ae7..dcf714b4c045 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -38,6 +38,12 @@ To ensure a smooth transition to Kuma 2.6.0, carefully review your existing conf The postgres driver `postgres` (lib/pq) is deprecated and will be removed in the future. Please migrate to the new postgres driver `pgx` by setting `DriverName=pgx` configuration option or `KUMA_STORE_POSTGRES_DRIVER_NAME=pgx` env variable. +### Make format SI valid for bandwidth in MeshFaultInjection policy + +Prior to this upgrade `mbps` and `gbps` were used for units for parameter `conf.responseBandwidth.percentage`. +These are not valid units according to the [International System of Units](https://en.wikipedia.org/wiki/International_System_of_Units) they are respectively corrected to `Gbps` and `Mbps` if using +these invalid units convert them into `kbps` prior to upgrade to avoid invalid format. + ## Upgrade to `2.5.x` ### Transparent-proxy and CNI v1 removal diff --git a/app/kumactl/cmd/install/testdata/install-control-plane.defaults.golden.yaml b/app/kumactl/cmd/install/testdata/install-control-plane.defaults.golden.yaml index 3543cad3c7c6..9b8661b13834 100644 --- a/app/kumactl/cmd/install/testdata/install-control-plane.defaults.golden.yaml +++ b/app/kumactl/cmd/install/testdata/install-control-plane.defaults.golden.yaml @@ -312,7 +312,7 @@ spec: properties: limit: description: |- - Limit is represented by value measure in gbps, mbps, kbps or bps, e.g. + Limit is represented by value measure in Gbps, Mbps, kbps, e.g. 10kbps type: string percentage: @@ -488,7 +488,7 @@ spec: properties: limit: description: |- - Limit is represented by value measure in gbps, mbps, kbps or bps, e.g. + Limit is represented by value measure in Gbps, Mbps, kbps, e.g. 10kbps type: string percentage: diff --git a/app/kumactl/cmd/install/testdata/install-control-plane.gateway-api-present-not-enabled.yaml b/app/kumactl/cmd/install/testdata/install-control-plane.gateway-api-present-not-enabled.yaml index 3543cad3c7c6..9b8661b13834 100644 --- a/app/kumactl/cmd/install/testdata/install-control-plane.gateway-api-present-not-enabled.yaml +++ b/app/kumactl/cmd/install/testdata/install-control-plane.gateway-api-present-not-enabled.yaml @@ -312,7 +312,7 @@ spec: properties: limit: description: |- - Limit is represented by value measure in gbps, mbps, kbps or bps, e.g. + Limit is represented by value measure in Gbps, Mbps, kbps, e.g. 10kbps type: string percentage: @@ -488,7 +488,7 @@ spec: properties: limit: description: |- - Limit is represented by value measure in gbps, mbps, kbps or bps, e.g. + Limit is represented by value measure in Gbps, Mbps, kbps, e.g. 10kbps type: string percentage: diff --git a/app/kumactl/cmd/install/testdata/install-control-plane.gateway-api-present.yaml b/app/kumactl/cmd/install/testdata/install-control-plane.gateway-api-present.yaml index 865ea9a9db8b..264e9b279644 100644 --- a/app/kumactl/cmd/install/testdata/install-control-plane.gateway-api-present.yaml +++ b/app/kumactl/cmd/install/testdata/install-control-plane.gateway-api-present.yaml @@ -312,7 +312,7 @@ spec: properties: limit: description: |- - Limit is represented by value measure in gbps, mbps, kbps or bps, e.g. + Limit is represented by value measure in Gbps, Mbps, kbps, e.g. 10kbps type: string percentage: @@ -488,7 +488,7 @@ spec: properties: limit: description: |- - Limit is represented by value measure in gbps, mbps, kbps or bps, e.g. + Limit is represented by value measure in Gbps, Mbps, kbps, e.g. 10kbps type: string percentage: diff --git a/app/kumactl/cmd/install/testdata/install-control-plane.with-helm-set.yaml b/app/kumactl/cmd/install/testdata/install-control-plane.with-helm-set.yaml index 3d324b8dfb0d..24e325ca961d 100644 --- a/app/kumactl/cmd/install/testdata/install-control-plane.with-helm-set.yaml +++ b/app/kumactl/cmd/install/testdata/install-control-plane.with-helm-set.yaml @@ -332,7 +332,7 @@ spec: properties: limit: description: |- - Limit is represented by value measure in gbps, mbps, kbps or bps, e.g. + Limit is represented by value measure in Gbps, Mbps, kbps, e.g. 10kbps type: string percentage: @@ -508,7 +508,7 @@ spec: properties: limit: description: |- - Limit is represented by value measure in gbps, mbps, kbps or bps, e.g. + Limit is represented by value measure in Gbps, Mbps, kbps, e.g. 10kbps type: string percentage: diff --git a/app/kumactl/cmd/install/testdata/install-crds.all.golden.yaml b/app/kumactl/cmd/install/testdata/install-crds.all.golden.yaml index 6342dbc8ede5..e2665249929d 100644 --- a/app/kumactl/cmd/install/testdata/install-crds.all.golden.yaml +++ b/app/kumactl/cmd/install/testdata/install-crds.all.golden.yaml @@ -1786,7 +1786,7 @@ spec: properties: limit: description: |- - Limit is represented by value measure in gbps, mbps, kbps or bps, e.g. + Limit is represented by value measure in Gbps, Mbps, kbps, e.g. 10kbps type: string percentage: @@ -1962,7 +1962,7 @@ spec: properties: limit: description: |- - Limit is represented by value measure in gbps, mbps, kbps or bps, e.g. + Limit is represented by value measure in Gbps, Mbps, kbps, e.g. 10kbps type: string percentage: diff --git a/app/kumactl/cmd/install/testdata/install-crds.experimental-gatewayapi.golden.yaml b/app/kumactl/cmd/install/testdata/install-crds.experimental-gatewayapi.golden.yaml index 483c5ca900a0..b775b7d64628 100644 --- a/app/kumactl/cmd/install/testdata/install-crds.experimental-gatewayapi.golden.yaml +++ b/app/kumactl/cmd/install/testdata/install-crds.experimental-gatewayapi.golden.yaml @@ -1786,7 +1786,7 @@ spec: properties: limit: description: |- - Limit is represented by value measure in gbps, mbps, kbps or bps, e.g. + Limit is represented by value measure in Gbps, Mbps, kbps, e.g. 10kbps type: string percentage: @@ -1962,7 +1962,7 @@ spec: properties: limit: description: |- - Limit is represented by value measure in gbps, mbps, kbps or bps, e.g. + Limit is represented by value measure in Gbps, Mbps, kbps, e.g. 10kbps type: string percentage: diff --git a/deployments/charts/kuma/crds/kuma.io_meshfaultinjections.yaml b/deployments/charts/kuma/crds/kuma.io_meshfaultinjections.yaml index 1e5ff5ec9cb6..4150c0fdde14 100644 --- a/deployments/charts/kuma/crds/kuma.io_meshfaultinjections.yaml +++ b/deployments/charts/kuma/crds/kuma.io_meshfaultinjections.yaml @@ -115,7 +115,7 @@ spec: properties: limit: description: |- - Limit is represented by value measure in gbps, mbps, kbps or bps, e.g. + Limit is represented by value measure in Gbps, Mbps, kbps, e.g. 10kbps type: string percentage: @@ -291,7 +291,7 @@ spec: properties: limit: description: |- - Limit is represented by value measure in gbps, mbps, kbps or bps, e.g. + Limit is represented by value measure in Gbps, Mbps, kbps, e.g. 10kbps type: string percentage: diff --git a/docs/generated/openapi.yaml b/docs/generated/openapi.yaml index 0f1bb647d0d6..ec076787f331 100644 --- a/docs/generated/openapi.yaml +++ b/docs/generated/openapi.yaml @@ -3524,7 +3524,7 @@ components: limit: description: >- Limit is represented by value measure in - gbps, mbps, kbps or bps, e.g. + Gbps, Mbps, kbps, e.g. 10kbps type: string @@ -3742,7 +3742,7 @@ components: limit: description: >- Limit is represented by value measure in - gbps, mbps, kbps or bps, e.g. + Gbps, Mbps, kbps, e.g. 10kbps type: string diff --git a/docs/generated/raw/crds/kuma.io_meshfaultinjections.yaml b/docs/generated/raw/crds/kuma.io_meshfaultinjections.yaml index 1e5ff5ec9cb6..4150c0fdde14 100644 --- a/docs/generated/raw/crds/kuma.io_meshfaultinjections.yaml +++ b/docs/generated/raw/crds/kuma.io_meshfaultinjections.yaml @@ -115,7 +115,7 @@ spec: properties: limit: description: |- - Limit is represented by value measure in gbps, mbps, kbps or bps, e.g. + Limit is represented by value measure in Gbps, Mbps, kbps, e.g. 10kbps type: string percentage: @@ -291,7 +291,7 @@ spec: properties: limit: description: |- - Limit is represented by value measure in gbps, mbps, kbps or bps, e.g. + Limit is represented by value measure in Gbps, Mbps, kbps, e.g. 10kbps type: string percentage: diff --git a/pkg/core/validators/common_validators.go b/pkg/core/validators/common_validators.go index 8ffb49c69db1..e4eb2dfc0066 100644 --- a/pkg/core/validators/common_validators.go +++ b/pkg/core/validators/common_validators.go @@ -92,7 +92,7 @@ func ValidateIntPercentageOrNil(path PathBuilder, percentage *int32) ValidationE return err } -func ValidatePercentage(path PathBuilder, percentage *intstr.IntOrString) ValidationError { +func ValidatePercentage(path PathBuilder, percentage *intstr.IntOrString, allow0 bool) ValidationError { var verr ValidationError if percentage == nil { verr.AddViolationAt(path, MustBeDefined) @@ -102,10 +102,14 @@ func ValidatePercentage(path PathBuilder, percentage *intstr.IntOrString) Valida dec, err := common_api.NewDecimalFromIntOrString(*percentage) if err != nil { verr.AddViolationAt(path, StringHasToBeValidNumber) + // Return early to avoid spam of similar errors + return verr } if dec.LessThan(decimal.Zero) || dec.GreaterThan(decimal.NewFromInt(100)) { verr.AddViolationAt(path, HasToBeInPercentageRange) + } else if !allow0 && dec.IsZero() { + verr.AddViolationAt(path, HasToBeGreaterThanZero) } return verr } @@ -119,31 +123,14 @@ func ValidatePercentageOrNil(path PathBuilder, percentage *intstr.IntOrString) V dec, err := common_api.NewDecimalFromIntOrString(*percentage) if err != nil { verr.AddViolationAt(path, StringHasToBeValidNumber) + return verr } - if dec.LessThan(decimal.Zero) || dec.GreaterThan(decimal.NewFromInt(100)) { verr.AddViolationAt(path, HasToBeInPercentageRange) } return verr } -func ValidateIntOrStringGreaterThan(path PathBuilder, number *intstr.IntOrString, minValue int) ValidationError { - var verr ValidationError - if number == nil { - return verr - } - - dec, err := common_api.NewDecimalFromIntOrString(*number) - if err != nil { - verr.AddViolationAt(path, StringHasToBeValidNumber) - } - if dec.LessThanOrEqual(decimal.NewFromInt(int64(minValue))) { - verr.AddViolationAt(path, fmt.Sprintf("%s: %d", HasToBeGreaterThan, minValue)) - } - - return verr -} - func ValidateIntOrStringGreaterOrEqualThan(path PathBuilder, number *intstr.IntOrString, minValue int) ValidationError { var verr ValidationError if number == nil { @@ -159,23 +146,6 @@ func ValidateIntOrStringGreaterOrEqualThan(path PathBuilder, number *intstr.IntO return verr } -func ValidateIntOrStringLessThan(path PathBuilder, number *intstr.IntOrString, maxValue int) ValidationError { - var verr ValidationError - if number == nil { - return verr - } - - dec, err := common_api.NewDecimalFromIntOrString(*number) - if err != nil { - verr.AddViolationAt(path, StringHasToBeValidNumber) - } - if dec.GreaterThanOrEqual(decimal.NewFromInt(int64(maxValue))) { - verr.AddViolationAt(path, fmt.Sprintf("%s: %d", HasToBeLessThan, maxValue)) - } - - return verr -} - func ValidateUInt32PercentageOrNil(path PathBuilder, percentage *uint32) ValidationError { var err ValidationError if percentage == nil { @@ -215,7 +185,7 @@ func ValidatePathOrNil(path PathBuilder, filePath *string) ValidationError { func ValidateStatusCode(path PathBuilder, status int32) ValidationError { var err ValidationError if status < 100 || status >= 600 { - err.AddViolationAt(path, "must be in range [100, 600)") + err.AddViolationAt(path, fmt.Sprintf(HasToBeInRangeFormat, 100, 599)) } return err @@ -253,14 +223,16 @@ func ValidateIntegerGreaterThan(path PathBuilder, value uint32, minValue uint32) return err } +var BandwidthRegex = regexp.MustCompile(`(\d*)\s?([GMk]?bps)`) + func ValidateBandwidth(path PathBuilder, value string) ValidationError { var err ValidationError if value == "" { err.AddViolationAt(path, MustBeDefined) return err } - if matched, _ := regexp.MatchString(`\d*\s?[gmk]bps`, value); !matched { - err.AddViolationAt(path, "has to be in kbps/mbps/gbps units") + if matched := BandwidthRegex.MatchString(value); !matched { + err.AddViolationAt(path, MustHaveBPSUnit) } return err } diff --git a/pkg/core/validators/messages.go b/pkg/core/validators/messages.go index 7e48e153ac10..71b2d1ac146b 100644 --- a/pkg/core/validators/messages.go +++ b/pkg/core/validators/messages.go @@ -18,10 +18,15 @@ const ( MustBeDefinedAndGreaterThanZero = "must be defined and greater than zero" WhenDefinedHasToBeNonNegative = "must not be negative when defined" WhenDefinedHasToBeGreaterThanZero = "must be greater than zero when defined" - HasToBeInPercentageRange = "has to be in [0.0 - 100.0] range" - HasToBeInUintPercentageRange = "has to be in [0 - 100] range" - WhenDefinedHasToBeValidPath = "has to be a valid path when defined" - StringHasToBeValidNumber = "string has to be a valid number" + HasToBeInRangeFormat = "must be in inclusive range [%v, %v]" + WhenDefinedHasToBeValidPath = "must be a valid path when defined" + StringHasToBeValidNumber = "string must be a valid number" + MustHaveBPSUnit = "must be in kbps/Mbps/Gbps units" +) + +var ( + HasToBeInPercentageRange = fmt.Sprintf(HasToBeInRangeFormat, "0.0", "100.0") + HasToBeInUintPercentageRange = fmt.Sprintf(HasToBeInRangeFormat, 0, 100) ) func MustHaveOnlyOne(entity string, allowedValues ...string) string { diff --git a/pkg/plugins/policies/meshcircuitbreaker/api/v1alpha1/validator_test.go b/pkg/plugins/policies/meshcircuitbreaker/api/v1alpha1/validator_test.go index 164d14c7b4c1..e51d9eef60db 100644 --- a/pkg/plugins/policies/meshcircuitbreaker/api/v1alpha1/validator_test.go +++ b/pkg/plugins/policies/meshcircuitbreaker/api/v1alpha1/validator_test.go @@ -414,9 +414,9 @@ to: expected: ` violations: - field: spec.to[0].default.outlierDetection.maxEjectionPercent - message: has to be in [0 - 100] range + message: must be in inclusive range [0, 100] - field: spec.to[0].default.outlierDetection.detectors.failurePercentage.threshold - message: has to be in [0 - 100] range`, + message: must be in inclusive range [0, 100]`, }), Entry("detectors are not defined", testCase{ inputYaml: ` diff --git a/pkg/plugins/policies/meshfaultinjection/api/v1alpha1/meshfaultinjection.go b/pkg/plugins/policies/meshfaultinjection/api/v1alpha1/meshfaultinjection.go index 8ea391e5216a..faee84859a89 100644 --- a/pkg/plugins/policies/meshfaultinjection/api/v1alpha1/meshfaultinjection.go +++ b/pkg/plugins/policies/meshfaultinjection/api/v1alpha1/meshfaultinjection.go @@ -75,7 +75,7 @@ type DelayConf struct { } type ResponseBandwidthConf struct { - // Limit is represented by value measure in gbps, mbps, kbps or bps, e.g. + // Limit is represented by value measure in Gbps, Mbps, kbps, e.g. // 10kbps Limit string `json:"limit"` // Percentage of requests on which response bandwidth limit will be diff --git a/pkg/plugins/policies/meshfaultinjection/api/v1alpha1/schema.yaml b/pkg/plugins/policies/meshfaultinjection/api/v1alpha1/schema.yaml index c36a4ee9b40e..719ea914d68d 100644 --- a/pkg/plugins/policies/meshfaultinjection/api/v1alpha1/schema.yaml +++ b/pkg/plugins/policies/meshfaultinjection/api/v1alpha1/schema.yaml @@ -76,7 +76,7 @@ properties: properties: limit: description: |- - Limit is represented by value measure in gbps, mbps, kbps or bps, e.g. + Limit is represented by value measure in Gbps, Mbps, kbps, e.g. 10kbps type: string percentage: @@ -244,7 +244,7 @@ properties: properties: limit: description: |- - Limit is represented by value measure in gbps, mbps, kbps or bps, e.g. + Limit is represented by value measure in Gbps, Mbps, kbps, e.g. 10kbps type: string percentage: diff --git a/pkg/plugins/policies/meshfaultinjection/api/v1alpha1/validator.go b/pkg/plugins/policies/meshfaultinjection/api/v1alpha1/validator.go index 82c1c6d99c61..d8879fa4378c 100644 --- a/pkg/plugins/policies/meshfaultinjection/api/v1alpha1/validator.go +++ b/pkg/plugins/policies/meshfaultinjection/api/v1alpha1/validator.go @@ -80,17 +80,17 @@ func validateDefault(path validators.PathBuilder, conf Conf) validators.Validati if fault.Abort != nil { path := path.Field("abort").Index(idx) verr.Add(validators.ValidateStatusCode(path.Field("httpStatus"), fault.Abort.HttpStatus)) - verr.Add(validators.ValidatePercentage(path.Field("percentage"), &fault.Abort.Percentage)) + verr.Add(validators.ValidatePercentage(path.Field("percentage"), &fault.Abort.Percentage, true)) } if fault.Delay != nil { path := path.Field("delay").Index(idx) verr.Add(validators.ValidateDurationNotNegative(path.Field("value"), &fault.Delay.Value)) - verr.Add(validators.ValidatePercentage(path.Field("percentage"), &fault.Delay.Percentage)) + verr.Add(validators.ValidatePercentage(path.Field("percentage"), &fault.Delay.Percentage, true)) } if fault.ResponseBandwidth != nil { path := path.Field("responseBandwidth").Index(idx) verr.Add(validators.ValidateBandwidth(path.Field("responseBandwidth"), fault.ResponseBandwidth.Limit)) - verr.Add(validators.ValidatePercentage(path.Field("percentage"), &fault.ResponseBandwidth.Percentage)) + verr.Add(validators.ValidatePercentage(path.Field("percentage"), &fault.ResponseBandwidth.Percentage, true)) } } return verr diff --git a/pkg/plugins/policies/meshfaultinjection/api/v1alpha1/validator_test.go b/pkg/plugins/policies/meshfaultinjection/api/v1alpha1/validator_test.go index 55a79d8bfea5..3b74a8cab05a 100644 --- a/pkg/plugins/policies/meshfaultinjection/api/v1alpha1/validator_test.go +++ b/pkg/plugins/policies/meshfaultinjection/api/v1alpha1/validator_test.go @@ -75,11 +75,11 @@ to: []validators.Violation{ { Field: `spec.from[0].default.http.abort[0].httpStatus`, - Message: `must be in range [100, 600)`, + Message: `must be in inclusive range [100, 599]`, }, { Field: `spec.from[0].default.http.abort[0].percentage`, - Message: `has to be in [0.0 - 100.0] range`, + Message: `must be in inclusive range [0.0, 100.0]`, }, { Field: "spec.from[0].default.http.delay[1].value", @@ -87,15 +87,15 @@ to: }, { Field: `spec.from[0].default.http.delay[1].percentage`, - Message: `has to be in [0.0 - 100.0] range`, + Message: `must be in inclusive range [0.0, 100.0]`, }, { Field: `spec.from[0].default.http.responseBandwidth[2].responseBandwidth`, - Message: `has to be in kbps/mbps/gbps units`, + Message: `must be in kbps/Mbps/Gbps units`, }, { Field: `spec.from[0].default.http.responseBandwidth[2].percentage`, - Message: `has to be in [0.0 - 100.0] range`, + Message: `must be in inclusive range [0.0, 100.0]`, }, }, ` type: MeshFaultInjection @@ -124,11 +124,11 @@ from: []validators.Violation{ { Field: "spec.from[0].default.http.abort[0].httpStatus", - Message: "must be in range [100, 600)", + Message: "must be in inclusive range [100, 599]", }, { Field: "spec.from[0].default.http.responseBandwidth[2].responseBandwidth", - Message: "has to be in kbps/mbps/gbps units", + Message: "must be in kbps/Mbps/Gbps units", }, }, ` type: MeshFaultInjection @@ -152,11 +152,11 @@ from: []validators.Violation{ { Field: "spec.from[0].default.http.responseBandwidth[0].responseBandwidth", - Message: "has to be in kbps/mbps/gbps units", + Message: "must be in kbps/Mbps/Gbps units", }, { Field: "spec.from[0].default.http.responseBandwidth[0].percentage", - Message: "string has to be a valid number", + Message: "string must be a valid number", }, }, ` type: MeshFaultInjection diff --git a/pkg/plugins/policies/meshfaultinjection/k8s/crd/kuma.io_meshfaultinjections.yaml b/pkg/plugins/policies/meshfaultinjection/k8s/crd/kuma.io_meshfaultinjections.yaml index 1e5ff5ec9cb6..4150c0fdde14 100644 --- a/pkg/plugins/policies/meshfaultinjection/k8s/crd/kuma.io_meshfaultinjections.yaml +++ b/pkg/plugins/policies/meshfaultinjection/k8s/crd/kuma.io_meshfaultinjections.yaml @@ -115,7 +115,7 @@ spec: properties: limit: description: |- - Limit is represented by value measure in gbps, mbps, kbps or bps, e.g. + Limit is represented by value measure in Gbps, Mbps, kbps, e.g. 10kbps type: string percentage: @@ -291,7 +291,7 @@ spec: properties: limit: description: |- - Limit is represented by value measure in gbps, mbps, kbps or bps, e.g. + Limit is represented by value measure in Gbps, Mbps, kbps, e.g. 10kbps type: string percentage: diff --git a/pkg/plugins/policies/meshfaultinjection/plugin/v1alpha1/plugin_test.go b/pkg/plugins/policies/meshfaultinjection/plugin/v1alpha1/plugin_test.go index c890e3728977..754027eacff5 100644 --- a/pkg/plugins/policies/meshfaultinjection/plugin/v1alpha1/plugin_test.go +++ b/pkg/plugins/policies/meshfaultinjection/plugin/v1alpha1/plugin_test.go @@ -137,7 +137,7 @@ var _ = Describe("MeshFaultInjection", func() { Percentage: intstr.FromString("55"), }, ResponseBandwidth: &api.ResponseBandwidthConf{ - Limit: "111mbps", + Limit: "111Mbps", Percentage: intstr.FromString("62.9"), }, }, @@ -164,7 +164,7 @@ var _ = Describe("MeshFaultInjection", func() { Percentage: intstr.FromInt32(22), }, ResponseBandwidth: &api.ResponseBandwidthConf{ - Limit: "333mbps", + Limit: "333Mbps", Percentage: intstr.FromString("33.3"), }, }, @@ -186,7 +186,7 @@ var _ = Describe("MeshFaultInjection", func() { Percentage: intstr.FromInt(55), }, ResponseBandwidth: &api.ResponseBandwidthConf{ - Limit: "111mbps", + Limit: "111Mbps", Percentage: intstr.FromString("62.9"), }, }, @@ -305,7 +305,7 @@ var _ = Describe("MeshFaultInjection", func() { Percentage: intstr.FromString("55"), }, ResponseBandwidth: &api.ResponseBandwidthConf{ - Limit: "111mbps", + Limit: "111Mbps", Percentage: intstr.FromString("62.9"), }, }, @@ -374,7 +374,7 @@ var _ = Describe("MeshFaultInjection", func() { Percentage: intstr.FromString("55"), }, ResponseBandwidth: &api.ResponseBandwidthConf{ - Limit: "111mbps", + Limit: "111Mbps", Percentage: intstr.FromString("62.9"), }, }, @@ -407,7 +407,7 @@ var _ = Describe("MeshFaultInjection", func() { Percentage: intstr.FromString("55"), }, ResponseBandwidth: &api.ResponseBandwidthConf{ - Limit: "111mbps", + Limit: "111Mbps", Percentage: intstr.FromString("62.9"), }, }, @@ -456,7 +456,7 @@ var _ = Describe("MeshFaultInjection", func() { Percentage: intstr.FromString("55"), }, ResponseBandwidth: &api.ResponseBandwidthConf{ - Limit: "111mbps", + Limit: "111Mbps", Percentage: intstr.FromString("62.9"), }, }, diff --git a/pkg/plugins/policies/meshhealthcheck/api/v1alpha1/validator_test.go b/pkg/plugins/policies/meshhealthcheck/api/v1alpha1/validator_test.go index ad5405b06575..4e76b308ea9b 100644 --- a/pkg/plugins/policies/meshhealthcheck/api/v1alpha1/validator_test.go +++ b/pkg/plugins/policies/meshhealthcheck/api/v1alpha1/validator_test.go @@ -220,9 +220,9 @@ to: expected: ` violations: - field: spec.to[0].default.intervalJitterPercent - message: has to be in [0 - 100] range + message: must be in inclusive range [0, 100] - field: spec.to[0].default.healthyPanicThreshold - message: has to be in [0.0 - 100.0] range`, + message: must be in inclusive range [0.0, 100.0]`, }), Entry("path is invalid", testCase{ inputYaml: ` @@ -244,7 +244,7 @@ to: expected: ` violations: - field: spec.to[0].default.eventLogPath - message: has to be a valid path when defined`, + message: must be a valid path when defined`, }), Entry("status codes out of range in expectedStatuses", testCase{ inputYaml: ` @@ -267,9 +267,9 @@ to: expected: ` violations: - field: spec.to[0].default.http.expectedStatuses[0] - message: must be in range [100, 600) + message: must be in inclusive range [100, 599] - field: spec.to[0].default.http.expectedStatuses[1] - message: must be in range [100, 600)`, + message: must be in inclusive range [100, 599]`, }), ) }) diff --git a/pkg/plugins/policies/meshloadbalancingstrategy/api/v1alpha1/validator.go b/pkg/plugins/policies/meshloadbalancingstrategy/api/v1alpha1/validator.go index a7d98eb37ee1..54c420a67aaf 100644 --- a/pkg/plugins/policies/meshloadbalancingstrategy/api/v1alpha1/validator.go +++ b/pkg/plugins/policies/meshloadbalancingstrategy/api/v1alpha1/validator.go @@ -136,8 +136,7 @@ func validateFailoverThreshold(failoverThreshold *FailoverThreshold) validators. if failoverThreshold == nil { return verr } - verr.Add(validators.ValidateIntOrStringGreaterThan(validators.RootedAt("percentage"), &failoverThreshold.Percentage, 0)) - verr.Add(validators.ValidateIntOrStringLessThan(validators.RootedAt("percentage"), &failoverThreshold.Percentage, 100)) + verr.Add(validators.ValidatePercentage(validators.RootedAt("percentage"), &failoverThreshold.Percentage, false)) return verr } diff --git a/pkg/plugins/policies/meshloadbalancingstrategy/api/v1alpha1/validator_test.go b/pkg/plugins/policies/meshloadbalancingstrategy/api/v1alpha1/validator_test.go index cb11d7a89cf2..48e4cf049b7a 100644 --- a/pkg/plugins/policies/meshloadbalancingstrategy/api/v1alpha1/validator_test.go +++ b/pkg/plugins/policies/meshloadbalancingstrategy/api/v1alpha1/validator_test.go @@ -292,7 +292,7 @@ to: `), resources.ErrorCases("percentage can't be zero", []validators.Violation{{ Field: "spec.to[0].default.localityAwareness.crossZone.failoverThreshold.percentage", - Message: "must be greater than: 0", + Message: "must be greater than 0", }}, ` type: MeshLoadBalancingStrategy mesh: mesh-1 @@ -309,9 +309,9 @@ to: failoverThreshold: percentage: 0 `), - resources.ErrorCases("percentage can't be set to 100", []validators.Violation{{ + resources.ErrorCases("percentage is not a parseable number", []validators.Violation{{ Field: "spec.to[0].default.localityAwareness.crossZone.failoverThreshold.percentage", - Message: "must be less than: 100", + Message: "string must be a valid number", }}, ` type: MeshLoadBalancingStrategy mesh: mesh-1 @@ -326,7 +326,7 @@ to: localityAwareness: crossZone: failoverThreshold: - percentage: 100 + percentage: "hello" `), resources.ErrorCases("broken failover rules", []validators.Violation{ { diff --git a/pkg/plugins/policies/meshloadbalancingstrategy/plugin/v1alpha1/locality_aware.go b/pkg/plugins/policies/meshloadbalancingstrategy/plugin/v1alpha1/locality_aware.go index 1125d38a52f8..3ffde922094b 100644 --- a/pkg/plugins/policies/meshloadbalancingstrategy/plugin/v1alpha1/locality_aware.go +++ b/pkg/plugins/policies/meshloadbalancingstrategy/plugin/v1alpha1/locality_aware.go @@ -11,6 +11,7 @@ import ( "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/known/wrapperspb" + common_api "github.com/kumahq/kuma/api/common/v1alpha1" mesh_proto "github.com/kumahq/kuma/api/mesh/v1alpha1" core_xds "github.com/kumahq/kuma/pkg/core/xds" api "github.com/kumahq/kuma/pkg/plugins/policies/meshloadbalancingstrategy/api/v1alpha1" @@ -114,15 +115,15 @@ func ConfigureEndpointLocalityAwareLb( clusterLb := loadAssignment.(*envoy_endpoint.ClusterLoadAssignment) overprovisingFactor := defaultOverprovisingFactor if conf.LocalityAwareness.CrossZone != nil && conf.LocalityAwareness.CrossZone.FailoverThreshold != nil { - overprovisingFactor = uint32(100/conf.LocalityAwareness.CrossZone.FailoverThreshold.Percentage.IntVal) * 100 + val, err := common_api.NewDecimalFromIntOrString(conf.LocalityAwareness.CrossZone.FailoverThreshold.Percentage) + if err == nil && !val.IsZero() { + overprovisingFactor = uint32(100/val.InexactFloat64()) * 100 + } } if clusterLb.Policy == nil { - clusterLb.Policy = &envoy_endpoint.ClusterLoadAssignment_Policy{ - OverprovisioningFactor: wrapperspb.UInt32(overprovisingFactor), - } - } else { - clusterLb.Policy.OverprovisioningFactor = wrapperspb.UInt32(overprovisingFactor) + clusterLb.Policy = &envoy_endpoint.ClusterLoadAssignment_Policy{} } + clusterLb.Policy.OverprovisioningFactor = wrapperspb.UInt32(overprovisingFactor) return loadAssignment, nil } diff --git a/pkg/plugins/policies/meshloadbalancingstrategy/plugin/v1alpha1/plugin_test.go b/pkg/plugins/policies/meshloadbalancingstrategy/plugin/v1alpha1/plugin_test.go index e9881b29de57..3729f781f8ba 100644 --- a/pkg/plugins/policies/meshloadbalancingstrategy/plugin/v1alpha1/plugin_test.go +++ b/pkg/plugins/policies/meshloadbalancingstrategy/plugin/v1alpha1/plugin_test.go @@ -472,6 +472,7 @@ var _ = Describe("MeshLoadBalancingStrategy", func() { }, }, CrossZone: &v1alpha1.CrossZone{ + FailoverThreshold: &v1alpha1.FailoverThreshold{Percentage: intstr.FromString("99")}, Failover: []v1alpha1.Failover{ { To: v1alpha1.ToZone{ diff --git a/pkg/plugins/policies/meshloadbalancingstrategy/plugin/v1alpha1/testdata/locality_aware_basic.endpoints.golden.yaml b/pkg/plugins/policies/meshloadbalancingstrategy/plugin/v1alpha1/testdata/locality_aware_basic.endpoints.golden.yaml index 1826a38f9ccc..6c98a7cb1265 100644 --- a/pkg/plugins/policies/meshloadbalancingstrategy/plugin/v1alpha1/testdata/locality_aware_basic.endpoints.golden.yaml +++ b/pkg/plugins/policies/meshloadbalancingstrategy/plugin/v1alpha1/testdata/locality_aware_basic.endpoints.golden.yaml @@ -135,4 +135,4 @@ resources: zone: zone-4 priority: 3 policy: - overprovisioningFactor: 200 + overprovisioningFactor: 100 diff --git a/pkg/test/resources/validation.go b/pkg/test/resources/validation.go index 1e57232b4692..311cdd7a6b31 100644 --- a/pkg/test/resources/validation.go +++ b/pkg/test/resources/validation.go @@ -85,6 +85,7 @@ func ErrorCase(description string, err validators.Violation, yaml string) TableE } func ErrorCases(description string, errs []validators.Violation, yaml string) TableEntry { + GinkgoHelper() return Entry( description, ResourceValidationCase{ diff --git a/pkg/xds/envoy/listeners/v3/util.go b/pkg/xds/envoy/listeners/v3/util.go index d975cf065941..b54533f6c31f 100644 --- a/pkg/xds/envoy/listeners/v3/util.go +++ b/pkg/xds/envoy/listeners/v3/util.go @@ -3,7 +3,6 @@ package v3 import ( "fmt" "math" - "regexp" "strconv" envoy_listener "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3" @@ -14,6 +13,8 @@ import ( "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/known/anypb" "google.golang.org/protobuf/types/known/wrapperspb" + + "github.com/kumahq/kuma/pkg/core/validators" ) func UpdateHTTPConnectionManager(filterChain *envoy_listener.FilterChain, updateFunc func(manager *envoy_hcm.HttpConnectionManager) error) error { @@ -110,10 +111,8 @@ func ConvertPercentage(percentage *wrapperspb.DoubleValue) *envoy_type.Fractiona } } -var bandwidthRegex = regexp.MustCompile(`(\d*)\s?([gmk]?bps)`) - func ConvertBandwidthToKbps(bandwidth string) (uint64, error) { - match := bandwidthRegex.FindStringSubmatch(bandwidth) + match := validators.BandwidthRegex.FindStringSubmatch(bandwidth) value, err := strconv.Atoi(match[1]) if err != nil { return 0, err @@ -125,9 +124,9 @@ func ConvertBandwidthToKbps(bandwidth string) (uint64, error) { switch units { case "kbps": factor = 1 - case "mbps": + case "Mbps": factor = 1000 - case "gbps": + case "Gbps": factor = 1000000 default: return 0, errors.New("unsupported unit type") diff --git a/pkg/xds/envoy/listeners/v3/util_test.go b/pkg/xds/envoy/listeners/v3/util_test.go index df38c4608f89..037a5a338e69 100644 --- a/pkg/xds/envoy/listeners/v3/util_test.go +++ b/pkg/xds/envoy/listeners/v3/util_test.go @@ -223,7 +223,7 @@ var _ = Describe("ConvertBandwidth", func() { input string expected uint64 } - DescribeTable("should properly converts to kbps from gbps, mbps, kbps", + DescribeTable("should properly convert to kbps from gbps, mbps, kbps", func(given testCase) { // when limitKbps, err := ConvertBandwidthToKbps(given.input) @@ -236,11 +236,11 @@ var _ = Describe("ConvertBandwidth", func() { expected: 120, }), Entry("mbps input", testCase{ - input: "120 mbps", + input: "120 Mbps", expected: 120000, }), Entry("gbps input", testCase{ - input: "120 gbps", + input: "120 Gbps", expected: 120000000, }), )