Generate a unique prefix instead of initials when naming subcharts (#283)

* Generate unique prefix when naming subcharts

* Limit max chart name len to 63 chars

* Refactor code

* Update pkg/utils/chart.go

Co-authored-by: Nitish Malhotra <nitish.malhotra@gmail.com>

Co-authored-by: Nitish Malhotra <nitish.malhotra@gmail.com>

controllers/appgroup_reconciler.go

@@ -147,7 +147,7 @@ func (r *ApplicationGroupReconciler) reconcileApplications(l logr.Logger, appGro
 					}
 				}
 
-				scc.Metadata.Name = utils.ConvertToDNS1123(utils.ToInitials(appCh.Metadata.Name) + "-" + scc.Metadata.Name)
+				scc.Metadata.Name = utils.GetSubchartName(appCh.Metadata.Name, scc.Metadata.Name)
 				path, err := registry.SaveChartPackage(scc, stagingDir)
 				if err != nil {
 					ll.Error(err, "failed to save subchart package as tgz")

pkg/utils/chart.go

@@ -0,0 +1,17 @@
+package utils
+
+const (
+	DNS1123NameMaximumLength = 63
+
+	// subchartNameMaxLen is the maximum length of a subchart name.
+	//
+	// The max name length limit enforced by DNS1123 is 63 chars. We reserve 10 chars
+	// for concatenating application name hash.
+	subchartNameMaxLen = 53
+)
+
+func GetSubchartName(appName, scName string) string {
+	scName = TruncateString(scName, subchartNameMaxLen)
+	appName = TruncateString(GetHash(appName), DNS1123NameMaximumLength-len(scName)-1)
+	return ConvertToDNS1123(appName + "-" + scName)
+}

pkg/utils/chart_test.go

@@ -0,0 +1,89 @@
+package utils
+
+import (
+	"testing"
+)
+
+func TestGetSubchartName(t *testing.T) {
+	type args struct {
+		appName string
+		scName  string
+	}
+	tests := []struct {
+		name string
+		args args
+		want string
+	}{
+		{
+			name: "testing empty",
+			args: args{
+				appName: "",
+				scName:  "",
+			},
+			want: GetHash("")[0:62] + "-",
+		},
+		{
+			name: "testing empty subchart name",
+			args: args{
+				appName: "10charhash",
+				scName:  "",
+			},
+			want: GetHash("10charhash")[0:62] + "-",
+		},
+		{
+			name: "testing empty application name",
+			args: args{
+				appName: "",
+				scName:  "myapp-name",
+			},
+			want: GetHash("")[0:52] + "-myapp-name",
+		},
+		{
+			name: "testing subchart name length < 53",
+			args: args{
+				appName: "appHash",
+				scName:  "mychart",
+			},
+			want: GetHash("appHash")[0:55] + "-mychart",
+		},
+		{
+			name: "testing subchart name length == 53",
+			args: args{
+				appName: "appHash",
+				scName:  "thisismychart-withbigname-equalto53chars0000000000000",
+			},
+			want: GetHash("appHash")[0:9] + "-thisismychart-withbigname-equalto53chars0000000000000",
+		},
+		{
+			name: "testing subchart name length > 53",
+			args: args{
+				appName: "appHash",
+				scName:  "thisismychart-withbigname-greaterthan53chars0987654321abcde",
+			},
+			want: GetHash("appHash")[0:9] + "-thisismychart-withbigname-greaterthan53chars098765432",
+		},
+		{
+			name: "testing subchart name length > 63",
+			args: args{
+				appName: "appHash",
+				scName:  "thisismyappchart-withbigname-greaterthan63chars0987654321abcde123456789",
+			},
+			want: GetHash("appHash")[0:9] + "-thisismyappchart-withbigname-greaterthan63chars098765",
+		},
+		{
+			name: "testing DNS1123 incompatible subchart name",
+			args: args{
+				appName: "appHash",
+				scName:  "thisismyappchart_withbigname_greaterthan63chars0987654321abcde123456789",
+			},
+			want: GetHash("appHash")[0:9] + "-thisismyappchart-withbigname-greaterthan63chars098765",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := GetSubchartName(tt.args.appName, tt.args.scName); got != tt.want {
+				t.Errorf("TestGetSubchartReleaseName() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

pkg/utils/helpers.go

@@ -1,25 +1,18 @@
 package utils
 
 import (
+	"crypto/sha256"
+	"encoding/hex"
+	"strings"
+
 	fluxhelmv2beta1 "github.com/fluxcd/helm-controller/api/v2beta1"
 	"sigs.k8s.io/yaml"
-	"strings"
 )
 
 func ConvertToDNS1123(in string) string {
 	return strings.ReplaceAll(in, "_", "-")
 }
 
-func ToInitials(in string) (out string) {
-	in = ConvertToDNS1123(in)
-	parts := strings.Split(in, "-")
-
-	for _, part := range parts {
-		out += string(part[0])
-	}
-	return out
-}
-
 func ConvertSliceToDNS1123(in []string) []string {
 	out := []string{}
 	for _, s := range in {
@@ -28,6 +21,20 @@ func ConvertSliceToDNS1123(in []string) []string {
 	return out
 }
 
+func GetHash(in string) string {
+	h := sha256.New()
+	h.Write([]byte(in))
+	return hex.EncodeToString(h.Sum(nil))
+}
+
+func TruncateString(in string, num int) string {
+	out := in
+	if len(in) > num {
+		out = in[0:num]
+	}
+	return out
+}
+
 func ToStrPtr(in string) *string {
 	return &in
 }

pkg/utils/helpers_test.go

@@ -0,0 +1,65 @@
+package utils
+
+import (
+	"testing"
+)
+
+func TestTruncateString(t *testing.T) {
+	type args struct {
+		in  string
+		num int
+	}
+	tests := []struct {
+		name string
+		args args
+		want string
+	}{
+		{
+			name: "testing empty",
+			args: args{
+				in:  "",
+				num: 0,
+			},
+			want: "",
+		},
+		{
+			name: "testing empty with > 0 requested length",
+			args: args{
+				in:  "",
+				num: 5,
+			},
+			want: "",
+		},
+		{
+			name: "testing input string length == requested length",
+			args: args{
+				in:  "hello, don't truncate me",
+				num: 24,
+			},
+			want: "hello, don't truncate me",
+		},
+		{
+			name: "testing input string length < requested length",
+			args: args{
+				in:  "hello again, don't truncate me",
+				num: 63,
+			},
+			want: "hello again, don't truncate me",
+		},
+		{
+			name: "testing input string length > requested length",
+			args: args{
+				in:  "truncate_this_string_so_that_its_length_is_less_than_sixty_three_characters",
+				num: 63,
+			},
+			want: "truncate_this_string_so_that_its_length_is_less_than_sixty_thre",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := TruncateString(tt.args.in, tt.args.num); got != tt.want {
+				t.Errorf("TruncateString() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

pkg/workflow/argo.go

@@ -615,19 +615,20 @@ func defaultExecutor(tplName string, action ExecutorAction) v1alpha12.Template {
 }
 
 func generateSubchartHelmRelease(a v1alpha1.Application, appName, scName, version, repo, targetNS string, isStaged bool) (*fluxhelmv2beta1.HelmRelease, error) {
+	chName := utils.GetSubchartName(appName, scName)
 	hr := &fluxhelmv2beta1.HelmRelease{
 		TypeMeta: v1.TypeMeta{
 			Kind:       fluxhelmv2beta1.HelmReleaseKind,
 			APIVersion: fluxhelmv2beta1.GroupVersion.String(),
 		},
 		ObjectMeta: v1.ObjectMeta{
-			Name:      utils.ConvertToDNS1123(utils.ToInitials(appName) + "-" + scName),
+			Name:      chName,
 			Namespace: targetNS,
 		},
 		Spec: fluxhelmv2beta1.HelmReleaseSpec{
 			Chart: fluxhelmv2beta1.HelmChartTemplate{
 				Spec: fluxhelmv2beta1.HelmChartTemplateSpec{
-					Chart:   utils.ConvertToDNS1123(utils.ToInitials(appName) + "-" + scName),
+					Chart:   chName,
 					Version: version,
 					SourceRef: fluxhelmv2beta1.CrossNamespaceObjectReference{
 						Kind:      fluxsourcev1beta1.HelmRepositoryKind,