Skip to content

feat: Add support to run notebooks on its own subdomains in Istio #562

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: notebooks-v1
Choose a base branch
from
Open

feat: Add support to run notebooks on its own subdomains in Istio #562

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion components/notebook-controller/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
204 changes: 166 additions & 38 deletions components/notebook-controller/controllers/notebook_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package controllers
import (
"context"
"encoding/json"
"errors"
"fmt"
"os"
"reflect"
Expand Down Expand Up @@ -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"
Expand All @@ -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"
Expand All @@ -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 {
Expand All @@ -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{}{
Expand Down Expand Up @@ -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
Expand Down