diff --git a/cmd/skopeo/copy.go b/cmd/skopeo/copy.go index 90a533e5..81f6f35d 100644 --- a/cmd/skopeo/copy.go +++ b/cmd/skopeo/copy.go @@ -17,13 +17,13 @@ import ( ) // contextsFromGlobalOptions returns source and destionation types.SystemContext depending on c. -func contextsFromGlobalOptions(c *cli.Context) (*types.SystemContext, *types.SystemContext, error) { - sourceCtx, err := contextFromGlobalOptions(c, "src-") +func contextsFromGlobalOptions(c *cli.Context, global *globalOptions) (*types.SystemContext, *types.SystemContext, error) { + sourceCtx, err := contextFromGlobalOptions(c, global, "src-") if err != nil { return nil, nil, err } - destinationCtx, err := contextFromGlobalOptions(c, "dest-") + destinationCtx, err := contextFromGlobalOptions(c, global, "dest-") if err != nil { return nil, nil, err } @@ -163,7 +163,7 @@ func (opts *copyOptions) run(c *cli.Context) error { return fmt.Errorf("Invalid destination name %s: %v", c.Args()[1], err) } - sourceCtx, destinationCtx, err := contextsFromGlobalOptions(c) + sourceCtx, destinationCtx, err := contextsFromGlobalOptions(c, opts.global) if err != nil { return err } diff --git a/cmd/skopeo/delete.go b/cmd/skopeo/delete.go index 9eee301d..e6e173f9 100644 --- a/cmd/skopeo/delete.go +++ b/cmd/skopeo/delete.go @@ -62,7 +62,7 @@ func (opts *deleteOptions) run(c *cli.Context) error { return fmt.Errorf("Invalid source name %s: %v", c.Args()[0], err) } - sys, err := contextFromGlobalOptions(c, "") + sys, err := contextFromGlobalOptions(c, opts.global, "") if err != nil { return err } diff --git a/cmd/skopeo/inspect.go b/cmd/skopeo/inspect.go index 9c8ecdd8..15a38ccf 100644 --- a/cmd/skopeo/inspect.go +++ b/cmd/skopeo/inspect.go @@ -81,7 +81,7 @@ func (opts *inspectOptions) run(c *cli.Context) (retErr error) { ctx, cancel := opts.global.commandTimeoutContext() defer cancel() - img, err := parseImage(ctx, c) + img, err := parseImage(ctx, c, opts.global) if err != nil { return err } @@ -127,7 +127,7 @@ func (opts *inspectOptions) run(c *cli.Context) (retErr error) { outputData.Name = dockerRef.Name() } if img.Reference().Transport() == docker.Transport { - sys, err := contextFromGlobalOptions(c, "") + sys, err := contextFromGlobalOptions(c, opts.global, "") if err != nil { return err } diff --git a/cmd/skopeo/layers.go b/cmd/skopeo/layers.go index 1cb77f53..f503af6e 100644 --- a/cmd/skopeo/layers.go +++ b/cmd/skopeo/layers.go @@ -39,12 +39,12 @@ func (opts *layersOptions) run(c *cli.Context) (retErr error) { ctx, cancel := opts.global.commandTimeoutContext() defer cancel() - sys, err := contextFromGlobalOptions(c, "") + sys, err := contextFromGlobalOptions(c, opts.global, "") if err != nil { return err } cache := blobinfocache.DefaultCache(sys) - rawSource, err := parseImageSource(ctx, c, c.Args()[0]) + rawSource, err := parseImageSource(ctx, c, opts.global, c.Args()[0]) if err != nil { return err } diff --git a/cmd/skopeo/main.go b/cmd/skopeo/main.go index dc1bea48..fb56568c 100644 --- a/cmd/skopeo/main.go +++ b/cmd/skopeo/main.go @@ -18,14 +18,18 @@ import ( var gitCommit = "" type globalOptions struct { - debug bool // Enable debug output - policyPath string // Path to a signature verification policy file - insecurePolicy bool // Use an "allow everything" signature verification policy - commandTimeout time.Duration // Timeout for the command execution + debug bool // Enable debug output + tlsVerify optionalBool // Require HTTPS and verify certificates (for docker: and docker-daemon:) + policyPath string // Path to a signature verification policy file + insecurePolicy bool // Use an "allow everything" signature verification policy + registriesDirPath string // Path to a "registries.d" registry configuratio directory + overrideArch string // Architecture to use for choosing images, instead of the runtime one + overrideOS string // OS to use for choosing images, instead of the runtime one + commandTimeout time.Duration // Timeout for the command execution } -// createApp returns a cli.App to be run or tested. -func createApp() *cli.App { +// createApp returns a cli.App, and the underlying globalOptions object, to be run or tested. +func createApp() (*cli.App, *globalOptions) { opts := globalOptions{} app := cli.NewApp() @@ -43,10 +47,11 @@ func createApp() *cli.App { Usage: "enable debug output", Destination: &opts.debug, }, - cli.BoolTFlag{ + cli.GenericFlag{ Name: "tls-verify", Usage: "require HTTPS and verify certificates when talking to container registries (defaults to true)", Hidden: true, + Value: newOptionalBoolValue(&opts.tlsVerify), }, cli.StringFlag{ Name: "policy", @@ -59,19 +64,19 @@ func createApp() *cli.App { Destination: &opts.insecurePolicy, }, cli.StringFlag{ - Name: "registries.d", - Value: "", - Usage: "use registry configuration files in `DIR` (e.g. for container signature storage)", + Name: "registries.d", + Usage: "use registry configuration files in `DIR` (e.g. for container signature storage)", + Destination: &opts.registriesDirPath, }, cli.StringFlag{ - Name: "override-arch", - Value: "", - Usage: "use `ARCH` instead of the architecture of the machine for choosing images", + Name: "override-arch", + Usage: "use `ARCH` instead of the architecture of the machine for choosing images", + Destination: &opts.overrideArch, }, cli.StringFlag{ - Name: "override-os", - Value: "", - Usage: "use `OS` instead of the running OS for choosing images", + Name: "override-os", + Usage: "use `OS` instead of the running OS for choosing images", + Destination: &opts.overrideOS, }, cli.DurationFlag{ Name: "command-timeout", @@ -90,15 +95,15 @@ func createApp() *cli.App { standaloneVerifyCmd(), untrustedSignatureDumpCmd(), } - return app + return app, &opts } // before is run by the cli package for any command, before running the command-specific handler. -func (opts *globalOptions) before(c *cli.Context) error { +func (opts *globalOptions) before(_ *cli.Context) error { if opts.debug { logrus.SetLevel(logrus.DebugLevel) } - if c.GlobalIsSet("tls-verify") { + if opts.tlsVerify.present { logrus.Warn("'--tls-verify' is deprecated, please set this on the specific subcommand") } return nil @@ -108,7 +113,7 @@ func main() { if reexec.Init() { return } - app := createApp() + app, _ := createApp() if err := app.Run(os.Args); err != nil { logrus.Fatal(err) } diff --git a/cmd/skopeo/main_test.go b/cmd/skopeo/main_test.go index 42df5069..8b1c421f 100644 --- a/cmd/skopeo/main_test.go +++ b/cmd/skopeo/main_test.go @@ -5,7 +5,7 @@ import "bytes" // runSkopeo creates an app object and runs it with args, with an implied first "skopeo". // Returns output intended for stdout and the returned error, if any. func runSkopeo(args ...string) (string, error) { - app := createApp() + app, _ := createApp() stdout := bytes.Buffer{} app.Writer = &stdout args = append([]string{"skopeo"}, args...) diff --git a/cmd/skopeo/utils.go b/cmd/skopeo/utils.go index f94e01dc..d6fa5375 100644 --- a/cmd/skopeo/utils.go +++ b/cmd/skopeo/utils.go @@ -10,11 +10,11 @@ import ( "github.com/urfave/cli" ) -func contextFromGlobalOptions(c *cli.Context, flagPrefix string) (*types.SystemContext, error) { +func contextFromGlobalOptions(c *cli.Context, global *globalOptions, flagPrefix string) (*types.SystemContext, error) { ctx := &types.SystemContext{ - RegistriesDirPath: c.GlobalString("registries.d"), - ArchitectureChoice: c.GlobalString("override-arch"), - OSChoice: c.GlobalString("override-os"), + RegistriesDirPath: global.registriesDirPath, + ArchitectureChoice: global.overrideArch, + OSChoice: global.overrideOS, DockerCertPath: c.String(flagPrefix + "cert-dir"), OSTreeTmpDirPath: c.String(flagPrefix + "ostree-tmp-dir"), OCISharedBlobDirPath: c.String(flagPrefix + "shared-blob-dir"), @@ -24,10 +24,9 @@ func contextFromGlobalOptions(c *cli.Context, flagPrefix string) (*types.SystemC DockerDaemonCertPath: c.String(flagPrefix + "cert-dir"), DockerDaemonInsecureSkipTLSVerify: !c.BoolT(flagPrefix + "tls-verify"), } - // DEPRECATED: we support --tls-verify for backward compatibility, but override - // it if per-subcommand flags are provided (see below). - if c.GlobalIsSet("tls-verify") { - ctx.DockerInsecureSkipTLSVerify = types.NewOptionalBool(!c.GlobalBoolT("tls-verify")) + // DEPRECATED: We support this for backward compatibility, but override it if a per-image flag is provided. + if global.tlsVerify.present { + ctx.DockerInsecureSkipTLSVerify = types.NewOptionalBool(!global.tlsVerify.value) } if c.IsSet(flagPrefix + "tls-verify") { ctx.DockerInsecureSkipTLSVerify = types.NewOptionalBool(!c.BoolT(flagPrefix + "tls-verify")) @@ -69,13 +68,13 @@ func getDockerAuth(creds string) (*types.DockerAuthConfig, error) { // parseImage converts image URL-like string to an initialized handler for that image. // The caller must call .Close() on the returned ImageCloser. -func parseImage(ctx context.Context, c *cli.Context) (types.ImageCloser, error) { +func parseImage(ctx context.Context, c *cli.Context, global *globalOptions) (types.ImageCloser, error) { imgName := c.Args().First() ref, err := alltransports.ParseImageName(imgName) if err != nil { return nil, err } - sys, err := contextFromGlobalOptions(c, "") + sys, err := contextFromGlobalOptions(c, global, "") if err != nil { return nil, err } @@ -84,12 +83,12 @@ func parseImage(ctx context.Context, c *cli.Context) (types.ImageCloser, error) // parseImageSource converts image URL-like string to an ImageSource. // The caller must call .Close() on the returned ImageSource. -func parseImageSource(ctx context.Context, c *cli.Context, name string) (types.ImageSource, error) { +func parseImageSource(ctx context.Context, c *cli.Context, global *globalOptions, name string) (types.ImageSource, error) { ref, err := alltransports.ParseImageName(name) if err != nil { return nil, err } - sys, err := contextFromGlobalOptions(c, "") + sys, err := contextFromGlobalOptions(c, global, "") if err != nil { return nil, err } diff --git a/cmd/skopeo/utils_test.go b/cmd/skopeo/utils_test.go index b174daf6..04b49913 100644 --- a/cmd/skopeo/utils_test.go +++ b/cmd/skopeo/utils_test.go @@ -12,8 +12,8 @@ import ( // fakeContext creates inputs for contextFromGlobalOptions. // NOTE: This is QUITE FAKE; none of the urfave/cli normalization and the like happens. -func fakeContext(t *testing.T, cmdName string, globalFlags []string, cmdFlags []string) *cli.Context { - app := createApp() +func fakeContext(t *testing.T, cmdName string, globalFlags []string, cmdFlags []string) (*cli.Context, *globalOptions) { + app, global := createApp() globalSet := flag.NewFlagSet(app.Name, flag.ContinueOnError) for _, f := range app.Flags { @@ -31,7 +31,7 @@ func fakeContext(t *testing.T, cmdName string, globalFlags []string, cmdFlags [] } err = cmdSet.Parse(cmdFlags) require.NoError(t, err) - return cli.NewContext(app, cmdSet, globalCtx) + return cli.NewContext(app, cmdSet, globalCtx), global } func TestContextFromGlobalOptions(t *testing.T) { @@ -39,21 +39,21 @@ func TestContextFromGlobalOptions(t *testing.T) { // FIXME FIXME: Apparently BoolT values are set to false if the flag is not declared for the specific subcommand!! // Default state - c := fakeContext(t, "copy", []string{}, []string{}) - res, err := contextFromGlobalOptions(c, "dest-") + c, global := fakeContext(t, "copy", []string{}, []string{}) + res, err := contextFromGlobalOptions(c, global, "dest-") require.NoError(t, err) assert.Equal(t, &types.SystemContext{}, res) // Explicitly set everything to default, except for when the default is “not present” - c = fakeContext(t, "copy", []string{}, []string{ + c, global = fakeContext(t, "copy", []string{}, []string{ "--dest-compress=false", }) - res, err = contextFromGlobalOptions(c, "dest-") + res, err = contextFromGlobalOptions(c, global, "dest-") require.NoError(t, err) assert.Equal(t, &types.SystemContext{}, res) // Set everything to non-default values. - c = fakeContext(t, "copy", []string{ + c, global = fakeContext(t, "copy", []string{ "--registries.d", "/srv/registries.d", "--override-arch", "overridden-arch", "--override-os", "overridden-os", @@ -67,7 +67,7 @@ func TestContextFromGlobalOptions(t *testing.T) { "--dest-tls-verify=false", "--dest-creds", "creds-user:creds-password", }) - res, err = contextFromGlobalOptions(c, "dest-") + res, err = contextFromGlobalOptions(c, global, "dest-") require.NoError(t, err) assert.Equal(t, &types.SystemContext{ RegistriesDirPath: "/srv/registries.d", @@ -109,15 +109,15 @@ func TestContextFromGlobalOptions(t *testing.T) { if c.cmd != "" { cmdFlags = append(cmdFlags, "--dest-tls-verify="+c.cmd) } - ctx := fakeContext(t, "copy", globalFlags, cmdFlags) - res, err = contextFromGlobalOptions(ctx, "dest-") + ctx, global := fakeContext(t, "copy", globalFlags, cmdFlags) + res, err = contextFromGlobalOptions(ctx, global, "dest-") require.NoError(t, err) assert.Equal(t, c.expectedDocker, res.DockerInsecureSkipTLSVerify, "%#v", c) assert.Equal(t, c.expectedDockerDaemon, res.DockerDaemonInsecureSkipTLSVerify, "%#v", c) } // Invalid option values - c = fakeContext(t, "copy", []string{}, []string{"--dest-creds", ""}) - _, err = contextFromGlobalOptions(c, "dest-") + c, global = fakeContext(t, "copy", []string{}, []string{"--dest-creds", ""}) + _, err = contextFromGlobalOptions(c, global, "dest-") assert.Error(t, err) }