Skip to content

Commit

Permalink
refactor(oci-layout): use Parser to process option input (#736)
Browse files Browse the repository at this point in the history
This PR adds a new option parser which can be used to process input
before cmd execution. This change will be used in OCI layout support.

Signed-off-by: Billy Zha <[email protected]>
  • Loading branch information
qweeah authored Jan 10, 2023
1 parent 1659011 commit dd279cd
Show file tree
Hide file tree
Showing 23 changed files with 257 additions and 94 deletions.
2 changes: 1 addition & 1 deletion cmd/oras/attach.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ Example - Attach file 'hi.txt' and export the pushed manifest to 'manifest.json'
`,
Args: cobra.MinimumNArgs(1),
PreRunE: func(cmd *cobra.Command, args []string) error {
return opts.ReadPassword()
return option.Parse(&opts)
},
RunE: func(cmd *cobra.Command, args []string) error {
opts.targetRef = args[0]
Expand Down
2 changes: 1 addition & 1 deletion cmd/oras/blob/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ Example - Delete a blob and print its descriptor:
if opts.OutputDescriptor && !opts.Confirmed {
return errors.New("must apply --force to confirm the deletion if the descriptor is outputted")
}
return opts.ReadPassword()
return option.Parse(&opts)
},
RunE: func(cmd *cobra.Command, args []string) error {
opts.targetRef = args[0]
Expand Down
2 changes: 1 addition & 1 deletion cmd/oras/blob/fetch.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ Example - Fetch the blob, save it to a local file and print the descriptor:
return errors.New("`--output -` cannot be used with `--descriptor` at the same time")
}

return opts.ReadPassword()
return option.Parse(&opts)
},
Aliases: []string{"get"},
RunE: func(cmd *cobra.Command, args []string) error {
Expand Down
2 changes: 1 addition & 1 deletion cmd/oras/blob/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ Example - Push blob without TLS:
return errors.New("`--size` must be provided if the blob is read from stdin")
}
}
return opts.ReadPassword()
return option.Parse(&opts)
},
RunE: func(cmd *cobra.Command, args []string) error {
return pushBlob(opts)
Expand Down
11 changes: 5 additions & 6 deletions cmd/oras/cp.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ Example - Copy the artifact tagged with 'v1' from repository 'localhost:5000/net
oras cp --concurrency 6 localhost:5000/net-monitor:v1 localhost:5000/net-monitor-copy:v1,tag2,tag3
`,
Args: cobra.ExactArgs(2),
PreRunE: func(cmd *cobra.Command, args []string) error {
return option.Parse(&opts)
},
RunE: func(cmd *cobra.Command, args []string) error {
opts.srcRef = args[0]
refs := strings.Split(args[1], ",")
Expand All @@ -88,10 +91,6 @@ Example - Copy the artifact tagged with 'v1' from repository 'localhost:5000/net

func runCopy(opts copyOptions) error {
ctx, _ := opts.SetLoggerLevel()
targetPlatform, err := opts.Parse()
if err != nil {
return err
}

// Prepare source
src, err := opts.src.NewRepository(opts.srcRef, opts.Common)
Expand Down Expand Up @@ -145,8 +144,8 @@ func runCopy(opts copyOptions) error {
copyOptions := oras.CopyOptions{
CopyGraphOptions: extendedCopyOptions.CopyGraphOptions,
}
if targetPlatform != nil {
copyOptions.WithTargetPlatform(targetPlatform)
if opts.Platform.Platform != nil {
copyOptions.WithTargetPlatform(opts.Platform.Platform)
}
desc, err = oras.Copy(ctx, src, opts.srcRef, dst, opts.dstRef, copyOptions)
}
Expand Down
8 changes: 2 additions & 6 deletions cmd/oras/discover.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ Example - Discover referrers with type 'test-artifact' of manifest 'hello:latest
`,
Args: cobra.ExactArgs(1),
PreRunE: func(cmd *cobra.Command, args []string) error {
return opts.ReadPassword()
return option.Parse(&opts)
},
RunE: func(cmd *cobra.Command, args []string) error {
opts.targetRef = args[0]
Expand All @@ -87,14 +87,10 @@ func runDiscover(opts discoverOptions) error {
if repo.Reference.Reference == "" {
return errors.NewErrInvalidReference(repo.Reference)
}
targetPlatform, err := opts.Parse()
if err != nil {
return err
}

// discover artifacts
resolveOpts := oras.DefaultResolveOptions
resolveOpts.TargetPlatform = targetPlatform
resolveOpts.TargetPlatform = opts.Platform.Platform
desc, err := oras.Resolve(ctx, repo, repo.Reference.Reference, resolveOpts)
if err != nil {
return err
Expand Down
16 changes: 4 additions & 12 deletions cmd/oras/internal/option/applier.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ limitations under the License.
package option

import (
"reflect"

"github.com/spf13/pflag"
)

Expand All @@ -31,14 +29,8 @@ type FlagApplier interface {
// NOTE: The option argument need to be a pointer to the options, so its value
// becomes addressable.
func ApplyFlags(optsPtr interface{}, target *pflag.FlagSet) {
v := reflect.ValueOf(optsPtr).Elem()
for i := 0; i < v.NumField(); i++ {
f := v.Field(i)
if f.CanSet() {
iface := f.Addr().Interface()
if a, ok := iface.(FlagApplier); ok {
a.ApplyFlags(target)
}
}
}
rangeFields(optsPtr, func(fa FlagApplier) error {
fa.ApplyFlags(target)
return nil
})
}
49 changes: 49 additions & 0 deletions cmd/oras/internal/option/applier_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/*
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_test

import (
"testing"

"github.com/spf13/pflag"
"oras.land/oras/cmd/oras/internal/option"
)

func (t *Test) ApplyFlags(fs *pflag.FlagSet) {
*t.CntPtr += 1
}

func TestApplyFlags(t *testing.T) {
cnt := 0
type args struct {
Test
}
tests := []struct {
name string
args args
wantErr bool
}{
{"flags should be applied once", args{Test{CntPtr: &cnt}}, false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
option.ApplyFlags(&tt.args, nil)
if cnt != 1 {
t.Errorf("Expect ApplyFlags() to be called once but got %v", cnt)
}
})
}
}
51 changes: 51 additions & 0 deletions cmd/oras/internal/option/parser.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*
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 (
"reflect"
)

// FlagParser parses flags in an option.
type FlagParser interface {
Parse() error
}

// Parse parses applicable fields of the passed-in option pointer and returns
// error during parsing.
func Parse(optsPtr interface{}) error {
return rangeFields(optsPtr, func(fp FlagParser) error {
return fp.Parse()
})
}

// rangeFields goes through all fields of ptr, optionally run fn if a field is
// public AND typed T.
func rangeFields[T any](ptr any, fn func(T) error) error {
v := reflect.ValueOf(ptr).Elem()
for i := 0; i < v.NumField(); i++ {
f := v.Field(i)
if f.CanSet() {
iface := f.Addr().Interface()
if opts, ok := iface.(T); ok {
if err := fn(opts); err != nil {
return err
}
}
}
}
return nil
}
88 changes: 88 additions & 0 deletions cmd/oras/internal/option/parser_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
/*
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_test

import (
"errors"
"testing"

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

type Test struct {
CntPtr *int
}

func (t *Test) Parse() error {
*t.CntPtr += 1
if *t.CntPtr == 2 {
return errors.New("should not be tried twice")
}
return nil
}

func TestParse_once(t *testing.T) {
cnt := 0
type args struct {
Test
}
tests := []struct {
name string
args args
wantErr bool
}{
{"parse should be called once", args{Test{CntPtr: &cnt}}, false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if err := option.Parse(&tt.args); (err != nil) != tt.wantErr {
t.Errorf("Parse() error = %v, wantErr %v", err, tt.wantErr)
}

if cnt != 1 {
t.Errorf("Expect Parse() to be called once but got %v", cnt)
}
})
}
}

func TestParse_err(t *testing.T) {
cnt := 0
type args struct {
Test1 Test
Test2 Test
Test3 Test
Test4 Test
}
tests := []struct {
name string
args args
wantErr bool
}{
{"parse should be called twice and aborted with error", args{Test{CntPtr: &cnt}, Test{CntPtr: &cnt}, Test{CntPtr: &cnt}, Test{CntPtr: &cnt}}, true},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if err := option.Parse(&tt.args); (err != nil) != tt.wantErr {
t.Errorf("Parse() error = %v, wantErr %v", err, tt.wantErr)
}

if cnt != 2 {
t.Errorf("Expect Parse() to be called twice but got %v", cnt)
}
})
}
}
22 changes: 12 additions & 10 deletions cmd/oras/internal/option/platform.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,25 +26,26 @@ import (

// Platform option struct.
type Platform struct {
Platform string
platform string
Platform *ocispec.Platform
}

// ApplyFlags applies flags to a command flag set.
func (opts *Platform) ApplyFlags(fs *pflag.FlagSet) {
fs.StringVarP(&opts.Platform, "platform", "", "", "request platform in the form of `os[/arch][/variant][:os_version]`")
fs.StringVarP(&opts.platform, "platform", "", "", "request platform in the form of `os[/arch][/variant][:os_version]`")
}

// parse parses the input platform flag to an oci platform type.
func (opts *Platform) Parse() (*ocispec.Platform, error) {
if opts.Platform == "" {
return nil, nil
func (opts *Platform) Parse() error {
if opts.platform == "" {
return nil
}

// OS[/Arch[/Variant]][:OSVersion]
// If Arch is not provided, will use GOARCH instead
var platformStr string
var p ocispec.Platform
platformStr, p.OSVersion, _ = strings.Cut(opts.Platform, ":")
platformStr, p.OSVersion, _ = strings.Cut(opts.platform, ":")
parts := strings.Split(platformStr, "/")
switch len(parts) {
case 3:
Expand All @@ -55,14 +56,15 @@ func (opts *Platform) Parse() (*ocispec.Platform, error) {
case 1:
p.Architecture = runtime.GOARCH
default:
return nil, fmt.Errorf("failed to parse platform %q: expected format os[/arch[/variant]]", opts.Platform)
return fmt.Errorf("failed to parse platform %q: expected format os[/arch[/variant]]", opts.platform)
}
p.OS = parts[0]
if p.OS == "" {
return nil, fmt.Errorf("invalid platform: OS cannot be empty")
return fmt.Errorf("invalid platform: OS cannot be empty")
}
if p.Architecture == "" {
return nil, fmt.Errorf("invalid platform: Architecture cannot be empty")
return fmt.Errorf("invalid platform: Architecture cannot be empty")
}
return &p, nil
opts.Platform = &p
return nil
}
Loading

0 comments on commit dd279cd

Please sign in to comment.