run bundle{-upgrade}: validate --mode before running a bundle

changelog/fragments/bugfix-validate-add-mode.yaml

@@ -0,0 +1,5 @@
+entries:
+  - description: >
+      `run bundle` and `run bundle-upgrade` now validate the value passed to the hidden flag `--mode` before
+      running a bundle.
+    kind: bugfix

internal/olm/operator/bundle/install.go

@@ -49,7 +49,9 @@ func NewInstall(cfg *operator.Configuration) Install {
 func (i *Install) BindFlags(fs *pflag.FlagSet) {
 	fs.StringVar(&i.IndexImage, "index-image", index.DefaultIndexImage, "index image in which to inject bundle")
 	fs.Var(&i.InstallMode, "install-mode", "install mode")
-	fs.StringVar(&i.BundleAddMode, "mode", "", "mode to use for adding bundle to index")
+
+	// --mode is hidden so only users who know what they're doing can alter add mode.
+	fs.StringVar((*string)(&i.BundleAddMode), "mode", "", "mode to use for adding bundle to index")
 	_ = fs.MarkHidden("mode")
 }
 
@@ -61,6 +63,17 @@ func (i Install) Run(ctx context.Context) (*v1alpha1.ClusterServiceVersion, erro
 }
 
 func (i *Install) setup(ctx context.Context) error {
+	// Default bundle add mode then validate in case it was set by a user.
+	if i.BundleAddMode == "" {
+		i.BundleAddMode = index.ReplacesBundleAddMode
+		if i.IndexImage == index.DefaultIndexImage {
+			i.BundleAddMode = index.SemverBundleAddMode
+		}
+	} else if err := i.BundleAddMode.Validate(); err != nil {
+		return err
+	}
+
+	// Load bundle labels and set label-dependent values.
 	labels, bundle, err := operator.LoadBundle(ctx, i.BundleImage)
 	if err != nil {
 		return err
@@ -79,12 +92,6 @@ func (i *Install) setup(ctx context.Context) error {
 
 	i.IndexImageCatalogCreator.PackageName = i.OperatorInstaller.PackageName
 	i.IndexImageCatalogCreator.BundleImage = i.BundleImage
-	if i.IndexImageCatalogCreator.BundleAddMode == "" {
-		i.IndexImageCatalogCreator.BundleAddMode = index.ReplacesBundleAddMode
-		if i.IndexImageCatalogCreator.IndexImage == index.DefaultIndexImage {
-			i.IndexImageCatalogCreator.BundleAddMode = index.SemverBundleAddMode
-		}
-	}
 
 	return nil
 }

internal/olm/operator/bundleupgrade/upgrade.go

@@ -47,7 +47,8 @@ func NewUpgrade(cfg *operator.Configuration) Upgrade {
 }
 
 func (u *Upgrade) BindFlags(fs *pflag.FlagSet) {
-	fs.StringVar(&u.BundleAddMode, "mode", "", "mode to use for adding new bundle version to index")
+	// --mode is hidden so only users who know what they're doing can alter add mode.
+	fs.StringVar((*string)(&u.BundleAddMode), "mode", "", "mode to use for adding new bundle version to index")
 	_ = fs.MarkHidden("mode")
 }
 
@@ -59,6 +60,14 @@ func (u Upgrade) Run(ctx context.Context) (*v1alpha1.ClusterServiceVersion, erro
 }
 
 func (u *Upgrade) setup(ctx context.Context) error {
+	// Bundle add mode is defaulted based on in-cluster metadata in u.UpgradeOperator(),
+	// so validate only if it was set by a user.
+	if u.BundleAddMode != "" {
+		if err := u.BundleAddMode.Validate(); err != nil {
+			return err
+		}
+	}
+
 	labels, bundle, err := operator.LoadBundle(ctx, u.BundleImage)
 	if err != nil {
 		return err

internal/olm/operator/registry/index/bundle_add_mode.go

@@ -0,0 +1,41 @@
+// Copyright 2021 The Operator-SDK Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package index
+
+import "fmt"
+
+// BundleAddMode is the mode to add a bundle to an index.
+type BundleAddMode string
+
+const (
+	// SemverBundleAddMode - bundle add mode for semver
+	SemverBundleAddMode BundleAddMode = "semver"
+	// ReplacesBundleAddMode - bundle add mode for replaces
+	ReplacesBundleAddMode BundleAddMode = "replaces"
+)
+
+var modes = []BundleAddMode{SemverBundleAddMode, ReplacesBundleAddMode}
+
+func (m BundleAddMode) Validate() error {
+	switch m {
+	case SemverBundleAddMode, ReplacesBundleAddMode:
+	case "":
+		return fmt.Errorf("bundle add mode cannot be empty, must be one of: %+q", modes)
+	default:
+		return fmt.Errorf("bundle add mode %q does not exist, must be one of: %+q", m, modes)
+	}
+
+	return nil
+}

internal/olm/operator/registry/index/registry_pod.go

@@ -35,15 +35,6 @@ import (
 	"github.com/operator-framework/operator-sdk/internal/util/k8sutil"
 )
 
-// BundleAddMode - type of BundleAddMode in RegistryPod struct
-type BundleAddMode = string
-
-const (
-	// SemverBundleAddMode - bundle add mode for semver
-	SemverBundleAddMode BundleAddMode = "semver"
-	// ReplacesBundleAddMode - bundle add mode for replaces
-	ReplacesBundleAddMode BundleAddMode = "replaces"
-)
 const (
 	// DefaultIndexImage is the index base image used if none is specified. It contains no bundles.
 	DefaultIndexImage = "quay.io/operator-framework/upstream-opm-builder:latest"
@@ -175,12 +166,8 @@ func (rp *RegistryPod) validate() error {
 		if item.ImageTag == "" {
 			return errors.New("bundle image cannot be empty")
 		}
-		if item.AddMode == "" {
-			return fmt.Errorf("bundle add mode for %q cannot be empty", item.ImageTag)
-		}
-		if item.AddMode != SemverBundleAddMode && item.AddMode != ReplacesBundleAddMode {
-			return fmt.Errorf("invalid bundle mode %q: must be one of [%q, %q]",
-				item.AddMode, ReplacesBundleAddMode, SemverBundleAddMode)
+		if err := item.AddMode.Validate(); err != nil {
+			return fmt.Errorf("invalid bundle add mode: %v", err)
 		}
 	}
 

internal/olm/operator/registry/index/registry_pod_test.go

@@ -138,7 +138,7 @@ var _ = Describe("RegistryPod", func() {
 			})
 
 			It("should not accept any other bundle add mode other than semver or replaces", func() {
-				expectedErr := "invalid bundle mode"
+				expectedErr := `bundle add mode "invalid" does not exist`
 				rp := &RegistryPod{
 					BundleItems: []BundleItem{{ImageTag: "quay.io/example/example-operator-bundle:0.2.0", AddMode: "invalid"}},
 				}