Skip to content

Commit

Permalink
Review fixes
Browse files Browse the repository at this point in the history 
* proper error handling.
* switch to Go's build-in errors package.
* set appropriate verbs when constructing SAR objects.

Signed-off-by: Apostolos Gerakaris <apoger@arrikto.com>
  • Loading branch information
@apo-ger
apo-ger committed Nov 17, 2022
1 parent dea8129 commit 0377ed3
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 18 deletions.
4 changes: 2 additions & 2 deletions pkg/new-ui/v1beta1/authzn.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@ package v1beta1

import (
"context"
"errors"
"fmt"
"log"
"net/http"
"strings"

"github.com/kubeflow/katib/pkg/util/v1beta1/env"
"github.com/pkg/errors"
v1 "k8s.io/api/authorization/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes"
Expand All @@ -30,7 +30,7 @@ func GetUsername(r *http.Request) (string, error) {
}

if r.Header.Get(USER_HEADER) == "" {
return "", errors.New("User header not present!")
return "", errors.New("user header not present")
}

user := r.Header.Get(USER_HEADER)
Expand Down
26 changes: 15 additions & 11 deletions pkg/new-ui/v1beta1/backend.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package v1beta1

import (
"encoding/json"
"errors"
"log"
"net/http"
"path/filepath"
Expand All @@ -31,7 +32,6 @@ import (
experimentv1beta1 "github.com/kubeflow/katib/pkg/apis/controller/experiments/v1beta1"
api_pb_v1beta1 "github.com/kubeflow/katib/pkg/apis/manager/v1beta1"
"github.com/kubeflow/katib/pkg/util/v1beta1/katibclient"
"github.com/pkg/errors"
)

func NewKatibUIHandler(dbManagerAddr string) *KatibUIHandler {
Expand All @@ -40,8 +40,12 @@ func NewKatibUIHandler(dbManagerAddr string) *KatibUIHandler {
log.Printf("NewClient for Katib failed: %v", err)
panic(err)
}
// create a new client-go client for sending SAR objects in the API-SERVER
// create a new client for manipulating SAR objects.
conf, err := config.GetConfig()
if err != nil {
log.Printf("Failed to create k8s rest config: %v", err)
panic(err)
}
sarclient, err := kubernetes.NewForConfig(conf)
if err != nil {
log.Printf("SarClient for Katib failes: %v", err)
Expand Down Expand Up @@ -147,7 +151,7 @@ func (k *KatibUIHandler) FetchNamespacedExperiments(w http.ResponseWriter, r *ht
namespaces, ok := r.URL.Query()["namespace"]
if !ok {
log.Printf("No 'namespace' query parameter was provided.")
err := errors.New("No 'namespace' query parameter was provided.")
err := errors.New("no 'namespace' query parameter was provided")
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
Expand Down Expand Up @@ -218,14 +222,14 @@ func (k *KatibUIHandler) DeleteExperiment(w http.ResponseWriter, r *http.Request
namespaces, ok := r.URL.Query()["namespace"]
if !ok {
log.Printf("No 'namespace' query parameter was provided.")
err := errors.New("No 'namespace' query parameter was provided.")
err := errors.New("no 'namespace' query parameter was provided")
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
experimentNames, ok := r.URL.Query()["experimentName"]
if !ok {
log.Printf("No experimentName provided in Query parameteres! Provide an 'experimentName' param")
err := errors.New("No experimentName provided!")
err := errors.New("no experimentName provided")
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
Expand Down Expand Up @@ -349,7 +353,7 @@ func (k *KatibUIHandler) AddTemplate(w http.ResponseWriter, r *http.Request) {
updatedConfigMapPath := data["updatedConfigMapPath"].(string)
updatedTemplateYaml := data["updatedTemplateYaml"].(string)

err = IsAuthorized(user, "add", updatedConfigMapNamespace, "", "v1", "configmaps", "", updatedConfigMapName, &k.sarClient)
err = IsAuthorized(user, "create", updatedConfigMapNamespace, "", "v1", "configmaps", "", updatedConfigMapName, &k.sarClient)
if err != nil {
log.Printf("The user: %s is not authorized to add configmap: %s from namespace: %s \n", user, updatedConfigMapName, updatedConfigMapNamespace)
http.Error(w, err.Error(), http.StatusForbidden)
Expand Down Expand Up @@ -404,7 +408,7 @@ func (k *KatibUIHandler) EditTemplate(w http.ResponseWriter, r *http.Request) {
updatedConfigMapPath := data["updatedConfigMapPath"].(string)
updatedTemplateYaml := data["updatedTemplateYaml"].(string)

err = IsAuthorized(user, "edit", updatedConfigMapNamespace, "", "v1", "configmaps", "", updatedConfigMapName, &k.sarClient)
err = IsAuthorized(user, "update", updatedConfigMapNamespace, "", "v1", "configmaps", "", updatedConfigMapName, &k.sarClient)
if err != nil {
log.Printf("The user: %s is not authorized to edit configmap: %s from namespace: %s \n", user, updatedConfigMapName, updatedConfigMapNamespace)
http.Error(w, err.Error(), http.StatusForbidden)
Expand Down Expand Up @@ -522,14 +526,14 @@ func (k *KatibUIHandler) FetchExperiment(w http.ResponseWriter, r *http.Request)
namespaces, ok := r.URL.Query()["namespace"]
if !ok {
log.Printf("No 'namespace' query parameter was provided.")
err := errors.New("No 'namespace' query parameter was provided.")
err := errors.New("no 'namespace' query parameter was provided")
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
experimentNames, ok := r.URL.Query()["experimentName"]
if !ok {
log.Printf("No experimentName provided in Query parameteres! Provide an 'experimentName' param")
err := errors.New("No experimentName provided!")
err := errors.New("no experimentName provided")
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
Expand Down Expand Up @@ -575,14 +579,14 @@ func (k *KatibUIHandler) FetchSuggestion(w http.ResponseWriter, r *http.Request)
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!")
err := errors.New("no 'namespace' provided")
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
suggestionNames, ok := r.URL.Query()["suggestionName"]
if !ok {
log.Printf("No experimentName provided in Query parameteres! Provide an 'experimentName' param")
err := errors.New("No experimentName provided!")
err := errors.New("no experimentName provided")
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
Expand Down
10 changes: 5 additions & 5 deletions pkg/new-ui/v1beta1/hp.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package v1beta1
import (
"context"
"encoding/json"
"errors"
"log"
"net/http"
"strconv"
Expand All @@ -27,7 +28,6 @@ import (

commonv1beta1 "github.com/kubeflow/katib/pkg/apis/controller/common/v1beta1"
api_pb_v1beta1 "github.com/kubeflow/katib/pkg/apis/manager/v1beta1"
"github.com/pkg/errors"
)

const kfpRunIDAnnotation = "kubeflow-kale.org/kfp-run-uuid"
Expand All @@ -45,14 +45,14 @@ func (k *KatibUIHandler) FetchHPJobInfo(w http.ResponseWriter, r *http.Request)
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!")
err := errors.New("no 'namespace' provided")
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
experimentNames, ok := r.URL.Query()["experimentName"]
if !ok {
log.Printf("No experimentName provided in Query parameteres! Provide an 'experimentName' param")
err := errors.New("No experimentName provided!")
err := errors.New("no experimentName provided")
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
Expand Down Expand Up @@ -201,15 +201,15 @@ func (k *KatibUIHandler) FetchHPJobTrialInfo(w http.ResponseWriter, r *http.Requ
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!")
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!")
err := errors.New("no 'trialName' provided")
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
Expand Down

0 comments on commit 0377ed3

Please sign in to comment.