From b4d63cfd84f4af44c36843540bbe26c32fbd04b7 Mon Sep 17 00:00:00 2001
From: Dmitriy Kalinin <dkalinin@vmware.com>
Date: Tue, 5 Oct 2021 20:51:22 -0400
Subject: [PATCH] allow long change group names

---
 pkg/kapp/diffgraph/change_group.go      | 14 +++++++-
 pkg/kapp/diffgraph/change_group_test.go | 45 +++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 1 deletion(-)
 create mode 100644 pkg/kapp/diffgraph/change_group_test.go

diff --git a/pkg/kapp/diffgraph/change_group.go b/pkg/kapp/diffgraph/change_group.go
index b93734023..dcff6197a 100644
--- a/pkg/kapp/diffgraph/change_group.go
+++ b/pkg/kapp/diffgraph/change_group.go
@@ -41,9 +41,21 @@ func (r ChangeGroup) Validate() error {
 	if len(r.Name) == 0 {
 		return fmt.Errorf("Expected non-empty group name")
 	}
-	errStrs := k8sval.IsQualifiedName(r.Name)
+	errStrs := r.isQualifiedNameWithoutLen(r.Name)
 	if len(errStrs) > 0 {
 		return fmt.Errorf("Expected change group name %q to be a qualified name: %s", r.Name, strings.Join(errStrs, "; "))
 	}
 	return nil
 }
+
+func (r ChangeGroup) isQualifiedNameWithoutLen(name string) []string {
+	errStrs := k8sval.IsQualifiedName(name)
+	var updatedErrStrs []string
+	for _, err := range errStrs {
+		// Allow change group names to have more characters than the default maxLength
+		if !strings.Contains(err, k8sval.MaxLenError(k8sval.DNS1035LabelMaxLength)) {
+			updatedErrStrs = append(updatedErrStrs, err)
+		}
+	}
+	return updatedErrStrs
+}
diff --git a/pkg/kapp/diffgraph/change_group_test.go b/pkg/kapp/diffgraph/change_group_test.go
new file mode 100644
index 000000000..462b8d59b
--- /dev/null
+++ b/pkg/kapp/diffgraph/change_group_test.go
@@ -0,0 +1,45 @@
+// Copyright 2021 VMware, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+package diffgraph_test
+
+import (
+	"testing"
+	"strings"
+
+	ctldgraph "github.com/k14s/kapp/pkg/kapp/diffgraph"
+	"github.com/stretchr/testify/require"
+)
+
+func TestNewChangeGroupFromAnnString(t *testing.T) {
+	names := []string{
+		"valid",
+		"valid-name",
+		"valid/valid",
+		"valid-name/valid",
+		"valid-name.com",
+		"valid-name.com/valid",
+		"valid-name.com/valid-name_Another_Name--valid",
+		"valid-name.com/valid-name_CustomResourceDefinition--valid",
+		// Allow arbitrary long names since it might be populated with data via placeholders
+		"valid-name.com/valid-name_CustomResourceDefinition--valid"+strings.Repeat("a", 1000),
+		// Example from pinniped of a long name
+		"change-groups.kapp.k14s.io/crds-authentication.concierge.pinniped.dev-WebhookAuthenticator",
+	}
+	for _, name := range names {
+		cg, err := ctldgraph.NewChangeGroupFromAnnString(name)
+		require.NoError(t, err)
+		require.Equal(t, cg.Name, name)
+	}
+
+	names = []string{
+		"_",
+		"invalid/",
+		"invalid/_",
+		"/_",
+	}
+	for _, name := range names {
+		_, err := ctldgraph.NewChangeGroupFromAnnString(name)
+		require.Error(t, err)
+	}
+}