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

refactor: status, metadata and content handlers for manifest index commands #1509

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
7 changes: 6 additions & 1 deletion cmd/oras/internal/display/content/discard.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,12 @@ func (discardHandler) OnContentFetched(ocispec.Descriptor, []byte) error {
return nil
}

// OnContentCreated implements ManifestIndexCreateHandler.
func (discardHandler) OnContentCreated([]byte) error {
return nil
}

// NewDiscardHandler returns a new discard handler.
func NewDiscardHandler() ManifestFetchHandler {
func NewDiscardHandler() discardHandler {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we export discardHandler just like status.DiscardHandler?

return discardHandler{}
}
9 changes: 9 additions & 0 deletions cmd/oras/internal/display/content/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,12 @@ type ManifestFetchHandler interface {
// OnContentFetched is called after the manifest content is fetched.
OnContentFetched(desc ocispec.Descriptor, content []byte) error
}

// ManifestIndexCreateHandler handles raw output for manifest index create events.
type ManifestIndexCreateHandler interface {
// OnContentCreated is called after the index content is created.
OnContentCreated(content []byte) error
}

// ManifestIndexUpdateHandler handles raw output for manifest index update events.
type ManifestIndexUpdateHandler ManifestIndexCreateHandler
58 changes: 58 additions & 0 deletions cmd/oras/internal/display/content/manifest_index.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
Copyright The ORAS Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package content

import (
"fmt"
"io"
"os"

"oras.land/oras/cmd/oras/internal/output"
)

// manifestIndexCreate handles raw content output.
type manifestIndexCreate struct {
pretty bool
stdout io.Writer
outputPath string
}

// OnContentCreated is called after index content is created.
func (h *manifestIndexCreate) OnContentCreated(manifest []byte) error {
out := h.stdout
if h.outputPath != "" && h.outputPath != "-" {
f, err := os.Create(h.outputPath)
if err != nil {
return fmt.Errorf("failed to open %q: %w", h.outputPath, err)

Check warning on line 39 in cmd/oras/internal/display/content/manifest_index.go

View check run for this annotation

Codecov / codecov/patch

cmd/oras/internal/display/content/manifest_index.go#L39

Added line #L39 was not covered by tests
}
defer f.Close()
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to create an issue to track handling errors returned by Close() in all write paths.

out = f
}
return output.PrintJSON(out, manifest, h.pretty)
qweeah marked this conversation as resolved.
Show resolved Hide resolved
}

// NewManifestIndexCreateHandler creates a new handler.
func NewManifestIndexCreateHandler(out io.Writer, pretty bool, outputPath string) ManifestIndexCreateHandler {
// ignore --pretty when output to a file
if outputPath != "" && outputPath != "-" {
pretty = false
}
Comment on lines +49 to +52
Copy link
Contributor

Choose a reason for hiding this comment

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

It is different from NewManifestFetchHandler. Is NewManifestFetchHandler wrong? /cc @qweeah

Copy link
Contributor

Choose a reason for hiding this comment

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

NewManifestFetchHandler is used only once by oras manifest fetch and such code snippet is in command code

// ignore --pretty when output to a file
case opts.outputPath != "" && opts.outputPath != "-":
opts.Pretty.Pretty = false
}

I think we need to remove it and change the flag reset to NewManifestFetchHandler too.

return &manifestIndexCreate{
qweeah marked this conversation as resolved.
Show resolved Hide resolved
pretty: pretty,
stdout: out,
outputPath: outputPath,
}
}
39 changes: 36 additions & 3 deletions cmd/oras/internal/display/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,9 +174,42 @@ func NewManifestPushHandler(printer *output.Printer) metadata.ManifestPushHandle
return text.NewManifestPushHandler(printer)
}

// NewManifestIndexCreateHandler returns an index create handler.
func NewManifestIndexCreateHandler(printer *output.Printer) metadata.ManifestIndexCreateHandler {
return text.NewManifestIndexCreateHandler(printer)
// NewManifestIndexCreateHandler returns status, metadata and content handlers for index create command.
func NewManifestIndexCreateHandler(outputPath string, printer *output.Printer, pretty bool) (
status.ManifestIndexCreateHandler,
metadata.ManifestIndexCreateHandler,
content.ManifestIndexCreateHandler,
error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

When will an error be returned?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch. Removed the error return.

statusHandler := status.NewTextManifestIndexCreateHandler(printer)
metadataHandler := text.NewManifestIndexCreateHandler(printer)
contentHandler := content.NewManifestIndexCreateHandler(printer, pretty, outputPath)
switch outputPath {
case "":
contentHandler = content.NewDiscardHandler()
case "-":
statusHandler = status.NewDiscardHandler()
metadataHandler = metadata.NewDiscardHandler()
}
return statusHandler, metadataHandler, contentHandler, nil
}

// NewManifestIndexUpdateHandler returns status, metadata and content handlers for index update command.
func NewManifestIndexUpdateHandler(outputPath string, printer *output.Printer, pretty bool) (
Copy link
Member

Choose a reason for hiding this comment

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

should there just be a NewManivestIndexHandler method for both for now and someone can add the other in the future if needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently we need both create and update handlers.

status.ManifestIndexUpdateHandler,
metadata.ManifestIndexUpdateHandler,
content.ManifestIndexUpdateHandler,
error) {
statusHandler := status.NewTextManifestIndexUpdateHandler(printer)
metadataHandler := text.NewManifestIndexUpdateHandler(printer)
contentHandler := content.NewManifestIndexCreateHandler(printer, pretty, outputPath)
switch outputPath {
case "":
contentHandler = content.NewDiscardHandler()
case "-":
statusHandler = status.NewDiscardHandler()
metadataHandler = metadata.NewDiscardHandler()
}
return statusHandler, metadataHandler, contentHandler, nil
}

// NewCopyHandler returns copy handlers.
Expand Down
40 changes: 39 additions & 1 deletion cmd/oras/internal/display/metadata/discard.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@

package metadata

import ocispec "github.com/opencontainers/image-spec/specs-go/v1"
import (
"github.com/opencontainers/go-digest"
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
)

type discard struct{}
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you want to export it just like other discard handlers?


Expand All @@ -28,3 +31,38 @@
func (discard) OnFetched(string, ocispec.Descriptor, []byte) error {
return nil
}

// OnTagged implements ManifestIndexCreateHandler.
func (discard) OnTagged(ocispec.Descriptor, string) error {
return nil

Check warning on line 37 in cmd/oras/internal/display/metadata/discard.go

View check run for this annotation

Codecov / codecov/patch

cmd/oras/internal/display/metadata/discard.go#L36-L37

Added lines #L36 - L37 were not covered by tests
}

// OnCompleted implements ManifestIndexCreateHandler.
func (discard) OnCompleted(ocispec.Descriptor) error {
return nil
}

// OnIndexPacked implements ManifestIndexCreateHandler.
func (discard) OnIndexPacked(ocispec.Descriptor) error {
return nil
}

// OnIndexPushed implements ManifestIndexCreateHandler.
func (discard) OnIndexPushed(string) error {
return nil

Check warning on line 52 in cmd/oras/internal/display/metadata/discard.go

View check run for this annotation

Codecov / codecov/patch

cmd/oras/internal/display/metadata/discard.go#L51-L52

Added lines #L51 - L52 were not covered by tests
}

// OnManifestRemoved implements ManifestIndexUpdateHandler.
func (discard) OnManifestRemoved(digest.Digest) error {
return nil

Check warning on line 57 in cmd/oras/internal/display/metadata/discard.go

View check run for this annotation

Codecov / codecov/patch

cmd/oras/internal/display/metadata/discard.go#L56-L57

Added lines #L56 - L57 were not covered by tests
}

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

// OnIndexMerged implements ManifestIndexUpdateHandler.
func (discard) OnIndexMerged(string, digest.Digest) error {
return nil

Check warning on line 67 in cmd/oras/internal/display/metadata/discard.go

View check run for this annotation

Codecov / codecov/patch

cmd/oras/internal/display/metadata/discard.go#L66-L67

Added lines #L66 - L67 were not covered by tests
}
17 changes: 17 additions & 0 deletions cmd/oras/internal/display/metadata/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ limitations under the License.
package metadata

import (
"github.com/opencontainers/go-digest"
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
"oras.land/oras/cmd/oras/internal/option"
)
Expand Down Expand Up @@ -78,9 +79,25 @@ type ManifestPushHandler interface {
TaggedHandler
}

type manifestPackHandler interface {
OnIndexPacked(desc ocispec.Descriptor) error
OnIndexPushed(path string) error
OnCompleted(desc ocispec.Descriptor) error
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you want to merge this to ManifestIndexCreateHandler?


// ManifestIndexCreateHandler handles metadata output for index create events.
type ManifestIndexCreateHandler interface {
TaggedHandler
manifestPackHandler
}

// ManifestIndexUpdateHandler handles metadata output for index update events.
type ManifestIndexUpdateHandler interface {
TaggedHandler
manifestPackHandler
OnManifestRemoved(digest digest.Digest) error
OnManifestAdded(manifestRef string, digest digest.Digest) error
OnIndexMerged(indexRef string, digest digest.Digest) error
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Change digest to descriptor since it will be used for formatted output in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The parameter of OnManifestRemoved(digest digest.Digest) error must be a digest, since the descriptor is unknown. Changed the parameter of the other two functions as descriptors.


// CopyHandler handles metadata output for cp events.
Expand Down
109 changes: 109 additions & 0 deletions cmd/oras/internal/display/metadata/text/manifest_index.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
/*
Copyright The ORAS Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package text

import (
"github.com/opencontainers/go-digest"
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
"oras.land/oras/cmd/oras/internal/display/metadata"
"oras.land/oras/cmd/oras/internal/output"
"oras.land/oras/internal/contentutil"
"oras.land/oras/internal/descriptor"
)

// ManifestIndexCreateHandler handles text metadata output for index create events.
type ManifestIndexCreateHandler struct {
printer *output.Printer
}

// NewManifestIndexCreateHandler returns a new handler for index create events.
func NewManifestIndexCreateHandler(printer *output.Printer) metadata.ManifestIndexCreateHandler {
return &ManifestIndexCreateHandler{
printer: printer,
}
}

// OnIndexPacked implements metadata.ManifestIndexCreateHandler.
func (h *ManifestIndexCreateHandler) OnIndexPacked(desc ocispec.Descriptor) error {
return h.printer.Println("Packed", descriptor.ShortDigest(desc), ocispec.MediaTypeImageIndex)
}

// OnIndexPushed implements metadata.ManifestIndexCreateHandler.
wangxiaoxuan273 marked this conversation as resolved.
Show resolved Hide resolved
func (h *ManifestIndexCreateHandler) OnIndexPushed(path string) error {
return h.printer.Println("Pushed", path)
}

// OnTagged implements metadata.TaggedHandler.
func (h *ManifestIndexCreateHandler) OnTagged(_ ocispec.Descriptor, tag string) error {
return h.printer.Println("Tagged", tag)
}

// OnCompleted implements metadata.ManifestIndexCreateHandler.
func (h *ManifestIndexCreateHandler) OnCompleted(desc ocispec.Descriptor) error {
return h.printer.Println("Digest:", desc.Digest)
}

type ManifestIndexUpdateHandler struct {
wangxiaoxuan273 marked this conversation as resolved.
Show resolved Hide resolved
printer *output.Printer
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you help separate ManifestIndexUpdateHandler and ManifestIndexCreateHandler into manifest_index_update.go and manifest_index_create.go?

}

// NewManifestIndexUpdateHandler returns a new handler for index update events.
func NewManifestIndexUpdateHandler(printer *output.Printer) metadata.ManifestIndexUpdateHandler {
return &ManifestIndexUpdateHandler{
printer: printer,
}
}

// OnManifestRemoved implements metadata.ManifestIndexUpdateHandler.
func (miuh ManifestIndexUpdateHandler) OnManifestRemoved(digest digest.Digest) error {
return miuh.printer.Println("Removed", digest)
}

// OnManifestAdded implements metadata.ManifestIndexUpdateHandler.
func (miuh ManifestIndexUpdateHandler) OnManifestAdded(ref string, digest digest.Digest) error {
if contentutil.IsDigest(ref) {
return miuh.printer.Println("Added", ref)
}
return miuh.printer.Println("Added", digest, ref)

Check warning on line 80 in cmd/oras/internal/display/metadata/text/manifest_index.go

View check run for this annotation

Codecov / codecov/patch

cmd/oras/internal/display/metadata/text/manifest_index.go#L80

Added line #L80 was not covered by tests
}

// OnIndexMerged implements metadata.ManifestIndexUpdateHandler.
func (miuh ManifestIndexUpdateHandler) OnIndexMerged(ref string, digest digest.Digest) error {
if contentutil.IsDigest(ref) {
return miuh.printer.Println("Merged", ref)

Check warning on line 86 in cmd/oras/internal/display/metadata/text/manifest_index.go

View check run for this annotation

Codecov / codecov/patch

cmd/oras/internal/display/metadata/text/manifest_index.go#L86

Added line #L86 was not covered by tests
}
return miuh.printer.Println("Merged", digest, ref)
}

// OnIndexUpdated implements metadata.ManifestIndexUpdateHandler.
func (miuh ManifestIndexUpdateHandler) OnIndexPacked(desc ocispec.Descriptor) error {
return miuh.printer.Println("Updated", desc.Digest)
}

// OnIndexPushed implements metadata.ManifestIndexUpdateHandler.
func (miuh ManifestIndexUpdateHandler) OnIndexPushed(indexRef string) error {
return miuh.printer.Println("Pushed", indexRef)
}

// OnTagged implements metadata.TaggedHandler.
func (h *ManifestIndexUpdateHandler) OnTagged(_ ocispec.Descriptor, tag string) error {
return h.printer.Println("Tagged", tag)
}

// OnCompleted implements metadata.ManifestIndexUpdateHandler.
func (h *ManifestIndexUpdateHandler) OnCompleted(desc ocispec.Descriptor) error {
return h.printer.Println("Digest:", desc.Digest)
}

This file was deleted.

Loading
Loading