Merge pull request #6491 from jsturtevant/windows-platforms

estesp · web-flow · commit c0bbaf5d15bd · 2022-02-18T13:07:37.000-05:00
Use Windows matcher when on Windows platform in all code paths
diff --git a/platforms/defaults_windows.go b/platforms/defaults_windows.go
@@ -22,7 +22,6 @@ import (
 	"strconv"
 	"strings"
 
-	imagespec "github.com/opencontainers/image-spec/specs-go/v1"
 	specs "github.com/opencontainers/image-spec/specs-go/v1"
 	"golang.org/x/sys/windows"
 )
@@ -39,25 +38,28 @@ func DefaultSpec() specs.Platform {
 	}
 }
 
-type matchComparer struct {
-	defaults        Matcher
+type windowsmatcher struct {
+	specs.Platform
 	osVersionPrefix string
+	defaultMatcher  Matcher
 }
 
 // Match matches platform with the same windows major, minor
 // and build version.
-func (m matchComparer) Match(p imagespec.Platform) bool {
-	if m.defaults.Match(p) {
-		// TODO(windows): Figure out whether OSVersion is deprecated.
-		return strings.HasPrefix(p.OSVersion, m.osVersionPrefix)
+func (m windowsmatcher) Match(p specs.Platform) bool {
+	match := m.defaultMatcher.Match(p)
+
+	if match && m.OS == "windows" {
+		return strings.HasPrefix(p.OSVersion, m.osVersionPrefix) && m.defaultMatcher.Match(p)
 	}
-	return false
+
+	return match
 }
 
 // Less sorts matched platforms in front of other platforms.
 // For matched platforms, it puts platforms with larger revision
 // number in front.
-func (m matchComparer) Less(p1, p2 imagespec.Platform) bool {
+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)
@@ -78,14 +80,15 @@ func revision(v string) int {
 	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 {
-	major, minor, build := windows.RtlGetNtVersionNumbers()
-	return matchComparer{
-		defaults: Ordered(DefaultSpec(), specs.Platform{
-			OS:           "linux",
-			Architecture: runtime.GOARCH,
-		}),
-		osVersionPrefix: fmt.Sprintf("%d.%d.%d", major, minor, build),
-	}
+	return Only(DefaultSpec())
 }
diff --git a/platforms/defaults_windows_test.go b/platforms/defaults_windows_test.go
@@ -47,15 +47,39 @@ func TestDefault(t *testing.T) {
 	}
 }
 
-func TestMatchComparerMatch(t *testing.T) {
+func TestDefaultMatchComparer(t *testing.T) {
+	defaultMatcher := Default()
+
+	for _, test := range []struct {
+		platform imagespec.Platform
+		match    bool
+	}{
+		{
+			platform: DefaultSpec(),
+			match:    true,
+		},
+		{
+			platform: imagespec.Platform{
+				OS:           "linux",
+				Architecture: runtime.GOARCH,
+			},
+			match: false,
+		},
+	} {
+		assert.Equal(t, test.match, defaultMatcher.Match(test.platform))
+	}
+
+}
+
+func TestMatchComparerMatch_WCOW(t *testing.T) {
 	major, minor, build := windows.RtlGetNtVersionNumbers()
 	buildStr := fmt.Sprintf("%d.%d.%d", major, minor, build)
-	m := matchComparer{
-		defaults: Only(imagespec.Platform{
-			Architecture: "amd64",
-			OS:           "windows",
-		}),
+	m := windowsmatcher{
+		Platform:        DefaultSpec(),
 		osVersionPrefix: buildStr,
+		defaultMatcher: &matcher{
+			Platform: Normalize(DefaultSpec()),
+		},
 	}
 	for _, test := range []struct {
 		platform imagespec.Platform
@@ -106,18 +130,86 @@ func TestMatchComparerMatch(t *testing.T) {
 			},
 			match: false,
 		},
+		{
+			platform: imagespec.Platform{
+				Architecture: "amd64",
+				OS:           "linux",
+			},
+			match: false,
+		},
 	} {
-		assert.Equal(t, test.match, m.Match(test.platform))
+		assert.Equal(t, test.match, m.Match(test.platform), "should match: %t, %s to %s", test.match, m.Platform, test.platform)
 	}
 }
 
-func TestMatchComparerLess(t *testing.T) {
-	m := matchComparer{
-		defaults: Only(imagespec.Platform{
+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",
-			OS:           "windows",
-		}),
+		},
+		osVersionPrefix: "",
+		defaultMatcher: &matcher{
+			Platform: Normalize(imagespec.Platform{
+				OS:           "linux",
+				Architecture: "amd64",
+			},
+			),
+		},
+	}
+	for _, test := range []struct {
+		platform imagespec.Platform
+		match    bool
+	}{
+		{
+			platform: DefaultSpec(),
+			match:    false,
+		},
+		{
+			platform: imagespec.Platform{
+				Architecture: "amd64",
+				OS:           "windows",
+			},
+			match: false,
+		},
+		{
+			platform: imagespec.Platform{
+				Architecture: "amd64",
+				OS:           "windows",
+				OSVersion:    buildStr + ".2",
+			},
+			match: false,
+		},
+		{
+			platform: imagespec.Platform{
+				Architecture: "amd64",
+				OS:           "windows",
+				// Use an nonexistent Windows build so we don't get a match. Ws2019's build is 17763/
+				OSVersion: "10.0.17762.1",
+			},
+			match: false,
+		},
+		{
+			platform: imagespec.Platform{
+				Architecture: "amd64",
+				OS:           "linux",
+			},
+			match: true,
+		},
+	} {
+		assert.Equal(t, test.match, m.Match(test.platform), "should match %b, %s to %s", test.match, m.Platform, test.platform)
+	}
+}
+
+func TestMatchComparerLess(t *testing.T) {
+	m := windowsmatcher{
+		Platform:        DefaultSpec(),
 		osVersionPrefix: "10.0.17763",
+		defaultMatcher: &matcher{
+			Platform: Normalize(DefaultSpec()),
+		},
 	}
 	platforms := []imagespec.Platform{
 		{
diff --git a/platforms/platforms.go b/platforms/platforms.go
@@ -136,9 +136,7 @@ type Matcher interface {
 //
 // Applications should opt to use `Match` over directly parsing specifiers.
 func NewMatcher(platform specs.Platform) Matcher {
-	return &matcher{
-		Platform: Normalize(platform),
-	}
+	return newDefaultMatcher(platform)
 }
 
 type matcher struct {
@@ -257,5 +255,6 @@ func Format(platform specs.Platform) string {
 func Normalize(platform specs.Platform) specs.Platform {
 	platform.OS = normalizeOS(platform.OS)
 	platform.Architecture, platform.Variant = normalizeArch(platform.Architecture, platform.Variant)
+
 	return platform
 }
diff --git a/platforms/platforms_other.go b/platforms/platforms_other.go
@@ -0,0 +1,31 @@
+//go:build !windows
+// +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
+
+       https://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/platforms_windows.go b/platforms/platforms_windows.go
@@ -0,0 +1,34 @@
+/*
+   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
+
+       https://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),
+		},
+	}
+}
