Skip to content

Commit

Permalink
Fix label truncation possibly ending with invalid character argoproj#…
Browse files Browse the repository at this point in the history
  • Loading branch information
s4nji committed Mar 18, 2024
1 parent 4a92ab7 commit a507f11
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 13 deletions.
19 changes: 16 additions & 3 deletions util/argo/resource_tracking.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package argo

import (
"fmt"
"regexp"
"strings"

"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
Expand All @@ -21,6 +22,7 @@ const (

var WrongResourceTrackingFormat = fmt.Errorf("wrong resource tracking format, should be <application-name>:<group>/<kind>:<namespace>/<name>")
var LabelMaxLength = 63
var nonAlphanumericSuffixRegex = regexp.MustCompile("[^a-zA-Z0-9]+$")

// ResourceTracking defines methods which allow setup and retrieve tracking information to resource
type ResourceTracking interface {
Expand Down Expand Up @@ -153,9 +155,7 @@ func (rt *resourceTracking) SetAppInstance(un *unstructured.Unstructured, key, v
if err != nil {
return err
}
if len(val) > LabelMaxLength {
val = val[:LabelMaxLength]
}
val = truncateLabelValue(val)
err = argokube.SetAppInstanceLabel(un, key, val)
if err != nil {
return fmt.Errorf("failed to set app instance label: %w", err)
Expand Down Expand Up @@ -239,3 +239,16 @@ func (rt *resourceTracking) Normalize(config, live *unstructured.Unstructured, l

return nil
}

// truncateLabelValue truncates input label value string to a valid Kubernetes label value
// Which is limited to 63 characters and must end with alphanumeric characters
// https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set
func truncateLabelValue(str string) string {
if len(str) > LabelMaxLength {
str = str[:LabelMaxLength]
}

// Remove non-alphanumeric characters at the end of the string
str = nonAlphanumericSuffixRegex.ReplaceAllString(str, "")
return str
}
32 changes: 22 additions & 10 deletions util/argo/resource_tracking_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,19 +47,31 @@ func TestSetAppInstanceAnnotation(t *testing.T) {
}

func TestSetAppInstanceAnnotationAndLabel(t *testing.T) {
yamlBytes, err := os.ReadFile("testdata/svc.yaml")
assert.Nil(t, err)
var obj unstructured.Unstructured
err = yaml.Unmarshal(yamlBytes, &obj)
assert.Nil(t, err)
testScenarios := map[string]string{
"my-app": "my-app",
"my-app-with-very-long-app-name-that-will-get-truncated-correctly-1234": "my-app-with-very-long-app-name-that-will-get-truncated-correctl",
"my-app-with-veryvery-long-app-name-that-will-not-get-truncated-correctly-1234": "my-app-with-veryvery-long-app-name-that-will-not-get-truncated",
"my-app-with-a-bunch-of-hyphens-------------------------------------------1234": "my-app-with-a-bunch-of-hyphens",
}

resourceTracking := NewResourceTracking()
for inputAppName, expectedLabelValue := range testScenarios {
yamlBytes, err := os.ReadFile("testdata/svc.yaml")
assert.Nil(t, err)

err = resourceTracking.SetAppInstance(&obj, common.LabelKeyAppInstance, "my-app", "", TrackingMethodAnnotationAndLabel)
assert.Nil(t, err)
var obj unstructured.Unstructured
err = yaml.Unmarshal(yamlBytes, &obj)
assert.Nil(t, err)

app := resourceTracking.GetAppName(&obj, common.LabelKeyAppInstance, TrackingMethodAnnotationAndLabel)
assert.Equal(t, "my-app", app)
resourceTracking := NewResourceTracking()
err = resourceTracking.SetAppInstance(&obj, common.LabelKeyAppInstance, inputAppName, "", TrackingMethodAnnotationAndLabel)
assert.Nil(t, err)

actualAppName := resourceTracking.GetAppName(&obj, common.LabelKeyAppInstance, TrackingMethodAnnotationAndLabel)
assert.Equal(t, inputAppName, actualAppName)

actualLabelValue := obj.GetLabels()[common.LabelKeyAppInstance]
assert.Equal(t, expectedLabelValue, actualLabelValue)
}
}

func TestSetAppInstanceAnnotationNotFound(t *testing.T) {
Expand Down

0 comments on commit a507f11

Please sign in to comment.