From 9134a8f5f645847605efb75d10678d4084ba9f74 Mon Sep 17 00:00:00 2001 From: Xiaoxuan Wang Date: Tue, 24 Sep 2024 16:51:11 +0800 Subject: [PATCH] refinement Signed-off-by: Xiaoxuan Wang --- cmd/oras/internal/display/handler.go | 44 ++++---- cmd/oras/internal/display/status/discard.go | 8 +- cmd/oras/internal/display/status/interface.go | 4 +- cmd/oras/internal/display/status/text.go | 16 +-- cmd/oras/root/manifest/index/create.go | 6 +- cmd/oras/root/manifest/index/update.go | 28 ++--- cmd/oras/root/manifest/index/update_test.go | 100 +++++++++--------- test/e2e/suite/command/manifest_index.go | 8 +- 8 files changed, 107 insertions(+), 107 deletions(-) diff --git a/cmd/oras/internal/display/handler.go b/cmd/oras/internal/display/handler.go index c92964b3d..9fec99a56 100644 --- a/cmd/oras/internal/display/handler.go +++ b/cmd/oras/internal/display/handler.go @@ -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 @@ -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) diff --git a/cmd/oras/internal/display/status/discard.go b/cmd/oras/internal/display/status/discard.go index 2f5d8984a..64e3e092b 100644 --- a/cmd/oras/internal/display/status/discard.go +++ b/cmd/oras/internal/display/status/discard.go @@ -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 } @@ -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 } @@ -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 } diff --git a/cmd/oras/internal/display/status/interface.go b/cmd/oras/internal/display/status/interface.go index 6a4cc4a47..4d006fdd1 100644 --- a/cmd/oras/internal/display/status/interface.go +++ b/cmd/oras/internal/display/status/interface.go @@ -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 } diff --git a/cmd/oras/internal/display/status/text.go b/cmd/oras/internal/display/status/text.go index e995769a0..2fa3ba8b3 100644 --- a/cmd/oras/internal/display/status/text.go +++ b/cmd/oras/internal/display/status/text.go @@ -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) } // 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) } - 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. diff --git a/cmd/oras/root/manifest/index/create.go b/cmd/oras/root/manifest/index/create.go index 5c2ffd9cb..5a38cc4b5 100644 --- a/cmd/oras/root/manifest/index/create.go +++ b/cmd/oras/root/manifest/index/create.go @@ -113,7 +113,7 @@ func createIndex(cmd *cobra.Command, opts createOptions) error { if err != nil { return err } - manifests, err := fetchSourceManifests(ctx, displayStatus, target, opts) + manifests, err := fetchSourceManifests(ctx, displayStatus, target, opts.sources) if err != nil { return err } @@ -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 } diff --git a/cmd/oras/root/manifest/index/update.go b/cmd/oras/root/manifest/index/update.go index 1df8b84a6..b0929f387 100644 --- a/cmd/oras/root/manifest/index/update.go +++ b/cmd/oras/root/manifest/index/update.go @@ -118,7 +118,7 @@ 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 } @@ -126,11 +126,11 @@ func updateIndex(cmd *cobra.Command, opts updateOptions) error { 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 } @@ -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) path := getPushPath(opts.RawReference, opts.Type, opts.Reference, opts.Path) switch opts.outputPath { case "": @@ -156,16 +156,16 @@ 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) + 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) var index ocispec.Index if err := json.Unmarshal(content, &index); err != nil { return ocispec.Index{}, err @@ -173,8 +173,8 @@ func fetchIndex(ctx context.Context, handler status.ManifestIndexUpdateHandler, 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) desc, content, err := oras.FetchBytes(ctx, target, manifestRef, oras.DefaultFetchBytesOptions) if err != nil { @@ -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) desc, content, err := oras.FetchBytes(ctx, target, indexRef, oras.DefaultFetchBytesOptions) if err != nil { diff --git a/cmd/oras/root/manifest/index/update_test.go b/cmd/oras/root/manifest/index/update_test.go index 1e9347a94..9c64b1bc5 100644 --- a/cmd/oras/root/manifest/index/update_test.go +++ b/cmd/oras/root/manifest/index/update_test.go @@ -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 diff --git a/test/e2e/suite/command/manifest_index.go b/test/e2e/suite/command/manifest_index.go index 0bcbd8a79..3560f5952 100644 --- a/test/e2e/suite/command/manifest_index.go +++ b/test/e2e/suite/command/manifest_index.go @@ -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() { @@ -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() { @@ -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() { @@ -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() {