From 2497f500d5fce80a48037f11b3c86b0bad0dab10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 18 Jul 2018 00:41:39 +0200 Subject: [PATCH] Add commandAction to make *cli.Context unavailable in command handlers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit That in turn makes sure that the cli.String() etc. flag access functions are not used, and all flag handling is done using the *Options structures and the Destination: members of cli.Flag. Signed-off-by: Miloslav Trmač --- cmd/skopeo/copy.go | 21 ++++++++++----------- cmd/skopeo/delete.go | 11 ++++++----- cmd/skopeo/inspect.go | 14 +++++++++----- cmd/skopeo/layers.go | 11 ++++++----- cmd/skopeo/manifest.go | 11 ++++++----- cmd/skopeo/signing.go | 39 ++++++++++++++++++++------------------- cmd/skopeo/utils.go | 21 +++++++++++++++++++++ 7 files changed, 78 insertions(+), 50 deletions(-) diff --git a/cmd/skopeo/copy.go b/cmd/skopeo/copy.go index 442af8d6..dd285600 100644 --- a/cmd/skopeo/copy.go +++ b/cmd/skopeo/copy.go @@ -3,7 +3,7 @@ package main import ( "errors" "fmt" - "os" + "io" "strings" "github.com/containers/image/copy" @@ -47,7 +47,7 @@ func copyCmd(global *globalOptions) cli.Command { See skopeo(1) section "IMAGE NAMES" for the expected format `, strings.Join(transports.ListNames(), ", ")), ArgsUsage: "SOURCE-IMAGE DESTINATION-IMAGE", - Action: opts.run, + Action: commandAction(opts.run), // FIXME: Do we need to namespace the GPG aspect? Flags: append(append(append([]cli.Flag{ cli.StringSliceFlag{ @@ -74,10 +74,9 @@ func copyCmd(global *globalOptions) cli.Command { } } -func (opts *copyOptions) run(c *cli.Context) error { - if len(c.Args()) != 2 { - cli.ShowCommandHelp(c, "copy") - return errors.New("Exactly two arguments expected") +func (opts *copyOptions) run(args []string, stdout io.Writer) error { + if len(args) != 2 { + return errorShouldDisplayUsage{errors.New("Exactly two arguments expected")} } policyContext, err := opts.global.getPolicyContext() @@ -86,13 +85,13 @@ func (opts *copyOptions) run(c *cli.Context) error { } defer policyContext.Destroy() - srcRef, err := alltransports.ParseImageName(c.Args()[0]) + srcRef, err := alltransports.ParseImageName(args[0]) if err != nil { - return fmt.Errorf("Invalid source name %s: %v", c.Args()[0], err) + return fmt.Errorf("Invalid source name %s: %v", args[0], err) } - destRef, err := alltransports.ParseImageName(c.Args()[1]) + destRef, err := alltransports.ParseImageName(args[1]) if err != nil { - return fmt.Errorf("Invalid destination name %s: %v", c.Args()[1], err) + return fmt.Errorf("Invalid destination name %s: %v", args[1], err) } sourceCtx, err := opts.srcImage.newSystemContext() @@ -136,7 +135,7 @@ func (opts *copyOptions) run(c *cli.Context) error { _, err = copy.Image(ctx, policyContext, destRef, srcRef, ©.Options{ RemoveSignatures: opts.removeSignatures, SignBy: opts.signByFingerprint, - ReportWriter: os.Stdout, + ReportWriter: stdout, SourceCtx: sourceCtx, DestinationCtx: destinationCtx, ForceManifestMIMEType: manifestType, diff --git a/cmd/skopeo/delete.go b/cmd/skopeo/delete.go index 72b85fcf..07a7cea2 100644 --- a/cmd/skopeo/delete.go +++ b/cmd/skopeo/delete.go @@ -3,6 +3,7 @@ package main import ( "errors" "fmt" + "io" "strings" "github.com/containers/image/transports" @@ -34,19 +35,19 @@ func deleteCmd(global *globalOptions) cli.Command { See skopeo(1) section "IMAGE NAMES" for the expected format `, strings.Join(transports.ListNames(), ", ")), ArgsUsage: "IMAGE-NAME", - Action: opts.run, + Action: commandAction(opts.run), Flags: append(sharedFlags, imageFlags...), } } -func (opts *deleteOptions) run(c *cli.Context) error { - if len(c.Args()) != 1 { +func (opts *deleteOptions) run(args []string, stdout io.Writer) error { + if len(args) != 1 { return errors.New("Usage: delete imageReference") } - ref, err := alltransports.ParseImageName(c.Args()[0]) + ref, err := alltransports.ParseImageName(args[0]) if err != nil { - return fmt.Errorf("Invalid source name %s: %v", c.Args()[0], err) + return fmt.Errorf("Invalid source name %s: %v", args[0], err) } sys, err := opts.image.newSystemContext() diff --git a/cmd/skopeo/inspect.go b/cmd/skopeo/inspect.go index 9cd7b3dd..a4418df5 100644 --- a/cmd/skopeo/inspect.go +++ b/cmd/skopeo/inspect.go @@ -3,6 +3,7 @@ package main import ( "encoding/json" "fmt" + "io" "strings" "time" @@ -61,15 +62,18 @@ func inspectCmd(global *globalOptions) cli.Command { Destination: &opts.raw, }, }, sharedFlags...), imageFlags...), - Action: opts.run, + Action: commandAction(opts.run), } } -func (opts *inspectOptions) run(c *cli.Context) (retErr error) { +func (opts *inspectOptions) run(args []string, stdout io.Writer) (retErr error) { ctx, cancel := opts.global.commandTimeoutContext() defer cancel() - img, err := parseImage(ctx, opts.image, c.Args().First()) + if len(args) != 1 { + return errors.New("Exactly one argument expected") + } + img, err := parseImage(ctx, opts.image, args[0]) if err != nil { return err } @@ -85,7 +89,7 @@ func (opts *inspectOptions) run(c *cli.Context) (retErr error) { return err } if opts.raw { - _, err := c.App.Writer.Write(rawManifest) + _, err := stdout.Write(rawManifest) if err != nil { return fmt.Errorf("Error writing manifest to standard output: %v", err) } @@ -134,6 +138,6 @@ func (opts *inspectOptions) run(c *cli.Context) (retErr error) { if err != nil { return err } - fmt.Fprintln(c.App.Writer, string(out)) + fmt.Fprintln(stdout, string(out)) return nil } diff --git a/cmd/skopeo/layers.go b/cmd/skopeo/layers.go index aa8b296a..5c236dc3 100644 --- a/cmd/skopeo/layers.go +++ b/cmd/skopeo/layers.go @@ -2,6 +2,7 @@ package main import ( "fmt" + "io" "io/ioutil" "os" "strings" @@ -32,14 +33,14 @@ func layersCmd(global *globalOptions) cli.Command { Usage: "Get layers of IMAGE-NAME", ArgsUsage: "IMAGE-NAME [LAYER...]", Hidden: true, - Action: opts.run, + Action: commandAction(opts.run), Flags: append(sharedFlags, imageFlags...), } } -func (opts *layersOptions) run(c *cli.Context) (retErr error) { +func (opts *layersOptions) run(args []string, stdout io.Writer) (retErr error) { fmt.Fprintln(os.Stderr, `DEPRECATED: skopeo layers is deprecated in favor of skopeo copy`) - if c.NArg() == 0 { + if len(args) == 0 { return errors.New("Usage: layers imageReference [layer...]") } @@ -51,7 +52,7 @@ func (opts *layersOptions) run(c *cli.Context) (retErr error) { return err } cache := blobinfocache.DefaultCache(sys) - rawSource, err := parseImageSource(ctx, opts.image, c.Args()[0]) + rawSource, err := parseImageSource(ctx, opts.image, args[0]) if err != nil { return err } @@ -74,7 +75,7 @@ func (opts *layersOptions) run(c *cli.Context) (retErr error) { isConfig bool } var blobDigests []blobDigest - for _, dString := range c.Args().Tail() { + for _, dString := range args[1:] { if !strings.HasPrefix(dString, "sha256:") { dString = "sha256:" + dString } diff --git a/cmd/skopeo/manifest.go b/cmd/skopeo/manifest.go index 177fefe5..5ada000a 100644 --- a/cmd/skopeo/manifest.go +++ b/cmd/skopeo/manifest.go @@ -3,6 +3,7 @@ package main import ( "errors" "fmt" + "io" "io/ioutil" "github.com/containers/image/manifest" @@ -18,15 +19,15 @@ func manifestDigestCmd() cli.Command { Name: "manifest-digest", Usage: "Compute a manifest digest of a file", ArgsUsage: "MANIFEST", - Action: opts.run, + Action: commandAction(opts.run), } } -func (opts *manifestDigestOptions) run(context *cli.Context) error { - if len(context.Args()) != 1 { +func (opts *manifestDigestOptions) run(args []string, stdout io.Writer) error { + if len(args) != 1 { return errors.New("Usage: skopeo manifest-digest manifest") } - manifestPath := context.Args()[0] + manifestPath := args[0] man, err := ioutil.ReadFile(manifestPath) if err != nil { @@ -36,6 +37,6 @@ func (opts *manifestDigestOptions) run(context *cli.Context) error { if err != nil { return fmt.Errorf("Error computing digest: %v", err) } - fmt.Fprintf(context.App.Writer, "%s\n", digest) + fmt.Fprintf(stdout, "%s\n", digest) return nil } diff --git a/cmd/skopeo/signing.go b/cmd/skopeo/signing.go index 9d7a29a5..6affd643 100644 --- a/cmd/skopeo/signing.go +++ b/cmd/skopeo/signing.go @@ -4,6 +4,7 @@ import ( "encoding/json" "errors" "fmt" + "io" "io/ioutil" "github.com/containers/image/signature" @@ -20,7 +21,7 @@ func standaloneSignCmd() cli.Command { Name: "standalone-sign", Usage: "Create a signature using local files", ArgsUsage: "MANIFEST DOCKER-REFERENCE KEY-FINGERPRINT", - Action: opts.run, + Action: commandAction(opts.run), Flags: []cli.Flag{ cli.StringFlag{ Name: "output, o", @@ -31,13 +32,13 @@ func standaloneSignCmd() cli.Command { } } -func (opts *standaloneSignOptions) run(c *cli.Context) error { - if len(c.Args()) != 3 || opts.output == "" { +func (opts *standaloneSignOptions) run(args []string, stdout io.Writer) error { + if len(args) != 3 || opts.output == "" { return errors.New("Usage: skopeo standalone-sign manifest docker-reference key-fingerprint -o signature") } - manifestPath := c.Args()[0] - dockerReference := c.Args()[1] - fingerprint := c.Args()[2] + manifestPath := args[0] + dockerReference := args[1] + fingerprint := args[2] manifest, err := ioutil.ReadFile(manifestPath) if err != nil { @@ -69,18 +70,18 @@ func standaloneVerifyCmd() cli.Command { Name: "standalone-verify", Usage: "Verify a signature using local files", ArgsUsage: "MANIFEST DOCKER-REFERENCE KEY-FINGERPRINT SIGNATURE", - Action: opts.run, + Action: commandAction(opts.run), } } -func (opts *standaloneVerifyOptions) run(c *cli.Context) error { - if len(c.Args()) != 4 { +func (opts *standaloneVerifyOptions) run(args []string, stdout io.Writer) error { + if len(args) != 4 { return errors.New("Usage: skopeo standalone-verify manifest docker-reference key-fingerprint signature") } - manifestPath := c.Args()[0] - expectedDockerReference := c.Args()[1] - expectedFingerprint := c.Args()[2] - signaturePath := c.Args()[3] + manifestPath := args[0] + expectedDockerReference := args[1] + expectedFingerprint := args[2] + signaturePath := args[3] unverifiedManifest, err := ioutil.ReadFile(manifestPath) if err != nil { @@ -101,7 +102,7 @@ func (opts *standaloneVerifyOptions) run(c *cli.Context) error { return fmt.Errorf("Error verifying signature: %v", err) } - fmt.Fprintf(c.App.Writer, "Signature verified, digest %s\n", sig.DockerManifestDigest) + fmt.Fprintf(stdout, "Signature verified, digest %s\n", sig.DockerManifestDigest) return nil } @@ -121,15 +122,15 @@ func untrustedSignatureDumpCmd() cli.Command { Usage: "Dump contents of a signature WITHOUT VERIFYING IT", ArgsUsage: "SIGNATURE", Hidden: true, - Action: opts.run, + Action: commandAction(opts.run), } } -func (opts *untrustedSignatureDumpOptions) run(c *cli.Context) error { - if len(c.Args()) != 1 { +func (opts *untrustedSignatureDumpOptions) run(args []string, stdout io.Writer) error { + if len(args) != 1 { return errors.New("Usage: skopeo untrusted-signature-dump-without-verification signature") } - untrustedSignaturePath := c.Args()[0] + untrustedSignaturePath := args[0] untrustedSignature, err := ioutil.ReadFile(untrustedSignaturePath) if err != nil { @@ -144,6 +145,6 @@ func (opts *untrustedSignatureDumpOptions) run(c *cli.Context) error { if err != nil { return err } - fmt.Fprintln(c.App.Writer, string(untrustedOut)) + fmt.Fprintln(stdout, string(untrustedOut)) return nil } diff --git a/cmd/skopeo/utils.go b/cmd/skopeo/utils.go index 66e0e6f0..d2636479 100644 --- a/cmd/skopeo/utils.go +++ b/cmd/skopeo/utils.go @@ -3,6 +3,7 @@ package main import ( "context" "errors" + "io" "strings" "github.com/containers/image/transports/alltransports" @@ -10,6 +11,26 @@ import ( "github.com/urfave/cli" ) +// errorShouldDisplayUsage is a subtype of error used by command handlers to indicate that cli.ShowSubcommandHelp should be called. +type errorShouldDisplayUsage struct { + error +} + +// commandAction intermediates between the cli.ActionFunc interface and the real handler, +// primarily to ensure that cli.Context is not available to the handler, which in turn +// makes sure that the cli.String() etc. flag access functions are not used, +// and everything is done using the *Options structures and the Destination: members of cli.Flag. +// handler may return errorShouldDisplayUsage to cause cli.ShowSubcommandHelp to be called. +func commandAction(handler func(args []string, stdout io.Writer) error) cli.ActionFunc { + return func(c *cli.Context) error { + err := handler(([]string)(c.Args()), c.App.Writer) + if _, ok := err.(errorShouldDisplayUsage); ok { + cli.ShowSubcommandHelp(c) + } + return err + } +} + // sharedImageOptions collects CLI flags which are image-related, but do not change across images. // This really should be a part of globalOptions, but that would break existing users of (skopeo copy --authfile=). type sharedImageOptions struct {