Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(bff): list user namespaces in dev mode #644

Merged
merged 2 commits into from
Dec 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading