Merge branch 'kubeflow:main' into main

patternfly-extension-testing · Dec 4, 2024 · 0e344aa · 0e344aa
2 parents 5efb1c6 + 8f25d29
commit 0e344aa
Show file tree

Hide file tree

Showing 59 changed files with 1,985 additions and 943 deletions.
diff --git a/Dockerfile b/Dockerfile
@@ -1,5 +1,5 @@
 # Build the model-registry binary
-FROM --platform=$BUILDPLATFORM registry.access.redhat.com/ubi8/go-toolset:1.21 AS builder
+FROM --platform=$BUILDPLATFORM registry.access.redhat.com/ubi8/go-toolset:1.22 AS builder
 ARG TARGETOS
 ARG TARGETARCH
 

diff --git a/Dockerfile.odh b/Dockerfile.odh
@@ -1,5 +1,5 @@
 # Build the model-registry binary
-FROM registry.access.redhat.com/ubi8/go-toolset:1.21 AS builder
+FROM registry.access.redhat.com/ubi8/go-toolset:1.22 AS builder
 
 WORKDIR /workspace
 # Copy the Go Modules manifests

diff --git a/Makefile b/Makefile
@@ -6,6 +6,10 @@ GO ?= "$(shell which go)"
 BFF_PATH := $(PROJECT_PATH)/clients/ui/bff
 UI_PATH := $(PROJECT_PATH)/clients/ui/frontend
 
+# ENVTEST_K8S_VERSION refers to the version of kubebuilder assets to be downloaded by envtest binary.
+ENVTEST_K8S_VERSION = 1.29
+ENVTEST ?= $(PROJECT_BIN)/setup-envtest
+
 # add tools bin directory
 PATH := $(PROJECT_BIN):$(PATH)
 
@@ -133,6 +137,9 @@ bin/protoc-gen-go:
 bin/protoc-gen-go-grpc:
 	GOBIN=$(PROJECT_BIN) ${GO} install google.golang.org/grpc/cmd/[email protected]
 
+bin/envtest:
+	GOBIN=$(PROJECT_BIN) ${GO} install sigs.k8s.io/controller-runtime/tools/[email protected]
+
 GOLANGCI_LINT ?= ${PROJECT_BIN}/golangci-lint
 bin/golangci-lint:
 	curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(PROJECT_BIN) v1.61.0
@@ -164,7 +171,7 @@ clean/deps:
 	rm -Rf bin/*
 
 .PHONY: deps
-deps: bin/protoc bin/go-enum bin/protoc-gen-go bin/protoc-gen-go-grpc bin/golangci-lint bin/goverter bin/openapi-generator-cli
+deps: bin/protoc bin/go-enum bin/protoc-gen-go bin/protoc-gen-go-grpc bin/golangci-lint bin/goverter bin/openapi-generator-cli bin/envtest
 
 .PHONY: vendor
 vendor:
@@ -196,20 +203,20 @@ gen: deps gen/grpc gen/openapi gen/openapi-server gen/converter
 
 .PHONY: lint
 lint:
-	${GOLANGCI_LINT} run main.go
-	${GOLANGCI_LINT} run cmd/... internal/... ./pkg/...
+	${GOLANGCI_LINT} run main.go  --timeout 3m
+	${GOLANGCI_LINT} run cmd/... internal/... ./pkg/...  --timeout 3m
 
 .PHONY: test
-test: gen
-	${GO} test ./internal/... ./pkg/...
+test: gen bin/envtest
+	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" ${GO} test ./internal/... ./pkg/...
 
 .PHONY: test-nocache
-test-nocache: gen
-	${GO} test ./internal/... ./pkg/... -count=1
+test-nocache: gen bin/envtest
+	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" ${GO} test ./internal/... ./pkg/... -count=1
 
 .PHONY: test-cover
-test-cover: gen
-	${GO} test ./internal/... ./pkg/... -coverprofile=coverage.txt
+test-cover: gen bin/envtest
+	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" ${GO} test ./internal/... ./pkg/... -coverprofile=coverage.txt
 	${GO} tool cover -html=coverage.txt -o coverage.html
 
 .PHONY: run/proxy
@@ -249,12 +256,12 @@ ifeq ($(DOCKER),docker)
 	# docker uses builder containers
 	- $(DOCKER) buildx rm model-registry-builder
 	$(DOCKER) buildx create --use --name model-registry-builder --platform=$(PLATFORMS)
-	$(DOCKER) buildx build --push --platform=$(PLATFORMS) --tag ${IMG} -f ${DOCKERFILE} .
+	$(DOCKER) buildx build --push --platform=$(PLATFORMS) --tag ${IMG}:$(IMG_VERSION) -f ${DOCKERFILE} .
 	$(DOCKER) buildx rm model-registry-builder
 else ifeq ($(DOCKER),podman)
 	# podman uses image manifests
 	$(DOCKER) manifest create -a ${IMG}
-	$(DOCKER) buildx build --platform=$(PLATFORMS) --manifest ${IMG} -f ${DOCKERFILE} .
+	$(DOCKER) buildx build --platform=$(PLATFORMS) --manifest ${IMG}:$(IMG_VERSION) -f ${DOCKERFILE} .
 	$(DOCKER) manifest push ${IMG}
 	$(DOCKER) manifest rm ${IMG}
 else

diff --git a/clients/python/poetry.lock b/clients/python/poetry.lock
diff --git a/clients/python/pyproject.toml b/clients/python/pyproject.toml
@@ -44,7 +44,7 @@ sphinx-autobuild = ">=2021.3.14,<2025.0.0"
 pytest = ">=7.4.2,<9.0.0"
 coverage = { extras = ["toml"], version = "^7.3.2" }
 pytest-cov = ">=4.1,<7.0"
-ruff = ">=0.5.2,<0.8.0"
+ruff = ">=0.5.2,<0.9.0"
 mypy = "^1.7.0"
 pytest-asyncio = ">=0.23.7,<0.25.0"
 requests = "^2.32.2"

diff --git a/clients/python/src/model_registry/_client.py b/clients/python/src/model_registry/_client.py
@@ -2,6 +2,7 @@
 
 from __future__ import annotations
 
+import logging
 import os
 from collections.abc import Mapping
 from pathlib import Path
@@ -22,6 +23,30 @@
 ModelTypes = Union[RegisteredModel, ModelVersion, ModelArtifact]
 TModel = TypeVar("TModel", bound=ModelTypes)
 
+logging.basicConfig(
+    format="%(asctime)s.%(msecs)03d - %(name)s:%(levelname)s: %(message)s",
+    datefmt="%H:%M:%S",
+    level=logging.WARNING,  # the default loglevel
+    handlers=[
+        # logging.FileHandler(
+        #     LOGS
+        #     / "log-{}-{}.log".format(
+        #         datetime.now(tz=datetime.now().astimezone().tzinfo).strftime(
+        #             "%Y-%m-%d-%H-%M-%S"
+        #         ),
+        #         os.getpid(),
+        #     ),
+        #     encoding="utf-8",
+        #     delay=False,
+        # ),
+        logging.StreamHandler(),
+    ],
+)
+
+logger = logging.getLogger("model-registry")
+
+DEFAULT_USER_TOKEN_ENVVAR = "KF_PIPELINES_SA_TOKEN_PATH"  # noqa: S105
+
 
 class ModelRegistry:
     """Model registry client."""
@@ -34,7 +59,10 @@ def __init__(
         author: str,
         is_secure: bool = True,
         user_token: str | None = None,
+        user_token_envvar: str = DEFAULT_USER_TOKEN_ENVVAR,
         custom_ca: str | None = None,
+        custom_ca_envvar: str | None = None,
+        log_level: int = logging.WARNING,
     ):
         """Constructor.
 
@@ -45,47 +73,59 @@ def __init__(
         Keyword Args:
             author: Name of the author.
             is_secure: Whether to use a secure connection. Defaults to True.
-            user_token: The PEM-encoded user token as a string. Defaults to content of path on envvar KF_PIPELINES_SA_TOKEN_PATH.
-            custom_ca: Path to the PEM-encoded root certificates as a string. Defaults to path on envvar CERT.
+            user_token: The PEM-encoded user token as a string.
+            user_token_envvar: Environment variable to read the user token from if it's not passed as an arg. Defaults to KF_PIPELINES_SA_TOKEN_PATH.
+            custom_ca: Path to the PEM-encoded root certificates as a string.
+            custom_ca_envvar: Environment variable to read the custom CA from if it's not passed as an arg.
+            log_level: Log level. Defaults to logging.WARNING.
         """
+        logger.setLevel(log_level)
+
         import nest_asyncio
 
+        logger.debug("Setting up reentrant async event loop")
         nest_asyncio.apply()
 
         # TODO: get remaining args from env
         self._author = author
 
-        if not user_token:
+        if not user_token and user_token_envvar:
+            logger.info("Reading user token from %s", user_token_envvar)
             # /var/run/secrets/kubernetes.io/serviceaccount/token
-            sa_token = os.environ.get("KF_PIPELINES_SA_TOKEN_PATH")
-            if sa_token:
+            if sa_token := os.environ.get(user_token_envvar):
+                if user_token_envvar == DEFAULT_USER_TOKEN_ENVVAR:
+                    logger.warning(
+                        f"Sourcing user token from default envvar: {DEFAULT_USER_TOKEN_ENVVAR}"
+                    )
                 user_token = Path(sa_token).read_text()
             else:
                 warn("User access token is missing", stacklevel=2)
 
         if is_secure:
-            root_ca = None
-            if not custom_ca:
-                if cert := os.getenv("CERT"):
-                    root_ca = cert
-                    # client might have a default CA setup
-            else:
-                root_ca = custom_ca
+            if (
+                not custom_ca
+                and custom_ca_envvar
+                and (cert := os.getenv(custom_ca_envvar))
+            ):
+                logger.info(
+                    "Using custom CA envvar %s",
+                    custom_ca_envvar,
+                )
+                custom_ca = cert
+                # client might have a default CA setup
 
             if not user_token:
                 msg = "user token must be provided for secure connection"
                 raise StoreError(msg)
 
             self._api = ModelRegistryAPIClient.secure_connection(
-                server_address, port, user_token=user_token, custom_ca=root_ca
+                server_address, port, user_token=user_token, custom_ca=custom_ca
             )
-        elif custom_ca:
-            msg = "Custom CA provided without secure connection, conflicting options"
-            raise StoreError(msg)
         else:
             self._api = ModelRegistryAPIClient.insecure_connection(
                 server_address, port, user_token
             )
+        self.get_registered_models().page_size(1)._next_page()
 
     def async_runner(self, coro: Any) -> Any:
         import asyncio

diff --git a/clients/ui/.env b/clients/ui/.env
@@ -1,5 +1,5 @@
 
 ############### Default settings ###############
 CONTAINER_TOOL=docker
-IMG_BFF=kubeflow/model-registry-bff:dev-latest
-IMG_FRONTEND=kubeflow/model-registry-ui:dev-latest
+IMG_BFF=kubeflow/model-registry-bff:latest
+IMG_FRONTEND=kubeflow/model-registry-ui:latest
diff --git a/clients/ui/OWNERS b/clients/ui/OWNERS
@@ -1,3 +1,5 @@
 approvers:
   - ederign
   - alexcreasy
+  - lucferbux
+  - Griffin-Sullivan
diff --git a/clients/ui/bff/README.md b/clients/ui/bff/README.md
@@ -71,21 +71,23 @@ make docker-build
 | POST /api/v1/model_registry/{model_registry_id}/model_versions/{model_version_id}/artifacts  | CreateModelArtifactByModelVersion            | Create a ModelArtifact entity for a specific ModelVersion   |
 
 ### Sample local calls
+
+You will need to inject your requests with a kubeflow-userid header for authorization purposes. When running the service with the mocked Kubernetes client (MOCK_K8S_CLIENT=true), the user [email protected] is preconfigured with the necessary RBAC permissions to perform these actions.
 ```
 # GET /v1/healthcheck
-curl -i localhost:4000/api/v1/healthcheck
+curl -i -H "kubeflow-userid: [email protected]" localhost:4000/api/v1/healthcheck
 ```
 ```
 # GET /v1/model_registry 
-curl -i localhost:4000/api/v1/model_registry
+curl -i -H "kubeflow-userid: [email protected]" localhost:4000/api/v1/model_registry
 ```
 ```
 # GET /v1/model_registry/{model_registry_id}/registered_models
-curl -i localhost:4000/api/v1/model_registry/model-registry/registered_models
+curl -i -H "kubeflow-userid: [email protected]" localhost:4000/api/v1/model_registry/model-registry/registered_models
 ```
 ```
 #POST /v1/model_registry/{model_registry_id}/registered_models
-curl -i -X POST "http://localhost:4000/api/v1/model_registry/model-registry/registered_models" \
+curl -i -H "kubeflow-userid: [email protected]" -X POST "http://localhost:4000/api/v1/model_registry/model-registry/registered_models" \
      -H "Content-Type: application/json" \
      -d '{ "data": {
   "customProperties": {
@@ -103,23 +105,23 @@ curl -i -X POST "http://localhost:4000/api/v1/model_registry/model-registry/regi
 ```
 ```
 # GET /v1/model_registry/{model_registry_id}/registered_models/{registered_model_id}
-curl -i localhost:4000/api/v1/model_registry/model-registry/registered_models/1
+curl -i -H "kubeflow-userid: [email protected]" localhost:4000/api/v1/model_registry/model-registry/registered_models/1
 ```
 ```
 # PATCH /v1/model_registry/{model_registry_id}/registered_models/{registered_model_id}
-curl -i -X PATCH "http://localhost:4000/api/v1/model_registry/model-registry/registered_models/1" \
+curl -i -H "kubeflow-userid: [email protected]" -X PATCH "http://localhost:4000/api/v1/model_registry/model-registry/registered_models/1" \
 -H "Content-Type: application/json" \
 -d '{ "data": {
   "description": "New description"
 }}'
 ```
 ```
 # GET /api/v1/model_registry/{model_registry_id}/model_versions/{model_version_id} 
-curl -i http://localhost:4000/api/v1/model_registry/model-registry/model_versions/1
+curl -i -H "kubeflow-userid: [email protected]" http://localhost:4000/api/v1/model_registry/model-registry/model_versions/1
 ```
 ```
 # POST /api/v1/model_registry/{model_registry_id}/model_versions
-curl -i -X POST "http://localhost:4000/api/v1/model_registry/model-registry/model_versions" \
+curl -i -H "kubeflow-userid: [email protected]" -X POST "http://localhost:4000/api/v1/model_registry/model-registry/model_versions" \
      -H "Content-Type: application/json" \
      -d '{ "data": {
   "customProperties": {
@@ -138,19 +140,19 @@ curl -i -X POST "http://localhost:4000/api/v1/model_registry/model-registry/mode
 ```
 ```
 # PATCH /api/v1/model_registry/{model_registry_id}/model_versions/{model_version_id}
-curl -i -X PATCH "http://localhost:4000/api/v1/model_registry/model-registry/model_versions/1" \
+curl -i -H "kubeflow-userid: [email protected]" -X PATCH "http://localhost:4000/api/v1/model_registry/model-registry/model_versions/1" \
      -H "Content-Type: application/json" \
 -d '{ "data": {
   "description": "New description 2"
 }}'
 ```
 ```
 # GET /v1/model_registry/{model_registry_id}/registered_models/{registered_model_id}/versions
-curl -i localhost:4000/api/v1/model_registry/model-registry/registered_models/1/versions
+curl -i -H "kubeflow-userid: [email protected]" localhost:4000/api/v1/model_registry/model-registry/registered_models/1/versions
 ```
 ```
 # POST /v1/model_registry/{model_registry_id}/registered_models/{registered_model_id}/versions
-curl -i -X POST "http://localhost:4000/api/v1/model_registry/model-registry/registered_models/1/versions" \
+curl -i -H "kubeflow-userid: [email protected]" -X POST "http://localhost:4000/api/v1/model_registry/model-registry/registered_models/1/versions" \
      -H "Content-Type: application/json" \
      -d '{ "data": {
   "customProperties": {
@@ -163,17 +165,17 @@ curl -i -X POST "http://localhost:4000/api/v1/model_registry/model-registry/regi
   "externalId": "9928",
   "name": "ModelVersion One",
   "state": "LIVE",
-  "author": "alex"
+  "author": "alex",
   "registeredModelId: "1"
 }}'
 ```
 ```
 # GET /api/v1/model_registry/{model_registry_id}/model_versions/{model_version_id}/artifacts
-curl -i http://localhost:4000/api/v1/model_registry/model-registry/model_versions/1/artifacts
+curl -i -H "kubeflow-userid: [email protected]" http://localhost:4000/api/v1/model_registry/model-registry/model_versions/1/artifacts
 ```
 ```
 # POST /api/v1/model_registry/{model_registry_id}/model_versions/{model_version_id}/artifacts
-curl -i -X POST "http://localhost:4000/api/v1/model_registry/model-registry/model_versions/1/artifacts" \
+curl -i -H "kubeflow-userid: [email protected]" -X POST "http://localhost:4000/api/v1/model_registry/model-registry/model_versions/1/artifacts" \
      -H "Content-Type: application/json" \
      -d '{ "data": {
   "customProperties": {
@@ -203,9 +205,9 @@ The following query parameters are supported by "Get All" style endpoints to con
 ### Sample local calls
 ```
 # Get with a page size of 5 getting a specific page.
-curl -i "http://localhost:4000/api/v1/model_registry/model-registry/registered_models?pageSize=5&nextPageToken=CAEQARoCCAE"
+curl -i -H "kubeflow-userid: [email protected]" "http://localhost:4000/api/v1/model_registry/model-registry/registered_models?pageSize=5&nextPageToken=CAEQARoCCAE"
 ```
 ```
 # Get with a page size of 5, order by last update time in descending order.
-curl -i "http://localhost:4000/api/v1/model_registry/model-registry/registered_models?pageSize=5&orderBy=LAST_UPDATE_TIME&sortOrder=DESC"
+curl -i -H "kubeflow-userid: [email protected]" "http://localhost:4000/api/v1/model_registry/model-registry/registered_models?pageSize=5&orderBy=LAST_UPDATE_TIME&sortOrder=DESC"
 ```
diff --git a/clients/ui/bff/go.mod b/clients/ui/bff/go.mod
@@ -10,7 +10,7 @@ require (
 	github.com/onsi/gomega v1.35.1
 	github.com/stretchr/testify v1.9.0
 	k8s.io/api v0.31.2
-	k8s.io/apimachinery v0.31.2
+	k8s.io/apimachinery v0.31.3
 	k8s.io/client-go v0.31.2
 	sigs.k8s.io/controller-runtime v0.19.1
 )

diff --git a/clients/ui/bff/go.sum b/clients/ui/bff/go.sum
@@ -182,8 +182,8 @@ k8s.io/api v0.31.2 h1:3wLBbL5Uom/8Zy98GRPXpJ254nEFpl+hwndmk9RwmL0=
 k8s.io/api v0.31.2/go.mod h1:bWmGvrGPssSK1ljmLzd3pwCQ9MgoTsRCuK35u6SygUk=
 k8s.io/apiextensions-apiserver v0.31.0 h1:fZgCVhGwsclj3qCw1buVXCV6khjRzKC5eCFt24kyLSk=
 k8s.io/apiextensions-apiserver v0.31.0/go.mod h1:b9aMDEYaEe5sdK+1T0KU78ApR/5ZVp4i56VacZYEHxk=
-k8s.io/apimachinery v0.31.2 h1:i4vUt2hPK56W6mlT7Ry+AO8eEsyxMD1U44NR22CLTYw=
-k8s.io/apimachinery v0.31.2/go.mod h1:rsPdaZJfTfLsNJSQzNHQvYoTmxhoOEofxtOsF3rtsMo=
+k8s.io/apimachinery v0.31.3 h1:6l0WhcYgasZ/wk9ktLq5vLaoXJJr5ts6lkaQzgeYPq4=
+k8s.io/apimachinery v0.31.3/go.mod h1:rsPdaZJfTfLsNJSQzNHQvYoTmxhoOEofxtOsF3rtsMo=
 k8s.io/client-go v0.31.2 h1:Y2F4dxU5d3AQj+ybwSMqQnpZH9F30//1ObxOKlTI9yc=
 k8s.io/client-go v0.31.2/go.mod h1:NPa74jSVR/+eez2dFsEIHNa+3o09vtNaWwWwb1qSxSs=
 k8s.io/klog/v2 v2.130.1 h1:n9Xl7H1Xvksem4KFG4PYbdQCQxqc/tTUyrgXaOhHSzk=

diff --git a/clients/ui/bff/internal/api/app.go b/clients/ui/bff/internal/api/app.go
@@ -106,5 +106,5 @@ func (app *App) Routes() http.Handler {
 	router.GET(ModelRegistryListPath, app.ModelRegistryHandler)
 	router.PATCH(ModelRegistryPath, app.AttachRESTClient(app.UpdateModelVersionHandler))
 
-	return app.RecoverPanic(app.enableCORS(router))
+	return app.RecoverPanic(app.enableCORS(app.RequireAccessControl(router)))
 }
diff --git a/clients/ui/bff/internal/api/errors.go b/clients/ui/bff/internal/api/errors.go
@@ -43,6 +43,17 @@ func (app *App) badRequestResponse(w http.ResponseWriter, r *http.Request, err e
 	app.errorResponse(w, r, httpError)
 }
 
+func (app *App) forbiddenResponse(w http.ResponseWriter, r *http.Request, message string) {
+	httpError := &integrations.HTTPError{
+		StatusCode: http.StatusForbidden,
+		ErrorResponse: integrations.ErrorResponse{
+			Code:    strconv.Itoa(http.StatusForbidden),
+			Message: message,
+		},
+	}
+	app.errorResponse(w, r, httpError)
+}
+
 func (app *App) errorResponse(w http.ResponseWriter, r *http.Request, error *integrations.HTTPError) {
 
 	env := ErrorEnvelope{Error: error}