Skip to content

Commit 367a87f

Browse files
authored
Federated mr implementation (#1288)
Enable MR access in federated mode Signed-off-by: lucferbux <[email protected]>
1 parent a7494aa commit 367a87f

File tree

13 files changed

+150
-48
lines changed

13 files changed

+150
-48
lines changed

clients/ui/Makefile

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,11 @@ dev-install-dependencies:
3232

3333
.PHONY: dev-bff
3434
dev-bff:
35-
cd bff && make run PORT=4000 MOCK_K8S_CLIENT=true MOCK_MR_CLIENT=true DEV_MODE=true STANDALONE_MODE=true FEDERATED_PLATFORM=false
35+
cd bff && make run PORT=4000 MOCK_K8S_CLIENT=true MOCK_MR_CLIENT=true DEV_MODE=true DEPLOYMENT_MODE=standalone
3636

3737
.PHONY: dev-frontend
3838
dev-frontend:
39-
DEPLOYMENT_MODE=standalone && STYLE_THEME=mui-theme cd frontend && npm run start:dev
39+
cd frontend && DEPLOYMENT_MODE=standalone STYLE_THEME=mui-theme npm run start:dev
4040

4141
.PHONY: dev-start
4242
dev-start:
@@ -49,24 +49,24 @@ dev-start-kubeflow:
4949

5050
.PHONY: dev-frontend-kubeflow
5151
dev-frontend-kubeflow:
52-
DEPLOYMENT_MODE=kubeflow && STYLE_THEME=mui-theme && cd frontend && npm run start:dev
52+
cd frontend && DEPLOYMENT_MODE=kubeflow STYLE_THEME=mui-theme npm run start:dev
5353

5454
.PHONY: dev-bff-kubeflow
5555
dev-bff-kubeflow:
56-
cd bff && make run PORT=4000 MOCK_K8S_CLIENT=false MOCK_MR_CLIENT=false DEV_MODE=true STANDALONE_MODE=false FEDERATED_PLATFORM=false DEV_MODE_PORT=8085
56+
cd bff && make run PORT=4000 MOCK_K8S_CLIENT=false MOCK_MR_CLIENT=false DEV_MODE=true DEPLOYMENT_MODE=kubeflow DEV_MODE_PORT=8085
5757

58-
########### Dev Federated ############
58+
########### Dev Federated ###########
5959
.PHONY: dev-start-federated
6060
dev-start-federated:
6161
make -j 2 dev-bff-federated dev-frontend-federated
6262

6363
.PHONY: dev-frontend-federated
6464
dev-frontend-federated:
65-
DEPLOYMENT_MODE=federated && STYLE_THEME=patternfly && cd frontend && npm run start:dev
65+
cd frontend && AUTH_METHOD=user_token DEPLOYMENT_MODE=federated STYLE_THEME=patternfly npm run start:dev
6666

6767
.PHONY: dev-bff-federated
6868
dev-bff-federated:
69-
cd bff && make run PORT=4000 MOCK_K8S_CLIENT=false MOCK_MR_CLIENT=false DEV_MODE=true STANDALONE_MODE=false FEDERATED_PLATFORM=true DEV_MODE_PORT=8085
69+
cd bff && make run PORT=4000 MOCK_K8S_CLIENT=false MOCK_MR_CLIENT=false DEV_MODE=true DEPLOYMENT_MODE=federated DEV_MODE_PORT=8085 AUTH_METHOD=user_token AUTH_TOKEN_HEADER=x-forwarded-access-token AUTH_TOKEN_PREFIX=""
7070

7171
############ Build ############
7272

@@ -121,7 +121,7 @@ kubeflow-deployment:
121121
############ Build ############
122122
.PHONY: frontend-build
123123
frontend-build:
124-
cd frontend && DEPLOYMENT_MODE=kubeflow && npm run build:prod
124+
cd frontend && DEPLOYMENT_MODE=kubeflow npm run build:prod
125125

126126
.PHONY: frontend-build-standalone
127127
frontend-build-standalone:

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

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"runtime/debug"
99
"strings"
1010

11+
"github.com/kubeflow/model-registry/ui/bff/internal/config"
1112
"github.com/kubeflow/model-registry/ui/bff/internal/integrations/kubernetes"
1213
"github.com/kubeflow/model-registry/ui/bff/internal/integrations/mrserver"
1314

@@ -102,7 +103,7 @@ func (app *App) AttachRESTClient(next func(http.ResponseWriter, *http.Request, h
102103
return
103104
}
104105

105-
modelRegistry, err := app.repositories.ModelRegistry.GetModelRegistry(r.Context(), client, namespace, modelRegistryID)
106+
modelRegistry, err := app.repositories.ModelRegistry.GetModelRegistryWithMode(r.Context(), client, namespace, modelRegistryID, app.config.DeploymentMode.IsFederatedMode())
106107
if err != nil {
107108
app.notFoundResponse(w, r)
108109
return
@@ -111,8 +112,9 @@ func (app *App) AttachRESTClient(next func(http.ResponseWriter, *http.Request, h
111112

112113
// If we are in dev mode, we need to resolve the server address to the local host
113114
// to allow the client to connect to the model registry via port forwarded from the cluster to the local machine.
114-
if app.config.DevMode {
115-
modelRegistryBaseURL = app.repositories.ModelRegistry.ResolveServerAddress("localhost", int32(app.config.DevModePort), modelRegistry.IsHTTPS)
115+
// If you are in federated mode, we do not want to override the server address.
116+
if app.config.DevMode && !app.config.DeploymentMode.IsFederatedMode() {
117+
modelRegistryBaseURL = app.repositories.ModelRegistry.ResolveServerAddress("localhost", int32(app.config.DevModePort), modelRegistry.IsHTTPS, "", app.config.DeploymentMode.IsFederatedMode())
116118
}
117119

118120
// Set up a child logger for the rest client that automatically adds the request id to all statements for
@@ -127,7 +129,21 @@ func (app *App) AttachRESTClient(next func(http.ResponseWriter, *http.Request, h
127129
}
128130
}
129131

130-
restHttpClient, err := mrserver.NewHTTPClient(restClientLogger, modelRegistryID, modelRegistryBaseURL)
132+
// Prepare headers for the REST client
133+
headers := http.Header{}
134+
135+
// If using user token authentication, extract and forward the authorization header
136+
if app.config.AuthMethod == config.AuthMethodUser {
137+
identity, ok := r.Context().Value(constants.RequestIdentityKey).(*kubernetes.RequestIdentity)
138+
if ok && identity != nil && identity.Token != "" {
139+
// Always send as "Authorization: Bearer <token>" regardless of incoming header format
140+
// The identity.Token already has any prefix removed by ExtractRequestIdentity
141+
authHeaderValue := "Bearer " + identity.Token
142+
headers.Set("Authorization", authHeaderValue)
143+
}
144+
}
145+
146+
restHttpClient, err := mrserver.NewHTTPClient(restClientLogger, modelRegistryID, modelRegistryBaseURL, headers)
131147
if err != nil {
132148
app.serverErrorResponse(w, r, fmt.Errorf("failed to create Kubernetes client: %v", err))
133149
return

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ func (app *App) GetAllModelRegistriesHandler(w http.ResponseWriter, r *http.Requ
2525
return
2626
}
2727

28-
registries, err := app.repositories.ModelRegistry.GetAllModelRegistries(r.Context(), client, namespace)
28+
registries, err := app.repositories.ModelRegistry.GetAllModelRegistriesWithMode(r.Context(), client, namespace, app.config.DeploymentMode.IsFederatedMode())
2929
if err != nil {
3030
app.serverErrorResponse(w, r, err)
3131
return

clients/ui/bff/internal/integrations/kubernetes/shared_k8s_client.go

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"fmt"
66
"log/slog"
7+
"strings"
78
"time"
89

910
"github.com/kubeflow/model-registry/ui/bff/internal/constants"
@@ -96,25 +97,30 @@ func buildServiceDetails(service *corev1.Service, logger *slog.Logger) (*Service
9697

9798
displayName := ""
9899
description := ""
100+
externalAddressRest := ""
101+
102+
// Check for annotations including external-address-rest
99103
if service.Annotations != nil {
100104
displayName = service.Annotations["displayName"]
101105
description = service.Annotations["description"]
102-
}
103106

104-
if displayName == "" {
105-
logger.Warn("service missing displayName annotation", "serviceName", service.Name)
106-
}
107-
if description == "" {
108-
logger.Warn("service missing description annotation", "serviceName", service.Name)
107+
// Look for external-address-rest annotation with any prefix
108+
for key, value := range service.Annotations {
109+
if strings.HasSuffix(key, "/external-address-rest") {
110+
externalAddressRest = value
111+
break
112+
}
113+
}
109114
}
110115

111116
return &ServiceDetails{
112-
Name: service.Name,
113-
DisplayName: displayName,
114-
Description: description,
115-
ClusterIP: service.Spec.ClusterIP,
116-
HTTPPort: httpPort,
117-
IsHTTPS: isHTTPS,
117+
Name: service.Name,
118+
DisplayName: displayName,
119+
Description: description,
120+
ClusterIP: service.Spec.ClusterIP,
121+
HTTPPort: httpPort,
122+
IsHTTPS: isHTTPS,
123+
ExternalAddressRest: externalAddressRest,
118124
}, nil
119125
}
120126

clients/ui/bff/internal/integrations/kubernetes/types.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
package kubernetes
22

33
type ServiceDetails struct {
4-
Name string
5-
DisplayName string
6-
Description string
7-
ClusterIP string
8-
HTTPPort int32
9-
IsHTTPS bool
4+
Name string
5+
DisplayName string
6+
Description string
7+
ClusterIP string
8+
HTTPPort int32
9+
IsHTTPS bool
10+
ExternalAddressRest string
1011
}
1112

1213
type RequestIdentity struct {

clients/ui/bff/internal/integrations/mrserver/http.go

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ type HTTPClient struct {
2424
baseURL string
2525
ModelRegistryID string
2626
logger *slog.Logger
27+
Headers http.Header
2728
}
2829

2930
type ErrorResponse struct {
@@ -40,7 +41,7 @@ func (e *HTTPError) Error() string {
4041
return fmt.Sprintf("HTTP %d: %s - %s", e.StatusCode, e.Code, e.Message)
4142
}
4243

43-
func NewHTTPClient(logger *slog.Logger, modelRegistryID string, baseURL string) (HTTPClientInterface, error) {
44+
func NewHTTPClient(logger *slog.Logger, modelRegistryID string, baseURL string, headers http.Header) (HTTPClientInterface, error) {
4445

4546
return &HTTPClient{
4647
client: &http.Client{Transport: &http.Transport{
@@ -49,6 +50,7 @@ func NewHTTPClient(logger *slog.Logger, modelRegistryID string, baseURL string)
4950
baseURL: baseURL,
5051
ModelRegistryID: modelRegistryID,
5152
logger: logger,
53+
Headers: headers,
5254
}, nil
5355
}
5456

@@ -65,6 +67,8 @@ func (c *HTTPClient) GET(url string) ([]byte, error) {
6567
return nil, err
6668
}
6769

70+
c.applyHeaders(req)
71+
6872
logUpstreamReq(c.logger, requestId, req)
6973

7074
response, err := c.client.Do(req)
@@ -120,6 +124,8 @@ func (c *HTTPClient) POST(url string, body io.Reader) ([]byte, error) {
120124

121125
req.Header.Set("Content-Type", "application/json")
122126

127+
c.applyHeaders(req)
128+
123129
logUpstreamReq(c.logger, requestId, req)
124130

125131
response, err := c.client.Do(req)
@@ -175,6 +181,8 @@ func (c *HTTPClient) PATCH(url string, body io.Reader) ([]byte, error) {
175181

176182
req.Header.Set("Content-Type", "application/json")
177183

184+
c.applyHeaders(req)
185+
178186
logUpstreamReq(c.logger, requestId, req)
179187

180188
response, err := c.client.Do(req)
@@ -218,6 +226,16 @@ func (c *HTTPClient) PATCH(url string, body io.Reader) ([]byte, error) {
218226
return responseBody, nil
219227
}
220228

229+
func (c *HTTPClient) applyHeaders(req *http.Request) {
230+
if c.Headers != nil {
231+
for key, values := range c.Headers {
232+
for _, value := range values {
233+
req.Header.Add(key, value)
234+
}
235+
}
236+
}
237+
}
238+
221239
func logUpstreamReq(logger *slog.Logger, reqId string, req *http.Request) {
222240
logger.Debug("Making upstream HTTP request", slog.String("request_id", reqId), slog.Any("request", helper.RequestLogValuer{Request: req}))
223241
}

clients/ui/bff/internal/integrations/mrserver/http_test.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ func TestHTTPClient_GET_Success(t *testing.T) {
3737

3838
// Create http client pointing to test server
3939
logger := setupTestLogger()
40-
client, err := NewHTTPClient(logger, "test-registry", server.URL)
40+
client, err := NewHTTPClient(logger, "test-registry", server.URL, nil)
4141
require.NoError(t, err)
4242

4343
// Make the request
@@ -73,7 +73,7 @@ func TestHTTPClient_GET_Error(t *testing.T) {
7373

7474
// Create http client pointing to test server
7575
logger := setupTestLogger()
76-
client, err := NewHTTPClient(logger, "test-registry", server.URL)
76+
client, err := NewHTTPClient(logger, "test-registry", server.URL, nil)
7777
require.NoError(t, err)
7878

7979
// Make the request
@@ -123,7 +123,7 @@ func TestHTTPClient_POST_Success(t *testing.T) {
123123

124124
// Create http client pointing to test server
125125
logger := setupTestLogger()
126-
client, err := NewHTTPClient(logger, "test-registry", server.URL)
126+
client, err := NewHTTPClient(logger, "test-registry", server.URL, nil)
127127
require.NoError(t, err)
128128

129129
// Prepare request body
@@ -164,7 +164,7 @@ func TestHTTPClient_POST_Error(t *testing.T) {
164164

165165
// Create http client pointing to test server
166166
logger := setupTestLogger()
167-
client, err := NewHTTPClient(logger, "test-registry", server.URL)
167+
client, err := NewHTTPClient(logger, "test-registry", server.URL, nil)
168168
require.NoError(t, err)
169169

170170
// Prepare request body
@@ -218,7 +218,7 @@ func TestHTTPClient_PATCH_Success(t *testing.T) {
218218

219219
// Create http client pointing to test server
220220
logger := setupTestLogger()
221-
client, err := NewHTTPClient(logger, "test-registry", server.URL)
221+
client, err := NewHTTPClient(logger, "test-registry", server.URL, nil)
222222
require.NoError(t, err)
223223

224224
// Prepare request body
@@ -259,7 +259,7 @@ func TestHTTPClient_PATCH_Error(t *testing.T) {
259259

260260
// Create client pointing to test server
261261
logger := setupTestLogger()
262-
client, err := NewHTTPClient(logger, "test-registry", server.URL)
262+
client, err := NewHTTPClient(logger, "test-registry", server.URL, nil)
263263
require.NoError(t, err)
264264

265265
// Prepare request body
@@ -300,7 +300,7 @@ func TestHTTPClient_GET_NonJSONError(t *testing.T) {
300300

301301
// Create http client pointing to test server
302302
logger := setupTestLogger()
303-
client, err := NewHTTPClient(logger, "test-registry", server.URL)
303+
client, err := NewHTTPClient(logger, "test-registry", server.URL, nil)
304304
require.NoError(t, err)
305305

306306
// Make the request

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

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,12 @@ func NewModelRegistryRepository() *ModelRegistryRepository {
1717
}
1818

1919
func (m *ModelRegistryRepository) GetAllModelRegistries(sessionCtx context.Context, client k8s.KubernetesClientInterface, namespace string) ([]models.ModelRegistryModel, error) {
20+
// Default to non-federated mode for backward compatibility
21+
return m.GetAllModelRegistriesWithMode(sessionCtx, client, namespace, false)
22+
}
23+
24+
// GetAllModelRegistriesWithMode fetches all model registries with support for federated mode
25+
func (m *ModelRegistryRepository) GetAllModelRegistriesWithMode(sessionCtx context.Context, client k8s.KubernetesClientInterface, namespace string, isFederatedMode bool) ([]models.ModelRegistryModel, error) {
2026

2127
// TODO: In default mode fetch Routes for external access.
2228
resources, err := client.GetServiceDetails(sessionCtx, namespace)
@@ -27,7 +33,7 @@ func (m *ModelRegistryRepository) GetAllModelRegistries(sessionCtx context.Conte
2733

2834
var registries = []models.ModelRegistryModel{}
2935
for _, s := range resources {
30-
serverAddress := m.ResolveServerAddress(s.ClusterIP, s.HTTPPort, s.IsHTTPS)
36+
serverAddress := m.ResolveServerAddress(s.ClusterIP, s.HTTPPort, s.IsHTTPS, s.ExternalAddressRest, isFederatedMode)
3137
registry := models.ModelRegistryModel{
3238
Name: s.Name,
3339
Description: s.Description,
@@ -42,6 +48,12 @@ func (m *ModelRegistryRepository) GetAllModelRegistries(sessionCtx context.Conte
4248
}
4349

4450
func (m *ModelRegistryRepository) GetModelRegistry(sessionCtx context.Context, client k8s.KubernetesClientInterface, namespace string, modelRegistryID string) (models.ModelRegistryModel, error) {
51+
// Default to non-federated mode for backward compatibility
52+
return m.GetModelRegistryWithMode(sessionCtx, client, namespace, modelRegistryID, false)
53+
}
54+
55+
// GetModelRegistryWithMode fetches a specific model registry with support for federated mode
56+
func (m *ModelRegistryRepository) GetModelRegistryWithMode(sessionCtx context.Context, client k8s.KubernetesClientInterface, namespace string, modelRegistryID string, isFederatedMode bool) (models.ModelRegistryModel, error) {
4557

4658
s, err := client.GetServiceDetailsByName(sessionCtx, namespace, modelRegistryID)
4759
if err != nil {
@@ -52,18 +64,26 @@ func (m *ModelRegistryRepository) GetModelRegistry(sessionCtx context.Context, c
5264
Name: s.Name,
5365
Description: s.Description,
5466
DisplayName: s.DisplayName,
55-
ServerAddress: m.ResolveServerAddress(s.ClusterIP, s.HTTPPort, s.IsHTTPS),
67+
ServerAddress: m.ResolveServerAddress(s.ClusterIP, s.HTTPPort, s.IsHTTPS, s.ExternalAddressRest, isFederatedMode),
5668
IsHTTPS: s.IsHTTPS,
5769
}
5870

5971
return modelRegistry, nil
6072
}
6173

62-
func (m *ModelRegistryRepository) ResolveServerAddress(clusterIP string, httpPort int32, isHTTPS bool) string {
74+
func (m *ModelRegistryRepository) ResolveServerAddress(clusterIP string, httpPort int32, isHTTPS bool, externalAddressRest string, isFederatedMode bool) string {
75+
// Default behavior - use cluster IP and port
6376
protocol := "http"
6477
if isHTTPS {
6578
protocol = "https"
6679
}
80+
// In federated mode, if external address is available, use it
81+
if isFederatedMode && externalAddressRest != "" {
82+
// External address is assumed to be HTTPS
83+
url := fmt.Sprintf("%s://%s/api/model_registry/v1alpha3", protocol, externalAddressRest)
84+
return url
85+
}
86+
6787
url := fmt.Sprintf("%s://%s:%d/api/model_registry/v1alpha3", protocol, clusterIP, httpPort)
6888
return url
6989
}

clients/ui/frontend/config/dotenv.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,7 @@ const setupDotenvFilesForEnv = ({ env }) => {
146146
setupDotenvFile(path.resolve(RELATIVE_DIRNAME, '.env'));
147147

148148
const DEPLOYMENT_MODE = process.env.DEPLOYMENT_MODE || 'kubeflow';
149+
const AUTH_METHOD = process.env.AUTH_METHOD || 'internal';
149150
const IMAGES_DIRNAME = process.env.IMAGES_DIRNAME || 'images';
150151
const PUBLIC_PATH = process.env.PUBLIC_PATH || '/';
151152
const SRC_DIR = path.resolve(RELATIVE_DIRNAME, process.env.SRC_DIR || TS_BASE_URL || 'src');
@@ -174,6 +175,7 @@ const setupDotenvFilesForEnv = ({ env }) => {
174175
process.env._OUTPUT_ONLY = OUTPUT_ONLY;
175176
process.env._DEV_MODE = DEV_MODE;
176177
process.env._DEPLOYMENT_MODE = DEPLOYMENT_MODE;
178+
process.env._AUTH_METHOD = AUTH_METHOD;
177179
};
178180

179181
module.exports = { setupWebpackDotenvFilesForEnv, setupDotenvFilesForEnv };

0 commit comments

Comments
 (0)