diff --git a/components/notebook-controller/README.md b/components/notebook-controller/README.md index c8c9db8f0..6b0befe39 100644 --- a/components/notebook-controller/README.md +++ b/components/notebook-controller/README.md @@ -34,7 +34,17 @@ All other fields will be filled in with default value if not specified. | --- | --- | |ADD_FSGROUP| If the value is true or unset, fsGroup: 100 will be included in the pod's security context. If this value is present and set to false, it will suppress the automatic addition of fsGroup: 100 to the security context of the pod.| |DEV| If the value is false or unset, then the default implementation of the Notebook Controller will be used. If the admins want to use a custom implementation from their local machine, they should set this value to true.| - +|USE_ISTIO| If the value is set to true, Istio will be used to route traffic. If set to false, Istio routing will be disabled.| +|ISTIO_GATEWAY|Name of the Istio Gateway resource used to route traffic to notebook servers. Only used if ``USE_ISTIO`` is enabled.| +|ISTIO_HOST|Specifies the host to use in the Istio VirtualService. Default is ``*``. Only used if ``USE_ISTIO`` is enabled.| +|ISTIO_USE_NOTEBOOK_SUBDOMAINS| If the value is true, notebooks hosted on a subdomain. This is recommended for security reasons but has some technical dependencies. If the value is set to false, secure subdomain feature is not enabled. Disabled by default.| +|ISTIO_HOST_NOTEBOOK| Domain template used by Istio to host notebooks (e.g., `${NAMESPACE}-notebook.kubeflow.example.com`). Required if ``ISTIO_USE_NOTEBOOK_SUBDOMAINS`` is enabled. | +|ISTIO_HOST_AUTH| Host used by Istio for handling authentication callbacks or login flows (e.g., `kubeflow.example.com`). Required if ``ISTIO_USE_NOTEBOOK_SUBDOMAINS`` is enabled. | +|ISTIO_AUTH_PATH|Specifies the path used for authentication callbacks or redirection when integrating with an identity provider through Istio. Default is ``/oauth2/``. Only used if ``ISTIO_USE_NOTEBOOK_SUBDOMAINS`` is enabled. | +|ENABLE_CULLING|If the value is true, the controller will do culling on idle notebooks. Not enabled by default.| +|CULL_IDLE_TIME|Specifies minutes until an idle notebook will be stopped. Default is 1 day.| +|IDLENESS_CHECK_PERIOD|Specifies minutes for the idleness check period. Default is 1 minute.| +|CLUSTER_DOMAIN| Specifies the Kubernetes internal cluster domain. Default is ``cluster.local``| ## Commandline parameters diff --git a/components/notebook-controller/controllers/notebook_controller.go b/components/notebook-controller/controllers/notebook_controller.go index a44075482..f6ba75e27 100644 --- a/components/notebook-controller/controllers/notebook_controller.go +++ b/components/notebook-controller/controllers/notebook_controller.go @@ -18,6 +18,7 @@ package controllers import ( "context" "encoding/json" + "errors" "fmt" "os" "reflect" @@ -468,7 +469,108 @@ func virtualServiceName(kfName string, namespace string) string { return fmt.Sprintf("notebook-%s-%s", namespace, kfName) } -func generateVirtualService(instance *v1beta1.Notebook) (*unstructured.Unstructured, error) { +func virtualServiceNameWithSuffix(kfName string, namespace string, suffix string) string { + return fmt.Sprintf("notebook-%s-%s-%s", namespace, kfName, suffix) +} + +func generateVirtualServiceForNotebookRedirect(name string, namespace string, prefix string, istioHostNotebook string, istioGateway string) (*unstructured.Unstructured, error) { + vsvc := &unstructured.Unstructured{} + vsvc.SetAPIVersion("networking.istio.io/v1alpha3") + vsvc.SetKind("VirtualService") + vsvc.SetName(virtualServiceNameWithSuffix(name, namespace, "notebook-redirect")) + vsvc.SetNamespace(namespace) + + istioHost := os.Getenv("ISTIO_HOST") + if len(istioHost) == 0 { + istioHost = "*" + } + if err := unstructured.SetNestedStringSlice(vsvc.Object, []string{istioHost}, "spec", "hosts"); err != nil { + return nil, fmt.Errorf("Set .spec.hosts error: %v", err) + + } + + if err := unstructured.SetNestedStringSlice(vsvc.Object, []string{istioGateway}, + "spec", "gateways"); err != nil { + return nil, fmt.Errorf("set .spec.gateways error: %v", err) + } + + // the http section of the istio VirtualService spec + http := []interface{}{ + map[string]interface{}{ + "match": []interface{}{ + map[string]interface{}{ + "uri": map[string]interface{}{ + "prefix": prefix, + }, + }, + }, + "redirect": map[string]interface{}{ + "authority": istioHostNotebook, + "redirectCode": int64(307), + }, + }, + } + + // add http section to istio VirtualService spec + if err := unstructured.SetNestedSlice(vsvc.Object, http, "spec", "http"); err != nil { + return nil, fmt.Errorf("set .spec.http error: %v", err) + } + + return vsvc, nil +} + +func generateVirtualServiceForAuthRedirect(name string, namespace string, prefix string, istioHostNotebook string, istioGateway string) (*unstructured.Unstructured, error) { + vsvc := &unstructured.Unstructured{} + vsvc.SetAPIVersion("networking.istio.io/v1alpha3") + vsvc.SetKind("VirtualService") + vsvc.SetName(virtualServiceNameWithSuffix(name, namespace, "auth-redirect")) + vsvc.SetNamespace(namespace) + + if err := unstructured.SetNestedStringSlice(vsvc.Object, []string{istioHostNotebook}, "spec", "hosts"); err != nil { + return nil, fmt.Errorf("set .spec.hosts error: %v", err) + } + + if err := unstructured.SetNestedStringSlice(vsvc.Object, []string{istioGateway}, + "spec", "gateways"); err != nil { + return nil, fmt.Errorf("set .spec.gateways error: %v", err) + } + + istioHostAuth := os.Getenv("ISTIO_HOST_AUTH") + if len(istioHostAuth) == 0 { + return nil, fmt.Errorf("invalid ISTIO_HOST_AUTH: When ISTIO_USE_NOTEBOOK_SUBDOMAINS is set, the ISTIO_HOST_AUTH environment variable must be defined.") + } + + istioAuthPath := os.Getenv("ISTIO_AUTH_PATH") + if len(istioAuthPath) == 0 { + istioAuthPath = "/oauth2/" + } + + // the http section of the istio VirtualService spec + http := []interface{}{ + map[string]interface{}{ + "match": []interface{}{ + map[string]interface{}{ + "uri": map[string]interface{}{ + "prefix": istioAuthPath, + }, + }, + }, + "redirect": map[string]interface{}{ + "authority": istioHostAuth, + "redirectCode": int64(307), + }, + }, + } + + // add http section to istio VirtualService spec + if err := unstructured.SetNestedSlice(vsvc.Object, http, "spec", "http"); err != nil { + return nil, fmt.Errorf("set .spec.http error: %v", err) + } + + return vsvc, nil +} + +func generateVirtualServices(instance *v1beta1.Notebook) ([]*unstructured.Unstructured, error) { name := instance.Name namespace := instance.Namespace clusterDomain := "cluster.local" @@ -492,20 +594,13 @@ func generateVirtualService(instance *v1beta1.Notebook) (*unstructured.Unstructu service := fmt.Sprintf("%s.%s.svc.%s", name, namespace, clusterDomain) vsvc := &unstructured.Unstructured{} + vsvcs := []*unstructured.Unstructured{vsvc} + vsvc.SetAPIVersion("networking.istio.io/v1alpha3") vsvc.SetKind("VirtualService") vsvc.SetName(virtualServiceName(name, namespace)) vsvc.SetNamespace(namespace) - istioHost := os.Getenv("ISTIO_HOST") - if len(istioHost) == 0 { - istioHost = "*" - } - if err := unstructured.SetNestedStringSlice(vsvc.Object, []string{istioHost}, "spec", "hosts"); err != nil { - return nil, fmt.Errorf("Set .spec.hosts error: %v", err) - - } - istioGateway := os.Getenv("ISTIO_GATEWAY") if len(istioGateway) == 0 { istioGateway = "kubeflow/kubeflow-gateway" @@ -515,6 +610,33 @@ func generateVirtualService(instance *v1beta1.Notebook) (*unstructured.Unstructu return nil, fmt.Errorf("set .spec.gateways error: %v", err) } + istioHost := os.Getenv("ISTIO_HOST") + if os.Getenv("ISTIO_USE_NOTEBOOK_SUBDOMAINS") == "true" { + istioHost = os.Getenv("ISTIO_HOST_NOTEBOOK") + if !strings.Contains(istioHost, "${NAMESPACE}") { + return nil, errors.New("invalid ISTIO_HOST_NOTEBOOK: When ISTIO_USE_NOTEBOOK_SUBDOMAINS is set, the placeholder ${NAMESPACE} must be defined in ISTIO_HOST_NOTEBOOK.") + } + istioHost = strings.ReplaceAll(istioHost, "${NAMESPACE}", namespace) + + vsvcForNotebookRedirect, err := generateVirtualServiceForNotebookRedirect(name, namespace, prefix, istioHost, istioGateway) + if err != nil { + return nil, fmt.Errorf("generate virtual service for notebook redirect error: %v", err) + } + + vsvcForAuthRedirect, err := generateVirtualServiceForAuthRedirect(name, namespace, prefix, istioHost, istioGateway) + if err != nil { + return nil, fmt.Errorf("generate virtual service for auth redirect error: %v", err) + } + + vsvcs = append(vsvcs, vsvcForAuthRedirect, vsvcForNotebookRedirect) + } else if len(istioHost) == 0 { + istioHost = "*" + } + if err := unstructured.SetNestedStringSlice(vsvc.Object, []string{istioHost}, "spec", "hosts"); err != nil { + return nil, fmt.Errorf("Set .spec.hosts error: %v", err) + + } + headersRequestSet := make(map[string]string) // If AnnotationHeadersRequestSet is present, use its values in "headers.request.set" if _, ok := annotations[AnnotationHeadersRequestSet]; ok && len(annotations[AnnotationHeadersRequestSet]) > 0 { @@ -535,7 +657,8 @@ func generateVirtualService(instance *v1beta1.Notebook) (*unstructured.Unstructu map[string]interface{}{ "headers": map[string]interface{}{ "request": map[string]interface{}{ - "set": headersRequestSetInterface, + "set": headersRequestSetInterface, + "remove": []interface{}{"Cookie"}, }, }, "match": []interface{}{ @@ -566,46 +689,51 @@ func generateVirtualService(instance *v1beta1.Notebook) (*unstructured.Unstructu return nil, fmt.Errorf("set .spec.http error: %v", err) } - return vsvc, nil + return vsvcs, nil } func (r *NotebookReconciler) reconcileVirtualService(instance *v1beta1.Notebook) error { log := r.Log.WithValues("notebook", instance.Namespace) - virtualService, err := generateVirtualService(instance) + virtualServices, err := generateVirtualServices(instance) if err != nil { log.Info("Unable to generate VirtualService...", err) return err } - if err := ctrl.SetControllerReference(instance, virtualService, r.Scheme); err != nil { - return err - } - // Check if the virtual service already exists. - foundVirtual := &unstructured.Unstructured{} - justCreated := false - foundVirtual.SetAPIVersion("networking.istio.io/v1alpha3") - foundVirtual.SetKind("VirtualService") - err = r.Get(context.TODO(), types.NamespacedName{Name: virtualServiceName(instance.Name, - instance.Namespace), Namespace: instance.Namespace}, foundVirtual) - if err != nil && apierrs.IsNotFound(err) { - log.Info("Creating virtual service", "namespace", instance.Namespace, "name", - virtualServiceName(instance.Name, instance.Namespace)) - err = r.Create(context.TODO(), virtualService) - justCreated = true - if err != nil { + + for _, virtualService := range virtualServices { + + if err := ctrl.SetControllerReference(instance, virtualService, r.Scheme); err != nil { return err } - } else if err != nil { - return err - } - - if !justCreated && reconcilehelper.CopyVirtualService(virtualService, foundVirtual) { - log.Info("Updating virtual service", "namespace", instance.Namespace, "name", - virtualServiceName(instance.Name, instance.Namespace)) - err = r.Update(context.TODO(), foundVirtual) - if err != nil { + // Check if the virtual service already exists. + foundVirtual := &unstructured.Unstructured{} + justCreated := false + foundVirtual.SetAPIVersion("networking.istio.io/v1alpha3") + foundVirtual.SetKind("VirtualService") + virtualServiceName := virtualService.GetName() + virtualServiceNamespace := virtualService.GetNamespace() + err = r.Get(context.TODO(), types.NamespacedName{Name: virtualServiceName, Namespace: virtualServiceNamespace}, foundVirtual) + if err != nil && apierrs.IsNotFound(err) { + log.Info("Creating virtual service", "namespace", virtualServiceNamespace, "name", + virtualServiceName) + err = r.Create(context.TODO(), virtualService) + justCreated = true + if err != nil { + return err + } + } else if err != nil { return err } + + if !justCreated && reconcilehelper.CopyVirtualService(virtualService, foundVirtual) { + log.Info("Updating virtual service", "namespace", virtualServiceNamespace, "name", + virtualServiceName) + err = r.Update(context.TODO(), foundVirtual) + if err != nil { + return err + } + } } return nil