From 77ed0a2614b459f49655d2060a6e39cf67cdcdbb Mon Sep 17 00:00:00 2001 From: Alexander Greene Date: Thu, 21 Jul 2022 09:23:59 -0700 Subject: [PATCH 1/2] Introduce protectedCopiedCSVNamespaces flag (#2811) Problem: Users rely on Copied CSVs in order to understand which operators are available in a given namespace. When installing All Namespace operators, a Copied CSV is created in every namespace which can place a huge performance strain on clusters with many namespaces. OLM introduced the ability to disable Copied CSVs for All Namespace mode operators in an effort to resolve the performance issues on large clusters, unfortunately removing the ability for users to identify which operators are available in a given namespace. Solution: The protectedCopiedCSVNamespaces runtime flag can be used to prevent Copied CSVs from being deleted even when Copied CSVs are disabled. An admin can then provide users with the proper RBAC to view which operators are running in All Namespace mode. Signed-off-by: Alexander Greene Upstream-repository: operator-lifecycle-manager Upstream-commit: 6c4e779554c892494921c1bc9a00ba547cb6a431 --- .../cmd/olm/main.go | 4 + .../pkg/controller/operators/olm/config.go | 55 ++++-- .../pkg/controller/operators/olm/operator.go | 184 ++++++++++++------ .../test/e2e/disabling_copied_csv_e2e_test.go | 55 +++++- .../cmd/olm/main.go | 4 + .../pkg/controller/operators/olm/config.go | 55 ++++-- .../pkg/controller/operators/olm/operator.go | 184 ++++++++++++------ 7 files changed, 372 insertions(+), 169 deletions(-) diff --git a/staging/operator-lifecycle-manager/cmd/olm/main.go b/staging/operator-lifecycle-manager/cmd/olm/main.go index 7786f5b823..fd063ffe56 100644 --- a/staging/operator-lifecycle-manager/cmd/olm/main.go +++ b/staging/operator-lifecycle-manager/cmd/olm/main.go @@ -60,6 +60,9 @@ var ( tlsKeyPath = pflag.String( "tls-key", "", "Path to use for private key (requires tls-cert)") + protectedCopiedCSVNamespaces = pflag.String("protectedCopiedCSVNamespaces", + "", "A comma-delimited set of namespaces where global Copied CSVs will always appear, even if Copied CSVs are disabled") + tlsCertPath = pflag.String( "tls-cert", "", "Path to use for certificate key (requires tls-key)") @@ -162,6 +165,7 @@ func main() { olm.WithOperatorClient(opClient), olm.WithRestConfig(config), olm.WithConfigClient(versionedConfigClient), + olm.WithProtectedCopiedCSVNamespaces(*protectedCopiedCSVNamespaces), ) if err != nil { logger.WithError(err).Fatal("error configuring operator") diff --git a/staging/operator-lifecycle-manager/pkg/controller/operators/olm/config.go b/staging/operator-lifecycle-manager/pkg/controller/operators/olm/config.go index adcee6cdce..b56e27358e 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/operators/olm/config.go +++ b/staging/operator-lifecycle-manager/pkg/controller/operators/olm/config.go @@ -1,6 +1,7 @@ package olm import ( + "strings" "time" "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/queueinformer" @@ -21,18 +22,19 @@ import ( type OperatorOption func(*operatorConfig) type operatorConfig struct { - resyncPeriod func() time.Duration - operatorNamespace string - watchedNamespaces []string - clock utilclock.Clock - logger *logrus.Logger - operatorClient operatorclient.ClientInterface - externalClient versioned.Interface - strategyResolver install.StrategyResolverInterface - apiReconciler APIIntersectionReconciler - apiLabeler labeler.Labeler - restConfig *rest.Config - configClient configv1client.Interface + protectedCopiedCSVNamespaces map[string]struct{} + resyncPeriod func() time.Duration + operatorNamespace string + watchedNamespaces []string + clock utilclock.Clock + logger *logrus.Logger + operatorClient operatorclient.ClientInterface + externalClient versioned.Interface + strategyResolver install.StrategyResolverInterface + apiReconciler APIIntersectionReconciler + apiLabeler labeler.Labeler + restConfig *rest.Config + configClient configv1client.Interface } func (o *operatorConfig) apply(options []OperatorOption) { @@ -77,14 +79,15 @@ func (o *operatorConfig) validate() (err error) { func defaultOperatorConfig() *operatorConfig { return &operatorConfig{ - resyncPeriod: queueinformer.ResyncWithJitter(30*time.Second, 0.2), - operatorNamespace: "default", - watchedNamespaces: []string{metav1.NamespaceAll}, - clock: utilclock.RealClock{}, - logger: logrus.New(), - strategyResolver: &install.StrategyResolver{}, - apiReconciler: APIIntersectionReconcileFunc(ReconcileAPIIntersection), - apiLabeler: labeler.Func(LabelSetsFor), + resyncPeriod: queueinformer.ResyncWithJitter(30*time.Second, 0.2), + operatorNamespace: "default", + watchedNamespaces: []string{metav1.NamespaceAll}, + clock: utilclock.RealClock{}, + logger: logrus.New(), + strategyResolver: &install.StrategyResolver{}, + apiReconciler: APIIntersectionReconcileFunc(ReconcileAPIIntersection), + apiLabeler: labeler.Func(LabelSetsFor), + protectedCopiedCSVNamespaces: map[string]struct{}{}, } } @@ -112,6 +115,18 @@ func WithLogger(logger *logrus.Logger) OperatorOption { } } +func WithProtectedCopiedCSVNamespaces(namespaces string) OperatorOption { + return func(config *operatorConfig) { + if namespaces == "" { + return + } + + for _, ns := range strings.Split(namespaces, ",") { + config.protectedCopiedCSVNamespaces[ns] = struct{}{} + } + } +} + func WithClock(clock utilclock.Clock) OperatorOption { return func(config *operatorConfig) { config.clock = clock diff --git a/staging/operator-lifecycle-manager/pkg/controller/operators/olm/operator.go b/staging/operator-lifecycle-manager/pkg/controller/operators/olm/operator.go index b6c3d2abc1..d33945aee8 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/operators/olm/operator.go +++ b/staging/operator-lifecycle-manager/pkg/controller/operators/olm/operator.go @@ -63,32 +63,33 @@ var ( type Operator struct { queueinformer.Operator - clock utilclock.Clock - logger *logrus.Logger - opClient operatorclient.ClientInterface - client versioned.Interface - lister operatorlister.OperatorLister - copiedCSVLister operatorsv1alpha1listers.ClusterServiceVersionLister - ogQueueSet *queueinformer.ResourceQueueSet - csvQueueSet *queueinformer.ResourceQueueSet - olmConfigQueue workqueue.RateLimitingInterface - csvCopyQueueSet *queueinformer.ResourceQueueSet - copiedCSVGCQueueSet *queueinformer.ResourceQueueSet - objGCQueueSet *queueinformer.ResourceQueueSet - nsQueueSet workqueue.RateLimitingInterface - apiServiceQueue workqueue.RateLimitingInterface - csvIndexers map[string]cache.Indexer - recorder record.EventRecorder - resolver install.StrategyResolverInterface - apiReconciler APIIntersectionReconciler - apiLabeler labeler.Labeler - csvSetGenerator csvutility.SetGenerator - csvReplaceFinder csvutility.ReplaceFinder - csvNotification csvutility.WatchNotification - serviceAccountSyncer *scoped.UserDefinedServiceAccountSyncer - clientAttenuator *scoped.ClientAttenuator - serviceAccountQuerier *scoped.UserDefinedServiceAccountQuerier - clientFactory clients.Factory + clock utilclock.Clock + logger *logrus.Logger + opClient operatorclient.ClientInterface + client versioned.Interface + lister operatorlister.OperatorLister + protectedCopiedCSVNamespaces map[string]struct{} + copiedCSVLister operatorsv1alpha1listers.ClusterServiceVersionLister + ogQueueSet *queueinformer.ResourceQueueSet + csvQueueSet *queueinformer.ResourceQueueSet + olmConfigQueue workqueue.RateLimitingInterface + csvCopyQueueSet *queueinformer.ResourceQueueSet + copiedCSVGCQueueSet *queueinformer.ResourceQueueSet + objGCQueueSet *queueinformer.ResourceQueueSet + nsQueueSet workqueue.RateLimitingInterface + apiServiceQueue workqueue.RateLimitingInterface + csvIndexers map[string]cache.Indexer + recorder record.EventRecorder + resolver install.StrategyResolverInterface + apiReconciler APIIntersectionReconciler + apiLabeler labeler.Labeler + csvSetGenerator csvutility.SetGenerator + csvReplaceFinder csvutility.ReplaceFinder + csvNotification csvutility.WatchNotification + serviceAccountSyncer *scoped.UserDefinedServiceAccountSyncer + clientAttenuator *scoped.ClientAttenuator + serviceAccountQuerier *scoped.UserDefinedServiceAccountQuerier + clientFactory clients.Factory } func NewOperator(ctx context.Context, options ...OperatorOption) (*Operator, error) { @@ -121,30 +122,31 @@ func newOperatorWithConfig(ctx context.Context, config *operatorConfig) (*Operat } op := &Operator{ - Operator: queueOperator, - clock: config.clock, - logger: config.logger, - opClient: config.operatorClient, - client: config.externalClient, - ogQueueSet: queueinformer.NewEmptyResourceQueueSet(), - csvQueueSet: queueinformer.NewEmptyResourceQueueSet(), - olmConfigQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "olmConfig"), - csvCopyQueueSet: queueinformer.NewEmptyResourceQueueSet(), - copiedCSVGCQueueSet: queueinformer.NewEmptyResourceQueueSet(), - objGCQueueSet: queueinformer.NewEmptyResourceQueueSet(), - apiServiceQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "apiservice"), - resolver: config.strategyResolver, - apiReconciler: config.apiReconciler, - lister: lister, - recorder: eventRecorder, - apiLabeler: config.apiLabeler, - csvIndexers: map[string]cache.Indexer{}, - csvSetGenerator: csvutility.NewSetGenerator(config.logger, lister), - csvReplaceFinder: csvutility.NewReplaceFinder(config.logger, config.externalClient), - serviceAccountSyncer: scoped.NewUserDefinedServiceAccountSyncer(config.logger, scheme, config.operatorClient, config.externalClient), - clientAttenuator: scoped.NewClientAttenuator(config.logger, config.restConfig, config.operatorClient), - serviceAccountQuerier: scoped.NewUserDefinedServiceAccountQuerier(config.logger, config.externalClient), - clientFactory: clients.NewFactory(config.restConfig), + Operator: queueOperator, + clock: config.clock, + logger: config.logger, + opClient: config.operatorClient, + client: config.externalClient, + ogQueueSet: queueinformer.NewEmptyResourceQueueSet(), + csvQueueSet: queueinformer.NewEmptyResourceQueueSet(), + olmConfigQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "olmConfig"), + csvCopyQueueSet: queueinformer.NewEmptyResourceQueueSet(), + copiedCSVGCQueueSet: queueinformer.NewEmptyResourceQueueSet(), + objGCQueueSet: queueinformer.NewEmptyResourceQueueSet(), + apiServiceQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "apiservice"), + resolver: config.strategyResolver, + apiReconciler: config.apiReconciler, + lister: lister, + recorder: eventRecorder, + apiLabeler: config.apiLabeler, + csvIndexers: map[string]cache.Indexer{}, + csvSetGenerator: csvutility.NewSetGenerator(config.logger, lister), + csvReplaceFinder: csvutility.NewReplaceFinder(config.logger, config.externalClient), + serviceAccountSyncer: scoped.NewUserDefinedServiceAccountSyncer(config.logger, scheme, config.operatorClient, config.externalClient), + clientAttenuator: scoped.NewClientAttenuator(config.logger, config.restConfig, config.operatorClient), + serviceAccountQuerier: scoped.NewUserDefinedServiceAccountQuerier(config.logger, config.externalClient), + clientFactory: clients.NewFactory(config.restConfig), + protectedCopiedCSVNamespaces: config.protectedCopiedCSVNamespaces, } // Set up syncing for namespace-scoped resources @@ -1299,10 +1301,14 @@ func (a *Operator) syncOLMConfig(obj interface{}) (syncError error) { return err } - // Filter to unique copies - uniqueCopiedCSVs := map[string]struct{}{} + // Create a map that points from CSV name to a map of namespaces it is copied to + // for quick lookups. + copiedCSVNamespaces := map[string]map[string]struct{}{} for _, copiedCSV := range copiedCSVs { - uniqueCopiedCSVs[copiedCSV.GetName()] = struct{}{} + if _, ok := copiedCSVNamespaces[copiedCSV.GetName()]; !ok { + copiedCSVNamespaces[copiedCSV.GetName()] = map[string]struct{}{} + } + copiedCSVNamespaces[copiedCSV.GetName()][copiedCSV.GetNamespace()] = struct{}{} } csvs, err := a.lister.OperatorsV1alpha1().ClusterServiceVersionLister().ClusterServiceVersions(og.GetNamespace()).List(labels.NewSelector().Add(*nonCopiedCSVRequirement)) @@ -1310,9 +1316,16 @@ func (a *Operator) syncOLMConfig(obj interface{}) (syncError error) { return err } + namespaces, err := a.lister.CoreV1().NamespaceLister().List(labels.Everything()) + if err != nil { + return err + } + + copiedCSVEvaluatorFunc := getCopiedCSVEvaluatorFunc(olmConfig.CopiedCSVsAreEnabled(), namespaces, a.protectedCopiedCSVNamespaces) + for _, csv := range csvs { - // If the correct number of copied CSVs were found, continue - if _, ok := uniqueCopiedCSVs[csv.GetName()]; ok == olmConfig.CopiedCSVsAreEnabled() { + // Ignore NS where actual CSV is installed + if copiedCSVEvaluatorFunc(copiedCSVNamespaces[csv.GetName()]) { continue } @@ -1324,7 +1337,7 @@ func (a *Operator) syncOLMConfig(obj interface{}) (syncError error) { } // Update the olmConfig status if it has changed. - condition := getCopiedCSVsCondition(!olmConfig.CopiedCSVsAreEnabled(), csvIsRequeued) + condition := getCopiedCSVsCondition(olmConfig.CopiedCSVsAreEnabled(), csvIsRequeued) if !isStatusConditionPresentAndAreTypeReasonMessageStatusEqual(olmConfig.Status.Conditions, condition) { meta.SetStatusCondition(&olmConfig.Status.Conditions, condition) if _, err := a.client.OperatorsV1().OLMConfigs().UpdateStatus(context.TODO(), olmConfig, metav1.UpdateOptions{}); err != nil { @@ -1335,6 +1348,37 @@ func (a *Operator) syncOLMConfig(obj interface{}) (syncError error) { return nil } +// getCopiedCSVEvaluatorFunc returns a function that evaluates if the a set of Copied CSVs exist in the expected namespaces. +func getCopiedCSVEvaluatorFunc(copiedCSVsEnabled bool, namespaces []*corev1.Namespace, protectedCopiedCSVNamespaces map[string]struct{}) func(map[string]struct{}) bool { + if copiedCSVsEnabled { + // Exclude the namespace hosting the original CSV + expectedCopiedCSVCount := -1 + for _, ns := range namespaces { + if ns.Status.Phase == corev1.NamespaceActive { + expectedCopiedCSVCount++ + } + } + return func(m map[string]struct{}) bool { + return expectedCopiedCSVCount == len(m) + } + } + + // Check that Copied CSVs exist in protected namespaces. + return func(m map[string]struct{}) bool { + if len(protectedCopiedCSVNamespaces) != len(m) { + return false + } + + for protectedNS := range protectedCopiedCSVNamespaces { + if _, ok := m[protectedNS]; !ok { + return false + } + } + + return true + } +} + func isStatusConditionPresentAndAreTypeReasonMessageStatusEqual(conditions []metav1.Condition, condition metav1.Condition) bool { foundCondition := meta.FindStatusCondition(conditions, condition.Type) if foundCondition == nil { @@ -1346,13 +1390,13 @@ func isStatusConditionPresentAndAreTypeReasonMessageStatusEqual(conditions []met foundCondition.Status == condition.Status } -func getCopiedCSVsCondition(isDisabled, csvIsRequeued bool) metav1.Condition { +func getCopiedCSVsCondition(enabled, csvIsRequeued bool) metav1.Condition { condition := metav1.Condition{ Type: operatorsv1.DisabledCopiedCSVsConditionType, LastTransitionTime: metav1.Now(), Status: metav1.ConditionFalse, } - if !isDisabled { + if enabled { condition.Reason = "CopiedCSVsEnabled" condition.Message = "Copied CSVs are enabled and present across the cluster" if csvIsRequeued { @@ -1361,15 +1405,14 @@ func getCopiedCSVsCondition(isDisabled, csvIsRequeued bool) metav1.Condition { return condition } + condition.Reason = "CopiedCSVsDisabled" if csvIsRequeued { - condition.Reason = "CopiedCSVsFound" - condition.Message = "Copied CSVs are disabled and at least one copied CSV was found for an operator installed in AllNamespace mode" + condition.Message = "Copied CSVs are disabled and at least one unexpected copied CSV was found for an operator installed in AllNamespace mode" return condition } condition.Status = metav1.ConditionTrue - condition.Reason = "NoCopiedCSVsFound" - condition.Message = "Copied CSVs are disabled and none were found for operators installed in AllNamespace mode" + condition.Message = "Copied CSVs are disabled and no unexpected copied CSVs were found for operators installed in AllNamespace mode" return condition } @@ -1444,7 +1487,24 @@ func (a *Operator) syncCopyCSV(obj interface{}) (syncError error) { return err } + // Ensure that the Copied CSVs exist in the protected namespaces. + protectedNamespaces := []string{} + for ns := range a.protectedCopiedCSVNamespaces { + if ns == clusterServiceVersion.GetNamespace() { + continue + } + protectedNamespaces = append(protectedNamespaces, ns) + } + + if err := a.ensureCSVsInNamespaces(clusterServiceVersion, operatorGroup, NewNamespaceSet(protectedNamespaces)); err != nil { + return err + } + + // Delete Copied CSVs in namespaces that are not protected. for _, copiedCSV := range copiedCSVs { + if _, ok := a.protectedCopiedCSVNamespaces[copiedCSV.Namespace]; ok { + continue + } err := a.client.OperatorsV1alpha1().ClusterServiceVersions(copiedCSV.Namespace).Delete(context.TODO(), copiedCSV.Name, metav1.DeleteOptions{}) if err != nil && !apierrors.IsNotFound(err) { return err diff --git a/staging/operator-lifecycle-manager/test/e2e/disabling_copied_csv_e2e_test.go b/staging/operator-lifecycle-manager/test/e2e/disabling_copied_csv_e2e_test.go index 3f89cd831f..5bafe69fc6 100644 --- a/staging/operator-lifecycle-manager/test/e2e/disabling_copied_csv_e2e_test.go +++ b/staging/operator-lifecycle-manager/test/e2e/disabling_copied_csv_e2e_test.go @@ -3,12 +3,14 @@ package e2e import ( "context" "fmt" + "strings" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" operatorsv1 "github.com/operator-framework/api/pkg/operators/v1" operatorsv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1" "github.com/operator-framework/operator-lifecycle-manager/test/e2e/ctx" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -19,11 +21,17 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) +const ( + olmDeploymentName = "olm-operator" + protectedCopiedCSVNamespacesRuntimeFlag = "--protectedCopiedCSVNamespaces" +) + var _ = Describe("Disabling copied CSVs", func() { var ( ns corev1.Namespace csv operatorsv1alpha1.ClusterServiceVersion nonTerminatingNamespaceSelector = fields.ParseSelectorOrDie("status.phase!=Terminating") + protectedCopiedCSVNamespaces = map[string]struct{}{} ) BeforeEach(func() { @@ -95,7 +103,6 @@ var _ = Describe("Disabling copied CSVs", func() { }) When("Copied CSVs are disabled", func() { - BeforeEach(func() { Eventually(func() error { var olmConfig operatorsv1.OLMConfig @@ -122,6 +129,10 @@ var _ = Describe("Disabling copied CSVs", func() { return nil }).Should(Succeed()) + + Eventually(func() error { + return setProtectedCopiedCSVNamespaces(protectedCopiedCSVNamespaces) + }).Should(Succeed()) }) It("should not have any copied CSVs", func() { @@ -139,8 +150,14 @@ var _ = Describe("Disabling copied CSVs", func() { return err } - if numCSVs := len(copiedCSVs.Items); numCSVs != 0 { - return fmt.Errorf("Found %d copied CSVs, should be 0", numCSVs) + if numCSVs := len(copiedCSVs.Items); numCSVs != len(protectedCopiedCSVNamespaces) { + return fmt.Errorf("Found %d copied CSVs, should be %d", numCSVs, len(protectedCopiedCSVNamespaces)) + } + + for _, csv := range copiedCSVs.Items { + if _, ok := protectedCopiedCSVNamespaces[csv.GetNamespace()]; !ok { + return fmt.Errorf("copied CSV %s/%s should not exist in the given namespace", csv.GetNamespace(), csv.GetName()) + } } return nil }).Should(Succeed()) @@ -159,8 +176,8 @@ var _ = Describe("Disabling copied CSVs", func() { } expectedCondition := metav1.Condition{ - Reason: "NoCopiedCSVsFound", - Message: "Copied CSVs are disabled and none were found for operators installed in AllNamespace mode", + Reason: "CopiedCSVsDisabled", + Message: "Copied CSVs are disabled and no unexpected copied CSVs were found for operators installed in AllNamespace mode", Status: metav1.ConditionTrue, } @@ -260,3 +277,31 @@ var _ = Describe("Disabling copied CSVs", func() { }) }) }) + +func setProtectedCopiedCSVNamespaces(protectedCopiedCSVNamespaces map[string]struct{}) error { + var olmDeployment appsv1.Deployment + if err := ctx.Ctx().Client().Get(context.TODO(), apitypes.NamespacedName{Name: olmDeploymentName, Namespace: operatorNamespace}, &olmDeployment); err != nil { + return err + } + + if protectedNamespaceArgument := getRuntimeFlagValue(&olmDeployment, olmDeploymentName, protectedCopiedCSVNamespacesRuntimeFlag); protectedNamespaceArgument != "" { + for _, namespace := range strings.Split(protectedNamespaceArgument, ",") { + protectedCopiedCSVNamespaces[namespace] = struct{}{} + } + } + + return nil +} + +func getRuntimeFlagValue(deployment *appsv1.Deployment, containerName string, runtimeFlag string) string { + for _, container := range deployment.Spec.Template.Spec.Containers { + if container.Name == containerName { + for i := range container.Args { + if container.Args[i] == runtimeFlag && len(container.Args) > i+1 { + return container.Args[i+1] + } + } + } + } + return "" +} diff --git a/vendor/github.com/operator-framework/operator-lifecycle-manager/cmd/olm/main.go b/vendor/github.com/operator-framework/operator-lifecycle-manager/cmd/olm/main.go index 7786f5b823..fd063ffe56 100644 --- a/vendor/github.com/operator-framework/operator-lifecycle-manager/cmd/olm/main.go +++ b/vendor/github.com/operator-framework/operator-lifecycle-manager/cmd/olm/main.go @@ -60,6 +60,9 @@ var ( tlsKeyPath = pflag.String( "tls-key", "", "Path to use for private key (requires tls-cert)") + protectedCopiedCSVNamespaces = pflag.String("protectedCopiedCSVNamespaces", + "", "A comma-delimited set of namespaces where global Copied CSVs will always appear, even if Copied CSVs are disabled") + tlsCertPath = pflag.String( "tls-cert", "", "Path to use for certificate key (requires tls-key)") @@ -162,6 +165,7 @@ func main() { olm.WithOperatorClient(opClient), olm.WithRestConfig(config), olm.WithConfigClient(versionedConfigClient), + olm.WithProtectedCopiedCSVNamespaces(*protectedCopiedCSVNamespaces), ) if err != nil { logger.WithError(err).Fatal("error configuring operator") diff --git a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/olm/config.go b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/olm/config.go index adcee6cdce..b56e27358e 100644 --- a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/olm/config.go +++ b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/olm/config.go @@ -1,6 +1,7 @@ package olm import ( + "strings" "time" "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/queueinformer" @@ -21,18 +22,19 @@ import ( type OperatorOption func(*operatorConfig) type operatorConfig struct { - resyncPeriod func() time.Duration - operatorNamespace string - watchedNamespaces []string - clock utilclock.Clock - logger *logrus.Logger - operatorClient operatorclient.ClientInterface - externalClient versioned.Interface - strategyResolver install.StrategyResolverInterface - apiReconciler APIIntersectionReconciler - apiLabeler labeler.Labeler - restConfig *rest.Config - configClient configv1client.Interface + protectedCopiedCSVNamespaces map[string]struct{} + resyncPeriod func() time.Duration + operatorNamespace string + watchedNamespaces []string + clock utilclock.Clock + logger *logrus.Logger + operatorClient operatorclient.ClientInterface + externalClient versioned.Interface + strategyResolver install.StrategyResolverInterface + apiReconciler APIIntersectionReconciler + apiLabeler labeler.Labeler + restConfig *rest.Config + configClient configv1client.Interface } func (o *operatorConfig) apply(options []OperatorOption) { @@ -77,14 +79,15 @@ func (o *operatorConfig) validate() (err error) { func defaultOperatorConfig() *operatorConfig { return &operatorConfig{ - resyncPeriod: queueinformer.ResyncWithJitter(30*time.Second, 0.2), - operatorNamespace: "default", - watchedNamespaces: []string{metav1.NamespaceAll}, - clock: utilclock.RealClock{}, - logger: logrus.New(), - strategyResolver: &install.StrategyResolver{}, - apiReconciler: APIIntersectionReconcileFunc(ReconcileAPIIntersection), - apiLabeler: labeler.Func(LabelSetsFor), + resyncPeriod: queueinformer.ResyncWithJitter(30*time.Second, 0.2), + operatorNamespace: "default", + watchedNamespaces: []string{metav1.NamespaceAll}, + clock: utilclock.RealClock{}, + logger: logrus.New(), + strategyResolver: &install.StrategyResolver{}, + apiReconciler: APIIntersectionReconcileFunc(ReconcileAPIIntersection), + apiLabeler: labeler.Func(LabelSetsFor), + protectedCopiedCSVNamespaces: map[string]struct{}{}, } } @@ -112,6 +115,18 @@ func WithLogger(logger *logrus.Logger) OperatorOption { } } +func WithProtectedCopiedCSVNamespaces(namespaces string) OperatorOption { + return func(config *operatorConfig) { + if namespaces == "" { + return + } + + for _, ns := range strings.Split(namespaces, ",") { + config.protectedCopiedCSVNamespaces[ns] = struct{}{} + } + } +} + func WithClock(clock utilclock.Clock) OperatorOption { return func(config *operatorConfig) { config.clock = clock diff --git a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/olm/operator.go b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/olm/operator.go index b6c3d2abc1..d33945aee8 100644 --- a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/olm/operator.go +++ b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/olm/operator.go @@ -63,32 +63,33 @@ var ( type Operator struct { queueinformer.Operator - clock utilclock.Clock - logger *logrus.Logger - opClient operatorclient.ClientInterface - client versioned.Interface - lister operatorlister.OperatorLister - copiedCSVLister operatorsv1alpha1listers.ClusterServiceVersionLister - ogQueueSet *queueinformer.ResourceQueueSet - csvQueueSet *queueinformer.ResourceQueueSet - olmConfigQueue workqueue.RateLimitingInterface - csvCopyQueueSet *queueinformer.ResourceQueueSet - copiedCSVGCQueueSet *queueinformer.ResourceQueueSet - objGCQueueSet *queueinformer.ResourceQueueSet - nsQueueSet workqueue.RateLimitingInterface - apiServiceQueue workqueue.RateLimitingInterface - csvIndexers map[string]cache.Indexer - recorder record.EventRecorder - resolver install.StrategyResolverInterface - apiReconciler APIIntersectionReconciler - apiLabeler labeler.Labeler - csvSetGenerator csvutility.SetGenerator - csvReplaceFinder csvutility.ReplaceFinder - csvNotification csvutility.WatchNotification - serviceAccountSyncer *scoped.UserDefinedServiceAccountSyncer - clientAttenuator *scoped.ClientAttenuator - serviceAccountQuerier *scoped.UserDefinedServiceAccountQuerier - clientFactory clients.Factory + clock utilclock.Clock + logger *logrus.Logger + opClient operatorclient.ClientInterface + client versioned.Interface + lister operatorlister.OperatorLister + protectedCopiedCSVNamespaces map[string]struct{} + copiedCSVLister operatorsv1alpha1listers.ClusterServiceVersionLister + ogQueueSet *queueinformer.ResourceQueueSet + csvQueueSet *queueinformer.ResourceQueueSet + olmConfigQueue workqueue.RateLimitingInterface + csvCopyQueueSet *queueinformer.ResourceQueueSet + copiedCSVGCQueueSet *queueinformer.ResourceQueueSet + objGCQueueSet *queueinformer.ResourceQueueSet + nsQueueSet workqueue.RateLimitingInterface + apiServiceQueue workqueue.RateLimitingInterface + csvIndexers map[string]cache.Indexer + recorder record.EventRecorder + resolver install.StrategyResolverInterface + apiReconciler APIIntersectionReconciler + apiLabeler labeler.Labeler + csvSetGenerator csvutility.SetGenerator + csvReplaceFinder csvutility.ReplaceFinder + csvNotification csvutility.WatchNotification + serviceAccountSyncer *scoped.UserDefinedServiceAccountSyncer + clientAttenuator *scoped.ClientAttenuator + serviceAccountQuerier *scoped.UserDefinedServiceAccountQuerier + clientFactory clients.Factory } func NewOperator(ctx context.Context, options ...OperatorOption) (*Operator, error) { @@ -121,30 +122,31 @@ func newOperatorWithConfig(ctx context.Context, config *operatorConfig) (*Operat } op := &Operator{ - Operator: queueOperator, - clock: config.clock, - logger: config.logger, - opClient: config.operatorClient, - client: config.externalClient, - ogQueueSet: queueinformer.NewEmptyResourceQueueSet(), - csvQueueSet: queueinformer.NewEmptyResourceQueueSet(), - olmConfigQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "olmConfig"), - csvCopyQueueSet: queueinformer.NewEmptyResourceQueueSet(), - copiedCSVGCQueueSet: queueinformer.NewEmptyResourceQueueSet(), - objGCQueueSet: queueinformer.NewEmptyResourceQueueSet(), - apiServiceQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "apiservice"), - resolver: config.strategyResolver, - apiReconciler: config.apiReconciler, - lister: lister, - recorder: eventRecorder, - apiLabeler: config.apiLabeler, - csvIndexers: map[string]cache.Indexer{}, - csvSetGenerator: csvutility.NewSetGenerator(config.logger, lister), - csvReplaceFinder: csvutility.NewReplaceFinder(config.logger, config.externalClient), - serviceAccountSyncer: scoped.NewUserDefinedServiceAccountSyncer(config.logger, scheme, config.operatorClient, config.externalClient), - clientAttenuator: scoped.NewClientAttenuator(config.logger, config.restConfig, config.operatorClient), - serviceAccountQuerier: scoped.NewUserDefinedServiceAccountQuerier(config.logger, config.externalClient), - clientFactory: clients.NewFactory(config.restConfig), + Operator: queueOperator, + clock: config.clock, + logger: config.logger, + opClient: config.operatorClient, + client: config.externalClient, + ogQueueSet: queueinformer.NewEmptyResourceQueueSet(), + csvQueueSet: queueinformer.NewEmptyResourceQueueSet(), + olmConfigQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "olmConfig"), + csvCopyQueueSet: queueinformer.NewEmptyResourceQueueSet(), + copiedCSVGCQueueSet: queueinformer.NewEmptyResourceQueueSet(), + objGCQueueSet: queueinformer.NewEmptyResourceQueueSet(), + apiServiceQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "apiservice"), + resolver: config.strategyResolver, + apiReconciler: config.apiReconciler, + lister: lister, + recorder: eventRecorder, + apiLabeler: config.apiLabeler, + csvIndexers: map[string]cache.Indexer{}, + csvSetGenerator: csvutility.NewSetGenerator(config.logger, lister), + csvReplaceFinder: csvutility.NewReplaceFinder(config.logger, config.externalClient), + serviceAccountSyncer: scoped.NewUserDefinedServiceAccountSyncer(config.logger, scheme, config.operatorClient, config.externalClient), + clientAttenuator: scoped.NewClientAttenuator(config.logger, config.restConfig, config.operatorClient), + serviceAccountQuerier: scoped.NewUserDefinedServiceAccountQuerier(config.logger, config.externalClient), + clientFactory: clients.NewFactory(config.restConfig), + protectedCopiedCSVNamespaces: config.protectedCopiedCSVNamespaces, } // Set up syncing for namespace-scoped resources @@ -1299,10 +1301,14 @@ func (a *Operator) syncOLMConfig(obj interface{}) (syncError error) { return err } - // Filter to unique copies - uniqueCopiedCSVs := map[string]struct{}{} + // Create a map that points from CSV name to a map of namespaces it is copied to + // for quick lookups. + copiedCSVNamespaces := map[string]map[string]struct{}{} for _, copiedCSV := range copiedCSVs { - uniqueCopiedCSVs[copiedCSV.GetName()] = struct{}{} + if _, ok := copiedCSVNamespaces[copiedCSV.GetName()]; !ok { + copiedCSVNamespaces[copiedCSV.GetName()] = map[string]struct{}{} + } + copiedCSVNamespaces[copiedCSV.GetName()][copiedCSV.GetNamespace()] = struct{}{} } csvs, err := a.lister.OperatorsV1alpha1().ClusterServiceVersionLister().ClusterServiceVersions(og.GetNamespace()).List(labels.NewSelector().Add(*nonCopiedCSVRequirement)) @@ -1310,9 +1316,16 @@ func (a *Operator) syncOLMConfig(obj interface{}) (syncError error) { return err } + namespaces, err := a.lister.CoreV1().NamespaceLister().List(labels.Everything()) + if err != nil { + return err + } + + copiedCSVEvaluatorFunc := getCopiedCSVEvaluatorFunc(olmConfig.CopiedCSVsAreEnabled(), namespaces, a.protectedCopiedCSVNamespaces) + for _, csv := range csvs { - // If the correct number of copied CSVs were found, continue - if _, ok := uniqueCopiedCSVs[csv.GetName()]; ok == olmConfig.CopiedCSVsAreEnabled() { + // Ignore NS where actual CSV is installed + if copiedCSVEvaluatorFunc(copiedCSVNamespaces[csv.GetName()]) { continue } @@ -1324,7 +1337,7 @@ func (a *Operator) syncOLMConfig(obj interface{}) (syncError error) { } // Update the olmConfig status if it has changed. - condition := getCopiedCSVsCondition(!olmConfig.CopiedCSVsAreEnabled(), csvIsRequeued) + condition := getCopiedCSVsCondition(olmConfig.CopiedCSVsAreEnabled(), csvIsRequeued) if !isStatusConditionPresentAndAreTypeReasonMessageStatusEqual(olmConfig.Status.Conditions, condition) { meta.SetStatusCondition(&olmConfig.Status.Conditions, condition) if _, err := a.client.OperatorsV1().OLMConfigs().UpdateStatus(context.TODO(), olmConfig, metav1.UpdateOptions{}); err != nil { @@ -1335,6 +1348,37 @@ func (a *Operator) syncOLMConfig(obj interface{}) (syncError error) { return nil } +// getCopiedCSVEvaluatorFunc returns a function that evaluates if the a set of Copied CSVs exist in the expected namespaces. +func getCopiedCSVEvaluatorFunc(copiedCSVsEnabled bool, namespaces []*corev1.Namespace, protectedCopiedCSVNamespaces map[string]struct{}) func(map[string]struct{}) bool { + if copiedCSVsEnabled { + // Exclude the namespace hosting the original CSV + expectedCopiedCSVCount := -1 + for _, ns := range namespaces { + if ns.Status.Phase == corev1.NamespaceActive { + expectedCopiedCSVCount++ + } + } + return func(m map[string]struct{}) bool { + return expectedCopiedCSVCount == len(m) + } + } + + // Check that Copied CSVs exist in protected namespaces. + return func(m map[string]struct{}) bool { + if len(protectedCopiedCSVNamespaces) != len(m) { + return false + } + + for protectedNS := range protectedCopiedCSVNamespaces { + if _, ok := m[protectedNS]; !ok { + return false + } + } + + return true + } +} + func isStatusConditionPresentAndAreTypeReasonMessageStatusEqual(conditions []metav1.Condition, condition metav1.Condition) bool { foundCondition := meta.FindStatusCondition(conditions, condition.Type) if foundCondition == nil { @@ -1346,13 +1390,13 @@ func isStatusConditionPresentAndAreTypeReasonMessageStatusEqual(conditions []met foundCondition.Status == condition.Status } -func getCopiedCSVsCondition(isDisabled, csvIsRequeued bool) metav1.Condition { +func getCopiedCSVsCondition(enabled, csvIsRequeued bool) metav1.Condition { condition := metav1.Condition{ Type: operatorsv1.DisabledCopiedCSVsConditionType, LastTransitionTime: metav1.Now(), Status: metav1.ConditionFalse, } - if !isDisabled { + if enabled { condition.Reason = "CopiedCSVsEnabled" condition.Message = "Copied CSVs are enabled and present across the cluster" if csvIsRequeued { @@ -1361,15 +1405,14 @@ func getCopiedCSVsCondition(isDisabled, csvIsRequeued bool) metav1.Condition { return condition } + condition.Reason = "CopiedCSVsDisabled" if csvIsRequeued { - condition.Reason = "CopiedCSVsFound" - condition.Message = "Copied CSVs are disabled and at least one copied CSV was found for an operator installed in AllNamespace mode" + condition.Message = "Copied CSVs are disabled and at least one unexpected copied CSV was found for an operator installed in AllNamespace mode" return condition } condition.Status = metav1.ConditionTrue - condition.Reason = "NoCopiedCSVsFound" - condition.Message = "Copied CSVs are disabled and none were found for operators installed in AllNamespace mode" + condition.Message = "Copied CSVs are disabled and no unexpected copied CSVs were found for operators installed in AllNamespace mode" return condition } @@ -1444,7 +1487,24 @@ func (a *Operator) syncCopyCSV(obj interface{}) (syncError error) { return err } + // Ensure that the Copied CSVs exist in the protected namespaces. + protectedNamespaces := []string{} + for ns := range a.protectedCopiedCSVNamespaces { + if ns == clusterServiceVersion.GetNamespace() { + continue + } + protectedNamespaces = append(protectedNamespaces, ns) + } + + if err := a.ensureCSVsInNamespaces(clusterServiceVersion, operatorGroup, NewNamespaceSet(protectedNamespaces)); err != nil { + return err + } + + // Delete Copied CSVs in namespaces that are not protected. for _, copiedCSV := range copiedCSVs { + if _, ok := a.protectedCopiedCSVNamespaces[copiedCSV.Namespace]; ok { + continue + } err := a.client.OperatorsV1alpha1().ClusterServiceVersions(copiedCSV.Namespace).Delete(context.TODO(), copiedCSV.Name, metav1.DeleteOptions{}) if err != nil && !apierrors.IsNotFound(err) { return err From c2ec825a57cd73e56247f0cb19580efc95463c36 Mon Sep 17 00:00:00 2001 From: Alexander Greene Date: Thu, 23 Jun 2022 12:42:37 -0700 Subject: [PATCH 2/2] Protect Global Copied CSVs in the openshift namespace Problem: After disabling Copied CSVs, the openshift console can no longer communicate to users which operators are available globally across the cluster. Solution: Configure OLM to ensure that Copied CSVs for operators scoped to All Namespaces appear in the openshift namespace. Update the OLM manifests to include RBAC for authenticated users to view CSVs in this namespace. --- ...operator.deployment.ibm-cloud-managed.yaml | 2 ++ ...000_50_olm_07-olm-operator.deployment.yaml | 2 ++ manifests/0000_50_olm_15-csv-viewer.rbac.yaml | 36 +++++++++++++++++++ scripts/generate_crds_manifests.sh | 35 ++++++++++++++++++ scripts/olm-deployment.patch.yaml | 8 +++++ 5 files changed, 83 insertions(+) create mode 100644 manifests/0000_50_olm_15-csv-viewer.rbac.yaml diff --git a/manifests/0000_50_olm_07-olm-operator.deployment.ibm-cloud-managed.yaml b/manifests/0000_50_olm_07-olm-operator.deployment.ibm-cloud-managed.yaml index bfe0877910..2bc94284f2 100644 --- a/manifests/0000_50_olm_07-olm-operator.deployment.ibm-cloud-managed.yaml +++ b/manifests/0000_50_olm_07-olm-operator.deployment.ibm-cloud-managed.yaml @@ -53,6 +53,8 @@ spec: - /srv-cert/tls.key - --client-ca - /profile-collector-cert/tls.crt + - --protectedCopiedCSVNamespaces + - openshift image: quay.io/operator-framework/olm@sha256:de396b540b82219812061d0d753440d5655250c621c753ed1dc67d6154741607 imagePullPolicy: IfNotPresent ports: diff --git a/manifests/0000_50_olm_07-olm-operator.deployment.yaml b/manifests/0000_50_olm_07-olm-operator.deployment.yaml index c4b5816d0c..5a7189055f 100644 --- a/manifests/0000_50_olm_07-olm-operator.deployment.yaml +++ b/manifests/0000_50_olm_07-olm-operator.deployment.yaml @@ -53,6 +53,8 @@ spec: - /srv-cert/tls.key - --client-ca - /profile-collector-cert/tls.crt + - --protectedCopiedCSVNamespaces + - openshift image: quay.io/operator-framework/olm@sha256:de396b540b82219812061d0d753440d5655250c621c753ed1dc67d6154741607 imagePullPolicy: IfNotPresent ports: diff --git a/manifests/0000_50_olm_15-csv-viewer.rbac.yaml b/manifests/0000_50_olm_15-csv-viewer.rbac.yaml new file mode 100644 index 0000000000..41520f7194 --- /dev/null +++ b/manifests/0000_50_olm_15-csv-viewer.rbac.yaml @@ -0,0 +1,36 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + annotations: + rbac.authorization.kubernetes.io/autoupdate: "true" + include.release.openshift.io/ibm-cloud-managed: "true" + include.release.openshift.io/self-managed-high-availability: "true" + name: copied-csv-viewer + namespace: openshift +rules: + - apiGroups: + - "operators.coreos.com" + resources: + - "clusterserviceversions" + verbs: + - get + - list + - watch +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + annotations: + rbac.authorization.kubernetes.io/autoupdate: "true" + include.release.openshift.io/ibm-cloud-managed: "true" + include.release.openshift.io/self-managed-high-availability: "true" + name: copied-csv-viewers + namespace: openshift +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: copied-csv-viewer +subjects: + - apiGroup: rbac.authorization.k8s.io + kind: Group + name: system:authenticated diff --git a/scripts/generate_crds_manifests.sh b/scripts/generate_crds_manifests.sh index 09c60e5896..193837b6cd 100755 --- a/scripts/generate_crds_manifests.sh +++ b/scripts/generate_crds_manifests.sh @@ -363,6 +363,41 @@ metadata: release.openshift.io/delete: "true" EOF +cat << EOF > manifests/0000_50_olm_15-csv-viewer.rbac.yaml +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + annotations: + rbac.authorization.kubernetes.io/autoupdate: "true" + name: copied-csv-viewer + namespace: openshift +rules: +- apiGroups: + - "operators.coreos.com" + resources: + - "clusterserviceversions" + verbs: + - get + - list + - watch +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + annotations: + rbac.authorization.kubernetes.io/autoupdate: "true" + name: copied-csv-viewers + namespace: openshift +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: copied-csv-viewer +subjects: +- apiGroup: rbac.authorization.k8s.io + kind: Group + name: system:authenticated +EOF + add_ibm_managed_cloud_annotations "${ROOT_DIR}/manifests" find "${ROOT_DIR}/manifests" -type f -exec $SED -i "/^#/d" {} \; diff --git a/scripts/olm-deployment.patch.yaml b/scripts/olm-deployment.patch.yaml index de1abc8eaa..815c989469 100644 --- a/scripts/olm-deployment.patch.yaml +++ b/scripts/olm-deployment.patch.yaml @@ -9,6 +9,14 @@ value: name: RELEASE_VERSION value: "0.0.1-snapshot" +- command: update + path: spec.template.spec.containers[0].args[+] + value: + --protectedCopiedCSVNamespaces +- command: update + path: spec.template.spec.containers[0].args[+] + value: + openshift - command: update path: spec.template.spec.containers[*].securityContext value: