LOG-9424: Make legacy SCC role cleanup non-blocking

internal/auth/rbac.go

@@ -1,7 +1,10 @@
 package auth
 
 import (
+	"context"
 	"fmt"
+
+	log "github.com/ViaQ/logerr/v2/log/static"
 	"github.com/openshift/cluster-logging-operator/internal/reconcile"
 	"github.com/openshift/cluster-logging-operator/internal/runtime"
 	"github.com/openshift/cluster-logging-operator/internal/utils"
@@ -12,21 +15,50 @@ import (
 
 // ReconcileRBAC reconciles the RBAC specifically for the service account and SCC
 func ReconcileRBAC(k8sClient client.Client, rbacName, saNamespace, saName string, owner metav1.OwnerReference) error {
-	desiredCRB := NewMetaDataReaderClusterRoleBinding(saNamespace, saName, owner)
+	desiredCRB := NewMetaDataReaderClusterRoleBinding(saNamespace, saName)
 	if err := reconcile.ClusterRoleBinding(k8sClient, desiredCRB.Name, func() *rbacv1.ClusterRoleBinding { return desiredCRB }); err != nil {
 		return err
 	}
-	desiredSCCRole := NewServiceAccountSCCRole(saNamespace, saName, owner)
+	desiredSCCRole := NewServiceAccountSCCRole(saNamespace, rbacName, saName, owner)
 	if err := reconcile.Role(k8sClient, desiredSCCRole); err != nil {
 		return err
 	}
 
-	desiredSCCRoleBinding := NewServiceAccountSCCRoleBinding(saNamespace, rbacName, desiredSCCRole.Name, saName, owner)
-	return reconcile.RoleBinding(k8sClient, desiredSCCRoleBinding)
+	desiredSCCRoleBinding := NewServiceAccountSCCRoleBinding(saNamespace, rbacName, saName, owner)
+	if err := reconcile.RoleBinding(k8sClient, desiredSCCRoleBinding); err != nil {
+		return err
+	}
+
+	// Best-effort cleanup of old resources with previous naming scheme.
+	// Errors are logged but not returned to avoid blocking reconciliation
+	// (e.g., namespace-scoped cache may not know about newly created namespaces).
+	cleanupLegacySCCRole(k8sClient, saNamespace, saName)
+
+	return nil
+}
+
+func cleanupLegacySCCRole(k8sClient client.Client, saNamespace, saName string) {
+	oldRoleName := fmt.Sprintf("%s-scc", saName)
+
+	roleBindings := &rbacv1.RoleBindingList{}
+	if err := k8sClient.List(context.TODO(), roleBindings, client.InNamespace(saNamespace)); err != nil {
+		log.V(3).Info("skipping legacy SCC role cleanup: unable to list rolebindings", "namespace", saNamespace, "error", err)
+		return
+	}
+
+	for _, rb := range roleBindings.Items {
+		if rb.RoleRef.Name == oldRoleName {
+			return
+		}
+	}
+
+	if err := reconcile.DeleteRole(k8sClient, saNamespace, oldRoleName); err != nil {
+		log.V(3).Info("skipping legacy SCC role cleanup: unable to delete old role", "namespace", saNamespace, "role", oldRoleName, "error", err)
+	}
 }
 
 // NewMetaDataReaderClusterRoleBinding stubs a clusterrolebinding to allow reading of pod metadata (i.e. labels)
-func NewMetaDataReaderClusterRoleBinding(saNamespace, saName string, owner metav1.OwnerReference) *rbacv1.ClusterRoleBinding {
+func NewMetaDataReaderClusterRoleBinding(saNamespace, saName string) *rbacv1.ClusterRoleBinding {
 	name := fmt.Sprintf("metadata-reader-%s-%s", saNamespace, saName)
 	desired := runtime.NewClusterRoleBinding(name,
 		rbacv1.RoleRef{
@@ -41,26 +73,26 @@ func NewMetaDataReaderClusterRoleBinding(saNamespace, saName string, owner metav
 		},
 	)
 
-	utils.AddOwnerRefToObject(desired, owner)
 	return desired
 }
 
-func NewServiceAccountSCCRole(namespace, name string, owner metav1.OwnerReference) *rbacv1.Role {
-	name = fmt.Sprintf("%s-scc", name)
+func NewServiceAccountSCCRole(namespace, name, saName string, owner metav1.OwnerReference) *rbacv1.Role {
+	roleName := fmt.Sprintf("%s-%s-scc", name, saName)
 	sccRule := rbacv1.PolicyRule{
 		APIGroups:     []string{"security.openshift.io"},
 		ResourceNames: []string{sccName},
 		Resources:     []string{"securitycontextconstraints"},
 		Verbs:         []string{"use"},
 	}
 
-	desired := runtime.NewRole(namespace, name, sccRule)
+	desired := runtime.NewRole(namespace, roleName, sccRule)
 
 	utils.AddOwnerRefToObject(desired, owner)
 	return desired
 }
 
-func NewServiceAccountSCCRoleBinding(namespace, name, roleName, saName string, owner metav1.OwnerReference) *rbacv1.RoleBinding {
+func NewServiceAccountSCCRoleBinding(namespace, name, saName string, owner metav1.OwnerReference) *rbacv1.RoleBinding {
+	roleName := fmt.Sprintf("%s-%s-scc", name, saName)
 	roleRef := rbacv1.RoleRef{
 		APIGroup: rbacv1.GroupName,
 		Kind:     "Role",
@@ -73,8 +105,8 @@ func NewServiceAccountSCCRoleBinding(namespace, name, roleName, saName string, o
 		Namespace: namespace,
 	}
 
-	name = fmt.Sprintf("%s-scc", name)
-	desired := runtime.NewRoleBinding(namespace, name, roleRef, subject)
+	roleBindingName := fmt.Sprintf("%s-scc", name)
+	desired := runtime.NewRoleBinding(namespace, roleBindingName, roleRef, subject)
 
 	utils.AddOwnerRefToObject(desired, owner)
 	return desired

internal/auth/rbac_test.go

@@ -11,7 +11,7 @@ import (
 
 var _ = Describe("NewMetaDataReaderClusterRoleBinding", func() {
 	It("should stub a well-formed clusterrolebinding", func() {
-		Expect(test.YAMLString(auth.NewMetaDataReaderClusterRoleBinding(constants.OpenshiftNS, "logcollector", metav1.OwnerReference{}))).To(MatchYAML(
+		Expect(test.YAMLString(auth.NewMetaDataReaderClusterRoleBinding(constants.OpenshiftNS, "logcollector"))).To(MatchYAML(
 			`apiVersion: rbac.authorization.k8s.io/v1
 kind: ClusterRoleBinding
 metadata:
@@ -27,16 +27,21 @@ subjects:
   namespace: openshift-logging
 `))
 	})
+
+	It("should not have owner references", func() {
+		crb := auth.NewMetaDataReaderClusterRoleBinding(constants.OpenshiftNS, "logcollector")
+		Expect(crb.OwnerReferences).To(BeEmpty())
+	})
 })
 
 var _ = Describe("ServiceAccount SCC Role & RoleBinding", func() {
 	It("should stub a well-formed role", func() {
-		Expect(test.YAMLString(auth.NewServiceAccountSCCRole(constants.OpenshiftNS, "my-sa", metav1.OwnerReference{}))).To(MatchYAML(
+		Expect(test.YAMLString(auth.NewServiceAccountSCCRole(constants.OpenshiftNS, "my-clf", "my-sa", metav1.OwnerReference{}))).To(MatchYAML(
 			`apiVersion: rbac.authorization.k8s.io/v1
 kind: Role
 metadata:
   creationTimestamp: null
-  name: my-sa-scc
+  name: my-clf-my-sa-scc
   namespace: openshift-logging
 rules:
 - apiGroups:
@@ -51,20 +56,20 @@ rules:
 	})
 
 	It("should stub a well-formed roleBinding", func() {
-		Expect(test.YAMLString(auth.NewServiceAccountSCCRoleBinding(constants.OpenshiftNS, "my-sa", "customSA-scc", "customSA", metav1.OwnerReference{}))).To(MatchYAML(
+		Expect(test.YAMLString(auth.NewServiceAccountSCCRoleBinding(constants.OpenshiftNS, "my-clf", "my-sa", metav1.OwnerReference{}))).To(MatchYAML(
 			`apiVersion: rbac.authorization.k8s.io/v1
 kind: RoleBinding
 metadata:
   creationTimestamp: null
-  name: my-sa-scc
+  name: my-clf-scc
   namespace: openshift-logging
 roleRef:
   apiGroup: rbac.authorization.k8s.io
   kind: Role
-  name: customSA-scc
+  name: my-clf-my-sa-scc
 subjects:
   - kind: ServiceAccount
-    name: customSA
+    name: my-sa
     namespace: openshift-logging
 `))
 	})

internal/reconcile/rbac.go

@@ -7,6 +7,7 @@ import (
 	log "github.com/ViaQ/logerr/v2/log/static"
 	"github.com/openshift/cluster-logging-operator/internal/runtime"
 	rbacv1 "k8s.io/api/rbac/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 )
@@ -69,3 +70,14 @@ func DeleteClusterRoleBinding(k8sClient client.Client, name string) error {
 	log.V(3).Info("Deleting", "object", object)
 	return k8sClient.Delete(context.TODO(), object)
 }
+
+func DeleteRole(k8sClient client.Client, namespace, name string) error {
+	object := runtime.NewRole(namespace, name)
+	log.V(3).Info("Deleting Role", "namespace", namespace, "name", name)
+	err := k8sClient.Delete(context.TODO(), object)
+	// Ignore NotFound errors - resource is already deleted
+	if apierrors.IsNotFound(err) {
+		return nil
+	}
+	return err
+}

test/e2e/collection/tuning/at_least_once_test.go

@@ -133,7 +133,7 @@ var _ = Describe("[tuning] deliveryMode AtLeastOnce", func() {
 
 		//wait for some logs from all streams to be received
 		// Verify some logs from all streams to be received
-		Expect(wait.PollUntilContextTimeout(context.TODO(), 5*time.Second, 3*time.Minute, true, func(context.Context) (done bool, err error) {
+		Expect(wait.PollUntilContextTimeout(context.TODO(), 5*time.Second, 5*time.Minute, true, func(context.Context) (done bool, err error) {
 			q, err := receiver.Query(utils.GetPtr(15 * time.Second))
 			if err != nil {
 				log.V(0).Error(err, "The error from querying the receiver")