From 126fe65c9875e122678201bfa33a80a46d1149de Mon Sep 17 00:00:00 2001 From: Tristan Smagghe Date: Tue, 2 Jun 2026 16:37:59 +0200 Subject: [PATCH] fix(apiserver): reset when extra args count stays the same (#1165) --- internal/builders/controlplane/deployment.go | 22 ++--- internal/utilities/checksum.go | 19 +++++ internal/utilities/checksum_test.go | 88 ++++++++++++++++++++ 3 files changed, 116 insertions(+), 13 deletions(-) create mode 100644 internal/utilities/checksum_test.go diff --git a/internal/builders/controlplane/deployment.go b/internal/builders/controlplane/deployment.go index 93723a7..eaa59f2 100644 --- a/internal/builders/controlplane/deployment.go +++ b/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) { diff --git a/internal/utilities/checksum.go b/internal/utilities/checksum.go index 597c54d..eb74d5f 100644 --- a/internal/utilities/checksum.go +++ b/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 { diff --git a/internal/utilities/checksum_test.go b/internal/utilities/checksum_test.go new file mode 100644 index 0000000..e14a2ff --- /dev/null +++ b/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[:]) +}