From 8dc05a721c5e1bf1d919ed470b1a0e6f4e404b85 Mon Sep 17 00:00:00 2001 From: Xiaoxuan Wang <103478229+wangxiaoxuan273@users.noreply.github.com> Date: Mon, 23 Sep 2024 09:53:59 +0800 Subject: [PATCH] feat: `--annotation` for `index create` (#1499) Signed-off-by: Xiaoxuan Wang --- cmd/oras/internal/option/annotation.go | 67 ++++++++++++++++++++++++ cmd/oras/internal/option/packer.go | 54 ++++++------------- cmd/oras/internal/option/packer_test.go | 67 +++++++++++++++--------- cmd/oras/root/attach.go | 10 ++-- cmd/oras/root/attach_test.go | 10 ++-- cmd/oras/root/manifest/index/create.go | 9 +++- cmd/oras/root/push.go | 10 ++-- test/e2e/suite/command/manifest_index.go | 34 ++++++++++++ 8 files changed, 179 insertions(+), 82 deletions(-) create mode 100644 cmd/oras/internal/option/annotation.go diff --git a/cmd/oras/internal/option/annotation.go b/cmd/oras/internal/option/annotation.go new file mode 100644 index 000000000..035307a40 --- /dev/null +++ b/cmd/oras/internal/option/annotation.go @@ -0,0 +1,67 @@ +/* +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/cobra" + "github.com/spf13/pflag" + oerrors "oras.land/oras/cmd/oras/internal/errors" +) + +var ( + errAnnotationFormat = errors.New("annotation value doesn't match the required format") + errAnnotationDuplication = errors.New("duplicate annotation key") +) + +// Annotation option struct. +type Annotation struct { + // ManifestAnnotations contains raw input of manifest annotation "key=value" pairs + ManifestAnnotations []string + + // Annotations contains parsed manifest and config annotations + Annotations map[string]map[string]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") +} + +// Parse parses the input annotation flags. +func (opts *Annotation) Parse(*cobra.Command) error { + manifestAnnotations := make(map[string]string) + for _, anno := range opts.ManifestAnnotations { + 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 + } + opts.Annotations = map[string]map[string]string{ + AnnotationManifest: manifestAnnotations, + } + return nil +} diff --git a/cmd/oras/internal/option/packer.go b/cmd/oras/internal/option/packer.go index 7217df5ee..a69b12f3a 100644 --- a/cmd/oras/internal/option/packer.go +++ b/cmd/oras/internal/option/packer.go @@ -39,26 +39,26 @@ const ( ) 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") + errAnnotationConflict = errors.New("`--annotation` and `--annotation-file` cannot be both specified") + 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) { + opts.Annotation.ApplyFlags(fs) + 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") } @@ -74,7 +74,8 @@ func (opts *Packer) ExportManifest(ctx context.Context, fetcher content.Fetcher, } return os.WriteFile(opts.ManifestExportPath, manifestBytes, 0666) } -func (opts *Packer) Parse(*cobra.Command) error { + +func (opts *Packer) Parse(cmd *cobra.Command) error { if !opts.PathValidationDisabled { var failedPaths []string for _, path := range opts.FileRefs { @@ -91,29 +92,26 @@ func (opts *Packer) Parse(*cobra.Command) error { return fmt.Errorf("%w: %v", errPathValidation, strings.Join(failedPaths, ", ")) } } - return nil + return opts.parseAnnotations(cmd) } -// LoadManifestAnnotations loads the manifest annotation map. -func (opts *Packer) LoadManifestAnnotations() (annotations map[string]map[string]string, err error) { +// parseAnnotations loads the manifest annotation map. +func (opts *Packer) parseAnnotations(cmd *cobra.Command) error { if opts.AnnotationFilePath != "" && len(opts.ManifestAnnotations) != 0 { - return nil, errAnnotationConflict + return errAnnotationConflict } if opts.AnnotationFilePath != "" { - if err = decodeJSON(opts.AnnotationFilePath, &annotations); err != nil { - return nil, &oerrors.Error{ + if err := decodeJSON(opts.AnnotationFilePath, &opts.Annotations); err != nil { + return &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 opts.Annotation.Parse(cmd) } - return + return nil } // decodeJSON decodes a json file v to filename. @@ -125,23 +123,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 -} diff --git a/cmd/oras/internal/option/packer_test.go b/cmd/oras/internal/option/packer_test.go index e0cd51339..35248ccd7 100644 --- a/cmd/oras/internal/option/packer_test.go +++ b/cmd/oras/internal/option/packer_test.go @@ -37,62 +37,73 @@ func TestPacker_FlagInit(t *testing.T) { ApplyFlags(&test, pflag.NewFlagSet("oras-test", pflag.ExitOnError)) } -func TestPacker_LoadManifestAnnotations_err(t *testing.T) { +func TestPacker_parseAnnotations_err(t *testing.T) { opts := Packer{ - AnnotationFilePath: "this is not a file", // testFile, - ManifestAnnotations: []string{"Key=Val"}, + Annotation: Annotation{ + ManifestAnnotations: []string{"Key=Val"}, + }, + AnnotationFilePath: "this is not a file", // testFile, } - if _, err := opts.LoadManifestAnnotations(); !errors.Is(err, errAnnotationConflict) { + if err := opts.parseAnnotations(nil); !errors.Is(err, errAnnotationConflict) { t.Fatalf("unexpected error: %v", err) } opts = Packer{ AnnotationFilePath: "this is not a file", // testFile, } - if _, err := opts.LoadManifestAnnotations(); err == nil { + if err := opts.parseAnnotations(nil); 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) { + if err := opts.parseAnnotations(nil); !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) { + if err := opts.parseAnnotations(nil); !errors.Is(err, errAnnotationDuplication) { t.Fatalf("unexpected error: %v", err) } } -func TestPacker_LoadManifestAnnotations_annotationFile(t *testing.T) { +func TestPacker_parseAnnotations_annotationFile(t *testing.T) { testFile := filepath.Join(t.TempDir(), "testAnnotationFile") err := os.WriteFile(testFile, []byte(testContent), fs.ModePerm) if err != nil { t.Fatalf("Error writing %s: %v", testFile, err) } - opts := Packer{AnnotationFilePath: testFile} + opts := Packer{ + AnnotationFilePath: testFile, + } - anno, err := opts.LoadManifestAnnotations() + err = opts.parseAnnotations(nil) if err != nil { t.Fatalf("unexpected error: %v", err) } - if !reflect.DeepEqual(anno, expectedResult) { - t.Fatalf("unexpected error: %v", anno) + if !reflect.DeepEqual(opts.Annotations, expectedResult) { + t.Fatalf("unexpected error: %v", opts.Annotations) } } -func TestPacker_LoadManifestAnnotations_annotationFlag(t *testing.T) { +func TestPacker_parseAnnotations_annotationFlag(t *testing.T) { // Item do not contains '=' invalidFlag0 := []string{ "Key", } - var annotations map[string]map[string]string - opts := Packer{ManifestAnnotations: invalidFlag0} - _, err := opts.LoadManifestAnnotations() + opts := Packer{ + Annotation: Annotation{ + ManifestAnnotations: invalidFlag0, + }, + } + err := opts.parseAnnotations(nil) if !errors.Is(err, errAnnotationFormat) { t.Fatalf("unexpected error: %v", err) } @@ -102,8 +113,12 @@ func TestPacker_LoadManifestAnnotations_annotationFlag(t *testing.T) { "Key=0", "Key=1", } - opts = Packer{ManifestAnnotations: invalidFlag1} - _, err = opts.LoadManifestAnnotations() + opts = Packer{ + Annotation: Annotation{ + ManifestAnnotations: invalidFlag1, + }, + } + err = opts.parseAnnotations(nil) if !errors.Is(err, errAnnotationDuplication) { t.Fatalf("unexpected error: %v", err) } @@ -114,15 +129,19 @@ 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} - annotations, err = opts.LoadManifestAnnotations() + opts = Packer{ + Annotation: Annotation{ + ManifestAnnotations: validFlag, + }, + } + err = opts.parseAnnotations(nil) if err != nil { t.Fatalf("unexpected error: %v", err) } - if _, ok := annotations["$manifest"]; !ok { + if _, ok := opts.Annotations["$manifest"]; !ok { t.Fatalf("unexpected error: failed when looking for '$manifest' in annotations") } - if !reflect.DeepEqual(annotations, + if !reflect.DeepEqual(opts.Annotations, map[string]map[string]string{ "$manifest": { "Key0": "", diff --git a/cmd/oras/root/attach.go b/cmd/oras/root/attach.go index 37d2ddf98..1531927d5 100644 --- a/cmd/oras/root/attach.go +++ b/cmd/oras/root/attach.go @@ -125,11 +125,7 @@ Example - Attach file to the manifest tagged 'v1' in an OCI image layout folder func runAttach(cmd *cobra.Command, opts *attachOptions) error { ctx, logger := command.GetLogger(cmd, &opts.Common) - annotations, err := opts.LoadManifestAnnotations() - if err != nil { - return err - } - if len(opts.FileRefs) == 0 && len(annotations[option.AnnotationManifest]) == 0 { + if len(opts.FileRefs) == 0 && len(opts.Annotations[option.AnnotationManifest]) == 0 { return &oerrors.Error{ Err: errors.New(`neither file nor annotation provided in the command`), Usage: fmt.Sprintf("%s %s", cmd.Parent().CommandPath(), cmd.Use), @@ -161,7 +157,7 @@ func runAttach(cmd *cobra.Command, opts *attachOptions) error { if err != nil { return err } - descs, err := loadFiles(ctx, store, annotations, opts.FileRefs, displayStatus) + descs, err := loadFiles(ctx, store, opts.Annotations, opts.FileRefs, displayStatus) if err != nil { return err } @@ -179,7 +175,7 @@ func runAttach(cmd *cobra.Command, opts *attachOptions) error { packOpts := oras.PackManifestOptions{ Subject: &subject, - ManifestAnnotations: annotations[option.AnnotationManifest], + ManifestAnnotations: opts.Annotations[option.AnnotationManifest], Layers: descs, } pack := func() (ocispec.Descriptor, error) { diff --git a/cmd/oras/root/attach_test.go b/cmd/oras/root/attach_test.go index 7bc666377..895bb30be 100644 --- a/cmd/oras/root/attach_test.go +++ b/cmd/oras/root/attach_test.go @@ -26,18 +26,20 @@ import ( ) func Test_runAttach_errType(t *testing.T) { - // prpare + // prepare cmd := &cobra.Command{} cmd.SetContext(context.Background()) // test opts := &attachOptions{ Packer: option.Packer{ - AnnotationFilePath: "/tmp/whatever", - ManifestAnnotations: []string{"one", "two"}, + Annotation: option.Annotation{ + ManifestAnnotations: []string{"one", "two"}, + }, + AnnotationFilePath: "/tmp/whatever", }, } - got := runAttach(cmd, opts).Error() + got := opts.Packer.Parse(cmd).Error() want := errors.New("`--annotation` and `--annotation-file` cannot be both specified").Error() if got != want { t.Fatalf("got %v, want %v", got, want) diff --git a/cmd/oras/root/manifest/index/create.go b/cmd/oras/root/manifest/index/create.go index 1d6164d5d..12b51e95d 100644 --- a/cmd/oras/root/manifest/index/create.go +++ b/cmd/oras/root/manifest/index/create.go @@ -47,6 +47,7 @@ type createOptions struct { option.Common option.Target option.Pretty + option.Annotation sources []string extraRefs []string @@ -72,6 +73,9 @@ Example - Create an index from source manifests using both tags and digests, and Example - Create an index and push it with multiple tags: oras manifest index create localhost:5000/hello:tag1,tag2,tag3 linux-amd64 linux-arm64 sha256:99e4703fbf30916f549cd6bfa9cdbab614b5392fbe64fdee971359a77073cdf9 +Example - Create and push an index with annotations: + oras manifest index create localhost:5000/hello:v1 linux-amd64 --annotation "key=val" + Example - Create an index and push to an OCI image layout folder 'layout-dir' and tag with 'v1': oras manifest index create layout-dir:v1 linux-amd64 sha256:99e4703fbf30916f549cd6bfa9cdbab614b5392fbe64fdee971359a77073cdf9 @@ -113,8 +117,9 @@ func createIndex(cmd *cobra.Command, opts createOptions) error { Versioned: specs.Versioned{ SchemaVersion: 2, }, - MediaType: ocispec.MediaTypeImageIndex, - Manifests: manifests, + MediaType: ocispec.MediaTypeImageIndex, + Manifests: manifests, + Annotations: opts.Annotations[option.AnnotationManifest], } indexBytes, err := json.Marshal(index) if err != nil { diff --git a/cmd/oras/root/push.go b/cmd/oras/root/push.go index f8afa6a3c..92ad30e5e 100644 --- a/cmd/oras/root/push.go +++ b/cmd/oras/root/push.go @@ -157,15 +157,11 @@ Example - Push file "hi.txt" into an OCI image layout folder 'layout-dir' with t func runPush(cmd *cobra.Command, opts *pushOptions) error { ctx, logger := command.GetLogger(cmd, &opts.Common) - annotations, err := opts.LoadManifestAnnotations() - if err != nil { - return err - } // prepare pack packOpts := oras.PackManifestOptions{ - ConfigAnnotations: annotations[option.AnnotationConfig], - ManifestAnnotations: annotations[option.AnnotationManifest], + ConfigAnnotations: opts.Annotations[option.AnnotationConfig], + ManifestAnnotations: opts.Annotations[option.AnnotationManifest], } store, err := file.New("") if err != nil { @@ -190,7 +186,7 @@ func runPush(cmd *cobra.Command, opts *pushOptions) error { if err != nil { return err } - descs, err := loadFiles(ctx, store, annotations, opts.FileRefs, displayStatus) + descs, err := loadFiles(ctx, store, opts.Annotations, opts.FileRefs, displayStatus) if err != nil { return err } diff --git a/test/e2e/suite/command/manifest_index.go b/test/e2e/suite/command/manifest_index.go index d6da1797a..0bcbd8a79 100644 --- a/test/e2e/suite/command/manifest_index.go +++ b/test/e2e/suite/command/manifest_index.go @@ -137,6 +137,19 @@ var _ = Describe("1.1 registry users:", func() { ValidateIndex(content, expectedManifests) }) + It("should create index with annotations", func() { + testRepo := indexTestRepo("create", "with-annotations") + key := "image-anno-key" + value := "image-anno-value" + CopyZOTRepo(ImageRepo, testRepo) + ORAS("manifest", "index", "create", RegistryRef(ZOTHost, testRepo, "v1"), "--annotation", fmt.Sprintf("%s=%s", key, value)).Exec() + // verify + content := ORAS("manifest", "fetch", RegistryRef(ZOTHost, testRepo, "v1")).Exec().Out.Contents() + var manifest ocispec.Manifest + Expect(json.Unmarshal(content, &manifest)).ShouldNot(HaveOccurred()) + Expect(manifest.Annotations[key]).To(Equal(value)) + }) + It("should output created index to file", func() { testRepo := indexTestRepo("create", "output-to-file") CopyZOTRepo(ImageRepo, testRepo) @@ -159,6 +172,14 @@ var _ = Describe("1.1 registry users:", func() { "does-not-exist").ExpectFailure(). MatchErrKeyWords("Error", "could not find", "does-not-exist").Exec() }) + + It("should fail if given annotation input of wrong format", func() { + testRepo := indexTestRepo("create", "bad-annotations") + CopyZOTRepo(ImageRepo, testRepo) + ORAS("manifest", "index", "create", RegistryRef(ZOTHost, testRepo, ""), + string(multi_arch.LinuxAMD64.Digest), "-a", "foo:bar").ExpectFailure(). + MatchErrKeyWords("Error", "annotation value doesn't match the required format").Exec() + }) }) When("running `manifest index update`", func() { @@ -405,6 +426,19 @@ var _ = Describe("OCI image layout users:", func() { ValidateIndex(content, expectedManifests) }) + It("should create index with annotations", func() { + root := PrepareTempOCI(ImageRepo) + indexRef := LayoutRef(root, "with-annotations") + key := "image-anno-key" + value := "image-anno-value" + ORAS("manifest", "index", "create", Flags.Layout, indexRef, "--annotation", fmt.Sprintf("%s=%s", key, value)).Exec() + // verify + content := ORAS("manifest", "fetch", Flags.Layout, indexRef).Exec().Out.Contents() + var manifest ocispec.Manifest + Expect(json.Unmarshal(content, &manifest)).ShouldNot(HaveOccurred()) + Expect(manifest.Annotations[key]).To(Equal(value)) + }) + It("should output created index to file", func() { root := PrepareTempOCI(ImageRepo) indexRef := LayoutRef(root, "output-to-file")