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(oci-layout): use Parser to process option input #736

Merged
merged 16 commits into from
Jan 10, 2023
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
15 changes: 3 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,7 @@ 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, err *error) {
fa.ApplyFlags(target)
})
}
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.
*/
qweeah marked this conversation as resolved.
Show resolved Hide resolved

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, err *error) {
*err = 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 {
qweeah marked this conversation as resolved.
Show resolved Hide resolved
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 {
err := new(error)
fn(opts, err)
if *err != nil {
return *err
}
}
}
}
return nil
}
86 changes: 86 additions & 0 deletions cmd/oras/internal/option/parser_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
/*
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.
*/
qweeah marked this conversation as resolved.
Show resolved Hide resolved

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)
}
})
}
}
24 changes: 12 additions & 12 deletions cmd/oras/internal/option/platform.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@ 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
qweeah marked this conversation as resolved.
Show resolved Hide resolved
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
Expand All @@ -26,25 +24,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 +54,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