From acbd6446881377b3e5f0c9c2950b9ed73e97e65b Mon Sep 17 00:00:00 2001
From: Jean-Philippe Evrard <jean-philippe.evrard-prestataire@ca-gip.fr>
Date: Mon, 13 Jan 2025 20:16:41 +0100
Subject: [PATCH] Support special rolebindings

Without this patch, the rolebindings for appops, customerops,
... are only using the old names, not the LDAP groups.

This is a problem, as it will prevent us to clean up the rest
of the code later.

This fixes it by relying on the config of the group base to
know which group to add in the rolebinding.

In the future, it would be best to template those from a
config map: it would help to not being directly interconnected
to LDAP by having a simple config to template for the cluster.
It would also be simpler to manage with a gitops tool.
---
 internal/services/rolebindings.go      | 45 ++++++++++++++++++++++++--
 internal/services/rolebindings_test.go | 36 +++++++++++++++++++++
 2 files changed, 79 insertions(+), 2 deletions(-)

diff --git a/internal/services/rolebindings.go b/internal/services/rolebindings.go
index e9b690e5..2bee01de 100644
--- a/internal/services/rolebindings.go
+++ b/internal/services/rolebindings.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"log/slog"
+	"strings"
 
 	"github.com/ca-gip/kubi/internal/utils"
 	cagipv1 "github.com/ca-gip/kubi/pkg/apis/cagip/v1"
@@ -24,13 +25,19 @@ var RoleBindingsCreation = promauto.NewCounterVec(prometheus.CounterOpts{
 
 // generateRoleBinding is convenience function for readability, returning a properly formatted rolebinding object.
 func newRoleBinding(name string, namespace string, clusterRole string, subjects []v1.Subject) *v1.RoleBinding {
+	var validSubjects []v1.Subject
+	for _, subject := range subjects {
+		if subject.Name != "" {
+			validSubjects = append(validSubjects, subject)
+		}
+	}
 	return &v1.RoleBinding{
 		RoleRef: v1.RoleRef{
 			APIGroup: "rbac.authorization.k8s.io",
 			Kind:     "ClusterRole",
 			Name:     clusterRole,
 		},
-		Subjects: subjects,
+		Subjects: validSubjects,
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      name,
 			Namespace: namespace,
@@ -102,7 +109,22 @@ func generateRoleBindings(project *cagipv1.Project, defaultServiceAccountRole st
 				{
 					APIGroup: "rbac.authorization.k8s.io",
 					Kind:     "Group",
-					Name:     project.Spec.SourceEntity,
+					Name:     project.Spec.SourceEntity, // the equivalent of $namespace-admin
+				},
+				{
+					APIGroup: "rbac.authorization.k8s.io",
+					Kind:     "Group",
+					Name:     toSubject(utils.Config.Ldap.AppMasterGroupBase), // the equivalent of application master (appops)
+				},
+				{
+					APIGroup: "rbac.authorization.k8s.io",
+					Kind:     "Group",
+					Name:     toSubject(utils.Config.Ldap.CustomerOpsGroupBase), // the equivalent of application master (customerops)
+				},
+				{
+					APIGroup: "rbac.authorization.k8s.io",
+					Kind:     "Group",
+					Name:     toSubject(utils.Config.Ldap.OpsMasterGroupBase), // the equivalent of ops master
 				},
 			},
 		},
@@ -115,6 +137,11 @@ func generateRoleBindings(project *cagipv1.Project, defaultServiceAccountRole st
 					Kind:     "Group",
 					Name:     utils.ApplicationViewer,
 				},
+				{
+					APIGroup: "rbac.authorization.k8s.io",
+					Kind:     "Group",
+					Name:     toSubject(utils.Config.Ldap.ViewerGroupBase),
+				},
 			},
 		},
 		{
@@ -152,3 +179,17 @@ func generateRoleBindings(project *cagipv1.Project, defaultServiceAccountRole st
 		}
 	}
 }
+
+// Quick and hacky way to parse DN from config, without having to load an ldap parser or doing any query
+// if not valid, return an empty string, which does not get applied in the list of subjects, as:
+// 1. it's not valid to not have a name
+// 2. We check whether we have a name in the generation of the rolebinding object.
+func toSubject(DN string) string {
+	parts := strings.Split(DN, ",")
+	for _, part := range parts {
+		if strings.HasPrefix(part, "CN=") {
+			return strings.TrimPrefix(part, "CN=")
+		}
+	}
+	return ""
+}
diff --git a/internal/services/rolebindings_test.go b/internal/services/rolebindings_test.go
index ef76189b..e4330120 100644
--- a/internal/services/rolebindings_test.go
+++ b/internal/services/rolebindings_test.go
@@ -60,3 +60,39 @@ func TestNewRoleBinding(t *testing.T) {
 		})
 	}
 }
+func TestToSubject(t *testing.T) {
+	tests := []struct {
+		DN       string
+		expected string
+	}{
+		{
+			DN:       "CN=test-user,OU=Users,DC=example,DC=com",
+			expected: "test-user",
+		},
+		{
+			DN:       "OU=Users,CN=test-user,DC=example,DC=com",
+			expected: "test-user",
+		},
+		{
+			DN:       "OU=Users,DC=example,DC=com",
+			expected: "",
+		},
+		{
+			DN:       "CN=test-user",
+			expected: "test-user",
+		},
+		{
+			DN:       "",
+			expected: "",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.DN, func(t *testing.T) {
+			result := toSubject(tt.DN)
+			if result != tt.expected {
+				t.Errorf("toSubject(%v) = %v, expected %v", tt.DN, result, tt.expected)
+			}
+		})
+	}
+}