Add special handling for Aggregatin ClusterRole

Signed-off-by: Luca Burgazzoli <lburgazzoli@gmail.com>

pkg/controller/actions/deploy/action_deploy.go

@@ -327,7 +327,6 @@ func (a *Action) patch(
 		if err := RemoveDeploymentsResources(obj); err != nil {
 			return nil, fmt.Errorf("failed to apply allow list to Deployment %s/%s: %w", obj.GetNamespace(), obj.GetName(), err)
 		}
-
 	default:
 		// do nothing
 		break
@@ -392,6 +391,18 @@ func (a *Action) apply(
 		if err := MergeDeployments(old, obj); err != nil {
 			return nil, fmt.Errorf("failed to merge Deployment %s/%s: %w", obj.GetNamespace(), obj.GetName(), err)
 		}
+	case gvk.ClusterRole:
+		// For ClusterRole, if AggregationRule is set, then the Rules are controller managed
+		// and direct changes to Rules will be stomped by the controller. This also happen if
+		// the rules are set to an empty slice or nil hence we are removing the rules field
+		// if the ClusterRole is set to be an aggregation role.
+		_, found, err := unstructured.NestedFieldNoCopy(obj.Object, "aggregationRule")
+		if err != nil {
+			return nil, err
+		}
+		if found {
+			unstructured.RemoveNestedField(obj.Object, "rules")
+		}
 	default:
 		// do nothing
 		break

pkg/controller/actions/deploy/action_deploy_test.go

@@ -2,29 +2,40 @@ package deploy_test
 
 import (
 	"context"
+	"path/filepath"
 	"strconv"
 	"strings"
 	"testing"
 
 	"github.com/blang/semver/v4"
+	"github.com/onsi/gomega/gstruct"
 	"github.com/operator-framework/api/pkg/lib/version"
 	"github.com/rs/xid"
 	appsv1 "k8s.io/api/apps/v1"
+	corev1 "k8s.io/api/core/v1"
+	rbacv1 "k8s.io/api/rbac/v1"
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime"
+	apimachinery "k8s.io/apimachinery/pkg/types"
+	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/envtest"
 
 	componentsv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1"
 	dscv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/datasciencecluster/v1"
 	dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/deploy"
+	odhCli "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/client"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/annotations"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/labels"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/resources"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/utils/test/fakeclient"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/utils/test/matchers/jq"
+	"github.com/opendatahub-io/opendatahub-operator/v2/tests/envtestutil"
 
 	. "github.com/onsi/gomega"
 )
@@ -239,3 +250,133 @@ func TestDeployNotOwnedCreate(t *testing.T) {
 		jq.Match(`.spec.strategy.type == "%s"`, appsv1.RollingUpdateDeploymentStrategyType),
 	))
 }
+
+func TestDeployClusterRole(t *testing.T) {
+	g := NewWithT(t)
+	s := runtime.NewScheme()
+
+	utilruntime.Must(corev1.AddToScheme(s))
+	utilruntime.Must(appsv1.AddToScheme(s))
+	utilruntime.Must(apiextensionsv1.AddToScheme(s))
+	utilruntime.Must(componentsv1.AddToScheme(s))
+	utilruntime.Must(rbacv1.AddToScheme(s))
+
+	projectDir, err := envtestutil.FindProjectRoot()
+	g.Expect(err).NotTo(HaveOccurred())
+
+	envTest := &envtest.Environment{
+		CRDInstallOptions: envtest.CRDInstallOptions{
+			Scheme: s,
+			Paths: []string{
+				filepath.Join(projectDir, "config", "crd", "bases"),
+			},
+			ErrorIfPathMissing: true,
+			CleanUpAfterUse:    false,
+		},
+	}
+
+	t.Cleanup(func() {
+		_ = envTest.Stop()
+	})
+
+	cfg, err := envTest.Start()
+	g.Expect(err).NotTo(HaveOccurred())
+
+	envTestClient, err := client.New(cfg, client.Options{Scheme: s})
+	g.Expect(err).NotTo(HaveOccurred())
+
+	cli, err := odhCli.NewFromConfig(cfg, envTestClient)
+	g.Expect(err).NotTo(HaveOccurred())
+
+	t.Run("aggregation", func(t *testing.T) {
+		ctx := context.Background()
+		name := xid.New().String()
+
+		deployClusterRoles(t, ctx, cli, rbacv1.ClusterRole{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: name,
+			},
+			Rules: []rbacv1.PolicyRule{{
+				Verbs:     []string{"*"},
+				Resources: []string{"*"},
+				APIGroups: []string{"*"},
+			}},
+			AggregationRule: &rbacv1.AggregationRule{
+				ClusterRoleSelectors: []metav1.LabelSelector{{
+					MatchLabels: map[string]string{"foo": "bar"},
+				}},
+			},
+		})
+
+		out := rbacv1.ClusterRole{}
+		err = cli.Get(ctx, client.ObjectKey{Name: name}, &out)
+		g.Expect(err).NotTo(HaveOccurred())
+
+		g.Expect(out).To(gstruct.MatchFields(gstruct.IgnoreExtras, gstruct.Fields{
+			"Rules": BeEmpty(),
+		}))
+	})
+
+	t.Run("no aggregation", func(t *testing.T) {
+		ctx := context.Background()
+		name := xid.New().String()
+
+		deployClusterRoles(t, ctx, cli, rbacv1.ClusterRole{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: name,
+			},
+			Rules: []rbacv1.PolicyRule{{
+				Verbs:     []string{"*"},
+				Resources: []string{"*"},
+				APIGroups: []string{"*"},
+			}},
+		})
+
+		out := rbacv1.ClusterRole{}
+		err = cli.Get(ctx, client.ObjectKey{Name: name}, &out)
+		g.Expect(err).NotTo(HaveOccurred())
+
+		g.Expect(out).To(gstruct.MatchFields(gstruct.IgnoreExtras, gstruct.Fields{
+			"Rules": HaveLen(1),
+		}))
+	})
+}
+
+func deployClusterRoles(t *testing.T, ctx context.Context, cli *odhCli.Client, roles ...rbacv1.ClusterRole) {
+	t.Helper()
+
+	g := NewWithT(t)
+
+	rr := types.ReconciliationRequest{
+		Client: cli,
+		DSCI: &dsciv1.DSCInitialization{Spec: dsciv1.DSCInitializationSpec{
+			ApplicationsNamespace: xid.New().String(),
+		}},
+		DSC: &dscv1.DataScienceCluster{},
+		Instance: &componentsv1.Dashboard{
+			ObjectMeta: metav1.ObjectMeta{
+				Generation: 1,
+				UID:        apimachinery.UID(xid.New().String()),
+			},
+		},
+		Release: cluster.Release{
+			Name: cluster.OpenDataHub,
+			Version: version.OperatorVersion{Version: semver.Version{
+				Major: 1, Minor: 2, Patch: 3,
+			}}},
+	}
+
+	for i := range roles {
+		g.Expect(
+			rr.AddResources(roles[i].DeepCopy()),
+		).ShouldNot(
+			HaveOccurred(),
+		)
+	}
+
+	g.Expect(
+		deploy.NewAction()(ctx, &rr),
+	).ShouldNot(
+		HaveOccurred(),
+	)
+}