Skip to content

Commit

Permalink
refinement
Browse files Browse the repository at this point in the history
Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>
  • Loading branch information
Xiaoxuan Wang committed Sep 24, 2024
1 parent f807af8 commit 9134a8f
Show file tree
Hide file tree
Showing 8 changed files with 107 additions and 107 deletions.
44 changes: 22 additions & 22 deletions cmd/oras/internal/display/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,28 +111,6 @@ func NewPullHandler(printer *output.Printer, format option.Format, path string,
return statusHandler, metadataHandler, nil
}

// NewManifestIndexCreateHandler returns status and metadata handlers for index create command.
func NewManifestIndexCreateHandler(outputPath string, printer *output.Printer) (status.ManifestIndexCreateHandler, metadata.ManifestIndexCreateHandler, error) {
var statusHandler status.ManifestIndexCreateHandler
if outputPath == "-" {
statusHandler = status.NewDiscardHandler()
} else {
statusHandler = status.NewTextManifestIndexCreateHandler(printer)
}
return statusHandler, text.NewManifestIndexCreateHandler(printer), nil
}

// NewManifestIndexUpdateHandler returns status and metadata handlers for index update command.
func NewManifestIndexUpdateHandler(outputPath string, printer *output.Printer) (status.ManifestIndexUpdateHandler, metadata.ManifestIndexUpdateHandler, error) {
var statusHandler status.ManifestIndexUpdateHandler
if outputPath == "-" {
statusHandler = status.NewDiscardHandler()
} else {
statusHandler = status.NewTextManifestIndexUpdateHandler(printer)
}
return statusHandler, text.NewManifestIndexCreateHandler(printer), nil
}

// NewDiscoverHandler returns status and metadata handlers for discover command.
func NewDiscoverHandler(out io.Writer, format option.Format, path string, rawReference string, desc ocispec.Descriptor, verbose bool) (metadata.DiscoverHandler, error) {
var handler metadata.DiscoverHandler
Expand Down Expand Up @@ -196,6 +174,28 @@ func NewManifestPushHandler(printer *output.Printer) metadata.ManifestPushHandle
return text.NewManifestPushHandler(printer)
}

// NewManifestIndexCreateHandler returns status and metadata handlers for index create command.
func NewManifestIndexCreateHandler(outputPath string, printer *output.Printer) (status.ManifestIndexCreateHandler, metadata.ManifestIndexCreateHandler, error) {
var statusHandler status.ManifestIndexCreateHandler
if outputPath == "-" {
statusHandler = status.NewDiscardHandler()
} else {
statusHandler = status.NewTextManifestIndexCreateHandler(printer)
}
return statusHandler, text.NewManifestIndexCreateHandler(printer), nil
}

// NewManifestIndexUpdateHandler returns status and metadata handlers for index update command.
func NewManifestIndexUpdateHandler(outputPath string, printer *output.Printer) (status.ManifestIndexUpdateHandler, metadata.ManifestIndexUpdateHandler, error) {
var statusHandler status.ManifestIndexUpdateHandler
if outputPath == "-" {
statusHandler = status.NewDiscardHandler()
} else {
statusHandler = status.NewTextManifestIndexUpdateHandler(printer)
}
return statusHandler, text.NewManifestIndexCreateHandler(printer), nil
}

// NewCopyHandler returns copy handlers.
func NewCopyHandler(printer *output.Printer, fetcher fetcher.Fetcher) (status.CopyHandler, metadata.CopyHandler) {
return status.NewTextCopyHandler(printer, fetcher), text.NewCopyHandler(printer)
Expand Down
8 changes: 4 additions & 4 deletions cmd/oras/internal/display/status/discard.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,12 +101,12 @@ func (DiscardHandler) OnSourceManifestFetched(source string) error {
}

// OnManifestFetching implements ManifestIndexUpdateHandler.
func (DiscardHandler) OnManifestFetching(source string) error {
func (DiscardHandler) OnManifestFetching(ref string) error {
return nil
}

// OnManifestFetched implements ManifestIndexUpdateHandler.
func (DiscardHandler) OnManifestFetched(source string, digest digest.Digest) error {
func (DiscardHandler) OnManifestFetched(ref string, digest digest.Digest) error {
return nil
}

Expand All @@ -116,7 +116,7 @@ func (DiscardHandler) OnManifestRemoved(digest digest.Digest) error {
}

// OnManifestAdded implements ManifestIndexUpdateHandler.
func (DiscardHandler) OnManifestAdded(source string, digest digest.Digest) error {
func (DiscardHandler) OnManifestAdded(ref string, digest digest.Digest) error {
return nil
}

Expand All @@ -141,7 +141,7 @@ func (DiscardHandler) OnIndexPacked(shortDigest string) error {
}

// OnIndexUpdated implements ManifestIndexUpdateHandler.
func (DiscardHandler) OnIndexUpdated(shortDigest string) error {
func (DiscardHandler) OnIndexUpdated(digest digest.Digest) error {
return nil
}

Expand Down
4 changes: 2 additions & 2 deletions cmd/oras/internal/display/status/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ type ManifestIndexUpdateHandler interface {
OnManifestFetched(manifestRef string, digest digest.Digest) error
OnManifestRemoved(digest digest.Digest) error
OnManifestAdded(manifestRef string, digest digest.Digest) error
OnIndexMerged(manifestRef string, digest digest.Digest) error
OnIndexUpdated(shortDigest string) error
OnIndexMerged(indexRef string, digest digest.Digest) error
OnIndexUpdated(digest digest.Digest) error
OnIndexPushed(path string) error
}
16 changes: 8 additions & 8 deletions cmd/oras/internal/display/status/text.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,27 +254,27 @@ func (miuh TextManifestIndexUpdateHandler) OnManifestRemoved(digest digest.Diges
// OnManifestAdded implements ManifestIndexUpdateHandler.
func (miuh TextManifestIndexUpdateHandler) OnManifestAdded(ref string, digest digest.Digest) error {
if contentutil.IsDigest(ref) {
return miuh.printer.Println(IndexPromptFetched, ref)
return miuh.printer.Println(IndexPromptAdded, ref)
}
return miuh.printer.Println(IndexPromptFetched, digest, ref)
return miuh.printer.Println(IndexPromptAdded, digest, ref)

Check warning on line 259 in cmd/oras/internal/display/status/text.go

View check run for this annotation

Codecov / codecov/patch

cmd/oras/internal/display/status/text.go#L259

Added line #L259 was not covered by tests
}

// OnIndexMerged implements ManifestIndexUpdateHandler.
func (miuh TextManifestIndexUpdateHandler) OnIndexMerged(ref string, digest digest.Digest) error {
if contentutil.IsDigest(ref) {
return miuh.printer.Println(IndexPromptFetched, ref)
return miuh.printer.Println(IndexPromptMerged, ref)

Check warning on line 265 in cmd/oras/internal/display/status/text.go

View check run for this annotation

Codecov / codecov/patch

cmd/oras/internal/display/status/text.go#L265

Added line #L265 was not covered by tests
}
return miuh.printer.Println(IndexPromptFetched, digest, ref)
return miuh.printer.Println(IndexPromptMerged, digest, ref)
}

// OnIndexUpdated implements ManifestIndexUpdateHandler.
func (miuh TextManifestIndexUpdateHandler) OnIndexUpdated(source string) error {
return miuh.printer.Println(IndexPromptUpdated, source)
func (miuh TextManifestIndexUpdateHandler) OnIndexUpdated(digest digest.Digest) error {
return miuh.printer.Println(IndexPromptUpdated, digest)
}

// OnIndexPushed implements ManifestIndexUpdateHandler.
func (miuh TextManifestIndexUpdateHandler) OnIndexPushed(source string) error {
return miuh.printer.Println(IndexPromptPushed, source)
func (miuh TextManifestIndexUpdateHandler) OnIndexPushed(indexRef string) error {
return miuh.printer.Println(IndexPromptPushed, indexRef)
}

// NewTextManifestIndexUpdateHandler returns a new handler for manifest index create command.
Expand Down
6 changes: 3 additions & 3 deletions cmd/oras/root/manifest/index/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func createIndex(cmd *cobra.Command, opts createOptions) error {
if err != nil {
return err

Check warning on line 114 in cmd/oras/root/manifest/index/create.go

View check run for this annotation

Codecov / codecov/patch

cmd/oras/root/manifest/index/create.go#L114

Added line #L114 was not covered by tests
}
manifests, err := fetchSourceManifests(ctx, displayStatus, target, opts)
manifests, err := fetchSourceManifests(ctx, displayStatus, target, opts.sources)
if err != nil {
return err
}
Expand Down Expand Up @@ -146,9 +146,9 @@ func createIndex(cmd *cobra.Command, opts createOptions) error {
return err
}

func fetchSourceManifests(ctx context.Context, displayStatus status.ManifestIndexCreateHandler, target oras.ReadOnlyTarget, opts createOptions) ([]ocispec.Descriptor, error) {
func fetchSourceManifests(ctx context.Context, displayStatus status.ManifestIndexCreateHandler, target oras.ReadOnlyTarget, sources []string) ([]ocispec.Descriptor, error) {
resolved := []ocispec.Descriptor{}
for _, source := range opts.sources {
for _, source := range sources {
if err := displayStatus.OnSourceManifestFetching(source); err != nil {
return nil, err

Check warning on line 153 in cmd/oras/root/manifest/index/create.go

View check run for this annotation

Codecov / codecov/patch

cmd/oras/root/manifest/index/create.go#L153

Added line #L153 was not covered by tests
}
Expand Down
28 changes: 14 additions & 14 deletions cmd/oras/root/manifest/index/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,19 +118,19 @@ func updateIndex(cmd *cobra.Command, opts updateOptions) error {
if err != nil {
return err
}
index, err := fetchIndex(ctx, displayStatus, target, opts)
index, err := fetchIndex(ctx, displayStatus, target, opts.Reference)
if err != nil {
return err
}
manifests, err := removeManifests(ctx, displayStatus, index.Manifests, target, opts)
if err != nil {
return err
}
manifests, err = addManifests(ctx, displayStatus, manifests, target, opts)
manifests, err = addManifests(ctx, displayStatus, manifests, target, opts.addArguments)
if err != nil {
return err
}
manifests, err = mergeIndexes(ctx, displayStatus, manifests, target, opts)
manifests, err = mergeIndexes(ctx, displayStatus, manifests, target, opts.mergeArguments)
if err != nil {
return err
}
Expand All @@ -142,7 +142,7 @@ func updateIndex(cmd *cobra.Command, opts updateOptions) error {
}
desc := content.NewDescriptorFromBytes(index.MediaType, indexBytes)

displayStatus.OnIndexUpdated(string(desc.Digest))
displayStatus.OnIndexUpdated(desc.Digest)

Check failure on line 145 in cmd/oras/root/manifest/index/update.go

View workflow job for this annotation

GitHub Actions / lint (1.23)

Error return value of `displayStatus.OnIndexUpdated` is not checked (errcheck)
path := getPushPath(opts.RawReference, opts.Type, opts.Reference, opts.Path)
switch opts.outputPath {
case "":
Expand All @@ -156,25 +156,25 @@ func updateIndex(cmd *cobra.Command, opts updateOptions) error {
return err
}

func fetchIndex(ctx context.Context, handler status.ManifestIndexUpdateHandler, target oras.ReadOnlyTarget, opts updateOptions) (ocispec.Index, error) {
handler.OnIndexFetching(opts.Reference)
desc, content, err := oras.FetchBytes(ctx, target, opts.Reference, oras.DefaultFetchBytesOptions)
func fetchIndex(ctx context.Context, handler status.ManifestIndexUpdateHandler, target oras.ReadOnlyTarget, reference string) (ocispec.Index, error) {
handler.OnIndexFetching(reference)

Check failure on line 160 in cmd/oras/root/manifest/index/update.go

View workflow job for this annotation

GitHub Actions / lint (1.23)

Error return value of `handler.OnIndexFetching` is not checked (errcheck)
desc, content, err := oras.FetchBytes(ctx, target, reference, oras.DefaultFetchBytesOptions)
if err != nil {
return ocispec.Index{}, fmt.Errorf("could not find the index %s: %w", opts.Reference, err)
return ocispec.Index{}, fmt.Errorf("could not find the index %s: %w", reference, err)
}
if !descriptor.IsIndex(desc) {
return ocispec.Index{}, fmt.Errorf("%s is not an index", opts.Reference)
return ocispec.Index{}, fmt.Errorf("%s is not an index", reference)
}
handler.OnIndexFetched(opts.Reference, desc.Digest)
handler.OnIndexFetched(reference, desc.Digest)

Check failure on line 168 in cmd/oras/root/manifest/index/update.go

View workflow job for this annotation

GitHub Actions / lint (1.23)

Error return value of `handler.OnIndexFetched` is not checked (errcheck)
var index ocispec.Index
if err := json.Unmarshal(content, &index); err != nil {
return ocispec.Index{}, err
}
return index, nil
}

func addManifests(ctx context.Context, handler status.ManifestIndexUpdateHandler, manifests []ocispec.Descriptor, target oras.ReadOnlyTarget, opts updateOptions) ([]ocispec.Descriptor, error) {
for _, manifestRef := range opts.addArguments {
func addManifests(ctx context.Context, handler status.ManifestIndexUpdateHandler, manifests []ocispec.Descriptor, target oras.ReadOnlyTarget, addArguments []string) ([]ocispec.Descriptor, error) {
for _, manifestRef := range addArguments {
handler.OnManifestFetching(manifestRef)

Check failure on line 178 in cmd/oras/root/manifest/index/update.go

View workflow job for this annotation

GitHub Actions / lint (1.23)

Error return value of `handler.OnManifestFetching` is not checked (errcheck)
desc, content, err := oras.FetchBytes(ctx, target, manifestRef, oras.DefaultFetchBytesOptions)
if err != nil {
Expand All @@ -196,8 +196,8 @@ func addManifests(ctx context.Context, handler status.ManifestIndexUpdateHandler
return manifests, nil
}

func mergeIndexes(ctx context.Context, handler status.ManifestIndexUpdateHandler, manifests []ocispec.Descriptor, target oras.ReadOnlyTarget, opts updateOptions) ([]ocispec.Descriptor, error) {
for _, indexRef := range opts.mergeArguments {
func mergeIndexes(ctx context.Context, handler status.ManifestIndexUpdateHandler, manifests []ocispec.Descriptor, target oras.ReadOnlyTarget, mergeArguments []string) ([]ocispec.Descriptor, error) {
for _, indexRef := range mergeArguments {
handler.OnIndexFetching(indexRef)

Check failure on line 201 in cmd/oras/root/manifest/index/update.go

View workflow job for this annotation

GitHub Actions / lint (1.23)

Error return value of `handler.OnIndexFetching` is not checked (errcheck)
desc, content, err := oras.FetchBytes(ctx, target, indexRef, oras.DefaultFetchBytesOptions)
if err != nil {
Expand Down
100 changes: 50 additions & 50 deletions cmd/oras/root/manifest/index/update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,72 +46,72 @@ var (

func Test_doRemoveManifests(t *testing.T) {
tests := []struct {
name string
manifests []ocispec.Descriptor
digestSet map[digest.Digest]bool
handler status.ManifestIndexUpdateHandler
indexRef string
want []ocispec.Descriptor
wantErr bool
name string
manifests []ocispec.Descriptor
digestSet map[digest.Digest]bool
displayStatus status.ManifestIndexUpdateHandler
indexRef string
want []ocispec.Descriptor
wantErr bool
}{
{
name: "remove one matched item",
manifests: []ocispec.Descriptor{A, B, C},
digestSet: map[digest.Digest]bool{B.Digest: false},
handler: status.NewTextManifestIndexUpdateHandler(output.NewPrinter(os.Stdout, os.Stderr, false)),
indexRef: "test01",
want: []ocispec.Descriptor{A, C},
wantErr: false,
name: "remove one matched item",
manifests: []ocispec.Descriptor{A, B, C},
digestSet: map[digest.Digest]bool{B.Digest: false},
displayStatus: status.NewTextManifestIndexUpdateHandler(output.NewPrinter(os.Stdout, os.Stderr, false)),
indexRef: "test01",
want: []ocispec.Descriptor{A, C},
wantErr: false,
},
{
name: "remove all matched items",
manifests: []ocispec.Descriptor{A, B, A, C, A, A, A},
digestSet: map[digest.Digest]bool{A.Digest: false},
handler: status.NewTextManifestIndexUpdateHandler(output.NewPrinter(os.Stdout, os.Stderr, false)),
indexRef: "test02",
want: []ocispec.Descriptor{B, C},
wantErr: false,
name: "remove all matched items",
manifests: []ocispec.Descriptor{A, B, A, C, A, A, A},
digestSet: map[digest.Digest]bool{A.Digest: false},
displayStatus: status.NewTextManifestIndexUpdateHandler(output.NewPrinter(os.Stdout, os.Stderr, false)),
indexRef: "test02",
want: []ocispec.Descriptor{B, C},
wantErr: false,
},
{
name: "remove correctly when there is only one item",
manifests: []ocispec.Descriptor{A},
digestSet: map[digest.Digest]bool{A.Digest: false},
handler: status.NewTextManifestIndexUpdateHandler(output.NewPrinter(os.Stdout, os.Stderr, false)),
indexRef: "test03",
want: []ocispec.Descriptor{},
wantErr: false,
name: "remove correctly when there is only one item",
manifests: []ocispec.Descriptor{A},
digestSet: map[digest.Digest]bool{A.Digest: false},
displayStatus: status.NewTextManifestIndexUpdateHandler(output.NewPrinter(os.Stdout, os.Stderr, false)),
indexRef: "test03",
want: []ocispec.Descriptor{},
wantErr: false,
},
{
name: "remove multiple distinct manifests",
manifests: []ocispec.Descriptor{A, B, C},
digestSet: map[digest.Digest]bool{A.Digest: false, C.Digest: false},
handler: status.NewTextManifestIndexUpdateHandler(output.NewPrinter(os.Stdout, os.Stderr, false)),
indexRef: "test04",
want: []ocispec.Descriptor{B},
wantErr: false,
name: "remove multiple distinct manifests",
manifests: []ocispec.Descriptor{A, B, C},
digestSet: map[digest.Digest]bool{A.Digest: false, C.Digest: false},
displayStatus: status.NewTextManifestIndexUpdateHandler(output.NewPrinter(os.Stdout, os.Stderr, false)),
indexRef: "test04",
want: []ocispec.Descriptor{B},
wantErr: false,
},
{
name: "remove multiple duplicate manifests",
manifests: []ocispec.Descriptor{A, B, C, C, B, A, B},
digestSet: map[digest.Digest]bool{A.Digest: false, C.Digest: false},
handler: status.NewTextManifestIndexUpdateHandler(output.NewPrinter(os.Stdout, os.Stderr, false)),
indexRef: "test04",
want: []ocispec.Descriptor{B, B, B},
wantErr: false,
name: "remove multiple duplicate manifests",
manifests: []ocispec.Descriptor{A, B, C, C, B, A, B},
digestSet: map[digest.Digest]bool{A.Digest: false, C.Digest: false},
displayStatus: status.NewTextManifestIndexUpdateHandler(output.NewPrinter(os.Stdout, os.Stderr, false)),
indexRef: "test04",
want: []ocispec.Descriptor{B, B, B},
wantErr: false,
},
{
name: "return error when deleting a nonexistent item",
manifests: []ocispec.Descriptor{A, C},
digestSet: map[digest.Digest]bool{B.Digest: false},
handler: status.NewTextManifestIndexUpdateHandler(output.NewPrinter(os.Stdout, os.Stderr, false)),
indexRef: "test04",
want: nil,
wantErr: true,
name: "return error when deleting a nonexistent item",
manifests: []ocispec.Descriptor{A, C},
digestSet: map[digest.Digest]bool{B.Digest: false},
displayStatus: status.NewTextManifestIndexUpdateHandler(output.NewPrinter(os.Stdout, os.Stderr, false)),
indexRef: "test04",
want: nil,
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := doRemoveManifests(tt.manifests, tt.digestSet, tt.handler, tt.indexRef)
got, err := doRemoveManifests(tt.manifests, tt.digestSet, tt.displayStatus, tt.indexRef)
if (err != nil) != tt.wantErr {
t.Errorf("removeManifestsFromIndex() error = %v, wantErr %v", err, tt.wantErr)
return
Expand Down
8 changes: 4 additions & 4 deletions test/e2e/suite/command/manifest_index.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ var _ = Describe("1.1 registry users:", func() {
testRepo := indexTestRepo("create", "output-to-stdout")
CopyZOTRepo(ImageRepo, testRepo)
ORAS("manifest", "index", "create", RegistryRef(ZOTHost, testRepo, ""), string(multi_arch.LinuxAMD64.Digest),
"--output", "-").MatchKeyWords(multi_arch.OutputIndex).Exec()
"--output", "-").MatchContent(multi_arch.OutputIndex).Exec()
})

It("should fail if given a reference that does not exist in the repo", func() {
Expand Down Expand Up @@ -270,7 +270,7 @@ var _ = Describe("1.1 registry users:", func() {
ORAS("manifest", "index", "create", RegistryRef(ZOTHost, testRepo, "v1")).Exec()
// add a manifest to the index
ORAS("manifest", "index", "update", RegistryRef(ZOTHost, testRepo, "v1"),
"--add", string(multi_arch.LinuxAMD64.Digest), "--output", "-").MatchKeyWords(multi_arch.OutputIndex).Exec()
"--add", string(multi_arch.LinuxAMD64.Digest), "--output", "-").MatchContent(multi_arch.OutputIndex).Exec()
})

It("should tell user nothing to update if no update flags are used", func() {
Expand Down Expand Up @@ -451,7 +451,7 @@ var _ = Describe("OCI image layout users:", func() {
root := PrepareTempOCI(ImageRepo)
indexRef := LayoutRef(root, "output-to-stdout")
ORAS("manifest", "index", "create", Flags.Layout, indexRef, string(multi_arch.LinuxAMD64.Digest),
"--output", "-").MatchKeyWords(multi_arch.OutputIndex).Exec()
"--output", "-").MatchContent(multi_arch.OutputIndex).Exec()
})

It("should fail if given a reference that does not exist in the repo", func() {
Expand Down Expand Up @@ -536,7 +536,7 @@ var _ = Describe("OCI image layout users:", func() {
ORAS("manifest", "index", "create", Flags.Layout, LayoutRef(root, "index01")).Exec()
// add a manifest to the index
ORAS("manifest", "index", "update", Flags.Layout, LayoutRef(root, "index01"),
"--add", string(multi_arch.LinuxAMD64.Digest), "--output", "-").MatchKeyWords(multi_arch.OutputIndex).Exec()
"--add", string(multi_arch.LinuxAMD64.Digest), "--output", "-").MatchContent(multi_arch.OutputIndex).Exec()
})

It("should tell user nothing to update if no update flags are used", func() {
Expand Down

0 comments on commit 9134a8f

Please sign in to comment.