operator-framework · perdasilva · Dec 13, 2021
diff --git a/cmd/catalog/main.go b/cmd/catalog/main.go
@@ -8,12 +8,9 @@ import (
 	"os"
 	"time"
 
-	configv1client "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1"
-	log "github.com/sirupsen/logrus"
-	utilclock "k8s.io/apimachinery/pkg/util/clock"
-	k8sscheme "k8s.io/client-go/kubernetes/scheme"
-	"k8s.io/client-go/tools/clientcmd"
+	"sigs.k8s.io/controller-runtime/pkg/client/config"
 
+	configv1client "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/api/client"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/catalog"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/catalogtemplate"
@@ -23,6 +20,9 @@ import (
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/signals"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/metrics"
 	olmversion "github.com/operator-framework/operator-lifecycle-manager/pkg/version"
+	log "github.com/sirupsen/logrus"
+	utilclock "k8s.io/apimachinery/pkg/util/clock"
+	k8sscheme "k8s.io/client-go/kubernetes/scheme"
 )
 
 const (
@@ -37,9 +37,6 @@ const (
 
 // config flags defined globally so that they appear on the test binary as well
 var (
-	kubeConfigPath = flag.String(
-		"kubeconfig", os.Getenv("KUBECONFIG"), "absolute path to the kubeconfig file")
-
 	wakeupInterval = flag.Duration(
 		"interval", defaultWakeupInterval, "wakeup interval")
 
@@ -119,27 +116,31 @@ func main() {
 	}()
 
 	// create a config client for operator status
-	config, err := clientcmd.BuildConfigFromFlags("", *kubeConfigPath)
+	kubeconfig, err := config.GetConfig()
+	if err != nil {
+		log.Fatalf("error getting kubeconfig: %s", err.Error())
+	}
+	configClient, err := configv1client.NewForConfig(kubeconfig)
 	if err != nil {
 		log.Fatalf("error configuring client: %s", err.Error())
 	}
-	configClient, err := configv1client.NewForConfig(config)
+	opClient, err := operatorclient.NewClientFromRestConfig(kubeconfig)
 	if err != nil {
 		log.Fatalf("error configuring client: %s", err.Error())
 	}
-	opClient := operatorclient.NewClientFromConfig(*kubeConfigPath, logger)
-	crClient, err := client.NewClient(*kubeConfigPath)
+
+	crClient, err := client.NewClient(kubeconfig)
 	if err != nil {
 		log.Fatalf("error configuring client: %s", err.Error())
 	}
 
 	// Create a new instance of the operator.
-	op, err := catalog.NewOperator(ctx, *kubeConfigPath, utilclock.RealClock{}, logger, *wakeupInterval, *configmapServerImage, *opmImage, *utilImage, *catalogNamespace, k8sscheme.Scheme, *installPlanTimeout, *bundleUnpackTimeout)
+	op, err := catalog.NewOperator(ctx, kubeconfig, utilclock.RealClock{}, logger, *wakeupInterval, *configmapServerImage, *opmImage, *utilImage, *catalogNamespace, k8sscheme.Scheme, *installPlanTimeout, *bundleUnpackTimeout)
 	if err != nil {
 		log.Fatalf("error configuring catalog operator: %s", err.Error())
 	}
 
-	opCatalogTemplate, err := catalogtemplate.NewOperator(ctx, *kubeConfigPath, logger, *wakeupInterval, *catalogNamespace)
+	opCatalogTemplate, err := catalogtemplate.NewOperator(ctx, kubeconfig, logger, *wakeupInterval, *catalogNamespace)
 	if err != nil {
 		log.Fatalf("error configuring catalog template operator: %s", err.Error())
 	}

diff --git a/pkg/api/client/client.go b/pkg/api/client/client.go
@@ -6,11 +6,6 @@ import (
 )
 
 // NewClient creates a client that can interact with OLM resources in k8s api
-func NewClient(kubeconfig string) (client versioned.Interface, err error) {
-	var config *rest.Config
-	config, err = getConfig(kubeconfig)
-	if err != nil {
-		return
-	}
+func NewClient(config *rest.Config) (client versioned.Interface, err error) {
 	return versioned.NewForConfig(config)
 }
diff --git a/pkg/api/client/util.go b/pkg/api/client/util.go
diff --git a/pkg/controller/operators/catalog/operator.go b/pkg/controller/operators/catalog/operator.go
@@ -11,6 +11,8 @@ import (
 	"sync"
 	"time"
 
+	"k8s.io/client-go/rest"
+
 	errorwrap "github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	"google.golang.org/grpc/connectivity"
@@ -36,7 +38,6 @@ import (
 	"k8s.io/client-go/dynamic"
 	"k8s.io/client-go/informers"
 	"k8s.io/client-go/tools/cache"
-	"k8s.io/client-go/tools/clientcmd"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/client-go/util/retry"
 	"k8s.io/client-go/util/workqueue"
@@ -122,12 +123,8 @@ type Operator struct {
 type CatalogSourceSyncFunc func(logger *logrus.Entry, in *v1alpha1.CatalogSource) (out *v1alpha1.CatalogSource, continueSync bool, syncError error)
 
 // NewOperator creates a new Catalog Operator.
-func NewOperator(ctx context.Context, kubeconfigPath string, clock utilclock.Clock, logger *logrus.Logger, resync time.Duration, configmapRegistryImage, opmImage, utilImage string, operatorNamespace string, scheme *runtime.Scheme, installPlanTimeout time.Duration, bundleUnpackTimeout time.Duration) (*Operator, error) {
+func NewOperator(ctx context.Context, config *rest.Config, clock utilclock.Clock, logger *logrus.Logger, resync time.Duration, configmapRegistryImage, opmImage, utilImage string, operatorNamespace string, scheme *runtime.Scheme, installPlanTimeout time.Duration, bundleUnpackTimeout time.Duration) (*Operator, error) {
 	resyncPeriod := queueinformer.ResyncWithJitter(resync, 0.2)
-	config, err := clientcmd.BuildConfigFromFlags("", kubeconfigPath)
-	if err != nil {
-		return nil, err
-	}
 
 	// Create a new client for OLM types (CRs)
 	crClient, err := versioned.NewForConfig(config)

diff --git a/pkg/controller/operators/catalogtemplate/operator.go b/pkg/controller/operators/catalogtemplate/operator.go
@@ -6,13 +6,14 @@ import (
 	"strings"
 	"time"
 
+	"k8s.io/client-go/rest"
+
 	"github.com/distribution/distribution/reference"
 	"github.com/operator-framework/api/pkg/operators/v1alpha1"
 	"github.com/sirupsen/logrus"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/discovery"
 	"k8s.io/client-go/dynamic"
-	"k8s.io/client-go/tools/clientcmd"
 	"k8s.io/client-go/util/workqueue"
 
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/clientset/versioned"
@@ -43,14 +44,9 @@ type Operator struct {
 	resyncPeriod                  func() time.Duration            // period of time between resync
 }
 
-func NewOperator(ctx context.Context, kubeconfigPath string, logger *logrus.Logger, resync time.Duration, operatorNamespace string) (*Operator, error) {
+func NewOperator(ctx context.Context, config *rest.Config, logger *logrus.Logger, resync time.Duration, operatorNamespace string) (*Operator, error) {
 	resyncPeriod := queueinformer.ResyncWithJitter(resync, 0.2)
 
-	config, err := clientcmd.BuildConfigFromFlags("", kubeconfigPath)
-	if err != nil {
-		return nil, err
-	}
-
 	// Create a new client for OLM types (CRs)
 	crClient, err := versioned.NewForConfig(config)
 	if err != nil {

diff --git a/pkg/controller/registry/resolver/resolver_test.go b/pkg/controller/registry/resolver/resolver_test.go
@@ -85,6 +85,8 @@ func TestSolveOperators_WithSystemConstraints(t *testing.T) {
 	existingPackageD := existingOperator(namespace, "packageD.v1", "packageD", "alpha", "", nil, nil, nil, nil)
 	existingPackageD.Annotations = map[string]string{"operatorframework.io/properties": `{"properties":[{"type":"olm.package","value":{"packageName":"packageD","version":"1.0.0"}}]}`}
 
+	// Fake system constraints provider
+	// In this case it behaves as a whitelist. Only whitelisted operators can be installed.
 	whiteListConstraintProvider := func(whiteList ...*cache.Entry) solver.ConstraintProviderFunc {
 		return func(entry *cache.Entry) ([]solver.Constraint, error) {
 			for _, whiteListedEntry := range whiteList {
@@ -111,7 +113,7 @@ func TestSolveOperators_WithSystemConstraints(t *testing.T) {
 		err                       string
 	}{
 		{
-			title:                     "No runtime constraints",
+			title:                     "no system constraints should output A and B operators",
 			snapshotEntries:           []*cache.Entry{packageA, packageB, packageC, packageD},
 			systemConstraintsProvider: nil,
 			expectedOperators:         cache.OperatorSet{"packageA.v1": packageA, "packageB.v1": packageB},
@@ -120,7 +122,7 @@ func TestSolveOperators_WithSystemConstraints(t *testing.T) {
 			err:                       "",
 		},
 		{
-			title:                     "Runtime constraints only accept packages A and C",
+			title:                     "system constraints (whitelist = [A, C]) should output A and C operators",
 			snapshotEntries:           []*cache.Entry{packageA, packageB, packageC, packageD},
 			systemConstraintsProvider: whiteListConstraintProvider(packageA, packageC),
 			expectedOperators:         cache.OperatorSet{"packageA.v1": packageA, "packageC.v1": packageC},
@@ -129,7 +131,7 @@ func TestSolveOperators_WithSystemConstraints(t *testing.T) {
 			err:                       "",
 		},
 		{
-			title:                     "Existing packages are ignored",
+			title:                     "system constraints (whitelist = [A, C]) and D already installed should output A and C operators",
 			snapshotEntries:           []*cache.Entry{packageA, packageB, packageC, packageD},
 			systemConstraintsProvider: whiteListConstraintProvider(packageA, packageC),
 			expectedOperators:         cache.OperatorSet{"packageA.v1": packageA, "packageC.v1": packageC},
@@ -138,7 +140,7 @@ func TestSolveOperators_WithSystemConstraints(t *testing.T) {
 			err:                       "",
 		},
 		{
-			title:                     "Runtime constraints don't allow A",
+			title:                     "system constraints (whitelist = []) raises error",
 			snapshotEntries:           []*cache.Entry{packageA, packageB, packageC, packageD},
 			systemConstraintsProvider: whiteListConstraintProvider(packageB, packageC, packageD),
 			expectedOperators:         nil,

diff --git a/pkg/lib/operatorclient/client.go b/pkg/lib/operatorclient/client.go
@@ -2,7 +2,6 @@
 package operatorclient
 
 import (
-	"github.com/sirupsen/logrus"
 	appsv1 "k8s.io/api/apps/v1"
 	v1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
@@ -12,7 +11,6 @@ import (
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
-	"k8s.io/client-go/tools/clientcmd"
 	apiregistrationv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1"
 	apiregistration "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset"
 )
@@ -146,26 +144,6 @@ type Client struct {
 	regInterface apiregistration.Interface
 }
 
-// NewClient creates a kubernetes client or bails out on on failures.
-func NewClientFromConfig(kubeconfig string, logger *logrus.Logger) ClientInterface {
-	var config *rest.Config
-	var err error
-
-	if kubeconfig != "" {
-		logger.Infof("Loading kube client config from path %q", kubeconfig)
-		config, err = clientcmd.BuildConfigFromFlags("", kubeconfig)
-	} else {
-		logger.Infof("Using in-cluster kube client config")
-		config, err = rest.InClusterConfig()
-	}
-
-	if err != nil {
-		logger.Fatalf("Cannot load config for REST client: %v", err)
-	}
-
-	return &Client{kubernetes.NewForConfigOrDie(config), apiextensions.NewForConfigOrDie(config), apiregistration.NewForConfigOrDie(config)}
-}
-
 func NewClientFromRestConfig(config *rest.Config) (client ClientInterface, err error) {
 	kubernetes, err := kubernetes.NewForConfig(config)
 	if err != nil {

diff --git a/pkg/package-server/server/server.go b/pkg/package-server/server/server.go
@@ -205,7 +205,7 @@ func (o *PackageServerOptions) Run(ctx context.Context) error {
 		return fmt.Errorf("unable to construct lister client: %v", err)
 	}
 
-	crClient, err := client.NewClient(o.Kubeconfig)
+	crClient, err := client.NewClient(clientConfig)
 	if err != nil {
 		return err
 	}