clean-up code and added delete path for rbac resources new & legacy

Author: JoaoBraveCoding

operator/internal/handlers/lokistack_cluster_scope_resources_create.go

@@ -7,6 +7,7 @@ import (
 	"github.com/ViaQ/logerr/v2/kverrors"
 	"github.com/go-logr/logr"
 	rbacv1 "k8s.io/api/rbac/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client" //nolint:typecheck
@@ -37,20 +38,10 @@ func CreateClusterScopedResources(ctx context.Context, log logr.Logger, dashboar
 	}
 	opts := openshift.NewOptionsClusterScope(operatorNs, manifests.ClusterScopeLabels(), gatewaySubjects, rulerSubjects)
 
-	var objs []client.Object
+	objs := openshift.BuildRBAC(opts)
 	if dashboards {
-		dashObjs, err := openshift.BuildDashboards(opts)
-		if err != nil {
-			return kverrors.Wrap(err, "failed to build dashboard manifests")
-		}
-		objs = append(objs, dashObjs...)
-	}
-
-	rbacOBjs, err := openshift.BuildRBAC(opts)
-	if err != nil {
-		return kverrors.Wrap(err, "failed to build RBAC manifests")
+		objs = append(objs, openshift.BuildDashboards(opts)...)
 	}
-	objs = append(objs, rbacOBjs...)
 
 	var errCount int32
 	for _, obj := range objs {
@@ -74,7 +65,26 @@ func CreateClusterScopedResources(ctx context.Context, log logr.Logger, dashboar
 	}
 
 	if errCount > 0 {
-		return kverrors.New("failed to configure lokistack dashboard resources")
+		return kverrors.New("failed to configure lokistack cluster-scoped resources")
+	}
+
+	// Delete legacy RBAC resources
+	var legacyObjs []client.Object
+	for _, stack := range stacks.Items {
+		// This name would clash with the new cluster-scoped resources. Skip it.
+		if stack.Name == "lokistack" {
+			continue
+		}
+		legacyObjs = append(legacyObjs, openshift.LegacyRBAC(manifests.GatewayName(stack.Name), manifests.RulerName(stack.Name))...)
+	}
+	for _, obj := range legacyObjs {
+		key := client.ObjectKeyFromObject(obj)
+		if err := k.Delete(ctx, obj, &client.DeleteOptions{}); err != nil {
+			if apierrors.IsNotFound(err) {
+				continue
+			}
+			return kverrors.Wrap(err, "failed to delete resource", "kind", obj.GetObjectKind(), "key", key)
+		}
 	}
 
 	return nil

operator/internal/handlers/lokistack_cluster_scope_resources_delete.go

@@ -16,21 +16,19 @@ import (
 
 // DeleteClusterScopedResources removes all cluster-scoped resources.
 func DeleteClusterScopedResources(ctx context.Context, k k8s.Client, operatorNs string, stacks lokiv1.LokiStackList) error {
+	// Since we are deleting we don't need to worry about the subjects.
 	opts := openshift.NewOptionsClusterScope(operatorNs, manifests.ClusterScopeLabels(), []rbacv1.Subject{}, []rbacv1.Subject{})
-	objs, err := openshift.BuildDashboards(opts)
-	if err != nil {
-		return kverrors.Wrap(err, "failed to build dashboards manifests")
-	}
 
-	// Delete all re
+	objs := openshift.BuildRBAC(opts)
+	objs = append(objs, openshift.BuildDashboards(opts)...)
 
 	for _, obj := range objs {
 		key := client.ObjectKeyFromObject(obj)
 		if err := k.Delete(ctx, obj, &client.DeleteOptions{}); err != nil {
 			if apierrors.IsNotFound(err) {
 				continue
 			}
-			return kverrors.Wrap(err, "failed to delete dashboard", "key", key)
+			return kverrors.Wrap(err, "failed to delete dashboard", "kind", obj.GetObjectKind(), "key", key)
 		}
 	}
 	return nil

operator/internal/handlers/lokistack_cluster_scope_resources_delete_test.go

@@ -16,12 +16,12 @@ import (
 
 func TestDeleteDashboards(t *testing.T) {
 	opts := openshift.NewOptionsClusterScope("operator-ns", nil, nil, nil)
-	objs, err := openshift.BuildDashboards(opts)
-	require.NoError(t, err)
+	objs := openshift.BuildRBAC(opts)
+	objs = append(objs, openshift.BuildDashboards(opts)...)
 
 	k := &k8sfakes.FakeClient{}
 
-	err = DeleteClusterScopedResources(context.TODO(), k, "operator-ns", v1.LokiStackList{})
+	err := DeleteClusterScopedResources(context.TODO(), k, "operator-ns", v1.LokiStackList{})
 	require.NoError(t, err)
 	require.Equal(t, k.DeleteCallCount(), len(objs))
 }

operator/internal/manifests/openshift/dashboards.go

@@ -16,21 +16,18 @@ const (
 	managedConfigNamespace = "openshift-config-managed"
 )
 
-func BuildDashboards(opts OptionsClusterScope) ([]client.Object, error) {
+func BuildDashboards(opts OptionsClusterScope) []client.Object {
 	ds, rules := dashboards.Content()
 
 	var objs []client.Object
 	for name, content := range ds {
 		objs = append(objs, newDashboardConfigMap(name, content))
 	}
 
-	promRule, err := newDashboardPrometheusRule(opts.BuildOpts.OperatorNs, rules)
-	if err != nil {
-		return nil, err
-	}
+	promRule := newDashboardPrometheusRule(opts.BuildOpts.OperatorNs, rules)
 	objs = append(objs, promRule)
 
-	return objs, nil
+	return objs
 }
 
 func newDashboardConfigMap(filename string, content []byte) *corev1.ConfigMap {
@@ -54,7 +51,7 @@ func newDashboardConfigMap(filename string, content []byte) *corev1.ConfigMap {
 	}
 }
 
-func newDashboardPrometheusRule(namespace string, spec *monitoringv1.PrometheusRuleSpec) (*monitoringv1.PrometheusRule, error) {
+func newDashboardPrometheusRule(namespace string, spec *monitoringv1.PrometheusRuleSpec) *monitoringv1.PrometheusRule {
 	return &monitoringv1.PrometheusRule{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       "PrometheusRule",
@@ -65,5 +62,5 @@ func newDashboardPrometheusRule(namespace string, spec *monitoringv1.PrometheusR
 			Namespace: namespace,
 		},
 		Spec: *spec,
-	}, nil
+	}
 }

operator/internal/manifests/openshift/dashboards_test.go

@@ -10,8 +10,7 @@ import (
 
 func TestBuildDashboards_ReturnsDashboardConfigMaps(t *testing.T) {
 	opts := NewOptionsClusterScope("test", nil, nil, nil)
-	objs, err := BuildDashboards(opts)
-	require.NoError(t, err)
+	objs := BuildDashboards(opts)
 
 	for _, d := range objs {
 		switch d.(type) {
@@ -24,8 +23,7 @@ func TestBuildDashboards_ReturnsDashboardConfigMaps(t *testing.T) {
 
 func TestBuildDashboards_ReturnsPrometheusRules(t *testing.T) {
 	opts := NewOptionsClusterScope("test", nil, nil, nil)
-	objs, err := BuildDashboards(opts)
-	require.NoError(t, err)
+	objs := BuildDashboards(opts)
 
 	rules := objs[len(objs)-1].(*monitoringv1.PrometheusRule)
 	require.Equal(t, rules.GetName(), dashboardPrometheusRulesName)

operator/internal/manifests/openshift/rbac.go

@@ -11,13 +11,34 @@ const (
 	rulerName   = "lokistack-ruler"
 )
 
-func BuildRBAC(opts OptionsClusterScope) ([]client.Object, error) {
+func BuildRBAC(opts OptionsClusterScope) []client.Object {
 	objs := make([]client.Object, 0, 4)
-	objs = append(objs, BuildGatewayClusterRole(opts))
-	objs = append(objs, BuildGatewayClusterRoleBinding(opts))
 	objs = append(objs, BuildRulerClusterRole(opts))
 	objs = append(objs, BuildRulerClusterRoleBinding(opts))
-	return objs, nil
+	return objs
+}
+
+func LegacyRBAC(gatewayName, rulerName string) []client.Object {
+	opts := NewOptionsClusterScope("", map[string]string{}, []rbacv1.Subject{}, []rbacv1.Subject{})
+	objs := make([]client.Object, 0, 4)
+
+	cr := BuildGatewayClusterRole(opts)
+	cr.Name = authorizerRbacName(gatewayName)
+	objs = append(objs, cr)
+
+	crb := BuildGatewayClusterRoleBinding(opts)
+	crb.Name = authorizerRbacName(gatewayName)
+	objs = append(objs, crb)
+
+	cr = BuildRulerClusterRole(opts)
+	cr.Name = authorizerRbacName(rulerName)
+	objs = append(objs, cr)
+
+	crb = BuildRulerClusterRoleBinding(opts)
+	crb.Name = authorizerRbacName(rulerName)
+	objs = append(objs, crb)
+
+	return objs
 }
 
 // BuildGatewayClusterRole returns a k8s ClusterRole object for the