fix(apiserver): reset when extra args count stays the same (#1165)

internal/builders/controlplane/deployment.go

@@ -9,7 +9,6 @@ import (
 	"fmt"
 	"path"
 	"sort"
-	"strconv"
 	"strings"
 
 	appsv1 "k8s.io/api/apps/v1"
@@ -1217,24 +1216,21 @@ func (d Deployment) resetKubeAPIServerFlags(resource *appsv1.Deployment, tcp kam
 	if resource.GetAnnotations() == nil {
 		resource.SetAnnotations(map[string]string{})
 	}
-	// retrieving the current amount of extra flags, used as a sort of hash:
+	// retrieving the previous hash for the apiserver args:
 	// in case of non-matching values, removing all the args in order to perform a full reconciliation from a clean start.
-	var count int
-
-	if v, ok := resource.GetAnnotations()[apiServerFlagsAnnotation]; ok {
-		var err error
-
-		if count, err = strconv.Atoi(v); err != nil {
-			return
-		}
+	previousHash := resource.GetAnnotations()[apiServerFlagsAnnotation]
+	currentHash, err := utilities.CalculateStringSliceChecksum(tcp.Spec.ControlPlane.Deployment.ExtraArgs.APIServer)
+	if err != nil {
+		return
 	}
-	// there's a mismatch in the count from the previous hash: let's reset and store the desired extra args count.
-	if count != len(tcp.Spec.ControlPlane.Deployment.ExtraArgs.APIServer) {
+
+	// there's a mismatch between current and previous hash: let's reset and store the new hash.
+	if previousHash != currentHash {
 		_, index := utilities.HasNamedContainer(resource.Spec.Template.Spec.Containers, apiServerContainerName)
 		resource.Spec.Template.Spec.Containers[index].Args = []string{}
 	}
 
-	resource.GetAnnotations()[apiServerFlagsAnnotation] = fmt.Sprintf("%d", len(tcp.Spec.ControlPlane.Deployment.ExtraArgs.APIServer))
+	resource.GetAnnotations()[apiServerFlagsAnnotation] = currentHash
 }
 
 func (d Deployment) setNodeSelector(spec *corev1.PodSpec, tcp kamajiv1alpha1.TenantControlPlane) {

internal/utilities/checksum.go

@@ -36,6 +36,25 @@ func SetObjectChecksum(obj client.Object, data any) {
 	obj.SetAnnotations(annotations)
 }
 
+// CalculateStringSliceChecksum calculates a checksum of slice, calculating the overall md5 of each values.
+// It takes order into account, as in :
+//
+//	CalculateStringSliceChecksum([a, b]) != CalculateStringSliceChecksum([b, a]) // (if a != b)
+func CalculateStringSliceChecksum(data []string) (string, error) {
+	h := md5.New()
+	for _, s := range data {
+		if _, err := h.Write([]byte(s)); err != nil {
+			return "", err
+		}
+		// Add separator to avoid collisions (e.g., ["ab", "c"] vs ["a", "bc"])
+		if _, err := h.Write([]byte{0}); err != nil {
+			return "", err
+		}
+	}
+
+	return hex.EncodeToString(h.Sum([]byte{})), nil
+}
+
 // CalculateMapChecksum orders the map according to its key, and calculating the overall md5 of the values.
 // It's expected to work with ConfigMap (map[string]string) and Secrets (map[string][]byte).
 func CalculateMapChecksum(data any) string {

internal/utilities/checksum_test.go

@@ -0,0 +1,88 @@
+// Copyright 2022 Clastix Labs
+// SPDX-License-Identifier: Apache-2.0
+
+package utilities_test
+
+import (
+	"crypto/md5"
+	"encoding/hex"
+	"testing"
+
+	"github.com/clastix/kamaji/internal/utilities"
+)
+
+func TestCalculateStringSliceChecksum(t *testing.T) {
+	tests := []struct {
+		name     string
+		data     []string
+		expected string
+	}{
+		{
+			name:     "empty slice",
+			data:     []string{},
+			expected: md5Hex([]byte{}),
+		},
+		{
+			name:     "single element",
+			data:     []string{"hello"},
+			expected: md5Hex([]byte("hello\x00")),
+		},
+		{
+			name:     "multiple elements",
+			data:     []string{"a", "b", "c"},
+			expected: md5Hex([]byte("a\x00b\x00c\x00")),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := utilities.CalculateStringSliceChecksum(tt.data)
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+			if got != tt.expected {
+				t.Errorf("CalculateStringSliceChecksum(%v) = %q, want %q", tt.data, got, tt.expected)
+			}
+		})
+	}
+}
+
+func TestCalculateStringSliceChecksumOrder(t *testing.T) {
+	// Test that different orders produce different checksums
+	a, err := utilities.CalculateStringSliceChecksum([]string{"a", "b"})
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	b, err := utilities.CalculateStringSliceChecksum([]string{"b", "a"})
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	if a == b {
+		t.Errorf("expected different checksums for [a,b] and [b,a], but got %q and %q", a, b)
+	}
+}
+
+func TestCalculateStringSliceChecksumCollision(t *testing.T) {
+	// Test that ["ab", "c"] and ["a", "bc"] produce different checksums
+	a, err := utilities.CalculateStringSliceChecksum([]string{"ab", "c"})
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	b, err := utilities.CalculateStringSliceChecksum([]string{"a", "bc"})
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	if a == b {
+		t.Errorf("expected different checksums for [ab,c] and [a,bc], but got %q and %q", a, b)
+	}
+}
+
+func md5Hex(data []byte) string {
+	hash := md5.Sum(data)
+
+	return hex.EncodeToString(hash[:])
+}