extend skipTLS option to podman

Ankita Thomas · Ankita Thomas · commit 935854c8692a · 2020-08-10T16:52:23.000-04:00
diff --git a/cmd/opm/index/add.go b/cmd/opm/index/add.go
@@ -108,9 +108,13 @@ func runIndexAddCmdFunc(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	skipTLS, err := cmd.Flags().GetBool("skip-tls")
-	if err != nil {
-		return err
+	var skipTLS *bool
+	if cmd.Flags().Changed("skip-tls") {
+		skipTLSVal, err := cmd.Flags().GetBool("skip-tls")
+		if err != nil {
+			return err
+		}
+		skipTLS = &skipTLSVal
 	}
 
 	mode, err := cmd.Flags().GetString("mode")
diff --git a/cmd/opm/index/delete.go b/cmd/opm/index/delete.go
@@ -92,9 +92,13 @@ func runIndexDeleteCmdFunc(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	skipTLS, err := cmd.Flags().GetBool("skip-tls")
-	if err != nil {
-		return err
+	var skipTLS *bool
+	if cmd.Flags().Changed("skip-tls") {
+		skipTLSVal, err := cmd.Flags().GetBool("skip-tls")
+		if err != nil {
+			return err
+		}
+		skipTLS = &skipTLSVal
 	}
 
 	logger := logrus.WithFields(logrus.Fields{"operators": operators})
diff --git a/cmd/opm/index/deprecate.go b/cmd/opm/index/deprecate.go
@@ -106,9 +106,13 @@ func runIndexDeprecateTruncateCmdFunc(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	skipTLS, err := cmd.Flags().GetBool("skip-tls")
-	if err != nil {
-		return err
+	var skipTLS *bool
+	if cmd.Flags().Changed("skip-tls") {
+		skipTLSVal, err := cmd.Flags().GetBool("skip-tls")
+		if err != nil {
+			return err
+		}
+		skipTLS = &skipTLSVal
 	}
 
 	logger := logrus.WithFields(logrus.Fields{"bundles": bundles})
diff --git a/cmd/opm/index/export.go b/cmd/opm/index/export.go
@@ -76,9 +76,13 @@ func runIndexExportCmdFunc(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	skipTLS, err := cmd.Flags().GetBool("skip-tls")
-	if err != nil {
-		return err
+	var skipTLS *bool
+	if cmd.Flags().Changed("skip-tls") {
+		skipTLSVal, err := cmd.Flags().GetBool("skip-tls")
+		if err != nil {
+			return err
+		}
+		skipTLS = &skipTLSVal
 	}
 
 	logger := logrus.WithFields(logrus.Fields{"index": index, "package": packageName})
diff --git a/cmd/opm/index/prune.go b/cmd/opm/index/prune.go
@@ -96,9 +96,13 @@ func runIndexPruneCmdFunc(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	skipTLS, err := cmd.Flags().GetBool("skip-tls")
-	if err != nil {
-		return err
+	var skipTLS *bool
+	if cmd.Flags().Changed("skip-tls") {
+		skipTLSVal, err := cmd.Flags().GetBool("skip-tls")
+		if err != nil {
+			return err
+		}
+		skipTLS = &skipTLSVal
 	}
 
 	logger := logrus.WithFields(logrus.Fields{"packages": packages})
diff --git a/cmd/opm/index/prunestranded.go b/cmd/opm/index/prunestranded.go
@@ -36,6 +36,7 @@ func newIndexPruneStrandedCmd() *cobra.Command {
 	indexCmd.Flags().StringP("binary-image", "i", "", "container image for on-image `opm` command")
 	indexCmd.Flags().StringP("container-tool", "c", "podman", "tool to interact with container images (save, build, etc.). One of: [docker, podman]")
 	indexCmd.Flags().StringP("tag", "t", "", "custom tag for container image being built")
+	indexCmd.Flags().Bool("skip-tls", false, "skip TLS certificate verification for container image registries while pulling index")
 
 	if err := indexCmd.Flags().MarkHidden("debug"); err != nil {
 		logrus.Panic(err.Error())
@@ -80,6 +81,15 @@ func runIndexPruneStrandedCmdFunc(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
+	var skipTLS *bool
+	if cmd.Flags().Changed("skip-tls") {
+		skipTLSVal, err := cmd.Flags().GetBool("skip-tls")
+		if err != nil {
+			return err
+		}
+		skipTLS = &skipTLSVal
+	}
+
 	logger := logrus.WithFields(logrus.Fields{})
 
 	logger.Info("pruning stranded bundles from the index")
@@ -92,6 +102,7 @@ func runIndexPruneStrandedCmdFunc(cmd *cobra.Command, args []string) error {
 		BinarySourceImage: binaryImage,
 		OutDockerfile:     outDockerfile,
 		Tag:               tag,
+		SkipTLS:           skipTLS,
 	}
 
 	err = indexPruner.PruneStrandedFromIndex(request)
diff --git a/cmd/opm/registry/add.go b/cmd/opm/registry/add.go
@@ -41,9 +41,13 @@ func addFunc(cmd *cobra.Command, args []string) error {
 	if err != nil {
 		return err
 	}
-	skipTLS, err := cmd.Flags().GetBool("skip-tls")
-	if err != nil {
-		return err
+	var skipTLS *bool
+	if cmd.Flags().Changed("skip-tls") {
+		skipTLSVal, err := cmd.Flags().GetBool("skip-tls")
+		if err != nil {
+			return err
+		}
+		skipTLS = &skipTLSVal
 	}
 	fromFilename, err := cmd.Flags().GetString("database")
 	if err != nil {
diff --git a/pkg/containertools/factory_docker.go b/pkg/containertools/factory_docker.go
@@ -22,6 +22,10 @@ func (d *DockerCommandFactory) BuildCommand(o BuildOptions) (*exec.Cmd, error) {
 		args = append(args, "-t", tag)
 	}
 
+	if o.secure {
+		args = append(args, "--tls")
+	}
+
 	if o.context == "" {
 		return nil, fmt.Errorf("context not provided")
 	}
diff --git a/pkg/containertools/factory_podman.go b/pkg/containertools/factory_podman.go
@@ -24,6 +24,10 @@ func (p *PodmanCommandFactory) BuildCommand(o BuildOptions) (*exec.Cmd, error) {
 		args = append(args, "-t", tag)
 	}
 
+	if !o.secure {
+		args = append(args, "--tls-verify=false")
+	}
+
 	if o.context == "" {
 		return nil, fmt.Errorf("context not provided")
 	}
diff --git a/pkg/containertools/factory_test.go b/pkg/containertools/factory_test.go
@@ -77,7 +77,7 @@ func TestBuildCommand(t *testing.T) {
 			Factory: &PodmanCommandFactory{},
 			Options: DefaultBuildOptions(),
 			Args: []string{
-				"podman", "build", "--format", "docker", ".",
+				"podman", "build", "--format", "docker", "--tls-verify=false", ".",
 			},
 		},
 		{
@@ -88,7 +88,7 @@ func TestBuildCommand(t *testing.T) {
 				format:  "oci",
 			},
 			Args: []string{
-				"podman", "build", "--format", "oci", ".",
+				"podman", "build", "--format", "oci", "--tls-verify=false", ".",
 			},
 		},
 		{
@@ -98,7 +98,7 @@ func TestBuildCommand(t *testing.T) {
 				context: "foo",
 			},
 			Args: []string{
-				"podman", "build", "--format", "docker", "foo",
+				"podman", "build", "--format", "docker", "--tls-verify=false", "foo",
 			},
 		},
 		{
@@ -109,7 +109,7 @@ func TestBuildCommand(t *testing.T) {
 				dockerfile: "foo",
 			},
 			Args: []string{
-				"podman", "build", "--format", "docker", "-f", "foo", ".",
+				"podman", "build", "--format", "docker", "-f", "foo", "--tls-verify=false", ".",
 			},
 		},
 		{
@@ -120,7 +120,7 @@ func TestBuildCommand(t *testing.T) {
 				tags:    []string{"foo"},
 			},
 			Args: []string{
-				"podman", "build", "--format", "docker", "-t", "foo", ".",
+				"podman", "build", "--format", "docker", "-t", "foo", "--tls-verify=false", ".",
 			},
 		},
 		{
@@ -131,7 +131,7 @@ func TestBuildCommand(t *testing.T) {
 				tags:    []string{"foo", "bar"},
 			},
 			Args: []string{
-				"podman", "build", "--format", "docker", "-t", "foo", "-t", "bar", ".",
+				"podman", "build", "--format", "docker", "-t", "foo", "-t", "bar", "--tls-verify=false", ".",
 			},
 		},
 		{
diff --git a/pkg/containertools/option_build.go b/pkg/containertools/option_build.go
@@ -5,6 +5,7 @@ type BuildOptions struct {
 	tags       []string
 	dockerfile string
 	context    string
+	secure    bool
 }
 
 func (o *BuildOptions) SetFormatDocker() {
@@ -27,6 +28,10 @@ func (o *BuildOptions) SetContext(context string) {
 	o.context = context
 }
 
+func (o *BuildOptions) SetSkipTLS(skipTLS bool) {
+	o.secure = !skipTLS
+}
+
 func DefaultBuildOptions() BuildOptions {
 	var o BuildOptions
 	o.SetFormatDocker()
diff --git a/pkg/containertools/runner.go b/pkg/containertools/runner.go
@@ -22,14 +22,75 @@ type CommandRunner interface {
 type ContainerCommandRunner struct {
 	logger        *logrus.Entry
 	containerTool ContainerTool
+	config        *RunnerConfig
+}
+
+type RunnerConfig struct {
+	SkipTLS bool
+}
+
+type RunnerOption func(config *RunnerConfig)
+
+func SkipTLS(skip *bool) RunnerOption {
+	return func(config *RunnerConfig) {
+		if skip != nil {
+			config.SkipTLS = *skip
+		}
+	}
+}
+
+func (r *RunnerConfig) apply(options []RunnerOption) {
+	for _, option := range options {
+		option(r)
+	}
+}
+
+func (r *ContainerCommandRunner) argsForCmd(cmd string, args... string) []string {
+	cmdArgs := []string{cmd}
+	switch r.containerTool {
+	case PodmanTool:
+		switch cmd {
+		case "build", "pull", "push", "login", "search":
+			// --tls-verify is a valid flag for these podman subcommands
+			if r.config.SkipTLS {
+				cmdArgs = append(cmdArgs, "--tls-verify=false")
+			}
+		}
+	case DockerTool:
+		if !r.config.SkipTLS {
+			cmdArgs = append(cmdArgs, "--tls")
+		}
+	default:
+	}
+	cmdArgs = append(cmdArgs, args...)
+	return cmdArgs
+}
+
+func defaultConfig(toolName string) *RunnerConfig {
+	switch toolName {
+	case "docker":
+		// docker disables tls verify by default, mimic that behavior
+		return &RunnerConfig{
+			SkipTLS: true,
+		}
+	case "podman":
+		return &RunnerConfig{
+			SkipTLS: false,
+		}
+	default:
+		return &RunnerConfig{}
+	}
 }
 
 // NewCommandRunner takes the containerTool as an input string and returns a
 // CommandRunner to run commands with that cli tool
-func NewCommandRunner(containerTool ContainerTool, logger *logrus.Entry) *ContainerCommandRunner {
+func NewCommandRunner(containerTool ContainerTool, logger *logrus.Entry, opts... RunnerOption) *ContainerCommandRunner {
+	config := defaultConfig(containerTool.String())
+	config.apply(opts)
 	r := &ContainerCommandRunner{
 		logger:        logger,
 		containerTool: containerTool,
+		config:        config,
 	}
 	return r
 }
@@ -42,7 +103,7 @@ func (r *ContainerCommandRunner) GetToolName() string {
 // Pull takes a container image path hosted on a container registry and runs the
 // pull command to download it onto the local environment
 func (r *ContainerCommandRunner) Pull(image string) error {
-	args := []string{"pull", image}
+	args := r.argsForCmd("pull", image)
 
 	command := exec.Command(r.containerTool.String(), args...)
 
@@ -65,6 +126,7 @@ func (r *ContainerCommandRunner) Build(dockerfile, tag string) error {
 	}
 	o.SetDockerfile(dockerfile)
 	o.SetContext(".")
+	o.SetSkipTLS(r.config.SkipTLS)
 	command, err := r.containerTool.CommandFactory().BuildCommand(o)
 	if err != nil {
 		return fmt.Errorf("unable to perform build: %v", err)
@@ -84,7 +146,7 @@ func (r *ContainerCommandRunner) Build(dockerfile, tag string) error {
 
 // Unpack copies a directory from a local container image to a directory in the local filesystem.
 func (r *ContainerCommandRunner) Unpack(image, src, dst string) error {
-	args := []string{"create", image, ""}
+	args := r.argsForCmd("create", image, "")
 
 	command := exec.Command(r.containerTool.String(), args...)
 
@@ -98,7 +160,7 @@ func (r *ContainerCommandRunner) Unpack(image, src, dst string) error {
 	}
 
 	id := strings.TrimSuffix(string(out), "\n")
-	args = []string{"cp", id + ":" + src, dst}
+	args = r.argsForCmd("cp", id + ":" + src, dst)
 	command = exec.Command(r.containerTool.String(), args...)
 
 	r.logger.Infof("running %s cp", r.containerTool)
@@ -110,7 +172,7 @@ func (r *ContainerCommandRunner) Unpack(image, src, dst string) error {
 		return fmt.Errorf("error copying container directory %s: %v", string(out), err)
 	}
 
-	args = []string{"rm", id}
+	args = r.argsForCmd("rm", id)
 	command = exec.Command(r.containerTool.String(), args...)
 
 	r.logger.Infof("running %s rm", r.containerTool)
@@ -128,7 +190,7 @@ func (r *ContainerCommandRunner) Unpack(image, src, dst string) error {
 // Inspect runs the 'inspect' command to get image metadata of a local container
 // image and returns a byte array of the command's output
 func (r *ContainerCommandRunner) Inspect(image string) ([]byte, error) {
-	args := []string{"inspect", image}
+	args := r.argsForCmd("inspect", image)
 
 	command := exec.Command(r.containerTool.String(), args...)
 
diff --git a/pkg/image/execregistry/registry.go b/pkg/image/execregistry/registry.go
@@ -26,10 +26,10 @@ type Registry struct {
 var _ image.Registry = &Registry{}
 
 // NewRegistry instantiates and returns a new registry which manipulates images via exec podman/docker commands.
-func NewRegistry(tool containertools.ContainerTool, logger *logrus.Entry) (registry *Registry, err error) {
+func NewRegistry(tool containertools.ContainerTool, logger *logrus.Entry, opts... containertools.RunnerOption) (registry *Registry, err error) {
 	return &Registry{
 		log: logger,
-		cmd: containertools.NewCommandRunner(tool, logger),
+		cmd: containertools.NewCommandRunner(tool, logger, opts...),
 	}, nil
 }
 
diff --git a/pkg/lib/indexer/indexer.go b/pkg/lib/indexer/indexer.go
diff --git a/pkg/lib/registry/registry.go b/pkg/lib/registry/registry.go

Original file line number	Diff line number	Diff line change
`@@ -22,6 +22,10 @@ func (d DockerCommandFactory) BuildCommand(o BuildOptions) (exec.Cmd, error) {`
`22`	`22`	`args = append(args, "-t", tag)`
`23`	`23`	`}`
`24`	`24`
	`25`	`+ if o.secure {`
	`26`	`+ args = append(args, "--tls")`
	`27`	`+ }`
	`28`	`+`
`25`	`29`	`if o.context == "" {`
`26`	`30`	`return nil, fmt.Errorf("context not provided")`
`27`	`31`	`}`
Original file line number	Diff line number	Diff line change
`@@ -24,6 +24,10 @@ func (p PodmanCommandFactory) BuildCommand(o BuildOptions) (exec.Cmd, error) {`
`24`	`24`	`args = append(args, "-t", tag)`
`25`	`25`	`}`
`26`	`26`
	`27`	`+ if !o.secure {`
	`28`	`+ args = append(args, "--tls-verify=false")`
	`29`	`+ }`
	`30`	`+`
`27`	`31`	`if o.context == "" {`
`28`	`32`	`return nil, fmt.Errorf("context not provided")`
`29`	`33`	`}`