From 6fb4da145790a8c3bcbe30a5beb2d1d585b98923 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Mon, 31 Jul 2023 18:13:40 +0200 Subject: [PATCH 01/13] Add a comment about mounting blobs with unknown locations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- docker/docker_image_dest.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docker/docker_image_dest.go b/docker/docker_image_dest.go index 63e372d677..0e7b154cc6 100644 --- a/docker/docker_image_dest.go +++ b/docker/docker_image_dest.go @@ -367,6 +367,11 @@ func (d *dockerImageDestination) TryReusingBlobWithOptions(ctx context.Context, // Sanity checks: if reference.Domain(candidateRepo) != reference.Domain(d.ref.ref) { + // OCI distribution spec 1.1 allows mounting blobs without specifying the source repo + // (the "from" parameter); in that case we might try to use these candidates as well. + // + // OTOH that would mean we can’t do the “blobExists” check, and if there is no match + // we could get an upload request that we would have to cancel. logrus.Debugf("... Internal error: domain %s does not match destination %s", reference.Domain(candidateRepo), reference.Domain(d.ref.ref)) continue } From 62f49acd3e7534ce3998a68a4c737599c7d252ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Mon, 31 Jul 2023 19:01:18 +0200 Subject: [PATCH 02/13] opencontainers/distribution-spec does not require errors to carry JSON MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ... so update one of our checks to rely on the specified status (only). Signed-off-by: Miloslav Trmač --- docker/docker_client.go | 6 +++--- docker/errors.go | 7 ++++++- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/docker/docker_client.go b/docker/docker_client.go index dd9127c5ac..b97758989c 100644 --- a/docker/docker_client.go +++ b/docker/docker_client.go @@ -1,7 +1,6 @@ package docker import ( - "bytes" "context" "crypto/tls" "encoding/json" @@ -1008,9 +1007,10 @@ func isManifestUnknownError(err error) bool { if errors.As(err, &e) && e.ErrorCode() == errcode.ErrorCodeUnknown && e.Message == "Not Found" { return true } - // ALSO registry.redhat.io as of October 2022 + // opencontainers/distribution-spec does not require the errcode.Error payloads to be used, + // but specifies that the HTTP status must be 404. var unexpected *unexpectedHTTPResponseError - if errors.As(err, &unexpected) && unexpected.StatusCode == http.StatusNotFound && bytes.Contains(unexpected.Response, []byte("Not found")) { + if errors.As(err, &unexpected) && unexpected.StatusCode == http.StatusNotFound { return true } return false diff --git a/docker/errors.go b/docker/errors.go index 2caa10d7d3..e039691890 100644 --- a/docker/errors.go +++ b/docker/errors.go @@ -47,7 +47,12 @@ func httpResponseToError(res *http.Response, context string) error { } // registryHTTPResponseToError creates a Go error from an HTTP error response of a docker/distribution -// registry +// registry. +// +// WARNING: The OCI distribution spec says +// “A `4XX` response code from the registry MAY return a body in any format.”; but if it is +// JSON, it MUST use the errcode.Error structure. +// So, callers should primarily decide based on HTTP StatusCode, not based on error type here. func registryHTTPResponseToError(res *http.Response) error { err := handleErrorResponse(res) // len(errs) == 0 should never be returned by handleErrorResponse; if it does, we don't modify it and let the caller report it as is. From 91536efe4338276006c5dd940ffd69b94409de3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Mon, 31 Jul 2023 21:11:56 +0200 Subject: [PATCH 03/13] UNTESTED: Log warnings on a Warning: header MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ... as now requested by distribution-spec. Untested apart from the added parser unit test. Signed-off-by: Miloslav Trmač --- docker/docker_client.go | 80 ++++++++++++++++++++++++++++++++++-- docker/docker_client_test.go | 22 ++++++++++ 2 files changed, 98 insertions(+), 4 deletions(-) diff --git a/docker/docker_client.go b/docker/docker_client.go index b97758989c..288dd1a93f 100644 --- a/docker/docker_client.go +++ b/docker/docker_client.go @@ -18,6 +18,7 @@ import ( "github.com/containers/image/v5/docker/reference" "github.com/containers/image/v5/internal/iolimits" + "github.com/containers/image/v5/internal/set" "github.com/containers/image/v5/internal/useragent" "github.com/containers/image/v5/manifest" "github.com/containers/image/v5/pkg/docker/config" @@ -120,6 +121,9 @@ type dockerClient struct { // Private state for detectProperties: detectPropertiesOnce sync.Once // detectPropertiesOnce is used to execute detectProperties() at most once. detectPropertiesError error // detectPropertiesError caches the initial error. + // Private state for logResponseWarnings + reportedWarningsLock sync.Mutex + reportedWarnings *set.Set[string] } type authScope struct { @@ -280,10 +284,11 @@ func newDockerClient(sys *types.SystemContext, registry, reference string) (*doc } return &dockerClient{ - sys: sys, - registry: registry, - userAgent: userAgent, - tlsClientConfig: tlsClientConfig, + sys: sys, + registry: registry, + userAgent: userAgent, + tlsClientConfig: tlsClientConfig, + reportedWarnings: set.New[string](), }, nil } @@ -623,9 +628,76 @@ func (c *dockerClient) makeRequestToResolvedURLOnce(ctx context.Context, method if err != nil { return nil, err } + if warnings := res.Header.Values("Warning"); len(warnings) != 0 { + c.logResponseWarnings(res, warnings) + } return res, nil } +// logResponseWarnings logs warningHeaders from res, if any. +func (c *dockerClient) logResponseWarnings(res *http.Response, warningHeaders []string) { + c.reportedWarningsLock.Lock() + defer c.reportedWarningsLock.Unlock() + + for _, header := range warningHeaders { + warningString := parseRegistryWarningHeader(header) + if warningString == "" { + logrus.Debugf("Ignored Warning: header from registry: %q", header) + } else { + if !c.reportedWarnings.Contains(warningString) { + c.reportedWarnings.Add(warningString) + // Note that reportedWarnings is based only on warningString, so that we don’t + // repeat the same warning for every request - but the warning includes the URL; + // so it may not be specific to that URL. + logrus.Warnf("Warning from registry (first encountered at %q): %q", res.Request.URL.Redacted(), warningString) + } else { + logrus.Debugf("Repeated warning from registry at %q: %q", res.Request.URL.Redacted(), warningString) + } + } + } +} + +// parseRegistryWarningHeader parses a Warning: header per RFC 7234, limited to the warning +// values allowed by opencontainers/distribution-spec. +// It returns the warning string if the header has the expected format, or "" otherwise. +func parseRegistryWarningHeader(header string) string { + const expectedPrefix = `299 - "` + const expectedSuffix = `"` + + // warning-value = warn-code SP warn-agent SP warn-text [ SP warn-date ] + // distribution-spec requires warn-code=299, warn-agent="-", warn-date missing + if !strings.HasPrefix(header, expectedPrefix) || !strings.HasSuffix(header, expectedSuffix) { + return "" + } + header = header[len(expectedPrefix) : len(header)-len(expectedSuffix)] + + // ”Recipients that process the value of a quoted-string MUST handle a quoted-pair + // as if it were replaced by the octet following the backslash.”, so let’s do that… + res := strings.Builder{} + afterBackslash := false + for _, c := range []byte(header) { // []byte because escaping is defined in terms of bytes, not Unicode code points + switch { + case c == 0x7F || (c < ' ' && c != '\t'): + return "" // Control characters are forbidden + case afterBackslash: + res.WriteByte(c) + afterBackslash = false + case c == '"': + // This terminates the warn-text and warn-date, forbidden by distribution-spec, follows, + // or completely invalid input. + return "" + case c == '\\': + afterBackslash = true + default: + res.WriteByte(c) + } + } + if afterBackslash { + return "" + } + return res.String() +} + // we're using the challenges from the /v2/ ping response and not the one from the destination // URL in this request because: // diff --git a/docker/docker_client_test.go b/docker/docker_client_test.go index 086bc132a6..67e38764b4 100644 --- a/docker/docker_client_test.go +++ b/docker/docker_client_test.go @@ -332,6 +332,28 @@ func TestNeedsNoRetry(t *testing.T) { } } +func TestParseRegistryWarningHeader(t *testing.T) { + for _, c := range []struct{ header, expected string }{ + {"completely invalid", ""}, + {`299 - "trivial"`, "trivial"}, + {`100 - "not-299"`, ""}, + {`299 localhost "warn-agent set"`, ""}, + {`299 - "no-terminating-quote`, ""}, + {"299 - \"\x01 control\"", ""}, + {"299 - \"\\\x01 escaped control\"", ""}, + {"299 - \"e\\scaped\"", "escaped"}, + {"299 - \"non-UTF8 \xA1\xA2\"", "non-UTF8 \xA1\xA2"}, + {"299 - \"non-UTF8 escaped \\\xA1\\\xA2\"", "non-UTF8 escaped \xA1\xA2"}, + {"299 - \"UTF8 žluťoučký\"", "UTF8 žluťoučký"}, + {"299 - \"UTF8 \\\xC5\\\xBEluťoučký\"", "UTF8 žluťoučký"}, + {`299 - "unterminated`, ""}, + {`299 - "warning" "some-date"`, ""}, + } { + res := parseRegistryWarningHeader(c.header) + assert.Equal(t, c.expected, res, c.header) + } +} + func TestIsManifestUnknownError(t *testing.T) { // Mostly a smoke test; we can add more registries here if they need special handling. From 8a2fca6ccc81bc8b1e88f50bfa1db0994f0af53d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Mon, 31 Jul 2023 21:55:06 +0200 Subject: [PATCH 04/13] Add a comment about using the os.version and os.flags fields. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- internal/pkg/platform/platform_matcher.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/pkg/platform/platform_matcher.go b/internal/pkg/platform/platform_matcher.go index 59b1d4b9e2..3ba0e4084f 100644 --- a/internal/pkg/platform/platform_matcher.go +++ b/internal/pkg/platform/platform_matcher.go @@ -128,6 +128,10 @@ var compatibility = map[string][]string{ // the most compatible platform is first. // If some option (arch, os, variant) is not present, a value from current platform is detected. func WantedPlatforms(ctx *types.SystemContext) ([]imgspecv1.Platform, error) { + // Note that this does not use Platform.OSFeatures and Platform.OSVersion at all. + // The fields are not specified by the OCI specification, as of version 1.1, usefully enough + // to be interoperable, anyway. + wantedArch := runtime.GOARCH wantedVariant := "" if ctx != nil && ctx.ArchitectureChoice != "" { From 3a26748d6e7ddfcbe459240422ad1d6fb8b0427e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Mon, 31 Jul 2023 21:55:50 +0200 Subject: [PATCH 05/13] RFC: Match variant in checkImageDestinationForCurrentRuntime MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As described in the comment, this might cause excess noise. Signed-off-by: Miloslav Trmač --- copy/single.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/copy/single.go b/copy/single.go index 5297f019f7..dff7aad22f 100644 --- a/copy/single.go +++ b/copy/single.go @@ -305,18 +305,18 @@ func checkImageDestinationForCurrentRuntime(ctx context.Context, sys *types.Syst options := newOrderedSet() match := false for _, wantedPlatform := range wantedPlatforms { - // Waiting for https://github.com/opencontainers/image-spec/pull/777 : - // This currently can’t use image.MatchesPlatform because we don’t know what to use - // for image.Variant. - if wantedPlatform.OS == c.OS && wantedPlatform.Architecture == c.Architecture { + // For a transitional period, this might trigger warnings because the Variant + // field was added to OCI config only recently. If this turns out to be too noisy, + // revert this check to only look for (OS, Architecture). + if platform.MatchesPlatform(c.Platform, wantedPlatform) { match = true break } - options.append(fmt.Sprintf("%s+%s", wantedPlatform.OS, wantedPlatform.Architecture)) + options.append(fmt.Sprintf("%s+%s+%q", wantedPlatform.OS, wantedPlatform.Architecture, wantedPlatform.Variant)) } if !match { - logrus.Infof("Image operating system mismatch: image uses OS %q+architecture %q, expecting one of %q", - c.OS, c.Architecture, strings.Join(options.list, ", ")) + logrus.Infof("Image operating system mismatch: image uses OS %q+architecture %q+%q, expecting one of %q", + c.OS, c.Architecture, c.Variant, strings.Join(options.list, ", ")) } } return nil From 98a9280a32254e115175dc0ea690e4a574336121 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Mon, 31 Jul 2023 22:28:46 +0200 Subject: [PATCH 06/13] Avoid a nil pointer dereference in EditInstances MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- internal/manifest/docker_schema2_list.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/internal/manifest/docker_schema2_list.go b/internal/manifest/docker_schema2_list.go index 14a476642e..ab86a63cd7 100644 --- a/internal/manifest/docker_schema2_list.go +++ b/internal/manifest/docker_schema2_list.go @@ -119,6 +119,12 @@ func (index *Schema2ListPublic) editInstances(editInstances []ListEdit) error { } index.Manifests[targetIndex].MediaType = editInstance.UpdateMediaType case ListOpAdd: + if editInstance.AddPlatform == nil { + // Should we create a struct with empty fields instead? + // Right now ListOpAdd is only called when an instance with the same platform value + // already exists in the manifest, so this should not be reached in practice. + return fmt.Errorf("adding a schema2 list instance with no platform specified is not supported") + } addInstance := Schema2ManifestDescriptor{ Schema2Descriptor{Digest: editInstance.AddDigest, Size: editInstance.AddSize, MediaType: editInstance.AddMediaType}, Schema2PlatformSpec{ From d4733eada2a8dc0d207086876acf2c66764b379c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Mon, 31 Jul 2023 22:27:14 +0200 Subject: [PATCH 07/13] Define helpers for cloning and conversion of platform descriptors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ... to reduce duplication. Should not change behavior. Signed-off-by: Miloslav Trmač --- internal/manifest/docker_schema2_list.go | 46 ++++++++------------ internal/manifest/oci_index.go | 54 +++++++++++++++--------- 2 files changed, 51 insertions(+), 49 deletions(-) diff --git a/internal/manifest/docker_schema2_list.go b/internal/manifest/docker_schema2_list.go index ab86a63cd7..f50b52f3ac 100644 --- a/internal/manifest/docker_schema2_list.go +++ b/internal/manifest/docker_schema2_list.go @@ -64,13 +64,8 @@ func (list *Schema2ListPublic) Instance(instanceDigest digest.Digest) (ListUpdat MediaType: manifest.MediaType, } ret.ReadOnly.CompressionAlgorithmNames = []string{compression.GzipAlgorithmName} - ret.ReadOnly.Platform = &imgspecv1.Platform{ - OS: manifest.Platform.OS, - Architecture: manifest.Platform.Architecture, - OSVersion: manifest.Platform.OSVersion, - OSFeatures: manifest.Platform.OSFeatures, - Variant: manifest.Platform.Variant, - } + platform := ociPlatformFromSchema2PlatformSpec(manifest.Platform) + ret.ReadOnly.Platform = &platform return ret, nil } } @@ -127,13 +122,7 @@ func (index *Schema2ListPublic) editInstances(editInstances []ListEdit) error { } addInstance := Schema2ManifestDescriptor{ Schema2Descriptor{Digest: editInstance.AddDigest, Size: editInstance.AddSize, MediaType: editInstance.AddMediaType}, - Schema2PlatformSpec{ - OS: editInstance.AddPlatform.OS, - Architecture: editInstance.AddPlatform.Architecture, - OSVersion: editInstance.AddPlatform.OSVersion, - OSFeatures: editInstance.AddPlatform.OSFeatures, - Variant: editInstance.AddPlatform.Variant, - }, + schema2PlatformSpecFromOCIPlatform(*editInstance.AddPlatform), } addedEntries = append(addedEntries, addInstance) default: @@ -164,13 +153,7 @@ func (list *Schema2ListPublic) ChooseInstance(ctx *types.SystemContext) (digest. } for _, wantedPlatform := range wantedPlatforms { for _, d := range list.Manifests { - imagePlatform := imgspecv1.Platform{ - Architecture: d.Platform.Architecture, - OS: d.Platform.OS, - OSVersion: d.Platform.OSVersion, - OSFeatures: slices.Clone(d.Platform.OSFeatures), - Variant: d.Platform.Variant, - } + imagePlatform := ociPlatformFromSchema2PlatformSpec(d.Platform) if platform.MatchesPlatform(imagePlatform, wantedPlatform) { return d.Digest, nil } @@ -230,18 +213,13 @@ func Schema2ListPublicClone(list *Schema2ListPublic) *Schema2ListPublic { func (list *Schema2ListPublic) ToOCI1Index() (*OCI1IndexPublic, error) { components := make([]imgspecv1.Descriptor, 0, len(list.Manifests)) for _, manifest := range list.Manifests { + platform := ociPlatformFromSchema2PlatformSpec(manifest.Platform) converted := imgspecv1.Descriptor{ MediaType: manifest.MediaType, Size: manifest.Size, Digest: manifest.Digest, URLs: slices.Clone(manifest.URLs), - Platform: &imgspecv1.Platform{ - OS: manifest.Platform.OS, - Architecture: manifest.Platform.Architecture, - OSFeatures: slices.Clone(manifest.Platform.OSFeatures), - OSVersion: manifest.Platform.OSVersion, - Variant: manifest.Platform.Variant, - }, + Platform: &platform, } components = append(components, converted) } @@ -318,3 +296,15 @@ func Schema2ListFromManifest(manifest []byte) (*Schema2List, error) { } return schema2ListFromPublic(public), nil } + +// ociPlatformFromSchema2PlatformSpec converts a schema2 platform p to the OCI struccture. +func ociPlatformFromSchema2PlatformSpec(p Schema2PlatformSpec) imgspecv1.Platform { + return imgspecv1.Platform{ + Architecture: p.Architecture, + OS: p.OS, + OSVersion: p.OSVersion, + OSFeatures: slices.Clone(p.OSFeatures), + Variant: p.Variant, + // Features is not supported in OCI, and discarded. + } +} diff --git a/internal/manifest/oci_index.go b/internal/manifest/oci_index.go index 8832caa3ee..00a05e6d90 100644 --- a/internal/manifest/oci_index.go +++ b/internal/manifest/oci_index.go @@ -239,13 +239,7 @@ func (index *OCI1IndexPublic) chooseInstance(ctx *types.SystemContext, preferGzi for manifestIndex, d := range index.Manifests { candidate := instanceCandidate{platformIndex: math.MaxInt, manifestPosition: manifestIndex, isZstd: instanceIsZstd(d), digest: d.Digest} if d.Platform != nil { - imagePlatform := imgspecv1.Platform{ - Architecture: d.Platform.Architecture, - OS: d.Platform.OS, - OSVersion: d.Platform.OSVersion, - OSFeatures: slices.Clone(d.Platform.OSFeatures), - Variant: d.Platform.Variant, - } + imagePlatform := ociPlatformClone(*d.Platform) platformIndex := slices.IndexFunc(wantedPlatforms, func(wantedPlatform imgspecv1.Platform) bool { return platform.MatchesPlatform(imagePlatform, wantedPlatform) }) @@ -299,13 +293,8 @@ func OCI1IndexPublicFromComponents(components []imgspecv1.Descriptor, annotation for i, component := range components { var platform *imgspecv1.Platform if component.Platform != nil { - platform = &imgspecv1.Platform{ - Architecture: component.Platform.Architecture, - OS: component.Platform.OS, - OSVersion: component.Platform.OSVersion, - OSFeatures: slices.Clone(component.Platform.OSFeatures), - Variant: component.Platform.Variant, - } + platformCopy := ociPlatformClone(*component.Platform) + platform = &platformCopy } m := imgspecv1.Descriptor{ MediaType: component.MediaType, @@ -349,13 +338,7 @@ func (index *OCI1IndexPublic) ToSchema2List() (*Schema2ListPublic, error) { Digest: manifest.Digest, URLs: slices.Clone(manifest.URLs), }, - Schema2PlatformSpec{ - OS: platform.OS, - Architecture: platform.Architecture, - OSFeatures: slices.Clone(platform.OSFeatures), - OSVersion: platform.OSVersion, - Variant: platform.Variant, - }, + schema2PlatformSpecFromOCIPlatform(*platform), } components = append(components, converted) } @@ -431,3 +414,32 @@ func OCI1IndexFromManifest(manifest []byte) (*OCI1Index, error) { } return oci1IndexFromPublic(public), nil } + +// ociPlatformClone returns an independent copy of p. +func ociPlatformClone(p imgspecv1.Platform) imgspecv1.Platform { + // The only practical way in Go to give read-only access to an array is to copy it. + // The only practical way in Go to copy a deep structure is to either do it manually field by field, + // or to use reflection (incl. a round-trip through JSON, which uses reflection). + // + // The combination of the two is just sad, and leads to code like this, which will + // need to be updated with every new Platform field. + return imgspecv1.Platform{ + Architecture: p.Architecture, + OS: p.OS, + OSVersion: p.OSVersion, + OSFeatures: slices.Clone(p.OSFeatures), + Variant: p.Variant, + } +} + +// schema2PlatformSpecFromOCIPlatform converts an OCI platform p to the schema2 structure. +func schema2PlatformSpecFromOCIPlatform(p imgspecv1.Platform) Schema2PlatformSpec { + return Schema2PlatformSpec{ + Architecture: p.Architecture, + OS: p.OS, + OSVersion: p.OSVersion, + OSFeatures: slices.Clone(p.OSFeatures), + Variant: p.Variant, + Features: nil, + } +} From 7daea33aa6d7b63ece55b78a81924e9b97da8bb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Mon, 31 Jul 2023 22:30:27 +0200 Subject: [PATCH 08/13] Avoid some single-use variables MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Should not change behavior. Signed-off-by: Miloslav Trmač --- internal/manifest/docker_schema2_list.go | 16 +++++++++------- internal/manifest/oci_index.go | 5 ++--- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/internal/manifest/docker_schema2_list.go b/internal/manifest/docker_schema2_list.go index f50b52f3ac..357e2f3d3e 100644 --- a/internal/manifest/docker_schema2_list.go +++ b/internal/manifest/docker_schema2_list.go @@ -120,11 +120,14 @@ func (index *Schema2ListPublic) editInstances(editInstances []ListEdit) error { // already exists in the manifest, so this should not be reached in practice. return fmt.Errorf("adding a schema2 list instance with no platform specified is not supported") } - addInstance := Schema2ManifestDescriptor{ - Schema2Descriptor{Digest: editInstance.AddDigest, Size: editInstance.AddSize, MediaType: editInstance.AddMediaType}, + addedEntries = append(addedEntries, Schema2ManifestDescriptor{ + Schema2Descriptor{ + Digest: editInstance.AddDigest, + Size: editInstance.AddSize, + MediaType: editInstance.AddMediaType, + }, schema2PlatformSpecFromOCIPlatform(*editInstance.AddPlatform), - } - addedEntries = append(addedEntries, addInstance) + }) default: return fmt.Errorf("internal error: invalid operation: %d", editInstance.ListOperation) } @@ -214,14 +217,13 @@ func (list *Schema2ListPublic) ToOCI1Index() (*OCI1IndexPublic, error) { components := make([]imgspecv1.Descriptor, 0, len(list.Manifests)) for _, manifest := range list.Manifests { platform := ociPlatformFromSchema2PlatformSpec(manifest.Platform) - converted := imgspecv1.Descriptor{ + components = append(components, imgspecv1.Descriptor{ MediaType: manifest.MediaType, Size: manifest.Size, Digest: manifest.Digest, URLs: slices.Clone(manifest.URLs), Platform: &platform, - } - components = append(components, converted) + }) } oci := OCI1IndexPublicFromComponents(components, nil) return oci, nil diff --git a/internal/manifest/oci_index.go b/internal/manifest/oci_index.go index 00a05e6d90..dcd2646d13 100644 --- a/internal/manifest/oci_index.go +++ b/internal/manifest/oci_index.go @@ -331,7 +331,7 @@ func (index *OCI1IndexPublic) ToSchema2List() (*Schema2ListPublic, error) { Architecture: runtime.GOARCH, } } - converted := Schema2ManifestDescriptor{ + components = append(components, Schema2ManifestDescriptor{ Schema2Descriptor{ MediaType: manifest.MediaType, Size: manifest.Size, @@ -339,8 +339,7 @@ func (index *OCI1IndexPublic) ToSchema2List() (*Schema2ListPublic, error) { URLs: slices.Clone(manifest.URLs), }, schema2PlatformSpecFromOCIPlatform(*platform), - } - components = append(components, converted) + }) } s2 := Schema2ListPublicFromComponents(components) return s2, nil From 9ee0a99dc8ea06b816900b51a61cb08e63b38970 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Tue, 1 Aug 2023 00:24:01 +0200 Subject: [PATCH 09/13] Support the new ArtifactType field when complaining about non-images MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- internal/image/oci.go | 6 +++--- internal/manifest/errors.go | 22 +++++++++++++++++++--- manifest/oci.go | 4 ++-- 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/internal/image/oci.go b/internal/image/oci.go index 166daa0e87..c6c0fb362f 100644 --- a/internal/image/oci.go +++ b/internal/image/oci.go @@ -86,7 +86,7 @@ func (m *manifestOCI1) ConfigBlob(ctx context.Context) ([]byte, error) { // old image manifests work (docker v2s1 especially). func (m *manifestOCI1) OCIConfig(ctx context.Context) (*imgspecv1.Image, error) { if m.m.Config.MediaType != imgspecv1.MediaTypeImageConfig { - return nil, internalManifest.NewNonImageArtifactError(m.m.Config.MediaType) + return nil, internalManifest.NewNonImageArtifactError(&m.m.Manifest) } cb, err := m.ConfigBlob(ctx) @@ -200,7 +200,7 @@ func (m *manifestOCI1) convertToManifestSchema2Generic(ctx context.Context, opti // This does not change the state of the original manifestOCI1 object. func (m *manifestOCI1) convertToManifestSchema2(_ context.Context, _ *types.ManifestUpdateOptions) (*manifestSchema2, error) { if m.m.Config.MediaType != imgspecv1.MediaTypeImageConfig { - return nil, internalManifest.NewNonImageArtifactError(m.m.Config.MediaType) + return nil, internalManifest.NewNonImageArtifactError(&m.m.Manifest) } // Create a copy of the descriptor. @@ -244,7 +244,7 @@ func (m *manifestOCI1) convertToManifestSchema2(_ context.Context, _ *types.Mani // This does not change the state of the original manifestOCI1 object. func (m *manifestOCI1) convertToManifestSchema1(ctx context.Context, options *types.ManifestUpdateOptions) (genericManifest, error) { if m.m.Config.MediaType != imgspecv1.MediaTypeImageConfig { - return nil, internalManifest.NewNonImageArtifactError(m.m.Config.MediaType) + return nil, internalManifest.NewNonImageArtifactError(&m.m.Manifest) } // We can't directly convert images to V1, but we can transitively convert via a V2 image diff --git a/internal/manifest/errors.go b/internal/manifest/errors.go index 6ebe4b24cf..6c8e233d97 100644 --- a/internal/manifest/errors.go +++ b/internal/manifest/errors.go @@ -1,6 +1,10 @@ package manifest -import "fmt" +import ( + "fmt" + + imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1" +) // FIXME: This is a duplicate of c/image/manifestDockerV2Schema2ConfigMediaType. // Deduplicate that, depending on outcome of https://github.com/containers/image/pull/1791 . @@ -26,8 +30,20 @@ type NonImageArtifactError struct { mimeType string } -// NewNonImageArtifactError returns a NonImageArtifactError about an artifact with mimeType. -func NewNonImageArtifactError(mimeType string) error { +// NewNonImageArtifactError returns a NonImageArtifactError about an artifact manifest. +// +// This is typically called if manifest.Config.MediaType != imgspecv1.MediaTypeImageConfig . +func NewNonImageArtifactError(manifest *imgspecv1.Manifest) error { + // Callers decide based on manifest.Config.MediaType that this is not an image; + // in that case manifest.ArtifactType can be optionally defined, and if it is, it is typically + // more relevant because config may be ~absent with imgspecv1.MediaTypeEmptyJSON. + // + // If ArtifactType and Config.MediaType are both defined and non-trivial, presumably + // ArtifactType is the “top-level” one, although that’s not defined by the spec. + mimeType := manifest.ArtifactType + if mimeType == "" { + mimeType = manifest.Config.MediaType + } return NonImageArtifactError{mimeType: mimeType} } diff --git a/manifest/oci.go b/manifest/oci.go index a70470d99a..a85641c36a 100644 --- a/manifest/oci.go +++ b/manifest/oci.go @@ -202,7 +202,7 @@ func (m *OCI1) Inspect(configGetter func(types.BlobInfo) ([]byte, error)) (*type // Most software calling this without human intervention is going to expect the values to be realistic and relevant, // and is probably better served by failing; we can always re-visit that later if we fail now, but // if we started returning some data for OCI artifacts now, we couldn’t start failing in this function later. - return nil, manifest.NewNonImageArtifactError(m.Config.MediaType) + return nil, manifest.NewNonImageArtifactError(&m.Manifest) } config, err := configGetter(m.ConfigInfo()) @@ -253,7 +253,7 @@ func (m *OCI1) ImageID([]digest.Digest) (string, error) { // (The only known caller of ImageID is storage/storageImageDestination.computeID, // which can’t work with non-image artifacts.) if m.Config.MediaType != imgspecv1.MediaTypeImageConfig { - return "", manifest.NewNonImageArtifactError(m.Config.MediaType) + return "", manifest.NewNonImageArtifactError(&m.Manifest) } if err := m.Config.Digest.Validate(); err != nil { From 94956794f133ac6eecd63f24fff875b5dc1b3789 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Tue, 1 Aug 2023 00:54:56 +0200 Subject: [PATCH 10/13] Don't hard-code a single digest in configBlobImageSource MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ... so that newOCI1ImageSource can support other configs. Should not change (test) behavior. Signed-off-by: Miloslav Trmač --- internal/image/docker_schema2_test.go | 15 +++++++++++---- internal/image/oci_test.go | 11 ++++++++--- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/internal/image/docker_schema2_test.go b/internal/image/docker_schema2_test.go index f2f8a3f1d0..4fe2e81090 100644 --- a/internal/image/docker_schema2_test.go +++ b/internal/image/docker_schema2_test.go @@ -22,6 +22,8 @@ import ( "golang.org/x/exp/slices" ) +const commonFixtureConfigDigest = "sha256:9ca4bda0a6b3727a6ffcc43e981cad0f24e2ec79d338f6ba325b4dfd0756fb8f" + func manifestSchema2FromFixture(t *testing.T, src types.ImageSource, fixture string, mustFail bool) genericManifest { manifest, err := os.ReadFile(filepath.Join("fixtures", fixture)) require.NoError(t, err) @@ -39,7 +41,7 @@ func manifestSchema2FromComponentsLikeFixture(configBlob []byte) genericManifest return manifestSchema2FromComponents(manifest.Schema2Descriptor{ MediaType: "application/octet-stream", Size: 5940, - Digest: "sha256:9ca4bda0a6b3727a6ffcc43e981cad0f24e2ec79d338f6ba325b4dfd0756fb8f", + Digest: commonFixtureConfigDigest, }, nil, configBlob, []manifest.Schema2Descriptor{ { MediaType: "application/vnd.docker.image.rootfs.diff.tar.gzip", @@ -114,7 +116,7 @@ func TestManifestSchema2ConfigInfo(t *testing.T) { } { assert.Equal(t, types.BlobInfo{ Size: 5940, - Digest: "sha256:9ca4bda0a6b3727a6ffcc43e981cad0f24e2ec79d338f6ba325b4dfd0756fb8f", + Digest: commonFixtureConfigDigest, MediaType: "application/octet-stream", }, m.ConfigInfo()) } @@ -123,11 +125,12 @@ func TestManifestSchema2ConfigInfo(t *testing.T) { // configBlobImageSource allows testing various GetBlob behaviors in .ConfigBlob() type configBlobImageSource struct { mocks.ForbiddenImageSource // We inherit almost all of the methods, which just panic() + expectedDigest digest.Digest f func() (io.ReadCloser, int64, error) } func (f configBlobImageSource) GetBlob(ctx context.Context, info types.BlobInfo, _ types.BlobInfoCache) (io.ReadCloser, int64, error) { - if info.Digest.String() != "sha256:9ca4bda0a6b3727a6ffcc43e981cad0f24e2ec79d338f6ba325b4dfd0756fb8f" { + if info.Digest != f.expectedDigest { panic("Unexpected digest in GetBlob") } return f.f() @@ -163,7 +166,10 @@ func TestManifestSchema2ConfigBlob(t *testing.T) { } { var src types.ImageSource if c.cbISfn != nil { - src = configBlobImageSource{f: c.cbISfn} + src = configBlobImageSource{ + expectedDigest: commonFixtureConfigDigest, + f: c.cbISfn, + } } else { src = nil } @@ -350,6 +356,7 @@ func newSchema2ImageSource(t *testing.T, dockerRef string) *schema2ImageSource { return &schema2ImageSource{ configBlobImageSource: configBlobImageSource{ + expectedDigest: commonFixtureConfigDigest, f: func() (io.ReadCloser, int64, error) { return io.NopCloser(bytes.NewReader(realConfigJSON)), int64(len(realConfigJSON)), nil }, diff --git a/internal/image/oci_test.go b/internal/image/oci_test.go index 84398dcf34..0531f842ee 100644 --- a/internal/image/oci_test.go +++ b/internal/image/oci_test.go @@ -15,6 +15,7 @@ import ( "github.com/containers/image/v5/internal/testing/mocks" "github.com/containers/image/v5/manifest" "github.com/containers/image/v5/types" + "github.com/opencontainers/go-digest" imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -33,7 +34,7 @@ func manifestOCI1FromComponentsLikeFixture(configBlob []byte) genericManifest { return manifestOCI1FromComponents(imgspecv1.Descriptor{ MediaType: imgspecv1.MediaTypeImageConfig, Size: 5940, - Digest: "sha256:9ca4bda0a6b3727a6ffcc43e981cad0f24e2ec79d338f6ba325b4dfd0756fb8f", + Digest: commonFixtureConfigDigest, Annotations: map[string]string{ "test-annotation-1": "one", }, @@ -117,7 +118,7 @@ func TestManifestOCI1ConfigInfo(t *testing.T) { } { assert.Equal(t, types.BlobInfo{ Size: 5940, - Digest: "sha256:9ca4bda0a6b3727a6ffcc43e981cad0f24e2ec79d338f6ba325b4dfd0756fb8f", + Digest: commonFixtureConfigDigest, Annotations: map[string]string{ "test-annotation-1": "one", }, @@ -156,7 +157,10 @@ func TestManifestOCI1ConfigBlob(t *testing.T) { } { var src types.ImageSource if c.cbISfn != nil { - src = configBlobImageSource{f: c.cbISfn} + src = configBlobImageSource{ + expectedDigest: commonFixtureConfigDigest, + f: c.cbISfn, + } } else { src = nil } @@ -362,6 +366,7 @@ func newOCI1ImageSource(t *testing.T, dockerRef string) *oci1ImageSource { return &oci1ImageSource{ configBlobImageSource: configBlobImageSource{ + expectedDigest: digest.FromBytes(realConfigJSON), f: func() (io.ReadCloser, int64, error) { return io.NopCloser(bytes.NewReader(realConfigJSON)), int64(len(realConfigJSON)), nil }, From 9c3d98d90b85f1e60c679a96a0e9a7b0e5935c41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Tue, 1 Aug 2023 00:38:40 +0200 Subject: [PATCH 11/13] Allow specifying the config fixture in newOCI1ImageSource MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Should not change (test) behavior for now. Signed-off-by: Miloslav Trmač --- internal/image/oci_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/internal/image/oci_test.go b/internal/image/oci_test.go index 0531f842ee..45a6bbe382 100644 --- a/internal/image/oci_test.go +++ b/internal/image/oci_test.go @@ -194,7 +194,7 @@ func TestManifestOCI1OCIConfig(t *testing.T) { err = json.Unmarshal(configJSON, &expectedConfig) require.NoError(t, err) - originalSrc := newOCI1ImageSource(t, "httpd:latest") + originalSrc := newOCI1ImageSource(t, "oci1-config.json", "httpd:latest") for _, m := range []genericManifest{ manifestOCI1FromFixture(t, originalSrc, "oci1.json"), manifestOCI1FromComponentsLikeFixture(configJSON), @@ -357,8 +357,8 @@ func (OCIis *oci1ImageSource) Reference() types.ImageReference { return refImageReferenceMock{ref: OCIis.ref} } -func newOCI1ImageSource(t *testing.T, dockerRef string) *oci1ImageSource { - realConfigJSON, err := os.ReadFile("fixtures/oci1-config.json") +func newOCI1ImageSource(t *testing.T, configFixture string, dockerRef string) *oci1ImageSource { + realConfigJSON, err := os.ReadFile(filepath.Join("fixtures", configFixture)) require.NoError(t, err) ref, err := reference.ParseNormalizedNamed(dockerRef) @@ -376,7 +376,7 @@ func newOCI1ImageSource(t *testing.T, dockerRef string) *oci1ImageSource { } func TestManifestOCI1UpdatedImage(t *testing.T) { - originalSrc := newOCI1ImageSource(t, "httpd:latest") + originalSrc := newOCI1ImageSource(t, "oci1-config.json", "httpd:latest") original := manifestOCI1FromFixture(t, originalSrc, "oci1.json") // LayerInfos: @@ -437,7 +437,7 @@ func TestManifestOCI1UpdatedImage(t *testing.T) { } func TestManifestOCI1ConvertToManifestSchema1(t *testing.T) { - originalSrc := newOCI1ImageSource(t, "httpd-copy:latest") + originalSrc := newOCI1ImageSource(t, "oci1-config.json", "httpd-copy:latest") original := manifestOCI1FromFixture(t, originalSrc, "oci1.json") memoryDest := &memoryImageDest{ref: originalSrc.ref} res, err := original.UpdatedImage(context.Background(), types.ManifestUpdateOptions{ @@ -506,7 +506,7 @@ func TestManifestOCI1ConvertToManifestSchema1(t *testing.T) { } func TestConvertToManifestSchema2(t *testing.T) { - originalSrc := newOCI1ImageSource(t, "httpd-copy:latest") + originalSrc := newOCI1ImageSource(t, "oci1-config.json", "httpd-copy:latest") original := manifestOCI1FromFixture(t, originalSrc, "oci1.json") res, err := original.UpdatedImage(context.Background(), types.ManifestUpdateOptions{ ManifestMIMEType: manifest.DockerV2Schema2MediaType, @@ -534,7 +534,7 @@ func TestConvertToManifestSchema2(t *testing.T) { } func TestConvertToManifestSchema2AllMediaTypes(t *testing.T) { - originalSrc := newOCI1ImageSource(t, "httpd-copy:latest") + originalSrc := newOCI1ImageSource(t, "oci1-config.json", "httpd-copy:latest") original := manifestOCI1FromFixture(t, originalSrc, "oci1-all-media-types.json") _, err := original.UpdatedImage(context.Background(), types.ManifestUpdateOptions{ ManifestMIMEType: manifest.DockerV2Schema2MediaType, @@ -543,7 +543,7 @@ func TestConvertToManifestSchema2AllMediaTypes(t *testing.T) { } func TestConvertToV2S2WithInvalidMIMEType(t *testing.T) { - originalSrc := newOCI1ImageSource(t, "httpd-copy:latest") + originalSrc := newOCI1ImageSource(t, "oci1-config.json", "httpd-copy:latest") manifest, err := os.ReadFile(filepath.Join("fixtures", "oci1-invalid-media-type.json")) require.NoError(t, err) From 6e8a5a7ff3ff6d58c07f2e1d875c822a57544623 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Tue, 1 Aug 2023 01:08:14 +0200 Subject: [PATCH 12/13] Add tests to ensure that extra fields in config.json are not rejected. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a MUST requirement from OCI image spec v1.1. Signed-off-by: Miloslav Trmač --- .../fixtures/oci1-config-extra-fields.json | 158 +++++++++++++ .../fixtures/oci1-extra-config-fields.json | 43 ++++ internal/image/oci_test.go | 214 ++++++++++-------- 3 files changed, 326 insertions(+), 89 deletions(-) create mode 100644 internal/image/fixtures/oci1-config-extra-fields.json create mode 100644 internal/image/fixtures/oci1-extra-config-fields.json diff --git a/internal/image/fixtures/oci1-config-extra-fields.json b/internal/image/fixtures/oci1-config-extra-fields.json new file mode 100644 index 0000000000..1d670d590b --- /dev/null +++ b/internal/image/fixtures/oci1-config-extra-fields.json @@ -0,0 +1,158 @@ +{ + "extra-string-field": "string", + "extra-object": {"foo":"bar"}, + "architecture": "amd64", + "config": { + "Hostname": "383850eeb47b", + "Domainname": "", + "User": "", + "AttachStdin": false, + "AttachStdout": false, + "AttachStderr": false, + "ExposedPorts": { + "80/tcp": {} + }, + "Tty": false, + "OpenStdin": false, + "StdinOnce": false, + "Env": [ + "PATH=/usr/local/apache2/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", + "HTTPD_PREFIX=/usr/local/apache2", + "HTTPD_VERSION=2.4.23", + "HTTPD_SHA1=5101be34ac4a509b245adb70a56690a84fcc4e7f", + "HTTPD_BZ2_URL=https://www.apache.org/dyn/closer.cgi?action=download\u0026filename=httpd/httpd-2.4.23.tar.bz2", + "HTTPD_ASC_URL=https://www.apache.org/dist/httpd/httpd-2.4.23.tar.bz2.asc" + ], + "Cmd": [ + "httpd-foreground" + ], + "ArgsEscaped": true, + "Image": "sha256:4f83530449c67c1ed8fca72583c5b92fdf446010990028c362a381e55dd84afd", + "Volumes": null, + "WorkingDir": "/usr/local/apache2", + "Entrypoint": null, + "OnBuild": [], + "Labels": {} + }, + "container": "8825acde1b009729807e4b70a65a89399dd8da8e53be9216b9aaabaff4339f69", + "container_config": { + "Hostname": "383850eeb47b", + "Domainname": "", + "User": "", + "AttachStdin": false, + "AttachStdout": false, + "AttachStderr": false, + "ExposedPorts": { + "80/tcp": {} + }, + "Tty": false, + "OpenStdin": false, + "StdinOnce": false, + "Env": [ + "PATH=/usr/local/apache2/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", + "HTTPD_PREFIX=/usr/local/apache2", + "HTTPD_VERSION=2.4.23", + "HTTPD_SHA1=5101be34ac4a509b245adb70a56690a84fcc4e7f", + "HTTPD_BZ2_URL=https://www.apache.org/dyn/closer.cgi?action=download\u0026filename=httpd/httpd-2.4.23.tar.bz2", + "HTTPD_ASC_URL=https://www.apache.org/dist/httpd/httpd-2.4.23.tar.bz2.asc" + ], + "Cmd": [ + "/bin/sh", + "-c", + "#(nop) ", + "CMD [\"httpd-foreground\"]" + ], + "ArgsEscaped": true, + "Image": "sha256:4f83530449c67c1ed8fca72583c5b92fdf446010990028c362a381e55dd84afd", + "Volumes": null, + "WorkingDir": "/usr/local/apache2", + "Entrypoint": null, + "OnBuild": [], + "Labels": {} + }, + "created": "2016-09-23T23:20:45.78976459Z", + "docker_version": "1.12.1", + "history": [ + { + "created": "2016-09-23T18:08:50.537223822Z", + "created_by": "/bin/sh -c #(nop) ADD file:c6c23585ab140b0b320d4e99bc1b0eb544c9e96c24d90fec5e069a6d57d335ca in / " + }, + { + "created": "2016-09-23T18:08:51.133779867Z", + "created_by": "/bin/sh -c #(nop) CMD [\"/bin/bash\"]", + "empty_layer": true + }, + { + "created": "2016-09-23T19:16:40.725768956Z", + "created_by": "/bin/sh -c #(nop) ENV HTTPD_PREFIX=/usr/local/apache2", + "empty_layer": true + }, + { + "created": "2016-09-23T19:16:41.037788416Z", + "created_by": "/bin/sh -c #(nop) ENV PATH=/usr/local/apache2/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", + "empty_layer": true + }, + { + "created": "2016-09-23T19:16:41.990121202Z", + "created_by": "/bin/sh -c mkdir -p \"$HTTPD_PREFIX\" \t\u0026\u0026 chown www-data:www-data \"$HTTPD_PREFIX\"" + }, + { + "created": "2016-09-23T19:16:42.339911155Z", + "created_by": "/bin/sh -c #(nop) WORKDIR /usr/local/apache2", + "empty_layer": true + }, + { + "created": "2016-09-23T19:16:54.948461741Z", + "created_by": "/bin/sh -c apt-get update \t\u0026\u0026 apt-get install -y --no-install-recommends \t\tlibapr1 \t\tlibaprutil1 \t\tlibaprutil1-ldap \t\tlibapr1-dev \t\tlibaprutil1-dev \t\tlibpcre++0 \t\tlibssl1.0.0 \t\u0026\u0026 rm -r /var/lib/apt/lists/*" + }, + { + "created": "2016-09-23T19:16:55.321573403Z", + "created_by": "/bin/sh -c #(nop) ENV HTTPD_VERSION=2.4.23", + "empty_layer": true + }, + { + "created": "2016-09-23T19:16:55.629947307Z", + "created_by": "/bin/sh -c #(nop) ENV HTTPD_SHA1=5101be34ac4a509b245adb70a56690a84fcc4e7f", + "empty_layer": true + }, + { + "created": "2016-09-23T23:19:03.705796801Z", + "created_by": "/bin/sh -c #(nop) ENV HTTPD_BZ2_URL=https://www.apache.org/dyn/closer.cgi?action=download\u0026filename=httpd/httpd-2.4.23.tar.bz2", + "empty_layer": true + }, + { + "created": "2016-09-23T23:19:04.009782822Z", + "created_by": "/bin/sh -c #(nop) ENV HTTPD_ASC_URL=https://www.apache.org/dist/httpd/httpd-2.4.23.tar.bz2.asc", + "empty_layer": true + }, + { + "created": "2016-09-23T23:20:44.585743332Z", + "created_by": "/bin/sh -c set -x \t\u0026\u0026 buildDeps=' \t\tbzip2 \t\tca-certificates \t\tgcc \t\tlibpcre++-dev \t\tlibssl-dev \t\tmake \t\twget \t' \t\u0026\u0026 apt-get update \t\u0026\u0026 apt-get install -y --no-install-recommends $buildDeps \t\u0026\u0026 rm -r /var/lib/apt/lists/* \t\t\u0026\u0026 wget -O httpd.tar.bz2 \"$HTTPD_BZ2_URL\" \t\u0026\u0026 echo \"$HTTPD_SHA1 *httpd.tar.bz2\" | sha1sum -c - \t\u0026\u0026 wget -O httpd.tar.bz2.asc \"$HTTPD_ASC_URL\" \t\u0026\u0026 export GNUPGHOME=\"$(mktemp -d)\" \t\u0026\u0026 gpg --keyserver ha.pool.sks-keyservers.net --recv-keys A93D62ECC3C8EA12DB220EC934EA76E6791485A8 \t\u0026\u0026 gpg --batch --verify httpd.tar.bz2.asc httpd.tar.bz2 \t\u0026\u0026 rm -r \"$GNUPGHOME\" httpd.tar.bz2.asc \t\t\u0026\u0026 mkdir -p src \t\u0026\u0026 tar -xvf httpd.tar.bz2 -C src --strip-components=1 \t\u0026\u0026 rm httpd.tar.bz2 \t\u0026\u0026 cd src \t\t\u0026\u0026 ./configure \t\t--prefix=\"$HTTPD_PREFIX\" \t\t--enable-mods-shared=reallyall \t\u0026\u0026 make -j\"$(nproc)\" \t\u0026\u0026 make install \t\t\u0026\u0026 cd .. \t\u0026\u0026 rm -r src \t\t\u0026\u0026 sed -ri \t\t-e 's!^(\\s*CustomLog)\\s+\\S+!\\1 /proc/self/fd/1!g' \t\t-e 's!^(\\s*ErrorLog)\\s+\\S+!\\1 /proc/self/fd/2!g' \t\t\"$HTTPD_PREFIX/conf/httpd.conf\" \t\t\u0026\u0026 apt-get purge -y --auto-remove $buildDeps" + }, + { + "created": "2016-09-23T23:20:45.127455562Z", + "created_by": "/bin/sh -c #(nop) COPY file:761e313354b918b6cd7ea99975a4f6b53ff5381ba689bab2984aec4dab597215 in /usr/local/bin/ " + }, + { + "created": "2016-09-23T23:20:45.453934921Z", + "created_by": "/bin/sh -c #(nop) EXPOSE 80/tcp", + "empty_layer": true + }, + { + "created": "2016-09-23T23:20:45.78976459Z", + "created_by": "/bin/sh -c #(nop) CMD [\"httpd-foreground\"]", + "empty_layer": true + } + ], + "os": "linux", + "rootfs": { + "type": "layers", + "diff_ids": [ + "sha256:142a601d97936307e75220c35dde0348971a9584c21e7cb42e1f7004005432ab", + "sha256:90fcc66ad3be9f1757f954b750deb37032f208428aa12599fcb02182b9065a9c", + "sha256:5a8624bb7e76d1e6829f9c64c43185e02bc07f97a2189eb048609a8914e72c56", + "sha256:d349ff6b3afc6a2800054768c82bfbf4289c9aa5da55c1290f802943dcd4d1e9", + "sha256:8c064bb1f60e84fa8cc6079b6d2e76e0423389fd6aeb7e497dfdae5e05b2b25b" + ] + } +} \ No newline at end of file diff --git a/internal/image/fixtures/oci1-extra-config-fields.json b/internal/image/fixtures/oci1-extra-config-fields.json new file mode 100644 index 0000000000..b297f4abcc --- /dev/null +++ b/internal/image/fixtures/oci1-extra-config-fields.json @@ -0,0 +1,43 @@ +{ + "schemaVersion": 2, + "mediaType": "application/vnd.oci.image.manifest.v1+json", + "config": { + "mediaType": "application/vnd.oci.image.config.v1+json", + "size": 7693, + "digest": "sha256:7f2a783ee2f07826b1856e68a40c930cd0430d6e7d4a88c29c2c8b7718706e74", + "annotations": { + "test-annotation-1": "one" + } + }, + "layers": [ + { + "mediaType": "application/vnd.oci.image.layer.v1.tar+gzip", + "size": 51354364, + "digest": "sha256:6a5a5368e0c2d3e5909184fa28ddfd56072e7ff3ee9a945876f7eee5896ef5bb" + }, + { + "mediaType": "application/vnd.oci.image.layer.v1.tar+gzip", + "size": 150, + "digest": "sha256:1bbf5d58d24c47512e234a5623474acf65ae00d4d1414272a893204f44cc680c" + }, + { + "mediaType": "application/vnd.oci.image.layer.v1.tar+gzip", + "size": 11739507, + "digest": "sha256:8f5dc8a4b12c307ac84de90cdd9a7f3915d1be04c9388868ca118831099c67a9", + "urls": ["https://layer.url"] + }, + { + "mediaType": "application/vnd.oci.image.layer.v1.tar+gzip", + "size": 8841833, + "digest": "sha256:bbd6b22eb11afce63cc76f6bc41042d99f10d6024c96b655dafba930b8d25909", + "annotations": { + "test-annotation-2": "two" + } + }, + { + "mediaType": "application/vnd.oci.image.layer.v1.tar+gzip", + "size": 291, + "digest": "sha256:960e52ecf8200cbd84e70eb2ad8678f4367e50d14357021872c10fa3fc5935fa" + } + ] +} diff --git a/internal/image/oci_test.go b/internal/image/oci_test.go index 45a6bbe382..70d96dcd6a 100644 --- a/internal/image/oci_test.go +++ b/internal/image/oci_test.go @@ -30,6 +30,40 @@ func manifestOCI1FromFixture(t *testing.T, src types.ImageSource, fixture string return m } +var layerDescriptorsLikeFixture = []imgspecv1.Descriptor{ + { + MediaType: imgspecv1.MediaTypeImageLayerGzip, + Digest: "sha256:6a5a5368e0c2d3e5909184fa28ddfd56072e7ff3ee9a945876f7eee5896ef5bb", + Size: 51354364, + }, + { + MediaType: imgspecv1.MediaTypeImageLayerGzip, + Digest: "sha256:1bbf5d58d24c47512e234a5623474acf65ae00d4d1414272a893204f44cc680c", + Size: 150, + }, + { + MediaType: imgspecv1.MediaTypeImageLayerGzip, + Digest: "sha256:8f5dc8a4b12c307ac84de90cdd9a7f3915d1be04c9388868ca118831099c67a9", + Size: 11739507, + URLs: []string{ + "https://layer.url", + }, + }, + { + MediaType: imgspecv1.MediaTypeImageLayerGzip, + Digest: "sha256:bbd6b22eb11afce63cc76f6bc41042d99f10d6024c96b655dafba930b8d25909", + Size: 8841833, + Annotations: map[string]string{ + "test-annotation-2": "two", + }, + }, + { + MediaType: imgspecv1.MediaTypeImageLayerGzip, + Digest: "sha256:960e52ecf8200cbd84e70eb2ad8678f4367e50d14357021872c10fa3fc5935fa", + Size: 291, + }, +} + func manifestOCI1FromComponentsLikeFixture(configBlob []byte) genericManifest { return manifestOCI1FromComponents(imgspecv1.Descriptor{ MediaType: imgspecv1.MediaTypeImageConfig, @@ -38,39 +72,20 @@ func manifestOCI1FromComponentsLikeFixture(configBlob []byte) genericManifest { Annotations: map[string]string{ "test-annotation-1": "one", }, - }, nil, configBlob, []imgspecv1.Descriptor{ - { - MediaType: imgspecv1.MediaTypeImageLayerGzip, - Digest: "sha256:6a5a5368e0c2d3e5909184fa28ddfd56072e7ff3ee9a945876f7eee5896ef5bb", - Size: 51354364, - }, - { - MediaType: imgspecv1.MediaTypeImageLayerGzip, - Digest: "sha256:1bbf5d58d24c47512e234a5623474acf65ae00d4d1414272a893204f44cc680c", - Size: 150, - }, - { - MediaType: imgspecv1.MediaTypeImageLayerGzip, - Digest: "sha256:8f5dc8a4b12c307ac84de90cdd9a7f3915d1be04c9388868ca118831099c67a9", - Size: 11739507, - URLs: []string{ - "https://layer.url", - }, - }, - { - MediaType: imgspecv1.MediaTypeImageLayerGzip, - Digest: "sha256:bbd6b22eb11afce63cc76f6bc41042d99f10d6024c96b655dafba930b8d25909", - Size: 8841833, - Annotations: map[string]string{ - "test-annotation-2": "two", - }, - }, - { - MediaType: imgspecv1.MediaTypeImageLayerGzip, - Digest: "sha256:960e52ecf8200cbd84e70eb2ad8678f4367e50d14357021872c10fa3fc5935fa", - Size: 291, + }, nil, configBlob, layerDescriptorsLikeFixture) +} + +func manifestOCI1FromComponentsWithExtraConfigFields(t *testing.T, src types.ImageSource) genericManifest { + configJSON, err := os.ReadFile("fixtures/oci1-config-extra-fields.json") + require.NoError(t, err) + return manifestOCI1FromComponents(imgspecv1.Descriptor{ + MediaType: imgspecv1.MediaTypeImageConfig, + Size: 7693, + Digest: "sha256:7f2a783ee2f07826b1856e68a40c930cd0430d6e7d4a88c29c2c8b7718706e74", + Annotations: map[string]string{ + "test-annotation-1": "one", }, - }) + }, src, configJSON, layerDescriptorsLikeFixture) } func TestManifestOCI1FromManifest(t *testing.T) { @@ -204,6 +219,19 @@ func TestManifestOCI1OCIConfig(t *testing.T) { assert.Equal(t, &expectedConfig, config) } + // “Any extra fields in the Image JSON struct are considered implementation specific + // and MUST NOT generate an error by any implementations which are unable to interpret them.” + // oci1-config-extra-fields.json is the same as oci1-config.json, apart from a few added fields. + srcWithExtraFields := newOCI1ImageSource(t, "oci1-config-extra-fields.json", "httpd:latest") + for _, m := range []genericManifest{ + manifestOCI1FromFixture(t, srcWithExtraFields, "oci1-extra-config-fields.json"), + manifestOCI1FromComponentsWithExtraConfigFields(t, srcWithExtraFields), + } { + config, err := m.OCIConfig(context.Background()) + require.NoError(t, err) + assert.Equal(t, &expectedConfig, config) + } + // This can share originalSrc because the config digest is the same between oci1-artifact.json and oci1.json artifact := manifestOCI1FromFixture(t, originalSrc, "oci1-artifact.json") _, err = artifact.OCIConfig(context.Background()) @@ -267,67 +295,75 @@ func TestManifestOCI1EmbeddedDockerReferenceConflicts(t *testing.T) { } func TestManifestOCI1Inspect(t *testing.T) { - configJSON, err := os.ReadFile("fixtures/oci1-config.json") - require.NoError(t, err) var emptyAnnotations map[string]string - m := manifestOCI1FromComponentsLikeFixture(configJSON) - ii, err := m.Inspect(context.Background()) - require.NoError(t, err) created := time.Date(2016, 9, 23, 23, 20, 45, 789764590, time.UTC) - assert.Equal(t, types.ImageInspectInfo{ - Tag: "", - Created: &created, - DockerVersion: "1.12.1", - Labels: map[string]string{}, - Architecture: "amd64", - Os: "linux", - Layers: []string{ - "sha256:6a5a5368e0c2d3e5909184fa28ddfd56072e7ff3ee9a945876f7eee5896ef5bb", - "sha256:1bbf5d58d24c47512e234a5623474acf65ae00d4d1414272a893204f44cc680c", - "sha256:8f5dc8a4b12c307ac84de90cdd9a7f3915d1be04c9388868ca118831099c67a9", - "sha256:bbd6b22eb11afce63cc76f6bc41042d99f10d6024c96b655dafba930b8d25909", - "sha256:960e52ecf8200cbd84e70eb2ad8678f4367e50d14357021872c10fa3fc5935fa", - }, - LayersData: []types.ImageInspectLayer{{ - MIMEType: "application/vnd.oci.image.layer.v1.tar+gzip", - Digest: "sha256:6a5a5368e0c2d3e5909184fa28ddfd56072e7ff3ee9a945876f7eee5896ef5bb", - Size: 51354364, - Annotations: emptyAnnotations, - }, { - MIMEType: "application/vnd.oci.image.layer.v1.tar+gzip", - Digest: "sha256:1bbf5d58d24c47512e234a5623474acf65ae00d4d1414272a893204f44cc680c", - Size: 150, - Annotations: emptyAnnotations, - }, { - MIMEType: "application/vnd.oci.image.layer.v1.tar+gzip", - Digest: "sha256:8f5dc8a4b12c307ac84de90cdd9a7f3915d1be04c9388868ca118831099c67a9", - Size: 11739507, - Annotations: emptyAnnotations, - }, { - MIMEType: "application/vnd.oci.image.layer.v1.tar+gzip", - Digest: "sha256:bbd6b22eb11afce63cc76f6bc41042d99f10d6024c96b655dafba930b8d25909", - Size: 8841833, - Annotations: map[string]string{"test-annotation-2": "two"}, - }, { - MIMEType: "application/vnd.oci.image.layer.v1.tar+gzip", - Digest: "sha256:960e52ecf8200cbd84e70eb2ad8678f4367e50d14357021872c10fa3fc5935fa", - Size: 291, - Annotations: emptyAnnotations, - }, - }, - Author: "", - Env: []string{ - "PATH=/usr/local/apache2/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", - "HTTPD_PREFIX=/usr/local/apache2", - "HTTPD_VERSION=2.4.23", - "HTTPD_SHA1=5101be34ac4a509b245adb70a56690a84fcc4e7f", - "HTTPD_BZ2_URL=https://www.apache.org/dyn/closer.cgi?action=download&filename=httpd/httpd-2.4.23.tar.bz2", - "HTTPD_ASC_URL=https://www.apache.org/dist/httpd/httpd-2.4.23.tar.bz2.asc", - }, - }, *ii) + + configJSON, err := os.ReadFile("fixtures/oci1-config.json") + require.NoError(t, err) + for _, m := range []genericManifest{ + manifestOCI1FromComponentsLikeFixture(configJSON), + // “Any extra fields in the Image JSON struct are considered implementation specific + // and MUST NOT generate an error by any implementations which are unable to interpret them.” + // oci1-config-extra-fields.json is the same as oci1-config.json, apart from a few added fields. + manifestOCI1FromComponentsWithExtraConfigFields(t, nil), + } { + ii, err := m.Inspect(context.Background()) + require.NoError(t, err) + assert.Equal(t, types.ImageInspectInfo{ + Tag: "", + Created: &created, + DockerVersion: "1.12.1", + Labels: map[string]string{}, + Architecture: "amd64", + Os: "linux", + Layers: []string{ + "sha256:6a5a5368e0c2d3e5909184fa28ddfd56072e7ff3ee9a945876f7eee5896ef5bb", + "sha256:1bbf5d58d24c47512e234a5623474acf65ae00d4d1414272a893204f44cc680c", + "sha256:8f5dc8a4b12c307ac84de90cdd9a7f3915d1be04c9388868ca118831099c67a9", + "sha256:bbd6b22eb11afce63cc76f6bc41042d99f10d6024c96b655dafba930b8d25909", + "sha256:960e52ecf8200cbd84e70eb2ad8678f4367e50d14357021872c10fa3fc5935fa", + }, + LayersData: []types.ImageInspectLayer{{ + MIMEType: "application/vnd.oci.image.layer.v1.tar+gzip", + Digest: "sha256:6a5a5368e0c2d3e5909184fa28ddfd56072e7ff3ee9a945876f7eee5896ef5bb", + Size: 51354364, + Annotations: emptyAnnotations, + }, { + MIMEType: "application/vnd.oci.image.layer.v1.tar+gzip", + Digest: "sha256:1bbf5d58d24c47512e234a5623474acf65ae00d4d1414272a893204f44cc680c", + Size: 150, + Annotations: emptyAnnotations, + }, { + MIMEType: "application/vnd.oci.image.layer.v1.tar+gzip", + Digest: "sha256:8f5dc8a4b12c307ac84de90cdd9a7f3915d1be04c9388868ca118831099c67a9", + Size: 11739507, + Annotations: emptyAnnotations, + }, { + MIMEType: "application/vnd.oci.image.layer.v1.tar+gzip", + Digest: "sha256:bbd6b22eb11afce63cc76f6bc41042d99f10d6024c96b655dafba930b8d25909", + Size: 8841833, + Annotations: map[string]string{"test-annotation-2": "two"}, + }, { + MIMEType: "application/vnd.oci.image.layer.v1.tar+gzip", + Digest: "sha256:960e52ecf8200cbd84e70eb2ad8678f4367e50d14357021872c10fa3fc5935fa", + Size: 291, + Annotations: emptyAnnotations, + }, + }, + Author: "", + Env: []string{ + "PATH=/usr/local/apache2/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", + "HTTPD_PREFIX=/usr/local/apache2", + "HTTPD_VERSION=2.4.23", + "HTTPD_SHA1=5101be34ac4a509b245adb70a56690a84fcc4e7f", + "HTTPD_BZ2_URL=https://www.apache.org/dyn/closer.cgi?action=download&filename=httpd/httpd-2.4.23.tar.bz2", + "HTTPD_ASC_URL=https://www.apache.org/dist/httpd/httpd-2.4.23.tar.bz2.asc", + }, + }, *ii) + } // nil configBlob will trigger an error in m.ConfigBlob() - m = manifestOCI1FromComponentsLikeFixture(nil) + m := manifestOCI1FromComponentsLikeFixture(nil) _, err = m.Inspect(context.Background()) assert.Error(t, err) From 5253f0137492d2e181f6b364ad36794ff40e3f02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Mon, 7 Aug 2023 21:49:11 +0200 Subject: [PATCH 13/13] Fix a possible division by zero MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Zero-layer artifacts are discouraged but allowed. Signed-off-by: Miloslav Trmač --- copy/single.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/copy/single.go b/copy/single.go index dff7aad22f..bb58ae6531 100644 --- a/copy/single.go +++ b/copy/single.go @@ -460,8 +460,14 @@ func (ic *imageCopier) copyLayers(ctx context.Context) ([]compressiontypes.Algor encryptAll = len(*ic.c.options.OciEncryptLayers) == 0 totalLayers := len(srcInfos) for _, l := range *ic.c.options.OciEncryptLayers { - // if layer is negative, it is reverse indexed. - layersToEncrypt.Add((totalLayers + l) % totalLayers) + switch { + case l >= 0 && l < totalLayers: + layersToEncrypt.Add(l) + case l < 0 && l+totalLayers >= 0: // Implies (l + totalLayers) < totalLayers + layersToEncrypt.Add(l + totalLayers) // If l is negative, it is reverse indexed. + default: + return nil, fmt.Errorf("when choosing layers to encrypt, layer index %d out of range (%d layers exist)", l, totalLayers) + } } if encryptAll {