From 780de1639ac542555a00a02b0107a93036e1db6c Mon Sep 17 00:00:00 2001 From: zaihaoyin Date: Fri, 15 Jul 2022 16:54:28 +0800 Subject: [PATCH 1/7] Use cobra cli for docker-generate command Signed-off-by: zaihaoyin --- cmd/docker-generate/generate.go | 13 +++--- cmd/docker-generate/main.go | 14 ++---- cmd/docker-generate/manifest.go | 28 ++++++----- cmd/docker-generate/manifest_test.go | 69 ++++++++++++++++++++++++++++ cmd/docker-generate/metadata.go | 23 ++++++---- go.mod | 5 +- go.sum | 9 +++- internal/cmd/flags.go | 67 +++++++++++++++++++++++++++ 8 files changed, 185 insertions(+), 43 deletions(-) create mode 100644 cmd/docker-generate/manifest_test.go diff --git a/cmd/docker-generate/generate.go b/cmd/docker-generate/generate.go index 32ceecbc7..d0f3d9165 100644 --- a/cmd/docker-generate/generate.go +++ b/cmd/docker-generate/generate.go @@ -1,12 +1,13 @@ package main import ( - "github.com/urfave/cli/v2" + "github.com/spf13/cobra" ) -var generateCommand = &cli.Command{ - Name: "generate", - Subcommands: []*cli.Command{ - manifestCommand, - }, +func generateCommand() *cobra.Command { + cmd := &cobra.Command{ + Use: "generate", + } + cmd.AddCommand(generateManifestCommand()) + return cmd } diff --git a/cmd/docker-generate/main.go b/cmd/docker-generate/main.go index 86fef92cc..1ba2aa06a 100644 --- a/cmd/docker-generate/main.go +++ b/cmd/docker-generate/main.go @@ -2,20 +2,16 @@ package main import ( "log" - "os" - "github.com/urfave/cli/v2" + "github.com/spf13/cobra" ) func main() { - app := &cli.App{ - Name: "docker", - Commands: []*cli.Command{ - generateCommand, - metadataCommand, - }, + cmd := &cobra.Command{ + Use: "docker", } - if err := app.Run(os.Args); err != nil { + cmd.AddCommand(generateCommand(), metadataCommand()) + if err := cmd.Execute(); err != nil { log.Fatal(err) } } diff --git a/cmd/docker-generate/manifest.go b/cmd/docker-generate/manifest.go index 7c717b9a6..c35cf581f 100644 --- a/cmd/docker-generate/manifest.go +++ b/cmd/docker-generate/manifest.go @@ -6,26 +6,24 @@ import ( "os/exec" "github.com/notaryproject/notation/pkg/docker" - "github.com/urfave/cli/v2" + "github.com/spf13/cobra" ) -var manifestCommand = &cli.Command{ - Name: "manifest", - Usage: "generates the manifest of a docker image", - ArgsUsage: "[]", - Flags: []cli.Flag{ - &cli.StringFlag{ - Name: "output", - Aliases: []string{"o"}, - Usage: "write to a file instead of stdout", +func generateManifestCommand() *cobra.Command { + cmd := &cobra.Command{ + Use: "manifest [reference]", + Short: "generates the manifest of a docker image", + RunE: func(cmd *cobra.Command, args []string) error { + return generateManifest(cmd) }, - }, - Action: generateManifest, + } + cmd.Flags().StringP("output", "o", "", "write to a file instead of stdout") + return cmd } -func generateManifest(ctx *cli.Context) error { +func generateManifest(cmd *cobra.Command) error { var reader io.Reader - if reference := ctx.Args().First(); reference != "" { + if reference := cmd.Flags().Arg(0); reference != "" { cmd := exec.Command("docker", "save", reference) cmd.Stderr = os.Stderr stdout, err := cmd.StdoutPipe() @@ -41,7 +39,7 @@ func generateManifest(ctx *cli.Context) error { } var writer io.Writer - if output := ctx.String("output"); output != "" { + if output, _ := cmd.Flags().GetString("output"); output != "" { file, err := os.Create(output) if err != nil { return err diff --git a/cmd/docker-generate/manifest_test.go b/cmd/docker-generate/manifest_test.go new file mode 100644 index 000000000..acc69726c --- /dev/null +++ b/cmd/docker-generate/manifest_test.go @@ -0,0 +1,69 @@ +package main + +import ( + "testing" +) + +func TestGenerateManifestCmd(t *testing.T) { + tests := []struct { + expectedOutput string + expectedReference string + args []string + expectedErr bool + }{ + { + expectedOutput: "abc", + expectedReference: "def", + args: []string{"-o", "abc", "def"}, + expectedErr: false, + }, + { + expectedOutput: "abc", + expectedReference: "", + args: []string{"-o", "abc"}, + expectedErr: false, + }, + { + expectedOutput: "", + expectedReference: "def", + args: []string{"def"}, + expectedErr: false, + }, + { + expectedOutput: "", + expectedReference: "", + args: []string{}, + expectedErr: false, + }, + { + expectedOutput: "abc", + expectedReference: "def", + args: []string{"def", "--output", "abc"}, + expectedErr: false, + }, + { + args: []string{"-o", "b", "-n", "x"}, + expectedErr: true, + }, + } + for _, test := range tests { + cmd := generateManifestCommand() + err := cmd.ParseFlags(test.args) + if err != nil && !test.expectedErr { + t.Fatalf("Test failed with error: %v", err) + } + if err == nil && test.expectedErr { + t.Fatalf("Expect test to error but it didn't: %v", test.args) + } + if err != nil { + continue + } + if output, _ := cmd.Flags().GetString("output"); output != test.expectedOutput { + t.Fatalf("Expect output: %v, got: %v", test.expectedOutput, output) + } + if arg := cmd.Flags().Arg(0); arg != test.expectedReference { + t.Fatalf("Expect reference: %v, got: %v", test.expectedReference, arg) + } + } + +} diff --git a/cmd/docker-generate/metadata.go b/cmd/docker-generate/metadata.go index c7ad710f0..92995b1af 100644 --- a/cmd/docker-generate/metadata.go +++ b/cmd/docker-generate/metadata.go @@ -5,9 +5,21 @@ import ( "os" "github.com/notaryproject/notation/internal/docker" - "github.com/urfave/cli/v2" + "github.com/spf13/cobra" ) +func metadataCommand() *cobra.Command { + cmd := &cobra.Command{ + Use: docker.PluginMetadataCommandName, + RunE: func(cmd *cobra.Command, args []string) error { + writer := json.NewEncoder(os.Stdout) + return writer.Encode(pluginMetadata) + }, + Hidden: true, + } + return cmd +} + var pluginMetadata = docker.PluginMetadata{ SchemaVersion: "0.1.0", Vendor: "CNCF Notary Project", @@ -16,12 +28,3 @@ var pluginMetadata = docker.PluginMetadata{ URL: "https://github.com/notaryproject/notation", Experimental: true, } - -var metadataCommand = &cli.Command{ - Name: docker.PluginMetadataCommandName, - Action: func(ctx *cli.Context) error { - writer := json.NewEncoder(os.Stdout) - return writer.Encode(pluginMetadata) - }, - Hidden: true, -} diff --git a/go.mod b/go.mod index 0de6e2ac5..61367697e 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,9 @@ require ( github.com/notaryproject/notation-core-go v0.0.0-20220712013708-3c4b3efa03c5 github.com/notaryproject/notation-go v0.9.0-alpha.1.0.20220712175603-962d79cd4090 github.com/opencontainers/go-digest v1.0.0 - github.com/urfave/cli/v2 v2.10.3 + github.com/spf13/cobra v1.5.0 + github.com/spf13/pflag v1.0.5 + github.com/urfave/cli/v2 v2.11.0 oras.land/oras-go/v2 v2.0.0-20220620164807-8b2a54608a94 ) @@ -17,6 +19,7 @@ require ( github.com/docker/docker v20.10.8+incompatible // indirect github.com/docker/docker-credential-helpers v0.6.4 // indirect github.com/golang-jwt/jwt/v4 v4.4.2 // indirect + github.com/inconshreveable/mousetrap v1.0.0 // indirect github.com/opencontainers/image-spec v1.0.2 // indirect github.com/oras-project/artifacts-spec v1.0.0-rc.1 // indirect github.com/pkg/errors v0.9.1 // indirect diff --git a/go.sum b/go.sum index faf038734..492594e5f 100644 --- a/go.sum +++ b/go.sum @@ -53,6 +53,7 @@ github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/ github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/gorilla/handlers v1.5.1/go.mod h1:t8XrUpc4KVXb7HGyJ4/cEnwQiaxrX/hz1Zv/4g96P1Q= github.com/gorilla/mux v1.8.0/go.mod h1:DVbg23sWSpFRCP0SfiEN6jmj59UnW/n46BH5rLB71So= +github.com/inconshreveable/mousetrap v1.0.0 h1:Z8tu5sraLXCXIcARxBp/8cbvlwVa7Z1NHg9XEKhtSvM= github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8= github.com/jmespath/go-jmespath v0.3.0/go.mod h1:9QtRXoHjLGCJ5IBSaohpXITPlowMeeYCZ7fLUTSywik= github.com/json-iterator/go v1.1.6/go.mod h1:+SdeFBvtyEkXs7REEP0seUULqWtbJapLOCVDaaPEHmU= @@ -107,15 +108,19 @@ github.com/sirupsen/logrus v1.2.0/go.mod h1:LxeOpSwHxABJmUn/MG1IvRgCAasNZTLOkJPx github.com/sirupsen/logrus v1.8.1 h1:dJKuHgqk1NNQlqoA6BTlM1Wf9DOH3NBjQyu0h9+AZZE= github.com/sirupsen/logrus v1.8.1/go.mod h1:yWOB1SBYBC5VeMP7gHvWumXLIWorT60ONWic61uBYv0= github.com/spf13/cobra v0.0.3/go.mod h1:1l0Ry5zgKvJasoi3XT1TypsSe7PqH0Sj9dhYf7v3XqQ= +github.com/spf13/cobra v1.5.0 h1:X+jTBEBqF0bHN+9cSMgmfuvv2VHJ9ezmFNf9Y/XstYU= +github.com/spf13/cobra v1.5.0/go.mod h1:dWXEIy2H428czQCjInthrTRUg7yKbok+2Qi/yBIJoUM= github.com/spf13/pflag v1.0.3/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4= +github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= +github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.5.1 h1:nOGnQDM7FYENwehXlg/kFVnos3rEvtKTjRvOWSzb6H4= github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA= -github.com/urfave/cli/v2 v2.10.3 h1:oi571Fxz5aHugfBAJd5nkwSk3fzATXtMlpxdLylSCMo= -github.com/urfave/cli/v2 v2.10.3/go.mod h1:f8iq5LtQ/bLxafbdBSLPPNsgaW0l/2fYYEHhAyPlwvo= +github.com/urfave/cli/v2 v2.11.0 h1:c6bD90aLd2iEsokxhxkY5Er0zA2V9fId2aJfwmrF+do= +github.com/urfave/cli/v2 v2.11.0/go.mod h1:f8iq5LtQ/bLxafbdBSLPPNsgaW0l/2fYYEHhAyPlwvo= github.com/xrash/smetrics v0.0.0-20201216005158-039620a65673 h1:bAn7/zixMGCfxrRTfdpNzjtPYqr8smhKouy9mxVdGPU= github.com/xrash/smetrics v0.0.0-20201216005158-039620a65673/go.mod h1:N3UwUGtsrSj3ccvlPHLoLsHnpR27oXr4ZE984MbSER8= github.com/yvasiyarov/go-metrics v0.0.0-20140926110328-57bccd1ccd43/go.mod h1:aX5oPXxHm3bOH+xeAttToC8pqch2ScQN/JoXYupl6xs= diff --git a/internal/cmd/flags.go b/internal/cmd/flags.go index 6f4aab737..1439fa828 100644 --- a/internal/cmd/flags.go +++ b/internal/cmd/flags.go @@ -5,7 +5,10 @@ import ( "errors" "fmt" "strings" + "time" + "github.com/spf13/cobra" + "github.com/spf13/pflag" "github.com/urfave/cli/v2" ) @@ -53,6 +56,70 @@ var ( } ) +var ( + PflagKey = &pflag.Flag{ + Name: "key", + Shorthand: "k", + Usage: "signing key name", + } + SetPflagKey = func(cmd *cobra.Command) { + cmd.Flags().StringP(PflagKey.Name, PflagKey.Shorthand, "", PflagKey.Usage) + } + + PflagKeyFile = &pflag.Flag{ + Name: "key-file", + Usage: "signing key file", + } + SetPflagKeyFile = func(cmd *cobra.Command) { + cmd.Flags().String(PflagKeyFile.Name, "", PflagKeyFile.Usage) + } + + PflagCertFile = &pflag.Flag{ + Name: "cert-file", + Usage: "signing certificate file", + } + SetPflagCertFile = func(cmd *cobra.Command) { + cmd.Flags().String(PflagCertFile.Name, "", PflagCertFile.Usage) + } + + PflagTimestamp = &pflag.Flag{ + Name: "timestamp", + Shorthand: "t", + Usage: "timestamp the signed signature via the remote TSA", + } + SetPflagTimestamp = func(cmd *cobra.Command) { + cmd.Flags().StringP(PflagTimestamp.Name, PflagTimestamp.Shorthand, "", PflagTimestamp.Usage) + } + + PflagExpiry = &pflag.Flag{ + Name: "expiry", + Shorthand: "e", + Usage: "expire duration", + } + SetPflagExpiry = func(cmd *cobra.Command) { + cmd.Flags().DurationP(PflagExpiry.Name, PflagExpiry.Shorthand, time.Duration(0), PflagExpiry.Usage) + } + + PflagReference = &pflag.Flag{ + Name: "reference", + Shorthand: "r", + Usage: "original reference", + } + SetPflagReference = func(cmd *cobra.Command) { + cmd.Flags().StringP(PflagReference.Name, PflagReference.Shorthand, "", PflagReference.Usage) + } + + // TODO: cobra does not support string shorthand + PflagPluginConfig = &pflag.Flag{ + Name: "pluginConfig", + Shorthand: "c", + Usage: "list of comma-separated {key}={value} pairs that are passed as is to the plugin, refer plugin documentation to set appropriate values", + } + SetPflagPluginConfig = func(cmd *cobra.Command) { + cmd.Flags().StringP(PflagPluginConfig.Name, PflagPluginConfig.Shorthand, "", PflagPluginConfig.Usage) + } +) + // KeyValueSlice is a flag with type int type KeyValueSlice interface { Set(value string) error From ba2b971b7208303db7cc5f17f0e6c4830566a7ae Mon Sep 17 00:00:00 2001 From: zaihaoyin Date: Fri, 15 Jul 2022 23:26:56 +0800 Subject: [PATCH 2/7] refactor docker-generate test and metadata cmd Signed-off-by: zaihaoyin --- cmd/docker-generate/manifest_test.go | 22 +++++++++++----------- cmd/docker-generate/metadata.go | 3 +-- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/cmd/docker-generate/manifest_test.go b/cmd/docker-generate/manifest_test.go index acc69726c..56315354a 100644 --- a/cmd/docker-generate/manifest_test.go +++ b/cmd/docker-generate/manifest_test.go @@ -12,21 +12,21 @@ func TestGenerateManifestCmd(t *testing.T) { expectedErr bool }{ { - expectedOutput: "abc", - expectedReference: "def", - args: []string{"-o", "abc", "def"}, + expectedOutput: "output", + expectedReference: "reference", + args: []string{"-o", "output", "reference"}, expectedErr: false, }, { - expectedOutput: "abc", + expectedOutput: "output", expectedReference: "", - args: []string{"-o", "abc"}, + args: []string{"-o", "output"}, expectedErr: false, }, { expectedOutput: "", - expectedReference: "def", - args: []string{"def"}, + expectedReference: "reference", + args: []string{"reference"}, expectedErr: false, }, { @@ -36,13 +36,13 @@ func TestGenerateManifestCmd(t *testing.T) { expectedErr: false, }, { - expectedOutput: "abc", - expectedReference: "def", - args: []string{"def", "--output", "abc"}, + expectedOutput: "output", + expectedReference: "reference", + args: []string{"reference", "--output", "output"}, expectedErr: false, }, { - args: []string{"-o", "b", "-n", "x"}, + args: []string{"-o", "output", "-n", "reference"}, expectedErr: true, }, } diff --git a/cmd/docker-generate/metadata.go b/cmd/docker-generate/metadata.go index 92995b1af..87ef63aa0 100644 --- a/cmd/docker-generate/metadata.go +++ b/cmd/docker-generate/metadata.go @@ -9,7 +9,7 @@ import ( ) func metadataCommand() *cobra.Command { - cmd := &cobra.Command{ + return &cobra.Command{ Use: docker.PluginMetadataCommandName, RunE: func(cmd *cobra.Command, args []string) error { writer := json.NewEncoder(os.Stdout) @@ -17,7 +17,6 @@ func metadataCommand() *cobra.Command { }, Hidden: true, } - return cmd } var pluginMetadata = docker.PluginMetadata{ From ae2f5e0b92198384408346b7ce6d337750cb837c Mon Sep 17 00:00:00 2001 From: zaihaoyin Date: Mon, 18 Jul 2022 14:14:22 +0800 Subject: [PATCH 3/7] refactor the usage of cobra by using variable binding Signed-off-by: zaihaoyin --- cmd/docker-generate/generate_test.go | 11 +++++ cmd/docker-generate/manifest.go | 34 ++++++++++---- cmd/docker-generate/manifest_test.go | 68 ++++++++++++++++------------ cmd/docker-generate/metadata_test.go | 40 ++++++++++++++++ 4 files changed, 115 insertions(+), 38 deletions(-) create mode 100644 cmd/docker-generate/generate_test.go create mode 100644 cmd/docker-generate/metadata_test.go diff --git a/cmd/docker-generate/generate_test.go b/cmd/docker-generate/generate_test.go new file mode 100644 index 000000000..085ed9dd2 --- /dev/null +++ b/cmd/docker-generate/generate_test.go @@ -0,0 +1,11 @@ +package main + +import "testing" + +func TestGenerateCommand(t *testing.T) { + cmd := generateCommand() + subCmds := cmd.Commands() + if len(subCmds) != 1 { + t.Fatalf("Expect generate command have 1 subcommand, got: %v", len(subCmds)) + } +} diff --git a/cmd/docker-generate/manifest.go b/cmd/docker-generate/manifest.go index c35cf581f..bca5911cd 100644 --- a/cmd/docker-generate/manifest.go +++ b/cmd/docker-generate/manifest.go @@ -9,22 +9,38 @@ import ( "github.com/spf13/cobra" ) -func generateManifestCommand() *cobra.Command { +type generateManifestOpts struct { + output string + reference string +} + +func generateManifestCommandWithOpts() (*cobra.Command, *generateManifestOpts) { + var opts generateManifestOpts cmd := &cobra.Command{ Use: "manifest [reference]", Short: "generates the manifest of a docker image", + PreRun: func(cmd *cobra.Command, args []string) { + if len(args) > 0 { + opts.reference = args[0] + } + }, RunE: func(cmd *cobra.Command, args []string) error { - return generateManifest(cmd) + return generateManifest(cmd, opts) }, } - cmd.Flags().StringP("output", "o", "", "write to a file instead of stdout") - return cmd + cmd.Flags().StringVarP(&opts.output, "output", "o", "", "write to a file instead of stdout") + return cmd, &opts +} + +func generateManifestCommand() *cobra.Command { + command, _ := generateManifestCommandWithOpts() + return command } -func generateManifest(cmd *cobra.Command) error { +func generateManifest(cmd *cobra.Command, opts generateManifestOpts) error { var reader io.Reader - if reference := cmd.Flags().Arg(0); reference != "" { - cmd := exec.Command("docker", "save", reference) + if opts.reference != "" { + cmd := exec.Command("docker", "save", opts.reference) cmd.Stderr = os.Stderr stdout, err := cmd.StdoutPipe() if err != nil { @@ -39,8 +55,8 @@ func generateManifest(cmd *cobra.Command) error { } var writer io.Writer - if output, _ := cmd.Flags().GetString("output"); output != "" { - file, err := os.Create(output) + if opts.output != "" { + file, err := os.Create(opts.output) if err != nil { return err } diff --git a/cmd/docker-generate/manifest_test.go b/cmd/docker-generate/manifest_test.go index 56315354a..eace01973 100644 --- a/cmd/docker-generate/manifest_test.go +++ b/cmd/docker-generate/manifest_test.go @@ -6,40 +6,49 @@ import ( func TestGenerateManifestCmd(t *testing.T) { tests := []struct { - expectedOutput string - expectedReference string - args []string - expectedErr bool + generateManifestOpts + args []string + expectedErr bool }{ { - expectedOutput: "output", - expectedReference: "reference", - args: []string{"-o", "output", "reference"}, - expectedErr: false, + generateManifestOpts{ + reference: "reference", + output: "output", + }, + []string{"-o", "output", "reference"}, + false, }, { - expectedOutput: "output", - expectedReference: "", - args: []string{"-o", "output"}, - expectedErr: false, + generateManifestOpts{ + reference: "", + output: "output", + }, + []string{"-o", "output"}, + false, }, { - expectedOutput: "", - expectedReference: "reference", - args: []string{"reference"}, - expectedErr: false, + generateManifestOpts{ + reference: "reference", + output: "", + }, + []string{"reference"}, + false, }, { - expectedOutput: "", - expectedReference: "", - args: []string{}, - expectedErr: false, + generateManifestOpts{ + reference: "", + output: "", + }, + []string{}, + false, }, { - expectedOutput: "output", - expectedReference: "reference", - args: []string{"reference", "--output", "output"}, - expectedErr: false, + generateManifestOpts{ + reference: "reference", + output: "output", + }, + []string{"reference", "--output", "output"}, + false, }, { args: []string{"-o", "output", "-n", "reference"}, @@ -47,7 +56,7 @@ func TestGenerateManifestCmd(t *testing.T) { }, } for _, test := range tests { - cmd := generateManifestCommand() + cmd, opts := generateManifestCommandWithOpts() err := cmd.ParseFlags(test.args) if err != nil && !test.expectedErr { t.Fatalf("Test failed with error: %v", err) @@ -58,11 +67,12 @@ func TestGenerateManifestCmd(t *testing.T) { if err != nil { continue } - if output, _ := cmd.Flags().GetString("output"); output != test.expectedOutput { - t.Fatalf("Expect output: %v, got: %v", test.expectedOutput, output) + cmd.PreRun(cmd, cmd.Flags().Args()) + if opts.output != test.output { + t.Fatalf("Expect output: %v, got: %v", test.output, opts.output) } - if arg := cmd.Flags().Arg(0); arg != test.expectedReference { - t.Fatalf("Expect reference: %v, got: %v", test.expectedReference, arg) + if opts.reference != test.reference { + t.Fatalf("Expect reference: %v, got: %v", test.reference, opts.reference) } } diff --git a/cmd/docker-generate/metadata_test.go b/cmd/docker-generate/metadata_test.go new file mode 100644 index 000000000..5e51dc89e --- /dev/null +++ b/cmd/docker-generate/metadata_test.go @@ -0,0 +1,40 @@ +package main + +import ( + "encoding/json" + "io" + "os" + "testing" + + "github.com/notaryproject/notation/internal/docker" +) + +func TestMetdaDatCommand(t *testing.T) { + oldStdOut := os.Stdout + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("Create pipe for metadata cmd failed: %v", err) + + } + os.Stdout = w + cmd := metadataCommand() + if err := cmd.RunE(cmd, []string{}); err != nil { + t.Fatalf("Running metadata cmd failed: %v", err) + } + w.Close() + data, err := io.ReadAll(r) + if err != nil { + t.Fatalf("Read metadata from stdout failed: %v", err) + } + var got docker.PluginMetadata + if err := json.Unmarshal(data, &got); err != nil { + t.Fatalf("Unmarshal metadata failed: %v", err) + } + if got != pluginMetadata { + t.Fatalf("Expect Metadata: %v, got: %v", data, pluginMetadata) + } + defer func() { + os.Stdout = oldStdOut + r.Close() + }() +} From 863f57edb2099bdd2cc63334fd06daf2d5edce8d Mon Sep 17 00:00:00 2001 From: zaihaoyin Date: Mon, 18 Jul 2022 14:15:20 +0800 Subject: [PATCH 4/7] refactor flags Signed-off-by: zaihaoyin --- internal/cmd/flags.go | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/internal/cmd/flags.go b/internal/cmd/flags.go index 1439fa828..9381e8db1 100644 --- a/internal/cmd/flags.go +++ b/internal/cmd/flags.go @@ -62,24 +62,24 @@ var ( Shorthand: "k", Usage: "signing key name", } - SetPflagKey = func(cmd *cobra.Command) { - cmd.Flags().StringP(PflagKey.Name, PflagKey.Shorthand, "", PflagKey.Usage) + SetPflagKey = func(cmd *cobra.Command, p *string) { + cmd.Flags().StringVarP(p, PflagKey.Name, PflagKey.Shorthand, "", PflagKey.Usage) } PflagKeyFile = &pflag.Flag{ Name: "key-file", Usage: "signing key file", } - SetPflagKeyFile = func(cmd *cobra.Command) { - cmd.Flags().String(PflagKeyFile.Name, "", PflagKeyFile.Usage) + SetPflagKeyFile = func(cmd *cobra.Command, p *string) { + cmd.Flags().StringVar(p, PflagKeyFile.Name, "", PflagKeyFile.Usage) } PflagCertFile = &pflag.Flag{ Name: "cert-file", Usage: "signing certificate file", } - SetPflagCertFile = func(cmd *cobra.Command) { - cmd.Flags().String(PflagCertFile.Name, "", PflagCertFile.Usage) + SetPflagCertFile = func(cmd *cobra.Command, p *string) { + cmd.Flags().StringVar(p, PflagCertFile.Name, "", PflagCertFile.Usage) } PflagTimestamp = &pflag.Flag{ @@ -87,8 +87,8 @@ var ( Shorthand: "t", Usage: "timestamp the signed signature via the remote TSA", } - SetPflagTimestamp = func(cmd *cobra.Command) { - cmd.Flags().StringP(PflagTimestamp.Name, PflagTimestamp.Shorthand, "", PflagTimestamp.Usage) + SetPflagTimestamp = func(cmd *cobra.Command, p *string) { + cmd.Flags().StringVarP(p, PflagTimestamp.Name, PflagTimestamp.Shorthand, "", PflagTimestamp.Usage) } PflagExpiry = &pflag.Flag{ @@ -96,8 +96,8 @@ var ( Shorthand: "e", Usage: "expire duration", } - SetPflagExpiry = func(cmd *cobra.Command) { - cmd.Flags().DurationP(PflagExpiry.Name, PflagExpiry.Shorthand, time.Duration(0), PflagExpiry.Usage) + SetPflagExpiry = func(cmd *cobra.Command, p *time.Duration) { + cmd.Flags().DurationVarP(p, PflagExpiry.Name, PflagExpiry.Shorthand, time.Duration(0), PflagExpiry.Usage) } PflagReference = &pflag.Flag{ @@ -105,18 +105,18 @@ var ( Shorthand: "r", Usage: "original reference", } - SetPflagReference = func(cmd *cobra.Command) { - cmd.Flags().StringP(PflagReference.Name, PflagReference.Shorthand, "", PflagReference.Usage) + SetPflagReference = func(cmd *cobra.Command, p *string) { + cmd.Flags().StringVarP(p, PflagReference.Name, PflagReference.Shorthand, "", PflagReference.Usage) } - // TODO: cobra does not support string shorthand + // TODO: cobra does not support letter shorthand PflagPluginConfig = &pflag.Flag{ Name: "pluginConfig", Shorthand: "c", Usage: "list of comma-separated {key}={value} pairs that are passed as is to the plugin, refer plugin documentation to set appropriate values", } - SetPflagPluginConfig = func(cmd *cobra.Command) { - cmd.Flags().StringP(PflagPluginConfig.Name, PflagPluginConfig.Shorthand, "", PflagPluginConfig.Usage) + SetPflagPluginConfig = func(cmd *cobra.Command, p *string) { + cmd.Flags().StringVarP(p, PflagPluginConfig.Name, PflagPluginConfig.Shorthand, "", PflagPluginConfig.Usage) } ) From c31a9e0b83ba5e7514f073a602c972438c9a8146 Mon Sep 17 00:00:00 2001 From: zaihaoyin Date: Mon, 18 Jul 2022 14:36:56 +0800 Subject: [PATCH 5/7] add maximum arg Signed-off-by: zaihaoyin --- cmd/docker-generate/manifest.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/docker-generate/manifest.go b/cmd/docker-generate/manifest.go index bca5911cd..32be915b3 100644 --- a/cmd/docker-generate/manifest.go +++ b/cmd/docker-generate/manifest.go @@ -19,6 +19,7 @@ func generateManifestCommandWithOpts() (*cobra.Command, *generateManifestOpts) { cmd := &cobra.Command{ Use: "manifest [reference]", Short: "generates the manifest of a docker image", + Args: cobra.MaximumNArgs(1), PreRun: func(cmd *cobra.Command, args []string) { if len(args) > 0 { opts.reference = args[0] From 0070de207092c0dd36b965a3be8eab0c9619f416 Mon Sep 17 00:00:00 2001 From: zaihaoyin Date: Mon, 18 Jul 2022 15:17:02 +0800 Subject: [PATCH 6/7] remove cmd from generateManifest Signed-off-by: zaihaoyin --- cmd/docker-generate/manifest.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/docker-generate/manifest.go b/cmd/docker-generate/manifest.go index 32be915b3..89cc49d75 100644 --- a/cmd/docker-generate/manifest.go +++ b/cmd/docker-generate/manifest.go @@ -26,7 +26,7 @@ func generateManifestCommandWithOpts() (*cobra.Command, *generateManifestOpts) { } }, RunE: func(cmd *cobra.Command, args []string) error { - return generateManifest(cmd, opts) + return generateManifest(opts) }, } cmd.Flags().StringVarP(&opts.output, "output", "o", "", "write to a file instead of stdout") @@ -38,7 +38,7 @@ func generateManifestCommand() *cobra.Command { return command } -func generateManifest(cmd *cobra.Command, opts generateManifestOpts) error { +func generateManifest(opts generateManifestOpts) error { var reader io.Reader if opts.reference != "" { cmd := exec.Command("docker", "save", opts.reference) From fe5becc3f49d2872c8124c163623ce3b400c54e9 Mon Sep 17 00:00:00 2001 From: zaihaoyin Date: Mon, 18 Jul 2022 16:08:56 +0800 Subject: [PATCH 7/7] refactor test Signed-off-by: zaihaoyin --- cmd/docker-generate/generate.go | 2 +- cmd/docker-generate/manifest.go | 17 +++++++---------- cmd/docker-generate/manifest_test.go | 10 ++++------ 3 files changed, 12 insertions(+), 17 deletions(-) diff --git a/cmd/docker-generate/generate.go b/cmd/docker-generate/generate.go index d0f3d9165..b72150275 100644 --- a/cmd/docker-generate/generate.go +++ b/cmd/docker-generate/generate.go @@ -8,6 +8,6 @@ func generateCommand() *cobra.Command { cmd := &cobra.Command{ Use: "generate", } - cmd.AddCommand(generateManifestCommand()) + cmd.AddCommand(generateManifestCommand(nil)) return cmd } diff --git a/cmd/docker-generate/manifest.go b/cmd/docker-generate/manifest.go index 89cc49d75..661e7dbe9 100644 --- a/cmd/docker-generate/manifest.go +++ b/cmd/docker-generate/manifest.go @@ -14,8 +14,10 @@ type generateManifestOpts struct { reference string } -func generateManifestCommandWithOpts() (*cobra.Command, *generateManifestOpts) { - var opts generateManifestOpts +func generateManifestCommand(opts *generateManifestOpts) *cobra.Command { + if opts == nil { + opts = &generateManifestOpts{} + } cmd := &cobra.Command{ Use: "manifest [reference]", Short: "generates the manifest of a docker image", @@ -26,19 +28,14 @@ func generateManifestCommandWithOpts() (*cobra.Command, *generateManifestOpts) { } }, RunE: func(cmd *cobra.Command, args []string) error { - return generateManifest(opts) + return generateManifest(cmd, opts) }, } cmd.Flags().StringVarP(&opts.output, "output", "o", "", "write to a file instead of stdout") - return cmd, &opts -} - -func generateManifestCommand() *cobra.Command { - command, _ := generateManifestCommandWithOpts() - return command + return cmd } -func generateManifest(opts generateManifestOpts) error { +func generateManifest(cmd *cobra.Command, opts *generateManifestOpts) error { var reader io.Reader if opts.reference != "" { cmd := exec.Command("docker", "save", opts.reference) diff --git a/cmd/docker-generate/manifest_test.go b/cmd/docker-generate/manifest_test.go index eace01973..30e9924b0 100644 --- a/cmd/docker-generate/manifest_test.go +++ b/cmd/docker-generate/manifest_test.go @@ -56,7 +56,8 @@ func TestGenerateManifestCmd(t *testing.T) { }, } for _, test := range tests { - cmd, opts := generateManifestCommandWithOpts() + opts := &generateManifestOpts{} + cmd := generateManifestCommand(opts) err := cmd.ParseFlags(test.args) if err != nil && !test.expectedErr { t.Fatalf("Test failed with error: %v", err) @@ -68,11 +69,8 @@ func TestGenerateManifestCmd(t *testing.T) { continue } cmd.PreRun(cmd, cmd.Flags().Args()) - if opts.output != test.output { - t.Fatalf("Expect output: %v, got: %v", test.output, opts.output) - } - if opts.reference != test.reference { - t.Fatalf("Expect reference: %v, got: %v", test.reference, opts.reference) + if *opts != test.generateManifestOpts { + t.Fatalf("Expect generate manifest opts: %v, got: %v", test.generateManifestOpts, *opts) } }