From 590f334c00cc87cbdd22648c3995cb3a1a6dc20a Mon Sep 17 00:00:00 2001 From: Jennifer Power Date: Fri, 14 Jan 2022 16:33:36 -0500 Subject: [PATCH 1/3] fix(registry): deprecated skip-tls and add use-http and skip-tls-verify The skip-tls flag currently uses HTTP instead of the expected behavior of skipping TLS cert validated with HTTPS registries. The new flags seperate untrusted HTTPS from HTTP registries and behave as expected Signed-off-by: Jennifer Power --- Makefile | 2 +- cmd/opm/alpha/bundle/unpack.go | 29 ++++++++++++-- cmd/opm/alpha/diff/cmd.go | 22 ++++++++++- cmd/opm/index/add.go | 19 +++++++++- cmd/opm/index/cmd.go | 6 +++ cmd/opm/index/delete.go | 19 +++++++++- cmd/opm/index/deprecatetruncate.go | 19 +++++++++- cmd/opm/index/export.go | 19 +++++++++- cmd/opm/index/prune.go | 19 +++++++++- cmd/opm/index/prunestranded.go | 19 +++++++++- cmd/opm/registry/add.go | 28 ++++++++++++-- docs/contributors/e2e_tests.md | 4 +- pkg/image/containerdregistry/options.go | 15 ++++++-- pkg/image/containerdregistry/resolver.go | 8 ++-- pkg/lib/bundle/exporter.go | 11 ++++-- pkg/lib/bundle/exporter_test.go | 7 +++- pkg/lib/indexer/indexer.go | 48 ++++++++++++++---------- pkg/lib/registry/registry.go | 11 ++++-- test/e2e/e2e_suite_test.go | 3 +- test/e2e/opm_test.go | 14 ++++--- 20 files changed, 262 insertions(+), 60 deletions(-) diff --git a/Makefile b/Makefile index 41c1674e5..e9743efa7 100644 --- a/Makefile +++ b/Makefile @@ -116,7 +116,7 @@ clean: .PHONY: e2e e2e: - $(GO) run github.com/onsi/ginkgo/ginkgo --v --randomizeAllSpecs --randomizeSuites --race $(if $(TEST),-focus '$(TEST)') $(TAGS) ./test/e2e -- $(if $(SKIPTLS),-skip-tls true) + $(GO) run github.com/onsi/ginkgo/ginkgo --v --randomizeAllSpecs --randomizeSuites --race $(if $(TEST),-focus '$(TEST)') $(TAGS) ./test/e2e -- $(if $(SKIPTLS),-skip-tls-verify true) $(if $(USEHTTP),-use-http true) .PHONY: release diff --git a/cmd/opm/alpha/bundle/unpack.go b/cmd/opm/alpha/bundle/unpack.go index 67f3b311a..ec8f648e3 100644 --- a/cmd/opm/alpha/bundle/unpack.go +++ b/cmd/opm/alpha/bundle/unpack.go @@ -28,11 +28,16 @@ func newBundleUnpackCmd() *cobra.Command { RunE: unpackBundle, } unpack.Flags().BoolP("debug", "d", false, "enable debug log output") - unpack.Flags().BoolP("skip-tls", "s", false, "disable TLS verification") + unpack.Flags().BoolP("skip-tls", "s", false, "use plain HTTP") + unpack.Flags().Bool("skip-tls-verify", false, "disable TLS verification") + unpack.Flags().Bool("use-http", false, "use plain HTTP") unpack.Flags().BoolP("skip-validation", "v", false, "disable bundle validation") unpack.Flags().StringP("root-ca", "c", "", "file path of a root CA to use when communicating with image registries") unpack.Flags().StringP("out", "o", "./", "directory in which to unpack operator bundle content") + if err := unpack.Flags().MarkDeprecated("skip-tls", "use --use-http and --skip-tls-verify instead"); err != nil { + logrus.Panic(err.Error()) + } return unpack } @@ -70,14 +75,30 @@ func unpackBundle(cmd *cobra.Command, args []string) error { } var ( - registryOpts []containerdregistry.RegistryOption - skipTLS bool + registryOpts []containerdregistry.RegistryOption + useHTTP bool + skipTLSVerify bool + skipTLS bool ) skipTLS, err = cmd.Flags().GetBool("skip-tls") if err != nil { return err } - registryOpts = append(registryOpts, containerdregistry.SkipTLS(skipTLS)) + skipTLSVerify, err = cmd.Flags().GetBool("skip-tls-verify") + if err != nil { + return err + } + useHTTP, err = cmd.Flags().GetBool("use-http") + if err != nil { + return err + } + if skipTLS { + // Set useHTTP when use deprecated skipTlS + // for functional parity with existing + useHTTP = true + } + + registryOpts = append(registryOpts, containerdregistry.SkipTLSVerify(skipTLSVerify), containerdregistry.WithPlainHTTP(useHTTP)) var skipValidation bool skipValidation, err = cmd.Flags().GetBool("skip-validation") diff --git a/cmd/opm/alpha/diff/cmd.go b/cmd/opm/alpha/diff/cmd.go index 752e4d343..b2c95462b 100644 --- a/cmd/opm/alpha/diff/cmd.go +++ b/cmd/opm/alpha/diff/cmd.go @@ -156,13 +156,31 @@ func (a *diff) addFunc(cmd *cobra.Command, args []string) error { skipTLS, err := cmd.Flags().GetBool("skip-tls") if err != nil { - logrus.Panic(err) + return err + } + skipTLSVerify, err := cmd.Flags().GetBool("skip-tls-verify") + if err != nil { + return err + } + useHTTP, err := cmd.Flags().GetBool("use-http") + if err != nil { + return err + } + if skipTLS { + // Set useHTTP when use deprecated skipTlS + // for functional parity with existing + useHTTP = true } rootCAs, err := certs.RootCAs(a.caFile) if err != nil { a.logger.Fatalf("error getting root CAs: %v", err) } - reg, err := containerd.NewRegistry(containerd.SkipTLS(skipTLS), containerd.WithLog(a.logger), containerd.WithRootCAs(rootCAs)) + reg, err := containerd.NewRegistry( + containerd.SkipTLSVerify(skipTLSVerify), + containerd.WithLog(a.logger), + containerd.WithRootCAs(rootCAs), + containerd.WithPlainHTTP(useHTTP), + ) if err != nil { a.logger.Fatalf("error creating containerd registry: %v", err) } diff --git a/cmd/opm/index/add.go b/cmd/opm/index/add.go index ea218d6e0..ecf333955 100644 --- a/cmd/opm/index/add.go +++ b/cmd/opm/index/add.go @@ -131,6 +131,22 @@ func runIndexAddCmdFunc(cmd *cobra.Command, _ []string) error { return err } + skipTLSVerify, err := cmd.Flags().GetBool("skip-tls-verify") + if err != nil { + return err + } + + useHTTP, err := cmd.Flags().GetBool("use-http") + if err != nil { + return err + } + + if skipTLS { + // Set useHTTP when use deprecated skipTlS + // for functional parity with existing + useHTTP = true + } + mode, err := cmd.Flags().GetString("mode") if err != nil { return err @@ -174,7 +190,8 @@ func runIndexAddCmdFunc(cmd *cobra.Command, _ []string) error { Bundles: bundles, Permissive: permissive, Mode: modeEnum, - SkipTLS: skipTLS, + SkipTLSVerify: skipTLSVerify, + PlainHTTP: useHTTP, Overwrite: overwrite, EnableAlpha: enableAlpha, } diff --git a/cmd/opm/index/cmd.go b/cmd/opm/index/cmd.go index b7cdd14d1..3e3aeb499 100644 --- a/cmd/opm/index/cmd.go +++ b/cmd/opm/index/cmd.go @@ -33,6 +33,12 @@ func AddCommand(parent *cobra.Command) { parent.AddCommand(cmd) parent.PersistentFlags().Bool("skip-tls", false, "skip TLS certificate verification for container image registries while pulling bundles or index") + parent.PersistentFlags().Bool("skip-tls-verify", false, "skip TLS certificate verification for container image registries while pulling bundles") + parent.PersistentFlags().Bool("use-http", false, "use plain HTTP for container image registries while pulling bundles") + if err := parent.PersistentFlags().MarkDeprecated("skip-tls", "use --use-http and --skip-tls-verify instead"); err != nil { + logrus.Panic(err.Error()) + } + cmd.AddCommand(newIndexDeleteCmd()) addIndexAddCmd(cmd) cmd.AddCommand(newIndexExportCmd()) diff --git a/cmd/opm/index/delete.go b/cmd/opm/index/delete.go index c9472b8ed..28eddeec7 100644 --- a/cmd/opm/index/delete.go +++ b/cmd/opm/index/delete.go @@ -100,6 +100,22 @@ func runIndexDeleteCmdFunc(cmd *cobra.Command, _ []string) error { return err } + skipTLSVerify, err := cmd.Flags().GetBool("skip-tls-verify") + if err != nil { + return err + } + + useHTTP, err := cmd.Flags().GetBool("use-http") + if err != nil { + return err + } + + if skipTLS { + // Set useHTTP when use deprecated skipTlS + // for functional parity with existing + useHTTP = true + } + logger := logrus.WithFields(logrus.Fields{"operators": operators}) logger.Info("building the index") @@ -117,7 +133,8 @@ func runIndexDeleteCmdFunc(cmd *cobra.Command, _ []string) error { Operators: operators, Tag: tag, Permissive: permissive, - SkipTLS: skipTLS, + SkipTLSVerify: skipTLSVerify, + PlainHTTP: useHTTP, } err = indexDeleter.DeleteFromIndex(request) diff --git a/cmd/opm/index/deprecatetruncate.go b/cmd/opm/index/deprecatetruncate.go index 9324d35b8..fb769b241 100644 --- a/cmd/opm/index/deprecatetruncate.go +++ b/cmd/opm/index/deprecatetruncate.go @@ -115,6 +115,22 @@ func runIndexDeprecateTruncateCmdFunc(cmd *cobra.Command, _ []string) error { return err } + skipTLSVerify, err := cmd.Flags().GetBool("skip-tls-verify") + if err != nil { + return err + } + + useHTTP, err := cmd.Flags().GetBool("use-http") + if err != nil { + return err + } + + if skipTLS { + // Set useHTTP when use deprecated skipTlS + // for functional parity with existing + useHTTP = true + } + allowPackageRemoval, err := cmd.Flags().GetBool("allow-package-removal") if err != nil { return err @@ -137,7 +153,8 @@ func runIndexDeprecateTruncateCmdFunc(cmd *cobra.Command, _ []string) error { Tag: tag, Bundles: bundles, Permissive: permissive, - SkipTLS: skipTLS, + SkipTLSVerify: skipTLSVerify, + PlainHTTP: useHTTP, AllowPackageRemoval: allowPackageRemoval, } diff --git a/cmd/opm/index/export.go b/cmd/opm/index/export.go index f4e32672e..522edc518 100644 --- a/cmd/opm/index/export.go +++ b/cmd/opm/index/export.go @@ -105,6 +105,22 @@ func runIndexExportCmdFunc(cmd *cobra.Command, _ []string) error { return err } + skipTLSVerify, err := cmd.Flags().GetBool("skip-tls-verify") + if err != nil { + return err + } + + useHTTP, err := cmd.Flags().GetBool("use-http") + if err != nil { + return err + } + + if skipTLS { + // Set useHTTP when use deprecated skipTlS + // for functional parity with existing + useHTTP = true + } + logger := logrus.WithFields(logrus.Fields{"index": index, "package": packages}) logger.Info("export from the index") @@ -116,7 +132,8 @@ func runIndexExportCmdFunc(cmd *cobra.Command, _ []string) error { Packages: packages, DownloadPath: downloadPath, ContainerTool: containertools.NewContainerTool(containerTool, containertools.NoneTool), - SkipTLS: skipTLS, + SkipTLSVerify: skipTLSVerify, + PlainHTTP: useHTTP, } err = indexExporter.ExportFromIndex(request) diff --git a/cmd/opm/index/prune.go b/cmd/opm/index/prune.go index c80ebf619..acfc581c0 100644 --- a/cmd/opm/index/prune.go +++ b/cmd/opm/index/prune.go @@ -104,6 +104,22 @@ func runIndexPruneCmdFunc(cmd *cobra.Command, _ []string) error { return err } + skipTLSVerify, err := cmd.Flags().GetBool("skip-tls-verify") + if err != nil { + return err + } + + useHTTP, err := cmd.Flags().GetBool("use-http") + if err != nil { + return err + } + + if skipTLS { + // Set useHTTP when use deprecated skipTlS + // for functional parity with existing + useHTTP = true + } + logger := logrus.WithFields(logrus.Fields{"packages": packages}) logger.Info("pruning the index") @@ -118,7 +134,8 @@ func runIndexPruneCmdFunc(cmd *cobra.Command, _ []string) error { Packages: packages, Tag: tag, Permissive: permissive, - SkipTLS: skipTLS, + SkipTLSVerify: skipTLSVerify, + PlainHTTP: useHTTP, } err = indexPruner.PruneFromIndex(request) diff --git a/cmd/opm/index/prunestranded.go b/cmd/opm/index/prunestranded.go index 03e739cff..ed6005e1f 100644 --- a/cmd/opm/index/prunestranded.go +++ b/cmd/opm/index/prunestranded.go @@ -89,6 +89,22 @@ func runIndexPruneStrandedCmdFunc(cmd *cobra.Command, _ []string) error { return err } + skipTLSVerify, err := cmd.Flags().GetBool("skip-tls-verify") + if err != nil { + return err + } + + useHTTP, err := cmd.Flags().GetBool("use-http") + if err != nil { + return err + } + + if skipTLS { + // Set useHTTP when use deprecated skipTlS + // for functional parity with existing + useHTTP = true + } + logger := logrus.WithFields(logrus.Fields{}) logger.Info("pruning stranded bundles from the index") @@ -101,7 +117,8 @@ func runIndexPruneStrandedCmdFunc(cmd *cobra.Command, _ []string) error { BinarySourceImage: binaryImage, OutDockerfile: outDockerfile, Tag: tag, - SkipTLS: skipTLS, + SkipTLSVerify: skipTLSVerify, + PlainHTTP: useHTTP, } err = indexPruner.PruneStrandedFromIndex(request) diff --git a/cmd/opm/registry/add.go b/cmd/opm/registry/add.go index ceb3fcd1a..e7b438394 100644 --- a/cmd/opm/registry/add.go +++ b/cmd/opm/registry/add.go @@ -36,7 +36,9 @@ func newRegistryAddCmd() *cobra.Command { rootCmd.Flags().StringP("database", "d", "bundles.db", "relative path to database file") rootCmd.Flags().StringSliceP("bundle-images", "b", []string{}, "comma separated list of links to bundle image") rootCmd.Flags().Bool("permissive", false, "allow registry load errors") - rootCmd.Flags().Bool("skip-tls", false, "skip TLS certificate verification for container image registries while pulling bundles") + rootCmd.Flags().Bool("skip-tls", false, "use Plain HTTP for container image registries while pulling bundles") + rootCmd.Flags().Bool("skip-tls-verify", false, "skip TLS certificate verification for container image registries while pulling bundles") + rootCmd.Flags().Bool("use-http", false, "use plain HTTP for container image registries while pulling bundles") rootCmd.Flags().String("ca-file", "", "the root certificates to use when --container-tool=none; see docker/podman docs for certificate loading instructions") rootCmd.Flags().StringP("mode", "", "replaces", "graph update mode that defines how channel graphs are updated. One of: [replaces, semver, semver-skippatch]") rootCmd.Flags().StringP("container-tool", "c", "none", "tool to interact with container images (save, build, etc.). One of: [none, docker, podman]") @@ -48,6 +50,9 @@ func newRegistryAddCmd() *cobra.Command { if err := rootCmd.Flags().MarkHidden("enable-alpha"); err != nil { logrus.Panic(err.Error()) } + if err := rootCmd.Flags().MarkDeprecated("skip-tls", "use --use-http and --skip-tls-verify instead"); err != nil { + logrus.Panic(err.Error()) + } return rootCmd } @@ -60,6 +65,14 @@ func addFunc(cmd *cobra.Command, _ []string) error { if err != nil { return err } + skipTLSVerify, err := cmd.Flags().GetBool("skip-tls-verify") + if err != nil { + return err + } + useHTTP, err := cmd.Flags().GetBool("use-http") + if err != nil { + return err + } caFile, err := cmd.Flags().GetString("ca-file") if err != nil { return err @@ -96,8 +109,8 @@ func addFunc(cmd *cobra.Command, _ []string) error { } if caFile != "" { - if skipTLS { - return errors.New("--skip-tls must be false when --ca-file is set") + if skipTLSVerify { + return errors.New("--skip-tls-verify must be false when --ca-file is set") } if containerTool != containertools.NoneTool { return fmt.Errorf("--ca-file cannot be set with --container-tool=%[1]s; "+ @@ -105,9 +118,16 @@ func addFunc(cmd *cobra.Command, _ []string) error { } } + if skipTLS { + // Set useHTTP when use + // deprecated skipTlS for functional parity with existing + useHTTP = true + } + request := registry.AddToRegistryRequest{ Permissive: permissive, - SkipTLS: skipTLS, + SkipTLSVerify: skipTLSVerify, + PlainHTTP: useHTTP, CaFile: caFile, InputDatabase: fromFilename, Bundles: bundleImages, diff --git a/docs/contributors/e2e_tests.md b/docs/contributors/e2e_tests.md index a0eb50eed..6d716128b 100644 --- a/docs/contributors/e2e_tests.md +++ b/docs/contributors/e2e_tests.md @@ -19,13 +19,13 @@ running even after the test suite has completed. 1. Start the e2e tests: ```bash - DOCKER_REGISTRY_HOST=localhost:5000 GOENV='GOOS=linux' make build e2e SKIPTLS="true" CLUSTER=kind + DOCKER_REGISTRY_HOST=localhost:5000 GOENV='GOOS=linux' make build e2e USEHTTP="true" CLUSTER=kind ``` 1. Run a specific BDD test using the `TEST` argument to make. Note that this argument uses regular expressions. ```bash - DOCKER_REGISTRY_HOST=localhost:5000 GOENV='GOOS=linux' make build e2e TEST='builds and manipulates bundle and index images' SKIPTLS="true" CLUSTER=kind + DOCKER_REGISTRY_HOST=localhost:5000 GOENV='GOOS=linux' make build e2e TEST='builds and manipulates bundle and index images' USEHTTP="true" CLUSTER=kind ``` 1. If you want a quick way to ensure that your TEST regex argument will work, you can bypass the diff --git a/pkg/image/containerdregistry/options.go b/pkg/image/containerdregistry/options.go index de0be2f70..569395feb 100644 --- a/pkg/image/containerdregistry/options.go +++ b/pkg/image/containerdregistry/options.go @@ -21,7 +21,8 @@ type RegistryConfig struct { DBPath string CacheDir string PreserveCache bool - SkipTLS bool + SkipTLSVerify bool + PlainHTTP bool Roots *x509.CertPool } @@ -90,7 +91,7 @@ func NewRegistry(options ...RegistryOption) (registry *Registry, err error) { } var resolver remotes.Resolver - resolver, err = NewResolver(config.ResolverConfigDir, config.SkipTLS, config.Roots) + resolver, err = NewResolver(config.ResolverConfigDir, config.SkipTLSVerify, config.PlainHTTP, config.Roots) if err != nil { return } @@ -140,8 +141,14 @@ func PreserveCache(preserve bool) RegistryOption { } } -func SkipTLS(skip bool) RegistryOption { +func SkipTLSVerify(skip bool) RegistryOption { return func(config *RegistryConfig) { - config.SkipTLS = skip + config.SkipTLSVerify = skip + } +} + +func WithPlainHTTP(insecure bool) RegistryOption { + return func(config *RegistryConfig) { + config.PlainHTTP = insecure } } diff --git a/pkg/image/containerdregistry/resolver.go b/pkg/image/containerdregistry/resolver.go index 2107215c4..7bae18a9e 100644 --- a/pkg/image/containerdregistry/resolver.go +++ b/pkg/image/containerdregistry/resolver.go @@ -18,7 +18,7 @@ import ( "github.com/docker/docker/registry" ) -func NewResolver(configDir string, insecure bool, roots *x509.CertPool) (remotes.Resolver, error) { +func NewResolver(configDir string, skipTlSVerify, plainHTTP bool, roots *x509.CertPool) (remotes.Resolver, error) { transport := &http.Transport{ Proxy: http.ProxyFromEnvironment, DialContext: (&net.Dialer{ @@ -35,9 +35,9 @@ func NewResolver(configDir string, insecure bool, roots *x509.CertPool) (remotes }, } - if insecure { + if plainHTTP || skipTlSVerify { transport.TLSClientConfig = &tls.Config{ - InsecureSkipVerify: insecure, + InsecureSkipVerify: true, } } headers := http.Header{} @@ -58,7 +58,7 @@ func NewResolver(configDir string, insecure bool, roots *x509.CertPool) (remotes )), docker.WithClient(client), } - if insecure { + if plainHTTP { regopts = append(regopts, docker.WithPlainHTTP(docker.MatchAllHosts)) } diff --git a/pkg/lib/bundle/exporter.go b/pkg/lib/bundle/exporter.go index 53469f868..5a25156a2 100644 --- a/pkg/lib/bundle/exporter.go +++ b/pkg/lib/bundle/exporter.go @@ -30,7 +30,7 @@ func NewExporterForBundle(image, directory string, containerTool containertools. } } -func (i *BundleExporter) Export(skipTLS bool) error { +func (i *BundleExporter) Export(skipTLSVerify, useHTTP bool) error { log := logrus.WithField("img", i.image) @@ -44,11 +44,16 @@ func (i *BundleExporter) Export(skipTLS bool) error { var rerr error switch i.containerTool { case containertools.NoneTool: - reg, rerr = containerdregistry.NewRegistry(containerdregistry.SkipTLS(skipTLS), containerdregistry.WithLog(log), containerdregistry.WithCacheDir(filepath.Join(tmpDir, "cacheDir"))) + reg, rerr = containerdregistry.NewRegistry( + containerdregistry.SkipTLSVerify(skipTLSVerify), + containerdregistry.WithPlainHTTP(useHTTP), + containerdregistry.WithLog(log), + containerdregistry.WithCacheDir(filepath.Join(tmpDir, "cacheDir")), + ) case containertools.PodmanTool: fallthrough case containertools.DockerTool: - reg, rerr = execregistry.NewRegistry(i.containerTool, log, containertools.SkipTLS(skipTLS)) + reg, rerr = execregistry.NewRegistry(i.containerTool, log, containertools.SkipTLS(skipTLSVerify)) } if rerr != nil { return rerr diff --git a/pkg/lib/bundle/exporter_test.go b/pkg/lib/bundle/exporter_test.go index 7a3df6336..6a9477497 100644 --- a/pkg/lib/bundle/exporter_test.go +++ b/pkg/lib/bundle/exporter_test.go @@ -9,10 +9,13 @@ import ( func TestExportForBundleWithBadImage(t *testing.T) { exporter := NewExporterForBundle("foo", "", containertools.DockerTool) - err := exporter.Export(true) + err := exporter.Export(true, false) + assert.Error(t, err) + + err = exporter.Export(false, true) assert.Error(t, err) exporter = NewExporterForBundle("foo", "", containertools.NoneTool) - err = exporter.Export(true) + err = exporter.Export(true, false) assert.Error(t, err) } diff --git a/pkg/lib/indexer/indexer.go b/pkg/lib/indexer/indexer.go index 54eae4db1..fb14341c9 100644 --- a/pkg/lib/indexer/indexer.go +++ b/pkg/lib/indexer/indexer.go @@ -63,7 +63,8 @@ type AddToIndexRequest struct { Tag string Mode pregistry.Mode CaFile string - SkipTLS bool + SkipTLSVerify bool + PlainHTTP bool Overwrite bool EnableAlpha bool } @@ -76,7 +77,7 @@ func (i ImageIndexer) AddToIndex(request AddToIndexRequest) error { return err } - databasePath, err := i.ExtractDatabase(buildDir, request.FromIndex, request.CaFile, request.SkipTLS) + databasePath, err := i.ExtractDatabase(buildDir, request.FromIndex, request.CaFile, request.SkipTLSVerify, request.PlainHTTP) if err != nil { return err } @@ -87,7 +88,7 @@ func (i ImageIndexer) AddToIndex(request AddToIndexRequest) error { InputDatabase: databasePath, Permissive: request.Permissive, Mode: request.Mode, - SkipTLS: request.SkipTLS, + SkipTLSVerify: request.SkipTLSVerify, ContainerTool: i.PullTool, Overwrite: request.Overwrite, EnableAlpha: request.EnableAlpha, @@ -129,7 +130,8 @@ type DeleteFromIndexRequest struct { OutDockerfile string Tag string Operators []string - SkipTLS bool + SkipTLSVerify bool + PlainHTTP bool CaFile string } @@ -142,7 +144,7 @@ func (i ImageIndexer) DeleteFromIndex(request DeleteFromIndexRequest) error { return err } - databasePath, err := i.ExtractDatabase(buildDir, request.FromIndex, request.CaFile, request.SkipTLS) + databasePath, err := i.ExtractDatabase(buildDir, request.FromIndex, request.CaFile, request.SkipTLSVerify, request.PlainHTTP) if err != nil { return err } @@ -188,7 +190,8 @@ type PruneStrandedFromIndexRequest struct { OutDockerfile string Tag string CaFile string - SkipTLS bool + SkipTLSVerify bool + PlainHTTP bool } // PruneStrandedFromIndex is an aggregate API used to generate a registry index image @@ -200,7 +203,7 @@ func (i ImageIndexer) PruneStrandedFromIndex(request PruneStrandedFromIndexReque return err } - databasePath, err := i.ExtractDatabase(buildDir, request.FromIndex, request.CaFile, request.SkipTLS) + databasePath, err := i.ExtractDatabase(buildDir, request.FromIndex, request.CaFile, request.SkipTLSVerify, request.PlainHTTP) if err != nil { return err } @@ -245,7 +248,8 @@ type PruneFromIndexRequest struct { Tag string Packages []string CaFile string - SkipTLS bool + SkipTLSVerify bool + PlainHTTP bool } func (i ImageIndexer) PruneFromIndex(request PruneFromIndexRequest) error { @@ -255,7 +259,7 @@ func (i ImageIndexer) PruneFromIndex(request PruneFromIndexRequest) error { return err } - databasePath, err := i.ExtractDatabase(buildDir, request.FromIndex, request.CaFile, request.SkipTLS) + databasePath, err := i.ExtractDatabase(buildDir, request.FromIndex, request.CaFile, request.SkipTLSVerify, request.PlainHTTP) if err != nil { return err } @@ -294,14 +298,14 @@ func (i ImageIndexer) PruneFromIndex(request PruneFromIndexRequest) error { } // ExtractDatabase sets a temp directory for unpacking an image -func (i ImageIndexer) ExtractDatabase(buildDir, fromIndex, caFile string, skipTLS bool) (string, error) { +func (i ImageIndexer) ExtractDatabase(buildDir, fromIndex, caFile string, skipTLSVerify, plainHTTP bool) (string, error) { tmpDir, err := ioutil.TempDir("./", tmpDirPrefix) if err != nil { return "", err } defer os.RemoveAll(tmpDir) - databaseFile, err := i.getDatabaseFile(tmpDir, fromIndex, caFile, skipTLS) + databaseFile, err := i.getDatabaseFile(tmpDir, fromIndex, caFile, skipTLSVerify, plainHTTP) if err != nil { return "", err } @@ -309,7 +313,7 @@ func (i ImageIndexer) ExtractDatabase(buildDir, fromIndex, caFile string, skipTL return copyDatabaseTo(databaseFile, filepath.Join(buildDir, defaultDatabaseFolder)) } -func (i ImageIndexer) getDatabaseFile(workingDir, fromIndex, caFile string, skipTLS bool) (string, error) { +func (i ImageIndexer) getDatabaseFile(workingDir, fromIndex, caFile string, skipTLSVerify, plainHTTP bool) (string, error) { if fromIndex == "" { return path.Join(workingDir, defaultDatabaseFile), nil } @@ -325,11 +329,15 @@ func (i ImageIndexer) getDatabaseFile(workingDir, fromIndex, caFile string, skip if err != nil { return "", fmt.Errorf("failed to get RootCAs: %v", err) } - reg, rerr = containerdregistry.NewRegistry(containerdregistry.SkipTLS(skipTLS), containerdregistry.WithLog(i.Logger), containerdregistry.WithRootCAs(rootCAs)) + reg, rerr = containerdregistry.NewRegistry( + containerdregistry.SkipTLSVerify(skipTLSVerify), + containerdregistry.WithPlainHTTP(plainHTTP), + containerdregistry.WithLog(i.Logger), + containerdregistry.WithRootCAs(rootCAs)) case containertools.PodmanTool: fallthrough case containertools.DockerTool: - reg, rerr = execregistry.NewRegistry(i.PullTool, i.Logger, containertools.SkipTLS(skipTLS)) + reg, rerr = execregistry.NewRegistry(i.PullTool, i.Logger, containertools.SkipTLS(skipTLSVerify)) } if rerr != nil { return "", rerr @@ -481,7 +489,8 @@ type ExportFromIndexRequest struct { DownloadPath string ContainerTool containertools.ContainerTool CaFile string - SkipTLS bool + SkipTLSVerify bool + PlainHTTP bool } // ExportFromIndex is an aggregate API used to specify operators from @@ -495,7 +504,7 @@ func (i ImageIndexer) ExportFromIndex(request ExportFromIndexRequest) error { defer os.RemoveAll(workingDir) // extract the index database to the file - databaseFile, err := i.getDatabaseFile(workingDir, request.Index, request.CaFile, request.SkipTLS) + databaseFile, err := i.getDatabaseFile(workingDir, request.Index, request.CaFile, request.SkipTLSVerify, request.PlainHTTP) if err != nil { return err } @@ -549,7 +558,7 @@ func (i ImageIndexer) ExportFromIndex(request ExportFromIndexRequest) error { bundleDir.bundleVersion = strconv.Itoa(rand.Intn(10000)) } exporter := bundle.NewExporterForBundle(bundleImage, filepath.Join(request.DownloadPath, bundleDir.pkgName, bundleDir.bundleVersion), request.ContainerTool) - if err := exporter.Export(request.SkipTLS); err != nil { + if err := exporter.Export(request.SkipTLSVerify, request.PlainHTTP); err != nil { err = fmt.Errorf("exporting bundle image:%s failed with %s", bundleImage, err) mu.Lock() errs = append(errs, err) @@ -651,7 +660,8 @@ type DeprecateFromIndexRequest struct { Bundles []string Tag string CaFile string - SkipTLS bool + SkipTLSVerify bool + PlainHTTP bool AllowPackageRemoval bool } @@ -664,7 +674,7 @@ func (i ImageIndexer) DeprecateFromIndex(request DeprecateFromIndexRequest) erro return err } - databasePath, err := i.ExtractDatabase(buildDir, request.FromIndex, request.CaFile, request.SkipTLS) + databasePath, err := i.ExtractDatabase(buildDir, request.FromIndex, request.CaFile, request.SkipTLSVerify, request.PlainHTTP) if err != nil { return err } diff --git a/pkg/lib/registry/registry.go b/pkg/lib/registry/registry.go index 9ed73940e..de61c65cc 100644 --- a/pkg/lib/registry/registry.go +++ b/pkg/lib/registry/registry.go @@ -24,7 +24,8 @@ type RegistryUpdater struct { type AddToRegistryRequest struct { Permissive bool - SkipTLS bool + SkipTLSVerify bool + PlainHTTP bool CaFile string InputDatabase string Bundles []string @@ -66,11 +67,15 @@ func (r RegistryUpdater) AddToRegistry(request AddToRegistryRequest) error { if err != nil { return fmt.Errorf("failed to get RootCAs: %v", err) } - reg, rerr = containerdregistry.NewRegistry(containerdregistry.SkipTLS(request.SkipTLS), containerdregistry.WithRootCAs(rootCAs)) + reg, rerr = containerdregistry.NewRegistry( + containerdregistry.SkipTLSVerify(request.SkipTLSVerify), + containerdregistry.WithPlainHTTP(request.PlainHTTP), + containerdregistry.WithRootCAs(rootCAs), + ) case containertools.PodmanTool: fallthrough case containertools.DockerTool: - reg, rerr = execregistry.NewRegistry(request.ContainerTool, r.Logger, containertools.SkipTLS(request.SkipTLS)) + reg, rerr = execregistry.NewRegistry(request.ContainerTool, r.Logger, containertools.SkipTLS(request.SkipTLSVerify)) } if rerr != nil { return rerr diff --git a/test/e2e/e2e_suite_test.go b/test/e2e/e2e_suite_test.go index d8736d868..3f4852e85 100644 --- a/test/e2e/e2e_suite_test.go +++ b/test/e2e/e2e_suite_test.go @@ -27,7 +27,8 @@ var ( // opm command under test. opm *cobra.Command - skipTLSForRegistry = flag.Bool("skip-tls", false, "skip TLS certificate verification for container image registries while pulling bundles or index") + skipTLSForRegistry = flag.Bool("skip-tls-verify", false, "skip TLS certificate verification for container image registries while pulling bundles or index") + useHTTPforRegistry = flag.Bool("use-http", false, "use HTTP for container image registries while pulling bundles or index") ) func TestE2E(t *testing.T) { diff --git a/test/e2e/opm_test.go b/test/e2e/opm_test.go index f0598a0eb..600d16c55 100644 --- a/test/e2e/opm_test.go +++ b/test/e2e/opm_test.go @@ -108,7 +108,8 @@ func buildIndexWith(containerTool, fromIndexImage, toIndexImage string, bundleIm Bundles: bundleImages, Permissive: false, Overwrite: overwriteLatest, - SkipTLS: *skipTLSForRegistry, + SkipTLSVerify: *skipTLSForRegistry, + PlainHTTP: *useHTTPforRegistry, } return indexAdder.AddToIndex(request) @@ -169,7 +170,8 @@ func exportPackageWith(containerTool string) error { Packages: packages, DownloadPath: "downloaded", ContainerTool: containertools.NewContainerTool(containerTool, containertools.NoneTool), - SkipTLS: *skipTLSForRegistry, + SkipTLSVerify: *skipTLSForRegistry, + PlainHTTP: *useHTTPforRegistry, } return indexExporter.ExportFromIndex(request) @@ -185,7 +187,8 @@ func exportIndexImageWith(containerTool string) error { Packages: []string{}, DownloadPath: "downloaded", ContainerTool: containertools.NewContainerTool(containerTool, containertools.NoneTool), - SkipTLS: *skipTLSForRegistry, + SkipTLSVerify: *skipTLSForRegistry, + PlainHTTP: *useHTTPforRegistry, } return indexExporter.ExportFromIndex(request) @@ -426,7 +429,7 @@ var _ = Describe("opm", func() { PullTool: tool, Logger: logger, } - dbFile, err := imageIndexer.ExtractDatabase(".", publishedIndex, "", true) + dbFile, err := imageIndexer.ExtractDatabase(".", publishedIndex, "", *skipTLSForRegistry, *useHTTPforRegistry) Expect(err).NotTo(HaveOccurred(), "error extracting registry db") db, err := sql.Open("sqlite3", fmt.Sprintf("file:%s", dbFile)) @@ -460,7 +463,8 @@ var _ = Describe("opm", func() { request := lregistry.AddToRegistryRequest{ Permissive: false, - SkipTLS: *skipTLSForRegistry, + SkipTLSVerify: *skipTLSForRegistry, + PlainHTTP: *useHTTPforRegistry, InputDatabase: dbFile, Bundles: []string{ch.Head.BundlePath}, Mode: registry.ReplacesMode, From f59097176ad58970772dc85548fb106aa144a52c Mon Sep 17 00:00:00 2001 From: Jennifer Power Date: Wed, 19 Jan 2022 19:03:35 -0500 Subject: [PATCH 2/3] chore: add helper function to validate and get TLS options for opm command Signed-off-by: Jennifer Power --- cmd/opm/alpha/bundle/unpack.go | 22 +++-------------- cmd/opm/alpha/diff/cmd.go | 17 +++---------- cmd/opm/index/add.go | 19 ++------------- cmd/opm/index/delete.go | 19 ++------------- cmd/opm/index/deprecatetruncate.go | 19 ++------------- cmd/opm/index/export.go | 19 ++------------- cmd/opm/index/prune.go | 19 ++------------- cmd/opm/index/prunestranded.go | 19 ++------------- cmd/opm/internal/util/util.go | 39 ++++++++++++++++++++++++++++++ cmd/opm/registry/add.go | 28 ++++++--------------- pkg/lib/bundle/exporter.go | 6 ++--- pkg/lib/indexer/indexer.go | 2 +- pkg/lib/registry/registry.go | 2 +- 13 files changed, 71 insertions(+), 159 deletions(-) create mode 100644 cmd/opm/internal/util/util.go diff --git a/cmd/opm/alpha/bundle/unpack.go b/cmd/opm/alpha/bundle/unpack.go index ec8f648e3..59ccab379 100644 --- a/cmd/opm/alpha/bundle/unpack.go +++ b/cmd/opm/alpha/bundle/unpack.go @@ -12,6 +12,7 @@ import ( "github.com/sirupsen/logrus" "github.com/spf13/cobra" + "github.com/operator-framework/operator-registry/cmd/opm/internal/util" "github.com/operator-framework/operator-registry/pkg/image" "github.com/operator-framework/operator-registry/pkg/image/containerdregistry" "github.com/operator-framework/operator-registry/pkg/lib/bundle" @@ -75,28 +76,13 @@ func unpackBundle(cmd *cobra.Command, args []string) error { } var ( - registryOpts []containerdregistry.RegistryOption - useHTTP bool - skipTLSVerify bool - skipTLS bool + registryOpts []containerdregistry.RegistryOption ) - skipTLS, err = cmd.Flags().GetBool("skip-tls") - if err != nil { - return err - } - skipTLSVerify, err = cmd.Flags().GetBool("skip-tls-verify") - if err != nil { - return err - } - useHTTP, err = cmd.Flags().GetBool("use-http") + + skipTLSVerify, useHTTP, err := util.GetTLSOptions(cmd) if err != nil { return err } - if skipTLS { - // Set useHTTP when use deprecated skipTlS - // for functional parity with existing - useHTTP = true - } registryOpts = append(registryOpts, containerdregistry.SkipTLSVerify(skipTLSVerify), containerdregistry.WithPlainHTTP(useHTTP)) diff --git a/cmd/opm/alpha/diff/cmd.go b/cmd/opm/alpha/diff/cmd.go index b2c95462b..54ed5aa54 100644 --- a/cmd/opm/alpha/diff/cmd.go +++ b/cmd/opm/alpha/diff/cmd.go @@ -14,6 +14,7 @@ import ( "github.com/operator-framework/operator-registry/alpha/action" "github.com/operator-framework/operator-registry/alpha/declcfg" + "github.com/operator-framework/operator-registry/cmd/opm/internal/util" containerd "github.com/operator-framework/operator-registry/pkg/image/containerdregistry" "github.com/operator-framework/operator-registry/pkg/lib/certs" ) @@ -154,23 +155,11 @@ func (a *diff) addFunc(cmd *cobra.Command, args []string) error { return fmt.Errorf("invalid --output value: %q", a.output) } - skipTLS, err := cmd.Flags().GetBool("skip-tls") + skipTLSVerify, useHTTP, err := util.GetTLSOptions(cmd) if err != nil { return err } - skipTLSVerify, err := cmd.Flags().GetBool("skip-tls-verify") - if err != nil { - return err - } - useHTTP, err := cmd.Flags().GetBool("use-http") - if err != nil { - return err - } - if skipTLS { - // Set useHTTP when use deprecated skipTlS - // for functional parity with existing - useHTTP = true - } + rootCAs, err := certs.RootCAs(a.caFile) if err != nil { a.logger.Fatalf("error getting root CAs: %v", err) diff --git a/cmd/opm/index/add.go b/cmd/opm/index/add.go index ecf333955..0cb7c4801 100644 --- a/cmd/opm/index/add.go +++ b/cmd/opm/index/add.go @@ -7,6 +7,7 @@ import ( "github.com/spf13/cobra" "k8s.io/kubectl/pkg/util/templates" + "github.com/operator-framework/operator-registry/cmd/opm/internal/util" "github.com/operator-framework/operator-registry/pkg/containertools" "github.com/operator-framework/operator-registry/pkg/lib/indexer" "github.com/operator-framework/operator-registry/pkg/registry" @@ -126,27 +127,11 @@ func runIndexAddCmdFunc(cmd *cobra.Command, _ []string) error { return err } - skipTLS, err := cmd.Flags().GetBool("skip-tls") + skipTLSVerify, useHTTP, err := util.GetTLSOptions(cmd) if err != nil { return err } - skipTLSVerify, err := cmd.Flags().GetBool("skip-tls-verify") - if err != nil { - return err - } - - useHTTP, err := cmd.Flags().GetBool("use-http") - if err != nil { - return err - } - - if skipTLS { - // Set useHTTP when use deprecated skipTlS - // for functional parity with existing - useHTTP = true - } - mode, err := cmd.Flags().GetString("mode") if err != nil { return err diff --git a/cmd/opm/index/delete.go b/cmd/opm/index/delete.go index 28eddeec7..f7970676a 100644 --- a/cmd/opm/index/delete.go +++ b/cmd/opm/index/delete.go @@ -4,6 +4,7 @@ import ( "github.com/sirupsen/logrus" "github.com/spf13/cobra" + "github.com/operator-framework/operator-registry/cmd/opm/internal/util" "github.com/operator-framework/operator-registry/pkg/containertools" "github.com/operator-framework/operator-registry/pkg/lib/indexer" "github.com/operator-framework/operator-registry/pkg/sqlite" @@ -95,27 +96,11 @@ func runIndexDeleteCmdFunc(cmd *cobra.Command, _ []string) error { return err } - skipTLS, err := cmd.Flags().GetBool("skip-tls") + skipTLSVerify, useHTTP, err := util.GetTLSOptions(cmd) if err != nil { return err } - skipTLSVerify, err := cmd.Flags().GetBool("skip-tls-verify") - if err != nil { - return err - } - - useHTTP, err := cmd.Flags().GetBool("use-http") - if err != nil { - return err - } - - if skipTLS { - // Set useHTTP when use deprecated skipTlS - // for functional parity with existing - useHTTP = true - } - logger := logrus.WithFields(logrus.Fields{"operators": operators}) logger.Info("building the index") diff --git a/cmd/opm/index/deprecatetruncate.go b/cmd/opm/index/deprecatetruncate.go index fb769b241..33206e323 100644 --- a/cmd/opm/index/deprecatetruncate.go +++ b/cmd/opm/index/deprecatetruncate.go @@ -5,6 +5,7 @@ import ( "github.com/spf13/cobra" "k8s.io/kubectl/pkg/util/templates" + "github.com/operator-framework/operator-registry/cmd/opm/internal/util" "github.com/operator-framework/operator-registry/pkg/containertools" "github.com/operator-framework/operator-registry/pkg/lib/indexer" "github.com/operator-framework/operator-registry/pkg/sqlite" @@ -110,27 +111,11 @@ func runIndexDeprecateTruncateCmdFunc(cmd *cobra.Command, _ []string) error { return err } - skipTLS, err := cmd.Flags().GetBool("skip-tls") + skipTLSVerify, useHTTP, err := util.GetTLSOptions(cmd) if err != nil { return err } - skipTLSVerify, err := cmd.Flags().GetBool("skip-tls-verify") - if err != nil { - return err - } - - useHTTP, err := cmd.Flags().GetBool("use-http") - if err != nil { - return err - } - - if skipTLS { - // Set useHTTP when use deprecated skipTlS - // for functional parity with existing - useHTTP = true - } - allowPackageRemoval, err := cmd.Flags().GetBool("allow-package-removal") if err != nil { return err diff --git a/cmd/opm/index/export.go b/cmd/opm/index/export.go index 522edc518..2541cbba6 100644 --- a/cmd/opm/index/export.go +++ b/cmd/opm/index/export.go @@ -7,6 +7,7 @@ import ( "github.com/spf13/cobra" "k8s.io/kubectl/pkg/util/templates" + "github.com/operator-framework/operator-registry/cmd/opm/internal/util" "github.com/operator-framework/operator-registry/pkg/containertools" "github.com/operator-framework/operator-registry/pkg/lib/indexer" "github.com/operator-framework/operator-registry/pkg/sqlite" @@ -100,27 +101,11 @@ func runIndexExportCmdFunc(cmd *cobra.Command, _ []string) error { return err } - skipTLS, err := cmd.Flags().GetBool("skip-tls") + skipTLSVerify, useHTTP, err := util.GetTLSOptions(cmd) if err != nil { return err } - skipTLSVerify, err := cmd.Flags().GetBool("skip-tls-verify") - if err != nil { - return err - } - - useHTTP, err := cmd.Flags().GetBool("use-http") - if err != nil { - return err - } - - if skipTLS { - // Set useHTTP when use deprecated skipTlS - // for functional parity with existing - useHTTP = true - } - logger := logrus.WithFields(logrus.Fields{"index": index, "package": packages}) logger.Info("export from the index") diff --git a/cmd/opm/index/prune.go b/cmd/opm/index/prune.go index acfc581c0..ec57174bb 100644 --- a/cmd/opm/index/prune.go +++ b/cmd/opm/index/prune.go @@ -6,6 +6,7 @@ import ( "github.com/sirupsen/logrus" "github.com/spf13/cobra" + "github.com/operator-framework/operator-registry/cmd/opm/internal/util" "github.com/operator-framework/operator-registry/pkg/containertools" "github.com/operator-framework/operator-registry/pkg/lib/indexer" "github.com/operator-framework/operator-registry/pkg/sqlite" @@ -99,27 +100,11 @@ func runIndexPruneCmdFunc(cmd *cobra.Command, _ []string) error { return err } - skipTLS, err := cmd.Flags().GetBool("skip-tls") + skipTLSVerify, useHTTP, err := util.GetTLSOptions(cmd) if err != nil { return err } - skipTLSVerify, err := cmd.Flags().GetBool("skip-tls-verify") - if err != nil { - return err - } - - useHTTP, err := cmd.Flags().GetBool("use-http") - if err != nil { - return err - } - - if skipTLS { - // Set useHTTP when use deprecated skipTlS - // for functional parity with existing - useHTTP = true - } - logger := logrus.WithFields(logrus.Fields{"packages": packages}) logger.Info("pruning the index") diff --git a/cmd/opm/index/prunestranded.go b/cmd/opm/index/prunestranded.go index ed6005e1f..4ba306919 100644 --- a/cmd/opm/index/prunestranded.go +++ b/cmd/opm/index/prunestranded.go @@ -6,6 +6,7 @@ import ( "github.com/sirupsen/logrus" "github.com/spf13/cobra" + "github.com/operator-framework/operator-registry/cmd/opm/internal/util" "github.com/operator-framework/operator-registry/pkg/containertools" "github.com/operator-framework/operator-registry/pkg/lib/indexer" "github.com/operator-framework/operator-registry/pkg/sqlite" @@ -84,27 +85,11 @@ func runIndexPruneStrandedCmdFunc(cmd *cobra.Command, _ []string) error { return err } - skipTLS, err := cmd.Flags().GetBool("skip-tls") + skipTLSVerify, useHTTP, err := util.GetTLSOptions(cmd) if err != nil { return err } - skipTLSVerify, err := cmd.Flags().GetBool("skip-tls-verify") - if err != nil { - return err - } - - useHTTP, err := cmd.Flags().GetBool("use-http") - if err != nil { - return err - } - - if skipTLS { - // Set useHTTP when use deprecated skipTlS - // for functional parity with existing - useHTTP = true - } - logger := logrus.WithFields(logrus.Fields{}) logger.Info("pruning stranded bundles from the index") diff --git a/cmd/opm/internal/util/util.go b/cmd/opm/internal/util/util.go new file mode 100644 index 000000000..fcb72f0c9 --- /dev/null +++ b/cmd/opm/internal/util/util.go @@ -0,0 +1,39 @@ +package util + +import ( + "fmt" + + "github.com/spf13/cobra" +) + +// GetTLSOptions validates and returns TLS options set by opm flags +func GetTLSOptions(cmd *cobra.Command) (bool, bool, error) { + skipTLS, err := cmd.Flags().GetBool("skip-tls") + if err != nil { + return false, false, err + } + skipTLSVerify, err := cmd.Flags().GetBool("skip-tls-verify") + if err != nil { + return false, false, err + } + useHTTP, err := cmd.Flags().GetBool("use-http") + if err != nil { + return false, false, err + } + + switch { + case cmd.Flags().Changed("skip-tls") && cmd.Flags().Changed("use-http"): + return false, false, fmt.Errorf("invalid flag combination: cannot use --use-http with --skip-tls") + case cmd.Flags().Changed("skip-tls") && cmd.Flags().Changed("skip-tls-verify"): + return false, false, fmt.Errorf("invalid flag combination: cannot use --skip-tls-verify with --skip-tls") + case skipTLSVerify && useHTTP: + return false, false, fmt.Errorf("invalid flag combination: --use-http and --skip-tls-verify cannot both be true") + default: + // return use HTTP true if just skipTLS + // is set for functional parity with existing + if skipTLS { + return false, true, nil + } + return skipTLSVerify, useHTTP, nil + } +} diff --git a/cmd/opm/registry/add.go b/cmd/opm/registry/add.go index e7b438394..4ef1895d7 100644 --- a/cmd/opm/registry/add.go +++ b/cmd/opm/registry/add.go @@ -7,6 +7,7 @@ import ( "github.com/sirupsen/logrus" "github.com/spf13/cobra" + "github.com/operator-framework/operator-registry/cmd/opm/internal/util" "github.com/operator-framework/operator-registry/pkg/containertools" "github.com/operator-framework/operator-registry/pkg/lib/registry" reg "github.com/operator-framework/operator-registry/pkg/registry" @@ -61,18 +62,6 @@ func addFunc(cmd *cobra.Command, _ []string) error { if err != nil { return err } - skipTLS, err := cmd.Flags().GetBool("skip-tls") - if err != nil { - return err - } - skipTLSVerify, err := cmd.Flags().GetBool("skip-tls-verify") - if err != nil { - return err - } - useHTTP, err := cmd.Flags().GetBool("use-http") - if err != nil { - return err - } caFile, err := cmd.Flags().GetString("ca-file") if err != nil { return err @@ -108,6 +97,11 @@ func addFunc(cmd *cobra.Command, _ []string) error { return err } + skipTLSVerify, useHTTP, err := util.GetTLSOptions(cmd) + if err != nil { + return err + } + if caFile != "" { if skipTLSVerify { return errors.New("--skip-tls-verify must be false when --ca-file is set") @@ -118,12 +112,6 @@ func addFunc(cmd *cobra.Command, _ []string) error { } } - if skipTLS { - // Set useHTTP when use - // deprecated skipTlS for functional parity with existing - useHTTP = true - } - request := registry.AddToRegistryRequest{ Permissive: permissive, SkipTLSVerify: skipTLSVerify, @@ -139,8 +127,8 @@ func addFunc(cmd *cobra.Command, _ []string) error { logger := logrus.WithFields(logrus.Fields{"bundles": bundleImages}) - if skipTLS { - logger.Warn("--skip-tls flag is set: this mode is insecure and meant for development purposes only.") + if skipTLSVerify { + logger.Warn("--skip-tls-verify flag is set: this mode is insecure and meant for development purposes only.") } logger.Info("adding to the registry") diff --git a/pkg/lib/bundle/exporter.go b/pkg/lib/bundle/exporter.go index 5a25156a2..be9f36312 100644 --- a/pkg/lib/bundle/exporter.go +++ b/pkg/lib/bundle/exporter.go @@ -30,7 +30,7 @@ func NewExporterForBundle(image, directory string, containerTool containertools. } } -func (i *BundleExporter) Export(skipTLSVerify, useHTTP bool) error { +func (i *BundleExporter) Export(skipTLSVerify, plainHTTP bool) error { log := logrus.WithField("img", i.image) @@ -46,14 +46,14 @@ func (i *BundleExporter) Export(skipTLSVerify, useHTTP bool) error { case containertools.NoneTool: reg, rerr = containerdregistry.NewRegistry( containerdregistry.SkipTLSVerify(skipTLSVerify), - containerdregistry.WithPlainHTTP(useHTTP), + containerdregistry.WithPlainHTTP(plainHTTP), containerdregistry.WithLog(log), containerdregistry.WithCacheDir(filepath.Join(tmpDir, "cacheDir")), ) case containertools.PodmanTool: fallthrough case containertools.DockerTool: - reg, rerr = execregistry.NewRegistry(i.containerTool, log, containertools.SkipTLS(skipTLSVerify)) + reg, rerr = execregistry.NewRegistry(i.containerTool, log, containertools.SkipTLS(plainHTTP)) } if rerr != nil { return rerr diff --git a/pkg/lib/indexer/indexer.go b/pkg/lib/indexer/indexer.go index fb14341c9..284c8150e 100644 --- a/pkg/lib/indexer/indexer.go +++ b/pkg/lib/indexer/indexer.go @@ -337,7 +337,7 @@ func (i ImageIndexer) getDatabaseFile(workingDir, fromIndex, caFile string, skip case containertools.PodmanTool: fallthrough case containertools.DockerTool: - reg, rerr = execregistry.NewRegistry(i.PullTool, i.Logger, containertools.SkipTLS(skipTLSVerify)) + reg, rerr = execregistry.NewRegistry(i.PullTool, i.Logger, containertools.SkipTLS(plainHTTP)) } if rerr != nil { return "", rerr diff --git a/pkg/lib/registry/registry.go b/pkg/lib/registry/registry.go index de61c65cc..5f7319630 100644 --- a/pkg/lib/registry/registry.go +++ b/pkg/lib/registry/registry.go @@ -75,7 +75,7 @@ func (r RegistryUpdater) AddToRegistry(request AddToRegistryRequest) error { case containertools.PodmanTool: fallthrough case containertools.DockerTool: - reg, rerr = execregistry.NewRegistry(request.ContainerTool, r.Logger, containertools.SkipTLS(request.SkipTLSVerify)) + reg, rerr = execregistry.NewRegistry(request.ContainerTool, r.Logger, containertools.SkipTLS(request.PlainHTTP)) } if rerr != nil { return rerr From 14d600d45f56e6fb6748a63fe9de83eb69a7924d Mon Sep 17 00:00:00 2001 From: Jennifer Power Date: Wed, 19 Jan 2022 19:17:00 -0500 Subject: [PATCH 3/3] chore: add warning message for plain HTTP in registy add command Signed-off-by: Jennifer Power --- cmd/opm/internal/util/util.go | 8 ++++---- cmd/opm/registry/add.go | 4 ++++ pkg/lib/indexer/indexer.go | 1 + 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/cmd/opm/internal/util/util.go b/cmd/opm/internal/util/util.go index fcb72f0c9..7d83cf66a 100644 --- a/cmd/opm/internal/util/util.go +++ b/cmd/opm/internal/util/util.go @@ -1,7 +1,7 @@ package util import ( - "fmt" + "errors" "github.com/spf13/cobra" ) @@ -23,11 +23,11 @@ func GetTLSOptions(cmd *cobra.Command) (bool, bool, error) { switch { case cmd.Flags().Changed("skip-tls") && cmd.Flags().Changed("use-http"): - return false, false, fmt.Errorf("invalid flag combination: cannot use --use-http with --skip-tls") + return false, false, errors.New("invalid flag combination: cannot use --use-http with --skip-tls") case cmd.Flags().Changed("skip-tls") && cmd.Flags().Changed("skip-tls-verify"): - return false, false, fmt.Errorf("invalid flag combination: cannot use --skip-tls-verify with --skip-tls") + return false, false, errors.New("invalid flag combination: cannot use --skip-tls-verify with --skip-tls") case skipTLSVerify && useHTTP: - return false, false, fmt.Errorf("invalid flag combination: --use-http and --skip-tls-verify cannot both be true") + return false, false, errors.New("invalid flag combination: --use-http and --skip-tls-verify cannot both be true") default: // return use HTTP true if just skipTLS // is set for functional parity with existing diff --git a/cmd/opm/registry/add.go b/cmd/opm/registry/add.go index 4ef1895d7..dd7237e37 100644 --- a/cmd/opm/registry/add.go +++ b/cmd/opm/registry/add.go @@ -131,6 +131,10 @@ func addFunc(cmd *cobra.Command, _ []string) error { logger.Warn("--skip-tls-verify flag is set: this mode is insecure and meant for development purposes only.") } + if useHTTP { + logger.Warn("--use-http flag is set: this mode is insecure and meant for development purposes only.") + } + logger.Info("adding to the registry") registryAdder := registry.NewRegistryAdder(logger) diff --git a/pkg/lib/indexer/indexer.go b/pkg/lib/indexer/indexer.go index 284c8150e..09b7340fa 100644 --- a/pkg/lib/indexer/indexer.go +++ b/pkg/lib/indexer/indexer.go @@ -89,6 +89,7 @@ func (i ImageIndexer) AddToIndex(request AddToIndexRequest) error { Permissive: request.Permissive, Mode: request.Mode, SkipTLSVerify: request.SkipTLSVerify, + PlainHTTP: request.PlainHTTP, ContainerTool: i.PullTool, Overwrite: request.Overwrite, EnableAlpha: request.EnableAlpha,