From fb0688362cafb5c24cfe6bd4f83765a0515af374 Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Fri, 18 Aug 2017 17:45:40 -0700 Subject: [PATCH 1/3] platforms: define selectors for platforms For supporting selection of images and runtimes in the containerized world, there is thin support for selecting objects by platform. This package defines a syntax to display to users that can express specific platforms in addition to wild cards for matching platforms. The plan is to extend this to provide support for parsing flag arguments and displaying platform types for images. This package will also provide a configurable matcher to allow match of platforms against arbitrary targets, invariant to the Go compilation. The internals are based the OCI Image Spec model. This changeset is being submitted for feedback on the approach before this gets larger. Specifically, review the unit tests and raise any concerns. Signed-off-by: Stephen J Day --- platforms/database.go | 77 ++++++++++++ platforms/defaults.go | 16 +++ platforms/defaults_test.go | 20 ++++ platforms/platforms.go | 129 ++++++++++++++++++++ platforms/platforms_test.go | 233 ++++++++++++++++++++++++++++++++++++ 5 files changed, 475 insertions(+) create mode 100644 platforms/database.go create mode 100644 platforms/defaults.go create mode 100644 platforms/defaults_test.go create mode 100644 platforms/platforms.go create mode 100644 platforms/platforms_test.go diff --git a/platforms/database.go b/platforms/database.go new file mode 100644 index 000000000..bd66e2517 --- /dev/null +++ b/platforms/database.go @@ -0,0 +1,77 @@ +package platforms + +import ( + "runtime" + "strings" +) + +// These function are generated from from https://golang.org/src/go/build/syslist.go. +// +// We use switch statements because they are slightly faster than map lookups +// and use a little less memory. + +// isKnownOS returns true if we know about the operating system. +// +// The OS value should be normalized before calling this function. +func isKnownOS(os string) bool { + switch os { + case "android", "darwin", "dragonfly", "freebsd", "linux", "nacl", "netbsd", "openbsd", "plan9", "solaris", "windows", "zos": + return true + } + return false +} + +// isKnownArch returns true if we know about the architecture. +// +// The arch value should be normalized before being passed to this function. +func isKnownArch(arch string) bool { + switch arch { + case "386", "amd64", "amd64p32", "arm", "armbe", "arm64", "arm64be", "ppc64", "ppc64le", "mips", "mipsle", "mips64", "mips64le", "mips64p32", "mips64p32le", "ppc", "s390", "s390x", "sparc", "sparc64": + return true + } + return false +} + +func normalizeOS(os string) string { + if os == "" { + return runtime.GOOS + } + os = strings.ToLower(os) + + switch os { + case "macos": + os = "darwin" + } + return os +} + +// normalizeArch normalizes the architecture. +func normalizeArch(arch, variant string) (string, string) { + arch, variant = strings.ToLower(arch), strings.ToLower(variant) + switch arch { + case "i386": + arch = "386" + variant = "" + case "x86_64", "x86-64": + arch = "amd64" + variant = "" + case "aarch64": + arch = "arm64" + variant = "" // v8 is implied + case "armhf": + arch = "arm" + variant = "" + case "armel": + arch = "arm" + variant = "v6" + case "arm": + switch variant { + case "v7", "7": + variant = "v7" + case "5", "6", "8": + variant = "v" + variant + } + } + + return arch, variant +} diff --git a/platforms/defaults.go b/platforms/defaults.go new file mode 100644 index 000000000..d49912052 --- /dev/null +++ b/platforms/defaults.go @@ -0,0 +1,16 @@ +package platforms + +import ( + "runtime" + + specs "github.com/opencontainers/image-spec/specs-go/v1" +) + +// Default returns the current platform's default platform specification. +func Default() specs.Platform { + return specs.Platform{ + OS: runtime.GOOS, + Architecture: runtime.GOARCH, + // TODO(stevvooe): Need to resolve GOARM for arm hosts. + } +} diff --git a/platforms/defaults_test.go b/platforms/defaults_test.go new file mode 100644 index 000000000..08c40be63 --- /dev/null +++ b/platforms/defaults_test.go @@ -0,0 +1,20 @@ +package platforms + +import ( + "reflect" + "runtime" + "testing" + + specs "github.com/opencontainers/image-spec/specs-go/v1" +) + +func TestDefault(t *testing.T) { + expected := specs.Platform{ + OS: runtime.GOOS, + Architecture: runtime.GOARCH, + } + p := Default() + if !reflect.DeepEqual(p, expected) { + t.Fatalf("default platform not as expected: %#v != %#v", p, expected) + } +} diff --git a/platforms/platforms.go b/platforms/platforms.go new file mode 100644 index 000000000..96ecf38ec --- /dev/null +++ b/platforms/platforms.go @@ -0,0 +1,129 @@ +package platforms + +import ( + "regexp" + "runtime" + "strings" + + "github.com/containerd/containerd/errdefs" + specs "github.com/opencontainers/image-spec/specs-go/v1" + "github.com/pkg/errors" +) + +type platformKey struct { + os string + arch string + variant string +} + +var ( + selectorRe = regexp.MustCompile(`^[A-Za-z0-9_-]+$`) +) + +// ParseSelector parses the platform selector syntax into a platform +// declaration. +// +// Platform selectors are in the format [/[/]]. The +// minimum required information for a platform selector is the operating system +// or architecture. 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. +func Parse(selector string) (specs.Platform, error) { + if strings.Contains(selector, "*") { + // TODO(stevvooe): need to work out exact wildcard handling + return specs.Platform{}, errors.Wrapf(errdefs.ErrInvalidArgument, "%q: wildcards not yet supported", selector) + } + + parts := strings.Split(selector, "/") + + for _, part := range parts { + if !selectorRe.MatchString(part) { + return specs.Platform{}, errors.Wrapf(errdefs.ErrInvalidArgument, "%q is an invalid component of %q: platform selector component must match %q", part, selector, selectorRe.String()) + } + } + + 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 + // 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 + if p.Architecture == "arm" { + // TODO(stevvooe): Resolve arm variant, if not v6 (default) + } + + return p, nil + } + + p.Architecture, p.Variant = normalizeArch(parts[0], "") + if isKnownArch(p.Architecture) { + p.OS = runtime.GOOS + return p, nil + } + + return specs.Platform{}, errors.Wrapf(errdefs.ErrInvalidArgument, "%q: unknown operating system or architecture", selector) + case 2: + // In this case, we treat as a regular os/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], "") + + 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]) + + return p, nil + } + + return specs.Platform{}, errors.Wrapf(errdefs.ErrInvalidArgument, "%q: cannot parse platform selector", selector) +} + +func Match(selector string, platform specs.Platform) bool { + return false +} + +// Format returns a string that provides a shortened overview of the platform. +func Format(platform specs.Platform) string { + if platform.OS == "" { + return "unknown" + } + + return joinNotEmpty(platform.OS, platform.Architecture, platform.Variant) +} + +func joinNotEmpty(s ...string) string { + var ss []string + for _, s := range s { + if s == "" { + continue + } + + ss = append(ss, s) + } + + return strings.Join(ss, "/") +} + +// Normalize validates and translate the platform to the canonical value. +// +// For example, if "Aarch64" is encountered, we change it to "arm64" or if +// "x86_64" is encountered, it becomes "amd64". +func Normalize(platform specs.Platform) specs.Platform { + platform.OS = normalizeOS(platform.OS) + platform.Architecture, platform.Variant = normalizeArch(platform.Architecture, platform.Variant) + + // these fields are deprecated, remove them + platform.OSFeatures = nil + platform.OSVersion = "" + + return platform +} diff --git a/platforms/platforms_test.go b/platforms/platforms_test.go new file mode 100644 index 000000000..a34fa4753 --- /dev/null +++ b/platforms/platforms_test.go @@ -0,0 +1,233 @@ +package platforms + +import ( + "reflect" + "runtime" + "testing" + + specs "github.com/opencontainers/image-spec/specs-go/v1" +) + +func TestParseSelector(t *testing.T) { + var ( + defaultOS = runtime.GOOS + defaultArch = runtime.GOARCH + ) + + for _, testcase := range []struct { + skip bool + input string + expected specs.Platform + formatted string + }{ + // While wildcards are a valid use case for platform selection, + // addressing these cases is outside the initial scope for this + // package. When we do add platform wildcards, we should add in these + // testcases to ensure that they are correctly represented. + { + skip: true, + input: "*", + expected: specs.Platform{ + OS: "*", + Architecture: "*", + }, + formatted: "*/*", + }, + { + skip: true, + input: "linux/*", + expected: specs.Platform{ + OS: "linux", + Architecture: "*", + }, + formatted: "linux/*", + }, + { + skip: true, + input: "*/arm64", + expected: specs.Platform{ + OS: "*", + Architecture: "arm64", + }, + formatted: "*/arm64", + }, + { + // NOTE(stevvooe): In this case, the consumer can assume this is v7 + // but we leave the variant blank. This will represent the vast + // majority of arm images. + input: "linux/arm", + expected: specs.Platform{ + OS: "linux", + Architecture: "arm", + }, + formatted: "linux/arm", + }, + { + input: "linux/arm/v6", + expected: specs.Platform{ + OS: "linux", + Architecture: "arm", + Variant: "v6", + }, + formatted: "linux/arm/v6", + }, + { + input: "linux/arm/v7", + expected: specs.Platform{ + OS: "linux", + Architecture: "arm", + Variant: "v7", + }, + formatted: "linux/arm/v7", + }, + { + input: "arm", + expected: specs.Platform{ + OS: defaultOS, + Architecture: "arm", + }, + formatted: "linux/arm", + }, + { + input: "armel", + expected: specs.Platform{ + OS: defaultOS, + Architecture: "arm", + Variant: "v6", + }, + formatted: "linux/arm/v6", + }, + { + input: "armhf", + expected: specs.Platform{ + OS: defaultOS, + Architecture: "arm", + }, + formatted: "linux/arm", + }, + { + input: "Aarch64", + expected: specs.Platform{ + OS: defaultOS, + Architecture: "arm64", + }, + formatted: joinNotEmpty(defaultOS, "arm64"), + }, + { + input: "x86_64", + expected: specs.Platform{ + OS: defaultOS, + Architecture: "amd64", + }, + formatted: joinNotEmpty(defaultOS, "amd64"), + }, + { + input: "Linux/x86_64", + expected: specs.Platform{ + OS: "linux", + Architecture: "amd64", + }, + formatted: "linux/amd64", + }, + { + input: "i386", + expected: specs.Platform{ + OS: defaultOS, + Architecture: "386", + }, + formatted: joinNotEmpty(defaultOS, "386"), + }, + { + input: "linux", + expected: specs.Platform{ + OS: "linux", + Architecture: defaultArch, + }, + formatted: joinNotEmpty("linux", defaultArch), + }, + { + input: "s390x", + expected: specs.Platform{ + OS: defaultOS, + Architecture: "s390x", + }, + formatted: joinNotEmpty(defaultOS, "s390x"), + }, + { + input: "linux/s390x", + expected: specs.Platform{ + OS: "linux", + Architecture: "s390x", + }, + formatted: "linux/s390x", + }, + { + input: "macOS", + expected: specs.Platform{ + OS: "darwin", + Architecture: defaultArch, + }, + formatted: joinNotEmpty("darwin", defaultArch), + }, + } { + t.Run(testcase.input, func(t *testing.T) { + if testcase.skip { + t.Skip("this case is not yet supported") + } + p, err := Parse(testcase.input) + if err != nil { + t.Fatal(err) + } + + if !reflect.DeepEqual(p, testcase.expected) { + t.Fatalf("platform did not match expected: %#v != %#v", p, testcase.expected) + } + + formatted := Format(p) + if formatted != testcase.formatted { + t.Fatalf("unexpected format: %q != %q", formatted, testcase.formatted) + } + + // re-parse the formatted output and ensure we are stable + reparsed, err := Parse(formatted) + if err != nil { + 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) + } + }) + } +} + +func TestParseSelectorInvalid(t *testing.T) { + for _, testcase := range []struct { + input string + }{ + { + input: "", // empty + }, + { + input: "/linux/arm", // leading slash + }, + { + input: "linux/arm/", // trailing slash + }, + { + input: "linux /arm", // spaces + }, + { + input: "linux/&arm", // invalid character + }, + { + input: "linux/arm/foo/bar", // too mayn components + }, + } { + t.Run(testcase.input, func(t *testing.T) { + if _, err := Parse(testcase.input); err == nil { + t.Fatalf("should have received an error") + } + }) + } +} From 94f6be5f1006849e5519d16c6d1a3c18c26df9c9 Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Fri, 8 Sep 2017 17:55:07 -0700 Subject: [PATCH 2/3] platforms: implement matcher support Matching support is now implemented in the platforms package. The `Parse` function now returns a matcher object that can be used to match OCI platform specifications. We define this as an interface to allow the creation of helpers oriented around platform selection. Signed-off-by: Stephen J Day --- differ/differ.go | 4 +- errdefs/errors.go | 6 +- errdefs/grpc.go | 4 +- platforms/platforms.go | 162 +++++++++++++++++++++++++++++------- platforms/platforms_test.go | 24 ++++-- services/diff/service.go | 4 +- 6 files changed, 160 insertions(+), 44 deletions(-) diff --git a/differ/differ.go b/differ/differ.go index 60e5d3eea..38d6aec85 100644 --- a/differ/differ.go +++ b/differ/differ.go @@ -66,7 +66,7 @@ func (s *walkingDiff) Apply(ctx context.Context, desc ocispec.Descriptor, mounts if strings.HasSuffix(desc.MediaType, ".tar.gzip") || strings.HasSuffix(desc.MediaType, ".tar+gzip") { isCompressed = true } else if !strings.HasSuffix(desc.MediaType, ".tar") { - return emptyDesc, errors.Wrapf(errdefs.ErrNotSupported, "unsupported diff media type: %v", desc.MediaType) + return emptyDesc, errors.Wrapf(errdefs.ErrNotImplemented, "unsupported diff media type: %v", desc.MediaType) } } @@ -128,7 +128,7 @@ func (s *walkingDiff) DiffMounts(ctx context.Context, lower, upper []mount.Mount media = ocispec.MediaTypeImageLayerGzip isCompressed = true default: - return emptyDesc, errors.Wrapf(errdefs.ErrNotSupported, "unsupported diff media type: %v", media) + return emptyDesc, errors.Wrapf(errdefs.ErrNotImplemented, "unsupported diff media type: %v", media) } aDir, err := ioutil.TempDir("", "left-") if err != nil { diff --git a/errdefs/errors.go b/errdefs/errors.go index c8f0a6c46..44ec72e7b 100644 --- a/errdefs/errors.go +++ b/errdefs/errors.go @@ -26,7 +26,7 @@ var ( ErrAlreadyExists = errors.New("already exists") ErrFailedPrecondition = errors.New("failed precondition") ErrUnavailable = errors.New("unavailable") - ErrNotSupported = errors.New("not supported") // represents not supported and unimplemented + ErrNotImplemented = errors.New("not implemented") // represents not supported and unimplemented ) func IsInvalidArgument(err error) bool { @@ -54,6 +54,6 @@ func IsUnavailable(err error) bool { return errors.Cause(err) == ErrUnavailable } -func IsNotSupported(err error) bool { - return errors.Cause(err) == ErrNotSupported +func IsNotImplemented(err error) bool { + return errors.Cause(err) == ErrNotImplemented } diff --git a/errdefs/grpc.go b/errdefs/grpc.go index 53e7ee2d4..76fb3eb3f 100644 --- a/errdefs/grpc.go +++ b/errdefs/grpc.go @@ -38,7 +38,7 @@ func ToGRPC(err error) error { return grpc.Errorf(codes.FailedPrecondition, err.Error()) case IsUnavailable(err): return grpc.Errorf(codes.Unavailable, err.Error()) - case IsNotSupported(err): + case IsNotImplemented(err): return grpc.Errorf(codes.Unimplemented, err.Error()) } @@ -72,7 +72,7 @@ func FromGRPC(err error) error { case codes.FailedPrecondition: cls = ErrFailedPrecondition case codes.Unimplemented: - cls = ErrNotSupported + cls = ErrNotImplemented default: cls = ErrUnknown } diff --git a/platforms/platforms.go b/platforms/platforms.go index 96ecf38ec..95c40d7f9 100644 --- a/platforms/platforms.go +++ b/platforms/platforms.go @@ -1,3 +1,92 @@ +// Package platforms provides a toolkit for normalizing, matching and +// specifying container platforms. +// +// Centered around OCI platform specifications, we define a string-based +// specifier syntax that can be used for user input. With a specifier, users +// only need to specify the parts of the platform that are relevant to their +// context, providing an operating system or architecture or both. +// +// How do I use this package? +// +// The vast majority of use cases should simply use the match function with +// user input. The first step is to parse a specifier into a matcher: +// +// m, err := Parse("linux") +// if err != nil { ... } +// +// Once you have a matcher, use it to match against the platform declared by a +// component, typically from an image or runtime. Since extracting an images +// platform is a little more involved, we'll use an example against the +// platform default: +// +// if ok := m.Match(Default()); !ok { /* doesn't match */ } +// +// This can be composed in loops for resolving runtimes or used as a filter for +// fetch and select images. +// +// More details of the specifier syntax and platform spec follow. +// +// Declaring Platform Support +// +// Components that have strict platform requirements should use the OCI +// platform specification to declare their support. Typically, this will be +// images and runtimes that should make these declaring which platform they +// support specifically. This looks roughly as follows: +// +// type Platform struct { +// Architecture string +// OS string +// Variant string +// } +// +// Most images and runtimes should at least set Architecture and OS, according +// to their GOARCH and GOOS values, respectively (follow the OCI image +// specification when in doubt). ARM should set variant under certain +// discussions, which are outlined below. +// +// Platform Specifiers +// +// While the OCI platform specifications provide a tool for components to +// specify structured information, user input typically doesn't need the full +// context and much can be inferred. To solve this problem, we introduced +// "specifiers". A specifier has the format `[/[/]]`. +// The user can provide either the operating system or the architecture or both. +// +// An example of a common specifier is `linux/amd64`. If the host has a default +// of runtime that matches this, the user can simply provide the component that +// matters. For example, if a image provides amd64 and arm64 support, the +// operating system, `linux` can be inferred, so they only have to provide +// `arm64` or `amd64`. Similar behavior is implemented for operating systems, +// where the architecture may be known but a runtime may support images from +// different operating systems. +// +// Normalization +// +// Because not all users are familiar with the way the Go runtime represents +// platforms, several normalizations have been provided to make this package +// easier to user. +// +// The following are performed for architectures: +// +// Value Normalized +// aarch64 arm64 +// armhf arm +// armel arm/v6 +// i386 386 +// x86_64 amd64 +// x86-64 amd64 +// +// We also normalize the operating system `macos` to `darwin`. +// +// ARM Support +// +// To qualify ARM architecture, the Variant field is used to qualify the arm +// version. The most common arm version, v7, is represented without the variant +// unless it is explicitly provided. This is treated as equivalent to armhf. A +// previous architecture, armel, will be normalized to arm/v6. +// +// While these normalizations are provided, their support on arm platforms has +// not yet been fully implemented and tested. package platforms import ( @@ -10,36 +99,56 @@ import ( "github.com/pkg/errors" ) -type platformKey struct { - os string - arch string - variant string -} - var ( - selectorRe = regexp.MustCompile(`^[A-Za-z0-9_-]+$`) + specifierRe = regexp.MustCompile(`^[A-Za-z0-9_-]+$`) ) -// ParseSelector parses the platform selector syntax into a platform -// declaration. +// Matcher matches platforms specifications, provided by an image or runtime. +type Matcher interface { + Spec() specs.Platform + Match(platform specs.Platform) bool +} + +type matcher struct { + specs.Platform +} + +func (m *matcher) Spec() specs.Platform { + return m.Platform +} + +func (m *matcher) Match(platform specs.Platform) bool { + normalized := Normalize(platform) + return m.OS == normalized.OS && + m.Architecture == normalized.Architecture && + m.Variant == normalized.Variant +} + +func (m *matcher) String() string { + return Format(m.Platform) +} + +// Parse parses the platform specifier syntax into a platform declaration. // -// Platform selectors are in the format [/[/]]. The -// minimum required information for a platform selector is the operating system +// 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 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. -func Parse(selector string) (specs.Platform, error) { - if strings.Contains(selector, "*") { +// +// Applications should opt to use `Match` over directly parsing specifiers. +func Parse(specifier string) (Matcher, error) { + if strings.Contains(specifier, "*") { // TODO(stevvooe): need to work out exact wildcard handling - return specs.Platform{}, errors.Wrapf(errdefs.ErrInvalidArgument, "%q: wildcards not yet supported", selector) + return nil, errors.Wrapf(errdefs.ErrInvalidArgument, "%q: wildcards not yet supported", specifier) } - parts := strings.Split(selector, "/") + parts := strings.Split(specifier, "/") for _, part := range parts { - if !selectorRe.MatchString(part) { - return specs.Platform{}, errors.Wrapf(errdefs.ErrInvalidArgument, "%q is an invalid component of %q: platform selector component must match %q", part, selector, selectorRe.String()) + if !specifierRe.MatchString(part) { + return nil, errors.Wrapf(errdefs.ErrInvalidArgument, "%q is an invalid component of %q: platform specifier component must match %q", part, specifier, specifierRe.String()) } } @@ -57,41 +166,38 @@ func Parse(selector string) (specs.Platform, error) { p.Architecture = runtime.GOARCH if p.Architecture == "arm" { // TODO(stevvooe): Resolve arm variant, if not v6 (default) + return nil, errors.Wrapf(errdefs.ErrNotImplemented, "arm support not fully implemented") } - return p, nil + return &matcher{p}, nil } p.Architecture, p.Variant = normalizeArch(parts[0], "") if isKnownArch(p.Architecture) { p.OS = runtime.GOOS - return p, nil + return &matcher{p}, nil } - return specs.Platform{}, errors.Wrapf(errdefs.ErrInvalidArgument, "%q: unknown operating system or architecture", selector) + return nil, errors.Wrapf(errdefs.ErrInvalidArgument, "%q: unknown operating system or architecture", specifier) case 2: // In this case, we treat as a regular os/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], "") - return p, nil + return &matcher{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]) - return p, nil + return &matcher{p}, nil } - return specs.Platform{}, errors.Wrapf(errdefs.ErrInvalidArgument, "%q: cannot parse platform selector", selector) + return nil, errors.Wrapf(errdefs.ErrInvalidArgument, "%q: cannot parse platform specifier", specifier) } -func Match(selector string, platform specs.Platform) bool { - return false -} - -// Format returns a string that provides a shortened overview of the platform. +// Format returns a string specifier from the provided platform specification. func Format(platform specs.Platform) string { if platform.OS == "" { return "unknown" diff --git a/platforms/platforms_test.go b/platforms/platforms_test.go index a34fa4753..a88dfbf42 100644 --- a/platforms/platforms_test.go +++ b/platforms/platforms_test.go @@ -1,6 +1,7 @@ package platforms import ( + "fmt" "reflect" "runtime" "testing" @@ -174,16 +175,25 @@ func TestParseSelector(t *testing.T) { if testcase.skip { t.Skip("this case is not yet supported") } - p, err := Parse(testcase.input) + m, err := Parse(testcase.input) if err != nil { t.Fatal(err) } - if !reflect.DeepEqual(p, testcase.expected) { - t.Fatalf("platform did not match expected: %#v != %#v", p, testcase.expected) + if !reflect.DeepEqual(m.Spec(), testcase.expected) { + t.Fatalf("platform did not match expected: %#v != %#v", m.Spec(), testcase.expected) } - formatted := Format(p) + // ensure that match works on the input to the output. + if ok := m.Match(testcase.expected); !ok { + t.Fatalf("expected specifier %q matches %v", testcase.input, testcase.expected) + } + + if fmt.Sprint(m) != testcase.formatted { + t.Fatalf("unexpected matcher string: %q != %q", fmt.Sprint(m), testcase.formatted) + } + + formatted := Format(m.Spec()) if formatted != testcase.formatted { t.Fatalf("unexpected format: %q != %q", formatted, testcase.formatted) } @@ -194,8 +204,8 @@ 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 Format(reparsed.Spec()) != formatted { + t.Fatalf("normalized output did not survive the round trip: %v != %v", Format(reparsed.Spec()), formatted) } }) } @@ -221,7 +231,7 @@ func TestParseSelectorInvalid(t *testing.T) { input: "linux/&arm", // invalid character }, { - input: "linux/arm/foo/bar", // too mayn components + input: "linux/arm/foo/bar", // too many components }, } { t.Run(testcase.input, func(t *testing.T) { diff --git a/services/diff/service.go b/services/diff/service.go index 6d65fd951..af6326b60 100644 --- a/services/diff/service.go +++ b/services/diff/service.go @@ -74,7 +74,7 @@ func (s *service) Apply(ctx context.Context, er *diffapi.ApplyRequest) (*diffapi for _, differ := range s.differs { ocidesc, err = differ.Apply(ctx, desc, mounts) - if !errdefs.IsNotSupported(err) { + if !errdefs.IsNotImplemented(err) { break } } @@ -99,7 +99,7 @@ func (s *service) Diff(ctx context.Context, dr *diffapi.DiffRequest) (*diffapi.D for _, differ := range s.differs { ocidesc, err = differ.DiffMounts(ctx, aMounts, bMounts, dr.MediaType, dr.Ref) - if !errdefs.IsNotSupported(err) { + if !errdefs.IsNotImplemented(err) { break } } From 775f7cea47815694fde145f72a764f5b43bcb661 Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Wed, 13 Sep 2017 11:41:45 -0700 Subject: [PATCH 3/3] platforms: update format for platform specifier Signed-off-by: Stephen J Day --- platforms/platforms.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/platforms/platforms.go b/platforms/platforms.go index 95c40d7f9..56c6ddc51 100644 --- a/platforms/platforms.go +++ b/platforms/platforms.go @@ -49,8 +49,9 @@ // While the OCI platform specifications provide a tool for components to // specify structured information, user input typically doesn't need the full // context and much can be inferred. To solve this problem, we introduced -// "specifiers". A specifier has the format `[/[/]]`. -// The user can provide either the operating system or the architecture or both. +// "specifiers". A specifier has the format +// `||/[/]`. The user can provide either the +// operating system or the architecture or both. // // An example of a common specifier is `linux/amd64`. If the host has a default // of runtime that matches this, the user can simply provide the component that @@ -130,10 +131,10 @@ func (m *matcher) String() string { // Parse parses the platform specifier syntax into a platform declaration. // -// 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 value -// will be matched against the known set of operating systems, then fall +// 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 +// 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. //