Skip to content

Commit 328431f

Browse files
ntkatholentkathole
committed
fix: Remove registry proto dump to enforce RBAC and add permission checks to Commit/Refresh RPCs
Signed-off-by: ntkathole <nikhilkathole2683@gmail.com>
1 parent 6ae80af commit 328431f

10 files changed

Lines changed: 113 additions & 130 deletions

File tree

‎infra/scripts/feature_server_docker_smoke.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ class _FakeRegistry:
99
def proto(self):
1010
return object()
1111

12+
def list_projects(self, allow_cache=True, tags=None):
13+
return []
14+
1215

1316
class _FakeStore:
1417
def __init__(self):

‎sdk/python/feast/api/registry/rest/metrics.py‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,19 @@ async def resource_counts(
7575
"""
7676

7777
def get_registry_last_updated() -> Optional[str]:
78+
"""Read registry last_updated from in-process registry (not the Proto gRPC RPC)."""
7879
try:
79-
from google.protobuf.empty_pb2 import Empty as EmptyProto
80-
81-
registry_proto = grpc_call(grpc_handler.Proto, EmptyProto())
82-
return registry_proto.get("lastUpdated", None)
80+
from google.protobuf.json_format import MessageToDict
81+
82+
registry = getattr(grpc_handler, "proxied_registry", None)
83+
if registry is None:
84+
return None
85+
registry_proto = registry.proto()
86+
if registry_proto is None or not registry_proto.HasField(
87+
"last_updated"
88+
):
89+
return None
90+
return MessageToDict(registry_proto).get("lastUpdated")
8391
except Exception:
8492
return None
8593

‎sdk/python/feast/cli/ui.py‎

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import click
22

3-
from feast.repo_operations import create_feature_store, registry_dump
3+
from feast.repo_operations import create_feature_store
44

55

66
@click.command()
@@ -20,14 +20,6 @@
2020
show_default=True,
2121
help="Specify a port for the server",
2222
)
23-
@click.option(
24-
"--registry_ttl_sec",
25-
"-r",
26-
help="Number of seconds after which the registry is refreshed",
27-
type=click.INT,
28-
default=5,
29-
show_default=True,
30-
)
3123
@click.option(
3224
"--root_path",
3325
help="Provide root path to make the UI working behind proxy",
@@ -57,7 +49,6 @@ def ui(
5749
ctx: click.Context,
5850
host: str,
5951
port: int,
60-
registry_ttl_sec: int,
6152
root_path: str = "",
6253
tls_key_path: str = "",
6354
tls_cert_path: str = "",
@@ -73,8 +64,6 @@ def ui(
7364
store.serve_ui(
7465
host=host,
7566
port=port,
76-
get_registry_dump=registry_dump,
77-
registry_ttl_sec=registry_ttl_sec,
7867
root_path=root_path,
7968
tls_key_path=tls_key_path,
8069
tls_cert_path=tls_cert_path,

‎sdk/python/feast/feature_server.py‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,6 @@ def get_app(
288288
"""
289289
proto_json.patch()
290290
# Asynchronously refresh registry, notifying shutdown and canceling the active timer if the app is shutting down
291-
registry_proto = None
292291
shutting_down = False
293292
active_timer: Optional[threading.Timer] = None
294293
# --- Offline write batching config and batcher ---
@@ -338,8 +337,6 @@ def async_refresh():
338337
return
339338

340339
store.refresh_registry()
341-
nonlocal registry_proto
342-
registry_proto = store.registry.proto()
343340

344341
if registry_ttl_sec:
345342
nonlocal active_timer
@@ -569,11 +566,11 @@ async def write_to_online_store(request: WriteToFeatureStoreRequest) -> None:
569566

570567
@app.get("/health")
571568
async def health():
572-
return (
573-
Response(status_code=status.HTTP_200_OK)
574-
if registry_proto
575-
else Response(status_code=status.HTTP_503_SERVICE_UNAVAILABLE)
576-
)
569+
try:
570+
store.registry.list_projects(allow_cache=True)
571+
return Response(status_code=status.HTTP_200_OK)
572+
except Exception:
573+
return Response(status_code=status.HTTP_503_SERVICE_UNAVAILABLE)
577574

578575
@app.post("/chat")
579576
async def chat(request: ChatRequest):

‎sdk/python/feast/feature_store.py‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
from typing import (
2323
TYPE_CHECKING,
2424
Any,
25-
Callable,
2625
Dict,
2726
Iterable,
2827
List,
@@ -3148,8 +3147,6 @@ def serve_ui(
31483147
self,
31493148
host: str,
31503149
port: int,
3151-
get_registry_dump: Callable,
3152-
registry_ttl_sec: int,
31533150
root_path: str = "",
31543151
tls_key_path: str = "",
31553152
tls_cert_path: str = "",
@@ -3165,9 +3162,7 @@ def serve_ui(
31653162
self,
31663163
host=host,
31673164
port=port,
3168-
get_registry_dump=get_registry_dump,
31693165
project_id=self.config.project,
3170-
registry_ttl_sec=registry_ttl_sec,
31713166
root_path=root_path,
31723167
tls_key_path=tls_key_path,
31733168
tls_cert_path=tls_cert_path,

‎sdk/python/feast/infra/feature_servers/multicloud/Dockerfile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
FROM registry.access.redhat.com/ubi9/python-312-minimal:1
1+
FROM registry.access.redhat.com/ubi9/python-312-minimal:latest
22

33
USER 0
44
RUN microdnf install -y git gcc libpq-devel python3.12-devel && microdnf clean all

‎sdk/python/feast/infra/feature_servers/multicloud/Dockerfile.dev‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
FROM registry.access.redhat.com/ubi9/python-312-minimal:1
1+
FROM registry.access.redhat.com/ubi9/python-312-minimal:latest
22

33
USER 0
44
RUN microdnf install -y npm git gcc libpq-devel python3.12-devel && microdnf clean all

‎sdk/python/feast/registry_server.py‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1117,16 +1117,19 @@ def GetObjectRelationships(
11171117
)
11181118

11191119
def Commit(self, request, context):
1120+
for project in self.proxied_registry.list_projects(allow_cache=True):
1121+
assert_permissions(resource=project, actions=[AuthzedAction.UPDATE])
11201122
self.proxied_registry.commit()
11211123
return Empty()
11221124

11231125
def Refresh(self, request, context):
1126+
project = self.proxied_registry.get_project(
1127+
name=request.project, allow_cache=True
1128+
)
1129+
assert_permissions(resource=project, actions=[AuthzedAction.UPDATE])
11241130
self.proxied_registry.refresh(request.project)
11251131
return Empty()
11261132

1127-
def Proto(self, request, context):
1128-
return self.proxied_registry.proto()
1129-
11301133
def ListFeatures(self, request: RegistryServer_pb2.ListFeaturesRequest, context):
11311134
"""
11321135
List all features in the registry, optionally filtered by project, feature_view, or name.

‎sdk/python/feast/ui_server.py‎

Lines changed: 22 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
import json
22
import logging
3-
import threading
43
from importlib import resources as importlib_resources
5-
from typing import Callable, Optional
64

75
import uvicorn
86
from fastapi import FastAPI, Response, status
@@ -21,23 +19,23 @@ def _build_projects_list(
2119
):
2220
"""Build the projects list for the UI."""
2321
discovered_projects = []
24-
registry = store.registry.proto()
25-
2622
registry_path_template = f"{root_path}/api/v1"
2723

28-
if registry and registry.projects and len(registry.projects) > 0:
29-
for proj in registry.projects:
30-
if proj.spec and proj.spec.name:
31-
discovered_projects.append(
32-
{
33-
"name": proj.spec.name.replace("_", " ").title(),
34-
"description": proj.spec.description
35-
or f"Project: {proj.spec.name}",
36-
"id": proj.spec.name,
37-
"registryPath": registry_path_template,
38-
}
39-
)
40-
else:
24+
try:
25+
projects = store.registry.list_projects(allow_cache=True)
26+
for proj in projects:
27+
discovered_projects.append(
28+
{
29+
"name": proj.name.replace("_", " ").title(),
30+
"description": proj.description or f"Project: {proj.name}",
31+
"id": proj.name,
32+
"registryPath": registry_path_template,
33+
}
34+
)
35+
except Exception:
36+
pass
37+
38+
if not discovered_projects:
4139
discovered_projects.append(
4240
{
4341
"name": "Project",
@@ -59,34 +57,11 @@ def _build_projects_list(
5957
return {"projects": discovered_projects}
6058

6159

62-
def _setup_rest_mode(app: FastAPI, store: "feast.FeatureStore", registry_ttl_secs: int):
60+
def _setup_rest_mode(app: FastAPI, store: "feast.FeatureStore"):
6361
"""Mount the REST registry API routes on the UI server under /api/v1."""
6462
from feast.api.registry.rest import register_all_routes
6563
from feast.registry_server import RegistryServer
6664

67-
registry_proto = None
68-
shutting_down = False
69-
active_timer: Optional[threading.Timer] = None
70-
71-
def async_refresh():
72-
store.refresh_registry()
73-
nonlocal registry_proto
74-
registry_proto = store.registry.proto()
75-
if shutting_down:
76-
return
77-
nonlocal active_timer
78-
active_timer = threading.Timer(registry_ttl_secs, async_refresh)
79-
active_timer.start()
80-
81-
@app.on_event("shutdown")
82-
def shutdown_event():
83-
nonlocal shutting_down
84-
shutting_down = True
85-
if active_timer:
86-
active_timer.cancel()
87-
88-
async_refresh()
89-
9065
grpc_handler = RegistryServer(store.registry)
9166

9267
rest_app = FastAPI(root_path="/api/v1")
@@ -95,19 +70,18 @@ def shutdown_event():
9570

9671
@app.get("/health")
9772
def health():
98-
return (
99-
Response(status_code=status.HTTP_200_OK)
100-
if registry_proto
101-
else Response(status_code=status.HTTP_503_SERVICE_UNAVAILABLE)
102-
)
73+
try:
74+
store.registry.list_projects(allow_cache=True)
75+
return Response(status_code=status.HTTP_200_OK)
76+
except Exception:
77+
return Response(status_code=status.HTTP_503_SERVICE_UNAVAILABLE)
10378

10479
logger.info("REST registry API mounted at /api/v1")
10580

10681

10782
def get_app(
10883
store: "feast.FeatureStore",
10984
project_id: str,
110-
registry_ttl_secs: int,
11185
root_path: str = "",
11286
):
11387
app = FastAPI()
@@ -120,7 +94,7 @@ def get_app(
12094
allow_headers=["*"],
12195
)
12296

123-
_setup_rest_mode(app, store, registry_ttl_secs)
97+
_setup_rest_mode(app, store)
12498

12599
ui_dir_ref = importlib_resources.files(__spec__.parent) / "ui/build/" # type: ignore[name-defined, arg-type]
126100
with importlib_resources.as_file(ui_dir_ref) as ui_dir:
@@ -148,17 +122,14 @@ def start_server(
148122
store: "feast.FeatureStore",
149123
host: str,
150124
port: int,
151-
get_registry_dump: Callable,
152125
project_id: str,
153-
registry_ttl_sec: int,
154126
root_path: str = "",
155127
tls_key_path: str = "",
156128
tls_cert_path: str = "",
157129
):
158130
app = get_app(
159131
store,
160132
project_id,
161-
registry_ttl_sec,
162133
root_path,
163134
)
164135

0 commit comments

Comments
 (0)