Skip to content

Commit

Permalink
Fix --tls-verify
Browse files Browse the repository at this point in the history 
Differentiate, again, between (skopeo --tls-verify subcommand)
and (skope subcommand --tls-verify), by
- using a "local" Corba flag for the (skopeo --tls-verify ...) variant
- adding separate --tls-verify flags to subcommands that only accept
  them as legacy, available through deprecatedTLSVerifyFlags
  (unlike the non-legacy path of dockerImageFlags());
- using TraverseChildren: true; this causes the global and
  per-subcommand flags to be treated separately by Corba,
  i.e. they no longer happen to share the "Hidden" flag
  and Corba actually sets the right flag variable now.

So, we can now warn on (skopeo --tls-verify command) again,
and --help lists the flag correctly (it is hidden at the
global level, and in subcommands like copy that deprecated it,
but visible in subcommands like inspect where it's not deprecated).

NOTE: This removes --tls-verify from (skopeo manifest-digest) and
the three signing commands; it never made sense there. This change
could, in principle, break some users.

Also update man pages to match.

Signed-off-by: Miloslav Trmač <[email protected]>
  • Loading branch information
@mtrmac
mtrmac committed Jul 23, 2021
1 parent bb447f2 commit 726d982
Show file tree
Hide file tree
Showing 14 changed files with 213 additions and 88 deletions.
43 changes: 24 additions & 19 deletions cmd/skopeo/copy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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)
Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion cmd/skopeo/delete.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion cmd/skopeo/inspect.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion cmd/skopeo/layers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion cmd/skopeo/list_tags.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
10 changes: 8 additions & 2 deletions cmd/skopeo/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)")
Expand All @@ -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),
Expand Down
39 changes: 22 additions & 17 deletions cmd/skopeo/sync.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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{
Expand All @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down
66 changes: 51 additions & 15 deletions cmd/skopeo/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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 {
Expand All @@ -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
Expand All @@ -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,
},
}

Expand All @@ -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")
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
Expand Down
Loading

0 comments on commit 726d982

Please sign in to comment.