-
Notifications
You must be signed in to change notification settings - Fork 63
feat(evalhub): Update EvalHub provider tests and add RBAC fixtures #1178
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 6 commits
1fe44f3
eb77148
783e4f6
55b31df
d297049
6fc160b
c26a84f
121348f
c9c8790
04a3b59
3771ceb
03dc1d4
663ce8d
a5812e4
4e7a02d
2017d59
8a19c7d
d975393
cb1b962
056e22c
06d8faf
7ed9fed
d7a0f95
c5772cc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,18 @@ | ||
| import shlex | ||
| from collections.abc import Generator | ||
| from typing import Any | ||
|
|
||
| import pytest | ||
| from kubernetes.dynamic import DynamicClient | ||
| from ocp_resources.deployment import Deployment | ||
| from ocp_resources.namespace import Namespace | ||
| from ocp_resources.role_binding import RoleBinding | ||
| from ocp_resources.route import Route | ||
| from ocp_resources.service_account import ServiceAccount | ||
| from pyhelper_utils.shell import run_command | ||
| from simple_logger.logger import get_logger | ||
|
|
||
| from tests.model_explainability.evalhub.constants import EVALHUB_PROVIDERS_ACCESS_CLUSTER_ROLE | ||
| from utilities.certificates_utils import create_ca_bundle_file | ||
| from utilities.constants import Timeout | ||
| from utilities.resources.evalhub import EvalHub | ||
|
|
@@ -67,3 +72,74 @@ def evalhub_ca_bundle_file( | |
| ) -> str: | ||
| """Create a CA bundle file for verifying the EvalHub route TLS certificate.""" | ||
| return create_ca_bundle_file(client=admin_client) | ||
|
|
||
|
|
||
| @pytest.fixture(scope="class") | ||
| def evalhub_scoped_sa( | ||
| admin_client: DynamicClient, | ||
| model_namespace: Namespace, | ||
| evalhub_deployment: Deployment, | ||
| ) -> Generator[ServiceAccount, Any, Any]: | ||
| """ServiceAccount with providers access in the test namespace.""" | ||
| with ServiceAccount( | ||
| client=admin_client, | ||
| name="evalhub-test-user", | ||
| namespace=model_namespace.name, | ||
| ) as sa: | ||
| yield sa | ||
|
|
||
|
|
||
| @pytest.fixture(scope="class") | ||
| def evalhub_providers_role_binding( | ||
| admin_client: DynamicClient, | ||
| model_namespace: Namespace, | ||
| evalhub_scoped_sa: ServiceAccount, | ||
| ) -> Generator[RoleBinding, Any, Any]: | ||
| """RoleBinding granting the scoped SA providers access via the ClusterRole.""" | ||
| with RoleBinding( | ||
| client=admin_client, | ||
| name="evalhub-test-providers-access", | ||
| namespace=model_namespace.name, | ||
| role_ref_kind="ClusterRole", | ||
| role_ref_name=EVALHUB_PROVIDERS_ACCESS_CLUSTER_ROLE, | ||
| subjects_kind="ServiceAccount", | ||
| subjects_name=evalhub_scoped_sa.name, | ||
| ) as rb: | ||
| yield rb | ||
|
|
||
|
|
||
| @pytest.fixture(scope="class") | ||
| def evalhub_scoped_token( | ||
| evalhub_scoped_sa: ServiceAccount, | ||
| model_namespace: Namespace, | ||
| ) -> str: | ||
| """Short-lived token for the scoped ServiceAccount.""" | ||
| return run_command( | ||
| shlex.split(f"oc create token -n {model_namespace.name} {evalhub_scoped_sa.name} --duration=30m") | ||
| )[1].strip() | ||
|
|
||
|
|
||
| @pytest.fixture(scope="class") | ||
| def evalhub_unauthorised_sa( | ||
| admin_client: DynamicClient, | ||
| model_namespace: Namespace, | ||
| evalhub_deployment: Deployment, | ||
| ) -> Generator[ServiceAccount, Any, Any]: | ||
| """ServiceAccount without any EvalHub RBAC in the test namespace.""" | ||
| with ServiceAccount( | ||
| client=admin_client, | ||
| name="evalhub-no-access-user", | ||
| namespace=model_namespace.name, | ||
| ) as sa: | ||
| yield sa | ||
|
|
||
|
|
||
| @pytest.fixture(scope="class") | ||
| def evalhub_unauthorised_token( | ||
| evalhub_unauthorised_sa: ServiceAccount, | ||
| model_namespace: Namespace, | ||
| ) -> str: | ||
| """Short-lived token for the unauthorised ServiceAccount.""" | ||
| return run_command( | ||
| shlex.split(f"oc create token -n {model_namespace.name} {evalhub_unauthorised_sa.name} --duration=30m") | ||
|
||
| )[1].strip() | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,232 @@ | ||
| import pytest | ||
| import requests | ||
| from ocp_resources.namespace import Namespace | ||
| from ocp_resources.role_binding import RoleBinding | ||
| from ocp_resources.route import Route | ||
|
|
||
| from tests.model_explainability.evalhub.utils import ( | ||
| get_evalhub_provider, | ||
| list_evalhub_providers, | ||
| ) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "model_namespace", | ||
| [ | ||
| pytest.param( | ||
| {"name": "test-evalhub-providers"}, | ||
| ), | ||
| ], | ||
| indirect=True, | ||
| ) | ||
| @pytest.mark.sanity | ||
| @pytest.mark.model_explainability | ||
| class TestEvalHubProviders: | ||
| """Tests for the EvalHub providers API using a scoped non-admin ServiceAccount.""" | ||
|
|
||
| def test_list_providers_returns_paginated_response( | ||
|
||
| self, | ||
| model_namespace: Namespace, | ||
| evalhub_scoped_token: str, | ||
| evalhub_providers_role_binding: RoleBinding, | ||
| evalhub_ca_bundle_file: str, | ||
| evalhub_route: Route, | ||
| ) -> None: | ||
| """Verify that a scoped user with providers access can list providers.""" | ||
| data = list_evalhub_providers( | ||
| host=evalhub_route.host, | ||
| token=evalhub_scoped_token, | ||
| ca_bundle_file=evalhub_ca_bundle_file, | ||
| tenant=model_namespace.name, | ||
| ) | ||
|
||
|
|
||
| assert "items" in data, "Response missing 'items' field" | ||
| assert isinstance(data["items"], list), "'items' must be a list" | ||
| assert "total_count" in data, "Response missing 'total_count' field" | ||
| assert "limit" in data, "Response missing 'limit' field" | ||
|
|
||
| def test_list_providers_has_registered_providers( | ||
| self, | ||
| model_namespace: Namespace, | ||
| evalhub_scoped_token: str, | ||
| evalhub_providers_role_binding: RoleBinding, | ||
| evalhub_ca_bundle_file: str, | ||
| evalhub_route: Route, | ||
| ) -> None: | ||
| """Verify that at least one provider is registered.""" | ||
| data = list_evalhub_providers( | ||
| host=evalhub_route.host, | ||
| token=evalhub_scoped_token, | ||
| ca_bundle_file=evalhub_ca_bundle_file, | ||
| tenant=model_namespace.name, | ||
| ) | ||
|
|
||
| assert data["total_count"] > 0, "Expected at least one registered provider" | ||
| assert len(data["items"]) > 0, "Expected at least one provider in items" | ||
|
|
||
| def test_provider_has_required_fields( | ||
| self, | ||
| model_namespace: Namespace, | ||
| evalhub_scoped_token: str, | ||
| evalhub_providers_role_binding: RoleBinding, | ||
| evalhub_ca_bundle_file: str, | ||
| evalhub_route: Route, | ||
| ) -> None: | ||
| """Verify that each provider contains the expected resource metadata and config fields.""" | ||
| data = list_evalhub_providers( | ||
| host=evalhub_route.host, | ||
| token=evalhub_scoped_token, | ||
| ca_bundle_file=evalhub_ca_bundle_file, | ||
| tenant=model_namespace.name, | ||
| ) | ||
|
|
||
| for provider in data["items"]: | ||
| assert "resource" in provider, f"Provider missing 'resource': {provider}" | ||
| assert "id" in provider["resource"], f"Provider resource missing 'id': {provider}" | ||
| assert provider["resource"]["id"], "Provider ID must not be empty" | ||
| assert "name" in provider, f"Provider missing 'name': {provider}" | ||
| assert "benchmarks" in provider, f"Provider missing 'benchmarks': {provider}" | ||
|
|
||
| def test_provider_benchmarks_have_required_fields( | ||
| self, | ||
| model_namespace: Namespace, | ||
| evalhub_scoped_token: str, | ||
| evalhub_providers_role_binding: RoleBinding, | ||
| evalhub_ca_bundle_file: str, | ||
| evalhub_route: Route, | ||
| ) -> None: | ||
| """Verify that benchmarks within each provider have id, name, and category.""" | ||
| data = list_evalhub_providers( | ||
| host=evalhub_route.host, | ||
| token=evalhub_scoped_token, | ||
| ca_bundle_file=evalhub_ca_bundle_file, | ||
| tenant=model_namespace.name, | ||
| ) | ||
|
|
||
| for provider in data["items"]: | ||
| provider_name = provider.get("name", "unknown") | ||
| for benchmark in provider.get("benchmarks", []): | ||
| assert "id" in benchmark, f"Benchmark in provider '{provider_name}' missing 'id'" | ||
| assert "name" in benchmark, f"Benchmark in provider '{provider_name}' missing 'name'" | ||
| assert "category" in benchmark, f"Benchmark in provider '{provider_name}' missing 'category'" | ||
|
|
||
| def test_lm_evaluation_harness_provider_exists( | ||
| self, | ||
| model_namespace: Namespace, | ||
| evalhub_scoped_token: str, | ||
| evalhub_providers_role_binding: RoleBinding, | ||
| evalhub_ca_bundle_file: str, | ||
| evalhub_route: Route, | ||
| ) -> None: | ||
| """Verify that the lm_evaluation_harness provider is registered and has benchmarks.""" | ||
| data = list_evalhub_providers( | ||
| host=evalhub_route.host, | ||
| token=evalhub_scoped_token, | ||
| ca_bundle_file=evalhub_ca_bundle_file, | ||
| tenant=model_namespace.name, | ||
| ) | ||
|
|
||
| provider_ids = [p["resource"]["id"] for p in data["items"]] | ||
|
||
| assert "lm_evaluation_harness" in provider_ids, ( | ||
| f"Expected 'lm_evaluation_harness' in providers, got: {provider_ids}" | ||
| ) | ||
|
|
||
| lmeval_provider = next(p for p in data["items"] if p["resource"]["id"] == "lm_evaluation_harness") | ||
|
||
| assert len(lmeval_provider["benchmarks"]) > 0, ( | ||
| "lm_evaluation_harness provider should have at least one benchmark" | ||
| ) | ||
|
|
||
| def test_get_single_provider( | ||
| self, | ||
| model_namespace: Namespace, | ||
| evalhub_scoped_token: str, | ||
| evalhub_providers_role_binding: RoleBinding, | ||
| evalhub_ca_bundle_file: str, | ||
| evalhub_route: Route, | ||
| ) -> None: | ||
| """Verify that a single provider can be retrieved by ID.""" | ||
| providers = list_evalhub_providers( | ||
| host=evalhub_route.host, | ||
| token=evalhub_scoped_token, | ||
| ca_bundle_file=evalhub_ca_bundle_file, | ||
| tenant=model_namespace.name, | ||
| ) | ||
| first_provider_id = providers["items"][0]["resource"]["id"] | ||
coderabbitai[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| data = get_evalhub_provider( | ||
| host=evalhub_route.host, | ||
| token=evalhub_scoped_token, | ||
| ca_bundle_file=evalhub_ca_bundle_file, | ||
| provider_id=first_provider_id, | ||
| tenant=model_namespace.name, | ||
| ) | ||
|
|
||
| assert data["resource"]["id"] == first_provider_id | ||
| assert "name" in data | ||
| assert "benchmarks" in data | ||
|
|
||
| def test_get_nonexistent_provider_returns_error( | ||
| self, | ||
| model_namespace: Namespace, | ||
| evalhub_scoped_token: str, | ||
| evalhub_providers_role_binding: RoleBinding, | ||
| evalhub_ca_bundle_file: str, | ||
| evalhub_route: Route, | ||
| ) -> None: | ||
| """Verify that requesting a non-existent provider ID returns an HTTP error.""" | ||
| with pytest.raises(requests.exceptions.HTTPError): | ||
| get_evalhub_provider( | ||
| host=evalhub_route.host, | ||
| token=evalhub_scoped_token, | ||
| ca_bundle_file=evalhub_ca_bundle_file, | ||
| provider_id="nonexistent-provider-id", | ||
| tenant=model_namespace.name, | ||
| ) | ||
coderabbitai[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "model_namespace", | ||
| [ | ||
| pytest.param( | ||
| {"name": "test-evalhub-providers"}, | ||
| ), | ||
| ], | ||
| indirect=True, | ||
| ) | ||
| @pytest.mark.sanity | ||
| @pytest.mark.model_explainability | ||
| class TestEvalHubProvidersUnauthorised: | ||
| """Tests verifying that a user without providers RBAC is denied access.""" | ||
|
|
||
| def test_list_providers_denied_without_role_binding( | ||
| self, | ||
| model_namespace: Namespace, | ||
| evalhub_unauthorised_token: str, | ||
| evalhub_ca_bundle_file: str, | ||
| evalhub_route: Route, | ||
| ) -> None: | ||
| """Verify that a user without providers ClusterRole binding gets 403.""" | ||
| with pytest.raises(requests.exceptions.HTTPError, match="403"): | ||
| list_evalhub_providers( | ||
| host=evalhub_route.host, | ||
| token=evalhub_unauthorised_token, | ||
| ca_bundle_file=evalhub_ca_bundle_file, | ||
| tenant=model_namespace.name, | ||
| ) | ||
|
Comment on lines
+153
to
+159
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: cd tests/model_explainability/evalhub && wc -l test_evalhub_providers.pyRepository: opendatahub-io/opendatahub-tests Length of output: 105 🏁 Script executed: cd tests/model_explainability/evalhub && sed -n '175,195p' test_evalhub_providers.pyRepository: opendatahub-io/opendatahub-tests Length of output: 829 🏁 Script executed: cd tests/model_explainability/evalhub && sed -n '210,240p' test_evalhub_providers.pyRepository: opendatahub-io/opendatahub-tests Length of output: 1201 🏁 Script executed: cd tests/model_explainability/evalhub && sed -n '65,85p' utils.pyRepository: opendatahub-io/opendatahub-tests Length of output: 764 🏁 Script executed: cd tests/model_explainability/evalhub && grep -n "raise_for_status\|HTTPError" utils.pyRepository: opendatahub-io/opendatahub-tests Length of output: 342 🏁 Script executed: cd tests/model_explainability/evalhub && sed -n '1,30p' test_evalhub_providers.pyRepository: opendatahub-io/opendatahub-tests Length of output: 829 🏁 Script executed: cd tests/model_explainability/evalhub && sed -n '85,110p' utils.pyRepository: opendatahub-io/opendatahub-tests Length of output: 734 🏁 Script executed: cd tests/model_explainability/evalhub && python3 << 'EOF'
import requests
# Verify that HTTPError from raise_for_status() has a response attribute
try:
# Create a mock response scenario to understand HTTPError structure
response = requests.Response()
response.status_code = 403
response.url = "http://example.com"
# Simulate what raise_for_status does
http_error = requests.exceptions.HTTPError()
http_error.response = response
# Check if we can access response.status_code
print(f"HTTPError.response accessible: {http_error.response is not None}")
print(f"HTTPError.response.status_code accessible: {http_error.response.status_code}")
except Exception as e:
print(f"Error: {e}")
EOFRepository: opendatahub-io/opendatahub-tests Length of output: 382 🏁 Script executed: cd tests/model_explainability/evalhub && grep -B5 -A5 "match=" test_evalhub_providers.py | head -30Repository: opendatahub-io/opendatahub-tests Length of output: 1103 Assert the 403 contract on
Suggested change- with pytest.raises(requests.exceptions.HTTPError, match="403"):
+ with pytest.raises(requests.exceptions.HTTPError) as excinfo:
list_evalhub_providers(
host=evalhub_route.host,
token=evalhub_unauthorised_token,
ca_bundle_file=evalhub_ca_bundle_file,
tenant=model_namespace.name,
)
+ assert excinfo.value.response is not None
+ assert excinfo.value.response.status_code == 403
...
- with pytest.raises(requests.exceptions.HTTPError, match="403"):
+ with pytest.raises(requests.exceptions.HTTPError) as excinfo:
get_evalhub_provider(
host=evalhub_route.host,
token=evalhub_unauthorised_token,
ca_bundle_file=evalhub_ca_bundle_file,
provider_id="lm_evaluation_harness",
tenant=model_namespace.name,
)
+ assert excinfo.value.response is not None
+ assert excinfo.value.response.status_code == 403Also applies to: Lines 229-236. |
||
|
|
||
| def test_get_provider_denied_without_role_binding( | ||
| self, | ||
| model_namespace: Namespace, | ||
| evalhub_unauthorised_token: str, | ||
| evalhub_ca_bundle_file: str, | ||
| evalhub_route: Route, | ||
| ) -> None: | ||
| """Verify that a user without providers ClusterRole binding cannot get a provider.""" | ||
| with pytest.raises(requests.exceptions.HTTPError, match="403"): | ||
| get_evalhub_provider( | ||
| host=evalhub_route.host, | ||
| token=evalhub_unauthorised_token, | ||
| ca_bundle_file=evalhub_ca_bundle_file, | ||
| provider_id="lm_evaluation_harness", | ||
| tenant=model_namespace.name, | ||
| ) | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Uh oh!
There was an error while loading. Please reload this page.