fix: app names with non-alphanumeric characters in position 63 break …

…syncs (issue #18237) (#18256)

* Ensure truncated app label does not end in a special character

Signed-off-by: Zack Robinson <[email protected]>

* Move regex to global variable and add out of bounds check

Signed-off-by: Zack Robinson <[email protected]>

* Add test for out-of-bounds check

Signed-off-by: Zack Robinson <[email protected]>

---------

Signed-off-by: Zack Robinson <[email protected]>

util/argo/resource_tracking.go

@@ -1,7 +1,9 @@
 package argo
 
 import (
+	"errors"
 	"fmt"
+	"regexp"
 	"strings"
 
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -21,6 +23,7 @@ const (
 
 var WrongResourceTrackingFormat = fmt.Errorf("wrong resource tracking format, should be <application-name>:<group>/<kind>:<namespace>/<name>")
 var LabelMaxLength = 63
+var OkEndPattern = regexp.MustCompile("[a-zA-Z0-9]$")
 
 // ResourceTracking defines methods which allow setup and retrieve tracking information to resource
 type ResourceTracking interface {
@@ -155,6 +158,14 @@ func (rt *resourceTracking) SetAppInstance(un *unstructured.Unstructured, key, v
 		}
 		if len(val) > LabelMaxLength {
 			val = val[:LabelMaxLength]
+			// Prevent errors if the truncated name ends in a special character.
+			// See https://github.com/argoproj/argo-cd/issues/18237.
+			for !OkEndPattern.MatchString(val) {
+				if len(val) <= 1 {
+					return errors.New("failed to set app instance label: unable to truncate label to not end with a special character")
+				}
+				val = val[:len(val)-1]
+			}
 		}
 		err = argokube.SetAppInstanceLabel(un, key, val)
 		if err != nil {

util/argo/resource_tracking_test.go

@@ -62,6 +62,60 @@ func TestSetAppInstanceAnnotationAndLabel(t *testing.T) {
 	assert.Equal(t, "my-app", app)
 }
 
+func TestSetAppInstanceAnnotationAndLabelLongName(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)
+
+	resourceTracking := NewResourceTracking()
+
+	err = resourceTracking.SetAppInstance(&obj, common.LabelKeyAppInstance, "my-app-with-an-extremely-long-name-that-is-over-sixty-three-characters", "", TrackingMethodAnnotationAndLabel)
+	assert.Nil(t, err)
+
+	// the annotation should still work, so the name from GetAppName should not be truncated
+	app := resourceTracking.GetAppName(&obj, common.LabelKeyAppInstance, TrackingMethodAnnotationAndLabel)
+	assert.Equal(t, "my-app-with-an-extremely-long-name-that-is-over-sixty-three-characters", app)
+
+	// the label should be truncated to 63 characters
+	assert.Equal(t, obj.GetLabels()[common.LabelKeyAppInstance], "my-app-with-an-extremely-long-name-that-is-over-sixty-three-cha")
+}
+
+func TestSetAppInstanceAnnotationAndLabelLongNameBadEnding(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)
+
+	resourceTracking := NewResourceTracking()
+
+	err = resourceTracking.SetAppInstance(&obj, common.LabelKeyAppInstance, "the-very-suspicious-name-with-precisely-sixty-three-characters-with-hyphen", "", TrackingMethodAnnotationAndLabel)
+	assert.Nil(t, err)
+
+	// the annotation should still work, so the name from GetAppName should not be truncated
+	app := resourceTracking.GetAppName(&obj, common.LabelKeyAppInstance, TrackingMethodAnnotationAndLabel)
+	assert.Equal(t, "the-very-suspicious-name-with-precisely-sixty-three-characters-with-hyphen", app)
+
+	// the label should be truncated to 63 characters, AND the hyphen should be removed
+	assert.Equal(t, obj.GetLabels()[common.LabelKeyAppInstance], "the-very-suspicious-name-with-precisely-sixty-three-characters")
+}
+
+func TestSetAppInstanceAnnotationAndLabelOutOfBounds(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)
+
+	resourceTracking := NewResourceTracking()
+
+	err = resourceTracking.SetAppInstance(&obj, common.LabelKeyAppInstance, "----------------------------------------------------------------", "", TrackingMethodAnnotationAndLabel)
+	// this should error because it can't truncate to a valid value
+	assert.EqualError(t, err, "failed to set app instance label: unable to truncate label to not end with a special character")
+}
+
 func TestSetAppInstanceAnnotationNotFound(t *testing.T) {
 	yamlBytes, err := os.ReadFile("testdata/svc.yaml")
 	assert.Nil(t, err)