Introduce imageDestOptions

This is an extension of imageOptions that carries destination-specific
flags.

This will allow us to handle --dest-* flags without also exposing
pointless --src-* flags.

(This is, also, where the type-safety somewhat breaks down;
after all the work to make the data flow and availability explicit,
everything ends up in an types.SystemContext, and it's easy enough
to use a destination-specific one for sources.  OTOH, this is
not making the situation worse in any sense.)

Signed-off-by: Miloslav Trmač <mitr@redhat.com>

cmd/skopeo/copy.go

@@ -23,7 +23,7 @@ func contextsFromCopyOptions(c *cli.Context, opts *copyOptions) (*types.SystemCo
 		return nil, nil, err
 	}
 
-	destinationCtx, err := contextFromImageOptions(c, opts.destImage)
+	destinationCtx, err := contextFromImageDestOptions(c, opts.destImage)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -34,7 +34,7 @@ func contextsFromCopyOptions(c *cli.Context, opts *copyOptions) (*types.SystemCo
 type copyOptions struct {
 	global            *globalOptions
 	srcImage          *imageOptions
-	destImage         *imageOptions
+	destImage         *imageDestOptions
 	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
@@ -43,7 +43,7 @@ type copyOptions struct {
 
 func copyCmd(global *globalOptions) cli.Command {
 	srcFlags, srcOpts := imageFlags(global, "src-", "screds")
-	destFlags, destOpts := imageFlags(global, "dest-", "dcreds")
+	destFlags, destOpts := imageDestFlags(global, "dest-", "dcreds")
 	opts := copyOptions{global: global,
 		srcImage:  srcOpts,
 		destImage: destOpts,

cmd/skopeo/utils.go

@@ -98,6 +98,28 @@ func contextFromImageOptions(c *cli.Context, opts *imageOptions) (*types.SystemC
 	return ctx, nil
 }
 
+// imageDestOptions is a superset of imageOptions specialized for iamge destinations.
+type imageDestOptions struct {
+	*imageOptions
+}
+
+// imageDestFlags prepares a collection of CLI flags writing into imageDestOptions, and the managed imageDestOptions structure.
+func imageDestFlags(global *globalOptions, flagPrefix, credsOptionAlias string) ([]cli.Flag, *imageDestOptions) {
+	genericFlags, genericOptions := imageFlags(global, flagPrefix, credsOptionAlias)
+	opts := imageDestOptions{imageOptions: genericOptions}
+
+	return append(genericFlags, []cli.Flag{}...), &opts
+}
+
+func contextFromImageDestOptions(c *cli.Context, opts *imageDestOptions) (*types.SystemContext, error) {
+	ctx, err := contextFromImageOptions(c, opts.imageOptions)
+	if err != nil {
+		return nil, err
+	}
+
+	return ctx, err
+}
+
 func parseCreds(creds string) (string, string, error) {
 	if creds == "" {
 		return "", "", errors.New("credentials can't be empty")

cmd/skopeo/utils_test.go

@@ -11,18 +11,26 @@ import (
 	"github.com/urfave/cli"
 )
 
-// fakeContext creates inputs for contextFromImageOptions.
+// fakeGlobalOptions creates globalOptions and sets it according to flags.
 // NOTE: This is QUITE FAKE; none of the urfave/cli normalization and the like happens.
-func fakeContext(t *testing.T, cmdName string, flagPrefix string, globalFlags []string, cmdFlags []string) (*cli.Context, *imageOptions) {
-	app, globalOpts := createApp()
+func fakeGlobalOptions(t *testing.T, flags []string) (*cli.App, *cli.Context, *globalOptions) {
+	app, opts := createApp()
 
-	globalSet := flag.NewFlagSet(app.Name, flag.ContinueOnError)
+	flagSet := flag.NewFlagSet(app.Name, flag.ContinueOnError)
 	for _, f := range app.Flags {
-		f.Apply(globalSet)
+		f.Apply(flagSet)
 	}
-	err := globalSet.Parse(globalFlags)
+	err := flagSet.Parse(flags)
 	require.NoError(t, err)
-	globalCtx := cli.NewContext(app, globalSet, nil)
+	ctx := cli.NewContext(app, flagSet, nil)
+
+	return app, ctx, opts
+}
+
+// fakeImageContext creates inputs for contextFromImageOptions.
+// NOTE: This is QUITE FAKE; none of the urfave/cli normalization and the like happens.
+func fakeImageContext(t *testing.T, cmdName string, flagPrefix string, globalFlags []string, cmdFlags []string) (*cli.Context, *imageOptions) {
+	app, globalCtx, globalOpts := fakeGlobalOptions(t, globalFlags)
 
 	cmd := app.Command(cmdName)
 	require.NotNil(t, cmd)
@@ -37,34 +45,34 @@ func fakeContext(t *testing.T, cmdName string, flagPrefix string, globalFlags []
 	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)
+	flagSet := flag.NewFlagSet(cmd.Name, flag.ContinueOnError)
 	for _, f := range imageFlags {
-		f.Apply(cmdSet)
+		f.Apply(flagSet)
 		appliedFlags[firstName(f)] = struct{}{}
 	}
 	for _, f := range cmd.Flags {
 		if _, ok := appliedFlags[firstName(f)]; !ok {
-			f.Apply(cmdSet)
+			f.Apply(flagSet)
 			appliedFlags[firstName(f)] = struct{}{}
 		}
 	}
 
-	err = cmdSet.Parse(cmdFlags)
+	err := flagSet.Parse(cmdFlags)
 	require.NoError(t, err)
-	return cli.NewContext(app, cmdSet, globalCtx), imageOpts
+	return cli.NewContext(app, flagSet, globalCtx), imageOpts
 }
 
 func TestContextFromImageOptions(t *testing.T) {
 	// FIXME: All of this only tests (skopeo copy --dest)
 
 	// Default state
-	c, opts := fakeContext(t, "copy", "dest-", []string{}, []string{})
+	c, opts := fakeImageContext(t, "copy", "dest-", []string{}, []string{})
 	res, err := contextFromImageOptions(c, opts)
 	require.NoError(t, err)
 	assert.Equal(t, &types.SystemContext{}, res)
 
 	// Explicitly set everything to default, except for when the default is “not present”
-	c, opts = fakeContext(t, "copy", "dest-", []string{}, []string{
+	c, opts = fakeImageContext(t, "copy", "dest-", []string{}, []string{
 		"--dest-compress=false",
 	})
 	res, err = contextFromImageOptions(c, opts)
@@ -72,7 +80,7 @@ func TestContextFromImageOptions(t *testing.T) {
 	assert.Equal(t, &types.SystemContext{}, res)
 
 	// Set everything to non-default values.
-	c, opts = fakeContext(t, "copy", "dest-", []string{
+	c, opts = fakeImageContext(t, "copy", "dest-", []string{
 		"--registries.d", "/srv/registries.d",
 		"--override-arch", "overridden-arch",
 		"--override-os", "overridden-os",
@@ -128,7 +136,7 @@ func TestContextFromImageOptions(t *testing.T) {
 		if c.cmd != "" {
 			cmdFlags = append(cmdFlags, "--dest-tls-verify="+c.cmd)
 		}
-		ctx, opts := fakeContext(t, "copy", "dest-", globalFlags, cmdFlags)
+		ctx, opts := fakeImageContext(t, "copy", "dest-", globalFlags, cmdFlags)
 		res, err = contextFromImageOptions(ctx, opts)
 		require.NoError(t, err)
 		assert.Equal(t, c.expectedDocker, res.DockerInsecureSkipTLSVerify, "%#v", c)
@@ -136,7 +144,98 @@ func TestContextFromImageOptions(t *testing.T) {
 	}
 
 	// Invalid option values
-	c, opts = fakeContext(t, "copy", "dest-", []string{}, []string{"--dest-creds", ""})
+	c, opts = fakeImageContext(t, "copy", "dest-", []string{}, []string{"--dest-creds", ""})
 	_, err = contextFromImageOptions(c, opts)
 	assert.Error(t, err)
 }
+
+// fakeImageDestContext creates inputs for contextFromImageDestOptions.
+// NOTE: This is QUITE FAKE; none of the urfave/cli normalization and the like happens.
+func fakeImageDestContext(t *testing.T, cmdName string, flagPrefix string, globalFlags []string, cmdFlags []string) (*cli.Context, *imageDestOptions) {
+	app, globalCtx, globalOpts := fakeGlobalOptions(t, globalFlags)
+
+	cmd := app.Command(cmdName)
+	require.NotNil(t, cmd)
+
+	imageFlags, imageOpts := imageDestFlags(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]
+	}
+	flagSet := flag.NewFlagSet(cmd.Name, flag.ContinueOnError)
+	for _, f := range imageFlags {
+		f.Apply(flagSet)
+		appliedFlags[firstName(f)] = struct{}{}
+	}
+	for _, f := range cmd.Flags {
+		if _, ok := appliedFlags[firstName(f)]; !ok {
+			f.Apply(flagSet)
+			appliedFlags[firstName(f)] = struct{}{}
+		}
+	}
+
+	err := flagSet.Parse(cmdFlags)
+	require.NoError(t, err)
+	return cli.NewContext(app, flagSet, globalCtx), imageOpts
+}
+
+func TestContextFromImageDestOptions(t *testing.T) {
+	// FIXME: All of this only tests (skopeo copy --dest)
+
+	// Default state
+	c, opts := fakeImageDestContext(t, "copy", "dest-", []string{}, []string{})
+	res, err := contextFromImageDestOptions(c, opts)
+	require.NoError(t, err)
+	assert.Equal(t, &types.SystemContext{}, res)
+
+	// Explicitly set everything to default, except for when the default is “not present”
+	c, opts = fakeImageDestContext(t, "copy", "dest-", []string{}, []string{
+		"--dest-compress=false",
+	})
+	res, err = contextFromImageDestOptions(c, opts)
+	require.NoError(t, err)
+	assert.Equal(t, &types.SystemContext{}, res)
+
+	// Set everything to non-default values.
+	c, opts = fakeImageDestContext(t, "copy", "dest-", []string{
+		"--registries.d", "/srv/registries.d",
+		"--override-arch", "overridden-arch",
+		"--override-os", "overridden-os",
+	}, []string{
+		"--authfile", "/srv/authfile",
+		"--dest-cert-dir", "/srv/cert-dir",
+		"--dest-ostree-tmp-dir", "/srv/ostree-tmp-dir",
+		"--dest-shared-blob-dir", "/srv/shared-blob-dir",
+		"--dest-compress=true",
+		"--dest-daemon-host", "daemon-host.example.com",
+		"--dest-tls-verify=false",
+		"--dest-creds", "creds-user:creds-password",
+	})
+	res, err = contextFromImageDestOptions(c, opts)
+	require.NoError(t, err)
+	assert.Equal(t, &types.SystemContext{
+		RegistriesDirPath:                 "/srv/registries.d",
+		AuthFilePath:                      "/srv/authfile",
+		ArchitectureChoice:                "overridden-arch",
+		OSChoice:                          "overridden-os",
+		OCISharedBlobDirPath:              "/srv/shared-blob-dir",
+		DockerCertPath:                    "/srv/cert-dir",
+		DockerInsecureSkipTLSVerify:       types.OptionalBoolTrue,
+		DockerAuthConfig:                  &types.DockerAuthConfig{Username: "creds-user", Password: "creds-password"},
+		OSTreeTmpDirPath:                  "/srv/ostree-tmp-dir",
+		DockerDaemonCertPath:              "/srv/cert-dir",
+		DockerDaemonHost:                  "daemon-host.example.com",
+		DockerDaemonInsecureSkipTLSVerify: true,
+		DirForceCompress:                  true,
+	}, res)
+
+	// Invalid option values in imageOptions
+	c, opts = fakeImageDestContext(t, "copy", "dest-", []string{}, []string{"--dest-creds", ""})
+	_, err = contextFromImageDestOptions(c, opts)
+	assert.Error(t, err)
+}