Skip to content

Commit 0377ed3

Browse files
apo-gerapo-ger
committed
Review fixes
* proper error handling. * switch to Go's build-in errors package. * set appropriate verbs when constructing SAR objects. Signed-off-by: Apostolos Gerakaris <[email protected]>
1 parent dea8129 commit 0377ed3

File tree

3 files changed

+22
-18
lines changed

3 files changed

+22
-18
lines changed

‎pkg/new-ui/v1beta1/authzn.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@ package v1beta1
22

33
import (
44
"context"
5+
"errors"
56
"fmt"
67
"log"
78
"net/http"
89
"strings"
910

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

3232
if r.Header.Get(USER_HEADER) == "" {
33-
return "", errors.New("User header not present!")
33+
return "", errors.New("user header not present")
3434
}
3535

3636
user := r.Header.Get(USER_HEADER)

‎pkg/new-ui/v1beta1/backend.go‎

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package v1beta1
1818

1919
import (
2020
"encoding/json"
21+
"errors"
2122
"log"
2223
"net/http"
2324
"path/filepath"
@@ -31,7 +32,6 @@ import (
3132
experimentv1beta1 "github.com/kubeflow/katib/pkg/apis/controller/experiments/v1beta1"
3233
api_pb_v1beta1 "github.com/kubeflow/katib/pkg/apis/manager/v1beta1"
3334
"github.com/kubeflow/katib/pkg/util/v1beta1/katibclient"
34-
"github.com/pkg/errors"
3535
)
3636

3737
func NewKatibUIHandler(dbManagerAddr string) *KatibUIHandler {
@@ -40,8 +40,12 @@ func NewKatibUIHandler(dbManagerAddr string) *KatibUIHandler {
4040
log.Printf("NewClient for Katib failed: %v", err)
4141
panic(err)
4242
}
43-
// create a new client-go client for sending SAR objects in the API-SERVER
43+
// create a new client for manipulating SAR objects.
4444
conf, err := config.GetConfig()
45+
if err != nil {
46+
log.Printf("Failed to create k8s rest config: %v", err)
47+
panic(err)
48+
}
4549
sarclient, err := kubernetes.NewForConfig(conf)
4650
if err != nil {
4751
log.Printf("SarClient for Katib failes: %v", err)
@@ -147,7 +151,7 @@ func (k *KatibUIHandler) FetchNamespacedExperiments(w http.ResponseWriter, r *ht
147151
namespaces, ok := r.URL.Query()["namespace"]
148152
if !ok {
149153
log.Printf("No 'namespace' query parameter was provided.")
150-
err := errors.New("No 'namespace' query parameter was provided.")
154+
err := errors.New("no 'namespace' query parameter was provided")
151155
http.Error(w, err.Error(), http.StatusBadRequest)
152156
return
153157
}
@@ -218,14 +222,14 @@ func (k *KatibUIHandler) DeleteExperiment(w http.ResponseWriter, r *http.Request
218222
namespaces, ok := r.URL.Query()["namespace"]
219223
if !ok {
220224
log.Printf("No 'namespace' query parameter was provided.")
221-
err := errors.New("No 'namespace' query parameter was provided.")
225+
err := errors.New("no 'namespace' query parameter was provided")
222226
http.Error(w, err.Error(), http.StatusBadRequest)
223227
return
224228
}
225229
experimentNames, ok := r.URL.Query()["experimentName"]
226230
if !ok {
227231
log.Printf("No experimentName provided in Query parameteres! Provide an 'experimentName' param")
228-
err := errors.New("No experimentName provided!")
232+
err := errors.New("no experimentName provided")
229233
http.Error(w, err.Error(), http.StatusBadRequest)
230234
return
231235
}
@@ -349,7 +353,7 @@ func (k *KatibUIHandler) AddTemplate(w http.ResponseWriter, r *http.Request) {
349353
updatedConfigMapPath := data["updatedConfigMapPath"].(string)
350354
updatedTemplateYaml := data["updatedTemplateYaml"].(string)
351355

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

407-
err = IsAuthorized(user, "edit", updatedConfigMapNamespace, "", "v1", "configmaps", "", updatedConfigMapName, &k.sarClient)
411+
err = IsAuthorized(user, "update", updatedConfigMapNamespace, "", "v1", "configmaps", "", updatedConfigMapName, &k.sarClient)
408412
if err != nil {
409413
log.Printf("The user: %s is not authorized to edit configmap: %s from namespace: %s \n", user, updatedConfigMapName, updatedConfigMapNamespace)
410414
http.Error(w, err.Error(), http.StatusForbidden)
@@ -522,14 +526,14 @@ func (k *KatibUIHandler) FetchExperiment(w http.ResponseWriter, r *http.Request)
522526
namespaces, ok := r.URL.Query()["namespace"]
523527
if !ok {
524528
log.Printf("No 'namespace' query parameter was provided.")
525-
err := errors.New("No 'namespace' query parameter was provided.")
529+
err := errors.New("no 'namespace' query parameter was provided")
526530
http.Error(w, err.Error(), http.StatusBadRequest)
527531
return
528532
}
529533
experimentNames, ok := r.URL.Query()["experimentName"]
530534
if !ok {
531535
log.Printf("No experimentName provided in Query parameteres! Provide an 'experimentName' param")
532-
err := errors.New("No experimentName provided!")
536+
err := errors.New("no experimentName provided")
533537
http.Error(w, err.Error(), http.StatusBadRequest)
534538
return
535539
}
@@ -575,14 +579,14 @@ func (k *KatibUIHandler) FetchSuggestion(w http.ResponseWriter, r *http.Request)
575579
namespaces, ok := r.URL.Query()["namespace"]
576580
if !ok {
577581
log.Printf("No namespace provided in Query parameters! Provide a 'namespace' param")
578-
err := errors.New("No 'namespace' provided!")
582+
err := errors.New("no 'namespace' provided")
579583
http.Error(w, err.Error(), http.StatusBadRequest)
580584
return
581585
}
582586
suggestionNames, ok := r.URL.Query()["suggestionName"]
583587
if !ok {
584588
log.Printf("No experimentName provided in Query parameteres! Provide an 'experimentName' param")
585-
err := errors.New("No experimentName provided!")
589+
err := errors.New("no experimentName provided")
586590
http.Error(w, err.Error(), http.StatusBadRequest)
587591
return
588592
}

‎pkg/new-ui/v1beta1/hp.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package v1beta1
1919
import (
2020
"context"
2121
"encoding/json"
22+
"errors"
2223
"log"
2324
"net/http"
2425
"strconv"
@@ -27,7 +28,6 @@ import (
2728

2829
commonv1beta1 "github.com/kubeflow/katib/pkg/apis/controller/common/v1beta1"
2930
api_pb_v1beta1 "github.com/kubeflow/katib/pkg/apis/manager/v1beta1"
30-
"github.com/pkg/errors"
3131
)
3232

3333
const kfpRunIDAnnotation = "kubeflow-kale.org/kfp-run-uuid"
@@ -45,14 +45,14 @@ func (k *KatibUIHandler) FetchHPJobInfo(w http.ResponseWriter, r *http.Request)
4545
namespaces, ok := r.URL.Query()["namespace"]
4646
if !ok {
4747
log.Printf("No namespace provided in Query parameters! Provide a 'namespace' param")
48-
err := errors.New("No 'namespace' provided!")
48+
err := errors.New("no 'namespace' provided")
4949
http.Error(w, err.Error(), http.StatusBadRequest)
5050
return
5151
}
5252
experimentNames, ok := r.URL.Query()["experimentName"]
5353
if !ok {
5454
log.Printf("No experimentName provided in Query parameteres! Provide an 'experimentName' param")
55-
err := errors.New("No experimentName provided!")
55+
err := errors.New("no experimentName provided")
5656
http.Error(w, err.Error(), http.StatusBadRequest)
5757
return
5858
}
@@ -201,15 +201,15 @@ func (k *KatibUIHandler) FetchHPJobTrialInfo(w http.ResponseWriter, r *http.Requ
201201
namespaces, ok := r.URL.Query()["namespace"]
202202
if !ok {
203203
log.Printf("No namespace provided in Query parameters! Provide a 'namespace' param")
204-
err := errors.New("No 'namespace' provided!")
204+
err := errors.New("no 'namespace' provided")
205205
http.Error(w, err.Error(), http.StatusBadRequest)
206206
return
207207
}
208208

209209
trialNames, ok := r.URL.Query()["trialName"]
210210
if !ok {
211211
log.Printf("No trialName provided in Query parameters! Provide a 'trialName' param")
212-
err := errors.New("No 'trialName' provided!")
212+
err := errors.New("no 'trialName' provided")
213213
http.Error(w, err.Error(), http.StatusBadRequest)
214214
return
215215
}

0 commit comments

Comments
 (0)