<run|cleanup> packagemanifests: remove --include-paths, change --operator-version to --version

changelog/fragments/run-packagemanifests-flags-1.yaml

@@ -0,0 +1,16 @@
+entries:
+  - description: Removed the `--include-paths` flag from `run packagemanifests`.
+    kind: removal
+    breaking: true
+    migration:
+      header: Create resources manually that were passed to `run packagemanifests --include-paths`
+      body: >
+        The `run packagemanifests` subcommand no longer has the `--include-paths` flag to create
+        additional resources. Instead, use `kubectl apply -f <paths>` before invoking `run packagemanifests`.
+  - description: Changed the `--operator-version` flag to `--version` in `run packagemanifests`.
+    kind: change
+    breaking: true
+    migration:
+      header: Change the `run packagemanifests` flag `--operator-version` to `--version`
+      body: >
+        `--operator-version` is now `--version`.

internal/olm/operator/operator_manager.go

@@ -15,10 +15,8 @@
 package olm
 
 import (
-	"bytes"
 	"context"
 	"fmt"
-	"io/ioutil"
 	"log"
 	"sync"
 	"time"
@@ -30,7 +28,6 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/kubernetes/scheme"
-	"sigs.k8s.io/yaml"
 
 	"github.com/operator-framework/operator-sdk/internal/olm"
 	internalolmclient "github.com/operator-framework/operator-sdk/internal/olm/client"
@@ -64,8 +61,7 @@ type OperatorCmd struct {
 	KubeconfigPath string
 	// OperatorNamespace is the cluster namespace in which operator resources
 	// are created.
-	// OperatorNamespace must already exist in the cluster or be defined in
-	// a manifest passed to IncludePaths.
+	// OperatorNamespace must already exist in the cluster.
 	OperatorNamespace string
 	// OLMNamespace is the namespace in which OLM is installed.
 	OLMNamespace string
@@ -75,9 +71,8 @@ type OperatorCmd struct {
 	// "InstallModeType=[ns1,ns2[, ...]]"
 	//
 	// The InstallModeType string passed must be marked as "supported" in the
-	// CSV being installed. The namespaces passed must exist or be created by
-	// passing a Namespace manifest to IncludePaths. An empty set of namespaces
-	// can be used for AllNamespaces.
+	// CSV being installed. The namespaces passed must exist in the cluster.
+	// An empty set of namespaces can be used for AllNamespaces.
 	InstallMode string
 	// Timeout dictates how long to wait for a REST call to complete. A call
 	// exceeding Timeout will generate an error.
@@ -95,8 +90,7 @@ func (c *OperatorCmd) AddToFlagSet(fs *pflag.FlagSet) {
 	fs.StringVar(&c.OLMNamespace, "olm-namespace", olm.DefaultOLMNamespace,
 		"The namespace where OLM is installed")
 	fs.StringVar(&c.OperatorNamespace, "operator-namespace", "",
-		"The namespace where operator resources are created. It must already exist "+
-			"in the cluster or be defined in a manifest passed to --include")
+		"The namespace where operator resources are created. It must already exist in the cluster")
 	fs.StringVar(&c.InstallMode, "install-mode", "",
 		"InstallMode to create OperatorGroup with. Format: "+installModeFormat)
 	fs.DurationVar(&c.Timeout, "timeout", defaultTimeout,
@@ -129,7 +123,6 @@ type operatorManager struct {
 
 	installMode      operatorsv1alpha1.InstallModeType //nolint:structcheck
 	targetNamespaces []string                          //nolint:structcheck
-	olmObjects       []runtime.Object
 }
 
 func (c *OperatorCmd) newManager() (*operatorManager, error) {
@@ -171,50 +164,3 @@ func (m *operatorManager) status(ctx context.Context, us ...*unstructured.Unstru
 	}
 	return m.client.GetObjectsStatus(ctx, objs...)
 }
-
-func (m operatorManager) hasCatalogSource() bool {
-	return containsKind(m.olmObjects, operatorsv1alpha1.CatalogSourceKind)
-}
-
-func (m operatorManager) hasSubscription() bool {
-	return containsKind(m.olmObjects, operatorsv1alpha1.SubscriptionKind)
-}
-
-func (m operatorManager) hasOperatorGroup() bool {
-	return containsKind(m.olmObjects, operatorsv1.OperatorGroupKind)
-}
-
-func containsKind(objs []runtime.Object, kind string) bool {
-	for _, obj := range objs {
-		if obj.GetObjectKind().GroupVersionKind().Kind == kind {
-			return true
-		}
-	}
-	return false
-}
-
-func readObjectsFromFile(path string) (objs []*unstructured.Unstructured, err error) {
-	b, err := ioutil.ReadFile(path)
-	if err != nil {
-		return nil, err
-	}
-	scanner := k8sutil.NewYAMLScanner(bytes.NewBuffer(b))
-	for scanner.Scan() {
-		b, err := yaml.YAMLToJSON(scanner.Bytes())
-		if err != nil {
-			return nil, fmt.Errorf("failed to convert YAML to JSON before decode: %v", err)
-		}
-		u := &unstructured.Unstructured{}
-		if err := u.UnmarshalJSON(b); err != nil {
-			return nil, fmt.Errorf("failed to decode object from manifest %s: %w", path, err)
-		}
-		objs = append(objs, u)
-	}
-	if scanner.Err() != nil {
-		return nil, fmt.Errorf("failed to scan manifest %s: %w", path, scanner.Err())
-	}
-	if len(objs) == 0 {
-		return nil, fmt.Errorf("no objects found in manifest %s", path)
-	}
-	return objs, nil
-}

internal/olm/operator/packagemanifests.go

@@ -30,32 +30,18 @@ type PackageManifestsCmd struct {
 
 	// ManifestsDir is a directory containing 1..N package directories and
 	// a package manifest.
-	// OperatorVersion can be set to the version of the desired operator package
+	// Version can be set to the version of the desired operator package
 	// and Run()/Cleanup() will deploy that operator version.
 	ManifestsDir string
-	// OperatorVersion is the version of the operator to deploy. It must be
+	// Version is the version of the operator to deploy. It must be
 	// a semantic version, ex. 0.0.1.
-	OperatorVersion string
-	// IncludePaths are path to manifests of Kubernetes resources that either
-	// supplement or override defaults generated by methods of PackageManifestsCmd. These
-	// manifests can be but are not limited to: RBAC, Subscriptions,
-	// CatalogSources, OperatorGroups.
-	//
-	// Kinds that are overridden if supplied:
-	// - CatalogSource
-	// - Subscription
-	// - OperatorGroup
-	IncludePaths []string
+	Version string
 }
 
 func (c *PackageManifestsCmd) AddToFlagSet(fs *pflag.FlagSet) {
 	c.OperatorCmd.AddToFlagSet(fs)
 
-	fs.StringVar(&c.OperatorVersion, "operator-version", "",
-		"Version of operator to deploy")
-	fs.StringSliceVar(&c.IncludePaths, "include", nil,
-		"Path to Kubernetes resource manifests, ex. Role, Subscription. "+
-			"These supplement or override defaults generated by run/cleanup")
+	fs.StringVar(&c.Version, "version", "", "Packaged version of the operator to deploy")
 }
 
 func (c *PackageManifestsCmd) validate() error {
@@ -70,7 +56,7 @@ func (c *PackageManifestsCmd) validate() error {
 		return fmt.Errorf("%s must be a directory", c.ManifestsDir)
 	}
 
-	if c.OperatorVersion == "" {
+	if c.Version == "" {
 		return errors.New("operator version must be set")
 	}
 

internal/olm/operator/packagemanifests_manager.go

@@ -41,32 +41,13 @@ type packageManifestsManager struct {
 
 func (c *PackageManifestsCmd) newManager() (m *packageManifestsManager, err error) {
 	m = &packageManifestsManager{
-		version:       c.OperatorVersion,
+		version:       c.Version,
 		forceRegistry: c.ForceRegistry,
 	}
 	if m.operatorManager, err = c.OperatorCmd.newManager(); err != nil {
 		return nil, err
 	}
 
-	// Parse k8s objects.
-	for _, path := range c.IncludePaths {
-		if path != "" {
-			objs, err := readObjectsFromFile(path)
-			if err != nil {
-				return nil, err
-			}
-			for _, obj := range objs {
-				m.olmObjects = append(m.olmObjects, obj)
-			}
-		}
-	}
-	// Since a Subscription refers to a CatalogSource, supplying one but
-	// not the other is an error.
-	hasSub, hasCatSrc := m.hasSubscription(), m.hasCatalogSource()
-	if hasSub || hasCatSrc && !(hasSub && hasCatSrc) {
-		return nil, errors.New("both a CatalogSource and Subscription must be supplied if one is supplied")
-	}
-
 	// Operator bundles and metadata.
 	m.pkg, m.bundles, err = apimanifests.GetManifestsDir(c.ManifestsDir)
 	if err != nil {
@@ -135,39 +116,22 @@ func (m *packageManifestsManager) run(ctx context.Context) (err error) {
 		return fmt.Errorf("error creating registry resources: %w", err)
 	}
 
+	// New CatalogSource.
+	registryGRPCAddr := internalregistry.GetRegistryServiceAddr(pkgName, m.olmNamespace)
+	catsrc := newCatalogSource(pkgName, m.operatorNamespace, withGRPC(registryGRPCAddr))
+	// New Subscription.
+	channel, err := getChannelForCSVName(m.pkg, csv.GetName())
+	if err != nil {
+		return err
+	}
+	sub := newSubscription(csv.GetName(), m.operatorNamespace,
+		withPackageChannel(pkgName, channel),
+		withCatalogSource(getCatalogSourceName(pkgName), m.operatorNamespace))
+	// New SDK-managed OperatorGroup.
+	og := newSDKOperatorGroup(m.operatorNamespace,
+		withTargetNamespaces(m.targetNamespaces...))
+	objects := []runtime.Object{catsrc, sub, og}
 	log.Info("Creating resources")
-	if !m.hasCatalogSource() {
-		registryGRPCAddr := internalregistry.GetRegistryServiceAddr(pkgName, m.olmNamespace)
-		catsrc := newCatalogSource(pkgName, m.operatorNamespace, withGRPC(registryGRPCAddr))
-		m.olmObjects = append(m.olmObjects, catsrc)
-	}
-	if !m.hasSubscription() {
-		channel, err := getChannelForCSVName(m.pkg, csv.GetName())
-		if err != nil {
-			return err
-		}
-		sub := newSubscription(csv.GetName(), m.operatorNamespace,
-			withPackageChannel(pkgName, channel),
-			withCatalogSource(getCatalogSourceName(pkgName), m.operatorNamespace))
-		m.olmObjects = append(m.olmObjects, sub)
-	}
-	if !m.hasOperatorGroup() {
-		og := newSDKOperatorGroup(m.operatorNamespace,
-			withTargetNamespaces(m.targetNamespaces...))
-		m.olmObjects = append(m.olmObjects, og)
-	}
-	// Check for Namespace objects and create those first.
-	namespaces, objects := []runtime.Object{}, []runtime.Object{}
-	for _, obj := range m.olmObjects {
-		if obj.GetObjectKind().GroupVersionKind().Kind == "Namespace" {
-			namespaces = append(namespaces, obj)
-		} else {
-			objects = append(objects, obj)
-		}
-	}
-	if err = m.client.DoCreate(ctx, namespaces...); err != nil {
-		return fmt.Errorf("error creating operator resources: %w", err)
-	}
 	if err = m.client.DoCreate(ctx, objects...); err != nil {
 		return fmt.Errorf("error creating operator resources: %w", err)
 	}
@@ -213,25 +177,18 @@ func (m *packageManifestsManager) cleanup(ctx context.Context) (err error) {
 		return fmt.Errorf("error removing registry resources: %w", err)
 	}
 
-	log.Info("Deleting resources")
-	if !m.hasCatalogSource() {
-		m.olmObjects = append(m.olmObjects, newCatalogSource(pkgName, m.operatorNamespace))
-	}
-	if !m.hasSubscription() {
-		m.olmObjects = append(m.olmObjects, newSubscription(csv.GetName(), m.operatorNamespace))
-	}
-	if !m.hasOperatorGroup() {
-		m.olmObjects = append(m.olmObjects, newSDKOperatorGroup(m.operatorNamespace))
-	}
-	toDelete := []runtime.Object{}
-	for _, obj := range m.olmObjects {
-		toDelete = append(toDelete, obj.DeepCopyObject())
+	// Delete CatalogSource, Subscription, the SDK-managed OperatorGroup, and any bundle objects.
+	toDelete := []runtime.Object{
+		newCatalogSource(pkgName, m.operatorNamespace),
+		newSubscription(csv.GetName(), m.operatorNamespace),
+		newSDKOperatorGroup(m.operatorNamespace),
 	}
 	for _, obj := range bundle.Objects {
 		objc := obj.DeepCopy()
 		objc.SetNamespace(m.operatorNamespace)
 		toDelete = append(toDelete, objc)
 	}
+	log.Info("Deleting resources")
 	if err = m.client.DoDelete(ctx, toDelete...); err != nil {
 		return fmt.Errorf("error deleting operator resources: %w", err)
 	}

test/e2e-helm/e2e_helm_olm_test.go

@@ -40,7 +40,7 @@ var _ = Describe("Integrating Helm Projects with OLM", func() {
 		AfterEach(func() {
 			By("destroying the deployed package manifests-formatted operator")
 			cleanupPkgManCmd := exec.Command(tc.BinaryName, "cleanup", "packagemanifests",
-				"--operator-version", operatorVersion,
+				"--version", operatorVersion,
 				"--timeout", "4m")
 			_, _ = tc.Run(cleanupPkgManCmd)
 
@@ -93,7 +93,7 @@ var _ = Describe("Integrating Helm Projects with OLM", func() {
 			By("running the package")
 			runPkgManCmd := exec.Command(tc.BinaryName, "run", "packagemanifests",
 				"--install-mode", "AllNamespaces",
-				"--operator-version", operatorVersion,
+				"--version", operatorVersion,
 				"--timeout", "4m")
 			_, err = tc.Run(runPkgManCmd)
 			Expect(err).NotTo(HaveOccurred())

test/e2e/e2e_suite.go

@@ -197,15 +197,15 @@ var _ = Describe("operator-sdk", func() {
 			runPkgManCmd := exec.Command(tc.BinaryName, "run", "packagemanifests",
 				"--install-mode", "AllNamespaces",
 				"--operator-namespace", tc.Kubectl.Namespace,
-				"--operator-version", operatorVersion,
+				"--version", operatorVersion,
 				"--timeout", "4m")
 			_, err = tc.Run(runPkgManCmd)
 			Expect(err).NotTo(HaveOccurred())
 
 			By("destroying the deployed package manifests-formatted operator")
 			cleanupPkgManCmd := exec.Command(tc.BinaryName, "cleanup", "packagemanifests",
 				"--operator-namespace", tc.Kubectl.Namespace,
-				"--operator-version", operatorVersion,
+				"--version", operatorVersion,
 				"--timeout", "4m")
 			_, err = tc.Run(cleanupPkgManCmd)
 			Expect(err).NotTo(HaveOccurred())

test/integration/integration_helpers.go

@@ -49,7 +49,7 @@ type DefinitionKey struct {
 
 type CSVTemplateConfig struct {
 	OperatorName    string
-	OperatorVersion string
+	Version         string
 	TestImageTag    string
 	ReplacesCSVName string
 	CRDKeys         []DefinitionKey
@@ -63,7 +63,7 @@ kind: ClusterServiceVersion
 metadata:
   annotations:
     capabilities: Basic Install
-  name: {{ .OperatorName }}.v{{ .OperatorVersion }}
+  name: {{ .OperatorName }}.v{{ .Version }}
   namespace: placeholder
 spec:
   apiservicedefinitions: {}
@@ -196,15 +196,15 @@ spec:
 {{- if .ReplacesCSVName }}
   replaces: {{ .ReplacesCSVName }}
 {{- end }}
-  version: {{ .OperatorVersion }}
+  version: {{ .Version }}
 `
 
 func writeOperatorManifests(dir string, csvConfig CSVTemplateConfig) error {
 	manifestDir := ""
 	if csvConfig.IsBundle {
 		manifestDir = filepath.Join(dir, bundle.ManifestsDir)
 	} else {
-		manifestDir = filepath.Join(dir, csvConfig.OperatorVersion)
+		manifestDir = filepath.Join(dir, csvConfig.Version)
 	}
 	for _, key := range csvConfig.CRDKeys {
 		crd := apiextv1beta1.CustomResourceDefinition{