refactor operator group cluster role name

Signed-off-by: Per Goncalves da Silva <[email protected]>

go.mod

@@ -14,6 +14,7 @@ require (
  github.com/go-logr/logr v1.2.4
  github.com/golang/mock v1.6.0
  github.com/google/go-cmp v0.5.9
+ github.com/google/uuid v1.3.0
  github.com/googleapis/gnostic v0.5.5
  github.com/itchyny/gojq v0.11.0
  github.com/maxbrunsfeld/counterfeiter/v6 v6.2.2
@@ -124,7 +125,6 @@ require (
  github.com/google/pprof v0.0.0-20210720184732-4bb14d4b1be1 // indirect
  github.com/google/safetext v0.0.0-20220905092116-b49f7bc46da2 // indirect
  github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 // indirect
- github.com/google/uuid v1.3.0 // indirect
  github.com/gorilla/mux v1.8.0 // indirect
  github.com/gosuri/uitable v0.0.4 // indirect
  github.com/gregjones/httpcache v0.0.0-20180305231024-9cad4c3443a7 // indirect

pkg/controller/operators/olm/operatorgroup.go

@@ -3,6 +3,7 @@ package olm
 import (
  "context"
  "fmt"
+ "github.com/google/uuid"
  "hash/fnv"
  "reflect"
  "strings"
@@ -978,33 +979,19 @@ func (a *Operator) updateNamespaceList(op *operatorsv1.OperatorGroup) ([]string,
 }
 
 func (a *Operator) ensureOpGroupClusterRole(op *operatorsv1.OperatorGroup, suffix string, apis cache.APISet) error {
- clusterRole := &rbacv1.ClusterRole{
- ObjectMeta: metav1.ObjectMeta{
- Name: strings.Join([]string{op.GetName(), suffix}, "-"),
- },
- }
- var selectors []metav1.LabelSelector
- for api := range apis {
- aggregationLabel, err := aggregationLabelFromAPIKey(api, suffix)
- if err != nil {
- return err
- }
- selectors = append(selectors, metav1.LabelSelector{
- MatchLabels: map[string]string{
- aggregationLabel: "true",
- },
- })
- }
- if len(selectors) > 0 {
- clusterRole.AggregationRule = &rbacv1.AggregationRule{
- ClusterRoleSelectors: selectors,
- }
+ clusterRole, err := a.getOpGroupClusterRole(op, suffix)
+ if err != nil {
+ return err
  }
- err := ownerutil.AddOwnerLabels(clusterRole, op)
+ clusterRole.AggregationRule, err = a.getClusterRoleAggregationRule(apis, suffix)
  if err != nil {
  return err
  }
 
+ if err := ownerutil.AddOwnerLabels(clusterRole, op); err != nil {
+ return err
+ }
+
  existingRole, err := a.lister.RbacV1().ClusterRoleLister().Get(clusterRole.Name)
  if err != nil && !apierrors.IsNotFound(err) {
  return err
@@ -1031,6 +1018,86 @@ func (a *Operator) ensureOpGroupClusterRole(op *operatorsv1.OperatorGroup, suffi
  return nil
 }
 
+func hash(str string) (string, error) {
+ // hash the string with some randomness to help avoid guessing attacks
+ h := fnv.New32a()
+ rnd := uuid.NewString()
+ if _, err := h.Write([]byte(fmt.Sprintf("%s.%s", rnd, str))); err != nil {
+ return "", err
+ }
+ return strings.Trim(fmt.Sprintf("%016x", h.Sum32()), "0"), nil
+}
+
+func (a *Operator) getOpGroupClusterRole(op *operatorsv1.OperatorGroup, suffix string) (*rbacv1.ClusterRole, error) {
+ roleNameHash, err := hash(fmt.Sprintf("%s/%s", op.GetNamespace(), op.GetName()))
+ if err != nil {
+ return nil, err
+ }
+ roleName := fmt.Sprintf("olm.operator-group.role.%s.%s", roleNameHash, suffix)
+
+ clusterRole := &rbacv1.ClusterRole{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: roleName,
+ },
+ }
+ return clusterRole, nil
+}
+
+func (a *Operator) getOldClusterRole(op *operatorsv1.OperatorGroup, suffix string) (*rbacv1.ClusterRole, error) {
+ // check if old role name exists
+ roles, err := a.lister.RbacV1().ClusterRoleLister().List(ownerutil.ClusterRoleSelector(op))
+ if err != nil && !apierrors.IsNotFound(err) {
+ return nil, err
+ }
+ var suffixRoles []*rbacv1.ClusterRole
+ for idx, _ := range roles {
+ role := roles[idx]
+ if strings.HasSuffix(role.GetName(), suffix) {
+ suffixRoles = append(suffixRoles, role)
+ }
+ }
+ // todo: finding multiple owned cluster roles with the same suffix is highly unlikely to happen. However,
+ // we need to test and document the manual clean up in case it ever happens
+ if len(suffixRoles) > 1 {
+ err := fmt.Errorf("found multiple cluster roles with suffix %s, please clean up manually", suffix)
+ a.logger.Error(err)
+ return nil, err
+ }
+ return suffixRoles[0].DeepCopy(), nil
+}
+
+func (a *Operator) getClusterRoleAggregationRule(apis cache.APISet, suffix string) (*rbacv1.AggregationRule, error) {
+ var selectors []metav1.LabelSelector
+ for api := range apis {
+ aggregationLabel, err := aggregationLabelFromAPIKey(api, suffix)
+ if err != nil {
+ return nil, err
+ }
+ selectors = append(selectors, metav1.LabelSelector{
+ MatchLabels: map[string]string{
+ aggregationLabel: "true",
+ },
+ })
+ }
+ if len(selectors) > 0 {
+ return &rbacv1.AggregationRule{
+ ClusterRoleSelectors: selectors,
+ }, nil
+ }
+ return nil, nil
+}
+
+func (a *Operator) clusterRoleExistsAndIsOwnedBy(roleName string, owner ownerutil.Owner) (bool, error) {
+ role, err := a.lister.RbacV1().ClusterRoleLister().Get(roleName)
+ if err != nil && !apierrors.IsNotFound(err) {
+ return false, err
+ }
+ if apierrors.IsNotFound(err) {
+ return false, nil
+ }
+ return ownerutil.IsOwnedBy(role, owner), nil
+}
+
 func (a *Operator) ensureOpGroupClusterRoles(op *operatorsv1.OperatorGroup, apis cache.APISet) error {
  for _, suffix := range Suffices {
  if err := a.ensureOpGroupClusterRole(op, suffix, apis); err != nil {

pkg/lib/ownerutil/util.go

@@ -272,6 +272,11 @@ func CSVOwnerSelector(owner *operatorsv1alpha1.ClusterServiceVersion) labels.Sel
  return labels.SelectorFromSet(OwnerLabel(owner, operatorsv1alpha1.ClusterServiceVersionKind))
 }
 
+// ClusterRoleSelector returns a label selector to find cluster role objects owned by owner
+func ClusterRoleSelector(owner *operatorsv1.OperatorGroup) labels.Selector {
+ return labels.SelectorFromSet(OwnerLabel(owner, operatorsv1.OperatorGroupKind))
+}
+
 // AddOwner adds an owner to the ownerref list.
 func AddOwner(object metav1.Object, owner Owner, blockOwnerDeletion, isController bool) {
  // Most of the time we won't have TypeMeta on the object, so we infer it for types we know about

test/e2e/operator_groups_e2e_test.go

@@ -340,21 +340,22 @@ var _ = Describe("Operator Group", func() {
  })
 
  // validate provided API clusterroles for the operatorgroup
- adminRole, err := c.KubernetesInterface().RbacV1().ClusterRoles().Get(context.TODO(), operatorGroup.Name+"-admin", metav1.GetOptions{})
+ roleNamePrefix := fmt.Sprintf("olm.%s.operator-group.%s.role.", opGroupNamespace, operatorGroup.Name)
+ adminRole, err := c.KubernetesInterface().RbacV1().ClusterRoles().Get(context.TODO(), roleNamePrefix+"admin", metav1.GetOptions{})
  require.NoError(GinkgoT(), err)
  adminPolicyRules := []rbacv1.PolicyRule{
  {Verbs: []string{"*"}, APIGroups: []string{mainCRD.Spec.Group}, Resources: []string{mainCRDPlural}},
  }
  require.Equal(GinkgoT(), adminPolicyRules, adminRole.Rules)
 
- editRole, err := c.KubernetesInterface().RbacV1().ClusterRoles().Get(context.TODO(), operatorGroup.Name+"-edit", metav1.GetOptions{})
+ editRole, err := c.KubernetesInterface().RbacV1().ClusterRoles().Get(context.TODO(), roleNamePrefix+"edit", metav1.GetOptions{})
  require.NoError(GinkgoT(), err)
  editPolicyRules := []rbacv1.PolicyRule{
  {Verbs: []string{"create", "update", "patch", "delete"}, APIGroups: []string{mainCRD.Spec.Group}, Resources: []string{mainCRDPlural}},
  }
  require.Equal(GinkgoT(), editPolicyRules, editRole.Rules)
 
- viewRole, err := c.KubernetesInterface().RbacV1().ClusterRoles().Get(context.TODO(), operatorGroup.Name+"-view", metav1.GetOptions{})
+ viewRole, err := c.KubernetesInterface().RbacV1().ClusterRoles().Get(context.TODO(), roleNamePrefix+"view", metav1.GetOptions{})
  require.NoError(GinkgoT(), err)
  viewPolicyRules := []rbacv1.PolicyRule{
  {Verbs: []string{"get"}, APIGroups: []string{"apiextensions.k8s.io"}, Resources: []string{"customresourcedefinitions"}, ResourceNames: []string{mainCRD.Name}},