Skip to content

feat: show annotations in default discovery output #1602

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 11 commits into from
Apr 16, 2025
Merged

feat: show annotations in default discovery output #1602

merged 11 commits into from
Apr 16, 2025

Conversation

Horiodino
Copy link
Contributor

What this PR does / why we need it:

This PR introduces a new --format tree-full flag for the oras discover command, as outlined in the ORAS diagnose experience proposal. The flag enables users to control the level of metadata output when discovering artifacts, including detailed annotations. By adding this feature, users will be able to print more granular metadata about the artifacts, which will improve the overall diagnostic and troubleshooting experience.

Which issue(s) this PR fixes *
Fixes #1534

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?

TerryHowe
TerryHowe reviewed Jan 6, 2025
View reviewed changes
@shizhMSFT shizhMSFT changed the title added --format tree-full flag feat: add --format tree-full flag Jan 7, 2025
@shizhMSFT shizhMSFT changed the title feat: add --format tree-full flag feat: add --format tree-full flag Jan 7, 2025
@Horiodino Horiodino requested a review from TerryHowe January 8, 2025 11:47
@Horiodino Horiodino requested a review from Wwwsylvia as a code owner January 8, 2025 12:18
@TerryHowe
Copy link
Member

The title of this PR needs to be updated when you get a chance

@Horiodino Horiodino changed the title feat: add --format tree-full flag feat: Show annotations in default discovery output Jan 8, 2025
@TerryHowe
Copy link
Member

This broke e2e

Summarizing 4 Failures:
  [FAIL] OCI image layout users: when running discover command with tree output [It] should discover all referrers of a subject with annotations
  /home/runner/work/oras/oras/test/e2e/internal/utils/match/keywords.go:49
  [FAIL] 1.0 registry users: when running discover command [It] should discover all referrers with annotation via tree output
  /home/runner/work/oras/oras/test/e2e/internal/utils/match/keywords.go:49
  [FAIL] 1.1 registry users: when running discover command with tree output [It] should display <unknown> if a referrer has an empty artifact type
  /home/runner/work/oras/oras/test/e2e/internal/utils/match/keywords.go:49
  [FAIL] 1.1 registry users: when running discover command with tree output [It] should discover all referrers of a subject with annotations
  /home/runner/work/oras/oras/test/e2e/internal/utils/match/keywords.go:49

@Horiodino Horiodino force-pushed the format-tree-full branch 2 times, most recently from 028a4c5 to 5c3eb6b Compare February 27, 2025 15:19
@Horiodino
Copy link
Contributor Author

im continously encounting this err , any idea ? https://github.com/oras-project/oras/blob/main/test/e2e/internal/utils/init.go#L128

  Captured StdOut/StdErr Output >>
  Testing based on temp binary locates in "/tmp/gexec_artifacts1720431950/g681540619/oras"
  << Captured StdOut/StdErr Output

  [FAILED] Unexpected error:
      <*exec.ExitError | 0xc00007cd20>: 
      exit status 1
      {
          ProcessState: {
              pid: 476515,
              status: 256,
              rusage: {
                  Utime: {Sec: 0, Usec: 7093},
                  Stime: {Sec: 0, Usec: 3523},
                  Maxrss: 14084,
                  Ixrss: 0,
                  Idrss: 0,
                  Isrss: 0,
                  Minflt: 1002,
                  Majflt: 0,
                  Nswap: 0,
                  Inblock: 0,
                  Oublock: 0,
                  Msgsnd: 0,
                  Msgrcv: 0,
                  Nsignals: 0,
                  Nvcsw: 95,
                  Nivcsw: 57,
              },
          },
          Stderr: nil,
      }
  occurred
  In [BeforeSuite] at: /home/yoi/oras/test/e2e/internal/utils/init.go:128 @ 02/27/25 22:47:34.467

@Wwwsylvia
Copy link
Member

@Horiodino Running E2E tests on local could be buggy sometimes. Can you try removing the zot container, restoring everything generated under the e2e folder, and run again?

Wwwsylvia
Wwwsylvia requested changes Mar 7, 2025
View reviewed changes
@Wwwsylvia
Copy link
Member

@Horiodino Could you also share a screenshot before and after updating the command?

@Horiodino
Copy link
Contributor Author 

[yoi@yoi oras]$ go run cmd/oras/main.go discover --no-tty mcr.microsoft.com/oss/deislabs/ratify-base:v1.2.3
mcr.microsoft.com/oss/deislabs/ratify-base@sha256:747e64865b0ec17018e50561714293a10cfd7faa6d9ca55218ce511c99960724
├── application/vnd.in-toto+json
│   └── sha256:330f67e4d8e9db9ab47fc103aff2aa5755dd3a5d6aaa6b6e8dc78bb1d07d25b2
│       ├── [annotations]
│       │   └── org.opencontainers.image.created: 2025-01-28T03:46:25Z
│       └── application/vnd.cncf.notary.signature
│           └── sha256:6a99e0d2706731106749cf87eaa1bac804a606fcb3ae75e6d7b120aadae33ecb
│               └── [annotations]
│                   ├── io.cncf.notary.x509chain.thumbprint#S256: [58b56ba9b4fe67646e45a882373a8eea761b5ea3182ee81f23151d6211a0f9b3,9b1894f223d934cbd6575af3c6e1f6096b9221a7da132185f5a5cdc92235b5dc,23ffe2b8bdb9a1711515d4cffda04bc7f793d513c76c243f1020507d8669b7db]
│                   └── org.opencontainers.image.created: 2025-01-28T03:48:15Z
├── application/spdx+json
│   ├── sha256:773d87e71c7d7c585126b658dc29f8d3afca2646956afd9f538a567a829f4219
│   │   ├── [annotations]
│   │   │   └── org.opencontainers.image.created: 2025-01-28T03:44:11Z
│   │   └── application/vnd.cncf.notary.signature
│   │       └── sha256:f7afd15611dc9bea17c2ee1813a56a2dbe96bd28ca8888881e3669474dc62667
│   │           └── [annotations]
│   │               ├── io.cncf.notary.x509chain.thumbprint#S256: [58b56ba9b4fe67646e45a882373a8eea761b5ea3182ee81f23151d6211a0f9b3,9b1894f223d934cbd6575af3c6e1f6096b9221a7da132185f5a5cdc92235b5dc,23ffe2b8bdb9a1711515d4cffda04bc7f793d513c76c243f1020507d8669b7db]
│   │               └── org.opencontainers.image.created: 2025-01-28T03:46:02Z
│   └── sha256:003ce017b2bf38b29baf80fc8e712143633be5b621a27b836482d9c238892f10
│       ├── [annotations]
│       │   └── org.opencontainers.image.created: 2025-01-28T03:44:10Z
│       └── application/vnd.cncf.notary.signature
│           └── sha256:087d4a575dd28d20773412ac631854e169676100130770e374b1da91f36ef957
│               └── [annotations]
│                   ├── io.cncf.notary.x509chain.thumbprint#S256: [99f9dee8cba8cd8acb36f752b5cd085ba56a40dad1ca9d4eed0c3c10aaf8c7e5,9b1894f223d934cbd6575af3c6e1f6096b9221a7da132185f5a5cdc92235b5dc,23ffe2b8bdb9a1711515d4cffda04bc7f793d513c76c243f1020507d8669b7db]
│                   └── org.opencontainers.image.created: 2025-01-28T03:46:00Z
└── application/vnd.cncf.notary.signature
    └── sha256:749c5528c82c13b0a18331b80a8ea5faa22551b36172f1d83ecef70584dfec5f
        └── [annotations]
            ├── io.cncf.notary.x509chain.thumbprint#S256: [99f9dee8cba8cd8acb36f752b5cd085ba56a40dad1ca9d4eed0c3c10aaf8c7e5,9b1894f223d934cbd6575af3c6e1f6096b9221a7da132185f5a5cdc92235b5dc,23ffe2b8bdb9a1711515d4cffda04bc7f793d513c76c243f1020507d8669b7db]
            └── org.opencontainers.image.created: 2025-01-28T03:43:55Z

@Wwwsylvia
Copy link
Member

@Horiodino Could you fix the conflict?

@codecov Codecov
Copy link

codecov bot commented Mar 15, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.77%. Comparing base (c257dee) to head (dd314b4).
Report is 1 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1602      +/-   ##
==========================================
+ Coverage   84.70%   84.77%   +0.06%     
==========================================
  Files         126      126              
  Lines        5683     5702      +19     
==========================================
+ Hits         4814     4834      +20     
+ Misses        619      618       -1     
  Partials      250      250

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Wwwsylvia
Wwwsylvia requested changes Mar 18, 2025
View reviewed changes
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.

Can we also mark --output and --verbose as deprecated and hide them from the help manual, like we did for other commands?

Wwwsylvia
Wwwsylvia requested changes Mar 20, 2025
View reviewed changes
@Horiodino
show annotations in default discovery output
f5bac02 
Signed-off-by: Horiodino <holiodin@gmail.com>

added test case

Signed-off-by: Horiodino <holiodin@gmail.com>

added test case

Signed-off-by: Horiodino <holiodin@gmail.com>

added test case

Signed-off-by: Horiodino <holiodin@gmail.com>

updated test case

Signed-off-by: Horiodino <holiodin@gmail.com>
@Horiodino Horiodino requested a review from Wwwsylvia March 20, 2025 13:21
Wwwsylvia
Wwwsylvia reviewed Mar 25, 2025
View reviewed changes
Copy link
Member

@Wwwsylvia Wwwsylvia left a comment •
edited
Loading

Choose a reason for hiding this comment

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

We also need some unit tests to meet the patch coverage. @Horiodino Can I work on your branch?

@Wwwsylvia
add color and ut
33128de 
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
@Wwwsylvia
update
aa6d9d0 
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
@Wwwsylvia
Copy link
Member

We will need to refactor this once #1653 gets merged

@Wwwsylvia
fix license
3a11105 
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
@Wwwsylvia
clean up
9619919 
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
@Wwwsylvia
Copy link
Member

Current display with TTY:
image

Without TTY:
image

@FeynmanZhou Can you check if this looks good?

@Wwwsylvia
Copy link
Member

#1653 has been merged. We now need to merge the main branch and refactor this PR.

@Wwwsylvia
Copy link
Member

Hi @Horiodino, @wangxiaoxuan273, who is an active ORAS contributor, can help refactor the PR and resolve the conflicts. But she needs to be added as a collaborator to your forked repo to get the push permission. Would you mind setting up the permissions on your side? It can be done by following this instruction.

image

@wangxiaoxuan273
Copy link
Contributor

Hi @Horiodino, I'm interested in doing the refactoring and it would be nice if you can give me the permission so that I can directly push to your branch. It will speed up the merge process of this PR.

Xiaoxuan Wang added 5 commits April 11, 2025 16:41
test push
8a4b2d0 
Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>
rebased discover.go
54b8d68 
Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>
rebase in progress
6afd672 
Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>
Merge remote-tracking branch 'upstream/main' into format-tree-full
dec16f4
add back verbose
7d05e2e 
Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>
@FeynmanZhou
Copy link
Member

Per the dicussion on ORAS community meeting on Apr 15, 2025, maintainers ageed on the UX in the screenshot above.

Wwwsylvia
Wwwsylvia approved these changes Apr 16, 2025
View reviewed changes
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 changed the title feat: Show annotations in default discovery output feat: show annotations in default discovery output Apr 16, 2025
shizhMSFT
shizhMSFT approved these changes Apr 16, 2025
View reviewed changes
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

@shizhMSFT shizhMSFT merged commit d29e8e9 into oras-project:main Apr 16, 2025
8 checks passed
@Wwwsylvia Wwwsylvia mentioned this pull request Apr 24, 2025
Merged
6 tasks
FeynmanZhou pushed a commit to FeynmanZhou/oras that referenced this pull request May 28, 2025
@Horiodino @Wwwsylvia @FeynmanZhou
feat: show annotations in default discovery output (oras-project#1602)
6418481 
Signed-off-by: Horiodino <holiodin@gmail.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>
Co-authored-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Co-authored-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>
@Wwwsylvia Wwwsylvia mentioned this pull request Sep 3, 2025
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FeynmanZhou FeynmanZhou FeynmanZhou left review comments

@Wwwsylvia Wwwsylvia Wwwsylvia approved these changes

@shizhMSFT shizhMSFT shizhMSFT approved these changes

@sajayantony sajayantony Awaiting requested review from sajayantony sajayantony is a code owner

@SteveLasker SteveLasker Awaiting requested review from SteveLasker SteveLasker is a code owner

@qweeah qweeah Awaiting requested review from qweeah qweeah is a code owner

@TerryHowe TerryHowe Awaiting requested review from TerryHowe TerryHowe is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

show annotations by default for oras discover --format tree
6 participants
@Horiodino @TerryHowe @Wwwsylvia @wangxiaoxuan273 @FeynmanZhou @shizhMSFT