From 7997df66d4e3e5a4a5ad36d94a134991c5044537 Mon Sep 17 00:00:00 2001 From: Tianon Gravi Date: Tue, 25 Aug 2026 11:56:09 -0700 Subject: [PATCH] Implement generic variant handling My old variant handling was creating explicit vector arrays, which doesn't scale well (algorithmically or maintainably). This adjusts the logic to do more direct comparisons and handle "satisfiability" separate from "preferred order" which cleans up a lot of the comparisons. This also makes it trivial to write a generic means of comparing variants that works for `v4` vs `v3` the same as `power10` vs `power8` and `rva23u64` vs `rva20u64`. As part of this work, it also folds in all the Windows-specific logic into a single matcher (because that's just standard platform matching logic that's naturally keyed off `os`, not a completely separate matcher). It also genericizes the arm64 variant matching to the same vX.Y -> vX-1.Y+5 logic that Go uses. Assisted-By: "claude my eyes right out" Signed-off-by: Tianon Gravi --- compare.go | 326 +++++++++++++++++++++---------------- compare_test.go | 90 +++++++++- defaults_test.go | 11 +- platform_windows_compat.go | 61 +++---- platforms.go | 99 +++++------ variant.go | 253 ++++++++++++++++++++++++++++ variant_test.go | 181 ++++++++++++++++++++ 7 files changed, 771 insertions(+), 250 deletions(-) create mode 100644 variant.go create mode 100644 variant_test.go diff --git a/compare.go b/compare.go index ea5e7aa..74a09f0 100644 --- a/compare.go +++ b/compare.go @@ -17,8 +17,7 @@ package platforms import ( - "strconv" - "strings" + "slices" specs "github.com/opencontainers/image-spec/specs-go/v1" ) @@ -31,125 +30,199 @@ type MatchComparer interface { Less(specs.Platform, specs.Platform) bool } -type platformVersions struct { - major []int - minor []int +// Only returns a match comparer for a single platform using default +// resolution logic for the platform. +// +// Match and Less answer two different questions and are implemented +// independently: Match asks "can this run here at all", and holds all the +// compatibility logic — feature requirements, the arm64 cross-generation +// offset, architecture fallbacks, and (on Windows) the stable-ABI version +// window between host and container OS versions (see +// checkWindowsHostAndContainerCompat) — computed directly (see variant.go) +// rather than by generating and searching the set of every platform that +// could be compatible, which would be unbounded for some architectures' +// variant schemes. Less asks "how would these sort, biased towards this +// host's own OS and architecture" and is a plain, match-independent sort +// key. +// +// Callers that want "the best runnable match" should filter with Match +// before ranking with Less — the same way images.Manifests filters an +// index's manifests before sorting the survivors — rather than sorting +// first and scanning for the first match: these lists are small enough +// that filtering first is never more expensive, and it avoids sorting +// entries that are just going to be discarded. +// +// For arm64/v9.x, will also match arm64/v9.{0..x-1} and arm64/v8.{0..x+5} +// For arm64/v8.x, will also match arm64/v8.{0..x-1} +// For arm/v8, will also match arm/v7, arm/v6 and arm/v5 +// For arm/v7, will also match arm/v6 and arm/v5 +// For arm/v6, will also match arm/v5 +// For amd64, will also match 386 +// For any other architecture with a ""-shaped +// variant (e.g. ppc64le's "powerN", riscv64's "rvaNNu64"), will also match +// lower numbered variants sharing the same prefix/suffix, as well as +// images that declare no variant at all. +// For windows, will also match any container OS version within the current +// Windows Server stable-ABI compatibility window, not just an exact +// OS version match. +func Only(platform specs.Platform) MatchComparer { + return &onlyComparer{ + platform: Normalize(platform), + } } -var arm64variantToVersion = map[string]platformVersions{ - "v8": {[]int{8}, []int{0}}, - "v8.0": {[]int{8}, []int{0}}, - "v8.1": {[]int{8}, []int{1}}, - "v8.2": {[]int{8}, []int{2}}, - "v8.3": {[]int{8}, []int{3}}, - "v8.4": {[]int{8}, []int{4}}, - "v8.5": {[]int{8}, []int{5}}, - "v8.6": {[]int{8}, []int{6}}, - "v8.7": {[]int{8}, []int{7}}, - "v8.8": {[]int{8}, []int{8}}, - "v8.9": {[]int{8}, []int{9}}, - "v9": {[]int{9, 8}, []int{0, 5}}, - "v9.0": {[]int{9, 8}, []int{0, 5}}, - "v9.1": {[]int{9, 8}, []int{1, 6}}, - "v9.2": {[]int{9, 8}, []int{2, 7}}, - "v9.3": {[]int{9, 8}, []int{3, 8}}, - "v9.4": {[]int{9, 8}, []int{4, 9}}, - "v9.5": {[]int{9, 8}, []int{5, 9}}, - "v9.6": {[]int{9, 8}, []int{6, 9}}, - "v9.7": {[]int{9, 8}, []int{7, 9}}, +// onlyComparer implements the matching and ranking behavior of Only. +type onlyComparer struct { + platform specs.Platform // normalized reference platform } -// platformVector returns an (ordered) vector of appropriate specs.Platform -// objects to try matching for the given platform object (see platforms.Only). -func platformVector(platform specs.Platform) []specs.Platform { - vector := []specs.Platform{platform} +// osIdentity reports whether p's OS name and OS version are ones +// c.platform could run. There's no "native vs. fallback OS" concept in Only +// (that would be for something like running Linux containers on Windows or +// FreeBSD), so an OS identity mismatch is an absolute disqualifier. +func (c *onlyComparer) osIdentity(p specs.Platform) bool { + normalized := Normalize(p) + if c.platform.OS != normalized.OS { + return false + } + return osVersionMatch(c.platform.OS, c.platform.OSVersion, p.OSVersion) +} - switch platform.Architecture { - case "amd64": - if amd64Version, err := strconv.Atoi(strings.TrimPrefix(platform.Variant, "v")); err == nil && amd64Version > 1 { - for amd64Version--; amd64Version >= 1; amd64Version-- { - vector = append(vector, specs.Platform{ - Architecture: platform.Architecture, - OS: platform.OS, - OSVersion: platform.OSVersion, - OSFeatures: platform.OSFeatures, - Variant: "v" + strconv.Itoa(amd64Version), - }) - } - } - vector = append(vector, specs.Platform{ - Architecture: "386", - OS: platform.OS, - OSVersion: platform.OSVersion, - OSFeatures: platform.OSFeatures, - }) - case "arm": - if armVersion, err := strconv.Atoi(strings.TrimPrefix(platform.Variant, "v")); err == nil && armVersion > 5 { - for armVersion--; armVersion >= 5; armVersion-- { - vector = append(vector, specs.Platform{ - Architecture: platform.Architecture, - OS: platform.OS, - OSVersion: platform.OSVersion, - OSFeatures: platform.OSFeatures, - Variant: "v" + strconv.Itoa(armVersion), - }) - } +// featuresOK reports whether p's OS features (ignoring win32k on Windows, +// which is missing on Nano Server) are a subset of c.platform's. +func (c *onlyComparer) featuresOK(p specs.Platform) bool { + features := c.stripIgnoredFeatures(Normalize(p).OSFeatures) + return osFeaturesSubset(features, c.platform.OSFeatures) +} + +// featureOverlap returns how many of p's OS features are also declared by +// c.platform. Used only by Less, to rank two candidates that are otherwise +// tied: a candidate's extra features should only count in its favor to the +// extent the host actually declares them. +func (c *onlyComparer) featureOverlap(p specs.Platform) int { + features := c.stripIgnoredFeatures(Normalize(p).OSFeatures) + have := c.platform.OSFeatures + n, j := 0, 0 + for _, f := range features { + for j < len(have) && have[j] < f { + j++ } - case "arm64": - variant := platform.Variant - if variant == "" { - variant = "v8" + if j < len(have) && have[j] == f { + n++ + j++ } + } + return n +} - vector = []specs.Platform{} // Reset vector, the first variant will be added in loop. - arm64Versions, ok := arm64variantToVersion[variant] - if !ok { - break - } - for i, major := range arm64Versions.major { - for minor := arm64Versions.minor[i]; minor >= 0; minor-- { - arm64Variant := "v" + strconv.Itoa(major) + "." + strconv.Itoa(minor) - if minor == 0 { - arm64Variant = "v" + strconv.Itoa(major) - } - vector = append(vector, specs.Platform{ - Architecture: "arm64", - OS: platform.OS, - OSVersion: platform.OSVersion, - OSFeatures: platform.OSFeatures, - Variant: arm64Variant, - }) - } +func (c *onlyComparer) stripIgnoredFeatures(features []string) []string { + if c.platform.OS == "windows" { + // win32k is missing on Nano Server; ignore it for matching purposes. + if i := slices.Index(features, "win32k"); i >= 0 { + return slices.Delete(slices.Clone(features), i, i+1) } + } + return features +} - // All arm64/v8.x and arm64/v9.x are compatible with arm/v8 (32-bits) and below. - // There's no arm64 v9 variant, so it's normalized to v8. - if strings.HasPrefix(variant, "v8") || strings.HasPrefix(variant, "v9") { - variant = "v8" +// fallbackArch returns the recognized cross-architecture fallback for +// hostArch (386 for amd64, arm for arm64), or "" if hostArch has none. Both +// archMatch and archRank need this same pairing, one for compatibility and +// one for ranking, so it's kept in one place. +func fallbackArch(hostArch string) string { + switch hostArch { + case "amd64": + return "386" + case "arm64": + return "arm" + } + return "" +} + +// archMatch reports whether p's architecture and variant are compatible +// with c.platform's: either the same architecture with a compatible +// variant, or a recognized cross-architecture fallback (see fallbackArch) +// with a compatible variant. +func (c *onlyComparer) archMatch(p specs.Platform) bool { + normalized := Normalize(p) + if normalized.Architecture == c.platform.Architecture { + switch c.platform.Architecture { + case "amd64": + return numberedVariantMatch(c.platform.Variant, normalized.Variant, "v1") + case "arm": + return numberedVariantMatch(c.platform.Variant, normalized.Variant, "v5") + case "arm64": + return arm64VariantMatch(c.platform.Variant, normalized.Variant) + default: + return genericVariantMatch(c.platform.Variant, normalized.Variant) } - vector = append(vector, platformVector(specs.Platform{ - Architecture: "arm", - OS: platform.OS, - OSVersion: platform.OSVersion, - OSFeatures: platform.OSFeatures, - Variant: variant, - })...) } + if normalized.Architecture != fallbackArch(c.platform.Architecture) { + return false + } + + switch c.platform.Architecture { + case "amd64": + return true // 386 has no variant to check; it's a match-or-nothing fallback. + default: // arm64 + return numberedVariantMatch("v8", normalized.Variant, "v5") + } +} - return vector +func (c *onlyComparer) Match(p specs.Platform) bool { + return c.osIdentity(p) && c.featuresOK(p) && c.archMatch(p) } -// Only returns a match comparer for a single platform -// using default resolution logic for the platform. -// -// For arm64/v9.x, will also match arm64/v9.{0..x-1} and arm64/v8.{0..x+5} -// For arm64/v8.x, will also match arm64/v8.{0..x-1} -// For arm/v8, will also match arm/v7, arm/v6 and arm/v5 -// For arm/v7, will also match arm/v6 and arm/v5 -// For arm/v6, will also match arm/v5 -// For amd64, will also match 386 -func Only(platform specs.Platform) MatchComparer { - return Ordered(platformVector(Normalize(platform))...) +// archRank returns how preferable arch is, for a host declaring +// hostArch: 2 for the host's own architecture, 1 for its recognized +// cross-architecture fallback (see fallbackArch), or 0 otherwise (ties are +// broken alphabetically by Less). Unlike archMatch, this doesn't check +// variant compatibility at all — it's a plain preference between +// architecture names, used only for ranking. +func archRank(hostArch, arch string) int { + switch { + case arch == hostArch: + return 2 + case arch != "" && arch == fallbackArch(hostArch): + return 1 + default: + return 0 + } +} + +func (c *onlyComparer) Less(p1, p2 specs.Platform) bool { + n1, n2 := Normalize(p1), Normalize(p2) + + // Prefer the host's own OS over any other; otherwise alphabetically. + native1, native2 := n1.OS == c.platform.OS, n2.OS == c.platform.OS + if native1 != native2 { + return native1 + } + if n1.OS != n2.OS { + return naturalLess(n1.OS, n2.OS) + } + + // Prefer the host's own architecture, then its recognized fallback + // architecture, then alphabetically. + a1, a2 := archRank(c.platform.Architecture, n1.Architecture), archRank(c.platform.Architecture, n2.Architecture) + if a1 != a2 { + return a1 > a2 + } + if n1.Architecture != n2.Architecture { + return naturalLess(n1.Architecture, n2.Architecture) + } + + // Then variant and OS version, newest/highest first. + if n1.Variant != n2.Variant { + return naturalLess(n2.Variant, n1.Variant) + } + if p1.OSVersion != p2.OSVersion { + return naturalLess(p2.OSVersion, p1.OSVersion) + } + + // Tied on everything above: prefer the one whose OS features overlap + // more with what the host actually declares. + return c.featureOverlap(p1) > c.featureOverlap(p2) } // OnlyOS returns a match comparer that matches only platforms with the same @@ -160,25 +233,14 @@ func OnlyOS(platform specs.Platform) MatchComparer { normalized := Normalize(platform) return onlyOSComparer{ platform: normalized, - osvM: newOSVersionMatcher(normalized), archOrder: orderedPlatformComparer{ matchers: []Matcher{NewMatcher(normalized)}, }, } } -func newOSVersionMatcher(platform specs.Platform) osVerMatcher { - if platform.OS == "windows" { - return &windowsVersionMatcher{ - windowsOSVersion: getWindowsOSVersion(platform.OSVersion), - } - } - return nil -} - type onlyOSComparer struct { platform specs.Platform - osvM osVerMatcher archOrder orderedPlatformComparer } @@ -187,34 +249,10 @@ func (c onlyOSComparer) matchOS(platform specs.Platform) bool { if c.platform.OS != normalized.OS { return false } - if c.osvM != nil { - if !c.osvM.Match(platform.OSVersion) { - return false - } - } - if len(normalized.OSFeatures) > 0 { - if len(c.platform.OSFeatures) < len(normalized.OSFeatures) { - return false - } - j := 0 - for _, feature := range normalized.OSFeatures { - found := false - for ; j < len(c.platform.OSFeatures); j++ { - if feature == c.platform.OSFeatures[j] { - found = true - j++ - break - } - if feature < c.platform.OSFeatures[j] { - return false - } - } - if !found { - return false - } - } + if !osVersionMatch(c.platform.OS, c.platform.OSVersion, platform.OSVersion) { + return false } - return true + return osFeaturesSubset(normalized.OSFeatures, c.platform.OSFeatures) } func (c onlyOSComparer) Match(platform specs.Platform) bool { diff --git a/compare_test.go b/compare_test.go index 1968e51..bdcd8c9 100644 --- a/compare_test.go +++ b/compare_test.go @@ -298,6 +298,56 @@ func TestOnly(t *testing.T) { }, }, }, + { + // v9.8 has no entry in any hardcoded table: it must be handled + // arithmetically (Armv9.N always carries the Armv8.(N+5) + // baseline, capped at Armv8.9) for it to match at all. + platform: "linux/arm64/v9.8", + matches: map[bool][]string{ + true: { + "linux/arm64/v9", + "linux/arm64/v9.8", + "linux/arm64/v8.9", // offset would be v8.13, capped at v8.9 + "linux/arm", + }, + false: { + "linux/arm64/v9.9", + "linux/amd64", + }, + }, + }, + { + platform: "linux/ppc64le/power10", + matches: map[bool][]string{ + true: { + "linux/ppc64le", // bare (no variant): least-preferred fallback, still a match + "linux/ppc64le/power8", + "linux/ppc64le/power9", + "linux/ppc64le/power10", + }, + false: { + "linux/ppc64le/power11", + "linux/amd64", + "linux/arm64", + }, + }, + }, + { + platform: "linux/riscv64/rva23u64", + matches: map[bool][]string{ + true: { + "linux/riscv64/rva23u64", + "linux/riscv64/rva22u64", + "linux/riscv64/rva20u64", + "linux/riscv64", // bare fallback + }, + false: { + "linux/riscv64/rva24u64", + "linux/riscv64/rva20s64", // different profile class (suffix) + "linux/amd64", + }, + }, + }, } { testcase := tc t.Run(testcase.platform, func(t *testing.T) { @@ -720,37 +770,57 @@ func TestCompareOSFeatures(t *testing.T) { platform string platforms []string expected []string + // expectedOnly overrides expected for the Only comparer specifically, + // when it legitimately differs from Ordered/Any/OnlyStrict: those are + // built from a caller-supplied platform list and have no notion of + // "OS family" at all, so a pair that matches neither has no defined + // order between them (an artifact of sort, not a rule). Only, by + // contrast, always treats OS compatibility as the dominant axis (see + // onlyComparer.Less in compare.go), so between two platforms that + // match neither the OS nor the architecture, one with the reference + // OS (even with the "wrong" architecture) outranks one with neither. + expectedOnly []string }{ { "linux/amd64", []string{"windows/amd64", "linux/amd64", "linux(+other)/amd64", "linux/arm64"}, []string{"linux/amd64", "linux(+other)/amd64", "windows/amd64", "linux/arm64"}, + []string{"linux/amd64", "linux(+other)/amd64", "linux/arm64", "windows/amd64"}, }, { "linux(+none)/amd64", []string{"windows/amd64", "linux/amd64", "linux/arm64", "linux(+other)/amd64"}, []string{"linux/amd64", "linux(+other)/amd64", "windows/amd64", "linux/arm64"}, + []string{"linux/amd64", "linux(+other)/amd64", "linux/arm64", "windows/amd64"}, }, { "linux(+other)/amd64", []string{"windows/amd64", "linux/amd64", "linux/arm64", "linux(+other)/amd64"}, []string{"linux(+other)/amd64", "linux/amd64", "windows/amd64", "linux/arm64"}, + []string{"linux(+other)/amd64", "linux/amd64", "linux/arm64", "windows/amd64"}, }, { "linux(+af+other+zf)/amd64", []string{"windows/amd64", "linux/amd64", "linux/arm64", "linux(+other)/amd64"}, []string{"linux(+other)/amd64", "linux/amd64", "windows/amd64", "linux/arm64"}, + []string{"linux(+other)/amd64", "linux/amd64", "linux/arm64", "windows/amd64"}, }, { "linux(+f1+f2)/amd64", []string{"linux/amd64", "linux(+f2)/amd64", "linux(+f1)/amd64", "linux(+f1+f2)/amd64"}, []string{"linux(+f1+f2)/amd64", "linux(+f2)/amd64", "linux(+f1)/amd64", "linux/amd64"}, + nil, }, { - // This test should likely fail and be updated when os version is considered for linux "linux(7.2+other)/amd64", []string{"linux/amd64", "linux(+other)/amd64", "linux(7.1)/amd64", "linux(7.2+other)/amd64"}, []string{"linux(+other)/amd64", "linux(7.2+other)/amd64", "linux/amd64", "linux(7.1)/amd64"}, + // Only's Less considers OS version (via natural sort, for + // ranking purposes only — Match still doesn't compare Linux + // OS versions at all), so the exact match sorts first, + // then the next-highest version, then the two unversioned + // candidates tied on version and broken by feature overlap. + []string{"linux(7.2+other)/amd64", "linux(7.1)/amd64", "linux(+other)/amd64", "linux/amd64"}, }, } { testcase := tc @@ -762,12 +832,14 @@ func TestCompareOSFeatures(t *testing.T) { } for _, stc := range []struct { - name string - mc MatchComparer + name string + mc MatchComparer + expected []string }{ { - name: "only", - mc: Only(p), + name: "only", + mc: Only(p), + expected: testcase.expectedOnly, }, { name: "only strict", @@ -783,6 +855,10 @@ func TestCompareOSFeatures(t *testing.T) { }, } { mc := stc.mc + expected := stc.expected + if expected == nil { + expected = testcase.expected + } testcase := testcase t.Run(stc.name, func(t *testing.T) { p, err := ParseAll(testcase.platforms) @@ -797,8 +873,8 @@ func TestCompareOSFeatures(t *testing.T) { actual[i] = FormatAll(ps) } - if !reflect.DeepEqual(testcase.expected, actual) { - t.Errorf("Wrong platform order:\nExpected: %#v\nActual: %#v", testcase.expected, actual) + if !reflect.DeepEqual(expected, actual) { + t.Errorf("Wrong platform order:\nExpected: %#v\nActual: %#v", expected, actual) } }) } diff --git a/defaults_test.go b/defaults_test.go index d0e1163..5760ac0 100644 --- a/defaults_test.go +++ b/defaults_test.go @@ -18,7 +18,6 @@ package platforms import ( "reflect" - "runtime" "sort" "testing" @@ -137,12 +136,8 @@ func TestWindowsMatchComparerLess(t *testing.T) { OSVersion: "10.0.17763.1", } - m := NewMatcher(p) - if runtime.GOOS != "windows" { - // By default NewMatcher only returns the MatchComparer interface on Windows (which is only for backwards compatibility). - // On other platforms, we need to wrap the matcher in a windowsMatchComparer since the test is using it. - m = &windowsMatchComparer{m} - } + // NewMatcher only returns a Matcher; wrap it to get the Less this test exercises. + m := MatchComparer(&windowsMatchComparer{NewMatcher(p)}) platforms := []imagespec.Platform{ { Architecture: "amd64", @@ -196,7 +191,7 @@ func TestWindowsMatchComparerLess(t *testing.T) { }, } sort.SliceStable(platforms, func(i, j int) bool { - return m.(MatchComparer).Less(platforms[i], platforms[j]) + return m.Less(platforms[i], platforms[j]) }) if !reflect.DeepEqual(platforms, expected) { t.Errorf("expected: %s\nactual : %s", expected, platforms) diff --git a/platform_windows_compat.go b/platform_windows_compat.go index 4aa162a..2abef3c 100644 --- a/platform_windows_compat.go +++ b/platform_windows_compat.go @@ -18,8 +18,6 @@ package platforms import ( "slices" - "strconv" - "strings" specs "github.com/opencontainers/image-spec/specs-go/v1" ) @@ -43,7 +41,7 @@ const ( // rs5 (version 1809, codename "Redstone 5") corresponds to Windows Server // 2019 (ltsc2019), and Windows 10 (October 2018 Update). rs5 = 17763 - // ltsc2019 (Windows Server 2019) is an alias for [RS5]. + // ltsc2019 (Windows Server 2019) is an alias for [rs5]. ltsc2019 = rs5 // v21H2Server corresponds to Windows Server 2022 (ltsc2022). @@ -113,50 +111,43 @@ func checkWindowsHostAndContainerCompat(host, ctr windowsOSVersion) bool { return supportedLTSCRelease <= ctr.Build && ctr.Build <= host.Build } +// getWindowsOSVersion parses the "..[.]" form +// of a Windows OS version (e.g. "10.0.17763.1"), using the same +// parseVersionedVariant that parses every other dotted/numbered variant in +// this package — only the major/minor/build field mapping and their bit +// widths (matching the real Windows version struct) are specific to +// Windows here. Anything from the revision field onward is ignored, same as +// the Windows API this mirrors. func getWindowsOSVersion(osVersionPrefix string) windowsOSVersion { - if strings.Count(osVersionPrefix, ".") < 2 { + v, ok := parseVersionedVariant(osVersionPrefix) + if !ok || v.prefix != "" || v.suffix != "" || len(v.numbers) < 3 { return windowsOSVersion{} } - - major, extra, _ := strings.Cut(osVersionPrefix, ".") - minor, extra, _ := strings.Cut(extra, ".") - build, _, _ := strings.Cut(extra, ".") - - majorVersion, err := strconv.ParseUint(major, 10, 8) - if err != nil { - return windowsOSVersion{} - } - - minorVersion, err := strconv.ParseUint(minor, 10, 8) - if err != nil { - return windowsOSVersion{} - } - buildNumber, err := strconv.ParseUint(build, 10, 16) - if err != nil { + major, minor, build := v.numbers[0], v.numbers[1], v.numbers[2] + // parseVersionedVariant only ever produces non-negative numbers (they + // come from a run of ASCII digits), but check explicitly anyway so the + // range check below is a complete bound, not just an upper one. + if major < 0 || major > 0xff || minor < 0 || minor > 0xff || build < 0 || build > 0xffff { return windowsOSVersion{} } return windowsOSVersion{ - MajorVersion: uint8(majorVersion), - MinorVersion: uint8(minorVersion), - Build: uint16(buildNumber), + MajorVersion: uint8(major), // #nosec G115 -- range-checked above + MinorVersion: uint8(minor), // #nosec G115 -- range-checked above + Build: uint16(build), // #nosec G115 -- range-checked above } } -type windowsVersionMatcher struct { - windowsOSVersion -} - -func (m windowsVersionMatcher) Match(v string) bool { - if m.isEmpty() || v == "" { +// windowsOSVersionMatch reports whether a container declaring OS version v +// can run on a Windows host declaring hostVersion, per the Windows Server +// stable-ABI compatibility rules (see checkWindowsHostAndContainerCompat). A +// missing version on either side means "don't care", and always matches. +func windowsOSVersionMatch(hostVersion, v string) bool { + host := getWindowsOSVersion(hostVersion) + if host == (windowsOSVersion{}) || v == "" { return true } - osv := getWindowsOSVersion(v) - return checkWindowsHostAndContainerCompat(m.windowsOSVersion, osv) -} - -func (m windowsVersionMatcher) isEmpty() bool { - return m.MajorVersion == 0 && m.MinorVersion == 0 && m.Build == 0 + return checkWindowsHostAndContainerCompat(host, getWindowsOSVersion(v)) } type windowsMatchComparer struct { diff --git a/platforms.go b/platforms.go index 81d7ee3..60f7b92 100644 --- a/platforms.go +++ b/platforms.go @@ -105,8 +105,10 @@ // Similarly, the most common arm64 version v8, and most common amd64 version v1 // are represented without the variant. // -// While these normalizations are provided, their support on arm platforms has -// not yet been fully implemented and tested. +// [Only] resolves variant compatibility beyond exact equality: descending +// amd64/arm versions, arm64's cross-generation offset between its v8.x and +// v9.x lines, and any other CPU variant scheme shaped like a number embedded +// in fixed text (e.g. ppc64le's "powerN", riscv64's "rvaNNu64"). package platforms import ( @@ -153,79 +155,64 @@ func NewMatcher(platform specs.Platform) Matcher { } if platform.OS == "windows" { - m.osvM = &windowsVersionMatcher{ - windowsOSVersion: getWindowsOSVersion(platform.OSVersion), - } - // In prior versions, the win32k os feature was not considered for matching, // strip out the win32k feature for comparison - var stripped Matcher = windowsStripFeaturesMatcher{m} - - // In prior versions, on windows, the returned matcher implements a - // MatchComprarer interface. - // This preserves that behavior for backwards compatibility. - // - // TODO: This isn't actually used in this package, except for a test case, - // which may have been an unintended side of some refactor. - // It was likely intended to be used in `Ordered` but it is not since - // `Less` that is implemented here ends up getting masked due to wrapping. - if runtime.GOOS == "windows" { - return &windowsMatchComparer{stripped} - } - return stripped + return windowsStripFeaturesMatcher{m} } return m } -type osVerMatcher interface { - Match(string) bool -} - type matcher struct { specs.Platform - osvM osVerMatcher } func (m *matcher) Match(platform specs.Platform) bool { normalized := Normalize(platform) - if m.OS == normalized.OS && + return m.OS == normalized.OS && m.Architecture == normalized.Architecture && m.Variant == normalized.Variant && - m.matchOSVersion(platform) { - if len(normalized.OSFeatures) == 0 { - return true - } - if len(m.OSFeatures) >= len(normalized.OSFeatures) { - // Ensure that normalized.OSFeatures is a subset of - // m.OSFeatures - j := 0 - for _, feature := range normalized.OSFeatures { - found := false - for ; j < len(m.OSFeatures); j++ { - if feature == m.OSFeatures[j] { - found = true - j++ - break - } - // Since both lists are ordered, if the feature is less - // than what is seen, it is not in the list - if feature < m.OSFeatures[j] { - return false - } - } - if !found { - return false - } + osVersionMatch(m.OS, m.OSVersion, platform.OSVersion) && + osFeaturesSubset(normalized.OSFeatures, m.OSFeatures) +} + +// osFeaturesSubset reports whether every feature in want is present in +// have. Both slices must already be sorted, as Normalize leaves them. +func osFeaturesSubset(want, have []string) bool { + if len(want) == 0 { + return true + } + if len(have) < len(want) { + return false + } + j := 0 + for _, feature := range want { + found := false + for ; j < len(have); j++ { + if feature == have[j] { + found = true + j++ + break } - return true + // Since both lists are ordered, if the feature is less + // than what is seen, it is not in the list + if feature < have[j] { + return false + } + } + if !found { + return false } } - return false + return true } -func (m *matcher) matchOSVersion(platform specs.Platform) bool { - if m.osvM != nil { - return m.osvM.Match(platform.OSVersion) +// osVersionMatch reports whether a candidate declaring osVersion can run on +// a host with the given hostOS and hostOSVersion. Only Windows has an OS +// version compatibility rule (see windowsOSVersionMatch); every other OS +// accepts any (or no) declared version. +func osVersionMatch(hostOS, hostOSVersion, osVersion string) bool { + if hostOS == "windows" { + return windowsOSVersionMatch(hostOSVersion, osVersion) } return true } diff --git a/variant.go b/variant.go new file mode 100644 index 0000000..62b2511 --- /dev/null +++ b/variant.go @@ -0,0 +1,253 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package platforms + +import ( + "strconv" + "strings" +) + +// parsedVariant is a variant string split around its dotted sequence of +// numbers, so two variants can be compared component-wise once their +// prefix/suffix are confirmed to match. +type parsedVariant struct { + prefix string + numbers []int + suffix string +} + +// parseVersionedVariant splits a variant into a leading non-numeric prefix, +// its leading run of digits, any further "."-separated runs of digits +// immediately following it, and whatever fixed text remains after that. +// The numbers are treated as the part of the variant that encodes a linear +// compatibility ordering; the prefix and suffix around them are held fixed. +// This covers every known OCI CPU variant scheme, and OS versions, without +// naming any of them: +// +// "v3" -> prefix "v", numbers [3], suffix "" +// "v8.3" -> prefix "v", numbers [8, 3], suffix "" +// "power10" -> prefix "power", numbers [10], suffix "" +// "rva23u64" -> prefix "rva", numbers [23], suffix "u64" +// "10.0.17763.1" -> prefix "", numbers [10, 0, 17763, 1], suffix "" +// +// A "." is only ever treated as separating two numbers, never as starting +// the suffix: "rva23u64" stops at "u64" (not a digit after the — nonexistent +// — dot) without mistaking the "64" in "u64" for a second component. +func parseVersionedVariant(variant string) (parsedVariant, bool) { + i := 0 + for i < len(variant) && (variant[i] < '0' || variant[i] > '9') { + i++ + } + prefix, rest := variant[:i], variant[i:] + + var numbers []int + for { + j := 0 + for j < len(rest) && rest[j] >= '0' && rest[j] <= '9' { + j++ + } + if j == 0 { + break + } + n, err := strconv.Atoi(rest[:j]) + if err != nil { + // Overflows int (absurdly long digit run): treat as + // unversioned rather than risk surprising wraparound behavior. + return parsedVariant{}, false + } + numbers = append(numbers, n) + rest = rest[j:] + + afterDot, isDotted := strings.CutPrefix(rest, ".") + if !isDotted || afterDot == "" || afterDot[0] < '0' || afterDot[0] > '9' { + break + } + rest = afterDot + } + if len(numbers) == 0 { + return parsedVariant{}, false + } + return parsedVariant{prefix: prefix, numbers: numbers, suffix: rest}, true +} + +// compareVersions compares two numeric version tuples component-wise, most +// significant first, treating a missing trailing component as 0 (so [8] == +// [8, 0]). It returns -1, 0, or 1 as a < b, a == b, or a > b. +func compareVersions(a, b []int) int { + for i := 0; i < max(len(a), len(b)); i++ { + var x, y int + if i < len(a) { + x = a[i] + } + if i < len(b) { + y = b[i] + } + switch { + case x < y: + return -1 + case x > y: + return 1 + } + } + return 0 +} + +// sameShapeNumbers parses hostVariant and imageVariant and, if they share +// the same prefix and suffix (differing only in their numbers), returns +// both their numeric components for comparison. +func sameShapeNumbers(hostVariant, imageVariant string) (hostNumbers, imageNumbers []int, ok bool) { + h, hok := parseVersionedVariant(hostVariant) + i, iok := parseVersionedVariant(imageVariant) + if !hok || !iok || h.prefix != i.prefix || h.suffix != i.suffix { + return nil, nil, false + } + return h.numbers, i.numbers, true +} + +// numberedVariantMatch reports whether an image declaring imageVariant can +// run on a host declaring hostVariant, for architectures with a real +// minimum ("floor") version — amd64 and arm — where a bare/empty variant is +// that architecture's own canonical form for its floor version, rather than +// "no requirement". +func numberedVariantMatch(hostVariant, imageVariant, floorVariant string) bool { + if hostVariant == "" { + hostVariant = floorVariant + } + if imageVariant == "" { + imageVariant = floorVariant + } + if imageVariant == hostVariant { + return true + } + + hostNumbers, imageNumbers, ok := sameShapeNumbers(hostVariant, imageVariant) + if !ok { + return false + } + floor, _ := parseVersionedVariant(floorVariant) + return compareVersions(imageNumbers, floor.numbers) >= 0 && compareVersions(imageNumbers, hostNumbers) <= 0 +} + +// genericVariantMatch handles any architecture without dedicated version +// handling (e.g. ppc64le's "powerN", riscv64's "rvaNNu64"), purely by the +// shape of the variant string rather than by architecture name. An image +// declaring no variant at all is treated as carrying no version +// requirement, so it matches any host of the same architecture. +func genericVariantMatch(hostVariant, imageVariant string) bool { + if imageVariant == hostVariant || imageVariant == "" { + return true + } + + hostNumbers, imageNumbers, ok := sameShapeNumbers(hostVariant, imageVariant) + if !ok { + return false + } + return compareVersions(imageNumbers, hostNumbers) <= 0 +} + +// arm64MaxV8Minor is the highest Armv8 minor version that will ever exist: +// Arm's own architecture documentation states that Armv9.N always carries +// the mandatory feature baseline of Armv8.(N+5), and that the Armv8 line +// stops being extended at .9 (all further baseline growth happens under the +// v9 line). This makes the offset a fixed constant rather than a table that +// needs an entry added for every new minor version. +const arm64MaxV8Minor = 9 + +// arm64Minor returns numbers[1] (the minor version), or 0 if it isn't +// specified (a bare "v8" means the same thing as "v8.0"). +func arm64Minor(numbers []int) int { + if len(numbers) > 1 { + return numbers[1] + } + return 0 +} + +// arm64VariantMatch reports whether an image declaring imageVariant can run +// on an arm64 host declaring hostVariant. Parsing is the same generic +// parseVersionedVariant used everywhere else; only the cross-generation +// offset below (see arm64MaxV8Minor) is genuinely arm64-specific, computed +// arithmetically instead of from a table enumerating every known +// major/minor pair. +func arm64VariantMatch(hostVariant, imageVariant string) bool { + if hostVariant == "" { + hostVariant = "v8" + } + if imageVariant == "" { + imageVariant = "v8" + } + if imageVariant == hostVariant { + return true + } + + h, hok := parseVersionedVariant(hostVariant) + i, iok := parseVersionedVariant(imageVariant) + if !hok || !iok || h.prefix != "v" || i.prefix != "v" || h.suffix != "" || i.suffix != "" || + len(h.numbers) > 2 || len(i.numbers) > 2 { + return false + } + hMajor, hMinor := h.numbers[0], arm64Minor(h.numbers) + iMajor, iMinor := i.numbers[0], arm64Minor(i.numbers) + + if iMajor == hMajor { + return iMinor <= hMinor + } + if hMajor == 9 && iMajor == 8 { + return iMinor <= min(hMinor+5, arm64MaxV8Minor) + } + return false +} + +// naturalLess reports whether a sorts before b in "natural" order: runs of +// ASCII digits are compared as numbers (so "9" < "10"), and everything else +// is compared as plain text. It has no knowledge of any particular variant +// or version scheme's shape — it's the same ordering a file manager uses to +// sort "power9" before "power10", or "v8.9" before "v8.10". +func naturalLess(a, b string) bool { + for len(a) > 0 && len(b) > 0 { + da, ra := splitLeadingDigits(a) + db, rb := splitLeadingDigits(b) + if da != "" && db != "" { + na := strings.TrimLeft(da, "0") + nb := strings.TrimLeft(db, "0") + switch { + case len(na) != len(nb): + return len(na) < len(nb) + case na != nb: + return na < nb + case len(da) != len(db): + // Numerically equal (differing only in leading zeros): + // fewer leading zeros sorts first, for a total order. + return len(da) < len(db) + } + a, b = ra, rb + continue + } + if a[0] != b[0] { + return a[0] < b[0] + } + a, b = a[1:], b[1:] + } + return len(a) < len(b) +} + +func splitLeadingDigits(s string) (digits, rest string) { + i := 0 + for i < len(s) && s[i] >= '0' && s[i] <= '9' { + i++ + } + return s[:i], s[i:] +} diff --git a/variant_test.go b/variant_test.go new file mode 100644 index 0000000..9686299 --- /dev/null +++ b/variant_test.go @@ -0,0 +1,181 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package platforms + +import ( + "slices" + "testing" +) + +func TestParseVersionedVariant(t *testing.T) { + for _, tc := range []struct { + variant string + prefix string + numbers []int + suffix string + ok bool + }{ + {variant: "v3", prefix: "v", numbers: []int{3}, suffix: "", ok: true}, + {variant: "v8.5", prefix: "v", numbers: []int{8, 5}, suffix: "", ok: true}, + {variant: "power10", prefix: "power", numbers: []int{10}, suffix: "", ok: true}, + {variant: "rva23u64", prefix: "rva", numbers: []int{23}, suffix: "u64", ok: true}, + {variant: "10.0.17763.1", prefix: "", numbers: []int{10, 0, 17763, 1}, suffix: "", ok: true}, + {variant: "v8.3.1", prefix: "v", numbers: []int{8, 3, 1}, suffix: "", ok: true}, + {variant: "", ok: false}, + {variant: "custom", ok: false}, + {variant: "v99999999999999999999", ok: false}, // overflows int + } { + t.Run(tc.variant, func(t *testing.T) { + p, ok := parseVersionedVariant(tc.variant) + if ok != tc.ok { + t.Fatalf("parseVersionedVariant(%q) ok = %v, want %v", tc.variant, ok, tc.ok) + } + if !tc.ok { + return + } + if p.prefix != tc.prefix || !slices.Equal(p.numbers, tc.numbers) || p.suffix != tc.suffix { + t.Fatalf("parseVersionedVariant(%q) = %+v, want {%q %v %q}", tc.variant, p, tc.prefix, tc.numbers, tc.suffix) + } + }) + } +} + +// TestParseVersionedVariantStopsAtUndottedDigits ensures a digit run that +// isn't introduced by a literal "." is left in the opaque suffix rather +// than treated as another ranked version component. "." is the separator +// every dotted version scheme already uses; parsedVariant has no way to +// represent "components separated by something else", so a fixed +// identifier that happens to end in digits (like riscv64's "u64"/"s64" +// mode+width suffix) is compared as one exact-match string, never split. +func TestParseVersionedVariantStopsAtUndottedDigits(t *testing.T) { + p, ok := parseVersionedVariant("rva20u64") + if !ok { + t.Fatal("expected rva20u64 to parse") + } + if !slices.Equal(p.numbers, []int{20}) { + t.Fatalf("numbers = %v, want [20]", p.numbers) + } + if p.suffix != "u64" { + t.Fatalf("suffix = %q, want %q", p.suffix, "u64") + } +} + +// TestNumberedVariantMatchHuge ensures compatibility between a host and +// image is computed directly, with no cost proportional to the magnitude of +// either variant's version number. +func TestNumberedVariantMatchHuge(t *testing.T) { + if !numberedVariantMatch("v1000000000", "v999999999", "v1") { + t.Fatal("expected a huge but lower image version to match") + } + if numberedVariantMatch("v5", "v6", "v1") { + t.Fatal("expected higher image version to not match a lower host version") + } +} + +func TestGenericVariantMatch(t *testing.T) { + for _, tc := range []struct { + name string + host string + image string + wantOK bool + }{ + {name: "exact", host: "power10", image: "power10", wantOK: true}, + {name: "lower", host: "power10", image: "power8", wantOK: true}, + {name: "higher", host: "power8", image: "power10", wantOK: false}, + {name: "bare image", host: "power10", image: "", wantOK: true}, + {name: "bare host, bare image", host: "", image: "", wantOK: true}, + {name: "bare host, versioned image", host: "", image: "power8", wantOK: false}, + {name: "different prefix", host: "power10", image: "rva20u64", wantOK: false}, + {name: "different suffix", host: "rva23u64", image: "rva20s64", wantOK: false}, + {name: "riscv lower", host: "rva23u64", image: "rva20u64", wantOK: true}, + } { + t.Run(tc.name, func(t *testing.T) { + if ok := genericVariantMatch(tc.host, tc.image); ok != tc.wantOK { + t.Fatalf("genericVariantMatch(%q, %q) = %v, want %v", tc.host, tc.image, ok, tc.wantOK) + } + }) + } +} + +func TestArm64VariantMatch(t *testing.T) { + for _, tc := range []struct { + name string + host string + image string + wantOK bool + }{ + {name: "v9.6 vs v8.9 (within v9.6+5 cap)", host: "v9.6", image: "v8.9", wantOK: true}, + {name: "v9.6 vs v8.10 (no such version, exceeds cap)", host: "v9.6", image: "v8.10", wantOK: false}, + {name: "v9 vs v8.5 (offset boundary)", host: "v9", image: "v8.5", wantOK: true}, + {name: "v9 vs v8.6 (past offset boundary)", host: "v9", image: "v8.6", wantOK: false}, + {name: "v8.1 vs v9 (lower major can't reach higher)", host: "v8.1", image: "v9", wantOK: false}, + {name: "v9.8 vs v9.0 (future minor, no table needed)", host: "v9.8", image: "v9.0", wantOK: true}, + {name: "v9.8 vs v8.9 (future minor, offset caps at .9)", host: "v9.8", image: "v8.9", wantOK: true}, + {name: "bare host is v8", host: "", image: "v8", wantOK: true}, + {name: "no third component", host: "v9.6", image: "v8.9.1", wantOK: false}, + } { + t.Run(tc.name, func(t *testing.T) { + if ok := arm64VariantMatch(tc.host, tc.image); ok != tc.wantOK { + t.Fatalf("arm64VariantMatch(%q, %q) = %v, want %v", tc.host, tc.image, ok, tc.wantOK) + } + }) + } +} + +func TestCompareVersions(t *testing.T) { + for _, tc := range []struct { + a, b []int + want int + }{ + {[]int{8}, []int{8}, 0}, + {[]int{8}, []int{8, 0}, 0}, // missing trailing component treated as 0 + {[]int{8}, []int{8, 1}, -1}, + {[]int{8, 3}, []int{8, 9}, -1}, + {[]int{9, 0}, []int{8, 9}, 1}, // first component dominates + {[]int{10, 0, 17763, 1}, []int{10, 0, 17763, 2}, -1}, + } { + if got := compareVersions(tc.a, tc.b); got != tc.want { + t.Errorf("compareVersions(%v, %v) = %d, want %d", tc.a, tc.b, got, tc.want) + } + } +} + +func TestNaturalLess(t *testing.T) { + for _, tc := range []struct { + a, b string + want bool + }{ + {"power9", "power10", true}, + {"power10", "power9", false}, + {"v8.5", "v8.10", true}, + {"v8.10", "v8.5", false}, + {"rva20u64", "rva23u64", true}, + {"rva23u64", "rva20u64", false}, + {"v9", "v9", false}, + {"v9", "v9.0", true}, // shorter string, otherwise identical, sorts first + {"", "a", true}, + {"9", "09", true}, // numerically equal; fewer leading zeros sorts first + {"09", "9", false}, + {"abc", "abd", true}, + } { + t.Run(tc.a+"_"+tc.b, func(t *testing.T) { + if got := naturalLess(tc.a, tc.b); got != tc.want { + t.Errorf("naturalLess(%q, %q) = %v, want %v", tc.a, tc.b, got, tc.want) + } + }) + } +}