diff --git a/components/odh-notebook-controller/config/rbac/role.yaml b/components/odh-notebook-controller/config/rbac/role.yaml index 1551e236714..94be8c5e262 100644 --- a/components/odh-notebook-controller/config/rbac/role.yaml +++ b/components/odh-notebook-controller/config/rbac/role.yaml @@ -58,6 +58,29 @@ rules: - patch - update - watch +- apiGroups: + - rbac.authorization.k8s.io + resources: + - rolebindings + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - rbac.authorization.k8s.io + resources: + - roles + verbs: + - create + - get + - list + - patch + - update + - watch - apiGroups: - route.openshift.io resources: diff --git a/components/odh-notebook-controller/controllers/notebook_controller.go b/components/odh-notebook-controller/controllers/notebook_controller.go index cce2e87386f..f584678a0a0 100644 --- a/components/odh-notebook-controller/controllers/notebook_controller.go +++ b/components/odh-notebook-controller/controllers/notebook_controller.go @@ -32,6 +32,7 @@ import ( "github.com/kubeflow/kubeflow/components/notebook-controller/pkg/culler" routev1 "github.com/openshift/api/route/v1" corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" apierrs "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -67,6 +68,8 @@ type OpenshiftNotebookReconciler struct { // +kubebuilder:rbac:groups="",resources=services;serviceaccounts;secrets;configmaps,verbs=get;list;watch;create;update;patch // +kubebuilder:rbac:groups=config.openshift.io,resources=proxies,verbs=get;list;watch // +kubebuilder:rbac:groups=networking.k8s.io,resources=networkpolicies,verbs=get;list;watch;create;update;patch +// +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles,verbs=get;list;watch;create;update;patch +// +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=rolebindings,verbs=get;list;watch;create;update;patch;delete // CompareNotebooks checks if two notebooks are equal, if not return false. func CompareNotebooks(nb1 nbv1.Notebook, nb2 nbv1.Notebook) bool { @@ -184,6 +187,12 @@ func (r *OpenshiftNotebookReconciler) Reconcile(ctx context.Context, req ctrl.Re return ctrl.Result{}, err } + // Call the Rolebinding reconciler + err = r.ReconcileRoleBindings(notebook, ctx) + if err != nil { + return ctrl.Result{}, err + } + if !ServiceMeshIsEnabled(notebook.ObjectMeta) { // Create the objects required by the OAuth proxy sidecar (see notebook_oauth.go file) if OAuthInjectionIsEnabled(notebook.ObjectMeta) { @@ -445,6 +454,7 @@ func (r *OpenshiftNotebookReconciler) SetupWithManager(mgr ctrl.Manager) error { Owns(&corev1.Service{}). Owns(&corev1.Secret{}). Owns(&netv1.NetworkPolicy{}). + Owns(&rbacv1.RoleBinding{}). // Watch for all the required ConfigMaps // odh-trusted-ca-bundle, kube-root-ca.crt, workbench-trusted-ca-bundle diff --git a/components/odh-notebook-controller/controllers/notebook_controller_test.go b/components/odh-notebook-controller/controllers/notebook_controller_test.go index 87369dd78ef..03c982a0fc3 100644 --- a/components/odh-notebook-controller/controllers/notebook_controller_test.go +++ b/components/odh-notebook-controller/controllers/notebook_controller_test.go @@ -24,6 +24,7 @@ import ( "github.com/go-logr/logr" "github.com/onsi/gomega/format" netv1 "k8s.io/api/networking/v1" + rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/api/resource" . "github.com/onsi/ginkgo" @@ -161,6 +162,70 @@ var _ = Describe("The Openshift Notebook controller", func() { }) + // New test case for RoleBinding reconciliation + When("ReconcileRoleBindings is called for a Notebook", func() { + const ( + name = "test-notebook-rolebinding" + namespace = "default" + ) + notebook := createNotebook(name, namespace) + + // Define the role and role-binding names and types used in the reconciliation + roleRefName := "ds-pipeline-user-access-dspa" + roleBindingName := "elyra-pipelines-" + name + + It("Should create a RoleBinding when the referenced Role exists", func() { + ctx := context.Background() + + By("Creating a Notebook and ensuring the Role exists") + Expect(cli.Create(ctx, notebook)).Should(Succeed()) + time.Sleep(interval) + + // Simulate the Role required by RoleBinding + role := &rbacv1.Role{ + ObjectMeta: metav1.ObjectMeta{ + Name: roleRefName, + Namespace: namespace, + }, + } + Expect(cli.Create(ctx, role)).Should(Succeed()) + defer func() { + if err := cli.Delete(ctx, role); err != nil { + GinkgoT().Logf("Failed to delete Role: %v", err) + } + }() + + By("Checking that the RoleBinding is created") + roleBinding := &rbacv1.RoleBinding{} + Eventually(func() error { + return cli.Get(ctx, types.NamespacedName{Name: roleBindingName, Namespace: namespace}, roleBinding) + }, duration, interval).Should(Succeed()) + + Expect(roleBinding.RoleRef.Name).To(Equal(roleRefName)) + Expect(roleBinding.Subjects[0].Name).To(Equal(name)) + Expect(roleBinding.Subjects[0].Kind).To(Equal("ServiceAccount")) + }) + + It("Should delete the RoleBinding when the Notebook is deleted", func() { + ctx := context.Background() + + By("Ensuring the RoleBinding exists") + roleBinding := &rbacv1.RoleBinding{} + Eventually(func() error { + return cli.Get(ctx, types.NamespacedName{Name: roleBindingName, Namespace: namespace}, roleBinding) + }, duration, interval).Should(Succeed()) + + By("Deleting the Notebook") + Expect(cli.Delete(ctx, notebook)).Should(Succeed()) + + By("Ensuring the RoleBinding is deleted") + Eventually(func() error { + return cli.Get(ctx, types.NamespacedName{Name: roleBindingName, Namespace: namespace}, roleBinding) + }, duration, interval).Should(Succeed()) + }) + + }) + // New test case for notebook creation When("Creating a Notebook, test certificate is mounted", func() { const ( diff --git a/components/odh-notebook-controller/controllers/notebook_rbac.go b/components/odh-notebook-controller/controllers/notebook_rbac.go new file mode 100644 index 00000000000..73a2d98bb63 --- /dev/null +++ b/components/odh-notebook-controller/controllers/notebook_rbac.go @@ -0,0 +1,166 @@ +/* + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controllers + +import ( + "context" + "reflect" + + nbv1 "github.com/kubeflow/kubeflow/components/notebook-controller/api/v1" + rbacv1 "k8s.io/api/rbac/v1" + apierrs "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" +) + +// NewRoleBinding defines the desired RoleBinding or ClusterRoleBinding object. +// Parameters: +// - notebook: The Notebook resource instance for which the RoleBinding or ClusterRoleBinding is being created. +// - rolebindingName: The name to assign to the RoleBinding or ClusterRoleBinding object. +// - roleRefKind: The kind of role reference to bind to, which can be either Role or ClusterRole. +// - roleRefName: The name of the Role or ClusterRole to reference. +func NewRoleBinding(notebook *nbv1.Notebook, rolebindingName, roleRefKind, roleRefName string) *rbacv1.RoleBinding { + return &rbacv1.RoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: rolebindingName, + Namespace: notebook.Namespace, + Labels: map[string]string{ + "notebook-name": notebook.Name, + }, + }, + Subjects: []rbacv1.Subject{ + { + Kind: "ServiceAccount", + Name: notebook.Name, + Namespace: notebook.Namespace, + }, + }, + RoleRef: rbacv1.RoleRef{ + Kind: roleRefKind, + Name: roleRefName, + APIGroup: "rbac.authorization.k8s.io", + }, + } +} + +// checkRoleExists checks if a Role or ClusterRole exists in the namespace. +func (r *OpenshiftNotebookReconciler) checkRoleExists(ctx context.Context, roleRefKind, roleRefName, namespace string) (bool, error) { + if roleRefKind == "ClusterRole" { + // Check ClusterRole if roleRefKind is ClusterRole + clusterRole := &rbacv1.ClusterRole{} + err := r.Get(ctx, types.NamespacedName{Name: roleRefName}, clusterRole) + if err != nil { + if apierrs.IsNotFound(err) { + // ClusterRole not found + return false, nil + } + return false, err // Some other error occurred + } + } else { + // Check Role if roleRefKind is Role + role := &rbacv1.Role{} + err := r.Get(ctx, types.NamespacedName{Name: roleRefName, Namespace: namespace}, role) + if err != nil { + if apierrs.IsNotFound(err) { + // Role not found + return false, nil + } + return false, err // Some other error occurred + } + } + return true, nil // Role or ClusterRole exists +} + +// Helper function to delete the RoleBinding if the notebooks deleted +func (r *OpenshiftNotebookReconciler) deleteRoleBinding(ctx context.Context, rolebindingName, namespace string) error { + roleBinding := &rbacv1.RoleBinding{} + err := r.Get(ctx, types.NamespacedName{Name: rolebindingName, Namespace: namespace}, roleBinding) + if err != nil { + if apierrs.IsNotFound(err) { + return nil // RoleBinding not found, nothing to delete + } + return err // Some other error occurred + } + + // Delete the RoleBinding + return r.Delete(ctx, roleBinding) +} + +// reconcileRoleBinding manages creation, update, and deletion of RoleBindings and ClusterRoleBindings +func (r *OpenshiftNotebookReconciler) reconcileRoleBinding( + notebook *nbv1.Notebook, ctx context.Context, rolebindingName, roleRefKind, roleRefName string) error { + + log := r.Log.WithValues("notebook", types.NamespacedName{Name: notebook.Name, Namespace: notebook.Namespace}) + + // Check if the Role or ClusterRole exists before proceeding + roleExists, err := r.checkRoleExists(ctx, roleRefKind, roleRefName, notebook.Namespace) + if err != nil { + log.Error(err, "Error checking if Role exists", "Role.Kind", roleRefKind, "Role.Name", roleRefName) + return err + } + if !roleExists { + return nil // Skip if dspa Role is not found on the namespace + } + + // Create a new RoleBinding based on provided parameters + roleBinding := NewRoleBinding(notebook, rolebindingName, roleRefKind, roleRefName) + + // Check if the RoleBinding already exists + found := &rbacv1.RoleBinding{} + err = r.Get(ctx, types.NamespacedName{Name: rolebindingName, Namespace: notebook.Namespace}, found) + if err != nil && apierrs.IsNotFound(err) { + log.Info("Creating RoleBinding", "RoleBinding.Namespace", roleBinding.Namespace, "RoleBinding.Name", roleBinding.Name) + err = r.Create(ctx, roleBinding) + if err != nil { + log.Error(err, "Failed to create RoleBinding", "RoleBinding.Namespace", roleBinding.Namespace, "RoleBinding.Name", roleBinding.Name) + return err + } + return nil + } else if err != nil { + log.Error(err, "Failed to get RoleBinding") + return err + } + + // Update RoleBinding if the subjects differ + if !reflect.DeepEqual(roleBinding.Subjects, found.Subjects) { + log.Info("Updating RoleBinding", "RoleBinding.Namespace", roleBinding.Namespace, "RoleBinding.Name", roleBinding.Name) + err = r.Update(ctx, roleBinding) + if err != nil { + log.Error(err, "Failed to update RoleBinding", "RoleBinding.Namespace", roleBinding.Namespace, "RoleBinding.Name", roleBinding.Name) + return err + } + } + + return nil +} + +// ReconcileRoleBindings will manage multiple RoleBinding and ClusterRoleBinding reconciliations +func (r *OpenshiftNotebookReconciler) ReconcileRoleBindings( + notebook *nbv1.Notebook, ctx context.Context) error { + + // Reconcile a RoleBinding for pipelines for the notebook service account + roleBindingName := "elyra-pipelines-" + notebook.Name + if err := r.reconcileRoleBinding(notebook, ctx, roleBindingName, "Role", "ds-pipeline-user-access-dspa"); err != nil { + return err + } + + // If the notebook is marked for deletion, remove the associated RoleBinding + if !notebook.ObjectMeta.DeletionTimestamp.IsZero() { + return r.deleteRoleBinding(ctx, roleBindingName, notebook.Namespace) + } + + return nil +}