Add auth + use constants for labels + cleanup

Author: d-gol

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,50 +624,48 @@ 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
 	}
 
 	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