olm,catalog: only validate the resources we label

pkg/controller/operators/catalog/operator.go

@@ -186,7 +186,7 @@ func NewOperator(ctx context.Context, kubeconfigPath string, clock utilclock.Clo
 		return nil, err
 	}
 
-	canFilter, err := labeller.Validate(ctx, logger, metadataClient, crClient)
+	canFilter, err := labeller.Validate(ctx, logger, metadataClient, crClient, labeller.IdentityCatalogOperator)
 	if err != nil {
 		return nil, err
 	}
@@ -541,6 +541,49 @@ func NewOperator(ctx context.Context, kubeconfigPath string, clock utilclock.Clo
 		return nil, err
 	}
 
+	{
+		gvr := servicesgvk
+		informer := serviceInformer.Informer()
+
+		logger := op.logger.WithFields(logrus.Fields{"gvr": gvr.String()})
+		logger.Info("registering owner reference fixer")
+
+		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(queueinformer.LegacySyncHandler(func(obj interface{}) error {
+				service, ok := obj.(*corev1.Service)
+				if !ok {
+					err := fmt.Errorf("wrong type %T, expected %T: %#v", obj, new(*corev1.Service), obj)
+					logger.WithError(err).Error("casting failed")
+					return fmt.Errorf("casting failed: %w", err)
+				}
+
+				deduped := deduplicateOwnerReferences(service.OwnerReferences)
+				if len(deduped) != len(service.OwnerReferences) {
+					localCopy := service.DeepCopy()
+					localCopy.OwnerReferences = deduped
+					if _, err := op.opClient.KubernetesInterface().CoreV1().Services(service.Namespace).Update(ctx, localCopy, metav1.UpdateOptions{}); err != nil {
+						return err
+					}
+				}
+				return nil
+			}).ToSyncer()),
+		)
+		if err != nil {
+			return nil, err
+		}
+
+		if err := op.RegisterQueueInformer(queueInformer); err != nil {
+			return nil, err
+		}
+	}
+
 	// Wire Pods for CatalogSource
 	catsrcReq, err := labels.NewRequirement(reconciler.CatalogSourceLabelKey, selection.Exists, nil)
 	if err != nil {
@@ -2860,3 +2903,7 @@ func (o *Operator) apiresourceFromGVK(gvk schema.GroupVersionKind) (metav1.APIRe
 	logger.Info("couldn't find GVK in api discovery")
 	return metav1.APIResource{}, olmerrors.GroupVersionKindNotFoundError{Group: gvk.Group, Version: gvk.Version, Kind: gvk.Kind}
 }
+
+func deduplicateOwnerReferences(refs []metav1.OwnerReference) []metav1.OwnerReference {
+	return sets.New(refs...).UnsortedList()
+}

pkg/controller/operators/catalog/ownerrefs_test.go

@@ -0,0 +1,42 @@
+package catalog
+
+import (
+	"testing"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func TestDedupe(t *testing.T) {
+	yes := true
+	refs := []metav1.OwnerReference{
+		{
+			APIVersion:         "apiversion",
+			Kind:               "kind",
+			Name:               "name",
+			UID:                "uid",
+			Controller:         &yes,
+			BlockOwnerDeletion: &yes,
+		},
+		{
+			APIVersion:         "apiversion",
+			Kind:               "kind",
+			Name:               "name",
+			UID:                "uid",
+			Controller:         &yes,
+			BlockOwnerDeletion: &yes,
+		},
+		{
+			APIVersion:         "apiversion",
+			Kind:               "kind",
+			Name:               "name",
+			UID:                "uid",
+			Controller:         &yes,
+			BlockOwnerDeletion: &yes,
+		},
+	}
+	deduped := deduplicateOwnerReferences(refs)
+	t.Logf("got %d deduped from %d", len(deduped), len(refs))
+	if len(deduped) == len(refs) {
+		t.Errorf("didn't dedupe: %#v", deduped)
+	}
+}

pkg/controller/operators/labeller/filters.go

@@ -80,7 +80,39 @@ var filters = map[schema.GroupVersionResource]func(metav1.Object) bool{
 	},
 }
 
-func Validate(ctx context.Context, logger *logrus.Logger, metadataClient metadata.Interface, operatorClient operators.Interface) (bool, error) {
+type Identity string
+
+const (
+	IdentityCatalogOperator = "catalog-operator"
+	IdentityOLMOperator     = "olm-operator"
+)
+
+func gvrRelevant(identity Identity) func(schema.GroupVersionResource) bool {
+	switch identity {
+	case IdentityCatalogOperator:
+		return sets.New(
+			rbacv1.SchemeGroupVersion.WithResource("roles"),
+			rbacv1.SchemeGroupVersion.WithResource("rolebindings"),
+			corev1.SchemeGroupVersion.WithResource("serviceaccounts"),
+			corev1.SchemeGroupVersion.WithResource("services"),
+			corev1.SchemeGroupVersion.WithResource("pods"),
+			corev1.SchemeGroupVersion.WithResource("configmaps"),
+			batchv1.SchemeGroupVersion.WithResource("jobs"),
+			apiextensionsv1.SchemeGroupVersion.WithResource("customresourcedefinitions"),
+		).Has
+	case IdentityOLMOperator:
+		return sets.New(
+			appsv1.SchemeGroupVersion.WithResource("deployments"),
+			rbacv1.SchemeGroupVersion.WithResource("clusterroles"),
+			rbacv1.SchemeGroupVersion.WithResource("clusterrolebindings"),
+			apiextensionsv1.SchemeGroupVersion.WithResource("customresourcedefinitions"),
+		).Has
+	default:
+		panic("programmer error: invalid identity")
+	}
+}
+
+func Validate(ctx context.Context, logger *logrus.Logger, metadataClient metadata.Interface, operatorClient operators.Interface, identity Identity) (bool, error) {
 	okLock := sync.Mutex{}
 	ok := true
 	g, ctx := errgroup.WithContext(ctx)
@@ -125,6 +157,10 @@ func Validate(ctx context.Context, logger *logrus.Logger, metadataClient metadat
 	})
 
 	for gvr, filter := range allFilters {
+		if !gvrRelevant(identity)(gvr) {
+			logger.WithField("gvr", gvr.String()).Info("skipping irrelevant gvr")
+			continue
+		}
 		gvr, filter := gvr, filter
 		g.Go(func() error {
 			list, err := metadataClient.Resource(gvr).List(ctx, metav1.ListOptions{})

pkg/controller/operators/olm/operator.go

@@ -183,7 +183,7 @@ func newOperatorWithConfig(ctx context.Context, config *operatorConfig) (*Operat
 		return nil, err
 	}
 
-	canFilter, err := labeller.Validate(ctx, config.logger, config.metadataClient, config.externalClient)
+	canFilter, err := labeller.Validate(ctx, config.logger, config.metadataClient, config.externalClient, labeller.IdentityOLMOperator)
 	if err != nil {
 		return nil, err
 	}

pkg/lib/ownerutil/util.go

@@ -167,20 +167,13 @@ func AddNonBlockingOwner(object metav1.Object, owner Owner) {
 		ownerRefs = []metav1.OwnerReference{}
 	}
 
-	// Infer TypeMeta for the target
-	if err := InferGroupVersionKind(owner); err != nil {
-		log.Warn(err.Error())
-	}
-	gvk := owner.GetObjectKind().GroupVersionKind()
-
+	nonBlockingOwner := NonBlockingOwner(owner)
 	for _, item := range ownerRefs {
-		if item.Kind == gvk.Kind {
-			if item.Name == owner.GetName() && item.UID == owner.GetUID() {
-				return
-			}
+		if item.Kind == nonBlockingOwner.Kind && item.Name == nonBlockingOwner.Name && item.UID == nonBlockingOwner.UID {
+			return
 		}
 	}
-	ownerRefs = append(ownerRefs, NonBlockingOwner(owner))
+	ownerRefs = append(ownerRefs, nonBlockingOwner)
 	object.SetOwnerReferences(ownerRefs)
 }
 
@@ -284,14 +277,20 @@ func AddOwner(object metav1.Object, owner Owner, blockOwnerDeletion, isControlle
 	}
 	gvk := owner.GetObjectKind().GroupVersionKind()
 	apiVersion, kind := gvk.ToAPIVersionAndKind()
-	ownerRefs = append(ownerRefs, metav1.OwnerReference{
+	ownerRef := metav1.OwnerReference{
 		APIVersion:         apiVersion,
 		Kind:               kind,
 		Name:               owner.GetName(),
 		UID:                owner.GetUID(),
 		BlockOwnerDeletion: &blockOwnerDeletion,
 		Controller:         &isController,
-	})
+	}
+	for _, ref := range ownerRefs {
+		if ref.Kind == ownerRef.Kind && ref.Name == ownerRef.Name && ref.UID == ownerRef.UID {
+			return
+		}
+	}
+	ownerRefs = append(ownerRefs, ownerRef)
 	object.SetOwnerReferences(ownerRefs)
 }
 

pkg/lib/testobj/runtime.go

@@ -135,14 +135,21 @@ func WithOwner(owner, obj RuntimeMetaObject) RuntimeMetaObject {
 	out := obj.DeepCopyObject().(RuntimeMetaObject)
 	gvk := owner.GetObjectKind().GroupVersionKind()
 	apiVersion, kind := gvk.ToAPIVersionAndKind()
-	refs := append(out.GetOwnerReferences(), metav1.OwnerReference{
+
+	nonBlockingOwner := metav1.OwnerReference{
 		APIVersion:         apiVersion,
 		Kind:               kind,
 		Name:               owner.GetName(),
 		UID:                owner.GetUID(),
 		BlockOwnerDeletion: &dontBlockOwnerDeletion,
 		Controller:         &notController,
-	})
+	}
+	for _, item := range out.GetOwnerReferences() {
+		if item.Kind == nonBlockingOwner.Kind && item.Name == nonBlockingOwner.Name && item.UID == nonBlockingOwner.UID {
+			return out
+		}
+	}
+	refs := append(out.GetOwnerReferences(), nonBlockingOwner)
 	out.SetOwnerReferences(refs)
 
 	return out