Skip to content

Conversation

@silvi-t
Copy link
Contributor

@silvi-t silvi-t commented Nov 3, 2025

Description

  • Refactored SpiceDB integration to support multiple dynamically deployed instances instead of relying on a single pre-configured external instance

Changes

Refactoring

  • testsuite/spicedb/spicedb.py: Complete refactor of SpiceDB integration
    • Replaced SpiceDBService with SpiceDB (Backend) and SpiceDBClient classes
    • Implemented custom HTTP client using httpx for SpiceDB HTTP API (/v1/schema/write, /v1/relationships/write, /v1/permissions/check)
    • Added Kubernetes deployment and service management as a Backend
    • Implemented lazy-initialized client with proper type annotations
    • Routes test suite traffic through Gateway via HTTPRoute instead of LoadBalancer
    • Maintains gRPC connectivity for Authorino using internal cluster DNS

Configuration

  • config/settings.yaml: Added spicedb.image configuration with default image quay.io/authzed/spicedb:latest
  • config/settings.local.yaml.tpl: Updated template to use image instead of url and password
  • testsuite/config/__init__.py: Removed validators for spicedb.url and spicedb.password as they are no longer needed

Verification steps

# Test with Kuadrant
poetry run pytest -vv testsuite/tests/singlecluster/authorino/authorization/spicedb/test_spicedb.py

# Test with Authorino standalone
poetry run pytest -vv --standalone testsuite/tests/singlecluster/authorino/authorization/spicedb/test_spicedb.py

Follow-up
After merging, step 2 from #770 needs to be completed: Remove the SpiceDB instance from tools.

@silvi-t silvi-t requested review from averevki and crstrn13 November 3, 2025 12:53
@silvi-t silvi-t self-assigned this Nov 3, 2025
Copy link
Contributor

@azgabur azgabur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the purpose of this PR is just to allow parallel runs of the same test I think only modification of fixtures schema_config and relationship_config was needed or maybe some other places. Currently (on main) the configs uses static names for all fields which I presume would override each other. Not sure how the spiceDB handles writes of the same schemas. Did you try making the names of the schemas unique between runs?

ServicePort(name="grpc", port=50051, targetPort="grpc"),
],
labels={"app": self.label},
service_type="LoadBalancer",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the service used outside the cluster like multicluster test? If not, we maybe don't need to use LoadBalancer type here. This could also apply to some services in tools. Needs investigation.
Especially if you want to choose hard-coded secret.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I understand, we used loadbalancer services for tools to replicate customers' behavior. So it makes sense for me to have spicedb loadbalanced as it is an authorization service supposed to be used together with kuadrant, but it doesn't make sense for me to have mockserver loadbalanced for example.

@silvi-t
Copy link
Contributor Author

silvi-t commented Nov 4, 2025

If the purpose of this PR is just to allow parallel runs of the same test I think only modification of fixtures schema_config and relationship_config was needed or maybe some other places. Currently (on main) the configs uses static names for all fields which I presume would override each other. Not sure how the spiceDB handles writes of the same schemas. Did you try making the names of the schemas unique between runs?

The problem is that the schema in SpiceDB only overwrites itself, and using unique names does not solve this problem. Therefore, you cannot have multiple schemas in a single SpiceDB instance. That is why we want to create a single instance for each test.

@silvi-t silvi-t force-pushed the spiceDB-multiple-instances branch from 5964f57 to 0c10f12 Compare November 7, 2025 13:42
@silvi-t silvi-t force-pushed the spiceDB-multiple-instances branch from 0c10f12 to 7b6a524 Compare December 3, 2025 14:04
@silvi-t silvi-t changed the title SpiceDB refactor refactor: SpiceDB refactor Dec 3, 2025
@silvi-t
Copy link
Contributor Author

silvi-t commented Dec 3, 2025

I've added a commit to handle SpiceDB connection issues related to LoadBalancer provisioning and I would like to hear your thoughts on the solution @azgabur, @averevki:
Context:
The LoadBalancer external IP in our OpenStack environment may be provisioned and active but not routable from the test environment. This is an infrastructure limitation that occurs sporadically:

  • On clusters with rhos-01: LoadBalancer becomes reachable after approximately 5 minutes
  • On clusters with rhos-d: LoadBalancer IP never becomes accessible when this issue occurs
  • The issue is not consistent and only happens occasionally

The retry logic gives the LoadBalancer time to become accessible on rhos-01 clusters, and if it remains unreachable after retries, the test is skipped with a clear explanation rather than producing a false test failure.

@azgabur
Copy link
Contributor

azgabur commented Dec 3, 2025

Loadbalancer problems on Openstack are indeed infrastructure problem. If SpiceDB is not used in multicluster testing maybe using local service url would suffice?
Also changing from spicedb in tools to deploying instances at runtime would make the problem worse in my opinion as you "roll the dice" multiple times instead of once at kuadrant deploy in which case it can be fixed by re-creating the loadbalancer service and from that point it will keep working. If it gets stuck at runtime you need to only re-run the test and hope the spicedb loadbalancer will work that time.
At least my opinion.

@silvi-t silvi-t force-pushed the spiceDB-multiple-instances branch from 7b6a524 to a4d1447 Compare December 10, 2025 12:09
@silvi-t
Copy link
Contributor Author

silvi-t commented Dec 10, 2025

Loadbalancer problems on Openstack are indeed infrastructure problem. If SpiceDB is not used in multicluster testing maybe using local service url would suffice? Also changing from spicedb in tools to deploying instances at runtime would make the problem worse in my opinion as you "roll the dice" multiple times instead of once at kuadrant deploy in which case it can be fixed by re-creating the loadbalancer service and from that point it will keep working. If it gets stuck at runtime you need to only re-run the test and hope the spicedb loadbalancer will work that time. At least my opinion.

@azgabur I've taken your suggestion to use local service URLs instead of relying on LoadBalancer IPs.

I've changed the approach to route to SpiceDB through an HTTPRoute via the Gateway. This eliminates the LoadBalancer reliability issues on OpenStack entirely.

The reason I had to switch to an HTTP client is that the official authzed Python client only supports gRPC, and HTTPRoute cannot route gRPC traffic in OpenShift. So I implemented a custom HTTP client using httpx that
calls SpiceDB's HTTP API endpoints like /v1/schema/write and /v1/relationships/write.

Regarding deploying instances at runtime versus using SpiceDB in tools - I actually think having one instance per test is better in this case. SpiceDB doesn't support running multiple schemas in a single instance, which
makes parallel testing (if we add more tests in the future) impossible if we share one SpiceDB instance. Each test needs its own isolated schema, so deploying separate instances allows us to run tests in parallel without
schema conflicts.

@silvi-t silvi-t changed the title refactor: SpiceDB refactor refactor: SpiceDB test Dec 10, 2025
@github-project-automation github-project-automation bot moved this to 🆕 New in Kuadrant QE Dec 10, 2025
@silvi-t silvi-t moved this to Ready For Review in Kuadrant Dec 10, 2025
@silvi-t silvi-t moved this from 🆕 New to In Review in Kuadrant QE Dec 10, 2025
Signed-off-by: Silvia Tarabova <starabov@redhat.com>
@silvi-t silvi-t force-pushed the spiceDB-multiple-instances branch from a4d1447 to 1a55de4 Compare December 15, 2025 13:05
averevki
averevki previously approved these changes Jan 2, 2026
Copy link
Contributor

@averevki averevki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@azgabur azgabur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why the library was removed in favor of direct http client but it works and I dont mind.

max_tries=3,
interval=5,
jitter=None,
on_giveup=lambda details: (_ for _ in ()).throw(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the (_ for _ in ()) for? Seems like a no-op

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The (_ for _ in ()).throw(...) pattern is not a no-op - it's a working technique to raise exceptions from lambda expressions. It creates a generator object and calls its .throw() method to raise an exception, which is a workaround for the fact that lambda functions can only contain expressions, not raise statements.

"""
Create an OpenShift Route to SpiceDB for external access from test suite.
"""
route = OpenshiftRoute.create_instance(cluster, blame("spicedb"), service_name=spicedb.name, target_port="http")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using OpenshiftRoute is not encouraged in testsuite because it only works on Openshift. Not kubernetes or kind. I now see why you need to access the spicedb externally, because you are configuring it via http requests from testsuite. Its similar to the MockserverBackend. I think loadbalancer service should be used here instead. I tried it like this and it worked:

From 9c25dfe35bb25bab990c312073d9198db8a97361 Mon Sep 17 00:00:00 2001
From: Alex Zgabur <azgabur@redhat.com>
Date: Tue, 6 Jan 2026 14:44:00 +0100
Subject: [PATCH] TEST

Signed-off-by: Alex Zgabur <azgabur@redhat.com>
---
 testsuite/spicedb/spicedb.py                                 | 5 ++++-
 .../authorino/authorization/spicedb/test_spicedb.py          | 2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/testsuite/spicedb/spicedb.py b/testsuite/spicedb/spicedb.py
index f5aa21e..2a30010 100644
--- a/testsuite/spicedb/spicedb.py
+++ b/testsuite/spicedb/spicedb.py
@@ -5,6 +5,7 @@ from typing import List
 import backoff
 import httpx
 
+from testsuite.config import settings
 from testsuite.backend import Backend
 from testsuite.kubernetes import Selector
 from testsuite.kubernetes.client import KubernetesClient
@@ -256,12 +257,14 @@ class SpiceDB(Backend):
             selector=match_labels,
             ports=[
                 ServicePort(name="grpc", port=50051, targetPort="grpc"),
-                ServicePort(name="http", port=8443, targetPort="http"),
+                ServicePort(name="http", port=80, targetPort="http"),
             ],
             labels={"app": self.label},
+            service_type="LoadBalancer",
         )
         self.service.commit()
 
     def wait_for_ready(self, timeout=60 * 5):
         """Waits until Deployment is marked as ready"""
         self.deployment.wait_for_ready(timeout)
+        self.service.wait_for_ready(timeout, settings["control_plane"]["slow_loadbalancers"])
diff --git a/testsuite/tests/singlecluster/authorino/authorization/spicedb/test_spicedb.py b/testsuite/tests/singlecluster/authorino/authorization/spicedb/test_spicedb.py
index ca08030..c68e78a 100644
--- a/testsuite/tests/singlecluster/authorino/authorization/spicedb/test_spicedb.py
+++ b/testsuite/tests/singlecluster/authorino/authorization/spicedb/test_spicedb.py
@@ -68,7 +68,7 @@ def spicedb_route(request, spicedb, blame, cluster):
     request.addfinalizer(route.delete)
     route.commit()
 
-    spicedb.set_http_url(f"http://{route.hostname}")
+    spicedb.set_http_url(f"http://{spicedb.service.external_ip}")
 
     return route

This is just a quick hack to check if it is working, I would expect the client to be created in the SpiceDB class.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@averevki why did you recommend using the Openshift route?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using OpenShift Route instead of LoadBalancer due to stability/connection issues with LoadBalancer services when accessing SpiceDB. Added skip for Kind clusters.

Signed-off-by: Silvia Tarabova <starabov@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Ready For Review
Status: In Review

Development

Successfully merging this pull request may close these issues.

3 participants