Skip to content

Commit

Permalink
Merge pull request #95 from tisnik/refactoring-auth-in-own-module
Browse files Browse the repository at this point in the history
Refactoring: auth as separated module to be pluginable
  • Loading branch information
tisnik authored Nov 8, 2024
2 parents 24645a0 + 548defd commit b2d9920
Show file tree
Hide file tree
Showing 11 changed files with 30 additions and 28 deletions.
2 changes: 1 addition & 1 deletion ols/app/endpoints/authorized.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
ForbiddenResponse,
UnauthorizedResponse,
)
from ols.utils.auth_dependency import AuthDependency
from ols.src.auth.k8s import AuthDependency

logger = logging.getLogger(__name__)
router = APIRouter(tags=["authorized"])
Expand Down
2 changes: 1 addition & 1 deletion ols/app/endpoints/feedback.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
StatusResponse,
UnauthorizedResponse,
)
from ols.utils.auth_dependency import AuthDependency
from ols.src.auth.k8s import AuthDependency
from ols.utils.suid import get_suid

logger = logging.getLogger(__name__)
Expand Down
2 changes: 1 addition & 1 deletion ols/app/endpoints/ols.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@
UnauthorizedResponse,
)
from ols.customize import keywords, prompts
from ols.src.auth.k8s import AuthDependency
from ols.src.llms.llm_loader import LLMConfigurationError, resolve_provider_config
from ols.src.query_helpers.attachment_appender import append_attachments_to_query
from ols.src.query_helpers.docs_summarizer import DocsSummarizer
from ols.src.query_helpers.question_validator import QuestionValidator
from ols.utils import errors_parsing, suid
from ols.utils.auth_dependency import AuthDependency
from ols.utils.token_handler import PromptTooLongError

logger = logging.getLogger(__name__)
Expand Down
2 changes: 1 addition & 1 deletion ols/app/metrics/metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
generate_latest,
)

from ols.utils.auth_dependency import AuthDependency
from ols.src.auth.k8s import AuthDependency
from ols.utils.config import AppConfig

router = APIRouter(tags=["metrics"])
Expand Down
1 change: 1 addition & 0 deletions ols/src/auth/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"""Various implementations of auth module."""
File renamed without changes.
2 changes: 1 addition & 1 deletion ols/user_data_collection/data_collector.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@


# pylint: disable-next=C0413
from ols.utils.auth_dependency import K8sClientSingleton # noqa: E402
from ols.src.auth.k8s import K8sClientSingleton # noqa: E402

INITIAL_WAIT = 60 * 5 # 5 minutes in seconds
INGRESS_TIMEOUT = 30 # seconds
Expand Down
8 changes: 4 additions & 4 deletions tests/integration/test_authorized.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ def test_post_authorized_no_token():


@pytest.mark.usefixtures("_enabled_auth")
@patch("ols.utils.auth_dependency.K8sClientSingleton.get_authn_api")
@patch("ols.utils.auth_dependency.K8sClientSingleton.get_authz_api")
@patch("ols.src.auth.k8s.K8sClientSingleton.get_authn_api")
@patch("ols.src.auth.k8s.K8sClientSingleton.get_authz_api")
def test_is_user_authorized_valid_token(mock_authz_api, mock_authn_api):
"""Tests the is_user_authorized function with a mocked valid-token."""
# Setup mock responses for valid token
Expand All @@ -87,8 +87,8 @@ def test_is_user_authorized_valid_token(mock_authz_api, mock_authn_api):


@pytest.mark.usefixtures("_enabled_auth")
@patch("ols.utils.auth_dependency.K8sClientSingleton.get_authn_api")
@patch("ols.utils.auth_dependency.K8sClientSingleton.get_authz_api")
@patch("ols.src.auth.k8s.K8sClientSingleton.get_authn_api")
@patch("ols.src.auth.k8s.K8sClientSingleton.get_authz_api")
def test_is_user_authorized_invalid_token(mock_authz_api, mock_authn_api):
"""Test the is_user_authorized function with a mocked invalid-token."""
# Setup mock responses for invalid token
Expand Down
8 changes: 4 additions & 4 deletions tests/unit/app/endpoints/test_authorized.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ def test_is_user_authorized_false_no_bearer_token():


@pytest.mark.usefixtures("_enabled_auth")
@patch("ols.utils.auth_dependency.K8sClientSingleton.get_authn_api")
@patch("ols.utils.auth_dependency.K8sClientSingleton.get_authz_api")
@patch("ols.src.auth.k8s.K8sClientSingleton.get_authn_api")
@patch("ols.src.auth.k8s.K8sClientSingleton.get_authz_api")
def test_is_user_authorized_valid_token(mock_authz_api, mock_authn_api):
"""Tests the is_user_authorized function with a mocked valid-token."""
# Setup mock responses for valid token
Expand All @@ -78,8 +78,8 @@ def test_is_user_authorized_valid_token(mock_authz_api, mock_authn_api):


@pytest.mark.usefixtures("_enabled_auth")
@patch("ols.utils.auth_dependency.K8sClientSingleton.get_authn_api")
@patch("ols.utils.auth_dependency.K8sClientSingleton.get_authz_api")
@patch("ols.src.auth.k8s.K8sClientSingleton.get_authn_api")
@patch("ols.src.auth.k8s.K8sClientSingleton.get_authz_api")
def test_is_user_authorized_invalid_token(mock_authz_api, mock_authn_api):
"""Test the is_user_authorized function with a mocked invalid-token."""
# Setup mock responses for invalid token
Expand Down
1 change: 1 addition & 0 deletions tests/unit/auth/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"""Unit tests for auth. dependency."""
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from kubernetes.client.rest import ApiException

from ols import config
from ols.utils.auth_dependency import (
from ols.src.auth.k8s import (
CLUSTER_ID_LOCAL,
AuthDependency,
ClusterIDUnavailableError,
Expand Down Expand Up @@ -40,8 +40,8 @@ def test_singleton_pattern():

@pytest.mark.usefixtures("_setup")
@pytest.mark.asyncio()
@patch("ols.utils.auth_dependency.K8sClientSingleton.get_authn_api")
@patch("ols.utils.auth_dependency.K8sClientSingleton.get_authz_api")
@patch("ols.src.auth.k8s.K8sClientSingleton.get_authn_api")
@patch("ols.src.auth.k8s.K8sClientSingleton.get_authz_api")
async def test_auth_dependency_valid_token(mock_authz_api, mock_authn_api):
"""Tests the auth dependency with a mocked valid-token."""
# Setup mock responses for valid token
Expand All @@ -66,8 +66,8 @@ async def test_auth_dependency_valid_token(mock_authz_api, mock_authn_api):

@pytest.mark.usefixtures("_setup")
@pytest.mark.asyncio()
@patch("ols.utils.auth_dependency.K8sClientSingleton.get_authn_api")
@patch("ols.utils.auth_dependency.K8sClientSingleton.get_authz_api")
@patch("ols.src.auth.k8s.K8sClientSingleton.get_authn_api")
@patch("ols.src.auth.k8s.K8sClientSingleton.get_authz_api")
async def test_auth_dependency_invalid_token(mock_authz_api, mock_authn_api):
"""Test the auth dependency with a mocked invalid-token."""
# Setup mock responses for invalid token
Expand All @@ -93,7 +93,7 @@ async def test_auth_dependency_invalid_token(mock_authz_api, mock_authn_api):

@pytest.mark.usefixtures("_setup")
@pytest.mark.asyncio()
@patch("ols.utils.auth_dependency.K8sClientSingleton.get_authz_api")
@patch("ols.src.auth.k8s.K8sClientSingleton.get_authz_api")
async def test_cluster_id_is_used_for_kube_admin(mock_authz_api):
"""Test the cluster id is used as user_id when user is kube:admin."""
mock_authz_api.return_value.create_subject_access_review.side_effect = (
Expand All @@ -107,13 +107,13 @@ async def test_cluster_id_is_used_for_kube_admin(mock_authz_api):

with (
patch(
"ols.utils.auth_dependency.get_user_info",
"ols.src.auth.k8s.get_user_info",
return_value=MockK8sResponseStatus(
True, True, "kube:admin", "some-uuid", "ols-group"
),
),
patch(
"ols.utils.auth_dependency.K8sClientSingleton.get_cluster_id",
"ols.src.auth.k8s.K8sClientSingleton.get_cluster_id",
return_value="some-cluster-id",
),
):
Expand All @@ -128,7 +128,7 @@ async def test_cluster_id_is_used_for_kube_admin(mock_authz_api):
@patch.dict(os.environ, {"KUBECONFIG": "tests/config/kubeconfig"})
def test_auth_dependency_config():
"""Test the auth dependency can load kubeconfig file."""
from ols.utils.auth_dependency import K8sClientSingleton
from ols.src.auth.k8s import K8sClientSingleton

authn_client = K8sClientSingleton.get_authn_api()
authz_client = K8sClientSingleton.get_authz_api()
Expand All @@ -140,7 +140,7 @@ def test_auth_dependency_config():
), "authz_client is not an instance of AuthorizationV1Api"


@patch("ols.utils.auth_dependency.K8sClientSingleton.get_custom_objects_api")
@patch("ols.src.auth.k8s.K8sClientSingleton.get_custom_objects_api")
def test_get_cluster_id(mock_get_custom_objects_api):
"""Test get_cluster_id function."""
cluster_id = {"spec": {"clusterID": "some-cluster-id"}}
Expand Down Expand Up @@ -176,17 +176,17 @@ def test_get_cluster_id(mock_get_custom_objects_api):
K8sClientSingleton._get_cluster_id()


@patch("ols.utils.auth_dependency.RUNNING_IN_CLUSTER", True)
@patch("ols.utils.auth_dependency.K8sClientSingleton.__new__")
@patch("ols.utils.auth_dependency.K8sClientSingleton._get_cluster_id")
@patch("ols.src.auth.k8s.RUNNING_IN_CLUSTER", True)
@patch("ols.src.auth.k8s.K8sClientSingleton.__new__")
@patch("ols.src.auth.k8s.K8sClientSingleton._get_cluster_id")
def test_get_cluster_id_in_cluster(mock_get_cluster_id, _mock_new):
"""Test get_cluster_id function when running inside of cluster."""
mock_get_cluster_id.return_value = "some-cluster-id"
assert K8sClientSingleton.get_cluster_id() == "some-cluster-id"


@patch("ols.utils.auth_dependency.RUNNING_IN_CLUSTER", False)
@patch("ols.utils.auth_dependency.K8sClientSingleton.__new__")
@patch("ols.src.auth.k8s.RUNNING_IN_CLUSTER", False)
@patch("ols.src.auth.k8s.K8sClientSingleton.__new__")
def test_get_cluster_id_outside_of_cluster(_mock_new):
"""Test get_cluster_id function when running outside of cluster."""
# ensure cluster_id is None to trigger the condition
Expand Down

0 comments on commit b2d9920

Please sign in to comment.