Use app label as app change label for long app names (#685) (#691)

If app names have more than 63 character, use the app label as app change label instead of the app name as there is a hard limit on the number of characters that can be present in a label value

Signed-off-by: Praveen Rewar <[email protected]>

pkg/kapp/app/recorded_app.go

@@ -359,7 +359,12 @@ func (a *RecordedApp) Delete() error {
 		return err
 	}
 
-	err = NewRecordedAppChanges(a.nsName, a.name, a.coreClient).DeleteAll()
+	meta, err := a.meta()
+	if err != nil {
+		return err
+	}
+
+	err = NewRecordedAppChanges(a.nsName, a.name, meta.LabelValue, a.coreClient).DeleteAll()
 	if err != nil {
 		return fmt.Errorf("Deleting app changes: %w", err)
 	}
@@ -498,7 +503,11 @@ func (a *RecordedApp) meta() (Meta, error) {
 }
 
 func (a *RecordedApp) Changes() ([]Change, error) {
-	return NewRecordedAppChanges(a.nsName, a.name, a.coreClient).List()
+	meta, err := a.meta()
+	if err != nil {
+		return nil, err
+	}
+	return NewRecordedAppChanges(a.nsName, a.name, meta.LabelValue, a.coreClient).List()
 }
 
 func (a *RecordedApp) LastChange() (Change, error) {
@@ -522,7 +531,12 @@ func (a *RecordedApp) LastChange() (Change, error) {
 }
 
 func (a *RecordedApp) BeginChange(meta ChangeMeta) (Change, error) {
-	change, err := NewRecordedAppChanges(a.nsName, a.name, a.coreClient).Begin(meta)
+	appMeta, err := a.meta()
+	if err != nil {
+		return nil, err
+	}
+
+	change, err := NewRecordedAppChanges(a.nsName, a.name, appMeta.LabelValue, a.coreClient).Begin(meta)
 	if err != nil {
 		return nil, err
 	}

pkg/kapp/app/recorded_app_changes.go

@@ -12,24 +12,27 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/apimachinery/pkg/util/validation"
 	"k8s.io/client-go/kubernetes"
 )
 
 const (
 	isChangeLabelKey   = "kapp.k14s.io/is-app-change"
 	isChangeLabelValue = ""
-	changeLabelKey     = "kapp.k14s.io/app-change-app" // holds app name
+	changeLabelKey     = "kapp.k14s.io/app-change-app" // holds app name or label
 )
 
 type RecordedAppChanges struct {
 	nsName  string
 	appName string
 
+	appLabelValue string
+
 	coreClient kubernetes.Interface
 }
 
-func NewRecordedAppChanges(nsName, appName string, coreClient kubernetes.Interface) RecordedAppChanges {
-	return RecordedAppChanges{nsName, appName, coreClient}
+func NewRecordedAppChanges(nsName, appName, appLabelValue string, coreClient kubernetes.Interface) RecordedAppChanges {
+	return RecordedAppChanges{nsName, appName, appLabelValue, coreClient}
 }
 
 func (a RecordedAppChanges) List() ([]Change, error) {
@@ -38,7 +41,7 @@ func (a RecordedAppChanges) List() ([]Change, error) {
 	listOpts := metav1.ListOptions{
 		LabelSelector: labels.Set(map[string]string{
 			isChangeLabelKey: isChangeLabelValue,
-			changeLabelKey:   a.appName,
+			changeLabelKey:   a.changeLabelValue(),
 		}).String(),
 	}
 
@@ -70,7 +73,7 @@ func (a RecordedAppChanges) DeleteAll() error {
 	listOpts := metav1.ListOptions{
 		LabelSelector: labels.Set(map[string]string{
 			isChangeLabelKey: isChangeLabelValue,
-			changeLabelKey:   a.appName,
+			changeLabelKey:   a.changeLabelValue(),
 		}).String(),
 	}
 
@@ -102,7 +105,7 @@ func (a RecordedAppChanges) Begin(meta ChangeMeta) (*ChangeImpl, error) {
 			Namespace:    a.nsName,
 			Labels: map[string]string{
 				isChangeLabelKey: isChangeLabelValue,
-				changeLabelKey:   a.appName,
+				changeLabelKey:   a.changeLabelValue(),
 			},
 		},
 		Data: newMeta.AsData(),
@@ -122,3 +125,10 @@ func (a RecordedAppChanges) Begin(meta ChangeMeta) (*ChangeImpl, error) {
 
 	return change, nil
 }
+
+func (a RecordedAppChanges) changeLabelValue() string {
+	if len(a.appName) > validation.LabelValueMaxLength {
+		return a.appLabelValue
+	}
+	return a.appName
+}

test/e2e/app_change_test.go

@@ -93,6 +93,53 @@ spec:
 	})
 }
 
+func TestAppChangeWithLongAppName(t *testing.T) {
+	env := BuildEnv(t)
+	logger := Logger{}
+	kapp := Kapp{t, env.Namespace, env.KappBinaryPath, logger}
+
+	yaml := `
+---
+apiVersion: v1
+kind: Service
+metadata:
+  name: redis-primary
+spec:
+  ports:
+  - port: 6380
+    targetPort: 6380
+  selector:
+    app: redis
+    tier: %s
+`
+
+	name := "test-app-change-with-a-very-very-very-very-very-very-very-long-app-name"
+	cleanUp := func() {
+		kapp.Run([]string{"delete", "-a", name})
+	}
+
+	cleanUp()
+	defer cleanUp()
+
+	logger.Section("deploy app", func() {
+		kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name}, RunOpts{IntoNs: true, StdinReader: strings.NewReader(fmt.Sprintf(yaml, "backend"))})
+	})
+
+	logger.Section("deploy with changes", func() {
+		kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name}, RunOpts{IntoNs: true, StdinReader: strings.NewReader(fmt.Sprintf(yaml, "frontend"))})
+	})
+
+	logger.Section("app change list", func() {
+		out, _ := kapp.RunWithOpts([]string{"app-change", "ls", "-a", name, "--json"}, RunOpts{})
+
+		resp := uitest.JSONUIFromBytes(t, []byte(out))
+
+		require.Equal(t, 2, len(resp.Tables[0].Rows), "Expected to have 2 app-changes")
+		require.Equal(t, "update: Op: 0 create, 0 delete, 1 update, 0 noop, 0 exists / Wait to: 1 reconcile, 0 delete, 0 noop", resp.Tables[0].Rows[0]["description"], "Expected description to match")
+		require.Equal(t, "update: Op: 1 create, 0 delete, 0 update, 0 noop, 0 exists / Wait to: 1 reconcile, 0 delete, 0 noop", resp.Tables[0].Rows[1]["description"], "Expected description to match")
+	})
+}
+
 func TestAppKindChangeWithMetadataOutput(t *testing.T) {
 	env := BuildEnv(t)
 	logger := Logger{}