Skip to content

Commit

Permalink
update
Browse files Browse the repository at this point in the history
  • Loading branch information
atheo89 committed Oct 28, 2024
1 parent 3923fbd commit 8cf6263
Showing 1 changed file with 34 additions and 6 deletions.
40 changes: 34 additions & 6 deletions components/odh-notebook-controller/controllers/notebook_rbac.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,29 @@ func NewPipelineRoleBinding(notebook *nbv1.Notebook) *rbacv1.RoleBinding {
},
RoleRef: rbacv1.RoleRef{
Kind: "Role",
Name: "ds-pipeline-user-access-dspa", //TODO: Putit outside
Name: "ds-pipeline-user-access-dspa", // TODO: Make this configurable if needed
APIGroup: "rbac.authorization.k8s.io",
},
}
}

// reconcileRoleBinding manages the creation, update, and deletion of the role binding when the notebook is reconciled
// roleExists checks if a Role exists in the specified namespace
func (r *OpenshiftNotebookReconciler) roleExists(ctx context.Context, notebook *nbv1.Notebook) (bool, error) {
role := &rbacv1.Role{}
err := r.Get(ctx, types.NamespacedName{
Name: "ds-pipeline-user-access-dspa", // The role to check
Namespace: notebook.Namespace,
}, role)
if err != nil {
if apierrs.IsNotFound(err) {
return false, nil // Role does not exist
}
return false, err // Some other error occurred
}
return true, nil // Role exists
}

// reconcileRoleBinding manages the creation and updating of the role binding when the notebook is reconciled
func (r *OpenshiftNotebookReconciler) reconcileRoleBinding(
notebook *nbv1.Notebook,
ctx context.Context,
Expand All @@ -50,12 +66,23 @@ func (r *OpenshiftNotebookReconciler) reconcileRoleBinding(
// Initialize the logger
log := r.Log.WithValues("notebook", types.NamespacedName{Name: notebook.Name, Namespace: notebook.Namespace})

// Check if the required Role exists before proceeding
roleExists, err := r.roleExists(ctx, notebook)
if err != nil {
log.Error(err, "Failed to check for role existence")
return err // You may choose to handle this differently
}
if !roleExists {
log.Info("Role does not exist; skipping RoleBinding creation", "Role", "ds-pipeline-user-access-dspa")
return nil // Skip creating the RoleBinding
}

// Define a new RoleBinding object
roleBinding := newRoleBinding(notebook)

// Check if the RoleBinding already exists
found := &rbacv1.RoleBinding{}
err := r.Get(ctx, types.NamespacedName{
err = r.Get(ctx, types.NamespacedName{
Name: roleBindingName,
Namespace: notebook.Namespace,
}, found)
Expand All @@ -72,9 +99,10 @@ func (r *OpenshiftNotebookReconciler) reconcileRoleBinding(
return err
}

// Update the RoleBinding if there are changes
if !reflect.DeepEqual(roleBinding.Subjects, found.Subjects) {
// Update the RoleBinding if there are changes, but avoid owner references or deletion implications
if !reflect.DeepEqual(roleBinding.Subjects, found.Subjects) || roleBinding.RoleRef != found.RoleRef {
log.Info("Updating RoleBinding", "RoleBinding.Namespace", roleBinding.Namespace, "RoleBinding.Name", roleBinding.Name)
roleBinding.ResourceVersion = found.ResourceVersion // Maintain the resource version to ensure we can update correctly
err = r.Update(ctx, roleBinding)
if err != nil {
log.Error(err, "Failed to update RoleBinding", "RoleBinding.Namespace", roleBinding.Namespace, "RoleBinding.Name", roleBinding.Name)
Expand All @@ -85,7 +113,7 @@ func (r *OpenshiftNotebookReconciler) reconcileRoleBinding(
return nil
}

// ReconcileRoleBindings manages the creation, update, and deletion of all necessary role bindings for the notebook
// ReconcileRoleBindings manages the creation and updating of all necessary role bindings for the notebook
func (r *OpenshiftNotebookReconciler) ReconcileRoleBindings(
notebook *nbv1.Notebook, ctx context.Context) error {
// Reconcile the Elyra pipelines role binding
Expand Down

0 comments on commit 8cf6263

Please sign in to comment.