diff --git a/cmd/skopeo/copy.go b/cmd/skopeo/copy.go index 81f6f35d..fdff7c1b 100644 --- a/cmd/skopeo/copy.go +++ b/cmd/skopeo/copy.go @@ -16,14 +16,14 @@ import ( "github.com/urfave/cli" ) -// contextsFromGlobalOptions returns source and destionation types.SystemContext depending on c. -func contextsFromGlobalOptions(c *cli.Context, global *globalOptions) (*types.SystemContext, *types.SystemContext, error) { - sourceCtx, err := contextFromGlobalOptions(c, global, "src-") +// contextsFromCopyOptions returns source and destionation types.SystemContext depending on c. +func contextsFromCopyOptions(c *cli.Context, opts *copyOptions) (*types.SystemContext, *types.SystemContext, error) { + sourceCtx, err := contextFromImageOptions(c, opts.srcImage, "src-") if err != nil { return nil, nil, err } - destinationCtx, err := contextFromGlobalOptions(c, global, "dest-") + destinationCtx, err := contextFromImageOptions(c, opts.destImage, "dest-") if err != nil { return nil, nil, err } @@ -33,6 +33,8 @@ func contextsFromGlobalOptions(c *cli.Context, global *globalOptions) (*types.Sy type copyOptions struct { global *globalOptions + srcImage *imageOptions + destImage *imageOptions additionalTags cli.StringSlice // For docker-archive: destinations, in addition to the name:tag specified as destination, also add these removeSignatures bool // Do not copy signatures from the source image signByFingerprint string // Sign the image using a GPG key with the specified fingerprint @@ -40,7 +42,13 @@ type copyOptions struct { } func copyCmd(global *globalOptions) cli.Command { - opts := copyOptions{global: global} + srcFlags, srcOpts := imageFlags(global, "src-") + destFlags, destOpts := imageFlags(global, "dest-") + opts := copyOptions{global: global, + srcImage: srcOpts, + destImage: destOpts, + } + return cli.Command{ Name: "copy", Usage: "Copy an IMAGE-NAME from one location to another", @@ -56,7 +64,7 @@ func copyCmd(global *globalOptions) cli.Command { ArgsUsage: "SOURCE-IMAGE DESTINATION-IMAGE", Action: opts.run, // FIXME: Do we need to namespace the GPG aspect? - Flags: []cli.Flag{ + Flags: append(append([]cli.Flag{ cli.StringSliceFlag{ Name: "additional-tag", Usage: "additional tags (supports docker-archive)", @@ -138,7 +146,7 @@ func copyCmd(global *globalOptions) cli.Command { Value: "", Usage: "use docker daemon host at `HOST` (docker-daemon destinations only)", }, - }, + }, srcFlags...), destFlags...), } } @@ -163,7 +171,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, opts.global) + sourceCtx, destinationCtx, err := contextsFromCopyOptions(c, opts) if err != nil { return err } diff --git a/cmd/skopeo/delete.go b/cmd/skopeo/delete.go index e6e173f9..5a9de127 100644 --- a/cmd/skopeo/delete.go +++ b/cmd/skopeo/delete.go @@ -12,10 +12,15 @@ import ( type deleteOptions struct { global *globalOptions + image *imageOptions } func deleteCmd(global *globalOptions) cli.Command { - opts := deleteOptions{global: global} + imageFlags, imageOpts := imageFlags(global, "") + opts := deleteOptions{ + global: global, + image: imageOpts, + } return cli.Command{ Name: "delete", Usage: "Delete image IMAGE-NAME", @@ -29,7 +34,7 @@ func deleteCmd(global *globalOptions) cli.Command { `, strings.Join(transports.ListNames(), ", ")), ArgsUsage: "IMAGE-NAME", Action: opts.run, - Flags: []cli.Flag{ + Flags: append([]cli.Flag{ cli.StringFlag{ Name: "authfile", Usage: "path of the authentication file. Default is ${XDG_RUNTIME_DIR}/containers/auth.json", @@ -48,7 +53,7 @@ func deleteCmd(global *globalOptions) cli.Command { Name: "tls-verify", Usage: "require HTTPS and verify certificates when talking to container registries (defaults to true)", }, - }, + }, imageFlags...), } } @@ -62,7 +67,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, opts.global, "") + sys, err := contextFromImageOptions(c, opts.image, "") if err != nil { return err } diff --git a/cmd/skopeo/inspect.go b/cmd/skopeo/inspect.go index 15a38ccf..d35b4a75 100644 --- a/cmd/skopeo/inspect.go +++ b/cmd/skopeo/inspect.go @@ -31,11 +31,16 @@ type inspectOutput struct { type inspectOptions struct { global *globalOptions + image *imageOptions raw bool // Output the raw manifest instead of parsing information about the image } func inspectCmd(global *globalOptions) cli.Command { - opts := inspectOptions{global: global} + imageFlags, imageOpts := imageFlags(global, "") + opts := inspectOptions{ + global: global, + image: imageOpts, + } return cli.Command{ Name: "inspect", Usage: "Inspect image IMAGE-NAME", @@ -48,7 +53,7 @@ func inspectCmd(global *globalOptions) cli.Command { See skopeo(1) section "IMAGE NAMES" for the expected format `, strings.Join(transports.ListNames(), ", ")), ArgsUsage: "IMAGE-NAME", - Flags: []cli.Flag{ + Flags: append([]cli.Flag{ cli.StringFlag{ Name: "authfile", Usage: "path of the authentication file. Default is ${XDG_RUNTIME_DIR}/containers/auth.json", @@ -72,7 +77,7 @@ func inspectCmd(global *globalOptions) cli.Command { Value: "", Usage: "Use `USERNAME[:PASSWORD]` for accessing the registry", }, - }, + }, imageFlags...), Action: opts.run, } } @@ -81,7 +86,7 @@ func (opts *inspectOptions) run(c *cli.Context) (retErr error) { ctx, cancel := opts.global.commandTimeoutContext() defer cancel() - img, err := parseImage(ctx, c, opts.global) + img, err := parseImage(ctx, c, opts.image) if err != nil { return err } @@ -127,7 +132,7 @@ func (opts *inspectOptions) run(c *cli.Context) (retErr error) { outputData.Name = dockerRef.Name() } if img.Reference().Transport() == docker.Transport { - sys, err := contextFromGlobalOptions(c, opts.global, "") + sys, err := contextFromImageOptions(c, opts.image, "") if err != nil { return err } diff --git a/cmd/skopeo/layers.go b/cmd/skopeo/layers.go index f503af6e..da04b625 100644 --- a/cmd/skopeo/layers.go +++ b/cmd/skopeo/layers.go @@ -17,16 +17,22 @@ import ( type layersOptions struct { global *globalOptions + image *imageOptions } func layersCmd(global *globalOptions) cli.Command { - opts := &layersOptions{global: global} + imageFlags, imageOpts := imageFlags(global, "") + opts := layersOptions{ + global: global, + image: imageOpts, + } return cli.Command{ Name: "layers", Usage: "Get layers of IMAGE-NAME", ArgsUsage: "IMAGE-NAME [LAYER...]", Hidden: true, Action: opts.run, + Flags: imageFlags, } } @@ -39,12 +45,12 @@ func (opts *layersOptions) run(c *cli.Context) (retErr error) { ctx, cancel := opts.global.commandTimeoutContext() defer cancel() - sys, err := contextFromGlobalOptions(c, opts.global, "") + sys, err := contextFromImageOptions(c, opts.image, "") if err != nil { return err } cache := blobinfocache.DefaultCache(sys) - rawSource, err := parseImageSource(ctx, c, opts.global, c.Args()[0]) + rawSource, err := parseImageSource(ctx, c, opts.image, c.Args()[0]) if err != nil { return err } diff --git a/cmd/skopeo/utils.go b/cmd/skopeo/utils.go index d6fa5375..79b0a182 100644 --- a/cmd/skopeo/utils.go +++ b/cmd/skopeo/utils.go @@ -10,11 +10,23 @@ import ( "github.com/urfave/cli" ) -func contextFromGlobalOptions(c *cli.Context, global *globalOptions, flagPrefix string) (*types.SystemContext, error) { +// imageOptions collects CLI flags which are the same across subcommands, but may be different for each image +// (e.g. may differ between the source and destination of a copy) +type imageOptions struct { + global *globalOptions // May be shared across several imageOptions instances. +} + +// imageFlags prepares a collection of CLI flags writing into imageOptions, and the managed imageOptions structure. +func imageFlags(global *globalOptions, flagPrefix string) ([]cli.Flag, *imageOptions) { + opts := imageOptions{global: global} + return []cli.Flag{}, &opts +} + +func contextFromImageOptions(c *cli.Context, opts *imageOptions, flagPrefix string) (*types.SystemContext, error) { ctx := &types.SystemContext{ - RegistriesDirPath: global.registriesDirPath, - ArchitectureChoice: global.overrideArch, - OSChoice: global.overrideOS, + RegistriesDirPath: opts.global.registriesDirPath, + ArchitectureChoice: opts.global.overrideArch, + OSChoice: opts.global.overrideOS, DockerCertPath: c.String(flagPrefix + "cert-dir"), OSTreeTmpDirPath: c.String(flagPrefix + "ostree-tmp-dir"), OCISharedBlobDirPath: c.String(flagPrefix + "shared-blob-dir"), @@ -25,8 +37,8 @@ func contextFromGlobalOptions(c *cli.Context, global *globalOptions, flagPrefix DockerDaemonInsecureSkipTLSVerify: !c.BoolT(flagPrefix + "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 opts.global.tlsVerify.present { + ctx.DockerInsecureSkipTLSVerify = types.NewOptionalBool(!opts.global.tlsVerify.value) } if c.IsSet(flagPrefix + "tls-verify") { ctx.DockerInsecureSkipTLSVerify = types.NewOptionalBool(!c.BoolT(flagPrefix + "tls-verify")) @@ -68,13 +80,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, global *globalOptions) (types.ImageCloser, error) { +func parseImage(ctx context.Context, c *cli.Context, opts *imageOptions) (types.ImageCloser, error) { imgName := c.Args().First() ref, err := alltransports.ParseImageName(imgName) if err != nil { return nil, err } - sys, err := contextFromGlobalOptions(c, global, "") + sys, err := contextFromImageOptions(c, opts, "") if err != nil { return nil, err } @@ -83,12 +95,12 @@ func parseImage(ctx context.Context, c *cli.Context, global *globalOptions) (typ // 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, global *globalOptions, name string) (types.ImageSource, error) { +func parseImageSource(ctx context.Context, c *cli.Context, opts *imageOptions, name string) (types.ImageSource, error) { ref, err := alltransports.ParseImageName(name) if err != nil { return nil, err } - sys, err := contextFromGlobalOptions(c, global, "") + sys, err := contextFromImageOptions(c, opts, "") if err != nil { return nil, err } diff --git a/cmd/skopeo/utils_test.go b/cmd/skopeo/utils_test.go index 04b49913..44e61d93 100644 --- a/cmd/skopeo/utils_test.go +++ b/cmd/skopeo/utils_test.go @@ -2,6 +2,7 @@ package main import ( "flag" + "strings" "testing" "github.com/containers/image/types" @@ -10,10 +11,10 @@ import ( "github.com/urfave/cli" ) -// fakeContext creates inputs for contextFromGlobalOptions. +// fakeContext creates inputs for contextFromImageOptions. // 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, *globalOptions) { - app, global := createApp() +func fakeContext(t *testing.T, cmdName string, flagPrefix string, globalFlags []string, cmdFlags []string) (*cli.Context, *imageOptions) { + app, globalOpts := createApp() globalSet := flag.NewFlagSet(app.Name, flag.ContinueOnError) for _, f := range app.Flags { @@ -25,35 +26,54 @@ func fakeContext(t *testing.T, cmdName string, globalFlags []string, cmdFlags [] cmd := app.Command(cmdName) require.NotNil(t, cmd) - cmdSet := flag.NewFlagSet(cmd.Name, flag.ContinueOnError) - for _, f := range cmd.Flags { - f.Apply(cmdSet) + + imageFlags, imageOpts := imageFlags(globalOpts, flagPrefix) + appliedFlags := map[string]struct{}{} + // Ugly: cmd.Flags includes imageFlags as well. For now, we need cmd.Flags to apply here + // to be able to test the non-Destination: flags, but we must not apply the same flag name twice. + // So, primarily use imageFlags (so that Destination: is used as expected), and then follow up with + // the remaining flags from cmd.Flags (so that cli.Context.String() etc. works). + // This is horribly ugly, but all of this will disappear within this PR. + firstName := func(f cli.Flag) string { // We even need to recognize "dest-creds,dcreds". This will disappear as well. + return strings.Split(f.GetName(), ",")[0] } + cmdSet := flag.NewFlagSet(cmd.Name, flag.ContinueOnError) + for _, f := range imageFlags { + f.Apply(cmdSet) + appliedFlags[firstName(f)] = struct{}{} + } + for _, f := range cmd.Flags { + if _, ok := appliedFlags[firstName(f)]; !ok { + f.Apply(cmdSet) + appliedFlags[firstName(f)] = struct{}{} + } + } + err = cmdSet.Parse(cmdFlags) require.NoError(t, err) - return cli.NewContext(app, cmdSet, globalCtx), global + return cli.NewContext(app, cmdSet, globalCtx), imageOpts } -func TestContextFromGlobalOptions(t *testing.T) { +func TestContextFromImageOptions(t *testing.T) { // FIXME: All of this only tests (skopeo copy --dest) // FIXME FIXME: Apparently BoolT values are set to false if the flag is not declared for the specific subcommand!! // Default state - c, global := fakeContext(t, "copy", []string{}, []string{}) - res, err := contextFromGlobalOptions(c, global, "dest-") + c, opts := fakeContext(t, "copy", "dest-", []string{}, []string{}) + res, err := contextFromImageOptions(c, opts, "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, global = fakeContext(t, "copy", []string{}, []string{ + c, opts = fakeContext(t, "copy", "dest-", []string{}, []string{ "--dest-compress=false", }) - res, err = contextFromGlobalOptions(c, global, "dest-") + res, err = contextFromImageOptions(c, opts, "dest-") require.NoError(t, err) assert.Equal(t, &types.SystemContext{}, res) // Set everything to non-default values. - c, global = fakeContext(t, "copy", []string{ + c, opts = fakeContext(t, "copy", "dest-", []string{ "--registries.d", "/srv/registries.d", "--override-arch", "overridden-arch", "--override-os", "overridden-os", @@ -67,7 +87,7 @@ func TestContextFromGlobalOptions(t *testing.T) { "--dest-tls-verify=false", "--dest-creds", "creds-user:creds-password", }) - res, err = contextFromGlobalOptions(c, global, "dest-") + res, err = contextFromImageOptions(c, opts, "dest-") require.NoError(t, err) assert.Equal(t, &types.SystemContext{ RegistriesDirPath: "/srv/registries.d", @@ -109,15 +129,15 @@ func TestContextFromGlobalOptions(t *testing.T) { if c.cmd != "" { cmdFlags = append(cmdFlags, "--dest-tls-verify="+c.cmd) } - ctx, global := fakeContext(t, "copy", globalFlags, cmdFlags) - res, err = contextFromGlobalOptions(ctx, global, "dest-") + ctx, opts := fakeContext(t, "copy", "dest-", globalFlags, cmdFlags) + res, err = contextFromImageOptions(ctx, opts, "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, global = fakeContext(t, "copy", []string{}, []string{"--dest-creds", ""}) - _, err = contextFromGlobalOptions(c, global, "dest-") + c, opts = fakeContext(t, "copy", "dest-", []string{}, []string{"--dest-creds", ""}) + _, err = contextFromImageOptions(c, opts, "dest-") assert.Error(t, err) }