Skip to content

Commit

Permalink
feat(bff): list user namespaces in dev mode (#644)
Browse files Browse the repository at this point in the history
* feat(bff): list user namespaces in dev mode

Signed-off-by: Eder Ignatowicz <[email protected]>

* Update clients/ui/bff/internal/api/namespaces_handler_test.go

Co-authored-by: Alex Creasy <[email protected]>
Signed-off-by: Eder Ignatowicz <[email protected]>

---------

Signed-off-by: Eder Ignatowicz <[email protected]>
Co-authored-by: Alex Creasy <[email protected]>
  • Loading branch information
ederign and alexcreasy authored Dec 19, 2024
1 parent a4e3600 commit 4867116
Show file tree
Hide file tree
Showing 15 changed files with 311 additions and 33 deletions.
7 changes: 6 additions & 1 deletion clients/ui/bff/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ make docker-build
|----------------------------------------------------------------------------------------------|----------------------------------------------|-------------------------------------------------------------|
| GET /v1/healthcheck | HealthcheckHandler | Show application information. |
| GET /v1/user | UserHandler | Show "kubeflow-user-id" from header information. |
| GET /v1/namespaces | NamespacesHandler | Get all user namespaces. |
| GET /v1/model_registry | ModelRegistryHandler | Get all model registries, |
| GET /v1/model_registry/{model_registry_id}/registered_models | GetAllRegisteredModelsHandler | Gets a list of all RegisteredModel entities. |
| POST /v1/model_registry/{model_registry_id}/registered_models | CreateRegisteredModelHandler | Create a RegisteredModel entity. |
Expand All @@ -83,6 +84,10 @@ curl -i -H "kubeflow-userid: [email protected]" localhost:4000/api/v1/healthcheck
curl -i -H "kubeflow-userid: [email protected]" localhost:4000/api/v1/user
```
```
# GET /v1/namespaces
curl -i -H "kubeflow-userid: [email protected]" localhost:4000/api/v1/namespaces
```
```
# GET /v1/model_registry
curl -i -H "kubeflow-userid: [email protected]" localhost:4000/api/v1/model_registry
```
Expand Down Expand Up @@ -250,4 +255,4 @@ The mock Kubernetes environment is activated when the environment variable `MOCK
- `model-registry`: resides in the `kubeflow` namespace with the label `component: model-registry`.
- `model-registry-dora`: resides in the `dora-namespace` namespace with the label `component: model-registry`.
- `model-registry-bella`: resides in the `kubeflow` namespace with the label `component: model-registry`.
- `non-model-registry`: resides in the `kubeflow` namespace *without* the label `component: model-registry`.
- `non-model-registry`: resides in the `kubeflow` namespace *without* the label `component: model-registry`.
15 changes: 12 additions & 3 deletions clients/ui/bff/internal/api/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ const (
HealthCheckPath = PathPrefix + "/healthcheck"
UserPath = PathPrefix + "/user"
ModelRegistryListPath = PathPrefix + "/model_registry"
NamespaceListPath = PathPrefix + "/namespaces"
ModelRegistryPath = ModelRegistryListPath + "/:" + ModelRegistryId
RegisteredModelListPath = ModelRegistryPath + "/registered_models"
RegisteredModelPath = RegisteredModelListPath + "/:" + RegisteredModelId
Expand Down Expand Up @@ -96,17 +97,25 @@ func (app *App) Routes() http.Handler {
router.PATCH(RegisteredModelPath, app.AttachRESTClient(app.UpdateRegisteredModelHandler))
router.GET(RegisteredModelVersionsPath, app.AttachRESTClient(app.GetAllModelVersionsForRegisteredModelHandler))
router.POST(RegisteredModelVersionsPath, app.AttachRESTClient(app.CreateModelVersionForRegisteredModelHandler))

router.GET(ModelVersionPath, app.AttachRESTClient(app.GetModelVersionHandler))
router.POST(ModelVersionListPath, app.AttachRESTClient(app.CreateModelVersionHandler))
router.PATCH(ModelVersionPath, app.AttachRESTClient(app.UpdateModelVersionHandler))
router.GET(ModelVersionArtifactListPath, app.AttachRESTClient(app.GetAllModelArtifactsByModelVersionHandler))
router.POST(ModelVersionArtifactListPath, app.AttachRESTClient(app.CreateModelArtifactByModelVersionHandler))
router.PATCH(ModelRegistryPath, app.AttachRESTClient(app.UpdateModelVersionHandler))

// Kubernetes client routes
router.GET(UserPath, app.UserHandler)
router.GET(ModelRegistryListPath, app.ModelRegistryHandler)
router.PATCH(ModelRegistryPath, app.AttachRESTClient(app.UpdateModelVersionHandler))
if app.config.DevMode {
router.GET(NamespaceListPath, app.GetNamespacesHandler)
}

accessControlExemptPaths := map[string]struct{}{
HealthCheckPath: {},
UserPath: {},
NamespaceListPath: {},
}

return app.RecoverPanic(app.enableCORS(app.RequireAccessControl(router)))
return app.RecoverPanic(app.enableCORS(app.RequireAccessControl(app.InjectUserHeaders(router), accessControlExemptPaths)))
}
6 changes: 4 additions & 2 deletions clients/ui/bff/internal/api/healthcheck__handler_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package api

import (
"context"
"encoding/json"
"github.com/kubeflow/model-registry/ui/bff/internal/config"
"github.com/kubeflow/model-registry/ui/bff/internal/mocks"
Expand All @@ -25,11 +26,12 @@ func TestHealthCheckHandler(t *testing.T) {

rr := httptest.NewRecorder()
req, err := http.NewRequest(http.MethodGet, HealthCheckPath, nil)
ctx := context.WithValue(req.Context(), KubeflowUserIdKey, mocks.KubeflowUserIDHeaderValue)
req = req.WithContext(ctx)
assert.NoError(t, err)

req.Header.Set(kubeflowUserId, mocks.KubeflowUserIDHeaderValue)

app.HealthcheckHandler(rr, req, nil)

rs := rr.Result()

defer rs.Body.Close()
Expand Down
9 changes: 7 additions & 2 deletions clients/ui/bff/internal/api/healthcheck_handler.go
Original file line number Diff line number Diff line change
@@ -1,15 +1,20 @@
package api

import (
"errors"
"github.com/julienschmidt/httprouter"
"net/http"
)

func (app *App) HealthcheckHandler(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {

userID := r.Header.Get(kubeflowUserId)
userId, ok := r.Context().Value(KubeflowUserIdKey).(string)
if !ok || userId == "" {
app.serverErrorResponse(w, r, errors.New("failed to retrieve kubeflow-userid from context"))
return
}

healthCheck, err := app.repositories.HealthCheck.HealthCheck(Version, userID)
healthCheck, err := app.repositories.HealthCheck.HealthCheck(Version, userId)
if err != nil {
app.serverErrorResponse(w, r, err)
return
Expand Down
48 changes: 36 additions & 12 deletions clients/ui/bff/internal/api/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package api

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

Expand All @@ -12,8 +13,17 @@ import (

type contextKey string

const httpClientKey contextKey = "httpClientKey"
const kubeflowUserId = "kubeflow-userid"
const (
httpClientKey contextKey = "httpClientKey"

//Kubeflow authorization operates using custom authentication headers:
// Note: The functionality for `kubeflow-groups` is not fully operational at Kubeflow platform at this time
// But it will be soon implemented on Model Registry BFF
KubeflowUserIdKey contextKey = "kubeflowUserId" // kubeflow-userid :contains the user's email address
KubeflowUserIDHeader = "kubeflow-userid"
KubeflowUserGroupsKey contextKey = "kubeflowUserGroups" // kubeflow-groups : Holds a comma-separated list of user groups
KubeflowUserGroupsIdHeader = "kubeflow-groups"
)

func (app *App) RecoverPanic(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
Expand All @@ -28,6 +38,26 @@ func (app *App) RecoverPanic(next http.Handler) http.Handler {
})
}

func (app *App) InjectUserHeaders(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

userId := r.Header.Get(KubeflowUserIDHeader)
userGroups := r.Header.Get(KubeflowUserGroupsIdHeader)

//Note: The functionality for `kubeflow-groups` is not fully operational at Kubeflow platform at this time
if userId == "" {
app.badRequestResponse(w, r, errors.New("missing required header: kubeflow-userid"))
return
}

ctx := r.Context()
ctx = context.WithValue(ctx, KubeflowUserIdKey, userId)
ctx = context.WithValue(ctx, KubeflowUserGroupsKey, userGroups)

next.ServeHTTP(w, r.WithContext(ctx))
})
}

func (app *App) enableCORS(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// TODO(ederign) restrict CORS to a much smaller set of trusted origins.
Expand Down Expand Up @@ -74,22 +104,16 @@ func resolveModelRegistryURL(id string, client integrations.KubernetesClientInte
return url, nil
}

func (app *App) RequireAccessControl(next http.Handler) http.Handler {
func (app *App) RequireAccessControl(next http.Handler, exemptPaths map[string]struct{}) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

// Skip SAR for health check
if r.URL.Path == HealthCheckPath {
next.ServeHTTP(w, r)
return
}

// Skip SAR for user info
if r.URL.Path == UserPath {
// Skip SAR for exempt paths
if _, exempt := exemptPaths[r.URL.Path]; exempt {
next.ServeHTTP(w, r)
return
}

user := r.Header.Get(kubeflowUserId)
user := r.Header.Get(KubeflowUserIDHeader)
if user == "" {
app.forbiddenResponse(w, r, "missing kubeflow-userid header")
return
Expand Down
36 changes: 36 additions & 0 deletions clients/ui/bff/internal/api/namespaces_handler.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package api

import (
"errors"
"github.com/kubeflow/model-registry/ui/bff/internal/models"
"net/http"

"github.com/julienschmidt/httprouter"
)

type NamespacesEnvelope Envelope[[]models.NamespaceModel, None]

func (app *App) GetNamespacesHandler(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {

userId, ok := r.Context().Value(KubeflowUserIdKey).(string)
if !ok || userId == "" {
app.serverErrorResponse(w, r, errors.New("failed to retrieve kubeflow-userid from context"))
return
}

namespaces, err := app.repositories.Namespace.GetNamespaces(app.kubernetesClient, userId)
if err != nil {
app.serverErrorResponse(w, r, err)
return
}

namespacesEnvelope := NamespacesEnvelope{
Data: namespaces,
}

err = app.WriteJSON(w, http.StatusOK, namespacesEnvelope, nil)

if err != nil {
app.serverErrorResponse(w, r, err)
}
}
113 changes: 113 additions & 0 deletions clients/ui/bff/internal/api/namespaces_handler_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
package api

import (
"context"
"encoding/json"
"github.com/kubeflow/model-registry/ui/bff/internal/config"
"github.com/kubeflow/model-registry/ui/bff/internal/mocks"
"github.com/kubeflow/model-registry/ui/bff/internal/models"
"github.com/kubeflow/model-registry/ui/bff/internal/repositories"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"io"
"net/http"
"net/http/httptest"
)

var _ = Describe("TestNamespacesHandler", func() {
Context("when running in dev mode", Ordered, func() {
var testApp App

BeforeAll(func() {
By("setting up the test app in dev mode")
testApp = App{
config: config.EnvConfig{DevMode: true},
kubernetesClient: k8sClient,
repositories: repositories.NewRepositories(mockMRClient),
logger: logger,
}
})

It("should return only dora-namespace for [email protected]", func() {
By("creating the HTTP request with the kubeflow-userid header")
req, err := http.NewRequest(http.MethodGet, NamespaceListPath, nil)
ctx := context.WithValue(req.Context(), KubeflowUserIdKey, mocks.DoraNonAdminUser)
req = req.WithContext(ctx)
Expect(err).NotTo(HaveOccurred())
rr := httptest.NewRecorder()

By("calling the GetNamespacesHandler")
testApp.GetNamespacesHandler(rr, req, nil)
rs := rr.Result()
defer rs.Body.Close()
body, err := io.ReadAll(rs.Body)
Expect(err).NotTo(HaveOccurred())

By("unmarshalling the response")
var actual NamespacesEnvelope
err = json.Unmarshal(body, &actual)
Expect(err).NotTo(HaveOccurred())
Expect(rr.Code).To(Equal(http.StatusOK))

By("validating the response contains only dora-namespace")
expected := []models.NamespaceModel{{Name: "dora-namespace"}}
Expect(actual.Data).To(ConsistOf(expected))
})

It("should return all namespaces for [email protected]", func() {
By("creating the HTTP request with the kubeflow-userid header")
req, err := http.NewRequest(http.MethodGet, NamespaceListPath, nil)
ctx := context.WithValue(req.Context(), KubeflowUserIdKey, mocks.KubeflowUserIDHeaderValue)
req = req.WithContext(ctx)
Expect(err).NotTo(HaveOccurred())
req.Header.Set("kubeflow-userid", "[email protected]")
rr := httptest.NewRecorder()

By("calling the GetNamespacesHandler")
testApp.GetNamespacesHandler(rr, req, nil)
rs := rr.Result()
defer rs.Body.Close()
body, err := io.ReadAll(rs.Body)
Expect(err).NotTo(HaveOccurred())

By("unmarshalling the response")
var actual NamespacesEnvelope
err = json.Unmarshal(body, &actual)
Expect(err).NotTo(HaveOccurred())
Expect(rr.Code).To(Equal(http.StatusOK))

By("validating the response contains all namespaces")
expected := []models.NamespaceModel{
{Name: "kubeflow"},
{Name: "dora-namespace"},
}
Expect(actual.Data).To(ContainElements(expected))
})

It("should return no namespaces for non-existent user", func() {
By("creating the HTTP request with a non-existent kubeflow-userid")
req, err := http.NewRequest(http.MethodGet, NamespaceListPath, nil)
ctx := context.WithValue(req.Context(), KubeflowUserIdKey, "[email protected]")
req = req.WithContext(ctx)
Expect(err).NotTo(HaveOccurred())
rr := httptest.NewRecorder()

By("calling the GetNamespacesHandler")
testApp.GetNamespacesHandler(rr, req, nil)
rs := rr.Result()
defer rs.Body.Close()
body, err := io.ReadAll(rs.Body)
Expect(err).NotTo(HaveOccurred())

By("unmarshalling the response")
var actual NamespacesEnvelope
err = json.Unmarshal(body, &actual)
Expect(err).NotTo(HaveOccurred())
Expect(rr.Code).To(Equal(http.StatusOK))

By("validating the response contains no namespaces")
Expect(actual.Data).To(BeEmpty())
})
})

})
4 changes: 2 additions & 2 deletions clients/ui/bff/internal/api/test_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
"net/http/httptest"
)

func setupApiTest[T any](method string, url string, body interface{}, k8sClient k8s.KubernetesClientInterface, kubeflowUserIDHeader string) (T, *http.Response, error) {
func setupApiTest[T any](method string, url string, body interface{}, k8sClient k8s.KubernetesClientInterface, kubeflowUserIDHeaderValue string) (T, *http.Response, error) {
mockMRClient, err := mocks.NewModelRegistryClient(nil)
if err != nil {
return *new(T), nil, err
Expand Down Expand Up @@ -44,7 +44,7 @@ func setupApiTest[T any](method string, url string, body interface{}, k8sClient
}

// Set the kubeflow-userid header
req.Header.Set(kubeflowUserId, kubeflowUserIDHeader)
req.Header.Set(KubeflowUserIDHeader, kubeflowUserIDHeaderValue)

ctx := context.WithValue(req.Context(), httpClientKey, mockClient)
req = req.WithContext(ctx)
Expand Down
8 changes: 4 additions & 4 deletions clients/ui/bff/internal/api/user_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@ type UserEnvelope Envelope[*models.User, None]

func (app *App) UserHandler(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {

userHeader := r.Header.Get(kubeflowUserId)
if userHeader == "" {
app.serverErrorResponse(w, r, errors.New("kubeflow-userid not present on header"))
userId, ok := r.Context().Value(KubeflowUserIdKey).(string)
if !ok || userId == "" {
app.serverErrorResponse(w, r, errors.New("failed to retrieve kubeflow-userid from context"))
return
}

user, err := app.repositories.User.GetUser(app.kubernetesClient, userHeader)
user, err := app.repositories.User.GetUser(app.kubernetesClient, userId)
if err != nil {
app.serverErrorResponse(w, r, err)
return
Expand Down
Loading

0 comments on commit 4867116

Please sign in to comment.