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!: support showing indirect referrers for all formats of oras discover #1653

Merged
merged 12 commits into from
Mar 27, 2025

Conversation

wangxiaoxuan273
Copy link
Contributor

@wangxiaoxuan273 wangxiaoxuan273 commented Mar 12, 2025

What this PR does / why we need it:
This pull request:

  1. adds a --depth flag to oras discover command and it applies to all formats. It represents the depth of referrers to display. For example when depth is 1, the command shows only direct referrers; when depth is 2, it shows direct referrers and their referrers. Default is 20, which in practice usually shows all referrers of an image.
  2. introduces breaking changes to table and json formats, which previously was showing only the direct referrers. Now all formats by default show all levels of 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):
Part of #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?

@wangxiaoxuan273 wangxiaoxuan273 changed the title feat: support indirect referrers for all formats of oras discover feat!: support indirect referrers for all formats of oras discover Mar 12, 2025
@wangxiaoxuan273 wangxiaoxuan273 changed the title feat!: support indirect referrers for all formats of oras discover feat!: support showing indirect referrers for all formats of oras discover Mar 12, 2025
Copy link

codecov bot commented Mar 12, 2025

Codecov Report

Attention: Patch coverage is 88.67925% with 6 lines in your changes missing coverage. Please review.

Project coverage is 84.69%. Comparing base (26551f7) to head (f4fa547).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...d/oras/internal/display/metadata/model/discover.go 85.00% 2 Missing and 1 partial ⚠️
cmd/oras/root/discover.go 87.50% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1653      +/-   ##
==========================================
+ Coverage   84.53%   84.69%   +0.16%     
==========================================
  Files         126      126              
  Lines        5682     5685       +3     
==========================================
+ Hits         4803     4815      +12     
+ Misses        625      619       -6     
+ Partials      254      251       -3     

☔ 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.

@wangxiaoxuan273 wangxiaoxuan273 marked this pull request as ready for review March 13, 2025 04:47
@wangxiaoxuan273
Copy link
Contributor Author

Spec related #1625

@shizhMSFT shizhMSFT requested a review from Copilot March 14, 2025 03:07
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a new --depth flag for the oras discover command to control the recursion depth when displaying referrers, making all output formats support multi-level referrers. Key changes include:

  • Adding tests to verify the behavior of the --depth flag for direct and indirect referrers in JSON, tree, and table outputs.
  • Updating the fetchAllReferrers logic and discover handler implementations to support recursive discovery based on the provided depth.
  • Refining output formatting and flag usage messages to reflect the new multi-level referrer functionality.

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

Show a summary per file
File Description
test/e2e/suite/command/discover.go Updated e2e tests validating error handling and output for various depth scenarios.
cmd/oras/root/discover.go Introduced the --depth flag, added validation for depth, and modified fetchAllReferrers to support recursion.
cmd/oras/internal/display/metadata/model/discover.go Refactored the Discover model to support multi-level referrers via a recursive tree structure.
Other files (json, template, tree, table) Updated metadata handlers to accommodate the refactored multi-level Discover model and remove deprecated options.
cmd/oras/internal/option/format.go Modified usage messages for output formats to align with multi-level discovery changes.
Comments suppressed due to low confidence (1)

cmd/oras/root/discover.go:153

  • Consider renaming the parameter 'depth' to 'maxDepth' to clearly convey that it represents the maximum recursion depth allowed. This could improve clarity when reading and maintaining the recursion logic.
func fetchAllReferrers(ctx context.Context, repo oras.ReadOnlyGraphTarget, desc ocispec.Descriptor, artifactType string, handler metadata.DiscoverHandler, currentDepth int, depth int) error {

Xiaoxuan Wang added 6 commits March 26, 2025 11:35
Signed-off-by: Xiaoxuan Wang <[email protected]>
Signed-off-by: Xiaoxuan Wang <[email protected]>
Signed-off-by: Xiaoxuan Wang <[email protected]>
Signed-off-by: Xiaoxuan Wang <[email protected]>
Signed-off-by: Xiaoxuan Wang <[email protected]>
Signed-off-by: Xiaoxuan Wang <[email protected]>
@wangxiaoxuan273 wangxiaoxuan273 requested a review from Copilot March 26, 2025 06:13
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request adds support for displaying indirect referrers for all output formats of the oras discover command by introducing a new --depth flag and updating the output models accordingly. Key changes include:

  • Adding and validating a new --depth flag in the discover command.
  • Updating end-to-end tests to verify that the depth flag correctly controls the levels of referrers displayed.
  • Modifying JSON, table, and template output handlers to render multi-level referrers.

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test/e2e/suite/command/discover.go Updated tests to cover invalid depth values and verify direct/indirect referrer behavior.
cmd/oras/root/discover.go Added depth flag processing and passed depth parameter into recursive referrer fetching.
cmd/oras/internal/display/metadata/model/discover.go Refactored discover model to support multi-level referrer trees.
cmd/oras/internal/display/metadata/json/discover.go, template/discover.go, tree/discover.go, table/discover.go Updated handlers to render the updated discover model.
cmd/oras/internal/option/format.go Adjusted usage strings to reflect multi-level support for output formats.
Comments suppressed due to low confidence (1)

test/e2e/suite/command/discover.go:121

  • [nitpick] The newly introduced type names 'referrer' and 'subject' may be ambiguous in the context of the tests. Consider renaming 'subject' to a more descriptive name such as 'discoveredSubject' to better convey its role.
type referrer struct {

Xiaoxuan Wang added 2 commits March 26, 2025 14:37
Signed-off-by: Xiaoxuan Wang <[email protected]>
Signed-off-by: Xiaoxuan Wang <[email protected]>
Signed-off-by: Xiaoxuan Wang <[email protected]>
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

@wangxiaoxuan273 wangxiaoxuan273 requested a review from Copilot March 26, 2025 07:17
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for displaying indirect referrers in the output of the ORAS discover command by introducing a new --depth flag. Key changes include:

  • Adding a new --depth flag that controls the levels of referrers to display.
  • Updating tests across JSON, tree, and table formats to validate both direct and indirect referrer output.
  • Adjusting the output handler implementations for JSON and template formats to support multi-level referrer discovery.

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

File Description
test/e2e/suite/command/discover.go Updated test cases to check behavior for various --depth values and referrer levels.
cmd/oras/root/discover.go Added depth flag parsing, validation, and integrated it with referrer fetching logic.
cmd/oras/internal/display/metadata/* Modified discovery models and handlers to support multi-level referrer discovery.
cmd/oras/internal/option/format.go Updated format usage text to reflect the change from direct to multi-level referrer output.
Comments suppressed due to low confidence (2)

cmd/oras/root/discover.go:88

  • The PR description indicates that the default depth should be 20, but the implementation uses a default of 0 (interpreted as unlimited) when the --depth flag is not provided. Consider aligning the default value with the description or updating the PR documentation to reflect the intended behavior.
if cmd.Flags().Changed("depth") && opts.depth < 1 {

cmd/oras/root/discover.go:163

  • [nitpick] Consider renaming 'nextDepth' to 'remainingDepth' for clarity, as it represents the remaining levels of recursion for fetching referrers.
var nextDepth int

Signed-off-by: Xiaoxuan Wang <[email protected]>
@wangxiaoxuan273 wangxiaoxuan273 requested a review from Copilot March 27, 2025 04:48
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enhances the oras discover command to support showing both direct and indirect referrers by introducing a new --depth flag and modifying the output for different formats.

  • Adds a new --depth flag to control the level of referrers displayed.
  • Updates E2E tests and output models (JSON, tree, table, and Go template) to support multi-level referrer discovery.
  • Adjusts internal models and handler implementations to recursively fetch and render referrer trees.

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

Show a summary per file
File Description
test/e2e/suite/command/discover.go Updates tests to validate --depth flag behavior and multi-level referrer outputs.
cmd/oras/root/discover.go Adds depth flag handling and adapts recursive fetching of referrers.
cmd/oras/internal/display/metadata/model/discover.go Updates the referrer model to represent a hierarchical tree.
cmd/oras/internal/display/metadata/json/discover.go, template/discover.go, table/discover.go Removes deprecated multi-level support and aligns output rendering with the new discovery model.
cmd/oras/internal/option/format.go Updates usage descriptions to reflect changes in how referrers are displayed.
Comments suppressed due to low confidence (1)

cmd/oras/root/discover.go:163

  • [nitpick] Consider renaming 'nextDepth' to 'childDepth' to more clearly indicate that it represents the remaining depth for recursive calls.
var nextDepth int

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

@Wwwsylvia Wwwsylvia merged commit bf8e2ee into oras-project:main Mar 27, 2025
8 checks passed
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.

4 participants