From b18369b993da993ce0fa577a103c288dcaeb118d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Felipe=20Godoy=20Hern=C3=A1ndez?= Date: Thu, 25 Nov 2021 11:12:11 -0300 Subject: [PATCH] Fix ClusterRole & ClusterRoleBinding hotreload (#92) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Watch every minute if a CustomGroup rule change and apply them * Fix: OwnerRefInvalidNamespace when refering to a Service Account that doesn't exists in the same namespace * fix conflict * bump up helm chart patch version * Fix: OwnerRefInvalidNamespace when refering to a Service Account that doesn't exists in the same namespace * bump up helm chart patch version * Fix ldap Service Account when email has `_` * Fix ldap Service Account when email has `_` Signed-off-by: Victor Godoy Hernández * bump up helm chart Signed-off-by: Victor Godoy Hernández * Add Test for emails with special characters Signed-off-by: Victor Godoy Hernández * Fix ClusterRole & ClusterRoleBinding hot reload Signed-off-by: Victor Godoy Hernández * Delete garbage ClusterRoles Signed-off-by: Victor Godoy Hernández * Fix missing error handle Signed-off-by: Victor Godoy Hernández * Fix missing error handle Signed-off-by: Victor Godoy Hernández * Delete ClusterRoleBinding or RoleBinding that reference ClusterRole Signed-off-by: Victor Godoy Hernández * Fix Errors unhandled Signed-off-by: Victor Godoy Hernández --- charts/jwt-to-rbac/Chart.yaml | 4 +- pkg/rbachandler/rbac_handler.go | 162 +++++++++++++++++++++----------- pkg/rbachandler/rbac_watcher.go | 68 ++++++++++++++ 3 files changed, 177 insertions(+), 57 deletions(-) diff --git a/charts/jwt-to-rbac/Chart.yaml b/charts/jwt-to-rbac/Chart.yaml index 385dd58..b0d027e 100644 --- a/charts/jwt-to-rbac/Chart.yaml +++ b/charts/jwt-to-rbac/Chart.yaml @@ -1,8 +1,8 @@ apiVersion: v2 -appVersion: 0.6.4 +appVersion: 0.6.5 description: A Helm chart for Kubernetes name: jwt-to-rbac -version: 0.6.4 +version: 0.6.5 home: https://github.com/banzaicloud/jwt-to-rbac maintainers: - name: BanzaiCloud diff --git a/pkg/rbachandler/rbac_handler.go b/pkg/rbachandler/rbac_handler.go index 28eea9d..ae0044e 100644 --- a/pkg/rbachandler/rbac_handler.go +++ b/pkg/rbachandler/rbac_handler.go @@ -187,6 +187,32 @@ func (rh *RBACHandler) listClusterroleBindings() ([]string, error) { return cRoleBindList, nil } +func (rh *RBACHandler) deleteLinkedCRoleBinding(crole string) error{ + bindings := rh.rbacClientSet.ClusterRoleBindings() + labelSelector := fmt.Sprintf("crole=%s", crole) + listOptions := metav1.ListOptions{ + LabelSelector: labelSelector, + } + err := bindings.DeleteCollection(&metav1.DeleteOptions{}, listOptions) + if err != nil { + return emperror.WrapWith(err, "unable to delete collection of clusterrolebindings", "ListOptions", metav1.ListOptions{}) + } + return nil +} + +func (rh *RBACHandler) deleteLinkedRoleBinding(crole string, namespace string) error{ + bindings := rh.rbacClientSet.RoleBindings(namespace) + labelSelector := fmt.Sprintf("crole=%s", crole) + listOptions := metav1.ListOptions{ + LabelSelector: labelSelector, + } + err := bindings.DeleteCollection(&metav1.DeleteOptions{}, listOptions) + if err != nil { + return emperror.WrapWith(err, "unable to delete collection of clusterrolebindings", "ListOptions", metav1.ListOptions{}) + } + return nil +} + func (rh *RBACHandler) listClusterroles() ([]string, error) { clusterRoles := rh.rbacClientSet.ClusterRoles() labelSelect := fmt.Sprintf("%s=%s", defautlLabelKey, defaultLabel[defautlLabelKey]) @@ -245,9 +271,6 @@ func (rh *RBACHandler) createServiceAccount(sa *ServiceAccount) error { } func (rh *RBACHandler) createClusterRoleBinding(crb *clusterRoleBinding) error { - if err := rh.getAndCheckCRoleBinding(crb.name); err == nil { - return nil - } var subjects []apirbacv1.Subject for _, ns := range crb.nameSpace { subject := apirbacv1.Subject{ @@ -258,6 +281,14 @@ func (rh *RBACHandler) createClusterRoleBinding(crb *clusterRoleBinding) error { } subjects = append(subjects, subject) } + + customLabels := map[string]string{ + "crole": crb.roleName, + } + for key, value := range customLabels { + crb.labels[key] = value + } + bindObj := &apirbacv1.ClusterRoleBinding{ TypeMeta: metav1.TypeMeta{ Kind: "ClusterRoleBinding", @@ -287,9 +318,6 @@ func (rh *RBACHandler) createClusterRoleBinding(crb *clusterRoleBinding) error { } func (rh *RBACHandler) createRoleBinding(rb *roleBinding) error { - if err := rh.getAndCheckRoleBinding(rb.name, rb.nameSpace); err == nil { - return nil - } var subjects []apirbacv1.Subject for _, ns := range rb.nameSpace { subject := apirbacv1.Subject{ @@ -301,6 +329,13 @@ func (rh *RBACHandler) createRoleBinding(rb *roleBinding) error { } subjects = append(subjects, subject) + customLabels := map[string]string{ + "crole": rb.roleName, + } + for key, value := range customLabels { + rb.labels[key] = value + } + bindObj := &apirbacv1.RoleBinding{ TypeMeta: metav1.TypeMeta{ Kind: "RoleBinding", @@ -328,9 +363,6 @@ func (rh *RBACHandler) createRoleBinding(rb *roleBinding) error { } func (rh *RBACHandler) createClusterRole(cr *clusterRole) error { - if err := rh.getAndCheckCRole(cr.name); err == nil { - return nil - } var rules []apirbacv1.PolicyRule for _, rule := range cr.rules { rule := apirbacv1.PolicyRule{ @@ -358,6 +390,20 @@ func (rh *RBACHandler) createClusterRole(cr *clusterRole) error { return nil } +func (rh *RBACHandler) getAndCheckCRole(CRName string) error { + cRole, err := rh.rbacClientSet.ClusterRoles().Get(CRName, metav1.GetOptions{}) + if err == nil { + if label, ok := cRole.ObjectMeta.Labels[defautlLabelKey]; !ok || label != defaultLabel[defautlLabelKey] { + return emperror.WrapWith(errors.New("label mismatch in clusterrole"), + "there is a ClusterRole without required label", + defautlLabelKey, defaultLabel[defautlLabelKey], + "cluster_role", CRName) + } + return nil + } + return err +} + func generateRules(groupName string, config *Config) []rule { var cRules []rule for _, cGroup := range config.CustomGroups { @@ -571,52 +617,6 @@ func (rh *RBACHandler) getAndCheckSA(saName string) (*apicorev1.ServiceAccount, return saDetails, nil } -func (rh *RBACHandler) getAndCheckCRole(CRName string) error { - cRole, err := rh.rbacClientSet.ClusterRoles().Get(CRName, metav1.GetOptions{}) - if err == nil { - if label, ok := cRole.ObjectMeta.Labels[defautlLabelKey]; !ok || label != defaultLabel[defautlLabelKey] { - return emperror.WrapWith(errors.New("label mismatch in clusterrole"), - "there is a ClusterRole without required label", - defautlLabelKey, defaultLabel[defautlLabelKey], - "cluster_role", CRName) - } - return nil - } - return err -} - -func (rh *RBACHandler) getAndCheckCRoleBinding(CRBindingName string) error { - cRoleBind, err := rh.rbacClientSet.ClusterRoleBindings().Get(CRBindingName, metav1.GetOptions{}) - if err == nil { - if label, ok := cRoleBind.ObjectMeta.Labels[defautlLabelKey]; !ok || label != defaultLabel[defautlLabelKey] { - return emperror.WrapWith(errors.New("label mismatch in clusterrole"), - "there is a ClusterRoleBinding without required label", - defautlLabelKey, defaultLabel[defautlLabelKey], - "cluster_rolebinding", CRBindingName) - } - return nil - } - return err -} - -func (rh *RBACHandler) getAndCheckRoleBinding(RBindingName string, NameSpaces []string) error { - - for _, namespace := range NameSpaces { - roleBind, err := rh.rbacClientSet.RoleBindings(namespace).Get(RBindingName, metav1.GetOptions{}) - if err == nil { - if label, ok := roleBind.ObjectMeta.Labels[defautlLabelKey]; !ok || label != defaultLabel[defautlLabelKey] { - return emperror.WrapWith(errors.New("label mismatch in clusterrole"), - "there is a RoleBinding without required label", - defautlLabelKey, defaultLabel[defautlLabelKey], - "rolebinding", RBindingName) - } - continue - } - return err - } - return nil -} - func (rh *RBACHandler) getSAReference(saName string) ([]metav1.OwnerReference, error) { saDetails, err := rh.getAndCheckSA(saName) if err != nil { @@ -643,6 +643,20 @@ func (rh *RBACHandler) removeServiceAccount(saName string, logger logur.Logger) return nil } +func (rh *RBACHandler) removeClusterRole(clusterRole string, logger logur.Logger) error { + if err := rh.getAndCheckCRole(clusterRole); err != nil { + return err + } + deleteOptions := &metav1.DeleteOptions{} + bg := metav1.DeletePropagationBackground + deleteOptions.PropagationPolicy = &bg + err := rh.rbacClientSet.ClusterRoles().Delete(clusterRole, deleteOptions) + if err != nil { + return emperror.WrapWith(err, "unable to delete ClusterRole", "cluster_role", clusterRole) + } + return nil +} + // DeleteRBAC deletes RBAC resources func DeleteRBAC(saName string, config *Config, logger logur.Logger) error { rbacHandler, err := NewRBACHandler(config.KubeConfig, logger) @@ -656,6 +670,19 @@ func DeleteRBAC(saName string, config *Config, logger logur.Logger) error { return nil } +// DeleteClusterRole deletes ClusterRole resources +func DeleteClusterRole(clusterRole string, config *Config, logger logur.Logger) error { + rbacHandler, err := NewRBACHandler(config.KubeConfig, logger) + if err != nil { + return err + } + if err := rbacHandler.removeClusterRole(clusterRole, logger); err != nil { + logger.Error(err.Error(), nil) + return err + } + return nil +} + // GetK8sToken getting serviceaccount secrets data func GetK8sToken(saName string, config *Config, logger logur.Logger) ([]*SACredential, error) { rbacHandler, err := NewRBACHandler(config.KubeConfig, logger) @@ -808,3 +835,28 @@ func tokenRandString(n int) string { } return "-token-" + string(b) } + +func (rh *RBACHandler) listCustomGroups(config *Config) ([]string) { + var customGroups []string + + for _, group := range config.CustomGroups { + customGroups = append(customGroups, group.GroupName) + } + + return customGroups +} + +func (rh *RBACHandler) listNamespaces() ([]string, error) { + var rnamespace []string + namespacelist, err :=rh.coreClientSet.Namespaces().List(metav1.ListOptions{}) + + if err != nil { + return nil, emperror.WrapWith(err, "List namespaces failed", "listNamespaces") + } + + for _, namespace := range namespacelist.Items{ + rnamespace = append(rnamespace, namespace.Name) + } + + return rnamespace, nil +} diff --git a/pkg/rbachandler/rbac_watcher.go b/pkg/rbachandler/rbac_watcher.go index 30bd5c4..e3b787c 100644 --- a/pkg/rbachandler/rbac_watcher.go +++ b/pkg/rbachandler/rbac_watcher.go @@ -16,6 +16,7 @@ package rbachandler import ( "fmt" + "strings" "time" "github.com/goph/emperror" @@ -63,6 +64,11 @@ func (rh *RBACHandler) evaluateClusterRoles(config *Config, logger logur.Logger, return err } + err = checkClusterRole(config, logger) + if err != nil { + return err + } + rbacResources, err := generateClusterRoleRBACResources(config, logger) if err != nil { return err @@ -116,3 +122,65 @@ func (rh *RBACHandler) checkTTL(secretName string) error { } return nil } + +func checkClusterRole(config *Config, logger logur.Logger) error { + var existingCustomGroups []string + rbacHandler, err := NewRBACHandler(config.KubeConfig, logger) + if err != nil { + return err + } + + existingClusterRoles, err := rbacHandler.listClusterroles() + if err != nil { + return err + } + + for _, clusterRole := range existingClusterRoles { + existingCustomGroups = append(existingCustomGroups, strings.ReplaceAll(clusterRole, "-from-jwt", "")) + } + + customGroups := rbacHandler.listCustomGroups(config) + + err = removeCustomGroupsDifference(existingCustomGroups, customGroups, config, logger) + if err != nil { + return err + } + + return nil +} + +func removeCustomGroupsDifference(existingClusterRoles []string, configCustomGroups []string, config *Config, logger logur.Logger) error { + rbacHandler, err := NewRBACHandler(config.KubeConfig, logger) + if err != nil { + return err + } + + mb := make(map[string]struct{}, len(configCustomGroups)) + for _, x := range configCustomGroups { + mb[x] = struct{}{} + } + + for _, x := range existingClusterRoles { + if _, found := mb[x]; !found { + err = rbacHandler.deleteLinkedCRoleBinding(x + "-from-jwt") + if err != nil { + return err + } + namespaces, err := rbacHandler.listNamespaces() + if err != nil { + return err + } + for _, namespace := range namespaces { + err = rbacHandler.deleteLinkedRoleBinding(x + "-from-jwt", namespace) + if err != nil { + return err + } + } + err = DeleteClusterRole(x + "-from-jwt", config, logger) + if err != nil { + return err + } + } + } + return nil +}