Add commandAction to make *cli.Context unavailable in command handlers

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č <mitr@redhat.com>

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 {
+func (opts *copyOptions) run(args []string, stdout io.Writer) error {
-	if len(c.Args()) != 2 {
+	if len(args) != 2 {
-		cli.ShowCommandHelp(c, "copy")
+		return errorShouldDisplayUsage{errors.New("Exactly two arguments expected")}
-		return 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, &copy.Options{
 		RemoveSignatures:      opts.removeSignatures,
 		SignBy:                opts.signByFingerprint,
-		ReportWriter:          os.Stdout,
+		ReportWriter:          stdout,
 		SourceCtx:             sourceCtx,
 		DestinationCtx:        destinationCtx,
 		ForceManifestMIMEType: manifestType,

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 {
+func (opts *deleteOptions) run(args []string, stdout io.Writer) error {
-	if len(c.Args()) != 1 {
+	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()

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
 }

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
 		}

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 {
+func (opts *manifestDigestOptions) run(args []string, stdout io.Writer) error {
-	if len(context.Args()) != 1 {
+	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
 }

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 {
+func (opts *standaloneSignOptions) run(args []string, stdout io.Writer) error {
-	if len(c.Args()) != 3 || opts.output == "" {
+	if len(args) != 3 || opts.output == "" {
 		return errors.New("Usage: skopeo standalone-sign manifest docker-reference key-fingerprint -o signature")
 	}
-	manifestPath := c.Args()[0]
+	manifestPath := args[0]
-	dockerReference := c.Args()[1]
+	dockerReference := args[1]
-	fingerprint := c.Args()[2]
+	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 {
+func (opts *standaloneVerifyOptions) run(args []string, stdout io.Writer) error {
-	if len(c.Args()) != 4 {
+	if len(args) != 4 {
 		return errors.New("Usage: skopeo standalone-verify manifest docker-reference key-fingerprint signature")
 	}
-	manifestPath := c.Args()[0]
+	manifestPath := args[0]
-	expectedDockerReference := c.Args()[1]
+	expectedDockerReference := args[1]
-	expectedFingerprint := c.Args()[2]
+	expectedFingerprint := args[2]
-	signaturePath := c.Args()[3]
+	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 {
+func (opts *untrustedSignatureDumpOptions) run(args []string, stdout io.Writer) error {
-	if len(c.Args()) != 1 {
+	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
 }

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 {