From f2f248ec474ebc3fbea4f35f7aa0c7cae8376355 Mon Sep 17 00:00:00 2001 From: KangManJoo Date: Sat, 21 Sep 2024 14:09:57 +0900 Subject: [PATCH 01/15] fix: Fix issue with escaping consecutive commas Signed-off-by: KangManJoo --- util/helm/cmd.go | 1 + 1 file changed, 1 insertion(+) diff --git a/util/helm/cmd.go b/util/helm/cmd.go index 6b0e30ed2fe75..0e4ea11a581bd 100644 --- a/util/helm/cmd.go +++ b/util/helm/cmd.go @@ -352,6 +352,7 @@ func cleanSetParameters(val string) string { if strings.HasPrefix(val, `{`) && strings.HasSuffix(val, `}`) { return val } + re := regexp.MustCompile(`,`) return re.ReplaceAllString(val, `$1\,`) } From 1fce27a7b4586b843963bc822953d29d9ba150b6 Mon Sep 17 00:00:00 2001 From: KangManJoo Date: Sat, 21 Sep 2024 14:11:54 +0900 Subject: [PATCH 02/15] test: add unit test for CleanSetParameters Signed-off-by: KangManJoo --- util/helm/cmd_test.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/util/helm/cmd_test.go b/util/helm/cmd_test.go index 8d472c75e25c9..6374ae31bb65c 100644 --- a/util/helm/cmd_test.go +++ b/util/helm/cmd_test.go @@ -48,3 +48,22 @@ func TestNewCmd_withProxy(t *testing.T) { assert.Equal(t, "https://proxy:8888", cmd.proxy) assert.Equal(t, ".argoproj.io", cmd.noProxy) } + +func TestCleanSetParameters(t *testing.T) { + val := "{key=value, key2=value2}" + result := cleanSetParameters(val) + require.NotEmpty(t, result) + assert.Equal(t, "{key=value, key2=value2}", result) + + val = "key=value,key2=value2" + result = cleanSetParameters(val) + assert.Equal(t, "key=value\\,key2=value2", result) + + val = "" + result = cleanSetParameters(val) + assert.Equal(t, "", result) + + val = ",,,,,,,,value" + result = cleanSetParameters(val) + assert.Equal(t, "\\,\\,\\,\\,\\,\\,\\,\\,value", result) +} From 9669dcec472593b6007d1dd2011f79620998ea31 Mon Sep 17 00:00:00 2001 From: daengdaengLee Date: Sun, 22 Sep 2024 17:27:57 +0900 Subject: [PATCH 03/15] test: add test case for helm arg cleaner Signed-off-by: daengdaengLee Signed-off-by: KangManJoo --- util/helm/helm_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/util/helm/helm_test.go b/util/helm/helm_test.go index 8468b9f36624b..e242d13bdb3b0 100644 --- a/util/helm/helm_test.go +++ b/util/helm/helm_test.go @@ -166,6 +166,7 @@ func TestHelmArgCleaner(t *testing.T) { `not, clean`: `not\, clean`, `a\,b,c`: `a\,b\,c`, `{a,b,c}`: `{a,b,c}`, + `,,,,,\,`: `\,\,\,\,\,\,`, } { cleaned := cleanSetParameters(input) assert.Equal(t, expected, cleaned) From 6827eac2fb3578c9e7de3e6738769fceecc8aa31 Mon Sep 17 00:00:00 2001 From: KangManJoo Date: Sun, 22 Sep 2024 18:30:36 +0900 Subject: [PATCH 04/15] test: delete duplicated test Signed-off-by: KangManJoo --- util/helm/cmd_test.go | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/util/helm/cmd_test.go b/util/helm/cmd_test.go index 6374ae31bb65c..8d472c75e25c9 100644 --- a/util/helm/cmd_test.go +++ b/util/helm/cmd_test.go @@ -48,22 +48,3 @@ func TestNewCmd_withProxy(t *testing.T) { assert.Equal(t, "https://proxy:8888", cmd.proxy) assert.Equal(t, ".argoproj.io", cmd.noProxy) } - -func TestCleanSetParameters(t *testing.T) { - val := "{key=value, key2=value2}" - result := cleanSetParameters(val) - require.NotEmpty(t, result) - assert.Equal(t, "{key=value, key2=value2}", result) - - val = "key=value,key2=value2" - result = cleanSetParameters(val) - assert.Equal(t, "key=value\\,key2=value2", result) - - val = "" - result = cleanSetParameters(val) - assert.Equal(t, "", result) - - val = ",,,,,,,,value" - result = cleanSetParameters(val) - assert.Equal(t, "\\,\\,\\,\\,\\,\\,\\,\\,value", result) -} From b5bdbf5cc36eb98dd0142c569a9bafc0fa41e380 Mon Sep 17 00:00:00 2001 From: KangManJoo Date: Sun, 22 Sep 2024 18:31:11 +0900 Subject: [PATCH 05/15] feat: Changing from using regular expressions to direct navigation Signed-off-by: KangManJoo --- util/helm/cmd.go | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/util/helm/cmd.go b/util/helm/cmd.go index 0e4ea11a581bd..daa06554976a6 100644 --- a/util/helm/cmd.go +++ b/util/helm/cmd.go @@ -343,7 +343,6 @@ type TemplateOpts struct { } var ( - re = regexp.MustCompile(`([^\\]),`) apiVersionsRemover = regexp.MustCompile(`(--api-versions [^ ]+ )+`) ) @@ -352,8 +351,21 @@ func cleanSetParameters(val string) string { if strings.HasPrefix(val, `{`) && strings.HasSuffix(val, `}`) { return val } - re := regexp.MustCompile(`,`) - return re.ReplaceAllString(val, `$1\,`) + + var result strings.Builder + for i := 0; i < len(val); i++ { + if val[i] == ',' { + if i == 0 || val[i-1] != '\\' { + result.WriteString(`\,`) + } else { + result.WriteByte(',') + } + } else { + result.WriteByte(val[i]) + } + } + + return result.String() } func (c *Cmd) template(chartPath string, opts *TemplateOpts) (string, string, error) { From 59594065b6511c0c69997a16200eafb9212db679 Mon Sep 17 00:00:00 2001 From: KangManJoo Date: Mon, 23 Sep 2024 23:34:26 +0900 Subject: [PATCH 06/15] fix: Changing a loop from index to range Signed-off-by: KangManJoo --- util/helm/cmd.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/util/helm/cmd.go b/util/helm/cmd.go index daa06554976a6..c84e2e376017d 100644 --- a/util/helm/cmd.go +++ b/util/helm/cmd.go @@ -353,15 +353,15 @@ func cleanSetParameters(val string) string { } var result strings.Builder - for i := 0; i < len(val); i++ { - if val[i] == ',' { - if i == 0 || val[i-1] != '\\' { + for _, r := range val { + if r == ',' { + if result.Len() == 0 || result.String()[result.Len()-1] != '\\' { result.WriteString(`\,`) } else { - result.WriteByte(',') + result.WriteRune(',') } } else { - result.WriteByte(val[i]) + result.WriteRune(r) } } From 7a464717299937121cc0f4ad98405f4302c497c4 Mon Sep 17 00:00:00 2001 From: KangManJoo Date: Wed, 25 Sep 2024 00:20:27 +0900 Subject: [PATCH 07/15] fix: Solve problem that building strings per loop Signed-off-by: KangManJoo --- util/helm/cmd.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/util/helm/cmd.go b/util/helm/cmd.go index c84e2e376017d..1516a58ccb4c7 100644 --- a/util/helm/cmd.go +++ b/util/helm/cmd.go @@ -353,9 +353,10 @@ func cleanSetParameters(val string) string { } var result strings.Builder - for _, r := range val { + var prevR rune + for i, r := range val { if r == ',' { - if result.Len() == 0 || result.String()[result.Len()-1] != '\\' { + if i == 0 || prevR != '\\' { result.WriteString(`\,`) } else { result.WriteRune(',') @@ -363,6 +364,7 @@ func cleanSetParameters(val string) string { } else { result.WriteRune(r) } + prevR = r } return result.String() From 75425af3ec6bd51e4b22cb66c4f2f8dad74eaf44 Mon Sep 17 00:00:00 2001 From: KangManJoo Date: Thu, 26 Sep 2024 22:56:44 +0900 Subject: [PATCH 08/15] fix: solve lint error Signed-off-by: KangManJoo --- util/helm/cmd.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/util/helm/cmd.go b/util/helm/cmd.go index 1516a58ccb4c7..215836e2d40b6 100644 --- a/util/helm/cmd.go +++ b/util/helm/cmd.go @@ -342,10 +342,6 @@ type TemplateOpts struct { SkipCrds bool } -var ( - apiVersionsRemover = regexp.MustCompile(`(--api-versions [^ ]+ )+`) -) - func cleanSetParameters(val string) string { // `{}` equal helm list parameters format, so don't escape `,`. if strings.HasPrefix(val, `{`) && strings.HasSuffix(val, `}`) { @@ -407,6 +403,8 @@ func (c *Cmd) template(chartPath string, opts *TemplateOpts) (string, string, er args = append(args, "--include-crds") } + var apiVersionsRemover = regexp.MustCompile(`(--api-versions [^ ]+ )+`) + out, command, err := c.run(args...) if err != nil { msg := err.Error() From ac09a06065d231a0c992f55ac5506449280d037d Mon Sep 17 00:00:00 2001 From: KangManJoo Date: Thu, 26 Sep 2024 23:15:39 +0900 Subject: [PATCH 09/15] fix: solve lint error Signed-off-by: KangManJoo --- util/helm/cmd.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/util/helm/cmd.go b/util/helm/cmd.go index 215836e2d40b6..07d94c7be5ccf 100644 --- a/util/helm/cmd.go +++ b/util/helm/cmd.go @@ -403,8 +403,8 @@ func (c *Cmd) template(chartPath string, opts *TemplateOpts) (string, string, er args = append(args, "--include-crds") } - var apiVersionsRemover = regexp.MustCompile(`(--api-versions [^ ]+ )+`) - + apiVersionsRemover := regexp.MustCompile(`(--api-versions [^ ]+ )+`) + out, command, err := c.run(args...) if err != nil { msg := err.Error() From 491eddae931d227fa6a4b446b07cca13c0535265 Mon Sep 17 00:00:00 2001 From: KangManJoo Date: Tue, 1 Oct 2024 00:24:53 +0900 Subject: [PATCH 10/15] refactor: Improve readability Signed-off-by: KangManJoo --- util/helm/cmd.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/util/helm/cmd.go b/util/helm/cmd.go index 07d94c7be5ccf..a39b96c6025d9 100644 --- a/util/helm/cmd.go +++ b/util/helm/cmd.go @@ -350,9 +350,9 @@ func cleanSetParameters(val string) string { var result strings.Builder var prevR rune - for i, r := range val { + for _, r := range val { if r == ',' { - if i == 0 || prevR != '\\' { + if prevR != '\\' { result.WriteString(`\,`) } else { result.WriteRune(',') From a5d460b879d826b9a84644d9b01180589ed595ef Mon Sep 17 00:00:00 2001 From: KangManJoo Date: Tue, 1 Oct 2024 00:39:43 +0900 Subject: [PATCH 11/15] refactor: Change regular expressions back to global variables to compile only once Signed-off-by: KangManJoo --- util/helm/cmd.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/util/helm/cmd.go b/util/helm/cmd.go index a39b96c6025d9..e7f3f6303033a 100644 --- a/util/helm/cmd.go +++ b/util/helm/cmd.go @@ -366,6 +366,8 @@ func cleanSetParameters(val string) string { return result.String() } +var apiVersionsRemover = regexp.MustCompile(`(--api-versions [^ ]+ )+`) + func (c *Cmd) template(chartPath string, opts *TemplateOpts) (string, string, error) { if callback, err := cleanupChartLockFile(filepath.Clean(path.Join(c.WorkDir, chartPath))); err == nil { defer callback() @@ -403,8 +405,6 @@ func (c *Cmd) template(chartPath string, opts *TemplateOpts) (string, string, er args = append(args, "--include-crds") } - apiVersionsRemover := regexp.MustCompile(`(--api-versions [^ ]+ )+`) - out, command, err := c.run(args...) if err != nil { msg := err.Error() From 9cadedc3f9cedfe565131686f33447f632e7cc40 Mon Sep 17 00:00:00 2001 From: KangManJoo Date: Wed, 2 Oct 2024 02:32:08 +0900 Subject: [PATCH 12/15] refactor: Separate functions that escape consecutive commas Signed-off-by: KangManJoo --- util/helm/cmd.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/util/helm/cmd.go b/util/helm/cmd.go index e7f3f6303033a..81e7dd0c6dcc4 100644 --- a/util/helm/cmd.go +++ b/util/helm/cmd.go @@ -342,27 +342,33 @@ type TemplateOpts struct { SkipCrds bool } +var re = regexp.MustCompile(`([^\\]),`) + func cleanSetParameters(val string) string { // `{}` equal helm list parameters format, so don't escape `,`. if strings.HasPrefix(val, `{`) && strings.HasSuffix(val, `}`) { return val } + val = replaceAllWithLookbehind(val, ',', `\,`, '\\') + return val +} + +func replaceAllWithLookbehind(val string, old rune, new string, lookbehind rune) string { var result strings.Builder var prevR rune for _, r := range val { - if r == ',' { - if prevR != '\\' { - result.WriteString(`\,`) + if r == old { + if prevR != lookbehind { + result.WriteString(new) } else { - result.WriteRune(',') + result.WriteRune(old) // 원래 문자를 그대로 추가 } } else { result.WriteRune(r) } prevR = r } - return result.String() } From 90bc2e351c898c5e5686b93c2b2a8ad56b3da1ff Mon Sep 17 00:00:00 2001 From: KangManJoo Date: Wed, 2 Oct 2024 02:34:33 +0900 Subject: [PATCH 13/15] test: Add tests for different consecutive commas Signed-off-by: KangManJoo --- util/helm/helm_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/util/helm/helm_test.go b/util/helm/helm_test.go index e242d13bdb3b0..58dd273481f27 100644 --- a/util/helm/helm_test.go +++ b/util/helm/helm_test.go @@ -167,6 +167,7 @@ func TestHelmArgCleaner(t *testing.T) { `a\,b,c`: `a\,b\,c`, `{a,b,c}`: `{a,b,c}`, `,,,,,\,`: `\,\,\,\,\,\,`, + `\,,\\,,`: `\,\,\\,\,`, } { cleaned := cleanSetParameters(input) assert.Equal(t, expected, cleaned) From e48f4cb28844a8764205f275c369456400a96dee Mon Sep 17 00:00:00 2001 From: KangManJoo Date: Wed, 2 Oct 2024 03:32:07 +0900 Subject: [PATCH 14/15] chore: delete unused variable Signed-off-by: KangManJoo --- util/helm/cmd.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/util/helm/cmd.go b/util/helm/cmd.go index 81e7dd0c6dcc4..ea1eb3f3fe5b8 100644 --- a/util/helm/cmd.go +++ b/util/helm/cmd.go @@ -342,8 +342,6 @@ type TemplateOpts struct { SkipCrds bool } -var re = regexp.MustCompile(`([^\\]),`) - func cleanSetParameters(val string) string { // `{}` equal helm list parameters format, so don't escape `,`. if strings.HasPrefix(val, `{`) && strings.HasSuffix(val, `}`) { From 3b6341651b1ef8753c92c971d74c4aca19729863 Mon Sep 17 00:00:00 2001 From: KangManJoo Date: Wed, 2 Oct 2024 23:13:31 +0900 Subject: [PATCH 15/15] chore: Delete non-English comments Signed-off-by: KangManJoo --- util/helm/cmd.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/helm/cmd.go b/util/helm/cmd.go index ea1eb3f3fe5b8..55d9e8670b461 100644 --- a/util/helm/cmd.go +++ b/util/helm/cmd.go @@ -360,7 +360,7 @@ func replaceAllWithLookbehind(val string, old rune, new string, lookbehind rune) if prevR != lookbehind { result.WriteString(new) } else { - result.WriteRune(old) // 원래 문자를 그대로 추가 + result.WriteRune(old) } } else { result.WriteRune(r)