Skip to content

Commit 820a843

Browse files
fix(multi-tenancy): address PR feedback and CI auth test failures
Fix CI auth workflow failures and address review feedback: - Add vector_store::* routing table ABAC rules alongside existing sql_record::vector_stores::* rules to cover both authorization layers - Remove stale agent_state and metadata_store configs from the CI auth workflow to prevent SQLAlchemy lazy-init race with the responses table - Add embedding_model parameter to vector store access control tests - Use per-table migration guards in kvstore-to-sql migration so a crash mid-migration can resume from the last incomplete table on next boot Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Francisco Javier Arceo <farceo@redhat.com>
1 parent 745ed7a commit 820a843

3 files changed

Lines changed: 106 additions & 83 deletions

File tree

.github/workflows/integration-responses-conversations-auth-tests.yml

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -121,16 +121,17 @@ jobs:
121121
provider_type: remote::openai
122122
config:
123123
api_key: \${env.OPENAI_API_KEY}
124+
- provider_id: sentence-transformers
125+
provider_type: inline::sentence-transformers
126+
config:
127+
trust_remote_code: true
124128
vector_io:
125129
- provider_id: faiss
126130
provider_type: inline::faiss
127131
config:
128132
persistence:
129133
namespace: vector_io::faiss
130134
backend: kv_default
131-
metadata_store:
132-
table_name: vector_store_metadata
133-
backend: sql_default
134135
tool_runtime:
135136
- provider_id: file-search
136137
provider_type: inline::file-search
@@ -140,9 +141,6 @@ jobs:
140141
provider_type: inline::builtin
141142
config:
142143
persistence:
143-
agent_state:
144-
namespace: responses
145-
backend: kv_default
146144
responses:
147145
table_name: responses
148146
backend: sql_default
@@ -169,13 +167,13 @@ jobs:
169167
prompts:
170168
table_name: prompts
171169
backend: sql_default
172-
vector_stores:
173-
table_name: vector_store_metadata
174-
backend: sql_default
175170
models:
176171
- model_id: openai/gpt-4o
177172
model_type: llm
178173
provider_id: openai
174+
- model_id: sentence-transformers/nomic-ai/nomic-embed-text-v1.5
175+
model_type: embedding
176+
provider_id: sentence-transformers
179177
server:
180178
port: 8321
181179
auth:
@@ -184,6 +182,16 @@ jobs:
184182
actions: [read]
185183
resource: model::*
186184
description: Any authenticated user can use configured models (inference, responses)
185+
- permit:
186+
actions: [create]
187+
resource: vector_store::*
188+
description: Any authenticated user can create vector stores (routing table)
189+
- permit:
190+
actions: [read, update, delete]
191+
resource: vector_store::*
192+
when:
193+
- user is owner
194+
description: Users can access their own vector stores (routing table)
187195
- permit:
188196
actions: [create]
189197
resource: sql_record::openai_files::*

src/ogx/providers/utils/memory/openai_vector_store_mixin.py

Lines changed: 81 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -225,105 +225,115 @@ async def _migrate_kvstore_to_sql(self) -> None:
225225

226226
sql_store = self.metadata_store.sql_store
227227

228-
existing = await sql_store.fetch_all(table=TABLE_VECTOR_STORES, limit=1)
229-
if existing.data:
230-
return
231-
232228
stores_data = await self.kvstore.values_in_range(
233229
OPENAI_VECTOR_STORES_PREFIX, f"{OPENAI_VECTOR_STORES_PREFIX}\xff"
234230
)
235231
if not stores_data:
236232
return
237233

238-
logger.info("Starting KVStore to SQL migration for vector store metadata", store_count=len(stores_data))
239-
240234
migrated_stores = 0
241235
migrated_files = 0
242236
migrated_chunks = 0
243237
migrated_batches = 0
244238

245-
for raw in stores_data:
246-
info = json.loads(raw)
247-
store_id = info["id"]
248-
await sql_store.insert(
249-
table=TABLE_VECTOR_STORES,
250-
data={
251-
"id": store_id,
252-
"store_data": info,
253-
"owner_principal": "",
254-
"access_attributes": None,
255-
},
256-
)
257-
migrated_stores += 1
258-
259-
file_keys = await self.kvstore.keys_in_range(
260-
f"{OPENAI_VECTOR_STORES_FILES_PREFIX}{store_id}:",
261-
f"{OPENAI_VECTOR_STORES_FILES_PREFIX}{store_id}:\xff",
239+
# Per-table migration: each table is checked independently so a crash
240+
# mid-migration doesn't skip remaining tables on the next boot.
241+
existing_stores = await sql_store.fetch_all(table=TABLE_VECTOR_STORES, limit=1)
242+
if not existing_stores.data:
243+
logger.info(
244+
"Starting KVStore to SQL migration for vector store metadata",
245+
store_count=len(stores_data),
262246
)
263-
for file_key in file_keys:
264-
suffix = file_key[len(OPENAI_VECTOR_STORES_FILES_PREFIX) :]
265-
file_id = suffix.split(":", 1)[1] if ":" in suffix else suffix
266-
raw_file = await self.kvstore.get(file_key)
267-
if not raw_file:
268-
continue
269-
file_info = json.loads(raw_file)
247+
for raw in stores_data:
248+
info = json.loads(raw)
249+
store_id = info["id"]
270250
await sql_store.insert(
271-
table=TABLE_VECTOR_STORE_FILES,
251+
table=TABLE_VECTOR_STORES,
272252
data={
273-
"id": f"{store_id}:{file_id}",
274-
"store_id": store_id,
275-
"file_id": file_id,
276-
"file_data": file_info,
253+
"id": store_id,
254+
"store_data": info,
277255
"owner_principal": "",
278256
"access_attributes": None,
279257
},
280258
)
281-
migrated_files += 1
282-
283-
chunk_prefix = f"{OPENAI_VECTOR_STORES_FILES_CONTENTS_PREFIX}{store_id}:{file_id}:"
284-
chunk_values = await self.kvstore.values_in_range(chunk_prefix, f"{chunk_prefix}\xff")
285-
for idx, raw_chunk in enumerate(chunk_values):
286-
chunk = json.loads(raw_chunk)
259+
migrated_stores += 1
260+
261+
existing_files = await sql_store.fetch_all(table=TABLE_VECTOR_STORE_FILES, limit=1)
262+
if not existing_files.data:
263+
for raw in stores_data:
264+
info = json.loads(raw)
265+
store_id = info["id"]
266+
file_keys = await self.kvstore.keys_in_range(
267+
f"{OPENAI_VECTOR_STORES_FILES_PREFIX}{store_id}:",
268+
f"{OPENAI_VECTOR_STORES_FILES_PREFIX}{store_id}:\xff",
269+
)
270+
for file_key in file_keys:
271+
suffix = file_key[len(OPENAI_VECTOR_STORES_FILES_PREFIX) :]
272+
file_id = suffix.split(":", 1)[1] if ":" in suffix else suffix
273+
raw_file = await self.kvstore.get(file_key)
274+
if not raw_file:
275+
continue
276+
file_info = json.loads(raw_file)
287277
await sql_store.insert(
288-
table=TABLE_VECTOR_STORE_FILE_CONTENTS,
278+
table=TABLE_VECTOR_STORE_FILES,
289279
data={
290-
"id": f"{store_id}:{file_id}:{idx}",
280+
"id": f"{store_id}:{file_id}",
291281
"store_id": store_id,
292282
"file_id": file_id,
293-
"chunk_index": idx,
294-
"chunk_data": chunk,
283+
"file_data": file_info,
295284
"owner_principal": "",
296285
"access_attributes": None,
297286
},
298287
)
299-
migrated_chunks += 1
288+
migrated_files += 1
289+
290+
chunk_prefix = f"{OPENAI_VECTOR_STORES_FILES_CONTENTS_PREFIX}{store_id}:{file_id}:"
291+
chunk_values = await self.kvstore.values_in_range(chunk_prefix, f"{chunk_prefix}\xff")
292+
for idx, raw_chunk in enumerate(chunk_values):
293+
chunk = json.loads(raw_chunk)
294+
await sql_store.insert(
295+
table=TABLE_VECTOR_STORE_FILE_CONTENTS,
296+
data={
297+
"id": f"{store_id}:{file_id}:{idx}",
298+
"store_id": store_id,
299+
"file_id": file_id,
300+
"chunk_index": idx,
301+
"chunk_data": chunk,
302+
"owner_principal": "",
303+
"access_attributes": None,
304+
},
305+
)
306+
migrated_chunks += 1
300307

301-
batch_data = await self.kvstore.values_in_range(
302-
OPENAI_VECTOR_STORES_FILE_BATCHES_PREFIX, f"{OPENAI_VECTOR_STORES_FILE_BATCHES_PREFIX}\xff"
303-
)
304-
for raw_batch in batch_data:
305-
batch_info = json.loads(raw_batch)
306-
batch_id = batch_info["id"]
307-
await sql_store.insert(
308-
table=TABLE_VECTOR_STORE_FILE_BATCHES,
309-
data={
310-
"id": batch_id,
311-
"store_id": batch_info.get("vector_store_id", ""),
312-
"batch_data": batch_info,
313-
"expires_at": batch_info.get("expires_at", 0),
314-
"owner_principal": "",
315-
"access_attributes": None,
316-
},
308+
existing_batches = await sql_store.fetch_all(table=TABLE_VECTOR_STORE_FILE_BATCHES, limit=1)
309+
if not existing_batches.data:
310+
batch_data = await self.kvstore.values_in_range(
311+
OPENAI_VECTOR_STORES_FILE_BATCHES_PREFIX, f"{OPENAI_VECTOR_STORES_FILE_BATCHES_PREFIX}\xff"
312+
)
313+
for raw_batch in batch_data:
314+
batch_info = json.loads(raw_batch)
315+
batch_id = batch_info["id"]
316+
await sql_store.insert(
317+
table=TABLE_VECTOR_STORE_FILE_BATCHES,
318+
data={
319+
"id": batch_id,
320+
"store_id": batch_info.get("vector_store_id", ""),
321+
"batch_data": batch_info,
322+
"expires_at": batch_info.get("expires_at", 0),
323+
"owner_principal": "",
324+
"access_attributes": None,
325+
},
326+
)
327+
migrated_batches += 1
328+
329+
if migrated_stores or migrated_files or migrated_chunks or migrated_batches:
330+
logger.info(
331+
"KVStore to SQL migration complete",
332+
stores=migrated_stores,
333+
files=migrated_files,
334+
chunks=migrated_chunks,
335+
batches=migrated_batches,
317336
)
318-
migrated_batches += 1
319-
320-
logger.info(
321-
"KVStore to SQL migration complete",
322-
stores=migrated_stores,
323-
files=migrated_files,
324-
chunks=migrated_chunks,
325-
batches=migrated_batches,
326-
)
327337

328338
async def _save_openai_vector_store(self, store_id: str, store_info: dict[str, Any]) -> None:
329339
"""Save vector store metadata to persistent storage."""

tests/integration/vector_io/test_vector_stores_access_control.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,14 @@ def _headers(self) -> dict[str, str]:
5252
"Content-Type": "application/json",
5353
}
5454

55-
def create(self, name: str) -> dict:
55+
def create(self, name: str, embedding_model: str | None = None) -> dict:
56+
body: dict = {"name": name}
57+
if embedding_model:
58+
body["embedding_model"] = embedding_model
5659
resp = httpx.post(
5760
f"{self.base_url}/v1/vector_stores",
5861
headers=self._headers(),
59-
json={"name": name},
62+
json=body,
6063
timeout=30.0,
6164
)
6265
resp.raise_for_status()
@@ -113,8 +116,10 @@ def bob(self, ogx_client) -> VectorStoresClient:
113116
token = get_auth_token("BOB_TOKEN", "token-bob")
114117
return VectorStoresClient(str(ogx_client.base_url), token)
115118

119+
EMBEDDING_MODEL = "sentence-transformers/nomic-ai/nomic-embed-text-v1.5"
120+
116121
def _create_store(self, client: VectorStoresClient, name: str = "test-store") -> str:
117-
data = client.create(name)
122+
data = client.create(name, embedding_model=self.EMBEDDING_MODEL)
118123
return data["id"]
119124

120125
def test_user_cannot_retrieve_other_users_vector_store(self, alice, bob, require_server):

0 commit comments

Comments
 (0)