From 7041102e601b8397b2d72a80936c3f9e15fed195 Mon Sep 17 00:00:00 2001 From: Eric Stroczynski Date: Mon, 1 Feb 2021 17:59:32 -0800 Subject: [PATCH] internal/olm/operator: validate BundleAddMode before running a bundle image Signed-off-by: Eric Stroczynski --- .../fragments/bugfix-validate-add-mode.yaml | 5 +++ internal/olm/operator/bundle/install.go | 21 ++++++---- .../olm/operator/bundleupgrade/upgrade.go | 11 ++++- .../registry/index/bundle_add_mode.go | 41 +++++++++++++++++++ .../operator/registry/index/registry_pod.go | 17 +------- .../registry/index/registry_pod_test.go | 2 +- 6 files changed, 73 insertions(+), 24 deletions(-) create mode 100644 changelog/fragments/bugfix-validate-add-mode.yaml create mode 100644 internal/olm/operator/registry/index/bundle_add_mode.go diff --git a/changelog/fragments/bugfix-validate-add-mode.yaml b/changelog/fragments/bugfix-validate-add-mode.yaml new file mode 100644 index 0000000000..dc5578c3bf --- /dev/null +++ b/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 diff --git a/internal/olm/operator/bundle/install.go b/internal/olm/operator/bundle/install.go index fa293c25b5..daeb9d0977 100644 --- a/internal/olm/operator/bundle/install.go +++ b/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 } diff --git a/internal/olm/operator/bundleupgrade/upgrade.go b/internal/olm/operator/bundleupgrade/upgrade.go index e0b8e3ed8e..516c4a49ab 100644 --- a/internal/olm/operator/bundleupgrade/upgrade.go +++ b/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 diff --git a/internal/olm/operator/registry/index/bundle_add_mode.go b/internal/olm/operator/registry/index/bundle_add_mode.go new file mode 100644 index 0000000000..cafca84531 --- /dev/null +++ b/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 +} diff --git a/internal/olm/operator/registry/index/registry_pod.go b/internal/olm/operator/registry/index/registry_pod.go index 33f686455e..cc2d94d7a3 100644 --- a/internal/olm/operator/registry/index/registry_pod.go +++ b/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) } } diff --git a/internal/olm/operator/registry/index/registry_pod_test.go b/internal/olm/operator/registry/index/registry_pod_test.go index c306a34d38..316bffb5d0 100644 --- a/internal/olm/operator/registry/index/registry_pod_test.go +++ b/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"}}, }