Do not modify object from the lister cache

Problem: Objects retrieved from the lister should
not be modified.

Solution: In locations where an object originating from the lister
cache must be modified, modify a copy of the object instead.

Signed-off-by: Alexander Greene <[email protected]>

Author: awgreene

Diff for: pkg/controller/bundle/bundle_unpacker.go

@@ -646,6 +646,7 @@ func (c *ConfigMapUnpacker) ensureRole(cmRef *corev1.ObjectReference) (role *rba
 			return
 		}
 	}
+	role = role.DeepCopy()
 	role.Rules = append(role.Rules, rule)
 
 	role, err = c.client.RbacV1().Roles(role.GetNamespace()).Update(context.TODO(), role, metav1.UpdateOptions{})

Diff for: pkg/controller/install/apiservice.go

@@ -41,6 +41,7 @@ func (i *StrategyDeploymentInstaller) createOrUpdateAPIService(caPEM []byte, des
 		}
 		apiService.SetName(apiServiceName)
 	} else {
+		apiService = apiService.DeepCopy()
 		csv, ok := i.owner.(*v1alpha1.ClusterServiceVersion)
 		if !ok {
 			return fmt.Errorf("failed to typecast the APIService owner: APIServices require a CSV owner")

Diff for: pkg/controller/install/webhook.go

@@ -106,6 +106,7 @@ func (i *StrategyDeploymentInstaller) createOrUpdateMutatingWebhook(ogNamespacel
 		}
 	}
 	for _, webhook := range existingWebhooks.Items {
+		webhook = *webhook.DeepCopy()
 		// Update the list of webhooks
 		webhook.Webhooks = []admissionregistrationv1.MutatingWebhook{
 			desc.GetMutatingWebhook(i.owner.GetNamespace(), ogNamespacelabelSelector, caPEM),
@@ -154,6 +155,7 @@ func (i *StrategyDeploymentInstaller) createOrUpdateValidatingWebhook(ogNamespac
 		return nil
 	}
 	for _, webhook := range existingWebhooks.Items {
+		webhook = *webhook.DeepCopy()
 		// Update the list of webhooks
 		webhook.Webhooks = []admissionregistrationv1.ValidatingWebhook{
 			desc.GetValidatingWebhook(i.owner.GetNamespace(), ogNamespacelabelSelector, caPEM),

Diff for: pkg/controller/operators/olm/operator.go

@@ -842,6 +842,7 @@ func (a *Operator) syncObject(obj interface{}) (syncError error) {
 	if related {
 		csvList := a.csvSet(metaObj.GetNamespace(), v1alpha1.CSVPhaseFailed)
 		for _, csv := range csvList {
+			csv = csv.DeepCopy()
 			if csv.Status.Reason != v1alpha1.CSVReasonComponentFailedNoRetry {
 				continue
 			}
@@ -1238,7 +1239,7 @@ func (a *Operator) allNamespaceOperatorGroups() ([]*v1.OperatorGroup, error) {
 	result := []*v1.OperatorGroup{}
 	for _, operatorGroup := range operatorGroups {
 		if NewNamespaceSet(operatorGroup.Status.Namespaces).IsAllNamespaces() {
-			result = append(result, operatorGroup)
+			result = append(result, operatorGroup.DeepCopy())
 		}
 	}
 	return result, nil
@@ -1477,7 +1478,7 @@ func (a *Operator) getCopiedCSVDisabledEventsForCSV(csv *v1alpha1.ClusterService
 			event.InvolvedObject.Name == csv.GetName() &&
 			event.InvolvedObject.UID == csv.GetUID() &&
 			event.Reason == v1.DisabledCopiedCSVsConditionType {
-			result = append(result, event)
+			result = append(result, *event.DeepCopy())
 		}
 	}
 
@@ -1586,7 +1587,7 @@ func (a *Operator) operatorGroupFromAnnotations(logger *logrus.Entry, csv *v1alp
 		return nil
 	}
 
-	return operatorGroup
+	return operatorGroup.DeepCopy()
 }
 
 func (a *Operator) operatorGroupForCSV(csv *v1alpha1.ClusterServiceVersion, logger *logrus.Entry) (*v1.OperatorGroup, error) {
@@ -1627,7 +1628,7 @@ func (a *Operator) operatorGroupForCSV(csv *v1alpha1.ClusterServiceVersion, logg
 			return nil, nil
 		}
 		logger.Debug("csv in operatorgroup")
-		return operatorGroup, nil
+		return operatorGroup.DeepCopy(), nil
 	default:
 		err = fmt.Errorf("csv created in namespace with multiple operatorgroups, can't pick one automatically")
 		logger.WithError(err).Warn("csv failed to become an operatorgroup member")

Diff for: pkg/controller/operators/olm/operatorgroup.go

@@ -73,7 +73,7 @@ func (a *Operator) syncOperatorGroups(obj interface{}) error {
 
 	// Check if there is a stale multiple OG condition and clear it if existed.
 	if len(groups) == 1 {
-		og := groups[0]
+		og := groups[0].DeepCopy()
 		if c := meta.FindStatusCondition(og.Status.Conditions, v1.MutlipleOperatorGroupCondition); c != nil {
 			meta.RemoveStatusCondition(&og.Status.Conditions, v1.MutlipleOperatorGroupCondition)
 			if og.GetName() == op.GetName() {
@@ -93,7 +93,8 @@ func (a *Operator) syncOperatorGroups(obj interface{}) error {
 			Reason:  v1.MultipleOperatorGroupsReason,
 			Message: "Multiple OperatorGroup found in the same namespace",
 		}
-		for _, og := range groups {
+		for i := range groups {
+			og := groups[i].DeepCopy()
 			if c := meta.FindStatusCondition(og.Status.Conditions, v1.MutlipleOperatorGroupCondition); c != nil {
 				continue
 			}
@@ -115,11 +116,12 @@ func (a *Operator) syncOperatorGroups(obj interface{}) error {
 		return err
 	}
 	if op.Status.ServiceAccountRef != previousRef {
-		crdList, err := a.lister.OperatorsV1alpha1().ClusterServiceVersionLister().List(labels.Everything())
+		csvList, err := a.lister.OperatorsV1alpha1().ClusterServiceVersionLister().List(labels.Everything())
 		if err != nil {
 			return err
 		}
-		for _, csv := range crdList {
+		for i := range csvList {
+			csv := csvList[i].DeepCopy()
 			if group, ok := csv.GetAnnotations()[v1.OperatorGroupAnnotationKey]; !ok || group != op.GetName() {
 				continue
 			}