From b1bc704e3a37efd61b58a943fca5ebefecf7fa3d Mon Sep 17 00:00:00 2001 From: rcourtman Date: Sat, 29 Nov 2025 22:57:33 +0000 Subject: [PATCH] Consolidate duplicate normalizeVersion functions into shared utility - Move normalizeVersion to utils.NormalizeVersion for single source of truth - Update agentupdate and dockeragent packages to use shared function - Add 14 test cases for version normalization This prevents bugs like issue #773 where a fix applied to one copy but not the other caused an update loop. --- internal/agentupdate/update.go | 8 ++---- internal/agentupdate/update_test.go | 10 +++++--- internal/dockeragent/agent.go | 7 +----- internal/utils/helpers.go | 7 ++++++ internal/utils/utils_test.go | 38 +++++++++++++++++++++++++++++ 5 files changed, 54 insertions(+), 16 deletions(-) diff --git a/internal/agentupdate/update.go b/internal/agentupdate/update.go index 61451619a..355b5db38 100644 --- a/internal/agentupdate/update.go +++ b/internal/agentupdate/update.go @@ -21,6 +21,7 @@ import ( "syscall" "time" + "github.com/rcourtman/pulse-go-rewrite/internal/utils" "github.com/rs/zerolog" ) @@ -159,7 +160,7 @@ func (u *Updater) CheckAndUpdate(ctx context.Context) { // Normalize both versions by stripping "v" prefix for comparison. // Server returns version without prefix (e.g., "4.33.1"), but agent's // CurrentVersion may include it (e.g., "v4.33.1") depending on build. - if normalizeVersion(serverVersion) == normalizeVersion(u.cfg.CurrentVersion) { + if utils.NormalizeVersion(serverVersion) == utils.NormalizeVersion(u.cfg.CurrentVersion) { u.logger.Debug().Str("version", u.cfg.CurrentVersion).Msg("Agent is up to date") return } @@ -440,11 +441,6 @@ func (u *Updater) performUpdate(ctx context.Context) error { return nil } -// normalizeVersion strips the "v" prefix from version strings for comparison. -func normalizeVersion(version string) string { - return strings.TrimPrefix(strings.TrimSpace(version), "v") -} - // determineArch returns the architecture string for download URLs (e.g., "linux-amd64", "darwin-arm64"). func determineArch() string { os := runtime.GOOS diff --git a/internal/agentupdate/update_test.go b/internal/agentupdate/update_test.go index 840598b65..c73a31a97 100644 --- a/internal/agentupdate/update_test.go +++ b/internal/agentupdate/update_test.go @@ -5,6 +5,8 @@ import ( "path/filepath" "runtime" "testing" + + "github.com/rcourtman/pulse-go-rewrite/internal/utils" ) func TestDetermineArch(t *testing.T) { @@ -398,10 +400,10 @@ func TestSymlinkResolution(t *testing.T) { } func TestNormalizeVersion(t *testing.T) { - if got := normalizeVersion("v4.33.1"); got != "4.33.1" { - t.Errorf("normalizeVersion(%q) = %q, want %q", "v4.33.1", got, "4.33.1") + if got := utils.NormalizeVersion("v4.33.1"); got != "4.33.1" { + t.Errorf("NormalizeVersion(%q) = %q, want %q", "v4.33.1", got, "4.33.1") } - if got := normalizeVersion("4.33.1"); got != "4.33.1" { - t.Errorf("normalizeVersion(%q) = %q, want %q", "4.33.1", got, "4.33.1") + if got := utils.NormalizeVersion("4.33.1"); got != "4.33.1" { + t.Errorf("NormalizeVersion(%q) = %q, want %q", "4.33.1", got, "4.33.1") } } diff --git a/internal/dockeragent/agent.go b/internal/dockeragent/agent.go index ec0397587..8cdf22e63 100644 --- a/internal/dockeragent/agent.go +++ b/internal/dockeragent/agent.go @@ -1618,7 +1618,7 @@ func (a *Agent) checkForUpdates(ctx context.Context) { // Compare versions - normalize by stripping "v" prefix for comparison. // Server returns version without prefix (e.g., "4.33.1"), but agent's // Version may include it (e.g., "v4.33.1") depending on build. - if normalizeVersion(versionResp.Version) == normalizeVersion(Version) { + if utils.NormalizeVersion(versionResp.Version) == utils.NormalizeVersion(Version) { a.logger.Debug().Str("version", Version).Msg("Agent is up to date") return } @@ -1884,8 +1884,3 @@ func (a *Agent) selfUpdate(ctx context.Context) error { return nil } - -// normalizeVersion strips the "v" prefix from version strings for comparison. -func normalizeVersion(version string) string { - return strings.TrimPrefix(strings.TrimSpace(version), "v") -} diff --git a/internal/utils/helpers.go b/internal/utils/helpers.go index f35136fad..2e1d7c063 100644 --- a/internal/utils/helpers.go +++ b/internal/utils/helpers.go @@ -41,3 +41,10 @@ func ParseBool(value string) bool { func GetenvTrim(key string) string { return strings.TrimSpace(os.Getenv(key)) } + +// NormalizeVersion strips the "v" prefix from version strings for comparison. +// This normalizes versions like "v4.33.1" to "4.33.1" so that version strings +// from different sources (agent vs server) can be compared consistently. +func NormalizeVersion(version string) string { + return strings.TrimPrefix(strings.TrimSpace(version), "v") +} diff --git a/internal/utils/utils_test.go b/internal/utils/utils_test.go index 0f80a1281..f772db16c 100644 --- a/internal/utils/utils_test.go +++ b/internal/utils/utils_test.go @@ -280,3 +280,41 @@ func TestWriteJSONResponse_LargePayload(t *testing.T) { t.Errorf("Decoded length = %d, want 1000", len(decoded)) } } + +func TestNormalizeVersion(t *testing.T) { + tests := []struct { + input string + expected string + }{ + // With v prefix + {"v4.33.1", "4.33.1"}, + {"v1.0.0", "1.0.0"}, + {"v0.0.1-rc1", "0.0.1-rc1"}, + + // Without v prefix + {"4.33.1", "4.33.1"}, + {"1.0.0", "1.0.0"}, + {"0.0.1-rc1", "0.0.1-rc1"}, + + // With whitespace + {" v4.33.1", "4.33.1"}, + {"v4.33.1 ", "4.33.1"}, + {" v4.33.1 ", "4.33.1"}, + {"\tv4.33.1\n", "4.33.1"}, + + // Edge cases + {"", ""}, + {"v", ""}, + {" ", ""}, + {"vv4.33.1", "v4.33.1"}, // Only removes one v + } + + for _, tc := range tests { + t.Run(tc.input, func(t *testing.T) { + result := NormalizeVersion(tc.input) + if result != tc.expected { + t.Errorf("NormalizeVersion(%q) = %q, want %q", tc.input, result, tc.expected) + } + }) + } +}