From 64c5e6432db7fc08de686275b851e5719b0e7119 Mon Sep 17 00:00:00 2001 From: Dejan Golubovic Date: Wed, 23 Nov 2022 12:19:17 +0000 Subject: [PATCH 1/9] Backend for getting logs of a trial --- cmd/new-ui/v1beta1/main.go | 1 + manifests/v1beta1/components/ui/rbac.yaml | 8 ++ pkg/new-ui/v1beta1/backend.go | 93 +++++++++++++++++++++++ 3 files changed, 102 insertions(+) diff --git a/cmd/new-ui/v1beta1/main.go b/cmd/new-ui/v1beta1/main.go index 190bd774ae1..0636532d218 100644 --- a/cmd/new-ui/v1beta1/main.go +++ b/cmd/new-ui/v1beta1/main.go @@ -67,6 +67,7 @@ func main() { http.HandleFunc("/katib/edit_template/", kuh.EditTemplate) http.HandleFunc("/katib/delete_template/", kuh.DeleteTemplate) http.HandleFunc("/katib/fetch_namespaces", kuh.FetchNamespaces) + http.HandleFunc("/katib/fetch_trial_logs/", kuh.FetchTrialLogs) log.Printf("Serving at %s:%s", *host, *port) if err := http.ListenAndServe(fmt.Sprintf("%s:%s", *host, *port), nil); err != nil { diff --git a/manifests/v1beta1/components/ui/rbac.yaml b/manifests/v1beta1/components/ui/rbac.yaml index c549bf351d3..9a47971a006 100644 --- a/manifests/v1beta1/components/ui/rbac.yaml +++ b/manifests/v1beta1/components/ui/rbac.yaml @@ -19,6 +19,14 @@ rules: - suggestions verbs: - "*" + - apiGroups: + - "" + resources: + - pods + - pods/log + verbs: + - list + - get --- apiVersion: v1 kind: ServiceAccount diff --git a/pkg/new-ui/v1beta1/backend.go b/pkg/new-ui/v1beta1/backend.go index 2594395cac2..c52c4ddd952 100644 --- a/pkg/new-ui/v1beta1/backend.go +++ b/pkg/new-ui/v1beta1/backend.go @@ -17,6 +17,8 @@ limitations under the License. package v1beta1 import ( + "bytes" + "context" "encoding/json" "errors" "log" @@ -28,11 +30,18 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" experimentv1beta1 "github.com/kubeflow/katib/pkg/apis/controller/experiments/v1beta1" + trialsv1beta1 "github.com/kubeflow/katib/pkg/apis/controller/trials/v1beta1" suggestionv1beta1 "github.com/kubeflow/katib/pkg/apis/controller/suggestions/v1beta1" api_pb_v1beta1 "github.com/kubeflow/katib/pkg/apis/manager/v1beta1" consts "github.com/kubeflow/katib/pkg/controller.v1beta1/consts" "github.com/kubeflow/katib/pkg/util/v1beta1/katibclient" corev1 "k8s.io/api/core/v1" + + apiv1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + corev1 "k8s.io/client-go/kubernetes/typed/core/v1" + "sigs.k8s.io/controller-runtime/pkg/client/config" ) func NewKatibUIHandler(dbManagerAddr string) *KatibUIHandler { @@ -574,3 +583,87 @@ func (k *KatibUIHandler) FetchTrial(w http.ResponseWriter, r *http.Request) { return } } + +// FetchTrialLogs fetches logs for a trial in specific namespace. +func (k *KatibUIHandler) FetchTrialLogs(w http.ResponseWriter, r *http.Request) { + log.Printf("Requesting logs") + + trialName := r.URL.Query()["trialName"][0] + namespace := r.URL.Query()["namespace"][0] + log.Printf("Requesting logs") + + logs, err := getTrialLogs(k, trialName, namespace) + if err != nil { + log.Printf("GetLogs failed: %v", err) + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + response, err := json.Marshal(logs) + if err != nil { + log.Printf("Marshal logs failed: %v", err) + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + w.Write(response) +} + +// GetTrialLogs returns logs of a master Pod for the given job name and namespace +func getTrialLogs(k *KatibUIHandler, trialName string, namespace string) (string, error) { + cfg, err := config.GetConfig() + if err != nil { + return "", err + } + + clientset, err := corev1.NewForConfig(cfg) + if err != nil { + return "", err + } + + trial := &trialsv1beta1.Trial{} + if err := k.katibClient.GetClient().Get(context.TODO(), types.NamespacedName{Name: trialName, Namespace: namespace}, trial); err != nil { + return "", err + } + + selectionLabel := "training.kubeflow.org/job-name=" + trialName + ",training.kubeflow.org/job-role=master" + if trial.Spec.RunSpec.GetKind() == "Job" { + selectionLabel = "job-name=" + trialName + } + + podList, err := clientset.Pods(namespace).List(context.Background(), metav1.ListOptions{LabelSelector: selectionLabel}) + if err != nil { + return "", err + } + + if len(podList.Items) == 0 { + message := `Logs for the trial could not be found. +Was 'retain: true' specified in the Experiment definition? +An example can be found here: https://github.com/kubeflow/katib/blob/master/examples/v1beta1/argo/argo-workflow.yaml#L33` + + return message, nil + } + + podLogOpts := apiv1.PodLogOptions{} + podLogOpts.Container = trial.Spec.PrimaryContainerName + for container := range podList.Items[0].Spec.Containers { + if podList.Items[0].Spec.Containers[container].Name == "metrics-logger-and-collector" { + podLogOpts.Container = "metrics-logger-and-collector" + break + } + } + + req := clientset.Pods(namespace).GetLogs(podList.Items[0].Name, &podLogOpts) + podLogs, err := req.Stream(context.Background()) + if err != nil { + return "", err + } + defer podLogs.Close() + + buf := new(bytes.Buffer) + _, err = io.Copy(buf, podLogs) + if err != nil { + return "", err + } + str := buf.String() + + return str, nil +} From e9b4daaba44b224bb1e3073526fc48d7fdc5c009 Mon Sep 17 00:00:00 2001 From: d-gol Date: Sun, 27 Nov 2022 11:23:10 +0100 Subject: [PATCH 2/9] Check Write return + use PrimaryPodLabels --- pkg/new-ui/v1beta1/backend.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/pkg/new-ui/v1beta1/backend.go b/pkg/new-ui/v1beta1/backend.go index c52c4ddd952..3301a16a00b 100644 --- a/pkg/new-ui/v1beta1/backend.go +++ b/pkg/new-ui/v1beta1/backend.go @@ -604,7 +604,11 @@ func (k *KatibUIHandler) FetchTrialLogs(w http.ResponseWriter, r *http.Request) http.Error(w, err.Error(), http.StatusInternalServerError) return } - w.Write(response) + if _, err = w.Write(response); err != nil { + log.Printf("Write logs failed: %v", err) + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } } // GetTrialLogs returns logs of a master Pod for the given job name and namespace @@ -624,9 +628,10 @@ func getTrialLogs(k *KatibUIHandler, trialName string, namespace string) (string return "", err } - selectionLabel := "training.kubeflow.org/job-name=" + trialName + ",training.kubeflow.org/job-role=master" - if trial.Spec.RunSpec.GetKind() == "Job" { - selectionLabel = "job-name=" + trialName + selectionLabel := "training.kubeflow.org/job-name=" + trialName + + for primaryKey, primaryValue := range trial.Spec.PrimaryPodLabels { + selectionLabel = selectionLabel + "," + primaryKey + "=" + primaryValue } podList, err := clientset.Pods(namespace).List(context.Background(), metav1.ListOptions{LabelSelector: selectionLabel}) From 77cbbb0d8fcbb4fd9af16e15e6027c7494d765a3 Mon Sep 17 00:00:00 2001 From: d-gol Date: Sat, 3 Dec 2022 14:38:26 +0100 Subject: [PATCH 3/9] Add auth + use constants for labels + cleanup --- pkg/new-ui/v1beta1/backend.go | 59 ++++++++++++++++++++--------------- 1 file changed, 34 insertions(+), 25 deletions(-) diff --git a/pkg/new-ui/v1beta1/backend.go b/pkg/new-ui/v1beta1/backend.go index 3301a16a00b..7745dc5fffa 100644 --- a/pkg/new-ui/v1beta1/backend.go +++ b/pkg/new-ui/v1beta1/backend.go @@ -21,6 +21,7 @@ import ( "context" "encoding/json" "errors" + "io" "log" "net/http" "path/filepath" @@ -30,17 +31,19 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" experimentv1beta1 "github.com/kubeflow/katib/pkg/apis/controller/experiments/v1beta1" - trialsv1beta1 "github.com/kubeflow/katib/pkg/apis/controller/trials/v1beta1" suggestionv1beta1 "github.com/kubeflow/katib/pkg/apis/controller/suggestions/v1beta1" + trialsv1beta1 "github.com/kubeflow/katib/pkg/apis/controller/trials/v1beta1" api_pb_v1beta1 "github.com/kubeflow/katib/pkg/apis/manager/v1beta1" consts "github.com/kubeflow/katib/pkg/controller.v1beta1/consts" "github.com/kubeflow/katib/pkg/util/v1beta1/katibclient" corev1 "k8s.io/api/core/v1" + common "github.com/kubeflow/katib/pkg/apis/controller/common/v1beta1" + mccommon "github.com/kubeflow/katib/pkg/metricscollector/v1beta1/common" apiv1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" - corev1 "k8s.io/client-go/kubernetes/typed/core/v1" + "k8s.io/client-go/kubernetes" "sigs.k8s.io/controller-runtime/pkg/client/config" ) @@ -586,11 +589,19 @@ func (k *KatibUIHandler) FetchTrial(w http.ResponseWriter, r *http.Request) { // FetchTrialLogs fetches logs for a trial in specific namespace. func (k *KatibUIHandler) FetchTrialLogs(w http.ResponseWriter, r *http.Request) { - log.Printf("Requesting logs") - trialName := r.URL.Query()["trialName"][0] namespace := r.URL.Query()["namespace"][0] - log.Printf("Requesting logs") + + user, err := IsAuthorized(consts.ActionTypeGet, namespace, consts.PluralTrial, "", trialName, trialsv1beta1.SchemeGroupVersion, k.katibClient.GetClient(), r) + if user == "" && err != nil { + log.Printf("No user provided in kubeflow-userid header.") + http.Error(w, err.Error(), http.StatusUnauthorized) + return + } else if err != nil { + log.Printf("The user: %s is not authorized to get trial: %s in namespace: %s \n", user, trialName, namespace) + http.Error(w, err.Error(), http.StatusForbidden) + return + } logs, err := getTrialLogs(k, trialName, namespace) if err != nil { @@ -613,28 +624,26 @@ func (k *KatibUIHandler) FetchTrialLogs(w http.ResponseWriter, r *http.Request) // GetTrialLogs returns logs of a master Pod for the given job name and namespace func getTrialLogs(k *KatibUIHandler, trialName string, namespace string) (string, error) { - cfg, err := config.GetConfig() - if err != nil { - return "", err - } - - clientset, err := corev1.NewForConfig(cfg) - if err != nil { - return "", err - } - trial := &trialsv1beta1.Trial{} if err := k.katibClient.GetClient().Get(context.TODO(), types.NamespacedName{Name: trialName, Namespace: namespace}, trial); err != nil { return "", err } - selectionLabel := "training.kubeflow.org/job-name=" + trialName - + selectionLabel := consts.LabelTrialName + "=" + trialName for primaryKey, primaryValue := range trial.Spec.PrimaryPodLabels { selectionLabel = selectionLabel + "," + primaryKey + "=" + primaryValue } - podList, err := clientset.Pods(namespace).List(context.Background(), metav1.ListOptions{LabelSelector: selectionLabel}) + cfg, err := config.GetConfig() + if err != nil { + return "", err + } + clientset, err := kubernetes.NewForConfig(cfg) + if err != nil { + return "", err + } + + podList, err := clientset.CoreV1().Pods(namespace).List(context.Background(), metav1.ListOptions{LabelSelector: selectionLabel}) if err != nil { return "", err } @@ -642,21 +651,21 @@ func getTrialLogs(k *KatibUIHandler, trialName string, namespace string) (string if len(podList.Items) == 0 { message := `Logs for the trial could not be found. Was 'retain: true' specified in the Experiment definition? -An example can be found here: https://github.com/kubeflow/katib/blob/master/examples/v1beta1/argo/argo-workflow.yaml#L33` +An example can be found here: https://github.com/kubeflow/katib/blob/7bf39225f7235ee4ba6cf285ecc2c455c6471234/examples/v1beta1/argo/argo-workflow.yaml#L33` return message, nil } + if len(podList.Items) > 1 { + return "", errors.New("More than one master replica found") + } podLogOpts := apiv1.PodLogOptions{} podLogOpts.Container = trial.Spec.PrimaryContainerName - for container := range podList.Items[0].Spec.Containers { - if podList.Items[0].Spec.Containers[container].Name == "metrics-logger-and-collector" { - podLogOpts.Container = "metrics-logger-and-collector" - break - } + if trial.Spec.MetricsCollector.Collector.Kind == common.StdOutCollector { + podLogOpts.Container = mccommon.MetricLoggerCollectorContainerName } - req := clientset.Pods(namespace).GetLogs(podList.Items[0].Name, &podLogOpts) + req := clientset.CoreV1().Pods(namespace).GetLogs(podList.Items[0].Name, &podLogOpts) podLogs, err := req.Stream(context.Background()) if err != nil { return "", err From 174acb8a87b688b2ae59debf3239da982ca91dfd Mon Sep 17 00:00:00 2001 From: d-gol Date: Mon, 5 Dec 2022 17:07:23 +0100 Subject: [PATCH 4/9] TODO comment for using controller-runtime client for logs --- pkg/new-ui/v1beta1/backend.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/new-ui/v1beta1/backend.go b/pkg/new-ui/v1beta1/backend.go index 7745dc5fffa..58c8aa9fc44 100644 --- a/pkg/new-ui/v1beta1/backend.go +++ b/pkg/new-ui/v1beta1/backend.go @@ -634,6 +634,7 @@ func getTrialLogs(k *KatibUIHandler, trialName string, namespace string) (string selectionLabel = selectionLabel + "," + primaryKey + "=" + primaryValue } + // TODO: Use controller-runtime client instead of kubernetes client to get logs, once this is available cfg, err := config.GetConfig() if err != nil { return "", err From ef89c9b97ac6abaca95ddb97200f43abcd5fc262 Mon Sep 17 00:00:00 2001 From: d-gol Date: Mon, 12 Dec 2022 13:16:39 +0100 Subject: [PATCH 5/9] Authorization for list pods and get logs, reduce RBAC --- manifests/v1beta1/components/ui/rbac.yaml | 6 +- pkg/new-ui/v1beta1/backend.go | 101 ++++++++++++++++------ 2 files changed, 79 insertions(+), 28 deletions(-) diff --git a/manifests/v1beta1/components/ui/rbac.yaml b/manifests/v1beta1/components/ui/rbac.yaml index 9a47971a006..85798dc2d2a 100644 --- a/manifests/v1beta1/components/ui/rbac.yaml +++ b/manifests/v1beta1/components/ui/rbac.yaml @@ -23,9 +23,13 @@ rules: - "" resources: - pods - - pods/log verbs: - list + - apiGroups: + - "" + resources: + - pods/log + verbs: - get --- apiVersion: v1 diff --git a/pkg/new-ui/v1beta1/backend.go b/pkg/new-ui/v1beta1/backend.go index 58c8aa9fc44..6691ab112b1 100644 --- a/pkg/new-ui/v1beta1/backend.go +++ b/pkg/new-ui/v1beta1/backend.go @@ -45,6 +45,8 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes" "sigs.k8s.io/controller-runtime/pkg/client/config" + + "k8s.io/apimachinery/pkg/runtime/schema" ) func NewKatibUIHandler(dbManagerAddr string) *KatibUIHandler { @@ -603,7 +605,57 @@ func (k *KatibUIHandler) FetchTrialLogs(w http.ResponseWriter, r *http.Request) return } - logs, err := getTrialLogs(k, trialName, namespace) + user, err = IsAuthorized(consts.ActionTypeList, namespace, "pods", "", "", schema.GroupVersion{Group: "apps", Version: "v1"}, k.katibClient.GetClient(), r) + if user == "" && err != nil { + log.Printf("No user provided in kubeflow-userid header.") + http.Error(w, err.Error(), http.StatusUnauthorized) + return + } else if err != nil { + log.Printf("The user: %s is not authorized to list pods in namespace: %s \n", user, namespace) + http.Error(w, err.Error(), http.StatusForbidden) + return + } + + trial := &trialsv1beta1.Trial{} + if err := k.katibClient.GetClient().Get(context.TODO(), types.NamespacedName{Name: trialName, Namespace: namespace}, trial); err != nil { + log.Printf("GetLogs failed: %v", err) + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + + // TODO: Use controller-runtime client instead of kubernetes client to get logs, once this is available + clientset, err := createKubernetesClientset() + if err != nil { + log.Printf("GetLogs failed: %v", err) + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + + podName, err := fetchMasterPodName(clientset, trial) + if err != nil { + log.Printf("GetLogs failed: %v", err) + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + + user, err = IsAuthorized(consts.ActionTypeGet, namespace, "pods", "log", podName, schema.GroupVersion{Group: "apps", Version: "v1"}, k.katibClient.GetClient(), r) + if user == "" && err != nil { + log.Printf("No user provided in kubeflow-userid header.") + http.Error(w, err.Error(), http.StatusUnauthorized) + return + } else if err != nil { + log.Printf("The user: %s is not authorized to list pod logs: %s in namespace: %s \n", user, podName, namespace) + http.Error(w, err.Error(), http.StatusForbidden) + return + } + + podLogOpts := apiv1.PodLogOptions{} + podLogOpts.Container = trial.Spec.PrimaryContainerName + if trial.Spec.MetricsCollector.Collector.Kind == common.StdOutCollector { + podLogOpts.Container = mccommon.MetricLoggerCollectorContainerName + } + + logs, err := fetchPodLogs(clientset, namespace, podName, podLogOpts) if err != nil { log.Printf("GetLogs failed: %v", err) http.Error(w, err.Error(), http.StatusInternalServerError) @@ -622,51 +674,46 @@ func (k *KatibUIHandler) FetchTrialLogs(w http.ResponseWriter, r *http.Request) } } -// GetTrialLogs returns logs of a master Pod for the given job name and namespace -func getTrialLogs(k *KatibUIHandler, trialName string, namespace string) (string, error) { - trial := &trialsv1beta1.Trial{} - if err := k.katibClient.GetClient().Get(context.TODO(), types.NamespacedName{Name: trialName, Namespace: namespace}, trial); err != nil { - return "", err - } - - selectionLabel := consts.LabelTrialName + "=" + trialName - for primaryKey, primaryValue := range trial.Spec.PrimaryPodLabels { - selectionLabel = selectionLabel + "," + primaryKey + "=" + primaryValue - } - - // TODO: Use controller-runtime client instead of kubernetes client to get logs, once this is available +// createKubernetesClientset returns kubernetes clientset +func createKubernetesClientset() (*kubernetes.Clientset, error) { cfg, err := config.GetConfig() if err != nil { - return "", err + return nil, err } clientset, err := kubernetes.NewForConfig(cfg) if err != nil { - return "", err + return nil, err } + return clientset, nil +} - podList, err := clientset.CoreV1().Pods(namespace).List(context.Background(), metav1.ListOptions{LabelSelector: selectionLabel}) +// fetchMasterPodName returns name of the master pod for a trial +func fetchMasterPodName(clientset *kubernetes.Clientset, trial *trialsv1beta1.Trial) (string, error) { + selectionLabel := consts.LabelTrialName + "=" + trial.ObjectMeta.Name + for primaryKey, primaryValue := range trial.Spec.PrimaryPodLabels { + selectionLabel = selectionLabel + "," + primaryKey + "=" + primaryValue + } + + podList, err := clientset.CoreV1().Pods(trial.ObjectMeta.Namespace).List(context.Background(), metav1.ListOptions{LabelSelector: selectionLabel}) if err != nil { return "", err } if len(podList.Items) == 0 { - message := `Logs for the trial could not be found. + return "", errors.New(`Logs for the trial could not be found. Was 'retain: true' specified in the Experiment definition? -An example can be found here: https://github.com/kubeflow/katib/blob/7bf39225f7235ee4ba6cf285ecc2c455c6471234/examples/v1beta1/argo/argo-workflow.yaml#L33` - - return message, nil +An example can be found here: https://github.com/kubeflow/katib/blob/7bf39225f7235ee4ba6cf285ecc2c455c6471234/examples/v1beta1/argo/argo-workflow.yaml#L33`) } if len(podList.Items) > 1 { return "", errors.New("More than one master replica found") } - podLogOpts := apiv1.PodLogOptions{} - podLogOpts.Container = trial.Spec.PrimaryContainerName - if trial.Spec.MetricsCollector.Collector.Kind == common.StdOutCollector { - podLogOpts.Container = mccommon.MetricLoggerCollectorContainerName - } + return podList.Items[0].Name, nil +} - req := clientset.CoreV1().Pods(namespace).GetLogs(podList.Items[0].Name, &podLogOpts) +// fetchPodLogs returns logs of a pod for the given job name and namespace +func fetchPodLogs(clientset *kubernetes.Clientset, namespace string, podName string, podLogOpts apiv1.PodLogOptions) (string, error) { + req := clientset.CoreV1().Pods(namespace).GetLogs(podName, &podLogOpts) podLogs, err := req.Stream(context.Background()) if err != nil { return "", err From b934379c480cd1cd79b5bb68d0f39662d7f8de05 Mon Sep 17 00:00:00 2001 From: d-gol Date: Mon, 12 Dec 2022 19:19:15 +0100 Subject: [PATCH 6/9] Use corev1 for specifying resources, edit kf install RBAC --- .../katib-with-kubeflow/kubeflow-katib-roles.yaml | 12 ++++++++++++ pkg/new-ui/v1beta1/backend.go | 8 +++----- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/manifests/v1beta1/installs/katib-with-kubeflow/kubeflow-katib-roles.yaml b/manifests/v1beta1/installs/katib-with-kubeflow/kubeflow-katib-roles.yaml index 6394146705e..57b0fbaf318 100644 --- a/manifests/v1beta1/installs/katib-with-kubeflow/kubeflow-katib-roles.yaml +++ b/manifests/v1beta1/installs/katib-with-kubeflow/kubeflow-katib-roles.yaml @@ -34,6 +34,18 @@ rules: - deletecollection - patch - update + - apiGroups: + - "" + resources: + - pods + verbs: + - list + - apiGroups: + - "" + resources: + - pods/log + verbs: + - get --- apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole diff --git a/pkg/new-ui/v1beta1/backend.go b/pkg/new-ui/v1beta1/backend.go index 6691ab112b1..bb801fd65db 100644 --- a/pkg/new-ui/v1beta1/backend.go +++ b/pkg/new-ui/v1beta1/backend.go @@ -45,8 +45,6 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes" "sigs.k8s.io/controller-runtime/pkg/client/config" - - "k8s.io/apimachinery/pkg/runtime/schema" ) func NewKatibUIHandler(dbManagerAddr string) *KatibUIHandler { @@ -605,7 +603,7 @@ func (k *KatibUIHandler) FetchTrialLogs(w http.ResponseWriter, r *http.Request) return } - user, err = IsAuthorized(consts.ActionTypeList, namespace, "pods", "", "", schema.GroupVersion{Group: "apps", Version: "v1"}, k.katibClient.GetClient(), r) + user, err = IsAuthorized(consts.ActionTypeList, namespace, corev1.ResourcePods.String(), "", "", corev1.SchemeGroupVersion, k.katibClient.GetClient(), r) if user == "" && err != nil { log.Printf("No user provided in kubeflow-userid header.") http.Error(w, err.Error(), http.StatusUnauthorized) @@ -638,13 +636,13 @@ func (k *KatibUIHandler) FetchTrialLogs(w http.ResponseWriter, r *http.Request) return } - user, err = IsAuthorized(consts.ActionTypeGet, namespace, "pods", "log", podName, schema.GroupVersion{Group: "apps", Version: "v1"}, k.katibClient.GetClient(), r) + user, err = IsAuthorized(consts.ActionTypeGet, namespace, corev1.ResourcePods.String(), "log", podName, corev1.SchemeGroupVersion, k.katibClient.GetClient(), r) if user == "" && err != nil { log.Printf("No user provided in kubeflow-userid header.") http.Error(w, err.Error(), http.StatusUnauthorized) return } else if err != nil { - log.Printf("The user: %s is not authorized to list pod logs: %s in namespace: %s \n", user, podName, namespace) + log.Printf("The user: %s is not authorized to get pod logs: %s in namespace: %s \n", user, podName, namespace) http.Error(w, err.Error(), http.StatusForbidden) return } From 0c017ff07de309ffb9aee1b2a8d52ce7124f635c Mon Sep 17 00:00:00 2001 From: d-gol Date: Tue, 13 Dec 2022 18:59:52 +0100 Subject: [PATCH 7/9] Check namespace and trialName from request --- pkg/new-ui/v1beta1/backend.go | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/pkg/new-ui/v1beta1/backend.go b/pkg/new-ui/v1beta1/backend.go index bb801fd65db..f720ede91ab 100644 --- a/pkg/new-ui/v1beta1/backend.go +++ b/pkg/new-ui/v1beta1/backend.go @@ -589,8 +589,24 @@ func (k *KatibUIHandler) FetchTrial(w http.ResponseWriter, r *http.Request) { // FetchTrialLogs fetches logs for a trial in specific namespace. func (k *KatibUIHandler) FetchTrialLogs(w http.ResponseWriter, r *http.Request) { - trialName := r.URL.Query()["trialName"][0] - namespace := r.URL.Query()["namespace"][0] + namespaces, ok := r.URL.Query()["namespace"] + if !ok { + log.Printf("No namespace provided in Query parameters! Provide a 'namespace' param") + err := errors.New("no 'namespace' provided") + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + + trialNames, ok := r.URL.Query()["trialName"] + if !ok { + log.Printf("No trialName provided in Query parameters! Provide a 'trialName' param") + err := errors.New("no 'trialName' provided") + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + + trialName := trialNames[0] + namespace := namespaces[0] user, err := IsAuthorized(consts.ActionTypeGet, namespace, consts.PluralTrial, "", trialName, trialsv1beta1.SchemeGroupVersion, k.katibClient.GetClient(), r) if user == "" && err != nil { From 3d1460abb2bd5c90b37eb1f5a32e08a649bbdce4 Mon Sep 17 00:00:00 2001 From: d-gol Date: Fri, 23 Dec 2022 15:40:44 +0100 Subject: [PATCH 8/9] Remove auth checks for listing the pods --- pkg/new-ui/v1beta1/backend.go | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/pkg/new-ui/v1beta1/backend.go b/pkg/new-ui/v1beta1/backend.go index f720ede91ab..40b0f2c8ac9 100644 --- a/pkg/new-ui/v1beta1/backend.go +++ b/pkg/new-ui/v1beta1/backend.go @@ -619,17 +619,6 @@ func (k *KatibUIHandler) FetchTrialLogs(w http.ResponseWriter, r *http.Request) return } - user, err = IsAuthorized(consts.ActionTypeList, namespace, corev1.ResourcePods.String(), "", "", corev1.SchemeGroupVersion, k.katibClient.GetClient(), r) - if user == "" && err != nil { - log.Printf("No user provided in kubeflow-userid header.") - http.Error(w, err.Error(), http.StatusUnauthorized) - return - } else if err != nil { - log.Printf("The user: %s is not authorized to list pods in namespace: %s \n", user, namespace) - http.Error(w, err.Error(), http.StatusForbidden) - return - } - trial := &trialsv1beta1.Trial{} if err := k.katibClient.GetClient().Get(context.TODO(), types.NamespacedName{Name: trialName, Namespace: namespace}, trial); err != nil { log.Printf("GetLogs failed: %v", err) From 55ba2ce23a558b61f4819ab8c253436737e7a138 Mon Sep 17 00:00:00 2001 From: d-gol Date: Fri, 23 Dec 2022 17:37:34 +0100 Subject: [PATCH 9/9] Use context.Background() --- pkg/new-ui/v1beta1/backend.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/new-ui/v1beta1/backend.go b/pkg/new-ui/v1beta1/backend.go index 40b0f2c8ac9..6e6817eddb3 100644 --- a/pkg/new-ui/v1beta1/backend.go +++ b/pkg/new-ui/v1beta1/backend.go @@ -620,7 +620,7 @@ func (k *KatibUIHandler) FetchTrialLogs(w http.ResponseWriter, r *http.Request) } trial := &trialsv1beta1.Trial{} - if err := k.katibClient.GetClient().Get(context.TODO(), types.NamespacedName{Name: trialName, Namespace: namespace}, trial); err != nil { + if err := k.katibClient.GetClient().Get(context.Background(), types.NamespacedName{Name: trialName, Namespace: namespace}, trial); err != nil { log.Printf("GetLogs failed: %v", err) http.Error(w, err.Error(), http.StatusInternalServerError) return