diff --git a/cmd/opm/index/add.go b/cmd/opm/index/add.go index f6c8caad9..a4e4fed87 100644 --- a/cmd/opm/index/add.go +++ b/cmd/opm/index/add.go @@ -55,7 +55,9 @@ func addIndexAddCmd(parent *cobra.Command) { } indexCmd.Flags().Bool("skip-tls", false, "skip TLS certificate verification for container image registries while pulling bundles") 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("container-tool", "c", "", "tool to interact with container images (save, build, etc.). One of: [docker, podman]") + indexCmd.Flags().StringP("build-tool", "u", "", "tool to build container images. One of: [docker, podman]. Defaults to podman. Overrides part of container-tool.") + indexCmd.Flags().StringP("pull-tool", "p", "", "tool to pull container images. One of: [none, docker, podman]. Defaults to none. Overrides part of container-tool.") indexCmd.Flags().StringP("tag", "t", "", "custom tag for container image being built") indexCmd.Flags().Bool("permissive", false, "allow registry load errors") indexCmd.Flags().StringP("mode", "", "replaces", "graph update mode that defines how channel graphs are updated. One of: [replaces, semver, semver-skippatch]") @@ -96,15 +98,6 @@ func runIndexAddCmdFunc(cmd *cobra.Command, args []string) error { return err } - containerTool, err := cmd.Flags().GetString("container-tool") - if err != nil { - return err - } - - if containerTool == "none" { - return fmt.Errorf("none is not a valid container-tool for index add") - } - tag, err := cmd.Flags().GetString("tag") if err != nil { return err @@ -130,11 +123,19 @@ func runIndexAddCmdFunc(cmd *cobra.Command, args []string) error { return err } + pullTool, buildTool, err := getContainerTools(cmd) + if err != nil { + return err + } + logger := logrus.WithFields(logrus.Fields{"bundles": bundles}) logger.Info("building the index") - indexAdder := indexer.NewIndexAdder(containertools.NewContainerTool(containerTool, containertools.PodmanTool), logger) + indexAdder := indexer.NewIndexAdder( + containertools.NewContainerTool(buildTool, containertools.PodmanTool), + containertools.NewContainerTool(pullTool, containertools.NoneTool), + logger) request := indexer.AddToIndexRequest{ Generate: generate, @@ -155,3 +156,46 @@ func runIndexAddCmdFunc(cmd *cobra.Command, args []string) error { return nil } + +// getContainerTools returns the pull and build tools based on command line input +// to preserve backwards compatibility and alias the legacy `container-tool` parameter +func getContainerTools(cmd *cobra.Command) (string, string, error) { + buildTool, err := cmd.Flags().GetString("build-tool") + if err != nil { + return "", "", err + } + + if buildTool == "none" { + return "", "", fmt.Errorf("none is not a valid container-tool for index add") + } + + pullTool, err := cmd.Flags().GetString("pull-tool") + if err != nil { + return "", "", err + } + + containerTool, err := cmd.Flags().GetString("container-tool") + if err != nil { + return "", "", err + } + + // Backwards compatiblity mode + if containerTool != "" { + if pullTool == "" && buildTool == "" { + return containerTool, containerTool, nil + } else { + return "", "", fmt.Errorf("container-tool cannot be set alongside pull-tool or build-tool") + } + } + + // Check for defaults, then return + if pullTool == "" { + pullTool = "none" + } + + if buildTool == "" { + buildTool = "podman" + } + + return pullTool, buildTool, nil +} diff --git a/pkg/lib/indexer/indexer.go b/pkg/lib/indexer/indexer.go index c7862a0af..98735fa27 100644 --- a/pkg/lib/indexer/indexer.go +++ b/pkg/lib/indexer/indexer.go @@ -42,7 +42,8 @@ type ImageIndexer struct { RegistryAdder registry.RegistryAdder RegistryDeleter registry.RegistryDeleter RegistryPruner registry.RegistryPruner - ContainerTool containertools.ContainerTool + BuildTool containertools.ContainerTool + PullTool containertools.ContainerTool Logger *logrus.Entry } @@ -71,7 +72,7 @@ func (i ImageIndexer) AddToIndex(request AddToIndexRequest) error { // this is in its own function context so that the deferred cleanup runs before we do a docker build // which prevents the full contents of the previous image from being in the build context var databasePath string - if err := func () error { + if err := func() error { tmpDir, err := ioutil.TempDir("./", tmpDirPrefix) if err != nil { @@ -99,7 +100,7 @@ func (i ImageIndexer) AddToIndex(request AddToIndexRequest) error { Permissive: request.Permissive, Mode: request.Mode, SkipTLS: request.SkipTLS, - ContainerTool: i.ContainerTool, + ContainerTool: i.PullTool, } // Add the bundles to the registry @@ -152,7 +153,7 @@ func (i ImageIndexer) DeleteFromIndex(request DeleteFromIndexRequest) error { // this is in its own function context so that the deferred cleanup runs before we do a docker build // which prevents the full contents of the previous image from being in the build context var databasePath string - if err := func () error { + if err := func() error { tmpDir, err := ioutil.TempDir("./", tmpDirPrefix) if err != nil { @@ -228,7 +229,7 @@ func (i ImageIndexer) PruneFromIndex(request PruneFromIndexRequest) error { // this is in its own function context so that the deferred cleanup runs before we do a docker build // which prevents the full contents of the previous image from being in the build context var databasePath string - if err := func () error { + if err := func() error { tmpDir, err := ioutil.TempDir("./", tmpDirPrefix) if err != nil { @@ -292,13 +293,13 @@ func (i ImageIndexer) getDatabaseFile(workingDir, fromIndex string) (string, err var reg image.Registry var rerr error - switch i.ContainerTool { + switch i.PullTool { case containertools.NoneTool: reg, rerr = containerdregistry.NewRegistry(containerdregistry.WithLog(i.Logger)) case containertools.PodmanTool: fallthrough case containertools.DockerTool: - reg, rerr = execregistry.NewRegistry(i.ContainerTool, i.Logger) + reg, rerr = execregistry.NewRegistry(i.PullTool, i.Logger) } if rerr != nil { return "", rerr @@ -339,7 +340,7 @@ func copyDatabaseTo(databaseFile, targetDir string) (string, error) { if err := os.MkdirAll(targetDir, 0777); err != nil { return "", err } - } else { + } else if err != nil { return "", err } @@ -353,7 +354,7 @@ func copyDatabaseTo(databaseFile, targetDir string) (string, error) { dbFile := path.Join(targetDir, defaultDatabaseFile) // define the path to copy to the database/index.db file - to, err := os.OpenFile(dbFile, os.O_RDWR|os.O_CREATE, 0666) + to, err := os.OpenFile(dbFile, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0666) if err != nil { return "", err } diff --git a/pkg/lib/indexer/interfaces.go b/pkg/lib/indexer/interfaces.go index 4918eec26..a9017ecc0 100644 --- a/pkg/lib/indexer/interfaces.go +++ b/pkg/lib/indexer/interfaces.go @@ -15,14 +15,15 @@ type IndexAdder interface { } // NewIndexAdder is a constructor that returns an IndexAdder -func NewIndexAdder(containerTool containertools.ContainerTool, logger *logrus.Entry) IndexAdder { +func NewIndexAdder(buildTool, pullTool containertools.ContainerTool, logger *logrus.Entry) IndexAdder { return ImageIndexer{ DockerfileGenerator: containertools.NewDockerfileGenerator(logger), - CommandRunner: containertools.NewCommandRunner(containerTool, logger), - LabelReader: containertools.NewLabelReader(containerTool, logger), + CommandRunner: containertools.NewCommandRunner(buildTool, logger), + LabelReader: containertools.NewLabelReader(pullTool, logger), RegistryAdder: registry.NewRegistryAdder(logger), - ImageReader: containertools.NewImageReader(containerTool, logger), - ContainerTool: containerTool, + ImageReader: containertools.NewImageReader(pullTool, logger), + BuildTool: buildTool, + PullTool: pullTool, Logger: logger, } } @@ -42,7 +43,8 @@ func NewIndexDeleter(containerTool containertools.ContainerTool, logger *logrus. LabelReader: containertools.NewLabelReader(containerTool, logger), RegistryDeleter: registry.NewRegistryDeleter(logger), ImageReader: containertools.NewImageReader(containerTool, logger), - ContainerTool: containerTool, + BuildTool: containerTool, + PullTool: containerTool, Logger: logger, } } @@ -59,7 +61,8 @@ func NewIndexExporter(containerTool containertools.ContainerTool, logger *logrus CommandRunner: containertools.NewCommandRunner(containerTool, logger), LabelReader: containertools.NewLabelReader(containerTool, logger), ImageReader: containertools.NewImageReader(containerTool, logger), - ContainerTool: containerTool, + BuildTool: containerTool, + PullTool: containerTool, Logger: logger, } } @@ -76,7 +79,8 @@ func NewIndexPruner(containerTool containertools.ContainerTool, logger *logrus.E LabelReader: containertools.NewLabelReader(containerTool, logger), RegistryPruner: registry.NewRegistryPruner(logger), ImageReader: containertools.NewImageReader(containerTool, logger), - ContainerTool: containerTool, + BuildTool: containerTool, + PullTool: containerTool, Logger: logger, } } diff --git a/test/e2e/opm_test.go b/test/e2e/opm_test.go index 729660913..453c5dde8 100644 --- a/test/e2e/opm_test.go +++ b/test/e2e/opm_test.go @@ -89,7 +89,7 @@ func buildIndexWith(containerTool string) error { bundleImage + ":" + bundleTag2, } logger := logrus.WithFields(logrus.Fields{"bundles": bundles}) - indexAdder := indexer.NewIndexAdder(containertools.NewContainerTool(containerTool, containertools.NoneTool), logger) + indexAdder := indexer.NewIndexAdder(containertools.NewContainerTool(containerTool, containertools.NoneTool), containertools.NewContainerTool(containerTool, containertools.NoneTool), logger) request := indexer.AddToIndexRequest{ Generate: false, @@ -109,7 +109,7 @@ func buildFromIndexWith(containerTool string) error { bundleImage + ":" + bundleTag3, } logger := logrus.WithFields(logrus.Fields{"bundles": bundles}) - indexAdder := indexer.NewIndexAdder(containertools.NewContainerTool(containerTool, containertools.NoneTool), logger) + indexAdder := indexer.NewIndexAdder(containertools.NewContainerTool(containerTool, containertools.NoneTool), containertools.NewContainerTool(containerTool, containertools.NoneTool), logger) request := indexer.AddToIndexRequest{ Generate: false,