Merge pull request #1369 from mtrmac/tls-verify

Fix --tls-verify

cmd/skopeo/copy.go

@@ -19,31 +19,34 @@ import (
 )
 
 type copyOptions struct {
-	global            *globalOptions
-	srcImage          *imageOptions
-	destImage         *imageDestOptions
-	retryOpts         *retry.RetryOptions
-	additionalTags    []string       // 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
-	digestFile        string         // Write digest to this file
-	format            optionalString // Force conversion of the image to a specified format
-	quiet             bool           // Suppress output information when copying images
-	all               bool           // Copy all of the images if the source is a list
-	encryptLayer      []int          // The list of layers to encrypt
-	encryptionKeys    []string       // Keys needed to encrypt the image
-	decryptionKeys    []string       // Keys needed to decrypt the image
+	global              *globalOptions
+	deprecatedTLSVerify *deprecatedTLSVerifyOption
+	srcImage            *imageOptions
+	destImage           *imageDestOptions
+	retryOpts           *retry.RetryOptions
+	additionalTags      []string       // 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
+	digestFile          string         // Write digest to this file
+	format              optionalString // Force conversion of the image to a specified format
+	quiet               bool           // Suppress output information when copying images
+	all                 bool           // Copy all of the images if the source is a list
+	encryptLayer        []int          // The list of layers to encrypt
+	encryptionKeys      []string       // Keys needed to encrypt the image
+	decryptionKeys      []string       // Keys needed to decrypt the image
 }
 
 func copyCmd(global *globalOptions) *cobra.Command {
 	sharedFlags, sharedOpts := sharedImageFlags()
-	srcFlags, srcOpts := imageFlags(global, sharedOpts, "src-", "screds")
-	destFlags, destOpts := imageDestFlags(global, sharedOpts, "dest-", "dcreds")
+	deprecatedTLSVerifyFlags, deprecatedTLSVerifyOpt := deprecatedTLSVerifyFlags()
+	srcFlags, srcOpts := imageFlags(global, sharedOpts, deprecatedTLSVerifyOpt, "src-", "screds")
+	destFlags, destOpts := imageDestFlags(global, sharedOpts, deprecatedTLSVerifyOpt, "dest-", "dcreds")
 	retryFlags, retryOpts := retryFlags()
 	opts := copyOptions{global: global,
-		srcImage:  srcOpts,
-		destImage: destOpts,
-		retryOpts: retryOpts,
+		deprecatedTLSVerify: deprecatedTLSVerifyOpt,
+		srcImage:            srcOpts,
+		destImage:           destOpts,
+		retryOpts:           retryOpts,
 	}
 	cmd := &cobra.Command{
 		Use:   "copy [command options] SOURCE-IMAGE DESTINATION-IMAGE",
@@ -61,6 +64,7 @@ See skopeo(1) section "IMAGE NAMES" for the expected format
 	adjustUsage(cmd)
 	flags := cmd.Flags()
 	flags.AddFlagSet(&sharedFlags)
+	flags.AddFlagSet(&deprecatedTLSVerifyFlags)
 	flags.AddFlagSet(&srcFlags)
 	flags.AddFlagSet(&destFlags)
 	flags.AddFlagSet(&retryFlags)
@@ -81,6 +85,7 @@ func (opts *copyOptions) run(args []string, stdout io.Writer) error {
 	if len(args) != 2 {
 		return errorShouldDisplayUsage{errors.New("Exactly two arguments expected")}
 	}
+	opts.deprecatedTLSVerify.warnIfUsed([]string{"--src-tls-verify", "--dest-tls-verify"})
 	imageNames := args
 
 	if err := reexecIfNecessaryForImages(imageNames...); err != nil {

cmd/skopeo/delete.go

@@ -20,7 +20,7 @@ type deleteOptions struct {
 
 func deleteCmd(global *globalOptions) *cobra.Command {
 	sharedFlags, sharedOpts := sharedImageFlags()
-	imageFlags, imageOpts := imageFlags(global, sharedOpts, "", "")
+	imageFlags, imageOpts := imageFlags(global, sharedOpts, nil, "", "")
 	retryFlags, retryOpts := retryFlags()
 	opts := deleteOptions{
 		global:    global,

cmd/skopeo/inspect.go

@@ -34,7 +34,7 @@ type inspectOptions struct {
 
 func inspectCmd(global *globalOptions) *cobra.Command {
 	sharedFlags, sharedOpts := sharedImageFlags()
-	imageFlags, imageOpts := imageFlags(global, sharedOpts, "", "")
+	imageFlags, imageOpts := imageFlags(global, sharedOpts, nil, "", "")
 	retryFlags, retryOpts := retryFlags()
 	opts := inspectOptions{
 		global:    global,

cmd/skopeo/layers.go

@@ -25,7 +25,7 @@ type layersOptions struct {
 
 func layersCmd(global *globalOptions) *cobra.Command {
 	sharedFlags, sharedOpts := sharedImageFlags()
-	imageFlags, imageOpts := imageFlags(global, sharedOpts, "", "")
+	imageFlags, imageOpts := imageFlags(global, sharedOpts, nil, "", "")
 	retryFlags, retryOpts := retryFlags()
 	opts := layersOptions{
 		global:    global,

cmd/skopeo/list_tags.go

@@ -30,7 +30,7 @@ type tagsOptions struct {
 
 func tagsCmd(global *globalOptions) *cobra.Command {
 	sharedFlags, sharedOpts := sharedImageFlags()
-	imageFlags, imageOpts := dockerImageFlags(global, sharedOpts, "", "")
+	imageFlags, imageOpts := dockerImageFlags(global, sharedOpts, nil, "", "")
 	retryFlags, retryOpts := retryFlags()
 
 	opts := tagsOptions{

cmd/skopeo/logout.go

@@ -4,12 +4,14 @@ import (
 	"io"
 
 	"github.com/containers/common/pkg/auth"
+	"github.com/containers/image/v5/types"
 	"github.com/spf13/cobra"
 )
 
 type logoutOptions struct {
 	global     *globalOptions
 	logoutOpts auth.LogoutOptions
+	tlsVerify  optionalBool
 }
 
 func logoutCmd(global *globalOptions) *cobra.Command {
@@ -24,12 +26,17 @@ func logoutCmd(global *globalOptions) *cobra.Command {
 		Example: `skopeo logout quay.io`,
 	}
 	adjustUsage(cmd)
-	cmd.Flags().AddFlagSet(auth.GetLogoutFlags(&opts.logoutOpts))
+	flags := cmd.Flags()
+	optionalBoolFlag(flags, &opts.tlsVerify, "tls-verify", "require HTTPS and verify certificates when accessing the registry")
+	flags.AddFlagSet(auth.GetLogoutFlags(&opts.logoutOpts))
 	return cmd
 }
 
 func (opts *logoutOptions) run(args []string, stdout io.Writer) error {
 	opts.logoutOpts.Stdout = stdout
 	sys := opts.global.newSystemContext()
+	if opts.tlsVerify.present {
+		sys.DockerInsecureSkipTLSVerify = types.NewOptionalBool(!opts.tlsVerify.value)
+	}
 	return auth.Logout(sys, &opts.logoutOpts, args)
 }

cmd/skopeo/main.go

@@ -50,6 +50,12 @@ func createApp() (*cobra.Command, *globalOptions) {
 		// already making us of that.  If Skopeo decides to follow, please
 		// remove the line below (and hide the `completion` command).
 		CompletionOptions: cobra.CompletionOptions{DisableDefaultCmd: true},
+		// This is documented to parse "local" (non-PersistentFlags) flags of parent commands before
+		// running subcommands and handling their options. We don't really run into such cases,
+		// because all of our flags on rootCommand are in PersistentFlags, except for the deprecated --tls-verify;
+		// in that case we need TraverseChildren so that we can distinguish between
+		// (skopeo --tls-verify inspect) (causes a warning) and (skopeo inspect --tls-verify) (no warning).
+		TraverseChildren: true,
 	}
 	if gitCommit != "" {
 		rootCommand.Version = fmt.Sprintf("%s commit: %s", version.Version, gitCommit)
@@ -60,8 +66,6 @@ func createApp() (*cobra.Command, *globalOptions) {
 	var dummyVersion bool
 	rootCommand.Flags().BoolVarP(&dummyVersion, "version", "v", false, "Version for Skopeo")
 	rootCommand.PersistentFlags().BoolVar(&opts.debug, "debug", false, "enable debug output")
-	flag := optionalBoolFlag(rootCommand.PersistentFlags(), &opts.tlsVerify, "tls-verify", "Require HTTPS and verify certificates when accessing the registry")
-	flag.Hidden = true
 	rootCommand.PersistentFlags().StringVar(&opts.policyPath, "policy", "", "Path to a trust policy file")
 	rootCommand.PersistentFlags().BoolVar(&opts.insecurePolicy, "insecure-policy", false, "run the tool without any policy check")
 	rootCommand.PersistentFlags().StringVar(&opts.registriesDirPath, "registries.d", "", "use registry configuration files in `DIR` (e.g. for container signature storage)")
@@ -74,6 +78,8 @@ func createApp() (*cobra.Command, *globalOptions) {
 		logrus.Fatal("unable to mark registries-conf flag as hidden")
 	}
 	rootCommand.PersistentFlags().StringVar(&opts.tmpDir, "tmpdir", "", "directory used to store temporary files")
+	flag := optionalBoolFlag(rootCommand.Flags(), &opts.tlsVerify, "tls-verify", "Require HTTPS and verify certificates when accessing the registry")
+	flag.Hidden = true
 	rootCommand.AddCommand(
 		copyCmd(&opts),
 		deleteCmd(&opts),

cmd/skopeo/sync.go

@@ -27,17 +27,18 @@ import (
 
 // syncOptions contains information retrieved from the skopeo sync command line.
 type syncOptions struct {
-	global            *globalOptions    // Global (not command dependent) skopeo options
-	srcImage          *imageOptions     // Source image options
-	destImage         *imageDestOptions // Destination image options
-	retryOpts         *retry.RetryOptions
-	removeSignatures  bool           // Do not copy signatures from the source image
-	signByFingerprint string         // Sign the image using a GPG key with the specified fingerprint
-	format            optionalString // Force conversion of the image to a specified format
-	source            string         // Source repository name
-	destination       string         // Destination registry name
-	scoped            bool           // When true, namespace copied images at destination using the source repository name
-	all               bool           // Copy all of the images if an image in the source is a list
+	global              *globalOptions // Global (not command dependent) skopeo options
+	deprecatedTLSVerify *deprecatedTLSVerifyOption
+	srcImage            *imageOptions     // Source image options
+	destImage           *imageDestOptions // Destination image options
+	retryOpts           *retry.RetryOptions
+	removeSignatures    bool           // Do not copy signatures from the source image
+	signByFingerprint   string         // Sign the image using a GPG key with the specified fingerprint
+	format              optionalString // Force conversion of the image to a specified format
+	source              string         // Source repository name
+	destination         string         // Destination registry name
+	scoped              bool           // When true, namespace copied images at destination using the source repository name
+	all                 bool           // Copy all of the images if an image in the source is a list
 }
 
 // repoDescriptor contains information of a single repository used as a sync source.
@@ -68,15 +69,17 @@ type sourceConfig map[string]registrySyncConfig
 
 func syncCmd(global *globalOptions) *cobra.Command {
 	sharedFlags, sharedOpts := sharedImageFlags()
-	srcFlags, srcOpts := dockerImageFlags(global, sharedOpts, "src-", "screds")
-	destFlags, destOpts := dockerImageFlags(global, sharedOpts, "dest-", "dcreds")
+	deprecatedTLSVerifyFlags, deprecatedTLSVerifyOpt := deprecatedTLSVerifyFlags()
+	srcFlags, srcOpts := dockerImageFlags(global, sharedOpts, deprecatedTLSVerifyOpt, "src-", "screds")
+	destFlags, destOpts := dockerImageFlags(global, sharedOpts, deprecatedTLSVerifyOpt, "dest-", "dcreds")
 	retryFlags, retryOpts := retryFlags()
 
 	opts := syncOptions{
-		global:    global,
-		srcImage:  srcOpts,
-		destImage: &imageDestOptions{imageOptions: destOpts},
-		retryOpts: retryOpts,
+		global:              global,
+		deprecatedTLSVerify: deprecatedTLSVerifyOpt,
+		srcImage:            srcOpts,
+		destImage:           &imageDestOptions{imageOptions: destOpts},
+		retryOpts:           retryOpts,
 	}
 
 	cmd := &cobra.Command{
@@ -102,6 +105,7 @@ See skopeo-sync(1) for details.
 	flags.BoolVar(&opts.scoped, "scoped", false, "Images at DESTINATION are prefix using the full source image path as scope")
 	flags.BoolVarP(&opts.all, "all", "a", false, "Copy all images if SOURCE-IMAGE is a list")
 	flags.AddFlagSet(&sharedFlags)
+	flags.AddFlagSet(&deprecatedTLSVerifyFlags)
 	flags.AddFlagSet(&srcFlags)
 	flags.AddFlagSet(&destFlags)
 	flags.AddFlagSet(&retryFlags)
@@ -492,6 +496,7 @@ func (opts *syncOptions) run(args []string, stdout io.Writer) error {
 	if len(args) != 2 {
 		return errorShouldDisplayUsage{errors.New("Exactly two arguments expected")}
 	}
+	opts.deprecatedTLSVerify.warnIfUsed([]string{"--src-tls-verify", "--dest-tls-verify"})
 
 	policyContext, err := opts.global.getPolicyContext()
 	if err != nil {

cmd/skopeo/utils.go

@@ -14,6 +14,7 @@ import (
 	"github.com/containers/image/v5/types"
 	imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 )
@@ -38,6 +39,35 @@ func commandAction(handler func(args []string, stdout io.Writer) error) func(cmd
 	}
 }
 
+// deprecatedTLSVerifyOption represents a deprecated --tls-verify option,
+// which was accepted for all subcommands, for a time.
+// Every user should call deprecatedTLSVerifyOption.warnIfUsed() as part of handling the CLI,
+// whether or not the value actually ends up being used.
+// DO NOT ADD ANY NEW USES OF THIS; just call dockerImageFlags with an appropriate, possibly empty, flagPrefix.
+type deprecatedTLSVerifyOption struct {
+	tlsVerify optionalBool // FIXME FIXME: Warn if this is used, or even if it is ignored.
+}
+
+// warnIfUsed warns if tlsVerify was set by the user, and suggests alternatives (which should
+// start with "--").
+// Every user should call this as part of handling the CLI, whether or not the value actually
+// ends up being used.
+func (opts *deprecatedTLSVerifyOption) warnIfUsed(alternatives []string) {
+	if opts.tlsVerify.present {
+		logrus.Warnf("'--tls-verify' is deprecated, instead use: %s", strings.Join(alternatives, ", "))
+	}
+}
+
+// deprecatedTLSVerifyFlags prepares the CLI flag writing into deprecatedTLSVerifyOption, and the managed deprecatedTLSVerifyOption structure.
+// DO NOT ADD ANY NEW USES OF THIS; just call dockerImageFlags with an appropriate, possibly empty, flagPrefix.
+func deprecatedTLSVerifyFlags() (pflag.FlagSet, *deprecatedTLSVerifyOption) {
+	opts := deprecatedTLSVerifyOption{}
+	fs := pflag.FlagSet{}
+	flag := optionalBoolFlag(&fs, &opts.tlsVerify, "tls-verify", "require HTTPS and verify certificates when accessing the container registry (defaults to true)")
+	flag.Hidden = true
+	return fs, &opts
+}
+
 // 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 {
@@ -56,14 +86,15 @@ func sharedImageFlags() (pflag.FlagSet, *sharedImageOptions) {
 // the same across subcommands, but may be different for each image
 // (e.g. may differ between the source and destination of a copy)
 type dockerImageOptions struct {
-	global         *globalOptions      // May be shared across several imageOptions instances.
-	shared         *sharedImageOptions // May be shared across several imageOptions instances.
-	authFilePath   optionalString      // Path to a */containers/auth.json (prefixed version to override shared image option).
-	credsOption    optionalString      // username[:password] for accessing a registry
-	registryToken  optionalString      // token to be used directly as a Bearer token when accessing the registry
-	dockerCertPath string              // A directory using Docker-like *.{crt,cert,key} files for connecting to a registry or a daemon
-	tlsVerify      optionalBool        // Require HTTPS and verify certificates (for docker: and docker-daemon:)
-	noCreds        bool                // Access the registry anonymously
+	global              *globalOptions             // May be shared across several imageOptions instances.
+	shared              *sharedImageOptions        // May be shared across several imageOptions instances.
+	deprecatedTLSVerify *deprecatedTLSVerifyOption // May be shared across several imageOptions instances, or nil.
+	authFilePath        optionalString             // Path to a */containers/auth.json (prefixed version to override shared image option).
+	credsOption         optionalString             // username[:password] for accessing a registry
+	registryToken       optionalString             // token to be used directly as a Bearer token when accessing the registry
+	dockerCertPath      string                     // A directory using Docker-like *.{crt,cert,key} files for connecting to a registry or a daemon
+	tlsVerify           optionalBool               // Require HTTPS and verify certificates (for docker: and docker-daemon:)
+	noCreds             bool                       // Access the registry anonymously
 }
 
 // imageOptions collects CLI flags which are the same across subcommands, but may be different for each image
@@ -76,11 +107,12 @@ type imageOptions struct {
 
 // dockerImageFlags prepares a collection of docker-transport specific CLI flags
 // writing into imageOptions, and the managed imageOptions structure.
-func dockerImageFlags(global *globalOptions, shared *sharedImageOptions, flagPrefix, credsOptionAlias string) (pflag.FlagSet, *imageOptions) {
+func dockerImageFlags(global *globalOptions, shared *sharedImageOptions, deprecatedTLSVerify *deprecatedTLSVerifyOption, flagPrefix, credsOptionAlias string) (pflag.FlagSet, *imageOptions) {
 	flags := imageOptions{
 		dockerImageOptions: dockerImageOptions{
-			global: global,
-			shared: shared,
+			global:              global,
+			shared:              shared,
+			deprecatedTLSVerify: deprecatedTLSVerify,
 		},
 	}
 
@@ -104,8 +136,8 @@ func dockerImageFlags(global *globalOptions, shared *sharedImageOptions, flagPre
 }
 
 // imageFlags prepares a collection of CLI flags writing into imageOptions, and the managed imageOptions structure.
-func imageFlags(global *globalOptions, shared *sharedImageOptions, flagPrefix, credsOptionAlias string) (pflag.FlagSet, *imageOptions) {
-	dockerFlags, opts := dockerImageFlags(global, shared, flagPrefix, credsOptionAlias)
+func imageFlags(global *globalOptions, shared *sharedImageOptions, deprecatedTLSVerify *deprecatedTLSVerifyOption, flagPrefix, credsOptionAlias string) (pflag.FlagSet, *imageOptions) {
+	dockerFlags, opts := dockerImageFlags(global, shared, deprecatedTLSVerify, flagPrefix, credsOptionAlias)
 
 	fs := pflag.FlagSet{}
 	fs.StringVar(&opts.sharedBlobDir, flagPrefix+"shared-blob-dir", "", "`DIRECTORY` to use to share blobs across OCI repositories")
@@ -135,6 +167,10 @@ func (opts *imageOptions) newSystemContext() (*types.SystemContext, error) {
 	if opts.dockerImageOptions.authFilePath.present {
 		ctx.AuthFilePath = opts.dockerImageOptions.authFilePath.value
 	}
+	if opts.deprecatedTLSVerify != nil && opts.deprecatedTLSVerify.tlsVerify.present {
+		// If both this deprecated option and a non-deprecated option is present, we use the latter value.
+		ctx.DockerInsecureSkipTLSVerify = types.NewOptionalBool(!opts.deprecatedTLSVerify.tlsVerify.value)
+	}
 	if opts.tlsVerify.present {
 		ctx.DockerDaemonInsecureSkipTLSVerify = !opts.tlsVerify.value
 	}
@@ -171,8 +207,8 @@ type imageDestOptions struct {
 }
 
 // imageDestFlags prepares a collection of CLI flags writing into imageDestOptions, and the managed imageDestOptions structure.
-func imageDestFlags(global *globalOptions, shared *sharedImageOptions, flagPrefix, credsOptionAlias string) (pflag.FlagSet, *imageDestOptions) {
-	genericFlags, genericOptions := imageFlags(global, shared, flagPrefix, credsOptionAlias)
+func imageDestFlags(global *globalOptions, shared *sharedImageOptions, deprecatedTLSVerify *deprecatedTLSVerifyOption, flagPrefix, credsOptionAlias string) (pflag.FlagSet, *imageDestOptions) {
+	genericFlags, genericOptions := imageFlags(global, shared, deprecatedTLSVerify, flagPrefix, credsOptionAlias)
 	opts := imageDestOptions{imageOptions: genericOptions}
 	fs := pflag.FlagSet{}
 	fs.AddFlagSet(&genericFlags)