diff --git a/internal/generate/clusterserviceversion/clusterserviceversion_updaters.go b/internal/generate/clusterserviceversion/clusterserviceversion_updaters.go index 98d7540705..25f19f0a1c 100644 --- a/internal/generate/clusterserviceversion/clusterserviceversion_updaters.go +++ b/internal/generate/clusterserviceversion/clusterserviceversion_updaters.go @@ -18,6 +18,7 @@ import ( "encoding/json" "errors" "fmt" + "slices" "sort" "strings" "time" @@ -31,6 +32,9 @@ import ( corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apimachinery/pkg/api/equality" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/operator-framework/operator-sdk/internal/generate/collector" @@ -57,8 +61,14 @@ func apply(c *collector.Manifests, csv *operatorsv1alpha1.ClusterServiceVersion, switch strategy.StrategyName { case operatorsv1alpha1.InstallStrategyNameDeployment: inPerms, inCPerms, _ := c.SplitCSVPermissionsObjects(extraSAs) - applyRoles(c, inPerms, &strategy.StrategySpec, extraSAs) - applyClusterRoles(c, inCPerms, &strategy.StrategySpec, extraSAs) + err := applyRoles(c, inPerms, &strategy.StrategySpec, extraSAs) + if err != nil { + return fmt.Errorf("can't apply roles: %w", err) + } + err = applyClusterRoles(c, inCPerms, &strategy.StrategySpec, extraSAs) + if err != nil { + return fmt.Errorf("can't apply cluster roles: %w", err) + } applyDeployments(c, &strategy.StrategySpec) } csv.Spec.InstallStrategy = strategy @@ -88,7 +98,7 @@ const defaultServiceAccountName = "default" // applyRoles applies Roles to strategy's permissions field by combining Roles bound to ServiceAccounts // into one set of permissions. -func applyRoles(c *collector.Manifests, objs []client.Object, strategy *operatorsv1alpha1.StrategyDetailsDeployment, extraSAs []string) { //nolint:dupl +func applyRoles(c *collector.Manifests, objs []client.Object, strategy *operatorsv1alpha1.StrategyDetailsDeployment, extraSAs []string) error { //nolint:dupl roleSet := make(map[string]rbacv1.Role) cRoleSet := make(map[string]rbacv1.ClusterRole) for i := range objs { @@ -120,8 +130,16 @@ func applyRoles(c *collector.Manifests, objs []client.Object, strategy *operator hasRole = has case "ClusterRole": role, has := cRoleSet[binding.RoleRef.Name] - rules = role.Rules + hasRole = has + if has { + var err error + rules, err = getClusterRoleRules(role, c.ClusterRoles) + if err != nil { + return fmt.Errorf("can't get ClusterRole rules: %w", err) + } + } + default: continue } @@ -143,11 +161,13 @@ func applyRoles(c *collector.Manifests, objs []client.Object, strategy *operator return perms[i].ServiceAccountName < perms[j].ServiceAccountName }) strategy.Permissions = perms + + return nil } // applyClusterRoles applies ClusterRoles to strategy's clusterPermissions field by combining ClusterRoles // bound to ServiceAccounts into one set of clusterPermissions. -func applyClusterRoles(c *collector.Manifests, objs []client.Object, strategy *operatorsv1alpha1.StrategyDetailsDeployment, extraSAs []string) { //nolint:dupl +func applyClusterRoles(c *collector.Manifests, objs []client.Object, strategy *operatorsv1alpha1.StrategyDetailsDeployment, extraSAs []string) error { //nolint:dupl roleSet := make(map[string]rbacv1.ClusterRole) for i := range objs { switch t := objs[i].(type) { @@ -166,7 +186,12 @@ func applyClusterRoles(c *collector.Manifests, objs []client.Object, strategy *o continue } if role, hasRole := roleSet[binding.RoleRef.Name]; hasRole { - perm.Rules = append(perm.Rules, role.Rules...) + rules, err := getClusterRoleRules(role, c.ClusterRoles) + if err != nil { + return fmt.Errorf("can't get ClusterRole rules: %w", err) + } + + perm.Rules = append(perm.Rules, rules...) saToPermissions[subject.Name] = perm } } @@ -183,6 +208,49 @@ func applyClusterRoles(c *collector.Manifests, objs []client.Object, strategy *o return perms[i].ServiceAccountName < perms[j].ServiceAccountName }) strategy.ClusterPermissions = perms + + return nil +} + +// getClusterRoleRules returns all PolicyRules for a given ClusterRole, including rules from aggregated ClusterRoles +// as specified by the AggregationRule. It recursively collects rules from other ClusterRoles that match the label selectors +// in the AggregationRule, ensuring no duplicate rules are added. +func getClusterRoleRules(clusterRole rbacv1.ClusterRole, clusterRoles []rbacv1.ClusterRole) ([]rbacv1.PolicyRule, error) { + rules := make([]rbacv1.PolicyRule, 0, len(clusterRole.Rules)) + rules = append(rules, clusterRole.Rules...) + + if clusterRole.AggregationRule == nil { + return rules, nil + } + + for _, crSelector := range clusterRole.AggregationRule.ClusterRoleSelectors { + labelSelector, err := metav1.LabelSelectorAsSelector(&crSelector) + if err != nil { + return nil, fmt.Errorf("can't create label selector from ClusterRole %q AggregationRule: %w", clusterRole.Name, err) + } + + for _, cr := range clusterRoles { + if cr.Name == clusterRole.Name { + continue + } + + if !labelSelector.Matches(labels.Set(cr.Labels)) { + continue + } + + for _, rule := range cr.Rules { + ruleExists := slices.ContainsFunc(rules, func(r rbacv1.PolicyRule) bool { + return equality.Semantic.DeepEqual(rule, r) + }) + + if !ruleExists { + rules = append(rules, rule) + } + } + } + } + + return rules, nil } // initPermissionSet initializes a map of ServiceAccount name to permissions, which are empty. diff --git a/internal/generate/clusterserviceversion/clusterserviceversion_updaters_test.go b/internal/generate/clusterserviceversion/clusterserviceversion_updaters_test.go index 212e32a6e0..1e4f507b9e 100644 --- a/internal/generate/clusterserviceversion/clusterserviceversion_updaters_test.go +++ b/internal/generate/clusterserviceversion/clusterserviceversion_updaters_test.go @@ -24,6 +24,7 @@ import ( rbacv1 "k8s.io/api/rbac/v1" apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apiextv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "sigs.k8s.io/controller-runtime/pkg/client" @@ -65,6 +66,8 @@ var _ = Describe("apply functions", func() { saName1 = "service-account-1" roleName1 = "role-1" cRoleName1 = "cluster-role-1" + cRoleName2 = "cluster-role-2" + cRoleName3 = "cluster-role-3" ) BeforeEach(func() { @@ -79,7 +82,8 @@ var _ = Describe("apply functions", func() { rules := []rbacv1.PolicyRule{{Verbs: []string{"create"}}} perms := []client.Object{newRole(roleName1, rules...)} c.RoleBindings = []rbacv1.RoleBinding{newRoleBinding("role-binding", newRoleRef(roleName1), newServiceAccountSubject(saName1))} - applyRoles(c, perms, strategy, nil) + err := applyRoles(c, perms, strategy, nil) + Expect(err).NotTo(HaveOccurred()) Expect(strategy.Permissions).To(Equal([]operatorsv1alpha1.StrategyDeploymentPermissions{ {ServiceAccountName: saName1, Rules: rules}, })) @@ -90,7 +94,49 @@ var _ = Describe("apply functions", func() { rules := []rbacv1.PolicyRule{{Verbs: []string{"create"}}} perms := []client.Object{newClusterRole(cRoleName1, rules...)} c.ClusterRoleBindings = []rbacv1.ClusterRoleBinding{newClusterRoleBinding("cluster-role-binding", newClusterRoleRef(cRoleName1), newServiceAccountSubject(saName1))} - applyClusterRoles(c, perms, strategy, nil) + err := applyClusterRoles(c, perms, strategy, nil) + Expect(err).NotTo(HaveOccurred()) + Expect(strategy.ClusterPermissions).To(Equal([]operatorsv1alpha1.StrategyDeploymentPermissions{ + {ServiceAccountName: saName1, Rules: rules}, + })) + }) + It("adds rules from aggregated ClusterRoles eliminating duplicates to the CSV deployment strategy", func() { + c.Deployments = []appsv1.Deployment{newDeploymentWithServiceAccount(depName1, saName1)} + c.ServiceAccounts = []corev1.ServiceAccount{newServiceAccount(saName1)} + rules := []rbacv1.PolicyRule{{Verbs: []string{"create"}}} + var emptyRules []rbacv1.PolicyRule + perms := []client.Object{ + func() *rbacv1.ClusterRole { + cr := newClusterRole(cRoleName1, emptyRules...) + cr.AggregationRule = &rbacv1.AggregationRule{ + ClusterRoleSelectors: []metav1.LabelSelector{ + { + MatchLabels: map[string]string{ + "aggregate-to-cluster-role-1": "true", + }, + }, + }, + } + return cr + }(), + func() *rbacv1.ClusterRole { + cr := newClusterRole(cRoleName2, rules...) + cr.Labels = map[string]string{ + "aggregate-to-cluster-role-1": "true", + } + return cr + }(), + func() *rbacv1.ClusterRole { + cr := newClusterRole(cRoleName3, rules...) + cr.Labels = map[string]string{ + "aggregate-to-cluster-role-1": "true", + } + return cr + }(), + } + c.ClusterRoleBindings = []rbacv1.ClusterRoleBinding{newClusterRoleBinding("cluster-role-binding", newClusterRoleRef(cRoleName1), newServiceAccountSubject(saName1))} + err := applyClusterRoles(c, perms, strategy, nil) + Expect(err).NotTo(HaveOccurred()) Expect(strategy.ClusterPermissions).To(Equal([]operatorsv1alpha1.StrategyDeploymentPermissions{ {ServiceAccountName: saName1, Rules: rules}, })) @@ -128,8 +174,10 @@ var _ = Describe("apply functions", func() { newClusterRoleBinding("cluster-role-binding-2", newClusterRoleRef(cRoleName2), newServiceAccountSubject(extraSAName)), newClusterRoleBinding("cluster-role-binding-3", newClusterRoleRef(cRoleName3), newServiceAccountSubject(extraSAName)), } - applyRoles(c, perms, strategy, []string{extraSAName}) - applyClusterRoles(c, cperms, strategy, []string{extraSAName}) + err := applyRoles(c, perms, strategy, []string{extraSAName}) + Expect(err).NotTo(HaveOccurred()) + err = applyClusterRoles(c, cperms, strategy, []string{extraSAName}) + Expect(err).NotTo(HaveOccurred()) Expect(strategy.Permissions).To(Equal([]operatorsv1alpha1.StrategyDeploymentPermissions{ {ServiceAccountName: saName1, Rules: rules}, {ServiceAccountName: extraSAName, Rules: rules}, @@ -146,14 +194,16 @@ var _ = Describe("apply functions", func() { c.Deployments = []appsv1.Deployment{newDeploymentWithServiceAccount(depName1, saName1)} c.ServiceAccounts = []corev1.ServiceAccount{newServiceAccount(saName1)} c.RoleBindings = []rbacv1.RoleBinding{newRoleBinding("role-binding", newRoleRef(roleName1), newServiceAccountSubject(saName1))} - applyRoles(c, nil, strategy, nil) + err := applyRoles(c, nil, strategy, nil) + Expect(err).NotTo(HaveOccurred()) Expect(strategy.Permissions).To(Equal([]operatorsv1alpha1.StrategyDeploymentPermissions{})) }) It("adds no ClusterPermissions to the CSV deployment strategy", func() { c.Deployments = []appsv1.Deployment{newDeploymentWithServiceAccount(depName1, saName1)} c.ServiceAccounts = []corev1.ServiceAccount{newServiceAccount(saName1)} c.ClusterRoleBindings = []rbacv1.ClusterRoleBinding{newClusterRoleBinding("cluster-role-binding", newClusterRoleRef(cRoleName1), newServiceAccountSubject(saName1))} - applyClusterRoles(c, nil, strategy, nil) + err := applyClusterRoles(c, nil, strategy, nil) + Expect(err).NotTo(HaveOccurred()) Expect(strategy.ClusterPermissions).To(Equal([]operatorsv1alpha1.StrategyDeploymentPermissions{})) }) })