pkg/controller: label RBAC with content hash (#3034)

When a CSV is processed, it is assumed that the InstallPlan has already run, or that a user that's creating a CSV as their entrypoint into the system has otherwise met all the preconditions for the CSV to exist. As part of validating these preconditions, the CSV logic today uses cluster-scoped listers for all RBAC resources. sets up an in-memory authorizer for these, and queries the CSV install strategie's permissions against those. We would like to restrict the amount of memory OLM uses, and part of that is not caching the world. For the above approach to work, all RBAC objects fulfilling CSV permission preconditions would need to be labelled. In the case that a user is creating a CSV manually, this will not be the case. We can use the SubjectAccessReview API to check for the presence of permissions without caching the world, but since a PolicyRule has slices of verbs, resources, subjects, etc and the SAR endpoint accepts but one of each, there will be (in the general case) a combinatorical explosion of calls to issue enough SARs to cover the full set of permissions. Therefore, we need to limit the amount of times we take that action. A simple optimization is to check for permissions created directly by OLM, as that's by far the most common entrypoint into the system (a user creates a Subscription, that triggers an InstallPlan, which creates the RBAC). As OLM chose to name the RBAC objects with random strings of characters, it's not possible to look at a list of permissions in a CSV and know which resources OLM would have created. Therefore, this PR adds a label to all relevant RBAC resources with the hash of their content. We already have the name of the CSV, but since CSV content is ostensibly mutable, this is not enough. Signed-off-by: Steve Kuznetsov <[email protected]>
operator-framework · Sep 18, 2023 · 8eb4f3e · 8eb4f3e
1 parent 46f4f89
commit 8eb4f3e
Show file tree

Hide file tree

Showing 6 changed files with 183 additions and 8 deletions.
diff --git a/pkg/controller/operators/catalog/operator.go b/pkg/controller/operators/catalog/operator.go
@@ -108,7 +108,6 @@ type Operator struct {
 	client                   versioned.Interface
 	dynamicClient            dynamic.Interface
 	lister                   operatorlister.OperatorLister
-	k8sLabelQueueSets        map[schema.GroupVersionResource]workqueue.RateLimitingInterface
 	catsrcQueueSet           *queueinformer.ResourceQueueSet
 	subQueueSet              *queueinformer.ResourceQueueSet
 	ipQueueSet               *queueinformer.ResourceQueueSet
@@ -202,7 +201,6 @@ func NewOperator(ctx context.Context, kubeconfigPath string, clock utilclock.Clo
 		lister:                   lister,
 		namespace:                operatorNamespace,
 		recorder:                 eventRecorder,
-		k8sLabelQueueSets:        map[schema.GroupVersionResource]workqueue.RateLimitingInterface{},
 		catsrcQueueSet:           queueinformer.NewEmptyResourceQueueSet(),
 		subQueueSet:              queueinformer.NewEmptyResourceQueueSet(),
 		ipQueueSet:               queueinformer.NewEmptyResourceQueueSet(),
@@ -386,11 +384,12 @@ func NewOperator(ctx context.Context, kubeconfigPath string, clock utilclock.Clo
 		if canFilter {
 			return nil
 		}
-		op.k8sLabelQueueSets[gvr] = workqueue.NewRateLimitingQueueWithConfig(workqueue.DefaultControllerRateLimiter(), workqueue.RateLimitingQueueConfig{
+		queue := workqueue.NewRateLimitingQueueWithConfig(workqueue.DefaultControllerRateLimiter(), workqueue.RateLimitingQueueConfig{
 			Name: gvr.String(),
 		})
 		queueInformer, err := queueinformer.NewQueueInformer(
 			ctx,
+			queueinformer.WithQueue(queue),
 			queueinformer.WithLogger(op.logger),
 			queueinformer.WithInformer(informer),
 			queueinformer.WithSyncer(sync.ToSyncer()),
@@ -416,6 +415,18 @@ func NewOperator(ctx context.Context, kubeconfigPath string, clock utilclock.Clo
 	)); err != nil {
 		return nil, err
 	}
+	if err := labelObjects(rolesgvk, roleInformer.Informer(), labeller.ContentHashLabeler[*rbacv1.Role, *rbacv1applyconfigurations.RoleApplyConfiguration](
+		ctx, op.logger, labeller.ContentHashFilter,
+		func(role *rbacv1.Role) (string, error) {
+			return resolver.PolicyRuleHashLabelValue(role.Rules)
+		},
+		rbacv1applyconfigurations.Role,
+		func(namespace string, ctx context.Context, cfg *rbacv1applyconfigurations.RoleApplyConfiguration, opts metav1.ApplyOptions) (*rbacv1.Role, error) {
+			return op.opClient.KubernetesInterface().RbacV1().Roles(namespace).Apply(ctx, cfg, opts)
+		},
+	)); err != nil {
+		return nil, err
+	}
 
 	// Wire RoleBindings
 	roleBindingInformer := k8sInformerFactory.Rbac().V1().RoleBindings()
@@ -432,6 +443,18 @@ func NewOperator(ctx context.Context, kubeconfigPath string, clock utilclock.Clo
 	)); err != nil {
 		return nil, err
 	}
+	if err := labelObjects(rolebindingsgvk, roleBindingInformer.Informer(), labeller.ContentHashLabeler[*rbacv1.RoleBinding, *rbacv1applyconfigurations.RoleBindingApplyConfiguration](
+		ctx, op.logger, labeller.ContentHashFilter,
+		func(roleBinding *rbacv1.RoleBinding) (string, error) {
+			return resolver.RoleReferenceAndSubjectHashLabelValue(roleBinding.RoleRef, roleBinding.Subjects)
+		},
+		rbacv1applyconfigurations.RoleBinding,
+		func(namespace string, ctx context.Context, cfg *rbacv1applyconfigurations.RoleBindingApplyConfiguration, opts metav1.ApplyOptions) (*rbacv1.RoleBinding, error) {
+			return op.opClient.KubernetesInterface().RbacV1().RoleBindings(namespace).Apply(ctx, cfg, opts)
+		},
+	)); err != nil {
+		return nil, err
+	}
 
 	// Wire ServiceAccounts
 	serviceAccountInformer := k8sInformerFactory.Core().V1().ServiceAccounts()

diff --git a/pkg/controller/operators/labeller/filters.go b/pkg/controller/operators/labeller/filters.go
@@ -21,6 +21,10 @@ import (
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/internal/alongside"
 )
 
+func ContentHashFilter(object metav1.Object) bool {
+	return HasOLMOwnerRef(object) && !hasHashLabel(object)
+}
+
 func Filter(gvr schema.GroupVersionResource) func(metav1.Object) bool {
 	if f, ok := filters[gvr]; ok {
 		return f
@@ -80,6 +84,18 @@ func Validate(ctx context.Context, logger *logrus.Logger, metadataClient metadat
 	allFilters[batchv1.SchemeGroupVersion.WithResource("jobs")] = JobFilter(func(namespace, name string) (metav1.Object, error) {
 		return metadataClient.Resource(corev1.SchemeGroupVersion.WithResource("configmaps")).Namespace(namespace).Get(ctx, name, metav1.GetOptions{})
 	})
+
+	for _, gvr := range []schema.GroupVersionResource{
+		rbacv1.SchemeGroupVersion.WithResource("roles"),
+		rbacv1.SchemeGroupVersion.WithResource("rolebindings"),
+		rbacv1.SchemeGroupVersion.WithResource("clusterroles"),
+		rbacv1.SchemeGroupVersion.WithResource("clusterrolebindings"),
+	} {
+		previous := allFilters[gvr]
+		allFilters[gvr] = func(object metav1.Object) bool {
+			return previous != nil && previous(object) && ContentHashFilter(object)
+		}
+	}
 	for gvr, filter := range allFilters {
 		gvr, filter := gvr, filter
 		g.Go(func() error {

diff --git a/pkg/controller/operators/labeller/rbac.go b/pkg/controller/operators/labeller/rbac.go
@@ -0,0 +1,56 @@
+package labeller
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/operator-framework/api/pkg/operators/v1alpha1"
+	"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver"
+	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/ownerutil"
+	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/queueinformer"
+	"github.com/sirupsen/logrus"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func hasHashLabel(obj metav1.Object) bool {
+	_, ok := obj.GetLabels()[resolver.ContentHashLabelKey]
+	return ok
+}
+
+func ContentHashLabeler[T metav1.Object, A ApplyConfig[A]](
+	ctx context.Context,
+	logger *logrus.Logger,
+	check func(metav1.Object) bool,
+	hasher func(object T) (string, error),
+	applyConfigFor func(name, namespace string) A,
+	apply func(namespace string, ctx context.Context, cfg A, opts metav1.ApplyOptions) (T, error),
+) queueinformer.LegacySyncHandler {
+	return func(obj interface{}) error {
+		cast, ok := obj.(T)
+		if !ok {
+			err := fmt.Errorf("wrong type %T, expected %T: %#v", obj, new(T), obj)
+			logger.WithError(err).Error("casting failed")
+			return fmt.Errorf("casting failed: %w", err)
+		}
+
+		if _, _, ok := ownerutil.GetOwnerByKindLabel(cast, v1alpha1.ClusterServiceVersionKind); !ok {
+			return nil
+		}
+
+		if !check(cast) || hasHashLabel(cast) {
+			return nil
+		}
+
+		hash, err := hasher(cast)
+		if err != nil {
+			return fmt.Errorf("failed to calculate hash: %w", err)
+		}
+
+		cfg := applyConfigFor(cast.GetName(), cast.GetNamespace())
+		cfg.WithLabels(map[string]string{
+			resolver.ContentHashLabelKey: hash,
+		})
+		_, err = apply(cast.GetNamespace(), ctx, cfg, metav1.ApplyOptions{})
+		return err
+	}
+}
diff --git a/pkg/controller/operators/olm/operator.go b/pkg/controller/operators/olm/operator.go
@@ -78,7 +78,6 @@ type Operator struct {
 	opClient                     operatorclient.ClientInterface
 	client                       versioned.Interface
 	lister                       operatorlister.OperatorLister
-	k8sLabelQueueSets            map[schema.GroupVersionResource]workqueue.RateLimitingInterface
 	protectedCopiedCSVNamespaces map[string]struct{}
 	copiedCSVLister              metadatalister.Lister
 	ogQueueSet                   *queueinformer.ResourceQueueSet
@@ -162,7 +161,6 @@ func newOperatorWithConfig(ctx context.Context, config *operatorConfig) (*Operat
 		resolver:                     config.strategyResolver,
 		apiReconciler:                config.apiReconciler,
 		lister:                       lister,
-		k8sLabelQueueSets:            map[schema.GroupVersionResource]workqueue.RateLimitingInterface{},
 		recorder:                     eventRecorder,
 		apiLabeler:                   config.apiLabeler,
 		csvIndexers:                  map[string]cache.Indexer{},
@@ -453,11 +451,12 @@ func newOperatorWithConfig(ctx context.Context, config *operatorConfig) (*Operat
 		if canFilter {
 			return nil
 		}
-		op.k8sLabelQueueSets[gvr] = workqueue.NewRateLimitingQueueWithConfig(workqueue.DefaultControllerRateLimiter(), workqueue.RateLimitingQueueConfig{
+		queue := workqueue.NewRateLimitingQueueWithConfig(workqueue.DefaultControllerRateLimiter(), workqueue.RateLimitingQueueConfig{
 			Name: gvr.String(),
 		})
 		queueInformer, err := queueinformer.NewQueueInformer(
 			ctx,
+			queueinformer.WithQueue(queue),
 			queueinformer.WithLogger(op.logger),
 			queueinformer.WithInformer(informer),
 			queueinformer.WithSyncer(sync.ToSyncer()),
@@ -557,6 +556,20 @@ func newOperatorWithConfig(ctx context.Context, config *operatorConfig) (*Operat
 	)); err != nil {
 		return nil, err
 	}
+	if err := labelObjects(clusterrolesgvk, clusterRoleInformer.Informer(), labeller.ContentHashLabeler[*rbacv1.ClusterRole, *rbacv1applyconfigurations.ClusterRoleApplyConfiguration](
+		ctx, op.logger, labeller.ContentHashFilter,
+		func(clusterRole *rbacv1.ClusterRole) (string, error) {
+			return resolver.PolicyRuleHashLabelValue(clusterRole.Rules)
+		},
+		func(name, _ string) *rbacv1applyconfigurations.ClusterRoleApplyConfiguration {
+			return rbacv1applyconfigurations.ClusterRole(name)
+		},
+		func(_ string, ctx context.Context, cfg *rbacv1applyconfigurations.ClusterRoleApplyConfiguration, opts metav1.ApplyOptions) (*rbacv1.ClusterRole, error) {
+			return op.opClient.KubernetesInterface().RbacV1().ClusterRoles().Apply(ctx, cfg, opts)
+		},
+	)); err != nil {
+		return nil, err
+	}
 
 	clusterRoleBindingInformer := k8sInformerFactory.Rbac().V1().ClusterRoleBindings()
 	informersByNamespace[metav1.NamespaceAll].ClusterRoleBindingInformer = clusterRoleBindingInformer
@@ -586,6 +599,20 @@ func newOperatorWithConfig(ctx context.Context, config *operatorConfig) (*Operat
 	)); err != nil {
 		return nil, err
 	}
+	if err := labelObjects(clusterrolebindingssgvk, clusterRoleBindingInformer.Informer(), labeller.ContentHashLabeler[*rbacv1.ClusterRoleBinding, *rbacv1applyconfigurations.ClusterRoleBindingApplyConfiguration](
+		ctx, op.logger, labeller.ContentHashFilter,
+		func(clusterRoleBinding *rbacv1.ClusterRoleBinding) (string, error) {
+			return resolver.RoleReferenceAndSubjectHashLabelValue(clusterRoleBinding.RoleRef, clusterRoleBinding.Subjects)
+		},
+		func(name, _ string) *rbacv1applyconfigurations.ClusterRoleBindingApplyConfiguration {
+			return rbacv1applyconfigurations.ClusterRoleBinding(name)
+		},
+		func(_ string, ctx context.Context, cfg *rbacv1applyconfigurations.ClusterRoleBindingApplyConfiguration, opts metav1.ApplyOptions) (*rbacv1.ClusterRoleBinding, error) {
+			return op.opClient.KubernetesInterface().RbacV1().ClusterRoleBindings().Apply(ctx, cfg, opts)
+		},
+	)); err != nil {
+		return nil, err
+	}
 
 	// register namespace queueinformer
 	namespaceInformer := k8sInformerFactory.Core().V1().Namespaces()

diff --git a/pkg/controller/registry/resolver/rbac.go b/pkg/controller/registry/resolver/rbac.go
@@ -1,8 +1,11 @@
 package resolver
 
 import (
+	"crypto/sha256"
+	"encoding/json"
 	"fmt"
 	"hash/fnv"
+	"math/big"
 
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
@@ -62,6 +65,37 @@ func (o *OperatorPermissions) AddClusterRoleBinding(clusterRoleBinding *rbacv1.C
 	o.ClusterRoleBindings = append(o.ClusterRoleBindings, clusterRoleBinding)
 }
 
+const ContentHashLabelKey = "olm.permissions.hash"
+
+func PolicyRuleHashLabelValue(rules []rbacv1.PolicyRule) (string, error) {
+	raw, err := json.Marshal(rules)
+	if err != nil {
+		return "", err
+	}
+	return toBase62(sha256.Sum224(raw)), nil
+}
+
+func RoleReferenceAndSubjectHashLabelValue(roleRef rbacv1.RoleRef, subjects []rbacv1.Subject) (string, error) {
+	var container = struct {
+		RoleRef  rbacv1.RoleRef
+		Subjects []rbacv1.Subject
+	}{
+		RoleRef:  roleRef,
+		Subjects: subjects,
+	}
+	raw, err := json.Marshal(&container)
+	if err != nil {
+		return "", err
+	}
+	return toBase62(sha256.Sum224(raw)), nil
+}
+
+func toBase62(hash [28]byte) string {
+	var i big.Int
+	i.SetBytes(hash[:])
+	return i.Text(62)
+}
+
 func RBACForClusterServiceVersion(csv *v1alpha1.ClusterServiceVersion) (map[string]*OperatorPermissions, error) {
 	permissions := map[string]*OperatorPermissions{}
 
@@ -100,6 +134,11 @@ func RBACForClusterServiceVersion(csv *v1alpha1.ClusterServiceVersion) (map[stri
 			},
 			Rules: permission.Rules,
 		}
+		hash, err := PolicyRuleHashLabelValue(permission.Rules)
+		if err != nil {
+			return nil, fmt.Errorf("failed to hash permission rules: %w", err)
+		}
+		role.Labels[ContentHashLabelKey] = hash
 		permissions[permission.ServiceAccountName].AddRole(role)
 
 		// Create RoleBinding
@@ -120,6 +159,11 @@ func RBACForClusterServiceVersion(csv *v1alpha1.ClusterServiceVersion) (map[stri
 				Namespace: csv.GetNamespace(),
 			}},
 		}
+		hash, err = RoleReferenceAndSubjectHashLabelValue(roleBinding.RoleRef, roleBinding.Subjects)
+		if err != nil {
+			return nil, fmt.Errorf("failed to hash binding content: %w", err)
+		}
+		roleBinding.Labels[ContentHashLabelKey] = hash
 		permissions[permission.ServiceAccountName].AddRoleBinding(roleBinding)
 	}
 
@@ -142,6 +186,11 @@ func RBACForClusterServiceVersion(csv *v1alpha1.ClusterServiceVersion) (map[stri
 			},
 			Rules: permission.Rules,
 		}
+		hash, err := PolicyRuleHashLabelValue(permission.Rules)
+		if err != nil {
+			return nil, fmt.Errorf("failed to hash permission rules: %w", err)
+		}
+		role.Labels[ContentHashLabelKey] = hash
 		permissions[permission.ServiceAccountName].AddClusterRole(role)
 
 		// Create ClusterRoleBinding
@@ -162,6 +211,11 @@ func RBACForClusterServiceVersion(csv *v1alpha1.ClusterServiceVersion) (map[stri
 				Namespace: csv.GetNamespace(),
 			}},
 		}
+		hash, err = RoleReferenceAndSubjectHashLabelValue(roleBinding.RoleRef, roleBinding.Subjects)
+		if err != nil {
+			return nil, fmt.Errorf("failed to hash binding content: %w", err)
+		}
+		roleBinding.Labels[ContentHashLabelKey] = hash
 		permissions[permission.ServiceAccountName].AddClusterRoleBinding(roleBinding)
 	}
 	return permissions, nil

diff --git a/pkg/package-server/provider/registry_test.go b/pkg/package-server/provider/registry_test.go
@@ -787,7 +787,7 @@ func TestRegistryProviderList(t *testing.T) {
 			globalNS:         "ns",
 			requestNamespace: "wisconsin",
 			expectedErr:      "",
-			expected:         &operators.PackageManifestList{Items: []operators.PackageManifest{}},
+			expected:         &operators.PackageManifestList{},
 		},
 		{
 			name:     "PackagesFound",
@@ -1230,7 +1230,6 @@ func TestRegistryProviderList(t *testing.T) {
 			} else {
 				require.Nil(t, err)
 			}
-
 			require.Equal(t, len(test.expected.Items), len(packageManifestList.Items))
 			require.ElementsMatch(t, test.expected.Items, packageManifestList.Items)
 		})