From 0f2012f89515bcc1a184a496e1d4b2b2a2e07c76 Mon Sep 17 00:00:00 2001 From: Rajwinder Mahal Date: Tue, 8 Jun 2021 21:05:56 -0700 Subject: [PATCH] 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 Co-authored-by: Nitish Malhotra --- controllers/appgroup_reconciler.go | 2 +- pkg/utils/chart.go | 17 ++++++ pkg/utils/chart_test.go | 89 ++++++++++++++++++++++++++++++ pkg/utils/helpers.go | 29 ++++++---- pkg/utils/helpers_test.go | 65 ++++++++++++++++++++++ pkg/workflow/argo.go | 5 +- 6 files changed, 193 insertions(+), 14 deletions(-) create mode 100644 pkg/utils/chart.go create mode 100644 pkg/utils/chart_test.go create mode 100644 pkg/utils/helpers_test.go diff --git a/controllers/appgroup_reconciler.go b/controllers/appgroup_reconciler.go index f5277c8..89e782f 100644 --- a/controllers/appgroup_reconciler.go +++ b/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") diff --git a/pkg/utils/chart.go b/pkg/utils/chart.go new file mode 100644 index 0000000..dbb865c --- /dev/null +++ b/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) +} diff --git a/pkg/utils/chart_test.go b/pkg/utils/chart_test.go new file mode 100644 index 0000000..6fb1919 --- /dev/null +++ b/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) + } + }) + } +} diff --git a/pkg/utils/helpers.go b/pkg/utils/helpers.go index 8b0e5cd..6df9ecf 100644 --- a/pkg/utils/helpers.go +++ b/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 } diff --git a/pkg/utils/helpers_test.go b/pkg/utils/helpers_test.go new file mode 100644 index 0000000..936c995 --- /dev/null +++ b/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) + } + }) + } +} diff --git a/pkg/workflow/argo.go b/pkg/workflow/argo.go index c9dc7c1..05fde06 100644 --- a/pkg/workflow/argo.go +++ b/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,