fix(operator): move authorization RBAC to dedicated cluster_scoped

JoaoBraveCoding · JoaoBraveCoding · commit 47351a31e0cc · 2025-03-13T16:52:45.000Z
reconciler
diff --git a/operator/internal/controller/loki/lokistack_cluster_scoped_controller.go b/operator/internal/controller/loki/lokistack_cluster_scoped_controller.go
@@ -47,15 +47,15 @@ func (r *DashboardsReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 	if len(stacks.Items) == 0 {
 		// Removes all LokiStack dashboard resources on OpenShift clusters when
 		// the last LokiStack custom resource is deleted.
-		if err := handlers.DeleteDashboards(ctx, r.Client, r.OperatorNs); err != nil {
+		if err := handlers.DeleteClusterScopedResources(ctx, r.Client, r.OperatorNs, stacks); err != nil {
 			return ctrl.Result{}, kverrors.Wrap(err, "failed to delete dashboard resources")
 		}
 		return ctrl.Result{}, nil
 	}
 
 	// Creates all LokiStack dashboard resources on OpenShift clusters when
 	// the first LokiStack custom resource is created.
-	if err := handlers.CreateDashboards(ctx, r.Log, r.OperatorNs, r.Client, r.Scheme); err != nil {
+	if err := handlers.CreateClusterScopedResources(ctx, r.Log, r.OperatorNs, r.Client, r.Scheme, stacks); err != nil {
 		return ctrl.Result{}, kverrors.Wrap(err, "failed to create dashboard resources", "req", req)
 	}
 	return ctrl.Result{}, nil
diff --git a/operator/internal/handlers/dashboards_create.go b/operator/internal/handlers/dashboards_create.go
diff --git a/operator/internal/handlers/lokistack_cluster_scope_resources_create.go b/operator/internal/handlers/lokistack_cluster_scope_resources_create.go
@@ -0,0 +1,77 @@
+package handlers
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/ViaQ/logerr/v2/kverrors"
+	"github.com/go-logr/logr"
+	rbacv1 "k8s.io/api/rbac/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client" //nolint:typecheck
+	ctrlutil "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+
+	lokiv1 "github.com/grafana/loki/operator/api/loki/v1"
+	"github.com/grafana/loki/operator/internal/external/k8s"
+	"github.com/grafana/loki/operator/internal/manifests"
+	"github.com/grafana/loki/operator/internal/manifests/openshift"
+)
+
+// CreateClusterScopedResources handles the LokiStack cluster scoped create events.
+func CreateClusterScopedResources(ctx context.Context, log logr.Logger, operatorNs string, k k8s.Client, s *runtime.Scheme, stacks lokiv1.LokiStackList) error {
+	// This has to be done here as to not introduce a circular dependency.
+	gatewaySubjects := make([]rbacv1.Subject, 0, len(stacks.Items))
+	rulerSubjects := make([]rbacv1.Subject, 0, len(stacks.Items))
+	for _, stack := range stacks.Items {
+		gatewaySubjects = append(gatewaySubjects, rbacv1.Subject{
+			Kind:      "ServiceAccount",
+			Name:      manifests.GatewayName(stack.Name),
+			Namespace: stack.Namespace,
+		})
+		rulerSubjects = append(rulerSubjects, rbacv1.Subject{
+			Kind:      "ServiceAccount",
+			Name:      manifests.RulerName(stack.Name),
+			Namespace: stack.Namespace,
+		})
+	}
+	opts := openshift.NewOptionsClusterScope(operatorNs, stacks, manifests.ClusterScopeLabels(), gatewaySubjects, rulerSubjects)
+
+	objs, err := openshift.BuildDashboards(opts)
+	if err != nil {
+		return kverrors.Wrap(err, "failed to build dashboard manifests")
+	}
+
+	rbacOBjs, err := openshift.BuildRBAC(opts)
+	if err != nil {
+		return kverrors.Wrap(err, "failed to build RBAC manifests")
+	}
+	objs = append(objs, rbacOBjs...)
+
+	var errCount int32
+	for _, obj := range objs {
+		desired := obj.DeepCopyObject().(client.Object)
+		mutateFn := manifests.MutateFuncFor(obj, desired, nil)
+
+		op, err := ctrl.CreateOrUpdate(ctx, k, obj, mutateFn)
+		if err != nil {
+			log.Error(err, "failed to configure resource")
+			errCount++
+			continue
+		}
+
+		msg := fmt.Sprintf("Resource has been %s", op)
+		switch op {
+		case ctrlutil.OperationResultNone:
+			log.V(1).Info(msg)
+		default:
+			log.Info(msg)
+		}
+	}
+
+	if errCount > 0 {
+		return kverrors.New("failed to configure lokistack dashboard resources")
+	}
+
+	return nil
+}
diff --git a/operator/internal/handlers/lokistack_cluster_scope_resources_create_test.go b/operator/internal/handlers/lokistack_cluster_scope_resources_create_test.go
@@ -53,7 +53,7 @@ func TestCreateDashboards_ReturnsResourcesInManagedNamespaces(t *testing.T) {
 
 	k.StatusStub = func() client.StatusWriter { return sw }
 
-	err := CreateDashboards(context.TODO(), logger, "test", k, scheme)
+	err := CreateClusterScopedResources(context.TODO(), logger, "test", k, scheme, lokiv1.LokiStackList{Items: []lokiv1.LokiStack{stack}})
 	require.NoError(t, err)
 
 	// make sure create was called
diff --git a/operator/internal/handlers/lokistack_cluster_scope_resources_delete.go b/operator/internal/handlers/lokistack_cluster_scope_resources_delete.go
@@ -4,20 +4,26 @@ import (
 	"context"
 
 	"github.com/ViaQ/logerr/v2/kverrors"
+	rbacv1 "k8s.io/api/rbac/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
+	lokiv1 "github.com/grafana/loki/operator/api/loki/v1"
 	"github.com/grafana/loki/operator/internal/external/k8s"
+	"github.com/grafana/loki/operator/internal/manifests"
 	"github.com/grafana/loki/operator/internal/manifests/openshift"
 )
 
-// DeleteDashboards removes all cluster-scoped dashboard resources.
-func DeleteDashboards(ctx context.Context, k k8s.Client, operatorNs string) error {
-	objs, err := openshift.BuildDashboards(operatorNs)
+// DeleteClusterScopedResources removes all cluster-scoped resources.
+func DeleteClusterScopedResources(ctx context.Context, k k8s.Client, operatorNs string, stacks lokiv1.LokiStackList) error {
+	opts := openshift.NewOptionsClusterScope(operatorNs, stacks, 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
+
 	for _, obj := range objs {
 		key := client.ObjectKeyFromObject(obj)
 		if err := k.Delete(ctx, obj, &client.DeleteOptions{}); err != nil {
diff --git a/operator/internal/handlers/lokistack_cluster_scope_resources_delete_test.go b/operator/internal/handlers/lokistack_cluster_scope_resources_delete_test.go
@@ -9,17 +9,19 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
+	v1 "github.com/grafana/loki/operator/api/loki/v1"
 	"github.com/grafana/loki/operator/internal/external/k8s/k8sfakes"
 	"github.com/grafana/loki/operator/internal/manifests/openshift"
 )
 
 func TestDeleteDashboards(t *testing.T) {
-	objs, err := openshift.BuildDashboards("operator-ns")
+	opts := openshift.NewOptionsClusterScope("operator-ns", v1.LokiStackList{}, nil, nil, nil)
+	objs, err := openshift.BuildDashboards(opts)
 	require.NoError(t, err)
 
 	k := &k8sfakes.FakeClient{}
 
-	err = DeleteDashboards(context.TODO(), k, "operator-ns")
+	err = DeleteClusterScopedResources(context.TODO(), k, "operator-ns", v1.LokiStackList{})
 	require.NoError(t, err)
 	require.Equal(t, k.DeleteCallCount(), len(objs))
 }
@@ -30,6 +32,6 @@ func TestDeleteDashboards_ReturnsNoError_WhenNotFound(t *testing.T) {
 		return apierrors.NewNotFound(schema.GroupResource{}, "something wasn't found")
 	}
 
-	err := DeleteDashboards(context.TODO(), k, "operator-ns")
+	err := DeleteClusterScopedResources(context.TODO(), k, "operator-ns", v1.LokiStackList{})
 	require.NoError(t, err)
 }
diff --git a/operator/internal/manifests/gateway_tenants.go b/operator/internal/manifests/gateway_tenants.go
@@ -152,8 +152,6 @@ func configureGatewayObjsForMode(objs []client.Object, opts Options) []client.Ob
 			}
 		}
 
-		openShiftObjs := openshift.BuildGatewayTenantModeObjects(opts.OpenShiftOptions)
-		objs = append(objs, openShiftObjs...)
 	}
 
 	return objs
diff --git a/operator/internal/manifests/gateway_test.go b/operator/internal/manifests/gateway_test.go
@@ -303,7 +303,7 @@ func TestBuildGateway_HasExtraObjectsForTenantMode(t *testing.T) {
 	})
 
 	require.NoError(t, err)
-	require.Len(t, objs, 13)
+	require.Len(t, objs, 11)
 }
 
 func TestBuildGateway_WithExtraObjectsForTenantMode_RouteSvcMatches(t *testing.T) {
@@ -987,7 +987,7 @@ func TestBuildGateway_PodDisruptionBudget(t *testing.T) {
 	}
 	objs, err := BuildGateway(opts)
 	require.NoError(t, err)
-	require.Len(t, objs, 13)
+	require.Len(t, objs, 11)
 
 	pdb := objs[6].(*policyv1.PodDisruptionBudget)
 	require.NotNil(t, pdb)
diff --git a/operator/internal/manifests/openshift/build.go b/operator/internal/manifests/openshift/build.go
@@ -15,24 +15,11 @@ func BuildGatewayObjects(opts Options) []client.Object {
 	}
 }
 
-// BuildGatewayTenantModeObjects returns a list of auxiliary openshift/k8s objects
-// for lokistack gateway deployments on OpenShift for tenant modes:
-// - openshift-logging
-// - openshift-network
-func BuildGatewayTenantModeObjects(opts Options) []client.Object {
-	return []client.Object{
-		BuildGatewayClusterRole(opts),
-		BuildGatewayClusterRoleBinding(opts),
-	}
-}
-
 // BuildRulerObjects returns a list of auxiliary openshift/k8s objects
 // for lokistack ruler deployments on OpenShift.
 func BuildRulerObjects(opts Options) []client.Object {
 	return []client.Object{
 		BuildAlertManagerCAConfigMap(opts),
 		BuildRulerServiceAccount(opts),
-		BuildRulerClusterRole(opts),
-		BuildRulerClusterRoleBinding(opts),
 	}
 }
diff --git a/operator/internal/manifests/openshift/build_test.go b/operator/internal/manifests/openshift/build_test.go
@@ -6,24 +6,9 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/require"
-	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
-
-	lokiv1 "github.com/grafana/loki/operator/api/loki/v1"
 )
 
-func TestBuildGatewayTenantModeObjects_ClusterRoleRefMatches(t *testing.T) {
-	opts := NewOptions("abc", "ns", "abc", "abc", "abc", 1*time.Minute, map[string]string{}, "abc").
-		WithTenantsForMode(lokiv1.OpenshiftLogging, "example.com", map[string]TenantData{})
-
-	objs := BuildGatewayTenantModeObjects(*opts)
-	cr := objs[0].(*rbacv1.ClusterRole)
-	rb := objs[1].(*rbacv1.ClusterRoleBinding)
-
-	require.Equal(t, cr.Kind, rb.RoleRef.Kind)
-	require.Equal(t, cr.Name, rb.RoleRef.Name)
-}
-
 func TestBuildGatewayObjects_MonitoringClusterRoleRefMatches(t *testing.T) {
 	opts := NewOptions("abc", "ns", "abc", "abc", "abc", 1*time.Minute, map[string]string{}, "abc")
 
@@ -49,18 +34,3 @@ func TestBuildGatewayObjets_RouteWithTimeoutAnnotation(t *testing.T) {
 	want := fmt.Sprintf("%.fs", routeTimeout.Seconds())
 	require.Equal(t, want, got)
 }
-
-func TestBuildRulerObjects_ClusterRoleRefMatches(t *testing.T) {
-	opts := NewOptions("abc", "ns", "abc", "abc", "abc", 1*time.Minute, map[string]string{}, "abc")
-
-	objs := BuildRulerObjects(*opts)
-	sa := objs[1].(*corev1.ServiceAccount)
-	cr := objs[2].(*rbacv1.ClusterRole)
-	rb := objs[3].(*rbacv1.ClusterRoleBinding)
-
-	require.Equal(t, sa.Kind, rb.Subjects[0].Kind)
-	require.Equal(t, sa.Name, rb.Subjects[0].Name)
-	require.Equal(t, sa.Namespace, rb.Subjects[0].Namespace)
-	require.Equal(t, cr.Kind, rb.RoleRef.Kind)
-	require.Equal(t, cr.Name, rb.RoleRef.Name)
-}
diff --git a/operator/internal/manifests/openshift/dashboards.go b/operator/internal/manifests/openshift/dashboards.go
@@ -16,15 +16,15 @@ const (
 	managedConfigNamespace = "openshift-config-managed"
 )
 
-func BuildDashboards(operatorNs string) ([]client.Object, error) {
+func BuildDashboards(opts OptionsClusterScope) ([]client.Object, error) {
 	ds, rules := dashboards.Content()
 
 	var objs []client.Object
 	for name, content := range ds {
 		objs = append(objs, newDashboardConfigMap(name, content))
 	}
 
-	promRule, err := newDashboardPrometheusRule(operatorNs, rules)
+	promRule, err := newDashboardPrometheusRule(opts.BuildOpts.OperatorNs, rules)
 	if err != nil {
 		return nil, err
 	}
diff --git a/operator/internal/manifests/openshift/dashboards_test.go b/operator/internal/manifests/openshift/dashboards_test.go
@@ -6,10 +6,13 @@ import (
 	monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
 	"github.com/stretchr/testify/require"
 	corev1 "k8s.io/api/core/v1"
+
+	v1 "github.com/grafana/loki/operator/api/loki/v1"
 )
 
 func TestBuildDashboards_ReturnsDashboardConfigMaps(t *testing.T) {
-	objs, err := BuildDashboards("test")
+	opts := NewOptionsClusterScope("test", v1.LokiStackList{}, nil, nil, nil)
+	objs, err := BuildDashboards(opts)
 	require.NoError(t, err)
 
 	for _, d := range objs {
@@ -22,7 +25,8 @@ func TestBuildDashboards_ReturnsDashboardConfigMaps(t *testing.T) {
 }
 
 func TestBuildDashboards_ReturnsPrometheusRules(t *testing.T) {
-	objs, err := BuildDashboards("test")
+	opts := NewOptionsClusterScope("test", v1.LokiStackList{}, nil, nil, nil)
+	objs, err := BuildDashboards(opts)
 	require.NoError(t, err)
 
 	rules := objs[len(objs)-1].(*monitoringv1.PrometheusRule)
diff --git a/operator/internal/manifests/openshift/options_cluster_scope.go b/operator/internal/manifests/openshift/options_cluster_scope.go
@@ -0,0 +1,33 @@
+package openshift
+
+import (
+	rbacv1 "k8s.io/api/rbac/v1"
+
+	lokiv1 "github.com/grafana/loki/operator/api/loki/v1"
+)
+
+type OptionsClusterScope struct {
+	BuildOpts BuildOptionsClusterScope
+}
+
+type BuildOptionsClusterScope struct {
+	OperatorNs      string
+	LokiStacks      lokiv1.LokiStackList
+	GatewaySubjects []rbacv1.Subject
+	RulerSubjects   []rbacv1.Subject
+	Labels          map[string]string
+}
+
+func NewOptionsClusterScope(operatorNs string, lokistacks lokiv1.LokiStackList, labels map[string]string, gatewaySubjects []rbacv1.Subject, rulerSubjects []rbacv1.Subject) OptionsClusterScope {
+	o := OptionsClusterScope{
+		BuildOpts: BuildOptionsClusterScope{
+			OperatorNs:      operatorNs,
+			LokiStacks:      lokistacks,
+			GatewaySubjects: gatewaySubjects,
+			RulerSubjects:   rulerSubjects,
+			Labels:          labels,
+		},
+	}
+
+	return o
+}
diff --git a/operator/internal/manifests/openshift/rbac.go b/operator/internal/manifests/openshift/rbac.go
diff --git a/operator/internal/manifests/openshift/var.go b/operator/internal/manifests/openshift/var.go
diff --git a/operator/internal/manifests/ruler_test.go b/operator/internal/manifests/ruler_test.go
diff --git a/operator/internal/manifests/var.go b/operator/internal/manifests/var.go

Original file line number	Diff line number	Diff line change
`@@ -47,15 +47,15 @@ func (r *DashboardsReconciler) Reconcile(ctx context.Context, req ctrl.Request)`
`47`	`47`	`if len(stacks.Items) == 0 {`
`48`	`48`	`// Removes all LokiStack dashboard resources on OpenShift clusters when`
`49`	`49`	`// the last LokiStack custom resource is deleted.`
`50`		`- if err := handlers.DeleteDashboards(ctx, r.Client, r.OperatorNs); err != nil {`
	`50`	`+ if err := handlers.DeleteClusterScopedResources(ctx, r.Client, r.OperatorNs, stacks); err != nil {`
`51`	`51`	`return ctrl.Result{}, kverrors.Wrap(err, "failed to delete dashboard resources")`
`52`	`52`	`}`
`53`	`53`	`return ctrl.Result{}, nil`
`54`	`54`	`}`
`55`	`55`
`56`	`56`	`// Creates all LokiStack dashboard resources on OpenShift clusters when`
`57`	`57`	`// the first LokiStack custom resource is created.`
`58`		`- if err := handlers.CreateDashboards(ctx, r.Log, r.OperatorNs, r.Client, r.Scheme); err != nil {`
	`58`	`+ if err := handlers.CreateClusterScopedResources(ctx, r.Log, r.OperatorNs, r.Client, r.Scheme, stacks); err != nil {`
`59`	`59`	`return ctrl.Result{}, kverrors.Wrap(err, "failed to create dashboard resources", "req", req)`
`60`	`60`	`}`
`61`	`61`	`return ctrl.Result{}, nil`
Original file line number	Diff line number	Diff line change
`@@ -9,17 +9,19 @@ import (`
`9`	`9`	`"k8s.io/apimachinery/pkg/runtime/schema"`
`10`	`10`	`"sigs.k8s.io/controller-runtime/pkg/client"`
`11`	`11`
	`12`	`+ v1 "github.com/grafana/loki/operator/api/loki/v1"`
`12`	`13`	`"github.com/grafana/loki/operator/internal/external/k8s/k8sfakes"`
`13`	`14`	`"github.com/grafana/loki/operator/internal/manifests/openshift"`
`14`	`15`	`)`
`15`	`16`
`16`	`17`	`func TestDeleteDashboards(t *testing.T) {`
`17`		`- objs, err := openshift.BuildDashboards("operator-ns")`
	`18`	`+ opts := openshift.NewOptionsClusterScope("operator-ns", v1.LokiStackList{}, nil, nil, nil)`
	`19`	`+ objs, err := openshift.BuildDashboards(opts)`
`18`	`20`	`require.NoError(t, err)`
`19`	`21`
`20`	`22`	`k := &k8sfakes.FakeClient{}`
`21`	`23`
`22`		`- err = DeleteDashboards(context.TODO(), k, "operator-ns")`
	`24`	`+ err = DeleteClusterScopedResources(context.TODO(), k, "operator-ns", v1.LokiStackList{})`
`23`	`25`	`require.NoError(t, err)`
`24`	`26`	`require.Equal(t, k.DeleteCallCount(), len(objs))`
`25`	`27`	`}`
`@@ -30,6 +32,6 @@ func TestDeleteDashboards_ReturnsNoError_WhenNotFound(t *testing.T) {`
`30`	`32`	`return apierrors.NewNotFound(schema.GroupResource{}, "something wasn't found")`
`31`	`33`	`}`
`32`	`34`
`33`		`- err := DeleteDashboards(context.TODO(), k, "operator-ns")`
	`35`	`+ err := DeleteClusterScopedResources(context.TODO(), k, "operator-ns", v1.LokiStackList{})`
`34`	`36`	`require.NoError(t, err)`
`35`	`37`	`}`
Original file line number	Diff line number	Diff line change
`@@ -152,8 +152,6 @@ func configureGatewayObjsForMode(objs []client.Object, opts Options) []client.Ob`
`152`	`152`	`}`
`153`	`153`	`}`
`154`	`154`
`155`		`- openShiftObjs := openshift.BuildGatewayTenantModeObjects(opts.OpenShiftOptions)`
`156`		`- objs = append(objs, openShiftObjs...)`
`157`	`155`	`}`
`158`	`156`
`159`	`157`	`return objs`
Original file line number	Diff line number	Diff line change
`@@ -303,7 +303,7 @@ func TestBuildGateway_HasExtraObjectsForTenantMode(t *testing.T) {`
`303`	`303`	`})`
`304`	`304`
`305`	`305`	`require.NoError(t, err)`
`306`		`- require.Len(t, objs, 13)`
	`306`	`+ require.Len(t, objs, 11)`
`307`	`307`	`}`
`308`	`308`
`309`	`309`	`func TestBuildGateway_WithExtraObjectsForTenantMode_RouteSvcMatches(t *testing.T) {`
`@@ -987,7 +987,7 @@ func TestBuildGateway_PodDisruptionBudget(t *testing.T) {`
`987`	`987`	`}`
`988`	`988`	`objs, err := BuildGateway(opts)`
`989`	`989`	`require.NoError(t, err)`
`990`		`- require.Len(t, objs, 13)`
	`990`	`+ require.Len(t, objs, 11)`
`991`	`991`
`992`	`992`	`pdb := objs[6].(*policyv1.PodDisruptionBudget)`
`993`	`993`	`require.NotNil(t, pdb)`
Original file line number	Diff line number	Diff line change
`@@ -15,24 +15,11 @@ func BuildGatewayObjects(opts Options) []client.Object {`
`15`	`15`	`}`
`16`	`16`	`}`
`17`	`17`
`18`		`-// BuildGatewayTenantModeObjects returns a list of auxiliary openshift/k8s objects`
`19`		`-// for lokistack gateway deployments on OpenShift for tenant modes:`
`20`		`-// - openshift-logging`
`21`		`-// - openshift-network`
`22`		`-func BuildGatewayTenantModeObjects(opts Options) []client.Object {`
`23`		`- return []client.Object{`
`24`		`- BuildGatewayClusterRole(opts),`
`25`		`- BuildGatewayClusterRoleBinding(opts),`
`26`		`- }`
`27`		`-}`
`28`		`-`
`29`	`18`	`// BuildRulerObjects returns a list of auxiliary openshift/k8s objects`
`30`	`19`	`// for lokistack ruler deployments on OpenShift.`
`31`	`20`	`func BuildRulerObjects(opts Options) []client.Object {`
`32`	`21`	`return []client.Object{`
`33`	`22`	`BuildAlertManagerCAConfigMap(opts),`
`34`	`23`	`BuildRulerServiceAccount(opts),`
`35`		`- BuildRulerClusterRole(opts),`
`36`		`- BuildRulerClusterRoleBinding(opts),`
`37`	`24`	`}`
`38`	`25`	`}`