chore: clean metadata handler for attach command (#1584)

Signed-off-by: Billy Zha <[email protected]>
oras-project · Dec 13, 2024 · 6fe0395 · 6fe0395
1 parent 53247dd
commit 6fe0395
Show file tree

Hide file tree

Showing 6 changed files with 38 additions and 30 deletions.
diff --git a/cmd/oras/internal/display/metadata/interface.go b/cmd/oras/internal/display/metadata/interface.go
@@ -35,8 +35,9 @@ type Renderer interface {
 
 // AttachHandler handles metadata output for attach events.
 type AttachHandler interface {
-	OnAttached(target *option.Target, root ocispec.Descriptor, subject ocispec.Descriptor)
 	Renderer
+
+	OnAttached(target *option.Target, root ocispec.Descriptor, subject ocispec.Descriptor)
 }
 
 // DiscoverHandler handles metadata output for discover events.

diff --git a/cmd/oras/internal/display/metadata/json/attach.go b/cmd/oras/internal/display/metadata/json/attach.go
@@ -27,9 +27,9 @@ import (
 
 // AttachHandler handles json metadata output for attach events.
 type AttachHandler struct {
-	out    io.Writer
-	target *option.Target
-	root   ocispec.Descriptor
+	out  io.Writer
+	path string
+	root ocispec.Descriptor
 }
 
 // NewAttachHandler creates a new handler for attach events.
@@ -41,11 +41,11 @@ func NewAttachHandler(out io.Writer) metadata.AttachHandler {
 
 // OnAttached implements AttachHandler.
 func (ah *AttachHandler) OnAttached(target *option.Target, root ocispec.Descriptor, _ ocispec.Descriptor) {
-	ah.target = target
+	ah.path = target.Path
 	ah.root = root
 }
 
 // Render is called when the attach command is completed.
 func (ah *AttachHandler) Render() error {
-	return output.PrintPrettyJSON(ah.out, model.NewAttach(ah.root, ah.target.Path))
+	return output.PrintPrettyJSON(ah.out, model.NewAttach(ah.root, ah.path))
 }
diff --git a/cmd/oras/internal/display/metadata/template/attach.go b/cmd/oras/internal/display/metadata/template/attach.go
@@ -29,7 +29,7 @@ import (
 type AttachHandler struct {
 	template string
 	out      io.Writer
-	target   *option.Target
+	path     string
 	root     ocispec.Descriptor
 }
 
@@ -43,11 +43,11 @@ func NewAttachHandler(out io.Writer, template string) metadata.AttachHandler {
 
 // OnAttached implements AttachHandler.
 func (ah *AttachHandler) OnAttached(target *option.Target, root ocispec.Descriptor, _ ocispec.Descriptor) {
-	ah.target = target
+	ah.path = target.Path
 	ah.root = root
 }
 
 // Render formats the metadata of attach command.
 func (ah *AttachHandler) Render() error {
-	return output.ParseAndWrite(ah.out, model.NewAttach(ah.root, ah.target.Path), ah.template)
+	return output.ParseAndWrite(ah.out, model.NewAttach(ah.root, ah.path), ah.template)
 }
diff --git a/cmd/oras/internal/display/metadata/text/attach.go b/cmd/oras/internal/display/metadata/text/attach.go
@@ -27,10 +27,9 @@ import (
 
 // AttachHandler handles text metadata output for attach events.
 type AttachHandler struct {
-	printer *output.Printer
-	target  *option.Target
-	root    ocispec.Descriptor
-	subject ocispec.Descriptor
+	printer            *output.Printer
+	subjectRefByDigest string
+	root               ocispec.Descriptor
 }
 
 // NewAttachHandler returns a new handler for attach events.
@@ -42,21 +41,22 @@ func NewAttachHandler(printer *output.Printer) metadata.AttachHandler {
 
 // OnAttached implements AttachHandler.
 func (ah *AttachHandler) OnAttached(target *option.Target, root ocispec.Descriptor, subject ocispec.Descriptor) {
-	ah.target = target
 	ah.root = root
-	ah.subject = subject
+	if strings.HasSuffix(target.RawReference, subject.Digest.String()) {
+		ah.subjectRefByDigest = target.AnnotatedReference()
+	} else {
+		// use subject digest instead of tag
+		newTarget := *target
+		newTarget.RawReference = fmt.Sprintf("%s@%s", target.Path, subject.Digest)
+		ah.subjectRefByDigest = newTarget.AnnotatedReference()
+	}
 }
 
 // Render is called when the attach command is complete.
 func (ah *AttachHandler) Render() error {
-	digest := ah.subject.Digest.String()
-	if !strings.HasSuffix(ah.target.RawReference, digest) {
-		ah.target.RawReference = fmt.Sprintf("%s@%s", ah.target.Path, ah.subject.Digest)
-	}
-	err := ah.printer.Println("Attached to", ah.target.AnnotatedReference())
+	err := ah.printer.Println("Attached to", ah.subjectRefByDigest)
 	if err != nil {
 		return err
 	}
-	err = ah.printer.Println("Digest:", ah.root.Digest)
-	return err
+	return ah.printer.Println("Digest:", ah.root.Digest)
 }
diff --git a/cmd/oras/root/attach.go b/cmd/oras/root/attach.go
@@ -210,9 +210,7 @@ func runAttach(cmd *cobra.Command, opts *attachOptions) error {
 	if err != nil {
 		return err
 	}
-
 	metadataHandler.OnAttached(&opts.Target, root, subject)
-
 	err = metadataHandler.Render()
 	if err != nil {
 		return err

diff --git a/test/e2e/suite/command/attach.go b/test/e2e/suite/command/attach.go
@@ -138,23 +138,32 @@ var _ = Describe("ORAS beginners:", func() {
 var _ = Describe("1.1 registry users:", func() {
 	When("running attach command", func() {
 		It("should attach a file to a subject and output status", func() {
-			testRepo := attachTestRepo("simple")
+			testRepo := attachTestRepo("attach-tag")
 			CopyZOTRepo(ImageRepo, testRepo)
 			subjectRef := RegistryRef(ZOTHost, testRepo, foobar.Tag)
 			ORAS("attach", "--artifact-type", "test/attach", subjectRef, fmt.Sprintf("%s:%s", foobar.AttachFileName, foobar.AttachFileMedia)).
 				WithWorkDir(PrepareTempFiles()).
+				MatchKeyWords(fmt.Sprintf("Attached to [registry] %s", RegistryRef(ZOTHost, testRepo, foobar.Digest))).
+				MatchStatus([]match.StateKey{foobar.AttachFileStateKey}, false, 1).Exec()
+		})
+
+		It("should attach a file to a subject and output status", func() {
+			testRepo := attachTestRepo("attach-digest")
+			CopyZOTRepo(ImageRepo, testRepo)
+			subjectRef := RegistryRef(ZOTHost, testRepo, foobar.Digest)
+			ORAS("attach", "--artifact-type", "test/attach", subjectRef, fmt.Sprintf("%s:%s", foobar.AttachFileName, foobar.AttachFileMedia)).
+				WithWorkDir(PrepareTempFiles()).
+				MatchKeyWords(fmt.Sprintf("Attached to [registry] %s", subjectRef)).
 				MatchStatus([]match.StateKey{foobar.AttachFileStateKey}, false, 1).Exec()
 		})
 
 		It("should attach a file to an arch-specific subject", func() {
+			// prepare
 			testRepo := attachTestRepo("arch-specific")
-			// Below line will cause unexpected 500
-			// pending for https://github.com/project-zot/zot/pull/2351 to be released
-			// CopyZOTRepo(ImageRepo, testRepo)
+			CopyZOTRepo(ImageRepo, testRepo)
+			// test
 			subjectRef := RegistryRef(ZOTHost, testRepo, multi_arch.Tag)
-			ORAS("cp", RegistryRef(ZOTHost, ImageRepo, multi_arch.Tag), subjectRef).Exec()
 			artifactType := "test/attach"
-			// test
 			out := ORAS("attach", "--artifact-type", artifactType, subjectRef, fmt.Sprintf("%s:%s", foobar.AttachFileName, foobar.AttachFileMedia), "--format", "go-template={{.digest}}", "--platform", "linux/amd64").
 				WithWorkDir(PrepareTempFiles()).Exec().Out.Contents()
 			// validate