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

oras copy error message is not clear enough #432

Closed
JeyJeyGao opened this issue Feb 6, 2023 · 2 comments · Fixed by #526
Closed

oras copy error message is not clear enough #432

JeyJeyGao opened this issue Feb 6, 2023 · 2 comments · Fixed by #526
Labels
enhancement New feature or request
Milestone

Comments

@JeyJeyGao
Copy link

I manully modified the size field of the index.json of OCI layout file to be an invalid value and got this error message.

oras cp --from-oci-layout out:latest --to-oci-layout out3:latest -d
Error: read failed: unexpected EOF

index.json file

{
  "schemaVersion": 2,
  "manifests": [
    {
      "mediaType": "application/vnd.oci.image.manifest.v1+json",
      "digest": "sha256:ff43fa9e6ee876230e679f2ff7d2c61f3c43ca437d3cf46b93c28c2231ec9773",
      "size": 12401, // invalid size
      "annotations": {
        "io.containerd.image.name": "docker.io/library/latest:latest",
        "org.opencontainers.image.created": "2023-01-30T02:26:00Z",
        "org.opencontainers.image.ref.name": "latest"
      },
      "platform": {
        "architecture": "amd64",
        "os": "linux"
      }
    }
  ]
}

It looks like the error message is not clear enough.

@TerryHowe
Copy link
Member

Do you have any suggestions on what would be more clear?

@shizhMSFT shizhMSFT added the enhancement New feature or request label Feb 6, 2023
@shizhMSFT shizhMSFT added this to the v2.1.0 milestone Feb 6, 2023
@JeyJeyGao
Copy link
Author

Do you have any suggestions on what would be more clear?

It looks like it did not validate the size otherwise it will return a more reasonable error. I think it should tell me what is not correct. For example, the oci layout is invalid, the index.json is invalid or the size is invalid.

@shizhMSFT shizhMSFT modified the milestones: v2.1.0, v2.2.0 May 4, 2023
@Wwwsylvia Wwwsylvia modified the milestones: v2.2.0, v2.3.0 May 24, 2023
Wwwsylvia pushed a commit that referenced this issue Jun 27, 2023
NewWithContext refers to both the `oci-layout` and `index.json` as "OCI
Image Layout", this PR fixes the latter.

ReadAll is a deeper change that will improve error reporting for a lot
of cases, at the cost of error volume. Some or all of that change could
happen higher up to satisfy this particular issue. I think the
expected/actual size output belongs here. There seem to be more than a
few code paths in this and other repos that end up calling ReadAll
multiple times without reporting which call produced an error so I think
it's appropriate to also output the digest here, but that's less
compelling.

The Verify comment clears up confusion I encountered while working on
this issue. "verifies size" implies checking for both too big AND too
small, but it only checks if the file is bigger (or reader is longer)
than expected.

Fixes #432

Signed-off-by: Clarence "Sparr" Risher <clrnc@amazon.com>
Wwwsylvia pushed a commit to Wwwsylvia/oras-go that referenced this issue Jul 4, 2023
NewWithContext refers to both the `oci-layout` and `index.json` as "OCI
Image Layout", this PR fixes the latter.

ReadAll is a deeper change that will improve error reporting for a lot
of cases, at the cost of error volume. Some or all of that change could
happen higher up to satisfy this particular issue. I think the
expected/actual size output belongs here. There seem to be more than a
few code paths in this and other repos that end up calling ReadAll
multiple times without reporting which call produced an error so I think
it's appropriate to also output the digest here, but that's less
compelling.

The Verify comment clears up confusion I encountered while working on
this issue. "verifies size" implies checking for both too big AND too
small, but it only checks if the file is bigger (or reader is longer)
than expected.

Fixes oras-project#432

Signed-off-by: Clarence "Sparr" Risher <clrnc@amazon.com>
shizhMSFT pushed a commit that referenced this issue Jul 4, 2023
NewWithContext refers to both the `oci-layout` and `index.json` as "OCI
Image Layout", this PR fixes the latter.

ReadAll is a deeper change that will improve error reporting for a lot
of cases, at the cost of error volume. Some or all of that change could
happen higher up to satisfy this particular issue. I think the
expected/actual size output belongs here. There seem to be more than a
few code paths in this and other repos that end up calling ReadAll
multiple times without reporting which call produced an error so I think
it's appropriate to also output the digest here, but that's less
compelling.

The Verify comment clears up confusion I encountered while working on
this issue. "verifies size" implies checking for both too big AND too
small, but it only checks if the file is bigger (or reader is longer)
than expected.

Fixes #432

Signed-off-by: Clarence "Sparr" Risher <clrnc@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

4 participants