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: 4 additions & 7 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,9 +144,7 @@ func runCopy(opts copyOptions) error {
copyOptions := oras.CopyOptions{
CopyGraphOptions: extendedCopyOptions.CopyGraphOptions,
}
if targetPlatform != nil {
copyOptions.WithTargetPlatform(targetPlatform)
}
copyOptions.WithTargetPlatform(opts.OCIPlatform)
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.OCIPlatform
desc, err := oras.Resolve(ctx, repo, repo.Reference.Reference, resolveOpts)
if err != nil {
return err
Expand Down
41 changes: 41 additions & 0 deletions cmd/oras/internal/option/parser.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
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 {
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.(FlagParser); ok {
if err := a.Parse(); err != nil {
return err
}
}
}
}
return nil
}
qweeah marked this conversation as resolved.
Show resolved Hide resolved
53 changes: 53 additions & 0 deletions cmd/oras/internal/option/parser_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/*
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 (
"testing"

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

type Test struct {
Cnt int
}

func (t *Test) Parse() error {
t.Cnt += 1
return nil
}

func TestParse(t *testing.T) {
type args struct {
Test
}
tests := []struct {
name string
args args
wantErr bool
}{
{"parse should be called", args{Test{Cnt: 0}}, 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 tt.args.Cnt != 1 {
t.Errorf("Expect Parse() to be called once but got %v", tt.args.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
OCIPlatform *ocispec.Platform
qweeah marked this conversation as resolved.
Show resolved Hide resolved
}

// 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.OCIPlatform = &p
return nil
}
37 changes: 17 additions & 20 deletions cmd/oras/internal/option/platform_test.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
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
Expand All @@ -27,8 +25,8 @@ import (
func TestPlatform_ApplyFlags(t *testing.T) {
var test struct{ Platform }
ApplyFlags(&test, pflag.NewFlagSet("oras-test", pflag.ExitOnError))
if test.Platform.Platform != "" {
t.Fatalf("expecting platform to be empty but got: %v", test.Platform.Platform)
if test.Platform.platform != "" {
t.Fatalf("expecting platform to be empty but got: %v", test.Platform.platform)
}
}

Expand All @@ -37,15 +35,15 @@ func TestPlatform_Parse_err(t *testing.T) {
name string
opts *Platform
}{
{name: "empty arch 1", opts: &Platform{"os/"}},
{name: "empty arch 2", opts: &Platform{"os//variant"}},
{name: "empty os", opts: &Platform{"/arch"}},
{name: "empty os with variant", opts: &Platform{"/arch/variant"}},
{name: "trailing slash", opts: &Platform{"os/arch/variant/llama"}},
{name: "empty arch 1", opts: &Platform{"os/", nil}},
{name: "empty arch 2", opts: &Platform{"os//variant", nil}},
{name: "empty os", opts: &Platform{"/arch", nil}},
{name: "empty os with variant", opts: &Platform{"/arch/variant", nil}},
{name: "trailing slash", opts: &Platform{"os/arch/variant/llama", nil}},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
_, err := tt.opts.Parse()
err := tt.opts.Parse()
if err == nil {
t.Errorf("Platform.Parse() error = %v, wantErr %v", err, true)
return
Expand All @@ -60,21 +58,20 @@ func TestPlatform_Parse(t *testing.T) {
opts *Platform
want *ocispec.Platform
}{
{name: "empty", opts: &Platform{""}, want: nil},
{name: "default arch", opts: &Platform{"os"}, want: &ocispec.Platform{OS: "os", Architecture: runtime.GOARCH}},
{name: "os&arch", opts: &Platform{"os/aRcH"}, want: &ocispec.Platform{OS: "os", Architecture: "aRcH"}},
{name: "empty variant", opts: &Platform{"os/aRcH/"}, want: &ocispec.Platform{OS: "os", Architecture: "aRcH", Variant: ""}},
{name: "os&arch&variant", opts: &Platform{"os/aRcH/vAriAnt"}, want: &ocispec.Platform{OS: "os", Architecture: "aRcH", Variant: "vAriAnt"}},
{name: "os version", opts: &Platform{"os/aRcH/vAriAnt:osversion"}, want: &ocispec.Platform{OS: "os", Architecture: "aRcH", Variant: "vAriAnt", OSVersion: "osversion"}},
{name: "long os version", opts: &Platform{"os/aRcH"}, want: &ocispec.Platform{OS: "os", Architecture: "aRcH"}},
{name: "empty", opts: &Platform{platform: ""}, want: nil},
{name: "default arch", opts: &Platform{platform: "os"}, want: &ocispec.Platform{OS: "os", Architecture: runtime.GOARCH}},
{name: "os&arch", opts: &Platform{platform: "os/aRcH"}, want: &ocispec.Platform{OS: "os", Architecture: "aRcH"}},
{name: "empty variant", opts: &Platform{platform: "os/aRcH/"}, want: &ocispec.Platform{OS: "os", Architecture: "aRcH", Variant: ""}},
{name: "os&arch&variant", opts: &Platform{platform: "os/aRcH/vAriAnt"}, want: &ocispec.Platform{OS: "os", Architecture: "aRcH", Variant: "vAriAnt"}},
{name: "os version", opts: &Platform{platform: "os/aRcH/vAriAnt:osversion"}, want: &ocispec.Platform{OS: "os", Architecture: "aRcH", Variant: "vAriAnt", OSVersion: "osversion"}},
{name: "long os version", opts: &Platform{platform: "os/aRcH"}, want: &ocispec.Platform{OS: "os", Architecture: "aRcH"}},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := tt.opts.Parse()
if err != nil {
if err := tt.opts.Parse(); err != nil {
t.Errorf("Platform.Parse() error = %v", err)
return
}
got := tt.opts.OCIPlatform
if !reflect.DeepEqual(got, tt.want) {
t.Errorf("Platform.Parse() = %v, want %v", got, tt.want)
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/oras/internal/option/remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ func (opts *Remote) ApplyFlagsWithPrefix(fs *pflag.FlagSet, prefix, description
}
}

// ReadPassword tries to read password with optional cmd prompt.
func (opts *Remote) ReadPassword() (err error) {
// Parse tries to read password with optional cmd prompt.
func (opts *Remote) Parse() error {
qweeah marked this conversation as resolved.
Show resolved Hide resolved
if opts.Password != "" {
fmt.Fprintln(os.Stderr, "WARNING! Using --password via the CLI is insecure. Use --password-stdin.")
} else if opts.PasswordFromStdin {
Expand Down
8 changes: 4 additions & 4 deletions cmd/oras/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ Example - Log in with username and password in an interactive terminal and no TL
`,
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.Hostname = args[0]
Expand Down Expand Up @@ -130,9 +130,9 @@ func runLogin(opts loginOptions) (err error) {
return nil
}

func readLine(prompt string, slient bool) (string, error) {
func readLine(prompt string, silent bool) (string, error) {
fmt.Print(prompt)
if slient {
if silent {
fd := os.Stdin.Fd()
state, err := term.SaveState(fd)
if err != nil {
Expand All @@ -147,7 +147,7 @@ func readLine(prompt string, slient bool) (string, error) {
if err != nil {
return "", err
}
if slient {
if silent {
fmt.Println()
}

Expand Down
2 changes: 1 addition & 1 deletion cmd/oras/manifest/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ Example - Delete a manifest by digest 'sha256:99e4703fbf30916f549cd6bfa9cdbab614
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(_ *cobra.Command, args []string) error {
opts.targetRef = args[0]
Expand Down
Loading