Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: change foreign layer error message at repository.Fetch() #377

Merged
merged 2 commits into from
Dec 8, 2022
Merged

feat: change foreign layer error message at repository.Fetch() #377

merged 2 commits into from
Dec 8, 2022

Conversation

wangxiaoxuan273
Copy link
Contributor

@wangxiaoxuan273 wangxiaoxuan273 commented Dec 7, 2022

Resolves #319

Current result with oras-cli:

$ oras copy docker.io/library/hello-world@sha256:314cc0309465c7be2936df274b258c843f5a49eb21f63b719d60b51564070b8f wxxacr.azurecr.io/temp439:test00

$ Error: the artifact with foreign layer sha256:5ead999142ecce15e02523c49706a633fa708374d94bb65a254e3a3c117d609b is not supported: sha256:5ead999142ecce15e02523c49706a633fa708374d94bb65a254e3a3c117d609b: not found

Signed-off-by: wangxiaoxuan273 wangxiaoxuan119@gmail.com

@codecov-commenter
Copy link

codecov-commenter commented Dec 7, 2022

Codecov Report

Merging #377 (617f44f) into main (ff05195) will increase coverage by 1.59%.
The diff coverage is 34.48%.

❗ Current head 617f44f differs from pull request most recent head dd68802. Consider uploading reports for the commit dd68802 to get more accurate results

@@            Coverage Diff             @@
##             main     #377      +/-   ##
==========================================
+ Coverage   72.24%   73.84%   +1.59%     
==========================================
  Files          42       45       +3     
  Lines        4093     4339     +246     
==========================================
+ Hits         2957     3204     +247     
+ Misses        855      844      -11     
- Partials      281      291      +10     
Impacted Files Coverage Δ
internal/syncutil/limitgroup.go 0.00% <0.00%> (ø)
registry/remote/repository.go 76.15% <53.84%> (+7.56%) ⬆️
content.go 74.55% <100.00%> (-1.20%) ⬇️
content/oci/readonlystorage.go 93.93% <0.00%> (ø)
content/oci/readonlyoci.go 66.12% <0.00%> (ø)
content/oci/storage.go 64.95% <0.00%> (+2.85%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

errdef/errors.go Outdated Show resolved Hide resolved
registry/remote/repository.go Outdated Show resolved Hide resolved
registry/remote/repository.go Outdated Show resolved Hide resolved
errdef/errors.go Outdated Show resolved Hide resolved
registry/remote/repository.go Outdated Show resolved Hide resolved
@qweeah
Copy link
Contributor

qweeah commented Dec 7, 2022

Some repositories may contain foreign layers which pointing to the repository itself, e.g.

> oras manifest fetch mcr.microsoft.com/windows/servercore:1607-KB5019964 --pretty --platform windows/amd64
{
  "schemaVersion": 2,
  "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
  "config": {
    "mediaType": "application/vnd.docker.container.image.v1+json",
    "size": 787,
    "digest": "sha256:f443e6ff891920258e9c0ce66f7f5472793b26691ea18d7fc87af0472c677430"
  },
  "layers": [
    {
      "mediaType": "application/vnd.docker.image.rootfs.foreign.diff.tar.gzip",
      "size": 4069985900,
      "digest": "sha256:3889bb8d808bbae6fa5a33e07093e65c31371bcf9e4c38c21be6b9af52ad1548",
      "urls": [
        "https://mcr.microsoft.com/v2/windows/servercore/blobs/sha256:3889bb8d808bbae6fa5a33e07093e65c31371bcf9e4c38c21be6b9af52ad1548"
      ]
    },
    {
      "mediaType": "application/vnd.docker.image.rootfs.foreign.diff.tar.gzip",
      "size": 2213621320,
      "digest": "sha256:29226bae1723379e75abf7d7da45bbf67d5d730611c58bf86df104245db30204",
      "urls": [
        "https://mcr.microsoft.com/v2/windows/servercore/blobs/sha256:29226bae1723379e75abf7d7da45bbf67d5d730611c58bf86df104245db30204"
      ]
    }
  ]
}

So it's better to return unsupported only when a layer is foreign and not found. Same logic should apply to OCI target.

internal/descriptor/descriptor.go Show resolved Hide resolved
copy.go Outdated Show resolved Hide resolved
content.go Outdated Show resolved Hide resolved
Signed-off-by: wangxiaoxuan273 <wangxiaoxuan119@gmail.com>
Signed-off-by: wangxiaoxuan273 <wangxiaoxuan119@gmail.com>
Copy link
Contributor

@shizhMSFT shizhMSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@Wwwsylvia Wwwsylvia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@shizhMSFT shizhMSFT merged commit 8500e54 into oras-project:main Dec 8, 2022
@wangxiaoxuan273 wangxiaoxuan273 deleted the foreign branch December 8, 2022 13:24
shizhMSFT added a commit that referenced this pull request Dec 9, 2022
Resolves #319
Related to #377

Signed-off-by: Shiwei Zhang <shizh@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: improve the error message if non-distributable(foreign) layer is not supported
6 participants