Match and Compare platforms with OSFeatures

compare.go

@@ -213,9 +213,20 @@ func (c orderedPlatformComparer) Less(p1 specs.Platform, p2 specs.Platform) bool
 			return true
 		}
 		if p1m || p2m {
+			if p1m && p2m {
+				// Prefer one with most matching features
+				if len(p1.OSFeatures) != len(p2.OSFeatures) {
+					return len(p1.OSFeatures) > len(p2.OSFeatures)
+				}
+			}
 			return false
 		}
 	}
+	if len(p1.OSFeatures) > 0 || len(p2.OSFeatures) > 0 {
+		p1.OSFeatures = nil
+		p2.OSFeatures = nil
+		return c.Less(p1, p2)
+	}
 	return false
 }
 
@@ -242,9 +253,20 @@ func (c anyPlatformComparer) Less(p1, p2 specs.Platform) bool {
 			p2m = true
 		}
 		if p1m && p2m {
-			return false
+			if len(p1.OSFeatures) != len(p2.OSFeatures) {
+				return len(p1.OSFeatures) > len(p2.OSFeatures)
+			}
+			break
 		}
 	}
+
+	// If neither match and has features, strip features and compare
+	if !p1m && !p2m && (len(p1.OSFeatures) > 0 || len(p2.OSFeatures) > 0) {
+		p1.OSFeatures = nil
+		p2.OSFeatures = nil
+		return c.Less(p1, p2)
+	}
+
 	// If one matches, and the other does, sort match first
 	return p1m && !p2m
 }

compare_test.go

@@ -17,6 +17,8 @@
 package platforms
 
 import (
+	"reflect"
+	"sort"
 	"testing"
 )
 
@@ -592,3 +594,94 @@ func TestOnlyStrict(t *testing.T) {
 		})
 	}
 }
+
+func TestCompareOSFeatures(t *testing.T) {
+	for _, tc := range []struct {
+		platform  string
+		platforms []string
+		expected  []string
+	}{
+		{
+			"linux/amd64",
+			[]string{"windows/amd64", "linux/amd64", "linux(+other)/amd64", "linux/arm64"},
+			[]string{"linux/amd64", "linux(+other)/amd64", "windows/amd64", "linux/arm64"},
+		},
+		{
+			"linux(+none)/amd64",
+			[]string{"windows/amd64", "linux/amd64", "linux/arm64", "linux(+other)/amd64"},
+			[]string{"linux/amd64", "linux(+other)/amd64", "windows/amd64", "linux/arm64"},
+		},
+		{
+			"linux(+other)/amd64",
+			[]string{"windows/amd64", "linux/amd64", "linux/arm64", "linux(+other)/amd64"},
+			[]string{"linux(+other)/amd64", "linux/amd64", "windows/amd64", "linux/arm64"},
+		},
+		{
+			"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"},
+		},
+		{
+			"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"},
+		},
+		{
+			// 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"},
+		},
+	} {
+		testcase := tc
+		t.Run(testcase.platform, func(t *testing.T) {
+			t.Parallel()
+			p, err := Parse(testcase.platform)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			for _, stc := range []struct {
+				name string
+				mc   MatchComparer
+			}{
+				{
+					name: "only",
+					mc:   Only(p),
+				},
+				{
+					name: "only strict",
+					mc:   OnlyStrict(p),
+				},
+				{
+					name: "ordered",
+					mc:   Ordered(p),
+				},
+				{
+					name: "any",
+					mc:   Any(p),
+				},
+			} {
+				mc := stc.mc
+				testcase := testcase
+				t.Run(stc.name, func(t *testing.T) {
+					p, err := ParseAll(testcase.platforms)
+					if err != nil {
+						t.Fatal(err)
+					}
+					sort.Slice(p, func(i, j int) bool {
+						return mc.Less(p[i], p[j])
+					})
+					actual := make([]string, len(p))
+					for i, ps := range p {
+						actual[i] = FormatAll(ps)
+					}
+
+					if !reflect.DeepEqual(testcase.expected, actual) {
+						t.Errorf("Wrong platform order:\nExpected: %#v\nActual:   %#v", testcase.expected, actual)
+					}
+				})
+			}
+		})
+	}
+}

go.mod

@@ -1,6 +1,6 @@
 module github.com/containerd/platforms
 
-go 1.20
+go 1.21
 
 require (
 	github.com/containerd/log v0.1.0

platforms.go

@@ -114,19 +114,18 @@ import (
 	"path"
 	"regexp"
 	"runtime"
+	"slices"
 	"strconv"
 	"strings"
 
 	specs "github.com/opencontainers/image-spec/specs-go/v1"
 )
 
 var (
-	specifierRe    = regexp.MustCompile(`^[A-Za-z0-9_.-]+$`)
-	osAndVersionRe = regexp.MustCompile(`^([A-Za-z0-9_-]+)(?:\(([A-Za-z0-9_.-]*)\))?$`)
+	specifierRe = regexp.MustCompile(`^[A-Za-z0-9_.-]+$`)
+	osRe        = regexp.MustCompile(`^([A-Za-z0-9_-]+)(?:\(([A-Za-z0-9_.-]*)((?:\+[A-Za-z0-9_.-]+)*)\))?$`)
 )
 
-const osAndVersionFormat = "%s(%s)"
-
 // Platform is a type alias for convenience, so there is no need to import image-spec package everywhere.
 type Platform = specs.Platform
 
@@ -143,6 +142,10 @@ type Matcher interface {
 // functionality.
 //
 // Applications should opt to use `Match` over directly parsing specifiers.
+//
+// For OSFeatures, this matcher will match if the platform to match has
+// OSFeatures which are a subset of the OSFeatures of the platform
+// provided to NewMatcher.
 func NewMatcher(platform specs.Platform) Matcher {
 	m := &matcher{
 		Platform: Normalize(platform),
@@ -178,10 +181,39 @@ type matcher struct {
 
 func (m *matcher) Match(platform specs.Platform) bool {
 	normalized := Normalize(platform)
-	return m.OS == normalized.OS &&
+	if m.OS == normalized.OS &&
 		m.Architecture == normalized.Architecture &&
 		m.Variant == normalized.Variant &&
-		m.matchOSVersion(platform)
+		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
+				}
+			}
+			return true
+		}
+	}
+	return false
 }
 
 func (m *matcher) matchOSVersion(platform specs.Platform) bool {
@@ -210,11 +242,14 @@ func ParseAll(specifiers []string) ([]specs.Platform, error) {
 
 // Parse parses the platform specifier syntax into a platform declaration.
 //
-// Platform specifiers are in the format `<os>[(<OSVersion>)]|<arch>|<os>[(<OSVersion>)]/<arch>[/<variant>]`.
+// Platform specifiers are in the format `<os>[(<os options>)]|<arch>|<os>[(<os options>)]/<arch>[/<variant>]`.
 // The minimum required information for a platform specifier is the operating
-// system or architecture. The OSVersion can be part of the OS like `windows(10.0.17763)`
-// When an OSVersion is specified, then specs.Platform.OSVersion is populated with that value,
-// and an empty string otherwise.
+// system or architecture. The "os options" may be OSVersion which can be part of the OS
+// like `windows(10.0.17763)`. When an OSVersion is specified, then specs.Platform.OSVersion is
+// populated with that value, and an empty string otherwise. The "os options" may also include an
+// array of OSFeatures, each feature prefixed with '+', without any other separator, and provided
+// after the OSVersion when the OSVersion is specified. An "os options" with version and features
+// is like `windows(10.0.17763+win32k)`.
 // If there is only a single string (no slashes), the
 // value will be matched against the known set of operating systems, then fall
 // back to the known set of architectures. The missing component will be
@@ -231,14 +266,17 @@ func Parse(specifier string) (specs.Platform, error) {
 	var p specs.Platform
 	for i, part := range parts {
 		if i == 0 {
-			// First element is <os>[(<OSVersion>)]
-			osVer := osAndVersionRe.FindStringSubmatch(part)
-			if osVer == nil {
-				return specs.Platform{}, fmt.Errorf("%q is an invalid OS component of %q: OSAndVersion specifier component must match %q: %w", part, specifier, osAndVersionRe.String(), errInvalidArgument)
+			// First element is <os>[(<OSVersion>[+<OSFeature>]*)]
+			osOptions := osRe.FindStringSubmatch(part)
+			if osOptions == nil {
+				return specs.Platform{}, fmt.Errorf("%q is an invalid OS component of %q: OSAndVersion specifier component must match %q: %w", part, specifier, osRe.String(), errInvalidArgument)
 			}
 
-			p.OS = normalizeOS(osVer[1])
-			p.OSVersion = osVer[2]
+			p.OS = normalizeOS(osOptions[1])
+			p.OSVersion = osOptions[2]
+			if osOptions[3] != "" {
+				p.OSFeatures = strings.Split(osOptions[3][1:], "+")
+			}
 		} else {
 			if !specifierRe.MatchString(part) {
 				return specs.Platform{}, fmt.Errorf("%q is an invalid component of %q: platform specifier component must match %q: %w", part, specifier, specifierRe.String(), errInvalidArgument)
@@ -322,8 +360,17 @@ func FormatAll(platform specs.Platform) string {
 		return "unknown"
 	}
 
-	if platform.OSVersion != "" {
-		OSAndVersion := fmt.Sprintf(osAndVersionFormat, platform.OS, platform.OSVersion)
+	osOptions := platform.OSVersion
+	features := platform.OSFeatures
+	if !slices.IsSorted(features) {
+		features = slices.Clone(features)
+		slices.Sort(features)
+	}
+	if len(features) > 0 {
+		osOptions += "+" + strings.Join(features, "+")
+	}
+	if osOptions != "" {
+		OSAndVersion := fmt.Sprintf("%s(%s)", platform.OS, osOptions)
 		return path.Join(OSAndVersion, platform.Architecture, platform.Variant)
 	}
 	return path.Join(platform.OS, platform.Architecture, platform.Variant)
@@ -336,6 +383,11 @@ func FormatAll(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)
+	if len(platform.OSFeatures) > 0 {
+		platform.OSFeatures = slices.Clone(platform.OSFeatures)
+		slices.Sort(platform.OSFeatures)
+		platform.OSFeatures = slices.Compact(platform.OSFeatures)
+	}
 
 	return platform
 }

platforms_test.go

@@ -343,6 +343,54 @@ func TestParseSelector(t *testing.T) {
 			formatted:   path.Join("windows(10.0.17763)", defaultArch, defaultVariant),
 			useV2Format: true,
 		},
+		{
+			input: "windows(10.0.17763+win32k)",
+			expected: specs.Platform{
+				OS:           "windows",
+				OSVersion:    "10.0.17763",
+				OSFeatures:   []string{"win32k"},
+				Architecture: defaultArch,
+				Variant:      defaultVariant,
+			},
+			formatted:   path.Join("windows(10.0.17763+win32k)", defaultArch, defaultVariant),
+			useV2Format: true,
+		},
+		{
+			input: "linux(+gpu)",
+			expected: specs.Platform{
+				OS:           "linux",
+				OSVersion:    "",
+				OSFeatures:   []string{"gpu"},
+				Architecture: defaultArch,
+				Variant:      defaultVariant,
+			},
+			formatted:   path.Join("linux(+gpu)", defaultArch, defaultVariant),
+			useV2Format: true,
+		},
+		{
+			input: "linux(+gpu+simd)",
+			expected: specs.Platform{
+				OS:           "linux",
+				OSVersion:    "",
+				OSFeatures:   []string{"gpu", "simd"},
+				Architecture: defaultArch,
+				Variant:      defaultVariant,
+			},
+			formatted:   path.Join("linux(+gpu+simd)", defaultArch, defaultVariant),
+			useV2Format: true,
+		},
+		{
+			input: "linux(+unsorted+erofs)",
+			expected: specs.Platform{
+				OS:           "linux",
+				OSVersion:    "",
+				OSFeatures:   []string{"unsorted", "erofs"},
+				Architecture: defaultArch,
+				Variant:      defaultVariant,
+			},
+			formatted:   path.Join("linux(+erofs+unsorted)", defaultArch, defaultVariant),
+			useV2Format: true,
+		},
 	} {
 		t.Run(testcase.input, func(t *testing.T) {
 			if testcase.skip {