From f90059882dd71a7e61b09d5e85156dbbd61ec211 Mon Sep 17 00:00:00 2001 From: Omer Aplatony Date: Sat, 30 Nov 2024 17:25:39 +0200 Subject: [PATCH 1/2] fix: truncate label values longer than 63 characters Signed-off-by: Omer Aplatony --- controllers/keda/hpa.go | 22 ++++------- controllers/keda/scaledobject_controller.go | 11 +++--- controllers/keda/util/string.go | 16 ++++++++ controllers/keda/util/string_test.go | 42 +++++++++++++++++++++ 4 files changed, 72 insertions(+), 19 deletions(-) create mode 100644 controllers/keda/util/string.go create mode 100644 controllers/keda/util/string_test.go diff --git a/controllers/keda/hpa.go b/controllers/keda/hpa.go index 5fb2c835eb7..1047a77bc6a 100644 --- a/controllers/keda/hpa.go +++ b/controllers/keda/hpa.go @@ -21,8 +21,6 @@ import ( "fmt" "sort" "strconv" - "strings" - "unicode" "github.com/go-logr/logr" autoscalingv2 "k8s.io/api/autoscaling/v2" @@ -82,21 +80,16 @@ func (r *ScaledObjectReconciler) newHPAForScaledObject(ctx context.Context, logg } // label can have max 63 chars - labelName := getHPAName(scaledObject) - if len(labelName) > 63 { - labelName = labelName[:63] - labelName = strings.TrimRightFunc(labelName, func(r rune) bool { - return !unicode.IsLetter(r) && !unicode.IsNumber(r) - }) - } + labelName := kedacontrollerutil.Truncate(getHPAName(scaledObject), 63) + scaleObjectName := kedacontrollerutil.Truncate(scaledObject.Name, 63) labels := map[string]string{ "app.kubernetes.io/name": labelName, "app.kubernetes.io/version": version.Version, - "app.kubernetes.io/part-of": scaledObject.Name, + "app.kubernetes.io/part-of": scaleObjectName, "app.kubernetes.io/managed-by": "keda-operator", } for key, value := range scaledObject.ObjectMeta.Labels { - labels[key] = value + labels[key] = kedacontrollerutil.Truncate(value, 63) } minReplicas := scaledObject.GetHPAMinReplicas() @@ -221,6 +214,7 @@ func (r *ScaledObjectReconciler) getScaledObjectMetricSpecs(ctx context.Context, } metricSpecs := cache.GetMetricSpecForScaling(ctx) + scaledObjectName := kedacontrollerutil.Truncate(scaledObject.Name, 63) for _, metricSpec := range metricSpecs { if metricSpec.Resource != nil { @@ -230,12 +224,12 @@ func (r *ScaledObjectReconciler) getScaledObjectMetricSpecs(ctx context.Context, if metricSpec.External != nil { externalMetricName := metricSpec.External.Metric.Name if kedacontrollerutil.Contains(externalMetricNames, externalMetricName) { - return nil, fmt.Errorf("metricName %s defined multiple times in ScaledObject %s", externalMetricName, scaledObject.Name) + return nil, fmt.Errorf("metricName %s defined multiple times in ScaledObject %s", externalMetricName, scaledObjectName) } // add the scaledobject.keda.sh/name label. This is how the MetricsAdapter will know which scaledobject a metric is for when the HPA queries it. metricSpec.External.Metric.Selector = &metav1.LabelSelector{MatchLabels: make(map[string]string)} - metricSpec.External.Metric.Selector.MatchLabels[kedav1alpha1.ScaledObjectOwnerAnnotation] = scaledObject.Name + metricSpec.External.Metric.Selector.MatchLabels[kedav1alpha1.ScaledObjectOwnerAnnotation] = scaledObjectName externalMetricNames = append(externalMetricNames, externalMetricName) } } @@ -293,7 +287,7 @@ func (r *ScaledObjectReconciler) getScaledObjectMetricSpecs(ctx context.Context, Metric: autoscalingv2.MetricIdentifier{ Name: compMetricName, Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{kedav1alpha1.ScaledObjectOwnerAnnotation: scaledObject.Name}, + MatchLabels: map[string]string{kedav1alpha1.ScaledObjectOwnerAnnotation: scaledObjectName}, }, }, Target: correctHpaTarget, diff --git a/controllers/keda/scaledobject_controller.go b/controllers/keda/scaledobject_controller.go index c480dc380c4..d4ccb87b32c 100755 --- a/controllers/keda/scaledobject_controller.go +++ b/controllers/keda/scaledobject_controller.go @@ -209,7 +209,7 @@ func (r *ScaledObjectReconciler) Reconcile(ctx context.Context, req ctrl.Request conditions.SetFallbackCondition(metav1.ConditionFalse, "NoFallbackFound", "No fallbacks are active on this scaled object") } - metricscollector.RecordScaledObjectPaused(scaledObject.Namespace, scaledObject.Name, conditions.GetPausedCondition().Status == metav1.ConditionTrue) + metricscollector.RecordScaledObjectPaused(scaledObject.Namespace, kedacontrollerutil.Truncate(scaledObject.Name, 63), conditions.GetPausedCondition().Status == metav1.ConditionTrue) if err := kedastatus.SetStatusConditions(ctx, r.Client, reqLogger, scaledObject, &conditions); err != nil { r.EventEmitter.Emit(scaledObject, req.NamespacedName.Namespace, corev1.EventTypeWarning, eventingv1alpha1.ScaledObjectFailedType, eventreason.ScaledObjectUpdateFailed, err.Error()) @@ -319,17 +319,18 @@ func (r *ScaledObjectReconciler) reconcileScaledObject(ctx context.Context, logg // ensureScaledObjectLabel ensures that scaledobject.keda.sh/name= label exist in the ScaledObject // This is how the MetricsAdapter will know which ScaledObject a metric is for when the HPA queries it. func (r *ScaledObjectReconciler) ensureScaledObjectLabel(ctx context.Context, logger logr.Logger, scaledObject *kedav1alpha1.ScaledObject) error { + scaledObjectNameTruncated := kedacontrollerutil.Truncate(scaledObject.Name, 63) if scaledObject.Labels == nil { - scaledObject.Labels = map[string]string{kedav1alpha1.ScaledObjectOwnerAnnotation: scaledObject.Name} + scaledObject.Labels = map[string]string{kedav1alpha1.ScaledObjectOwnerAnnotation: scaledObjectNameTruncated} } else { value, found := scaledObject.Labels[kedav1alpha1.ScaledObjectOwnerAnnotation] - if found && value == scaledObject.Name { + if found && value == scaledObjectNameTruncated { return nil } - scaledObject.Labels[kedav1alpha1.ScaledObjectOwnerAnnotation] = scaledObject.Name + scaledObject.Labels[kedav1alpha1.ScaledObjectOwnerAnnotation] = scaledObjectNameTruncated } - logger.V(1).Info("Adding \"scaledobject.keda.sh/name\" label on ScaledObject", "value", scaledObject.Name) + logger.V(1).Info("Adding \"scaledobject.keda.sh/name\" label on ScaledObject", "value", scaledObjectNameTruncated) return r.Client.Update(ctx, scaledObject) } diff --git a/controllers/keda/util/string.go b/controllers/keda/util/string.go new file mode 100644 index 00000000000..6c194b779d9 --- /dev/null +++ b/controllers/keda/util/string.go @@ -0,0 +1,16 @@ +package util + +import ( + "strings" + "unicode" +) + +func Truncate(s string, threshold int) string { + if len(s) > threshold { + s = s[:threshold] + s = strings.TrimRightFunc(s, func(r rune) bool { + return !unicode.IsLetter(r) && !unicode.IsNumber(r) + }) + } + return s +} diff --git a/controllers/keda/util/string_test.go b/controllers/keda/util/string_test.go new file mode 100644 index 00000000000..3ec106a8d3a --- /dev/null +++ b/controllers/keda/util/string_test.go @@ -0,0 +1,42 @@ +package util + +import ( + "testing" +) + +func TestTruncate(t *testing.T) { + tests := []struct { + name string + input string + threshold int + want string + }{ + { + name: "string shorter than threshold", + input: "hello", + threshold: 10, + want: "hello", + }, + { + name: "string longer than threshold ending with special chars", + input: "abc---def---ghi---", + threshold: 5, + want: "abc", + }, + { + name: "63 character limit case", + input: "this-is-64-characters-name-xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", + threshold: 63, + want: "this-is-64-characters-name-xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"[:63], + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := Truncate(tt.input, tt.threshold) + if got != tt.want { + t.Errorf("Truncuate() = %v, want %v", got, tt.want) + } + }) + } +} From 6b168e21de6e62bcf87b5bf108c0d179440b13bc Mon Sep 17 00:00:00 2001 From: Omer Aplatony Date: Sat, 30 Nov 2024 18:06:05 +0200 Subject: [PATCH 2/2] Add header for string and string_test Signed-off-by: Omer Aplatony --- controllers/keda/util/string.go | 16 ++++++++++++++++ controllers/keda/util/string_test.go | 16 ++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/controllers/keda/util/string.go b/controllers/keda/util/string.go index 6c194b779d9..37bfe8b57af 100644 --- a/controllers/keda/util/string.go +++ b/controllers/keda/util/string.go @@ -1,3 +1,19 @@ +/* +Copyright 2021 The KEDA Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package util import ( diff --git a/controllers/keda/util/string_test.go b/controllers/keda/util/string_test.go index 3ec106a8d3a..0cdbbdb829a 100644 --- a/controllers/keda/util/string_test.go +++ b/controllers/keda/util/string_test.go @@ -1,3 +1,19 @@ +/* +Copyright 2021 The KEDA Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package util import (