From 38a74d209d3bd4091fa83db35061ce32da31b5c3 Mon Sep 17 00:00:00 2001 From: Kirtana Ashok Date: Wed, 31 Jan 2024 15:47:46 -0800 Subject: [PATCH] Add grammar for platform string Platform string to be of the form [()]||[()]/[/] OSVersion is optional only and currently used only by Windows OS. Signed-off-by: Kirtana Ashok --- defaults.go | 6 +- defaults_unix_test.go | 2 +- defaults_windows_test.go | 2 +- platforms.go | 72 +++++++++++++++--------- platforms_other.go | 4 -- platforms_test.go | 118 ++++++++++++++++++++++++++++++--------- platforms_windows.go | 8 --- 7 files changed, 142 insertions(+), 70 deletions(-) diff --git a/defaults.go b/defaults.go index cfa3ff3..9d898d6 100644 --- a/defaults.go +++ b/defaults.go @@ -16,9 +16,11 @@ package platforms -// DefaultString returns the default string specifier for the platform. +// DefaultString returns the default string specifier for the platform, +// with [PR#6](https://github.com/containerd/platforms/pull/6) the result +// may now also include the OSVersion from the provided platform specification. func DefaultString() string { - return Format(DefaultSpec()) + return FormatAll(DefaultSpec()) } // DefaultStrict returns strict form of Default. diff --git a/defaults_unix_test.go b/defaults_unix_test.go index 8aba958..a37570f 100644 --- a/defaults_unix_test.go +++ b/defaults_unix_test.go @@ -38,7 +38,7 @@ func TestDefault(t *testing.T) { } s := DefaultString() - if s != Format(p) { + if s != FormatAll(p) { t.Fatalf("default specifier should match formatted default spec: %v != %v", s, p) } } diff --git a/defaults_windows_test.go b/defaults_windows_test.go index ab73ddd..fae35bc 100644 --- a/defaults_windows_test.go +++ b/defaults_windows_test.go @@ -42,7 +42,7 @@ func TestDefault(t *testing.T) { } s := DefaultString() - if s != Format(p) { + if s != FormatAll(p) { t.Fatalf("default specifier should match formatted default spec: %v != %v", s, p) } } diff --git a/platforms.go b/platforms.go index 43e4ad3..1bbbdb9 100644 --- a/platforms.go +++ b/platforms.go @@ -121,9 +121,12 @@ import ( ) var ( - specifierRe = regexp.MustCompile(`^[A-Za-z0-9_-]+$`) + specifierRe = regexp.MustCompile(`^[A-Za-z0-9_-]+$`) + osAndVersionRe = regexp.MustCompile(`^([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 @@ -156,7 +159,7 @@ func (m *matcher) Match(platform specs.Platform) bool { } func (m *matcher) String() string { - return Format(m.Platform) + return FormatAll(m.Platform) } // ParseAll parses a list of platform specifiers into a list of platform. @@ -174,9 +177,12 @@ func ParseAll(specifiers []string) ([]specs.Platform, error) { // Parse parses the platform specifier syntax into a platform declaration. // -// Platform specifiers are in the format `||/[/]`. +// Platform specifiers are in the format `[()]||[()]/[/]`. // The minimum required information for a platform specifier is the operating -// system or architecture. If there is only a single string (no slashes), the +// 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. +// 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 // inferred based on the local environment. @@ -186,23 +192,35 @@ func Parse(specifier string) (specs.Platform, error) { return specs.Platform{}, fmt.Errorf("%q: wildcards not yet supported: %w", specifier, errInvalidArgument) } - parts := strings.Split(specifier, "/") + // Limit to 4 elements to prevent unbounded split + parts := strings.SplitN(specifier, "/", 4) - for _, part := range parts { - 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) + var p specs.Platform + for i, part := range parts { + if i == 0 { + // First element is [()] + 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) + } + + p.OS = normalizeOS(osVer[1]) + p.OSVersion = osVer[2] + } 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) + } } } - var p specs.Platform switch len(parts) { case 1: - // in this case, we will test that the value might be an OS, then look - // it up. If it is not known, we'll treat it as an architecture. Since + // in this case, we will test that the value might be an OS (with or + // without the optional OSVersion specified) and look it up. + // If it is not known, we'll treat it as an architecture. Since // we have very little information about the platform here, we are // going to be a little more strict if we don't know about the argument // value. - p.OS = normalizeOS(parts[0]) if isKnownOS(p.OS) { // picks a default architecture p.Architecture = runtime.GOARCH @@ -210,10 +228,6 @@ func Parse(specifier string) (specs.Platform, error) { p.Variant = cpuVariant() } - if p.OS == "windows" { - p.OSVersion = GetWindowsOsVersion() - } - return p, nil } @@ -228,31 +242,21 @@ func Parse(specifier string) (specs.Platform, error) { return specs.Platform{}, fmt.Errorf("%q: unknown operating system or architecture: %w", specifier, errInvalidArgument) case 2: - // In this case, we treat as a regular os/arch pair. We don't care + // In this case, we treat as a regular OS[(OSVersion)]/arch pair. We don't care // about whether or not we know of the platform. - p.OS = normalizeOS(parts[0]) p.Architecture, p.Variant = normalizeArch(parts[1], "") if p.Architecture == "arm" && p.Variant == "v7" { p.Variant = "" } - if p.OS == "windows" { - p.OSVersion = GetWindowsOsVersion() - } - return p, nil case 3: // we have a fully specified variant, this is rare - p.OS = normalizeOS(parts[0]) p.Architecture, p.Variant = normalizeArch(parts[1], parts[2]) if p.Architecture == "arm64" && p.Variant == "" { p.Variant = "v8" } - if p.OS == "windows" { - p.OSVersion = GetWindowsOsVersion() - } - return p, nil } @@ -278,6 +282,20 @@ func Format(platform specs.Platform) string { return path.Join(platform.OS, platform.Architecture, platform.Variant) } +// FormatAll returns a string specifier that also includes the OSVersion from the +// provided platform specification. +func FormatAll(platform specs.Platform) string { + if platform.OS == "" { + return "unknown" + } + + if platform.OSVersion != "" { + OSAndVersion := fmt.Sprintf(osAndVersionFormat, platform.OS, platform.OSVersion) + return path.Join(OSAndVersion, platform.Architecture, platform.Variant) + } + return path.Join(platform.OS, platform.Architecture, platform.Variant) +} + // Normalize validates and translate the platform to the canonical value. // // For example, if "Aarch64" is encountered, we change it to "arm64" or if diff --git a/platforms_other.go b/platforms_other.go index 59beeb3..03f4dcd 100644 --- a/platforms_other.go +++ b/platforms_other.go @@ -28,7 +28,3 @@ func newDefaultMatcher(platform specs.Platform) Matcher { Platform: Normalize(platform), } } - -func GetWindowsOsVersion() string { - return "" -} diff --git a/platforms_test.go b/platforms_test.go index c2af021..8a26f5c 100644 --- a/platforms_test.go +++ b/platforms_test.go @@ -37,11 +37,12 @@ func TestParseSelector(t *testing.T) { } for _, testcase := range []struct { - skip bool - input string - expected specs.Platform - matches []specs.Platform - formatted string + skip bool + input string + expected specs.Platform + matches []specs.Platform + formatted string + useV2Format bool }{ // While wildcards are a valid use case for platform selection, // addressing these cases is outside the initial scope for this @@ -54,7 +55,8 @@ func TestParseSelector(t *testing.T) { OS: "*", Architecture: "*", }, - formatted: "*/*", + formatted: "*/*", + useV2Format: false, }, { skip: true, @@ -63,7 +65,8 @@ func TestParseSelector(t *testing.T) { OS: "linux", Architecture: "*", }, - formatted: "linux/*", + formatted: "linux/*", + useV2Format: false, }, { skip: true, @@ -88,7 +91,8 @@ func TestParseSelector(t *testing.T) { Variant: "v8", }, }, - formatted: "*/arm64", + formatted: "*/arm64", + useV2Format: false, }, { input: "linux/arm64", @@ -112,7 +116,8 @@ func TestParseSelector(t *testing.T) { Variant: "v8", }, }, - formatted: "linux/arm64", + formatted: "linux/arm64", + useV2Format: false, }, { input: "linux/arm64/v8", @@ -136,7 +141,8 @@ func TestParseSelector(t *testing.T) { Architecture: "arm64", }, }, - formatted: "linux/arm64/v8", + formatted: "linux/arm64/v8", + useV2Format: false, }, { // NOTE(stevvooe): In this case, the consumer can assume this is v7 @@ -163,7 +169,8 @@ func TestParseSelector(t *testing.T) { Variant: "7", }, }, - formatted: "linux/arm", + formatted: "linux/arm", + useV2Format: false, }, { input: "linux/arm/v6", @@ -178,7 +185,8 @@ func TestParseSelector(t *testing.T) { Architecture: "armel", }, }, - formatted: "linux/arm/v6", + formatted: "linux/arm/v6", + useV2Format: false, }, { input: "linux/arm/v7", @@ -197,7 +205,8 @@ func TestParseSelector(t *testing.T) { Architecture: "armhf", }, }, - formatted: "linux/arm/v7", + formatted: "linux/arm/v7", + useV2Format: false, }, { input: "arm", @@ -205,7 +214,8 @@ func TestParseSelector(t *testing.T) { OS: defaultOS, Architecture: "arm", }, - formatted: path.Join(defaultOS, "arm"), + formatted: path.Join(defaultOS, "arm"), + useV2Format: false, }, { input: "armel", @@ -214,7 +224,8 @@ func TestParseSelector(t *testing.T) { Architecture: "arm", Variant: "v6", }, - formatted: path.Join(defaultOS, "arm/v6"), + formatted: path.Join(defaultOS, "arm/v6"), + useV2Format: false, }, { input: "armhf", @@ -222,7 +233,8 @@ func TestParseSelector(t *testing.T) { OS: defaultOS, Architecture: "arm", }, - formatted: path.Join(defaultOS, "arm"), + formatted: path.Join(defaultOS, "arm"), + useV2Format: false, }, { input: "Aarch64", @@ -230,7 +242,8 @@ func TestParseSelector(t *testing.T) { OS: defaultOS, Architecture: "arm64", }, - formatted: path.Join(defaultOS, "arm64"), + formatted: path.Join(defaultOS, "arm64"), + useV2Format: false, }, { input: "x86_64", @@ -238,7 +251,8 @@ func TestParseSelector(t *testing.T) { OS: defaultOS, Architecture: "amd64", }, - formatted: path.Join(defaultOS, "amd64"), + formatted: path.Join(defaultOS, "amd64"), + useV2Format: false, }, { input: "Linux/x86_64", @@ -246,7 +260,8 @@ func TestParseSelector(t *testing.T) { OS: "linux", Architecture: "amd64", }, - formatted: "linux/amd64", + formatted: "linux/amd64", + useV2Format: false, }, { input: "i386", @@ -254,7 +269,8 @@ func TestParseSelector(t *testing.T) { OS: defaultOS, Architecture: "386", }, - formatted: path.Join(defaultOS, "386"), + formatted: path.Join(defaultOS, "386"), + useV2Format: false, }, { input: "linux", @@ -263,7 +279,8 @@ func TestParseSelector(t *testing.T) { Architecture: defaultArch, Variant: defaultVariant, }, - formatted: path.Join("linux", defaultArch, defaultVariant), + formatted: path.Join("linux", defaultArch, defaultVariant), + useV2Format: false, }, { input: "s390x", @@ -271,7 +288,8 @@ func TestParseSelector(t *testing.T) { OS: defaultOS, Architecture: "s390x", }, - formatted: path.Join(defaultOS, "s390x"), + formatted: path.Join(defaultOS, "s390x"), + useV2Format: false, }, { input: "linux/s390x", @@ -279,7 +297,8 @@ func TestParseSelector(t *testing.T) { OS: "linux", Architecture: "s390x", }, - formatted: "linux/s390x", + formatted: "linux/s390x", + useV2Format: false, }, { input: "macOS", @@ -288,7 +307,41 @@ func TestParseSelector(t *testing.T) { Architecture: defaultArch, Variant: defaultVariant, }, - formatted: path.Join("darwin", defaultArch, defaultVariant), + formatted: path.Join("darwin", defaultArch, defaultVariant), + useV2Format: false, + }, + { + input: "windows", + expected: specs.Platform{ + OS: "windows", + OSVersion: "", + Architecture: defaultArch, + Variant: defaultVariant, + }, + formatted: path.Join("windows", defaultArch, defaultVariant), + useV2Format: false, + }, + { + input: "windows()", + expected: specs.Platform{ + OS: "windows", + OSVersion: "", + Architecture: defaultArch, + Variant: defaultVariant, + }, + formatted: path.Join("windows", defaultArch, defaultVariant), + useV2Format: true, + }, + { + input: "windows(10.0.17763)", + expected: specs.Platform{ + OS: "windows", + OSVersion: "10.0.17763", + Architecture: defaultArch, + Variant: defaultVariant, + }, + formatted: path.Join("windows(10.0.17763)", defaultArch, defaultVariant), + useV2Format: true, }, } { t.Run(testcase.input, func(t *testing.T) { @@ -316,7 +369,12 @@ func TestParseSelector(t *testing.T) { } } - formatted := Format(p) + formatted := "" + if testcase.useV2Format == false { + formatted = Format(p) + } else { + formatted = FormatAll(p) + } if formatted != testcase.formatted { t.Fatalf("unexpected format: %q != %q", formatted, testcase.formatted) } @@ -327,8 +385,14 @@ func TestParseSelector(t *testing.T) { t.Fatalf("error parsing formatted output: %v", err) } - if Format(reparsed) != formatted { - t.Fatalf("normalized output did not survive the round trip: %v != %v", Format(reparsed), formatted) + if testcase.useV2Format == false { + if Format(reparsed) != formatted { + t.Fatalf("normalized output did not survive the round trip: %v != %v", Format(reparsed), formatted) + } + } else { + if FormatAll(reparsed) != formatted { + t.Fatalf("normalized output did not survive the round trip: %v != %v", FormatAll(reparsed), formatted) + } } }) } diff --git a/platforms_windows.go b/platforms_windows.go index 733d18d..950e2a2 100644 --- a/platforms_windows.go +++ b/platforms_windows.go @@ -17,10 +17,7 @@ package platforms import ( - "fmt" - specs "github.com/opencontainers/image-spec/specs-go/v1" - "golang.org/x/sys/windows" ) // NewMatcher returns a Windows matcher that will match on osVersionPrefix if @@ -35,8 +32,3 @@ func newDefaultMatcher(platform specs.Platform) Matcher { }, } } - -func GetWindowsOsVersion() string { - major, minor, build := windows.RtlGetNtVersionNumbers() - return fmt.Sprintf("%d.%d.%d", major, minor, build) -}