diff --git a/defaults_test.go b/defaults_test.go new file mode 100644 index 0000000..d0e1163 --- /dev/null +++ b/defaults_test.go @@ -0,0 +1,204 @@ +/* + 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 ( + "reflect" + "runtime" + "sort" + "testing" + + imagespec "github.com/opencontainers/image-spec/specs-go/v1" +) + +// TestMatchComparerMatch_ABICheckWCOW checks windows platform matcher +// behavior for stable ABI and non-stable ABI compliant versions +func TestMatchComparerMatch_ABICheckWCOW(t *testing.T) { + platformNoVersion := imagespec.Platform{ + Architecture: "amd64", + OS: "windows", + } + platformWS2019 := imagespec.Platform{ + Architecture: "amd64", + OS: "windows", + OSVersion: "10.0.17763", + } + platformWS2022 := imagespec.Platform{ + Architecture: "amd64", + OS: "windows", + OSVersion: "10.0.20348", + } + platformWindows11 := imagespec.Platform{ + Architecture: "amd64", + OS: "windows", + OSVersion: "10.0.22621", + } + + for _, test := range []struct { + hostPlatform imagespec.Platform + testPlatform imagespec.Platform + match bool + }{ + { + hostPlatform: platformWS2019, + testPlatform: imagespec.Platform{ + Architecture: "amd64", + OS: "windows", + OSVersion: "10.0.17763", + }, + match: true, + }, + { + hostPlatform: platformWS2019, + testPlatform: imagespec.Platform{ + Architecture: "amd64", + OS: "windows", + OSVersion: "10.0.20348", + }, + match: false, + }, + { + hostPlatform: platformWS2022, + testPlatform: imagespec.Platform{ + Architecture: "amd64", + OS: "windows", + OSVersion: "10.0.17763", + }, + match: false, + }, + { + hostPlatform: platformWS2022, + testPlatform: imagespec.Platform{ + Architecture: "amd64", + OS: "windows", + OSVersion: "10.0.20348", + }, + match: true, + }, + { + hostPlatform: platformWindows11, + testPlatform: imagespec.Platform{ + Architecture: "amd64", + OS: "windows", + OSVersion: "10.0.17763", + }, + match: false, + }, + { + hostPlatform: platformWindows11, + testPlatform: imagespec.Platform{ + Architecture: "amd64", + OS: "windows", + OSVersion: "10.0.20348", + }, + match: true, + }, + { + hostPlatform: platformNoVersion, + testPlatform: platformWS2019, + match: true, + }, + { + hostPlatform: platformNoVersion, + testPlatform: platformNoVersion, + match: true, + }, + { + hostPlatform: platformNoVersion, + testPlatform: platformWindows11, + match: true, + }, + } { + matcher := NewMatcher(test.hostPlatform) + if actual := matcher.Match(test.testPlatform); actual != test.match { + t.Errorf("should match: %t, test(%s) to host(%s)", test.match, test.hostPlatform, test.testPlatform) + } + } +} + +func TestWindowsMatchComparerLess(t *testing.T) { + p := imagespec.Platform{ + Architecture: "amd64", + OS: "windows", + 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} + } + platforms := []imagespec.Platform{ + { + Architecture: "amd64", + OS: "windows", + OSVersion: "10.0.17764.1", + }, + { + Architecture: "amd64", + OS: "windows", + }, + { + Architecture: "amd64", + OS: "windows", + OSVersion: "10.0.17763.1", + }, + { + Architecture: "amd64", + OS: "windows", + OSVersion: "10.0.17763.2", + }, + { + Architecture: "amd64", + OS: "windows", + OSVersion: "10.0.17762.1", + }, + } + expected := []imagespec.Platform{ + { + Architecture: "amd64", + OS: "windows", + OSVersion: "10.0.17763.2", + }, + { + Architecture: "amd64", + OS: "windows", + OSVersion: "10.0.17763.1", + }, + { + Architecture: "amd64", + OS: "windows", + }, + { + Architecture: "amd64", + OS: "windows", + OSVersion: "10.0.17764.1", + }, + { + Architecture: "amd64", + OS: "windows", + OSVersion: "10.0.17762.1", + }, + } + sort.SliceStable(platforms, func(i, j int) bool { + return m.(MatchComparer).Less(platforms[i], platforms[j]) + }) + if !reflect.DeepEqual(platforms, expected) { + t.Errorf("expected: %s\nactual : %s", expected, platforms) + } +} diff --git a/defaults_windows.go b/defaults_windows.go index 8bae4eb..0165ade 100644 --- a/defaults_windows.go +++ b/defaults_windows.go @@ -19,8 +19,6 @@ package platforms import ( "fmt" "runtime" - "strconv" - "strings" specs "github.com/opencontainers/image-spec/specs-go/v1" "golang.org/x/sys/windows" @@ -38,80 +36,6 @@ func DefaultSpec() specs.Platform { } } -type windowsmatcher struct { - specs.Platform - osVersionPrefix string - defaultMatcher Matcher -} - -// Match matches platform with the same windows major, minor -// and build version. -func (m windowsmatcher) Match(p specs.Platform) bool { - match := m.defaultMatcher.Match(p) - - if match && m.OS == "windows" { - // HPC containers do not have OS version filled - if m.OSVersion == "" || p.OSVersion == "" { - return true - } - - hostOsVersion := getOSVersion(m.osVersionPrefix) - ctrOsVersion := getOSVersion(p.OSVersion) - return checkHostAndContainerCompat(hostOsVersion, ctrOsVersion) - } - - return match -} - -func getOSVersion(osVersionPrefix string) osVersion { - parts := strings.Split(osVersionPrefix, ".") - if len(parts) < 3 { - return osVersion{} - } - - majorVersion, _ := strconv.ParseUint(parts[0], 10, 8) - minorVersion, _ := strconv.ParseUint(parts[1], 10, 8) - buildNumber, _ := strconv.ParseUint(parts[2], 10, 16) - - return osVersion{ - MajorVersion: uint8(majorVersion), - MinorVersion: uint8(minorVersion), - Build: uint16(buildNumber), - } -} - -// Less sorts matched platforms in front of other platforms. -// For matched platforms, it puts platforms with larger revision -// number in front. -func (m windowsmatcher) Less(p1, p2 specs.Platform) bool { - m1, m2 := m.Match(p1), m.Match(p2) - if m1 && m2 { - r1, r2 := revision(p1.OSVersion), revision(p2.OSVersion) - return r1 > r2 - } - return m1 && !m2 -} - -func revision(v string) int { - parts := strings.Split(v, ".") - if len(parts) < 4 { - return 0 - } - r, err := strconv.Atoi(parts[3]) - if err != nil { - return 0 - } - return r -} - -func prefix(v string) string { - parts := strings.Split(v, ".") - if len(parts) < 4 { - return v - } - return strings.Join(parts[0:3], ".") -} - // Default returns the current platform's default platform specification. func Default() MatchComparer { return Only(DefaultSpec()) diff --git a/defaults_windows_test.go b/defaults_windows_test.go index 8d459e3..c3193a5 100644 --- a/defaults_windows_test.go +++ b/defaults_windows_test.go @@ -20,7 +20,6 @@ import ( "fmt" "reflect" "runtime" - "sort" "testing" imagespec "github.com/opencontainers/image-spec/specs-go/v1" @@ -69,19 +68,14 @@ func TestDefaultMatchComparer(t *testing.T) { t.Errorf("expected: %v, actual: %v", test.match, actual) } } - } func TestMatchComparerMatch_WCOW(t *testing.T) { major, minor, build := windows.RtlGetNtVersionNumbers() buildStr := fmt.Sprintf("%d.%d.%d", major, minor, build) - m := windowsmatcher{ - Platform: DefaultSpec(), - osVersionPrefix: buildStr, - defaultMatcher: &matcher{ - Platform: Normalize(DefaultSpec()), - }, - } + platform := DefaultSpec() + m := NewMatcher(platform) + for _, test := range []struct { platform imagespec.Platform match bool @@ -140,133 +134,7 @@ func TestMatchComparerMatch_WCOW(t *testing.T) { }, } { if actual := m.Match(test.platform); actual != test.match { - t.Errorf("should match: %t, %s to %s", test.match, m.Platform, test.platform) - } - } -} - -// TestMatchComparerMatch_ABICheckWCOW checks windows platform matcher -// behavior for stable ABI and non-stable ABI compliant versions -func TestMatchComparerMatch_ABICheckWCOW(t *testing.T) { - platformNoVersion := imagespec.Platform{ - Architecture: "amd64", - OS: "windows", - } - platformWS2019 := imagespec.Platform{ - Architecture: "amd64", - OS: "windows", - OSVersion: "10.0.17763", - } - platformWS2022 := imagespec.Platform{ - Architecture: "amd64", - OS: "windows", - OSVersion: "10.0.20348", - } - platformWindows11 := imagespec.Platform{ - Architecture: "amd64", - OS: "windows", - OSVersion: "10.0.22621", - } - matcherNoVersion := NewMatcher(platformNoVersion).(windowsmatcher) - matcherWS2019 := windowsmatcher{ - Platform: platformWS2019, - osVersionPrefix: platformWS2019.OSVersion, - defaultMatcher: &matcher{ - Platform: Normalize(platformWS2019), - }, - } - matcherWS2022 := windowsmatcher{ - Platform: platformWS2022, - osVersionPrefix: platformWS2022.OSVersion, - defaultMatcher: &matcher{ - Platform: Normalize(platformWS2022), - }, - } - matcherWindows11 := windowsmatcher{ - Platform: platformWindows11, - osVersionPrefix: platformWindows11.OSVersion, - defaultMatcher: &matcher{ - Platform: Normalize(platformWindows11), - }, - } - - for _, test := range []struct { - hostPlatformMatcher windowsmatcher - testPlatform imagespec.Platform - match bool - }{ - { - hostPlatformMatcher: matcherWS2019, - testPlatform: imagespec.Platform{ - Architecture: "amd64", - OS: "windows", - OSVersion: "10.0.17763", - }, - match: true, - }, - { - hostPlatformMatcher: matcherWS2019, - testPlatform: imagespec.Platform{ - Architecture: "amd64", - OS: "windows", - OSVersion: "10.0.20348", - }, - match: false, - }, - { - hostPlatformMatcher: matcherWS2022, - testPlatform: imagespec.Platform{ - Architecture: "amd64", - OS: "windows", - OSVersion: "10.0.17763", - }, - match: false, - }, - { - hostPlatformMatcher: matcherWS2022, - testPlatform: imagespec.Platform{ - Architecture: "amd64", - OS: "windows", - OSVersion: "10.0.20348", - }, - match: true, - }, - { - hostPlatformMatcher: matcherWindows11, - testPlatform: imagespec.Platform{ - Architecture: "amd64", - OS: "windows", - OSVersion: "10.0.17763", - }, - match: false, - }, - { - hostPlatformMatcher: matcherWindows11, - testPlatform: imagespec.Platform{ - Architecture: "amd64", - OS: "windows", - OSVersion: "10.0.20348", - }, - match: true, - }, - { - hostPlatformMatcher: matcherNoVersion, - testPlatform: platformWS2019, - match: true, - }, - { - hostPlatformMatcher: matcherNoVersion, - testPlatform: platformNoVersion, - match: true, - }, - { - hostPlatformMatcher: matcherNoVersion, - testPlatform: platformWindows11, - match: true, - }, - } { - if actual := test.hostPlatformMatcher.Match(test.testPlatform); actual != test.match { - t.Errorf("should match: %t, %s to %s", test.match, test.hostPlatformMatcher.Platform, test.testPlatform) + t.Errorf("should match: %t, %s to %s", test.match, platform, test.platform) } } } @@ -274,20 +142,9 @@ func TestMatchComparerMatch_ABICheckWCOW(t *testing.T) { func TestMatchComparerMatch_LCOW(t *testing.T) { major, minor, build := windows.RtlGetNtVersionNumbers() buildStr := fmt.Sprintf("%d.%d.%d", major, minor, build) - m := windowsmatcher{ - Platform: imagespec.Platform{ - OS: "linux", - Architecture: "amd64", - }, - osVersionPrefix: "", - defaultMatcher: &matcher{ - Platform: Normalize(imagespec.Platform{ - OS: "linux", - Architecture: "amd64", - }, - ), - }, - } + + pLinux := imagespec.Platform{OS: "linux", Architecture: "amd64"} + m := NewMatcher(pLinux) for _, test := range []struct { platform imagespec.Platform match bool @@ -329,75 +186,7 @@ func TestMatchComparerMatch_LCOW(t *testing.T) { }, } { if actual := m.Match(test.platform); actual != test.match { - t.Errorf("should match: %t, %s to %s", test.match, m.Platform, test.platform) + t.Errorf("should match: %t, %s to %s", test.match, pLinux, test.platform) } } } - -func TestMatchComparerLess(t *testing.T) { - m := windowsmatcher{ - Platform: DefaultSpec(), - osVersionPrefix: "10.0.17763", - defaultMatcher: &matcher{ - Platform: Normalize(DefaultSpec()), - }, - } - platforms := []imagespec.Platform{ - { - Architecture: "amd64", - OS: "windows", - OSVersion: "10.0.17764.1", - }, - { - Architecture: "amd64", - OS: "windows", - }, - { - Architecture: "amd64", - OS: "windows", - OSVersion: "10.0.17763.1", - }, - { - Architecture: "amd64", - OS: "windows", - OSVersion: "10.0.17763.2", - }, - { - Architecture: "amd64", - OS: "windows", - OSVersion: "10.0.17762.1", - }, - } - expected := []imagespec.Platform{ - { - Architecture: "amd64", - OS: "windows", - OSVersion: "10.0.17763.2", - }, - { - Architecture: "amd64", - OS: "windows", - OSVersion: "10.0.17763.1", - }, - { - Architecture: "amd64", - OS: "windows", - }, - { - Architecture: "amd64", - OS: "windows", - OSVersion: "10.0.17764.1", - }, - { - Architecture: "amd64", - OS: "windows", - OSVersion: "10.0.17762.1", - }, - } - sort.SliceStable(platforms, func(i, j int) bool { - return m.Less(platforms[i], platforms[j]) - }) - if !reflect.DeepEqual(platforms, expected) { - t.Errorf("expected: %s\nactual : %s", expected, platforms) - } -} diff --git a/platform_compat_windows.go b/platform_windows_compat.go similarity index 62% rename from platform_compat_windows.go rename to platform_windows_compat.go index 89e66f0..ce375ce 100644 --- a/platform_compat_windows.go +++ b/platform_windows_compat.go @@ -16,9 +16,16 @@ package platforms +import ( + "strconv" + "strings" + + specs "github.com/opencontainers/image-spec/specs-go/v1" +) + // osVersion is a wrapper for Windows version information // https://msdn.microsoft.com/en-us/library/windows/desktop/ms724439(v=vs.85).aspx -type osVersion struct { +type windowsOSVersion struct { Version uint32 MajorVersion uint8 MinorVersion uint8 @@ -55,7 +62,7 @@ var compatLTSCReleases = []uint16{ // Every release after WS 2022 will support the previous ltsc // container image. Stable ABI is in preview mode for windows 11 client. // Refer: https://learn.microsoft.com/en-us/virtualization/windowscontainers/deploy-containers/version-compatibility?tabs=windows-server-2022%2Cwindows-10#windows-server-host-os-compatibility -func checkHostAndContainerCompat(host, ctr osVersion) bool { +func checkWindowsHostAndContainerCompat(host, ctr windowsOSVersion) bool { // check major minor versions of host and guest if host.MajorVersion != ctr.MajorVersion || host.MinorVersion != ctr.MinorVersion { @@ -76,3 +83,64 @@ func checkHostAndContainerCompat(host, ctr osVersion) bool { } return ctr.Build >= supportedLtscRelease && ctr.Build <= host.Build } + +func getWindowsOSVersion(osVersionPrefix string) windowsOSVersion { + if strings.Count(osVersionPrefix, ".") < 2 { + return windowsOSVersion{} + } + + major, extra, _ := strings.Cut(osVersionPrefix, ".") + minor, extra, _ := strings.Cut(extra, ".") + build, _, _ := strings.Cut(extra, ".") + + majorVersion, _ := strconv.ParseUint(major, 10, 8) + minorVersion, _ := strconv.ParseUint(minor, 10, 8) + buildNumber, _ := strconv.ParseUint(build, 10, 16) + + return windowsOSVersion{ + MajorVersion: uint8(majorVersion), + MinorVersion: uint8(minorVersion), + Build: uint16(buildNumber), + } +} + +func winRevision(v string) int { + parts := strings.Split(v, ".") + if len(parts) < 4 { + return 0 + } + r, err := strconv.Atoi(parts[3]) + if err != nil { + return 0 + } + return r +} + +type windowsVersionMatcher struct { + windowsOSVersion +} + +func (m *windowsVersionMatcher) Match(v string) bool { + if m.isEmpty() || 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 +} + +type windowsMatchComparer struct { + Matcher +} + +func (c *windowsMatchComparer) Less(p1, p2 specs.Platform) bool { + m1, m2 := c.Match(p1), c.Match(p2) + if m1 && m2 { + r1, r2 := winRevision(p1.OSVersion), winRevision(p2.OSVersion) + return r1 > r2 + } + return m1 && !m2 +} diff --git a/platform_compat_windows_test.go b/platform_windows_compat_test.go similarity index 92% rename from platform_compat_windows_test.go rename to platform_windows_compat_test.go index 482c181..6f687e1 100644 --- a/platform_compat_windows_test.go +++ b/platform_windows_compat_test.go @@ -63,17 +63,17 @@ func Test_PlatformCompat(t *testing.T) { // Check if ltsc2019/ltsc2022 guest images are compatible on // the given host OS versions // - hostOSVersion := osVersion{ + hostOSVersion := windowsOSVersion{ MajorVersion: 10, MinorVersion: 0, Build: tc.hostOs, } - ctrOSVersion := osVersion{ + ctrOSVersion := windowsOSVersion{ MajorVersion: 10, MinorVersion: 0, Build: tc.ctrOs, } - if checkHostAndContainerCompat(hostOSVersion, ctrOSVersion) != tc.shouldRun { + if checkWindowsHostAndContainerCompat(hostOSVersion, ctrOSVersion) != tc.shouldRun { var expectedResultStr string if !tc.shouldRun { expectedResultStr = " NOT" diff --git a/platforms.go b/platforms.go index 7a84449..4e31555 100644 --- a/platforms.go +++ b/platforms.go @@ -144,18 +144,51 @@ type Matcher interface { // // Applications should opt to use `Match` over directly parsing specifiers. func NewMatcher(platform specs.Platform) Matcher { - return newDefaultMatcher(platform) + m := &matcher{ + Platform: Normalize(platform), + } + + if platform.OS == "windows" { + m.osvM = &windowsVersionMatcher{ + windowsOSVersion: getWindowsOSVersion(platform.OSVersion), + } + // 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 at all, which may have been + // an unintended side of some refactor. + // I suspect that it was 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{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) return m.OS == normalized.OS && m.Architecture == normalized.Architecture && - m.Variant == normalized.Variant + m.Variant == normalized.Variant && + m.matchOSVersion(platform) +} + +func (m *matcher) matchOSVersion(platform specs.Platform) bool { + if m.osvM != nil { + return m.osvM.Match(platform.OSVersion) + } + return true } func (m *matcher) String() string { diff --git a/platforms_other.go b/platforms_other.go deleted file mode 100644 index 03f4dcd..0000000 --- a/platforms_other.go +++ /dev/null @@ -1,30 +0,0 @@ -//go:build !windows - -/* - 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 ( - specs "github.com/opencontainers/image-spec/specs-go/v1" -) - -// NewMatcher returns the default Matcher for containerd -func newDefaultMatcher(platform specs.Platform) Matcher { - return &matcher{ - Platform: Normalize(platform), - } -} diff --git a/platforms_windows.go b/platforms_windows.go deleted file mode 100644 index 950e2a2..0000000 --- a/platforms_windows.go +++ /dev/null @@ -1,34 +0,0 @@ -/* - 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 ( - specs "github.com/opencontainers/image-spec/specs-go/v1" -) - -// NewMatcher returns a Windows matcher that will match on osVersionPrefix if -// the platform is Windows otherwise use the default matcher -func newDefaultMatcher(platform specs.Platform) Matcher { - prefix := prefix(platform.OSVersion) - return windowsmatcher{ - Platform: platform, - osVersionPrefix: prefix, - defaultMatcher: &matcher{ - Platform: Normalize(platform), - }, - } -}