Log 9591 Do not overwrite ConfigMaps owned by other resources

internal/collector/trust_bundle.go

@@ -8,6 +8,7 @@ import (
 	log "github.com/ViaQ/logerr/v2/log/static"
 	"github.com/openshift/cluster-logging-operator/internal/constants"
 	"github.com/openshift/cluster-logging-operator/internal/runtime"
+	"github.com/openshift/cluster-logging-operator/internal/utils"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/wait"
@@ -25,6 +26,9 @@ var (
 func ReconcileTrustedCABundleConfigMap(k8sClient client.Client, namespace, name string, owner metav1.OwnerReference) error {
 	cm := runtime.NewConfigMap(namespace, name, nil)
 	op, err := controllerutil.CreateOrUpdate(context.TODO(), k8sClient, cm, func() error {
+		if err := utils.EnsureCanUpdateOwnedResource(cm, owner); err != nil {
+			return err
+		}
 		cm.Labels = map[string]string{constants.InjectTrustedCABundleLabel: "true"}
 		cm.OwnerReferences = []metav1.OwnerReference{owner}
 		return nil

internal/reconcile/configmaps.go

@@ -13,6 +13,9 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
+// Configmap creates or updates a ConfigMap owned by the desired ownerReferences.
+// If a ConfigMap with the same name already exists and is not owned by the desired
+// owners, it is left unchanged and an error is returned (LOG-9591).
 func Configmap(k8Client client.Client, reader client.Reader, configMap *corev1.ConfigMap, opts ...comparators.ComparisonOption) error {
 	return retry.RetryOnConflict(retry.DefaultRetry, func() error {
 		current := &corev1.ConfigMap{}
@@ -23,14 +26,19 @@ func Configmap(k8Client client.Client, reader client.Reader, configMap *corev1.C
 			}
 			return fmt.Errorf("failed to get %v configmap: %v", key, err)
 		}
-		if configmaps.AreSame(current, configMap, opts...) && utils.HasSameOwner(current.OwnerReferences, configMap.OwnerReferences) {
+
+		if err := utils.EnsureCanUpdateOwnedResource(current, configMap.OwnerReferences...); err != nil {
+			return err
+		}
+
+		if configmaps.AreSame(current, configMap, opts...) {
 			return nil
-		} else {
-			current.Data = configMap.Data
-			current.Labels = configMap.Labels
-			current.Annotations = configMap.Annotations
-			current.OwnerReferences = configMap.OwnerReferences
 		}
+
+		current.Data = configMap.Data
+		current.Labels = configMap.Labels
+		current.Annotations = configMap.Annotations
+		current.OwnerReferences = configMap.OwnerReferences
 		return k8Client.Update(context.TODO(), current)
 	})
 }

internal/reconcile/configmaps_test.go

@@ -0,0 +1,135 @@
+package reconcile_test
+
+import (
+	"context"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	"github.com/openshift/cluster-logging-operator/internal/reconcile"
+	"github.com/openshift/cluster-logging-operator/internal/runtime"
+	"github.com/openshift/cluster-logging-operator/internal/utils"
+	"github.com/openshift/cluster-logging-operator/internal/utils/comparators"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	k8sruntime "k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/client-go/kubernetes/scheme"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+)
+
+var _ = Describe("reconciling ConfigMap", func() {
+
+	const (
+		namespace = "openshift-logging"
+		name      = "lokistack-config"
+	)
+
+	var (
+		clfOwner = metav1.OwnerReference{
+			APIVersion: "observability.openshift.io/v1",
+			Kind:       "ClusterLogForwarder",
+			Name:       "lokistack",
+			UID:        "clf-uid",
+			Controller: utils.GetPtr(true),
+		}
+		lokiOwner = metav1.OwnerReference{
+			APIVersion: "loki.grafana.com/v1",
+			Kind:       "LokiStack",
+			Name:       "lokistack",
+			UID:        "loki-uid",
+			Controller: utils.GetPtr(true),
+		}
+	)
+
+	newClient := func(objs ...client.Object) client.Client {
+		globalScheme := k8sruntime.NewScheme()
+		Expect(scheme.AddToScheme(globalScheme)).To(Succeed())
+		return fake.NewClientBuilder().WithScheme(globalScheme).WithObjects(objs...).Build()
+	}
+
+	getConfigMap := func(k8sClient client.Client) *corev1.ConfigMap {
+		result := &corev1.ConfigMap{}
+		Expect(k8sClient.Get(context.TODO(), client.ObjectKey{Namespace: namespace, Name: name}, result)).To(Succeed())
+		return result
+	}
+
+	desiredCollectorCM := func() *corev1.ConfigMap {
+		cm := runtime.NewConfigMap(namespace, name, map[string]string{
+			"vector.toml": "data_dir = \"/var/lib/vector\"",
+		})
+		utils.AddOwnerRefToObject(cm, clfOwner)
+		return cm
+	}
+
+	It("should create the ConfigMap when it does not exist", func() {
+		k8sClient := newClient()
+		desired := desiredCollectorCM()
+
+		Expect(reconcile.Configmap(k8sClient, k8sClient, desired, comparators.CompareLabels)).To(Succeed())
+
+		result := getConfigMap(k8sClient)
+		Expect(result.Data).To(HaveKey("vector.toml"))
+		Expect(result.OwnerReferences).To(Equal([]metav1.OwnerReference{clfOwner}))
+	})
+
+	It("should update the ConfigMap when owned by the ClusterLogForwarder", func() {
+		existing := runtime.NewConfigMap(namespace, name, map[string]string{
+			"vector.toml": "old",
+		})
+		utils.AddOwnerRefToObject(existing, clfOwner)
+		k8sClient := newClient(existing)
+
+		desired := desiredCollectorCM()
+		Expect(reconcile.Configmap(k8sClient, k8sClient, desired, comparators.CompareLabels)).To(Succeed())
+
+		result := getConfigMap(k8sClient)
+		Expect(result.Data["vector.toml"]).To(Equal("data_dir = \"/var/lib/vector\""))
+		Expect(result.OwnerReferences).To(Equal([]metav1.OwnerReference{clfOwner}))
+	})
+
+	It("should refuse to overwrite a ConfigMap owned by another resource", func() {
+		existing := runtime.NewConfigMap(namespace, name, map[string]string{
+			"config.yaml": "auth_enabled: false",
+		})
+		utils.AddOwnerRefToObject(existing, lokiOwner)
+		k8sClient := newClient(existing)
+
+		desired := desiredCollectorCM()
+		err := reconcile.Configmap(k8sClient, k8sClient, desired, comparators.CompareLabels)
+		Expect(err).To(HaveOccurred())
+		Expect(err.Error()).To(ContainSubstring("refusing to overwrite"))
+
+		result := getConfigMap(k8sClient)
+		Expect(result.Data).To(HaveKey("config.yaml"))
+		Expect(result.Data).NotTo(HaveKey("vector.toml"))
+		Expect(result.OwnerReferences).To(Equal([]metav1.OwnerReference{lokiOwner}))
+	})
+
+	It("should refuse an identical ConfigMap owned by another resource", func() {
+		existing := runtime.NewConfigMap(namespace, name, map[string]string{
+			"vector.toml": "data_dir = \"/var/lib/vector\"",
+		})
+		utils.AddOwnerRefToObject(existing, lokiOwner)
+		k8sClient := newClient(existing)
+
+		err := reconcile.Configmap(k8sClient, k8sClient, desiredCollectorCM(), comparators.CompareLabels)
+		Expect(err).To(HaveOccurred())
+		Expect(err.Error()).To(ContainSubstring("refusing to overwrite"))
+	})
+
+	It("should refuse to take ownership of an existing ConfigMap with no owner", func() {
+		existing := runtime.NewConfigMap(namespace, name, map[string]string{
+			"config.yaml": "auth_enabled: false",
+		})
+		k8sClient := newClient(existing)
+
+		desired := desiredCollectorCM()
+		err := reconcile.Configmap(k8sClient, k8sClient, desired, comparators.CompareLabels)
+		Expect(err).To(HaveOccurred())
+		Expect(err.Error()).To(ContainSubstring("refusing to overwrite"))
+
+		result := getConfigMap(k8sClient)
+		Expect(result.Data).To(HaveKey("config.yaml"))
+		Expect(result.OwnerReferences).To(BeEmpty())
+	})
+})

internal/reconcile/daemon_sets.go

@@ -6,6 +6,7 @@ import (
 
 	log "github.com/ViaQ/logerr/v2/log/static"
 	"github.com/openshift/cluster-logging-operator/internal/runtime"
+	"github.com/openshift/cluster-logging-operator/internal/utils"
 	apps "k8s.io/api/apps/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
@@ -16,7 +17,9 @@ import (
 func DaemonSet(k8Client client.Client, desired *apps.DaemonSet) error {
 	ds := runtime.NewDaemonSet(desired.Namespace, desired.Name)
 	op, err := controllerutil.CreateOrUpdate(context.TODO(), k8Client, ds, func() error {
-		// Update the daemonset with our desired state
+		if err := utils.EnsureCanUpdateOwnedResource(ds, desired.OwnerReferences...); err != nil {
+			return err
+		}
 		ds.Labels = desired.Labels
 		ds.Spec = desired.Spec
 		ds.OwnerReferences = desired.OwnerReferences

internal/reconcile/daemonset_ownership_test.go

@@ -0,0 +1,73 @@
+package reconcile_test
+
+import (
+	"context"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	"github.com/openshift/cluster-logging-operator/internal/reconcile"
+	"github.com/openshift/cluster-logging-operator/internal/runtime"
+	"github.com/openshift/cluster-logging-operator/internal/utils"
+	appsv1 "k8s.io/api/apps/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	k8sruntime "k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/client-go/kubernetes/scheme"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+)
+
+var _ = Describe("reconciling DaemonSet ownership", func() {
+	const (
+		namespace = "openshift-logging"
+		name      = "lokistack"
+	)
+
+	var (
+		clfOwner = metav1.OwnerReference{
+			APIVersion: "observability.openshift.io/v1",
+			Kind:       "ClusterLogForwarder",
+			Name:       "lokistack",
+			UID:        "clf-uid",
+			Controller: utils.GetPtr(true),
+		}
+		lokiOwner = metav1.OwnerReference{
+			APIVersion: "loki.grafana.com/v1",
+			Kind:       "LokiStack",
+			Name:       "lokistack",
+			UID:        "loki-uid",
+			Controller: utils.GetPtr(true),
+		}
+	)
+
+	newClient := func(objs ...client.Object) client.Client {
+		globalScheme := k8sruntime.NewScheme()
+		Expect(scheme.AddToScheme(globalScheme)).To(Succeed())
+		Expect(appsv1.AddToScheme(globalScheme)).To(Succeed())
+		return fake.NewClientBuilder().WithScheme(globalScheme).WithObjects(objs...).Build()
+	}
+
+	desiredDS := func() *appsv1.DaemonSet {
+		ds := runtime.NewDaemonSet(namespace, name)
+		ds.Spec.Selector = &metav1.LabelSelector{MatchLabels: map[string]string{"app": "collector"}}
+		ds.Spec.Template.Labels = map[string]string{"app": "collector"}
+		utils.AddOwnerRefToObject(ds, clfOwner)
+		return ds
+	}
+
+	It("should refuse to overwrite a DaemonSet owned by another resource", func() {
+		existing := runtime.NewDaemonSet(namespace, name)
+		existing.Spec.Selector = &metav1.LabelSelector{MatchLabels: map[string]string{"app": "loki"}}
+		existing.Spec.Template.Labels = map[string]string{"app": "loki"}
+		utils.AddOwnerRefToObject(existing, lokiOwner)
+		k8sClient := newClient(existing)
+
+		err := reconcile.DaemonSet(k8sClient, desiredDS())
+		Expect(err).To(HaveOccurred())
+		Expect(err.Error()).To(ContainSubstring("refusing to overwrite"))
+
+		got := &appsv1.DaemonSet{}
+		Expect(k8sClient.Get(context.TODO(), client.ObjectKey{Namespace: namespace, Name: name}, got)).To(Succeed())
+		Expect(got.Spec.Selector.MatchLabels["app"]).To(Equal("loki"))
+		Expect(got.OwnerReferences).To(Equal([]metav1.OwnerReference{lokiOwner}))
+	})
+})

internal/reconcile/deployment.go

@@ -6,6 +6,7 @@ import (
 
 	log "github.com/ViaQ/logerr/v2/log/static"
 	"github.com/openshift/cluster-logging-operator/internal/runtime"
+	"github.com/openshift/cluster-logging-operator/internal/utils"
 	apps "k8s.io/api/apps/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
@@ -16,7 +17,9 @@ import (
 func Deployment(k8Client client.Client, desired *apps.Deployment) error {
 	dpl := runtime.NewDeployment(desired.Namespace, desired.Name)
 	op, err := controllerutil.CreateOrUpdate(context.TODO(), k8Client, dpl, func() error {
-		// Update the deployment with our desired state
+		if err := utils.EnsureCanUpdateOwnedResource(dpl, desired.OwnerReferences...); err != nil {
+			return err
+		}
 		dpl.Labels = desired.Labels
 		dpl.Spec = desired.Spec
 		dpl.OwnerReferences = desired.OwnerReferences

internal/reconcile/networkpolicy.go

@@ -6,6 +6,7 @@ import (
 
 	log "github.com/ViaQ/logerr/v2/log/static"
 	"github.com/openshift/cluster-logging-operator/internal/runtime"
+	"github.com/openshift/cluster-logging-operator/internal/utils"
 	networkingv1 "k8s.io/api/networking/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
@@ -16,6 +17,9 @@ import (
 func NetworkPolicy(k8Client client.Client, desired *networkingv1.NetworkPolicy) error {
 	np := runtime.NewNetworkPolicy(desired.Namespace, desired.Name)
 	op, err := controllerutil.CreateOrUpdate(context.TODO(), k8Client, np, func() error {
+		if err := utils.EnsureCanUpdateOwnedResource(np, desired.OwnerReferences...); err != nil {
+			return err
+		}
 		np.Labels = desired.Labels
 		np.Spec = desired.Spec
 		np.OwnerReferences = desired.OwnerReferences

internal/reconcile/rbac.go

@@ -6,6 +6,7 @@ import (
 
 	log "github.com/ViaQ/logerr/v2/log/static"
 	"github.com/openshift/cluster-logging-operator/internal/runtime"
+	"github.com/openshift/cluster-logging-operator/internal/utils"
 	rbacv1 "k8s.io/api/rbac/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -15,7 +16,9 @@ import (
 func Role(k8Client client.Client, desired *rbacv1.Role) error {
 	role := runtime.NewRole(desired.Namespace, desired.Name)
 	op, err := controllerutil.CreateOrUpdate(context.TODO(), k8Client, role, func() error {
-		// Update the role with our desired state
+		if err := utils.EnsureCanUpdateOwnedResource(role, desired.OwnerReferences...); err != nil {
+			return err
+		}
 		role.Rules = desired.Rules
 		role.OwnerReferences = desired.OwnerReferences
 		return nil
@@ -38,6 +41,10 @@ func RoleBinding(k8Client client.Client, desired *rbacv1.RoleBinding) error {
 		return err
 	}
 
+	if err := utils.EnsureCanUpdateOwnedResource(existing, desired.OwnerReferences...); err != nil {
+		return err
+	}
+
 	if existing.RoleRef != desired.RoleRef {
 		log.V(3).Info("Deleting roleBinding due to roleRef change", "name", desired.Name, "namespace", desired.Namespace)
 		if err := k8Client.Delete(context.TODO(), existing); err != nil {
@@ -65,6 +72,10 @@ func ClusterRoleBinding(k8sClient client.Client, name string, generator func() *
 		return err
 	}
 
+	if err := utils.EnsureCanUpdateOwnedResource(existing, desired.OwnerReferences...); err != nil {
+		return err
+	}
+
 	if existing.RoleRef != desired.RoleRef {
 		log.V(3).Info("Deleting clusterRoleBinding due to roleRef change", "name", name)
 		if err := k8sClient.Delete(context.TODO(), existing); err != nil {
@@ -75,6 +86,7 @@ func ClusterRoleBinding(k8sClient client.Client, name string, generator func() *
 	}
 
 	existing.Subjects = desired.Subjects
+	existing.OwnerReferences = desired.OwnerReferences
 	log.V(3).Info("Updating clusterRoleBinding", "name", name)
 	return k8sClient.Update(context.TODO(), existing)
 }