Skip to content

Commit 1581a64

Browse files
fix(bff): remove kubeflow-userid requirement from health check endpoint (kubeflow#898)
Signed-off-by: Christian Vogt <cvogt@redhat.com>
1 parent d6c20c4 commit 1581a64

10 files changed

Lines changed: 31 additions & 41 deletions

File tree

clients/ui/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ To run the a mocked dev environment you can either:
2727

2828
### Kubernetes Deployment
2929

30-
For a in-depth guide on how to deploy the Model Registry UI, please refer to the [local kubernetes deployment](./bff/docs/dev-guide.md) documentation.
30+
For a in-depth guide on how to deploy the Model Registry UI, please refer to the [local kubernetes deployment](./bff/docs/local-development-guide.md) documentation.
3131

3232
To quickly enable the Model Registry UI in your Kind cluster, you can use the following command:
3333

clients/ui/api/openapi/mod-arch.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ servers:
1010
- url: "https://localhost:8080"
1111
- url: "http://localhost:8080"
1212
paths:
13-
/api/v1/healthcheck:
13+
/healthcheck:
1414
summary: Path targeted for healthcheck purposes.
1515
description: >-
1616
The REST endpoint/path used to allow a healthcheck update.

clients/ui/bff/internal/api/app.go

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ const (
2727
ModelVersionId = "model_version_id"
2828
ModelArtifactId = "model_artifact_id"
2929
ArtifactId = "artifact_id"
30-
HealthCheckPath = ApiPathPrefix + "/healthcheck"
30+
HealthCheckPath = "/healthcheck"
3131
UserPath = ApiPathPrefix + "/user"
3232
ModelRegistryListPath = ApiPathPrefix + "/model_registry"
3333
ModelRegistryPath = ModelRegistryListPath + "/:" + ModelRegistryId
@@ -106,7 +106,6 @@ func (app *App) Routes() http.Handler {
106106

107107
// HTTP client routes (requests that we forward to Model Registry API)
108108
// on those, we perform SAR on Specific Service on a given namespace
109-
apiRouter.GET(HealthCheckPath, app.HealthcheckHandler)
110109
apiRouter.GET(RegisteredModelListPath, app.AttachNamespace(app.PerformSARonSpecificService(app.AttachRESTClient(app.GetAllRegisteredModelsHandler))))
111110
apiRouter.GET(RegisteredModelPath, app.AttachNamespace(app.PerformSARonSpecificService(app.AttachRESTClient(app.GetRegisteredModelHandler))))
112111
apiRouter.POST(RegisteredModelListPath, app.AttachNamespace(app.PerformSARonSpecificService(app.AttachRESTClient(app.CreateRegisteredModelHandler))))
@@ -162,5 +161,16 @@ func (app *App) Routes() http.Handler {
162161
http.ServeFile(w, r, path.Join(app.config.StaticAssetsDir, "index.html"))
163162
})
164163

165-
return app.RecoverPanic(app.EnableTelemetry(app.EnableCORS(app.InjectUserHeaders(appMux))))
164+
// Create a mux for the healthcheck endpoint
165+
healthcheckMux := http.NewServeMux()
166+
healthcheckRouter := httprouter.New()
167+
healthcheckRouter.GET(HealthCheckPath, app.HealthcheckHandler)
168+
healthcheckMux.Handle(HealthCheckPath, app.RecoverPanic(app.EnableTelemetry(healthcheckRouter)))
169+
170+
// Combines the healthcheck endpoint with the rest of the routes
171+
combinedMux := http.NewServeMux()
172+
combinedMux.Handle(HealthCheckPath, healthcheckMux)
173+
combinedMux.Handle("/", app.RecoverPanic(app.EnableTelemetry(app.EnableCORS(app.InjectUserHeaders(appMux)))))
174+
175+
return combinedMux
166176
}

clients/ui/bff/internal/api/healthcheck__handler_test.go

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,20 @@
11
package api
22

33
import (
4-
"context"
54
"encoding/json"
5+
"io"
6+
"net/http"
7+
"net/http/httptest"
8+
"testing"
9+
610
"github.com/kubeflow/model-registry/ui/bff/internal/config"
7-
"github.com/kubeflow/model-registry/ui/bff/internal/constants"
811
"github.com/kubeflow/model-registry/ui/bff/internal/mocks"
912
"github.com/kubeflow/model-registry/ui/bff/internal/models"
1013
"github.com/kubeflow/model-registry/ui/bff/internal/repositories"
1114
"github.com/stretchr/testify/assert"
12-
"io"
13-
"net/http"
14-
"net/http/httptest"
15-
"testing"
1615
)
1716

1817
func TestHealthCheckHandler(t *testing.T) {
19-
2018
mockMRClient, _ := mocks.NewModelRegistryClient(nil)
2119

2220
app := App{config: config.EnvConfig{
@@ -27,14 +25,11 @@ func TestHealthCheckHandler(t *testing.T) {
2725

2826
rr := httptest.NewRecorder()
2927
req, err := http.NewRequest(http.MethodGet, HealthCheckPath, nil)
30-
ctx := context.WithValue(req.Context(), constants.KubeflowUserIdKey, mocks.KubeflowUserIDHeaderValue)
31-
req = req.WithContext(ctx)
3228
assert.NoError(t, err)
3329

3430
app.HealthcheckHandler(rr, req, nil)
3531

3632
rs := rr.Result()
37-
3833
defer rs.Body.Close()
3934

4035
body, err := io.ReadAll(rs.Body)
@@ -51,7 +46,6 @@ func TestHealthCheckHandler(t *testing.T) {
5146
SystemInfo: models.SystemInfo{
5247
Version: Version,
5348
},
54-
UserID: mocks.KubeflowUserIDHeaderValue,
5549
}
5650

5751
assert.Equal(t, expected, healthCheckRes)
Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,20 @@
11
package api
22

33
import (
4-
"errors"
5-
"github.com/julienschmidt/httprouter"
6-
"github.com/kubeflow/model-registry/ui/bff/internal/constants"
74
"net/http"
5+
6+
"github.com/julienschmidt/httprouter"
87
)
98

109
func (app *App) HealthcheckHandler(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
11-
12-
userId, ok := r.Context().Value(constants.KubeflowUserIdKey).(string)
13-
if !ok || userId == "" {
14-
app.serverErrorResponse(w, r, errors.New("failed to retrieve kubeflow-userid from context"))
15-
return
16-
}
17-
18-
healthCheck, err := app.repositories.HealthCheck.HealthCheck(Version, userId)
10+
healthCheck, err := app.repositories.HealthCheck.HealthCheck(Version)
1911
if err != nil {
2012
app.serverErrorResponse(w, r, err)
2113
return
2214
}
2315

2416
err = app.WriteJSON(w, http.StatusOK, healthCheck, nil)
25-
2617
if err != nil {
2718
app.serverErrorResponse(w, r, err)
2819
}
29-
3020
}

clients/ui/bff/internal/models/health_check.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,4 @@ type SystemInfo struct {
77
type HealthCheckModel struct {
88
Status string `json:"status"`
99
SystemInfo SystemInfo `json:"system_info"`
10-
UserID string `json:"userId"`
1110
}

clients/ui/bff/internal/repositories/health_check.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,12 @@ func NewHealthCheckRepository() *HealthCheckRepository {
88
return &HealthCheckRepository{}
99
}
1010

11-
func (r *HealthCheckRepository) HealthCheck(version string, userID string) (models.HealthCheckModel, error) {
12-
11+
func (r *HealthCheckRepository) HealthCheck(version string) (models.HealthCheckModel, error) {
1312
var res = models.HealthCheckModel{
1413
Status: "available",
1514
SystemInfo: models.SystemInfo{
1615
Version: version,
1716
},
18-
UserID: userID,
1917
}
2018

2119
return res, nil

clients/ui/docs/local-deployment-guide-ui.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ First you need to set up your new image
4343

4444
```shell
4545
cd manifests/kustomize/options/ui/base
46-
kustomize edit set image model-registry-ui-image=${IMG_UI_STANDALONE}
46+
kustomize edit set image model-registry-ui=${IMG_UI_STANDALONE}
4747
```
4848

4949
Now you can set the namespace to kubeflow and apply the manifests:

clients/ui/scripts/deploy_kind_cluster.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ command -v docker >/dev/null 2>&1 || { echo >&2 "Docker is required but it's not
55
command -v kubectl >/dev/null 2>&1 || { echo >&2 "kubectl is required but it's not installed. Aborting."; exit 1; }
66
command -v kind >/dev/null 2>&1 || { echo >&2 "kind is required but it's not installed. Aborting."; exit 1; }
77

8-
echo "WARNING: You must have proper push / pull access to ${IMG_UI_STANDALONE}". If this is a new image, make sure you set it to public to avoid issues."
8+
echo "WARNING: You must have proper push / pull access to ${IMG_UI_STANDALONE}. If this is a new image, make sure you set it to public to avoid issues."
99

1010
if kubectl get deployment model-registry-deployment -n kubeflow >/dev/null 2>&1; then
1111
echo "Model Registry deployment already exists. Skipping to step 4."
@@ -42,7 +42,7 @@ make docker-push-standalone
4242

4343
echo "Editing kustomize image..."
4444
pushd ../../manifests/kustomize/options/ui/base
45-
kustomize edit set image model-registry-ui-image=${IMG_UI_STANDALONE}
45+
kustomize edit set image model-registry-ui=${IMG_UI_STANDALONE}
4646

4747
pushd ../overlays/standalone
4848
# Step 4: Deploy model registry UI

manifests/kustomize/options/ui/base/model-registry-ui-deployment.yaml

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,21 +24,20 @@ spec:
2424
image: model-registry-ui
2525
imagePullPolicy: Always
2626
livenessProbe:
27-
tcpSocket:
27+
httpGet:
28+
path: /healthcheck
2829
port: 8080
30+
scheme: HTTP
2931
initialDelaySeconds: 30
3032
timeoutSeconds: 15
3133
periodSeconds: 30
3234
successThreshold: 1
3335
failureThreshold: 3
3436
readinessProbe:
3537
httpGet:
36-
path: /api/v1/healthcheck
38+
path: /healthcheck
3739
port: 8080
3840
scheme: HTTP
39-
httpHeaders:
40-
- name: kubeflow-userid
41-
value: user@example.com
4241
initialDelaySeconds: 15
4342
timeoutSeconds: 15
4443
periodSeconds: 30

0 commit comments

Comments
 (0)