moved annotation out of packer

Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>

cmd/oras/internal/option/annotation.go

@@ -0,0 +1,91 @@
+/*
+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 option
+
+import (
+	"errors"
+	"fmt"
+	"strings"
+
+	"github.com/spf13/pflag"
+	oerrors "oras.land/oras/cmd/oras/internal/errors"
+)
+
+// Pre-defined annotation keys for annotation file
+const (
+	AnnotationManifest = "$manifest"
+	AnnotationConfig   = "$config"
+)
+
+var (
+	errAnnotationConflict    = errors.New("`--annotation` and `--annotation-file` cannot be both specified")
+	errAnnotationFormat      = errors.New("annotation value doesn't match the required format")
+	errAnnotationDuplication = errors.New("duplicate annotation key")
+)
+
+// Packer option struct.
+type Annotation struct {
+	AnnotationFilePath  string
+	ManifestAnnotations []string
+}
+
+// ApplyFlags applies flags to a command flag set.
+func (opts *Annotation) ApplyFlags(fs *pflag.FlagSet) {
+	fs.StringArrayVarP(&opts.ManifestAnnotations, "annotation", "a", nil, "manifest annotations")
+	fs.StringVarP(&opts.AnnotationFilePath, "annotation-file", "", "", "path of the annotation file")
+}
+
+// LoadManifestAnnotations loads the manifest annotation map.
+func (opts *Annotation) LoadManifestAnnotations() (annotations map[string]map[string]string, err error) {
+	if opts.AnnotationFilePath != "" && len(opts.ManifestAnnotations) != 0 {
+		return nil, errAnnotationConflict
+	}
+	if opts.AnnotationFilePath != "" {
+		if err = decodeJSON(opts.AnnotationFilePath, &annotations); err != nil {
+			return nil, &oerrors.Error{
+				Err:            fmt.Errorf(`invalid annotation json file: failed to load annotations from %s`, opts.AnnotationFilePath),
+				Recommendation: `Annotation file doesn't match the required format. Please refer to the document at https://oras.land/docs/how_to_guides/manifest_annotations`,
+			}
+		}
+	}
+	if len(opts.ManifestAnnotations) != 0 {
+		annotations = make(map[string]map[string]string)
+		if err = parseAnnotationFlags(opts.ManifestAnnotations, annotations); err != nil {
+			return nil, err
+		}
+	}
+	return
+}
+
+// parseAnnotationFlags parses annotation flags into a map.
+func parseAnnotationFlags(flags []string, annotations map[string]map[string]string) error {
+	manifestAnnotations := make(map[string]string)
+	for _, anno := range flags {
+		key, val, success := strings.Cut(anno, "=")
+		if !success {
+			return &oerrors.Error{
+				Err:            errAnnotationFormat,
+				Recommendation: `Please use the correct format in the flag: --annotation "key=value"`,
+			}
+		}
+		if _, ok := manifestAnnotations[key]; ok {
+			return fmt.Errorf("%w: %v, ", errAnnotationDuplication, key)
+		}
+		manifestAnnotations[key] = val
+	}
+	annotations[AnnotationManifest] = manifestAnnotations
+	return nil
+}

cmd/oras/internal/option/packer.go

@@ -28,38 +28,25 @@ import (
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 	"oras.land/oras-go/v2/content"
-	oerrors "oras.land/oras/cmd/oras/internal/errors"
 	"oras.land/oras/cmd/oras/internal/fileref"
 )
 
-// Pre-defined annotation keys for annotation file
-const (
-	AnnotationManifest = "$manifest"
-	AnnotationConfig   = "$config"
-)
-
 var (
-	errAnnotationConflict    = errors.New("`--annotation` and `--annotation-file` cannot be both specified")
-	errAnnotationFormat      = errors.New("annotation value doesn't match the required format")
-	errAnnotationDuplication = errors.New("duplicate annotation key")
-	errPathValidation        = errors.New("absolute file path detected. If it's intentional, use --disable-path-validation flag to skip this check")
+	errPathValidation = errors.New("absolute file path detected. If it's intentional, use --disable-path-validation flag to skip this check")
 )
 
 // Packer option struct.
 type Packer struct {
+	Annotation
 	ManifestExportPath     string
 	PathValidationDisabled bool
-	AnnotationFilePath     string
-	ManifestAnnotations    []string
 
 	FileRefs []string
 }
 
 // ApplyFlags applies flags to a command flag set.
 func (opts *Packer) ApplyFlags(fs *pflag.FlagSet) {
 	fs.StringVarP(&opts.ManifestExportPath, "export-manifest", "", "", "`path` of the pushed manifest")
-	fs.StringArrayVarP(&opts.ManifestAnnotations, "annotation", "a", nil, "manifest annotations")
-	fs.StringVarP(&opts.AnnotationFilePath, "annotation-file", "", "", "path of the annotation file")
 	fs.BoolVarP(&opts.PathValidationDisabled, "disable-path-validation", "", false, "skip path validation")
 }
 
@@ -94,28 +81,6 @@ func (opts *Packer) Parse(*cobra.Command) error {
 	return nil
 }
 
-// LoadManifestAnnotations loads the manifest annotation map.
-func (opts *Packer) LoadManifestAnnotations() (annotations map[string]map[string]string, err error) {
-	if opts.AnnotationFilePath != "" && len(opts.ManifestAnnotations) != 0 {
-		return nil, errAnnotationConflict
-	}
-	if opts.AnnotationFilePath != "" {
-		if err = decodeJSON(opts.AnnotationFilePath, &annotations); err != nil {
-			return nil, &oerrors.Error{
-				Err:            fmt.Errorf(`invalid annotation json file: failed to load annotations from %s`, opts.AnnotationFilePath),
-				Recommendation: `Annotation file doesn't match the required format. Please refer to the document at https://oras.land/docs/how_to_guides/manifest_annotations`,
-			}
-		}
-	}
-	if len(opts.ManifestAnnotations) != 0 {
-		annotations = make(map[string]map[string]string)
-		if err = parseAnnotationFlags(opts.ManifestAnnotations, annotations); err != nil {
-			return nil, err
-		}
-	}
-	return
-}
-
 // decodeJSON decodes a json file v to filename.
 func decodeJSON(filename string, v interface{}) error {
 	file, err := os.Open(filename)
@@ -125,23 +90,3 @@ func decodeJSON(filename string, v interface{}) error {
 	defer file.Close()
 	return json.NewDecoder(file).Decode(v)
 }
-
-// parseAnnotationFlags parses annotation flags into a map.
-func parseAnnotationFlags(flags []string, annotations map[string]map[string]string) error {
-	manifestAnnotations := make(map[string]string)
-	for _, anno := range flags {
-		key, val, success := strings.Cut(anno, "=")
-		if !success {
-			return &oerrors.Error{
-				Err:            errAnnotationFormat,
-				Recommendation: `Please use the correct format in the flag: --annotation "key=value"`,
-			}
-		}
-		if _, ok := manifestAnnotations[key]; ok {
-			return fmt.Errorf("%w: %v, ", errAnnotationDuplication, key)
-		}
-		manifestAnnotations[key] = val
-	}
-	annotations[AnnotationManifest] = manifestAnnotations
-	return nil
-}

cmd/oras/internal/option/packer_test.go

@@ -39,29 +39,37 @@ func TestPacker_FlagInit(t *testing.T) {
 
 func TestPacker_LoadManifestAnnotations_err(t *testing.T) {
 	opts := Packer{
-		AnnotationFilePath:  "this is not a file", // testFile,
-		ManifestAnnotations: []string{"Key=Val"},
+		Annotation: Annotation{
+			AnnotationFilePath:  "this is not a file", // testFile,
+			ManifestAnnotations: []string{"Key=Val"},
+		},
 	}
 	if _, err := opts.LoadManifestAnnotations(); !errors.Is(err, errAnnotationConflict) {
 		t.Fatalf("unexpected error: %v", err)
 	}
 
 	opts = Packer{
-		AnnotationFilePath: "this is not a file", // testFile,
+		Annotation: Annotation{
+			AnnotationFilePath: "this is not a file", // testFile,
+		},
 	}
 	if _, err := opts.LoadManifestAnnotations(); err == nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
 
 	opts = Packer{
-		ManifestAnnotations: []string{"KeyVal"},
+		Annotation: Annotation{
+			ManifestAnnotations: []string{"KeyVal"},
+		},
 	}
 	if _, err := opts.LoadManifestAnnotations(); !errors.Is(err, errAnnotationFormat) {
 		t.Fatalf("unexpected error: %v", err)
 	}
 
 	opts = Packer{
-		ManifestAnnotations: []string{"Key=Val1", "Key=Val2"},
+		Annotation: Annotation{
+			ManifestAnnotations: []string{"Key=Val1", "Key=Val2"},
+		},
 	}
 	if _, err := opts.LoadManifestAnnotations(); !errors.Is(err, errAnnotationDuplication) {
 		t.Fatalf("unexpected error: %v", err)
@@ -74,7 +82,11 @@ func TestPacker_LoadManifestAnnotations_annotationFile(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Error writing %s: %v", testFile, err)
 	}
-	opts := Packer{AnnotationFilePath: testFile}
+	opts := Packer{
+		Annotation: Annotation{
+			AnnotationFilePath: testFile,
+		},
+	}
 
 	anno, err := opts.LoadManifestAnnotations()
 	if err != nil {
@@ -91,7 +103,11 @@ func TestPacker_LoadManifestAnnotations_annotationFlag(t *testing.T) {
 		"Key",
 	}
 	var annotations map[string]map[string]string
-	opts := Packer{ManifestAnnotations: invalidFlag0}
+	opts := Packer{
+		Annotation: Annotation{
+			ManifestAnnotations: invalidFlag0,
+		},
+	}
 	_, err := opts.LoadManifestAnnotations()
 	if !errors.Is(err, errAnnotationFormat) {
 		t.Fatalf("unexpected error: %v", err)
@@ -102,7 +118,11 @@ func TestPacker_LoadManifestAnnotations_annotationFlag(t *testing.T) {
 		"Key=0",
 		"Key=1",
 	}
-	opts = Packer{ManifestAnnotations: invalidFlag1}
+	opts = Packer{
+		Annotation: Annotation{
+			ManifestAnnotations: invalidFlag1,
+		},
+	}
 	_, err = opts.LoadManifestAnnotations()
 	if !errors.Is(err, errAnnotationDuplication) {
 		t.Fatalf("unexpected error: %v", err)
@@ -114,7 +134,11 @@ func TestPacker_LoadManifestAnnotations_annotationFlag(t *testing.T) {
 		"Key1=Val",             // 2. Normal Item
 		"Key2=${env:USERNAME}", // 3. Item contains variable eg. "${env:USERNAME}"
 	}
-	opts = Packer{ManifestAnnotations: validFlag}
+	opts = Packer{
+		Annotation: Annotation{
+			ManifestAnnotations: validFlag,
+		},
+	}
 	annotations, err = opts.LoadManifestAnnotations()
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)

cmd/oras/root/attach_test.go

@@ -33,8 +33,10 @@ func Test_runAttach_errType(t *testing.T) {
 	// test
 	opts := &attachOptions{
 		Packer: option.Packer{
-			AnnotationFilePath:  "/tmp/whatever",
-			ManifestAnnotations: []string{"one", "two"},
+			Annotation: option.Annotation{
+				AnnotationFilePath:  "/tmp/whatever",
+				ManifestAnnotations: []string{"one", "two"},
+			},
 		},
 	}
 	got := runAttach(cmd, opts).Error()