diff --git a/linux/svtminion.sh b/linux/svtminion.sh index d6ef2cf..85950a5 100755 --- a/linux/svtminion.sh +++ b/linux/svtminion.sh @@ -408,14 +408,17 @@ _set_log_level() { # # _salt_onedir_dir_is_ga # -# True (status 0) if the onedir directory name is GA numeric CalVer only -# (digits and dots). Prerelease names (e.g. 3008.0rc1) are not GA. +# True (status 0) if the onedir directory name is GA CalVer, optionally +# with a -N package-release suffix (e.g. 3008.1 or 3008.1-1). +# Prerelease names (e.g. 3008.0rc1) are not GA. A -N suffix is a +# repackage of the same version, not a prerelease, so it counts as GA +# and can win latest/major-series selection via sort -V. # # Results: # 0 if GA, 1 otherwise # _salt_onedir_dir_is_ga() { - local _ga_re='^[0-9]+\.[0-9]+(\.[0-9]+)*$' + local _ga_re='^[0-9]+\.[0-9]+(\.[0-9]+)*(-[0-9]+)?$' [[ -n "$1" && "$1" =~ ${_ga_re} ]] } diff --git a/tests/linux/test_version_resolution.sh b/tests/linux/test_version_resolution.sh new file mode 100644 index 0000000..b0947e1 --- /dev/null +++ b/tests/linux/test_version_resolution.sh @@ -0,0 +1,96 @@ +#!/usr/bin/env bash +# +# Lightweight, fixture-free regression test for linux/svtminion.sh's Salt +# version resolution logic (_salt_onedir_dir_is_ga / _get_desired_salt_version_fn). +# +# Unlike test-linux.sh, this does not perform a real install and needs no +# onedir tarball fixtures -- it extracts the two functions under test +# directly from the real script and exercises them against a directory of +# zero-byte stub files, so it only tests directory-name resolution logic +# (GA classification, latest/major-series selection, exact-name lookup), +# not extraction or installation. +# +# Run directly: bash tests/linux/test_version_resolution.sh + +set -o nounset +set -o errexit +set -o pipefail + +_test_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +_repo_root="$(cd "${_test_dir}/../.." && pwd)" +_script="${_repo_root}/linux/svtminion.sh" + +# Stub the logging functions _get_desired_salt_version_fn depends on so it +# can run standalone without the rest of svtminion.sh's global state. +_error_log() { echo "ERROR: $*" 1>&2; } +_info_log() { :; } +_debug_log() { :; } + +# Extract the two functions under test verbatim from the real script, so +# this test always exercises the current implementation rather than a +# hand-copied duplicate that could drift out of sync. +_extracted="$(mktemp)" +trap 'rm -f "${_extracted}"' EXIT + +sed -n '/^_salt_onedir_dir_is_ga() {/,/^}/p' "${_script}" > "${_extracted}" +sed -n '/^_get_desired_salt_version_fn() {/,/^}/p' "${_script}" >> "${_extracted}" + +# shellcheck disable=SC1090 +source "${_extracted}" + +_fixture_dir="$(mktemp -d)" +trap 'rm -rf "${_fixture_dir}"; rm -f "${_extracted}"' EXIT + +for v in "3008.0rc1" "3008.1" "3008.1-1" "3007.9"; do + touch "${_fixture_dir}/${v}" +done + +_failed=0 + +_assert_resolves_to() { + local requested="$1" + local expected="$2" + local label="$3" + + salt_url_version="${requested}" + _GENERIC_PKG_VERSION="" + salt_specific_version="" + + if ! _get_desired_salt_version_fn "${_fixture_dir}"; then + echo "FAILED: ${label} -- _get_desired_salt_version_fn returned non-zero for '${requested}'" + _failed=1 + return + fi + + if [ "${salt_specific_version}" != "${expected}" ]; then + echo "FAILED: ${label} -- requested '${requested}', expected '${expected}', got '${salt_specific_version}'" + _failed=1 + else + echo "OK: ${label} -- '${requested}' resolved to '${expected}'" + fi +} + +# latest / major-series resolution must prefer the newest GA build, +# including a -N repackage of a version that also exists without the suffix. +_assert_resolves_to "latest" "3008.1-1" "latest picks -N repackage over bare version" +_assert_resolves_to "3008" "3008.1-1" "major-series picks -N repackage over bare version" + +# Exact-name request for a prerelease still works (unchanged RC behavior). +_assert_resolves_to "3008.0rc1" "3008.0rc1" "exact RC request" + +# Exact-name request for a -N build works. +_assert_resolves_to "3008.1-1" "3008.1-1" "exact -N request" + +# Mere-presence check: requesting an unrelated version must not fail/crash +# just because -N and rc directories exist alongside it in the listing. +# This is the same failure mode that previously broke RC handling before +# GA classification distinguished "present" from "requested". +_assert_resolves_to "3007.9" "3007.9" "mere presence of -N/rc does not break unrelated exact request" + +if [ "${_failed}" -ne 0 ]; then + echo "test_version_resolution.sh: FAILED" + exit 1 +fi + +echo "test_version_resolution.sh: All tests passed" +exit 0 diff --git a/tests/windows/functional/test_input_validation.ps1 b/tests/windows/functional/test_input_validation.ps1 index 5425339..597b28c 100644 --- a/tests/windows/functional/test_input_validation.ps1 +++ b/tests/windows/functional/test_input_validation.ps1 @@ -42,12 +42,15 @@ function test_Test-MinionVersionParameter { @{ MinionVersion = "3008.0rc1"; Expected = $true } @{ MinionVersion = "3009.0rc2"; Expected = $true } @{ MinionVersion = "3009.1.2"; Expected = $true } + @{ MinionVersion = "3008.1-1"; Expected = $true } + @{ MinionVersion = "3004.2-1"; Expected = $true } @{ MinionVersion = "bad version"; Expected = $false } @{ MinionVersion = "3006/evil"; Expected = $false } @{ MinionVersion = "abc"; Expected = $false } @{ MinionVersion = '3006;rm -rf /'; Expected = $false } @{ MinionVersion = ""; Expected = $false } @{ MinionVersion = "3006 .2"; Expected = $false } + @{ MinionVersion = "3008.1-1-2"; Expected = $false } ) $failed = 0 foreach ($c in $cases) { diff --git a/tests/windows/functional/test_version_helpers.ps1 b/tests/windows/functional/test_version_helpers.ps1 index f4f93b9..b0f4942 100644 --- a/tests/windows/functional/test_version_helpers.ps1 +++ b/tests/windows/functional/test_version_helpers.ps1 @@ -16,6 +16,7 @@ function test_Test-SaltOnedirVersionIsGA { @{ Version = "3006.24"; Expected = $true } @{ Version = "3008.0"; Expected = $true } @{ Version = "3006.1"; Expected = $true } + @{ Version = "3008.1-1"; Expected = $true } @{ Version = "3008.0rc1"; Expected = $false } @{ Version = "3007.1dev1"; Expected = $false } @{ Version = "notaversion"; Expected = $false } @@ -42,6 +43,11 @@ function test_Compare-SaltCalVer { @{ Left = "3006.24"; Right = "3006.8"; Expected = 1 } @{ Left = "3006.8"; Right = "3006.24"; Expected = -1 } @{ Left = "3008.0"; Right = "3007.99"; Expected = 1 } + @{ Left = "3008.1-1"; Right = "3008.1"; Expected = 1 } + @{ Left = "3008.1"; Right = "3008.1-1"; Expected = -1 } + @{ Left = "3008.1-2"; Right = "3008.1-1"; Expected = 1 } + @{ Left = "3008.2"; Right = "3008.1-1"; Expected = 1 } + @{ Left = "3008.1-1"; Right = "3008.1-1"; Expected = 0 } ) $failed = 0 foreach ( $c in $cases ) { diff --git a/windows/svtminion.ps1 b/windows/svtminion.ps1 index 87593f8..75fb525 100644 --- a/windows/svtminion.ps1 +++ b/windows/svtminion.ps1 @@ -1753,13 +1753,14 @@ function Test-MinionVersionParameter { ) # Valid: "latest", 4-digit major (3006), major.minor (3006.2), - # multi-part (3006.24), rc suffix (3008.0rc1) - if ($MinionVersion -notmatch '^(latest|\d{4}(\.\d+(\.\d+)*(rc\d+)?)?)$') { + # multi-part (3006.24), rc suffix (3008.0rc1), package-release suffix + # (3008.1-1) + if ($MinionVersion -notmatch '^(latest|\d{4}(\.\d+(\.\d+)*(rc\d+|-\d+)?)?)$') { $msg = "Invalid MinionVersion: $MinionVersion" Write-Log $msg -Level error Write-Host $msg -ForegroundColor Red $msg = "Must be 'latest', a major version (e.g. 3006), " + - "or a full version (e.g. 3006.2, 3008.0rc1)" + "or a full version (e.g. 3006.2, 3008.0rc1, 3008.1-1)" Write-Host $msg -ForegroundColor Yellow return $false } @@ -1875,21 +1876,27 @@ function Get-MajorVersion { function Test-SaltOnedirVersionIsGA { - # True if the onedir directory name is a GA CalVer (digits and dots only). - # Prerelease dirs (e.g. 3008.0rc1) are not GA; install those only via exact - # -MinionVersion matching the directory name. + # True if the onedir directory name is a GA CalVer, optionally with a -N + # package-release suffix (e.g. 3008.1 or 3008.1-1). Prerelease dirs (e.g. + # 3008.0rc1) are not GA; install those only via exact -MinionVersion + # matching the directory name. A -N suffix is a repackage of the same + # version, not a prerelease, so it counts as GA and participates in + # latest/major-series selection via Compare-SaltCalVer. [CmdletBinding()] param( [Parameter(Mandatory=$true)] [String] $Version ) - return [bool]( $Version -match '^\d+\.\d+(\.\d+)*$' ) + return [bool]( $Version -match '^\d+\.\d+(\.\d+)*(-\d+)?$' ) } function Compare-SaltCalVer { - # Compare two GA numeric CalVer strings (e.g. 3006.24 vs 3007.0). Returns 1 - # if Left is greater than Right, -1 if less, 0 if equal. + # Compare two GA CalVer strings, each optionally carrying a -N + # package-release suffix (e.g. 3006.24, 3008.1-1). Returns 1 if Left is + # greater than Right, -1 if less, 0 if equal. The dotted version is + # compared first; a missing -N suffix is treated as release 0, so + # 3008.1-1 compares greater than 3008.1. [CmdletBinding()] param( [Parameter(Mandatory=$true)] @@ -1897,15 +1904,30 @@ function Compare-SaltCalVer { [Parameter(Mandatory=$true)] [String] $Right ) - $left_parts = @( ($Left -split '\.') | ForEach-Object { [int]$_ } ) - $right_parts = @( ($Right -split '\.') | ForEach-Object { [int]$_ } ) - $max_len = [Math]::Max($left_parts.Count, $right_parts.Count) + function Get-CalVerParts { + param([String] $Version) + $release = 0 + $dotted = $Version + if ( $Version -match '^(.+)-(\d+)$' ) { + $dotted = $Matches[1] + $release = [int]$Matches[2] + } + return @{ + Dotted = @( ($dotted -split '\.') | ForEach-Object { [int]$_ } ) + Release = $release + } + } + $left_parts = Get-CalVerParts $Left + $right_parts = Get-CalVerParts $Right + $max_len = [Math]::Max($left_parts.Dotted.Count, $right_parts.Dotted.Count) for ( $i = 0; $i -lt $max_len; $i++ ) { - $a = if ( $i -lt $left_parts.Count ) { $left_parts[$i] } else { 0 } - $b = if ( $i -lt $right_parts.Count ) { $right_parts[$i] } else { 0 } + $a = if ( $i -lt $left_parts.Dotted.Count ) { $left_parts.Dotted[$i] } else { 0 } + $b = if ( $i -lt $right_parts.Dotted.Count ) { $right_parts.Dotted[$i] } else { 0 } if ( $a -gt $b ) { return 1 } if ( $a -lt $b ) { return -1 } } + if ( $left_parts.Release -gt $right_parts.Release ) { return 1 } + if ( $left_parts.Release -lt $right_parts.Release ) { return -1 } return 0 }