From 5d61120b23a8ca7b2c6470d69002586a2911ba7b Mon Sep 17 00:00:00 2001 From: GitHub Actions Date: Fri, 18 Oct 2024 08:49:45 +0000 Subject: [PATCH 1/5] Update odh and notebook-controller with image main-648689f --- .../notebook-controller/config/overlays/openshift/params.env | 2 +- components/odh-notebook-controller/Makefile | 2 +- components/odh-notebook-controller/config/base/params.env | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/components/notebook-controller/config/overlays/openshift/params.env b/components/notebook-controller/config/overlays/openshift/params.env index afc156ff326..aead3391138 100644 --- a/components/notebook-controller/config/overlays/openshift/params.env +++ b/components/notebook-controller/config/overlays/openshift/params.env @@ -1 +1 @@ -odh-kf-notebook-controller-image=quay.io/opendatahub/kubeflow-notebook-controller:main-7986030 +odh-kf-notebook-controller-image=quay.io/opendatahub/kubeflow-notebook-controller:main-648689f diff --git a/components/odh-notebook-controller/Makefile b/components/odh-notebook-controller/Makefile index 2795b4e96ad..8c223728285 100644 --- a/components/odh-notebook-controller/Makefile +++ b/components/odh-notebook-controller/Makefile @@ -4,7 +4,7 @@ IMG ?= quay.io/opendatahub/odh-notebook-controller TAG ?= $(shell git describe --tags --always) KF_IMG ?= quay.io/opendatahub/kubeflow-notebook-controller -KF_TAG ?= main-7986030 +KF_TAG ?= main-648689f CONTAINER_ENGINE ?= podman diff --git a/components/odh-notebook-controller/config/base/params.env b/components/odh-notebook-controller/config/base/params.env index 0cc0a43c330..a53394c9199 100644 --- a/components/odh-notebook-controller/config/base/params.env +++ b/components/odh-notebook-controller/config/base/params.env @@ -1 +1 @@ -odh-notebook-controller-image=quay.io/opendatahub/odh-notebook-controller:main-7986030 +odh-notebook-controller-image=quay.io/opendatahub/odh-notebook-controller:main-648689f From 9117408d7806bc953b308d8312c65f58faef7e3c Mon Sep 17 00:00:00 2001 From: Jan Stourac Date: Wed, 30 Oct 2024 18:36:16 +0100 Subject: [PATCH 2/5] [RHOAIENG-14687] Extend tests for certificate to assure that they are readable https://issues.redhat.com/browse/RHOAIENG-14687 --- .../controllers/notebook_controller_test.go | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/components/odh-notebook-controller/controllers/notebook_controller_test.go b/components/odh-notebook-controller/controllers/notebook_controller_test.go index 87369dd78ef..4d251af574d 100644 --- a/components/odh-notebook-controller/controllers/notebook_controller_test.go +++ b/components/odh-notebook-controller/controllers/notebook_controller_test.go @@ -17,6 +17,8 @@ package controllers import ( "context" + "crypto/x509" + "encoding/pem" "io/ioutil" "strings" "time" @@ -230,6 +232,12 @@ var _ = Describe("The Openshift Notebook controller", func() { } // Check if the volume is present and matches the expected one Expect(notebook.Spec.Template.Spec.Volumes).To(ContainElement(expectedVolume)) + + // Check the content in workbench-trusted-ca-bundle matches what we expect: + // - have 2 certificates there in ca-bundle.crt + // - both certificates are valid + configMapName := "workbench-trusted-ca-bundle" + checkCertConfigMap(ctx, notebook.Namespace, configMapName, "ca-bundle.crt", 2) }) }) @@ -329,6 +337,12 @@ var _ = Describe("The Openshift Notebook controller", func() { }, } Expect(notebook.Spec.Template.Spec.Volumes).To(ContainElement(expectedVolume)) + + // Check the content in workbench-trusted-ca-bundle matches what we expect: + // - have 2 certificates there in ca-bundle.crt + // - both certificates are valid + configMapName := "workbench-trusted-ca-bundle" + checkCertConfigMap(ctx, notebook.Namespace, configMapName, "ca-bundle.crt", 2) }) }) @@ -1039,3 +1053,32 @@ func createOAuthConfigmap(name, namespace string, label map[string]string, confi Data: configMapData, } } + +// checkCertConfigMap checks the content of a config map defined by the name and namespace +// It triest to parse the given certFileName and checks that all certificates can be parsed there and that the number of the certificates matches what we expect. +func checkCertConfigMap(ctx context.Context, namespace string, configMapName string, certFileName string, expNumberCerts int) { + configMap := &corev1.ConfigMap{} + key := types.NamespacedName{Namespace: namespace, Name: configMapName} + Expect(cli.Get(ctx, key, configMap)).Should(Succeed()) + + // Attempt to decode PEM encoded certificates so we are sure all are readable as expected + certData := configMap.Data[certFileName] + certDataByte := []byte(certData) + certificatesFound := 0 + for len(certDataByte) > 0 { + block, remainder := pem.Decode(certDataByte) + certDataByte = remainder + + if block == nil { + break + } + + if block.Type == "CERTIFICATE" { + // Attempt to parse the certificate + _, err := x509.ParseCertificate(block.Bytes) + Expect(err).ShouldNot(HaveOccurred()) + certificatesFound++ + } + } + Expect(certificatesFound).Should(Equal(expNumberCerts), "Number of parsed certificates don't match expected one:\n"+certData) +} From 4a185e08761da954ad15d7e02fdf69fa754dfc41 Mon Sep 17 00:00:00 2001 From: Jiri Danek Date: Mon, 4 Nov 2024 15:05:40 +0100 Subject: [PATCH 3/5] RHOAIENG-15335: feat(odh-nbc/webhook): only update oauth-proxy image in CREATE events --- .../controllers/notebook_controller_test.go | 32 ++++++++++--------- .../controllers/notebook_webhook.go | 18 +++++++---- 2 files changed, 28 insertions(+), 22 deletions(-) diff --git a/components/odh-notebook-controller/controllers/notebook_controller_test.go b/components/odh-notebook-controller/controllers/notebook_controller_test.go index 4d251af574d..5411440e3f8 100644 --- a/components/odh-notebook-controller/controllers/notebook_controller_test.go +++ b/components/odh-notebook-controller/controllers/notebook_controller_test.go @@ -608,21 +608,23 @@ var _ = Describe("The Openshift Notebook controller", func() { }, duration, interval).Should(BeTrue()) }) - It("Should reconcile the Notebook when modified", func() { - By("By simulating a manual Notebook modification") - notebook.Spec.Template.Spec.ServiceAccountName = "foo" - notebook.Spec.Template.Spec.Containers[1].Image = "bar" - notebook.Spec.Template.Spec.Volumes[1].VolumeSource = corev1.VolumeSource{} - Expect(cli.Update(ctx, notebook)).Should(Succeed()) - time.Sleep(interval) - - By("By checking that the webhook has restored the Notebook spec") - Eventually(func() error { - key := types.NamespacedName{Name: Name, Namespace: Namespace} - return cli.Get(ctx, key, notebook) - }, duration, interval).Should(Succeed()) - Expect(CompareNotebooks(*notebook, expectedNotebook)).Should(BeTrue()) - }) + // RHOAIENG-14552: We *do not* reconcile OAuth in the notebook when it's modified + + //It("Should reconcile the Notebook when modified", func() { + // By("By simulating a manual Notebook modification") + // notebook.Spec.Template.Spec.ServiceAccountName = "foo" + // notebook.Spec.Template.Spec.Containers[1].Image = "bar" + // notebook.Spec.Template.Spec.Volumes[1].VolumeSource = corev1.VolumeSource{} + // Expect(cli.Update(ctx, notebook)).Should(Succeed()) + // time.Sleep(interval) + // + // By("By checking that the webhook has restored the Notebook spec") + // Eventually(func() error { + // key := types.NamespacedName{Name: Name, Namespace: Namespace} + // return cli.Get(ctx, key, notebook) + // }, duration, interval).Should(Succeed()) + // Expect(CompareNotebooks(*notebook, expectedNotebook)).Should(BeTrue()) + //}) serviceAccount := &corev1.ServiceAccount{} expectedServiceAccount := createOAuthServiceAccount(Name, Namespace) diff --git a/components/odh-notebook-controller/controllers/notebook_webhook.go b/components/odh-notebook-controller/controllers/notebook_webhook.go index bd91ae05470..49e2a009e48 100644 --- a/components/odh-notebook-controller/controllers/notebook_webhook.go +++ b/components/odh-notebook-controller/controllers/notebook_webhook.go @@ -265,13 +265,17 @@ func (w *NotebookWebhook) Handle(ctx context.Context, req admission.Request) adm } // Inject the OAuth proxy if the annotation is present but only if Service Mesh is disabled - if OAuthInjectionIsEnabled(notebook.ObjectMeta) { - if ServiceMeshIsEnabled(notebook.ObjectMeta) { - return admission.Denied(fmt.Sprintf("Cannot have both %s and %s set to true. Pick one.", AnnotationServiceMesh, AnnotationInjectOAuth)) - } - err = InjectOAuthProxy(notebook, w.OAuthConfig) - if err != nil { - return admission.Errored(http.StatusInternalServerError, err) + if OAuthInjectionIsEnabled(notebook.ObjectMeta) && ServiceMeshIsEnabled(notebook.ObjectMeta) { + return admission.Denied(fmt.Sprintf("Cannot have both %s and %s set to true. Pick one.", AnnotationServiceMesh, AnnotationInjectOAuth)) + } + // RHOAIENG-14552: Updating oauth-proxy in a running notebook may lead to notebook restart. Updating only + // on Create is safe as it cannot cause a restart in already running notebook when oauth-proxy image changes. + if req.Operation == admissionv1.Create { + if OAuthInjectionIsEnabled(notebook.ObjectMeta) { + err = InjectOAuthProxy(notebook, w.OAuthConfig) + if err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } } } From 900f028b6314a54260bb7605fa48f023adab01fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jiri=20Dan=C4=9Bk?= Date: Wed, 6 Nov 2024 13:13:21 +0100 Subject: [PATCH 4/5] Revert "Revert "[v1.9-branch] RHOAIENG-10827: feat(nbcs): update ose-oauth-proxy image digest reference from 4.8 to the latest 4.14 version (#388)"" --- .../odh-notebook-controller/config/manager/manager.yaml | 2 +- .../odh-notebook-controller/controllers/notebook_oauth.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/components/odh-notebook-controller/config/manager/manager.yaml b/components/odh-notebook-controller/config/manager/manager.yaml index 991878bfbf3..98f25094bd8 100644 --- a/components/odh-notebook-controller/config/manager/manager.yaml +++ b/components/odh-notebook-controller/config/manager/manager.yaml @@ -25,7 +25,7 @@ spec: imagePullPolicy: Always command: - /manager - args: ["--oauth-proxy-image", "registry.redhat.io/openshift4/ose-oauth-proxy@sha256:4bef31eb993feb6f1096b51b4876c65a6fb1f4401fee97fa4f4542b6b7c9bc46"] + args: ["--oauth-proxy-image", "registry.redhat.io/openshift4/ose-oauth-proxy@sha256:4f8d66597feeb32bb18699326029f9a71a5aca4a57679d636b876377c2e95695"] securityContext: allowPrivilegeEscalation: false ports: diff --git a/components/odh-notebook-controller/controllers/notebook_oauth.go b/components/odh-notebook-controller/controllers/notebook_oauth.go index f289dfc2a27..81f78f94a5a 100644 --- a/components/odh-notebook-controller/controllers/notebook_oauth.go +++ b/components/odh-notebook-controller/controllers/notebook_oauth.go @@ -35,10 +35,10 @@ import ( const ( OAuthServicePort = 443 OAuthServicePortName = "oauth-proxy" - // OAuthProxyImage uses sha256 manifest list digest value of v4.8 image for AMD64 as default to be compatible with imagePullPolicy: IfNotPresent, overridable - // taken from https://catalog.redhat.com/software/containers/openshift4/ose-oauth-proxy/5cdb2133bed8bd5717d5ae64?image=6306f12280cc9b3291272668&architecture=amd64&container-tabs=overview + // OAuthProxyImage uses sha256 manifest list digest value of v4.14 image for AMD64 as default to be compatible with imagePullPolicy: IfNotPresent, overridable + // taken from https://catalog.redhat.com/software/containers/openshift4/ose-oauth-proxy/5cdb2133bed8bd5717d5ae64?image=66cefc14401df6ff4664ec43&architecture=amd64&container-tabs=overview // and kept in sync with the manifests here and in ClusterServiceVersion metadata of opendatahub operator - OAuthProxyImage = "registry.redhat.io/openshift4/ose-oauth-proxy@sha256:4bef31eb993feb6f1096b51b4876c65a6fb1f4401fee97fa4f4542b6b7c9bc46" + OAuthProxyImage = "registry.redhat.io/openshift4/ose-oauth-proxy@sha256:4f8d66597feeb32bb18699326029f9a71a5aca4a57679d636b876377c2e95695" ) type OAuthConfig struct { From 3f931d21bcec377150ba01a8f32dde27339a3a7a Mon Sep 17 00:00:00 2001 From: aTheo Date: Thu, 7 Nov 2024 10:18:46 +0100 Subject: [PATCH 5/5] Introduce rbac controller to handle RoleBindings (#434) * Introduce rolebinding controller to handle rbacs * Use SET_PIPELINE_RBAC configuration env * Skip tests if SET_PIPELINE_RBAC is set to false * Remove configmap and use a simple env in odh managers deployment * Swap deletion before creation on rbac controller * Add log error in case of error on the rbac reconciler * Set proper the ownerReferences when we create a rolebinding --- components/odh-notebook-controller/Makefile | 11 +- .../config/manager/manager.yaml | 3 + .../config/rbac/role.yaml | 23 +++ .../controllers/notebook_controller.go | 15 ++ .../controllers/notebook_controller_test.go | 75 +++++++++ .../controllers/notebook_rbac.go | 154 ++++++++++++++++++ 6 files changed, 279 insertions(+), 2 deletions(-) create mode 100644 components/odh-notebook-controller/controllers/notebook_rbac.go diff --git a/components/odh-notebook-controller/Makefile b/components/odh-notebook-controller/Makefile index 8c223728285..9fdb3af172b 100644 --- a/components/odh-notebook-controller/Makefile +++ b/components/odh-notebook-controller/Makefile @@ -86,8 +86,15 @@ fmt: ## Run go fmt against code. vet: ## Run go vet against code. go vet ./... -.PHONY: test -test: manifests generate fmt vet envtest ## Run tests. +.PHONY: test test-with-rbac-false test-with-rbac-true +test: test-with-rbac-false test-with-rbac-true +test-with-rbac-false: manifests generate fmt vet envtest ## Run tests. + export SET_PIPELINE_RBAC=false && \ + ACK_GINKGO_DEPRECATIONS=1.16.5 \ + KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" \ + go test ./controllers/... -ginkgo.v -ginkgo.progress -test.v -coverprofile cover.out +test-with-rbac-true: manifests generate fmt vet envtest ## Run tests. + export SET_PIPELINE_RBAC=true && \ ACK_GINKGO_DEPRECATIONS=1.16.5 \ KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" \ go test ./controllers/... -ginkgo.v -ginkgo.progress -test.v -coverprofile cover.out diff --git a/components/odh-notebook-controller/config/manager/manager.yaml b/components/odh-notebook-controller/config/manager/manager.yaml index 98f25094bd8..89f450985d4 100644 --- a/components/odh-notebook-controller/config/manager/manager.yaml +++ b/components/odh-notebook-controller/config/manager/manager.yaml @@ -54,3 +54,6 @@ spec: requests: cpu: 500m memory: 256Mi + env: + - name: SET_PIPELINE_RBAC + value: "false" 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..3e667ed06cb 100644 --- a/components/odh-notebook-controller/controllers/notebook_controller.go +++ b/components/odh-notebook-controller/controllers/notebook_controller.go @@ -21,8 +21,10 @@ import ( "crypto/x509" "encoding/pem" "errors" + "os" "reflect" "strconv" + "strings" "time" netv1 "k8s.io/api/networking/v1" @@ -32,6 +34,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 +70,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 +189,15 @@ func (r *OpenshiftNotebookReconciler) Reconcile(ctx context.Context, req ctrl.Re return ctrl.Result{}, err } + // Call the Rolebinding reconciler + if strings.ToLower(strings.TrimSpace(os.Getenv("SET_PIPELINE_RBAC"))) == "true" { + err = r.ReconcileRoleBindings(notebook, ctx) + if err != nil { + log.Error(err, "Unable to Reconcile Rolebinding") + 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 +459,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 5411440e3f8..262f46c0646 100644 --- a/components/odh-notebook-controller/controllers/notebook_controller_test.go +++ b/components/odh-notebook-controller/controllers/notebook_controller_test.go @@ -19,13 +19,16 @@ import ( "context" "crypto/x509" "encoding/pem" + "fmt" "io/ioutil" + "os" "strings" "time" "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" @@ -163,6 +166,78 @@ var _ = Describe("The Openshift Notebook controller", func() { }) + // New test case for RoleBinding reconciliation + When("Reconcile RoleBindings 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 + + BeforeEach(func() { + // Skip the tests if SET_PIPELINE_RBAC is not set to "true" + fmt.Printf("SET_PIPELINE_RBAC is: %s\n", os.Getenv("SET_PIPELINE_RBAC")) + if os.Getenv("SET_PIPELINE_RBAC") != "true" { + Skip("Skipping RoleBinding reconciliation tests as SET_PIPELINE_RBAC is not set to 'true'") + } + }) + + 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..6055eceb974 --- /dev/null +++ b/components/odh-notebook-controller/controllers/notebook_rbac.go @@ -0,0 +1,154 @@ +/* + +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" + ctrl "sigs.k8s.io/controller-runtime" +) + +// 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 +} + +// 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) + + // Add .metatada.ownerReferences to the Rolebinding to be deleted by + // the Kubernetes garbage collector if the notebook is deleted + if err := ctrl.SetControllerReference(notebook, roleBinding, r.Scheme); err != nil { + log.Error(err, "Failed to set controller reference for RoleBinding") + return err + } + 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 { + + roleBindingName := "elyra-pipelines-" + notebook.Name + // Reconcile a RoleBinding for pipelines for the notebook service account + if err := r.reconcileRoleBinding(notebook, ctx, roleBindingName, "Role", "ds-pipeline-user-access-dspa"); err != nil { + return err + } + + return nil +}