@@ -4,8 +4,10 @@ import (
44 "context"
55 "fmt"
66 "hash/fnv"
7- "math/rand "
7+ "math"
88 "reflect"
9+ "regexp"
10+ "sort"
911 "strings"
1012
1113 "github.com/sirupsen/logrus"
@@ -35,6 +37,12 @@ const (
3537 AdminSuffix = "admin"
3638 EditSuffix = "edit"
3739 ViewSuffix = "view"
40+
41+ // kubeResourceNameLimit is the maximum length of a Kubernetes resource name
42+ kubeResourceNameLimit = 253
43+
44+ // resourceRandomPrefixLengh is the length of the random prefix added to the resource name
45+ resourceRandomPrefixLength = 5
3846)
3947
4048var (
@@ -986,46 +994,30 @@ func (a *Operator) updateNamespaceList(op *operatorsv1.OperatorGroup) ([]string,
986994 return namespaceList , nil
987995}
988996
989- func (a * Operator ) stableRand (op * operatorsv1.OperatorGroup ) (string , error ) {
990- key := fmt .Sprintf ("%s/%s" , op .GetNamespace (), op .GetName ())
991- h := fnv .New64a ()
992- _ , err := h .Write ([]byte (key ))
993- if err != nil {
994- return "" , err
995- }
996- rnd := rand .NewSource (int64 (h .Sum64 ()))
997- const alphabet = "abcdefghijklmnopqrstuvwxyz0123456789"
997+ func (a * Operator ) ensureOpGroupClusterRole (op * operatorsv1.OperatorGroup , roleType string , apis cache.APISet ) error {
998+ // create target cluster role spec
999+ var clusterRole * rbacv1.ClusterRole
9981000
999- b := make ([]byte , 7 )
1000- for i := range b {
1001- b [i ] = alphabet [rnd .Int63 ()% int64 (len (alphabet ))]
1002- }
1003- return string (b ), nil
1004- }
1001+ // to respect kubernetes resource length limitations we need to truncate the operator group name
1002+ template := "olm.og.%s.%s-" // final name will look like, e.g. olm.og.my-og.admin-pll5s
1003+ numTemplateChars := len (strings .Replace (template , "%s" , "" , - 1 ))
1004+ roleTypeLength := len (roleType )
10051005
1006- func (a * Operator ) getClusterRoleName (op * operatorsv1.OperatorGroup , roleType string ) (string , error ) {
1007- roleSuffix , err := a .stableRand (op )
1008- if err != nil {
1009- return "" , err
1010- }
1011- return fmt .Sprintf ("olm.operatorgroup.%s-%s" , roleType , roleSuffix ), nil
1012- }
1006+ // the operator group component of the name must be limited by the resource name length limit
1007+ // minus the number of characters used up by the other components of the name
1008+ nameLimit := kubeResourceNameLimit - numTemplateChars - roleTypeLength - resourceRandomPrefixLength
1009+ operatorGroupName := op .GetName ()[:int (math .Min (float64 (nameLimit ), float64 (len (op .GetName ()))))]
10131010
1014- func (a * Operator ) ensureOpGroupClusterRole (op * operatorsv1.OperatorGroup , suffix string , apis cache.APISet ) error {
1015- // create target cluster role spec
1016- var clusterRole * rbacv1.ClusterRole
1017- clusterRoleName , err := a .getClusterRoleName (op , suffix )
1018- if err != nil {
1019- return err
1020- }
1021- aggregationRule , err := a .getClusterRoleAggregationRule (apis , suffix )
1011+ clusterRoleName := fmt .Sprintf (template , operatorGroupName , roleType )
1012+ aggregationRule , err := a .getClusterRoleAggregationRule (apis , roleType )
10221013 if err != nil {
10231014 return err
10241015 }
10251016
1017+ // if we'll be creating a fresh cluster role, use generateName to avoid name collisions
10261018 clusterRole = & rbacv1.ClusterRole {
10271019 ObjectMeta : metav1.ObjectMeta {
1028- Name : clusterRoleName ,
1020+ GenerateName : clusterRoleName ,
10291021 },
10301022 AggregationRule : aggregationRule ,
10311023 }
@@ -1034,29 +1026,74 @@ func (a *Operator) ensureOpGroupClusterRole(op *operatorsv1.OperatorGroup, suffi
10341026 return err
10351027 }
10361028
1037- // get existing cluster role for this level (suffix: admin, edit, view))
1038- existingRole , err := a .lister .RbacV1 ().ClusterRoleLister ().Get ( clusterRoleName )
1039- if err != nil && ! apierrors . IsNotFound ( err ) {
1029+ // list current cluster roles owned by this operator group
1030+ clusterRoles , err := a .lister .RbacV1 ().ClusterRoleLister ().List ( ownerutil . OperatorGroupOwnerSelector ( op ) )
1031+ if err != nil {
10401032 return err
10411033 }
1042- if existingRole != nil && existingRole .Name == clusterRoleName {
1043- // if the cluster role already exists, check if it needs to be updated
1044- if labels .Equals (existingRole .Labels , clusterRole .Labels ) && reflect .DeepEqual (existingRole .AggregationRule , aggregationRule ) {
1045- return nil
1034+
1035+ // find the cluster role that matches the template and is of the correct type
1036+ var existingClusterRoles []* rbacv1.ClusterRole
1037+ re , ok := ogClusterRoleNameRegExMap [roleType ]
1038+ if ! ok {
1039+ return fmt .Errorf ("no regex found for role type: %s" , roleType )
1040+ }
1041+ for _ , cr := range clusterRoles {
1042+ if re .FindStringSubmatch (cr .GetName ()) != nil {
1043+ existingClusterRoles = append (existingClusterRoles , cr )
10461044 }
1047- // if skew was found, correct it
1048- if _ , err := a .opClient .UpdateClusterRole (clusterRole ); err != nil {
1049- a .logger .WithError (err ).Errorf ("Update existing cluster role failed: %v" , clusterRole )
1045+ }
1046+
1047+ switch len (existingClusterRoles ) {
1048+ // if no cluster role exists, create one
1049+ case 0 :
1050+ a .logger .Infof ("Creating cluster role: %s owned by operator group: %s/%s" , clusterRole .GetGenerateName (), op .GetNamespace (), op .GetName ())
1051+ if _ , err = a .opClient .KubernetesInterface ().RbacV1 ().ClusterRoles ().Create (context .TODO (), clusterRole , metav1.CreateOptions {}); err != nil {
1052+ // name collision, try again with a new name in the next reconcile
1053+ a .logger .WithError (err ).Errorf ("Create cluster role failed: %v" , clusterRole )
10501054 }
10511055 return err
1052- }
1056+ // if an existing cluster role resource is found, update it if necessary
1057+ case 1 :
1058+ existingClusterRole := existingClusterRoles [0 ].DeepCopy ()
1059+ // the cluster role will need to be updated if the aggregation rules have changed - otherwise, nothing to do
1060+ // the resource is guaranteed to have the correct ownership labels because we reached this part of the code
1061+ if ! reflect .DeepEqual (existingClusterRole .AggregationRule , aggregationRule ) {
1062+ if _ , err := a .opClient .UpdateClusterRole (existingClusterRole ); err != nil {
1063+ a .logger .WithError (err ).Errorf ("Update existing cluster role failed: %v" , clusterRole )
1064+ }
1065+ return err
1066+ }
1067+ // the inherent race condition created by listing to check if a resource exists and then creating it
1068+ // and the fact that we are using generateName means that it is possible for multiple cluster roles of the same
1069+ // role type to be created for the same operator group. In this case, we'll disambiguate by keeping the oldest
1070+ // cluster role (by creation timestamp - tie-breaking by name) and deleting the others
1071+ default :
1072+ a .logger .Warnf ("multiple (%d) cluster roles of type %s owned by operator group %s/%s found" , len (existingClusterRoles ), roleType , op .GetNamespace (), op .GetName ())
1073+ // sort by creation timestamp and tie-break by name
1074+ sort .Slice (existingClusterRoles , func (i , j int ) bool {
1075+ creationTimeI := existingClusterRoles [i ].GetCreationTimestamp ()
1076+ creationTimeJ := existingClusterRoles [j ].GetCreationTimestamp ()
1077+ if creationTimeI .Equal (& creationTimeJ ) {
1078+ return strings .Compare (existingClusterRoles [i ].GetName (), existingClusterRoles [j ].GetName ()) < 0
1079+ }
1080+ return creationTimeI .Before (& creationTimeJ )
1081+ })
1082+
1083+ // delete all but the oldest cluster role
1084+ a .logger .Infof ("keeping cluster role: %s owned by operator group: %s/%s and deleting %d others" , existingClusterRoles [0 ].GetName (), op .GetNamespace (), op .GetName (), len (existingClusterRoles )- 1 )
1085+ for _ , cr := range existingClusterRoles [1 :] {
1086+ a .logger .Infof ("deleting cluster role: %s owned by operator group: %s/%s - there may be only one" , cr .GetName (), op .GetNamespace (), op .GetName ())
1087+ if err := a .opClient .DeleteClusterRole (cr .GetName (), & metav1.DeleteOptions {}); err != nil {
1088+ a .logger .WithError (err ).Errorf ("Delete cluster role failed: %v" , cr )
1089+ return err
1090+ }
1091+ }
10531092
1054- a .logger .Infof ("Creating cluster role: %s owned by operator group: %s/%s" , clusterRole .GetName (), op .GetNamespace (), op .GetName ())
1055- if _ , err = a .opClient .KubernetesInterface ().RbacV1 ().ClusterRoles ().Create (context .TODO (), clusterRole , metav1.CreateOptions {}); err != nil {
1056- // name collision, try again with a new name in the next reconcile
1057- a .logger .WithError (err ).Errorf ("Create cluster role failed: %v" , clusterRole )
1093+ // now that we're (hopefully) down to one cluster role, re-reconcile
1094+ return fmt .Errorf ("multiple cluster roles of type %s owned by operator group %s/%s found" , roleType , op .GetNamespace (), op .GetName ())
10581095 }
1059- return err
1096+ return nil
10601097}
10611098
10621099func (a * Operator ) getClusterRoleAggregationRule (apis cache.APISet , suffix string ) (* rbacv1.AggregationRule , error ) {
@@ -1174,3 +1211,15 @@ func csvCopyPrototype(src, dst *v1alpha1.ClusterServiceVersion) {
11741211 dst .Status .Reason = v1alpha1 .CSVReasonCopied
11751212 dst .Status .Message = fmt .Sprintf ("The operator is running in %s but is managing this namespace" , src .GetNamespace ())
11761213}
1214+
1215+ // ogClusterRoleNameRegEx returns a regexp for the naming format used for cluster roles owned by operator groups
1216+ // for a particular role type e.g. olm.og.my-og.admin-pll2k (where pll2k is a random suffix and admin is the role type)
1217+ var ogClusterRoleNameRegExMap = map [string ]* regexp.Regexp {
1218+ "admin" : ogClusterRoleNameRegEx ("admin" ),
1219+ "edit" : ogClusterRoleNameRegEx ("edit" ),
1220+ "view" : ogClusterRoleNameRegEx ("view" ),
1221+ }
1222+
1223+ func ogClusterRoleNameRegEx (roleType string ) * regexp.Regexp {
1224+ return regexp .MustCompile (fmt .Sprintf (`^olm\.og\.[^\.]+\.%s-[a-z0-9]{5}$` , roleType ))
1225+ }
0 commit comments