feat: improve helm and kubeconfig packages

pkg/addon-operator/operator_test.go

@@ -242,7 +242,7 @@
 
 	res.helmClient.ReleaseNames = []string{moduleToPurge, moduleToDelete}
 
 	type taskInfo struct {
 		taskType         sh_task.TaskType
 		bindingType      BindingType
 		moduleName       string
@@ -250,7 +250,11 @@
 		spawnerTaskPhase string
 	}
 
-	taskHandleHistory := make([]taskInfo, 0)
+	// Use thread-safe TaskHandleHistory
+	taskHandleHistory := TaskHandleHistory{
+		taskHandleHistory: make([]TaskInfo, 0),
+		historyMu:         sync.Mutex{},
+	}
 	op.engine.TaskQueues.GetMain().WithHandler(func(ctx context.Context, tsk sh_task.Task) queue.TaskResult {
 		// Put task info to history.
 		hm := task.HookMetadataAccessor(tsk)
@@ -261,7 +265,7 @@
 		case task.ModuleRun:
 			phase = string(op.ModuleManager.GetModule(hm.ModuleName).GetPhase())
 		}
-		taskHandleHistory = append(taskHandleHistory, taskInfo{
+		taskHandleHistory.Add(&TaskInfo{
 			taskType:         tsk.GetType(),
 			bindingType:      hm.BindingType,
 			moduleName:       hm.ModuleName,
@@ -338,7 +342,8 @@
 		{task.ConvergeModules, "", "", string(converge.WaitAfterAll)},
 	}
 
-	for i, historyInfo := range taskHandleHistory {
+	historyItems := taskHandleHistory.Get()
+	for i, historyInfo := range historyItems {
 		if i >= len(historyExpects) {
 			break
 		}
@@ -380,8 +385,10 @@
 	canHandleTasks := make(chan struct{})
 	triggerPause := true
 
+	// Use thread-safe TaskHandleHistory
 	taskHandleHistory := TaskHandleHistory{
 		taskHandleHistory: make([]TaskInfo, 0),
+		historyMu:         sync.Mutex{},
 	}
 
 	op.engine.TaskQueues.GetMain().WithHandler(func(ctx context.Context, tsk sh_task.Task) queue.TaskResult {
@@ -395,22 +402,37 @@
 			convergeEvent = tsk.GetProp(converge.ConvergeEventProp).(converge.ConvergeEvent)
 		case task.ModuleRun:
 			if triggerPause {
+				// First add information about the current task to history
+				taskHandleHistory.Add(&TaskInfo{
+					id:               tsk.GetId(),
+					taskType:         tsk.GetType(),
+					bindingType:      hm.BindingType,
+					moduleName:       hm.ModuleName,
+					hookName:         hm.HookName,
+					spawnerTaskPhase: string(op.ModuleManager.GetModule(hm.ModuleName).GetPhase()),
+					convergeEvent:    convergeEvent,
+				})
+
+				// Then start the asynchronous operation
 				close(canChangeConfigMap)
 				<-canHandleTasks
 				triggerPause = false
 			}
 			phase = string(op.ModuleManager.GetModule(hm.ModuleName).GetPhase())
 		}
 
-		taskHandleHistory.Add(&TaskInfo{
-			id:               tsk.GetId(),
-			taskType:         tsk.GetType(),
-			bindingType:      hm.BindingType,
-			moduleName:       hm.ModuleName,
-			hookName:         hm.HookName,
-			spawnerTaskPhase: phase,
-			convergeEvent:    convergeEvent,
-		})
+		// Add information to history only if we haven't added it yet
+		if !triggerPause || tsk.GetType() != task.ModuleRun {
+			taskHandleHistory.Add(&TaskInfo{
+				id:               tsk.GetId(),
+				taskType:         tsk.GetType(),
+				bindingType:      hm.BindingType,
+				moduleName:       hm.ModuleName,
+				hookName:         hm.HookName,
+				spawnerTaskPhase: phase,
+				convergeEvent:    convergeEvent,
+			})
+		}
 
 		// Handle it.
 		return op.TaskService.Handle(ctx, tsk)
@@ -443,16 +465,17 @@
 	g.Eventually(convergeDone(op), "30s", "200ms").Should(BeTrue())
 
 	hasReloadAllInStandby := false
-	for i, tsk := range taskHandleHistory.Get() {
+	historyItems := taskHandleHistory.Get()
+	for i, tsk := range historyItems {
 		// if i < ignoreTasksCount {
 		//	continue
 		//}
 		if tsk.taskType != task.ApplyKubeConfigValues {
 			continue
 		}
 
-		g.Expect(taskHandleHistory.Len() > i+1).Should(BeTrue(), "history should not end on ApplyKubeConfigValues")
-		next := taskHandleHistory.Get()[i+1]
+		g.Expect(i+1 < len(historyItems)).Should(BeTrue(), "history should not end on ApplyKubeConfigValues")
+		next := historyItems[i+1]
 		g.Expect(next.convergeEvent).Should(Equal(converge.ReloadAllModules))
 		g.Expect(next.spawnerTaskPhase).Should(Equal(string(converge.StandBy)))
 		hasReloadAllInStandby = true
@@ -547,6 +570,7 @@
 
 	taskHandleHistory := TaskHandleHistory{
 		taskHandleHistory: make([]TaskInfo, 0),
+		historyMu:         sync.Mutex{},
 	}
 
 	op.engine.TaskQueues.GetMain().WithHandler(func(ctx context.Context, tsk sh_task.Task) queue.TaskResult {

pkg/helm/helm3lib/helm3lib.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"log/slog"
+	"maps"
 	"os"
 	"sort"
 	"strconv"
@@ -81,8 +82,11 @@ func (h *LibClient) WithLogLabels(logLabels map[string]string) {
 }
 
 func (h *LibClient) WithExtraLabels(labels map[string]string) {
-	for k, v := range labels {
-		h.labels[k] = v
+	if h.labels == nil {
+		h.labels = make(map[string]string)
+	}
+	if labels != nil {
+		maps.Copy(h.labels, labels)
 	}
 }
 
@@ -210,13 +214,7 @@ func (h *LibClient) upgradeRelease(releaseName string, chartName string, valuesP
 	}
 
 	if len(setValues) > 0 {
-		m := make(map[string]interface{})
-		for _, sv := range setValues {
-			arr := strings.Split(sv, "=")
-			if len(arr) == 2 {
-				m[arr[0]] = arr[1]
-			}
-		}
+		m := parseSetValues(setValues)
 		resultValues = chartutil.CoalesceTables(resultValues, m)
 	}
 
@@ -447,6 +445,40 @@ func (h *LibClient) ListReleasesNames() ([]string, error) {
 	return releases, nil
 }
 
+// parseSetValues parses setValues slice into a map, supporting '=' in values.
+// Each entry should be in the format "key=value".
+//
+// Behavior:
+//   - Supports '=' characters within values (only the first '=' is used as delimiter)
+//   - Trims whitespace from both keys and values
+//   - Skips entries that don't contain '=' (malformed entries)
+//   - Skips entries with empty keys (after trimming whitespace)
+//   - For duplicate keys, the last occurrence in the slice takes precedence
+//   - Returns an empty map if setValues is nil or empty
+//
+// Examples:
+//   - "key=value" -> {"key": "value"}
+//   - "key=value=with=equals" -> {"key": "value=with=equals"}
+//   - " key = value " -> {"key": "value"}
+//   - "=value" -> skipped (empty key)
+//   - "key" -> skipped (no '=' delimiter)
+//   - ["key=value1", "key=value2"] -> {"key": "value2"} (last wins)
+func parseSetValues(setValues []string) map[string]any {
+	m := make(map[string]any)
+	for _, sv := range setValues {
+		arr := strings.SplitN(sv, "=", 2)
+		if len(arr) == 2 {
+			key := strings.TrimSpace(arr[0])
+			if key == "" {
+				// skip entries with empty key
+				continue
+			}
+			m[key] = strings.TrimSpace(arr[1])
+		}
+	}
+	return m
+}
+
 func (h *LibClient) Render(releaseName, chartName string, valuesPaths, setValues []string, namespace string, debug bool) (string, error) {
 	chart, err := loader.Load(chartName)
 	if err != nil {
@@ -465,13 +497,7 @@ func (h *LibClient) Render(releaseName, chartName string, valuesPaths, setValues
 	}
 
 	if len(setValues) > 0 {
-		m := make(map[string]interface{})
-		for _, sv := range setValues {
-			arr := strings.Split(sv, "=")
-			if len(arr) == 2 {
-				m[arr[0]] = arr[1]
-			}
-		}
+		m := parseSetValues(setValues)
 		resultValues = chartutil.CoalesceTables(resultValues, m)
 	}
 

pkg/helm/helm3lib/helm3lib_test.go

@@ -174,6 +174,79 @@ func TestLastReleaseStatusReturnsErrorForNonExistingRelease(t *testing.T) {
 	g.Expect(err).Should(HaveOccurred(), "should return an error for a non-existing release")
 }
 
+func TestParseSetValues(t *testing.T) {
+	g := NewWithT(t)
+
+	tests := []struct {
+		name     string
+		input    []string
+		expected map[string]any
+	}{
+		{
+			name:     "empty input returns empty map",
+			input:    nil,
+			expected: map[string]any{},
+		},
+		{
+			name:     "empty slice returns empty map",
+			input:    []string{},
+			expected: map[string]any{},
+		},
+		{
+			name:     "single key-value",
+			input:    []string{"foo=bar"},
+			expected: map[string]any{"foo": "bar"},
+		},
+		{
+			name:     "multiple key-values",
+			input:    []string{"foo=bar", "baz=qux"},
+			expected: map[string]any{"foo": "bar", "baz": "qux"},
+		},
+		{
+			name:     "value with equals sign",
+			input:    []string{"foo=bar=baz"},
+			expected: map[string]any{"foo": "bar=baz"},
+		},
+		{
+			name:     "key and value with whitespace",
+			input:    []string{" foo = bar "},
+			expected: map[string]any{"foo": "bar"},
+		},
+		{
+			name:     "entry with no equals is skipped",
+			input:    []string{"foo"},
+			expected: map[string]any{},
+		},
+		{
+			name:     "entry with empty key is skipped",
+			input:    []string{"=bar"},
+			expected: map[string]any{},
+		},
+		{
+			name:     "entry with empty value",
+			input:    []string{"foo="},
+			expected: map[string]any{"foo": ""},
+		},
+		{
+			name:     "duplicate keys, last wins",
+			input:    []string{"foo=bar", "foo=baz"},
+			expected: map[string]any{"foo": "baz"},
+		},
+		{
+			name:     "mixed valid and invalid entries",
+			input:    []string{"foo=bar", "baz", "=qux", "key=value=with=equals", "  spaced = spaced value  "},
+			expected: map[string]any{"foo": "bar", "key": "value=with=equals", "spaced": "spaced value"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(_ *testing.T) {
+			result := parseSetValues(tt.input)
+			g.Expect(result).To(Equal(tt.expected))
+		})
+	}
+}
+
 func initHelmClient(t *testing.T) *LibClient {
 	g := NewWithT(t)
 	err := Init(&Options{

pkg/helm/post_renderer/post_renderer.go

@@ -3,6 +3,7 @@ package post_renderer
 import (
 	"bytes"
 	"fmt"
+	"maps"
 
 	"sigs.k8s.io/kustomize/kyaml/kio"
 )
@@ -29,8 +30,11 @@ func (p *PostRenderer) Run(renderedManifests *bytes.Buffer) (*bytes.Buffer, erro
 
 	for _, node := range nodes {
 		labels := node.GetLabels()
-		for k, v := range p.extraLabels {
-			labels[k] = v
+		if labels == nil {
+			labels = make(map[string]string)
+		}
+		if p.extraLabels != nil {
+			maps.Copy(labels, p.extraLabels)
 		}
 
 		_ = node.SetLabels(labels)