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

docs: improve formatted output for oras discover #1625

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

FeynmanZhou
Copy link
Member

@FeynmanZhou FeynmanZhou commented Feb 18, 2025

What this PR does / why we need it:

This PR improves formatted output for oras discover. For example, if the referrers have associated referrers, ORAS should be able to show the manifest content of the subject image and all referrers.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #1403

Please check the following list:

  • Does the affected code have corresponding tests, e.g. unit test, E2E test?
  • Does this change require a documentation update?
  • Does this introduce breaking changes that would require an announcement or bumping the major version?
  • Do all new files have an appropriate license header?

Copy link

codecov bot commented Feb 18, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.37%. Comparing base (0abb460) to head (2a7e9d2).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1625      +/-   ##
==========================================
- Coverage   84.41%   84.37%   -0.04%     
==========================================
  Files         120      120              
  Lines        5460     5460              
==========================================
- Hits         4609     4607       -2     
- Misses        604      606       +2     
  Partials      247      247              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

- `size`: referrer file size in bytes
- `artifactType`: artifact type of a referrer
- `annotations`: contains arbitrary metadata in a referrer
- `referrerManifests`: the list of referrers' manifest
Copy link
Contributor

@sajayantony sajayantony Feb 19, 2025

Choose a reason for hiding this comment

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

What about manifests or referrers basically the schema type is the type of the root object.

Copy link
Member

Choose a reason for hiding this comment

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

In terms of consistency, manifests actually makes more sense as the schema is the same, just that users may not understand what does "manifests" mean. Maybe we should have called the root manifests referrers in the first place😂...

Copy link
Member Author

Choose a reason for hiding this comment

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

We will use referrers as the field name in those two places based on the community discussion.

- `size`: referrer file size in bytes
- `artifactType`: artifact type of a referrer
- `annotations`: contains arbitrary metadata in a referrer
- `referrerManifests`: the list of referrers' manifest
Copy link
Contributor

Choose a reason for hiding this comment

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

What if this field becomes a field of the manifest?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes the ORAS manifest output schema can have an optional referrers which is a self referencing type.

oras discover localhost:5000/kubernetes/kubectl@sha256:bece4f4746a39cb39e38451c70fa5a1e5ea4fa20d4cca40136b51d9557918b01 --format json --recursive
```

```
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: this code block is not a valid json

- `annotations`: contains arbitrary metadata in a referrer
- `referrerManifests`: the list of referrers' manifest

For example, when there are two refferers lifecycle metadata and in-toto attestation associated with a sample image, the signatures are associated with these two files respectively. The output in a tree view will be:
Copy link
Contributor

Choose a reason for hiding this comment

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

There are typos: refferers --> referrers.

When showing the subject image and all referrers' manifests recursively in a pretty JSON output, the following JSON output should be returned:

```bash
oras discover localhost:5000/kubernetes/kubectl@sha256:bece4f4746a39cb39e38451c70fa5a1e5ea4fa20d4cca40136b51d9557918b01 --format json --recursive
Copy link
Contributor

@sajayantony sajayantony Feb 19, 2025

Choose a reason for hiding this comment

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

+1 to @Wwwsylvia's comment - --recursive is default (true) in tree and not in -format json sounds ood.
flag defaults changing depending on output formats seems really odd.

When showing the subject image and all referrers' manifests recursively in a pretty JSON output, the following JSON output should be returned:

```bash
oras discover localhost:5000/kubernetes/kubectl@sha256:bece4f4746a39cb39e38451c70fa5a1e5ea4fa20d4cca40136b51d9557918b01 --format json --recursive
Copy link
Contributor

Choose a reason for hiding this comment

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

As @Wwwsylvia pointed out, the --recursive flag is newly introduced and may be inconsistent for other formats.

Copy link
Contributor

Choose a reason for hiding this comment

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

Instead, we can introduce a --depth flag.

When showing the subject image and all referrers' manifests recursively in a pretty JSON output, the following JSON output should be returned:

```bash
oras discover localhost:5000/kubernetes/kubectl@sha256:bece4f4746a39cb39e38451c70fa5a1e5ea4fa20d4cca40136b51d9557918b01 --format json --recursive
Copy link

@yizha1 yizha1 Feb 19, 2025

Choose a reason for hiding this comment

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

Is there a need to set a limit on the recursive depth?

@FeynmanZhou
Copy link
Member Author

FeynmanZhou commented Feb 19, 2025

Based on the community discussion on Feb 18, @oras-project/oras-cli ORAS maintainers agreed on these things:

  • Show root image and its all referrers in the JSON output by default. This ensures data consistency among other data format output (e.g. tree, table)
  • Use referrers as the field name in the top-level and second-level referrers, instead of manifests
  • Show the referrers with full depth by default, introduce a flag --depth to allow users to limit the maximum depth of referrers in the formatted output.
  • ORAS will not introduce an additional flag to limit the number of referrers now since there is no clear performace concern so far. ORAS can provide the automatic timeout mechanism to users in case too many referrers with a root image.
  • Use JSON Schema and text to list the fields in formatted output

@shizhMSFT shizhMSFT changed the title doc: improve formatted output for oras discover docs: improve formatted output for oras discover Feb 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants