diff --git a/poetry.lock b/poetry.lock index a0ed4770e..f52cd9c97 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1175,16 +1175,6 @@ files = [ {file = "MarkupSafe-2.1.3-cp311-cp311-musllinux_1_1_x86_64.whl", hash = "sha256:5bbe06f8eeafd38e5d0a4894ffec89378b6c6a625ff57e3028921f8ff59318ac"}, {file = "MarkupSafe-2.1.3-cp311-cp311-win32.whl", hash = "sha256:dd15ff04ffd7e05ffcb7fe79f1b98041b8ea30ae9234aed2a9168b5797c3effb"}, {file = "MarkupSafe-2.1.3-cp311-cp311-win_amd64.whl", hash = "sha256:134da1eca9ec0ae528110ccc9e48041e0828d79f24121a1a146161103c76e686"}, - {file = "MarkupSafe-2.1.3-cp312-cp312-macosx_10_9_universal2.whl", hash = "sha256:f698de3fd0c4e6972b92290a45bd9b1536bffe8c6759c62471efaa8acb4c37bc"}, - {file = "MarkupSafe-2.1.3-cp312-cp312-macosx_10_9_x86_64.whl", hash = "sha256:aa57bd9cf8ae831a362185ee444e15a93ecb2e344c8e52e4d721ea3ab6ef1823"}, - {file = "MarkupSafe-2.1.3-cp312-cp312-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:ffcc3f7c66b5f5b7931a5aa68fc9cecc51e685ef90282f4a82f0f5e9b704ad11"}, - {file = "MarkupSafe-2.1.3-cp312-cp312-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:47d4f1c5f80fc62fdd7777d0d40a2e9dda0a05883ab11374334f6c4de38adffd"}, - {file = "MarkupSafe-2.1.3-cp312-cp312-manylinux_2_5_i686.manylinux1_i686.manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:1f67c7038d560d92149c060157d623c542173016c4babc0c1913cca0564b9939"}, - {file = "MarkupSafe-2.1.3-cp312-cp312-musllinux_1_1_aarch64.whl", hash = "sha256:9aad3c1755095ce347e26488214ef77e0485a3c34a50c5a5e2471dff60b9dd9c"}, - {file = "MarkupSafe-2.1.3-cp312-cp312-musllinux_1_1_i686.whl", hash = "sha256:14ff806850827afd6b07a5f32bd917fb7f45b046ba40c57abdb636674a8b559c"}, - {file = "MarkupSafe-2.1.3-cp312-cp312-musllinux_1_1_x86_64.whl", hash = "sha256:8f9293864fe09b8149f0cc42ce56e3f0e54de883a9de90cd427f191c346eb2e1"}, - {file = "MarkupSafe-2.1.3-cp312-cp312-win32.whl", hash = "sha256:715d3562f79d540f251b99ebd6d8baa547118974341db04f5ad06d5ea3eb8007"}, - {file = "MarkupSafe-2.1.3-cp312-cp312-win_amd64.whl", hash = "sha256:1b8dd8c3fd14349433c79fa8abeb573a55fc0fdd769133baac1f5e07abf54aeb"}, {file = "MarkupSafe-2.1.3-cp37-cp37m-macosx_10_9_x86_64.whl", hash = "sha256:8e254ae696c88d98da6555f5ace2279cf7cd5b3f52be2b5cf97feafe883b58d2"}, {file = "MarkupSafe-2.1.3-cp37-cp37m-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:cb0932dc158471523c9637e807d9bfb93e06a95cbf010f1a38b98623b929ef2b"}, {file = "MarkupSafe-2.1.3-cp37-cp37m-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:9402b03f1a1b4dc4c19845e5c749e3ab82d5078d16a2a4c2cd2df62d57bb0707"}, diff --git a/src/codeflare_sdk/utils/generate_yaml.py b/src/codeflare_sdk/utils/generate_yaml.py index 95c962d19..2d507c8ee 100755 --- a/src/codeflare_sdk/utils/generate_yaml.py +++ b/src/codeflare_sdk/utils/generate_yaml.py @@ -29,8 +29,6 @@ from base64 import b64encode from urllib3.util import parse_url -from .kube_api_helpers import _get_api_host - def read_template(template): with open(template, "r") as stream: @@ -557,10 +555,6 @@ def enable_openshift_oauth(user_yaml, cluster_name, namespace): tls_secret_name = f"{cluster_name}-proxy-tls-secret" tls_volume_name = "proxy-tls-secret" port_name = "oauth-proxy" - host = _get_api_host(k8_client) - host = host.replace( - "api.", f"{gen_dashboard_ingress_name(cluster_name)}-{namespace}.apps." - ) oauth_sidecar = _create_oauth_sidecar_object( namespace, tls_mount_location, diff --git a/src/codeflare_sdk/utils/kube_api_helpers.py b/src/codeflare_sdk/utils/kube_api_helpers.py index 01a93ef5c..17cf6dbe3 100644 --- a/src/codeflare_sdk/utils/kube_api_helpers.py +++ b/src/codeflare_sdk/utils/kube_api_helpers.py @@ -47,7 +47,3 @@ def _kube_api_error_handling( elif e.reason == "Conflict": raise FileExistsError(exists_msg) raise e - - -def _get_api_host(api_client: client.ApiClient): # pragma: no cover - return parse_url(api_client.configuration.host).host diff --git a/src/codeflare_sdk/utils/openshift_oauth.py b/src/codeflare_sdk/utils/openshift_oauth.py index 14e55b96b..37038c563 100644 --- a/src/codeflare_sdk/utils/openshift_oauth.py +++ b/src/codeflare_sdk/utils/openshift_oauth.py @@ -1,39 +1,40 @@ from urllib3.util import parse_url -from .generate_yaml import gen_dashboard_ingress_name -from .kube_api_helpers import _get_api_host -from base64 import b64decode +import yaml from ..cluster.auth import config_check, api_config_handler from kubernetes import client +from kubernetes import dynamic + + +def _route_api_getter(): + return dynamic.DynamicClient( + api_config_handler() or client.ApiClient() + ).resources.get(api_version="route.openshift.io/v1", kind="Route") def create_openshift_oauth_objects(cluster_name, namespace): config_check() - api_client = api_config_handler() or client.ApiClient() oauth_port = 8443 oauth_sa_name = f"{cluster_name}-oauth-proxy" tls_secret_name = _gen_tls_secret_name(cluster_name) service_name = f"{cluster_name}-oauth" port_name = "oauth-proxy" - host = _get_api_host(api_client) - - # replace "^api" with the expected host - host = f"{gen_dashboard_ingress_name(cluster_name)}-{namespace}.apps" + host.lstrip( - "api" - ) - _create_or_replace_oauth_sa(namespace, oauth_sa_name, host) + _create_or_replace_oauth_sa(namespace, oauth_sa_name, cluster_name) _create_or_replace_oauth_service_obj( cluster_name, namespace, oauth_port, tls_secret_name, service_name, port_name ) - _create_or_replace_oauth_ingress_object( - cluster_name, namespace, service_name, port_name, host + _create_or_replace_oauth_route_object( + cluster_name, + namespace, + service_name, + port_name, ) _create_or_replace_oauth_rb(cluster_name, namespace, oauth_sa_name) -def _create_or_replace_oauth_sa(namespace, oauth_sa_name, host): +def _create_or_replace_oauth_sa(namespace, oauth_sa_name, cluster_name): oauth_sa = client.V1ServiceAccount( api_version="v1", kind="ServiceAccount", @@ -41,7 +42,10 @@ def _create_or_replace_oauth_sa(namespace, oauth_sa_name, host): name=oauth_sa_name, namespace=namespace, annotations={ - "serviceaccounts.openshift.io/oauth-redirecturi.first": f"https://{host}" + "serviceaccounts.openshift.io/oauth-redirectreference.first": '{"kind":"OAuthRedirectReference","apiVersion":"v1","reference":{"kind":"Route","name":"' + + "ray-dashboard-" + + cluster_name + + '"}}' }, ), ) @@ -98,15 +102,14 @@ def delete_openshift_oauth_objects(cluster_name, namespace): # for an existing cluster before calling this => the objects should never be deleted twice oauth_sa_name = f"{cluster_name}-oauth-proxy" service_name = f"{cluster_name}-oauth" + v1_routes = _route_api_getter() client.CoreV1Api(api_config_handler()).delete_namespaced_service_account( name=oauth_sa_name, namespace=namespace ) client.CoreV1Api(api_config_handler()).delete_namespaced_service( name=service_name, namespace=namespace ) - client.NetworkingV1Api(api_config_handler()).delete_namespaced_ingress( - name=f"{cluster_name}-ingress", namespace=namespace - ) + v1_routes.delete(name=f"ray-dashboard-{cluster_name}", namespace=namespace) client.RbacAuthorizationV1Api(api_config_handler()).delete_cluster_role_binding( name=f"{cluster_name}-rb" ) @@ -161,52 +164,36 @@ def _create_or_replace_oauth_service_obj( raise e -def _create_or_replace_oauth_ingress_object( +def _create_or_replace_oauth_route_object( cluster_name: str, namespace: str, service_name: str, port_name: str, - host: str, -) -> client.V1Ingress: - ingress = client.V1Ingress( - api_version="networking.k8s.io/v1", - kind="Ingress", - metadata=client.V1ObjectMeta( - annotations={"route.openshift.io/termination": "passthrough"}, - name=f"{cluster_name}-ingress", - namespace=namespace, - ), - spec=client.V1IngressSpec( - rules=[ - client.V1IngressRule( - host=host, - http=client.V1HTTPIngressRuleValue( - paths=[ - client.V1HTTPIngressPath( - backend=client.V1IngressBackend( - service=client.V1IngressServiceBackend( - name=service_name, - port=client.V1ServiceBackendPort( - name=port_name - ), - ) - ), - path_type="ImplementationSpecific", - ) - ] - ), - ) - ] - ), - ) +): + route = f""" + apiVersion: route.openshift.io/v1 + kind: Route + metadata: + name: ray-dashboard-{cluster_name} + namespace: {namespace} + spec: + port: + targetPort: {port_name} + tls: + termination: passthrough + to: + kind: Service + name: {service_name} + """ + route_data = yaml.safe_load(route) + v1_routes = _route_api_getter() try: - client.NetworkingV1Api(api_config_handler()).create_namespaced_ingress( - namespace=namespace, body=ingress + existing_route = v1_routes.get( + name=f"ray-dashboard-{cluster_name}", namespace=namespace ) - except client.ApiException as e: - if e.reason == "Conflict": - client.NetworkingV1Api(api_config_handler()).replace_namespaced_ingress( - namespace=namespace, body=ingress, name=f"{cluster_name}-ingress" - ) - else: - raise e + route_data["metadata"]["resourceVersion"] = existing_route["metadata"][ + "resourceVersion" + ] + v1_routes.replace(body=route_data) + except dynamic.client.ApiException: + v1_routes.create(body=route_data) diff --git a/tests/unit_test.py b/tests/unit_test.py index ab9e3dcda..a7b5d9a6f 100644 --- a/tests/unit_test.py +++ b/tests/unit_test.py @@ -27,7 +27,7 @@ aw_dir = os.path.expanduser("~/.codeflare/appwrapper/") sys.path.append(str(parent) + "/src") -from kubernetes import client, config +from kubernetes import client, config, dynamic from codeflare_sdk.cluster.awload import AWManager from codeflare_sdk.cluster.cluster import ( Cluster, @@ -91,6 +91,7 @@ read_template, enable_local_interactive, ) +import codeflare_sdk.utils.openshift_oauth as sdk_oauth import openshift from openshift.selector import Selector @@ -110,6 +111,24 @@ fake_res = openshift.Result("fake") +def mock_routes_api(mocker): + mocker.patch.object( + sdk_oauth, + "_route_api_getter", + return_value=MagicMock( + resources=MagicMock( + get=MagicMock( + return_value=MagicMock( + create=MagicMock(), + replace=MagicMock(), + delete=MagicMock(), + ) + ) + ) + ), + ) + + def arg_side_effect(*args): fake_res.high_level_operation = args return fake_res @@ -536,17 +555,14 @@ def test_delete_openshift_oauth_objects(mocker): mocker.patch.object(client.CoreV1Api, "delete_namespaced_service") mocker.patch.object(client.NetworkingV1Api, "delete_namespaced_ingress") mocker.patch.object(client.RbacAuthorizationV1Api, "delete_cluster_role_binding") + mock_routes_api(mocker) delete_openshift_oauth_objects("test-cluster", "test-namespace") - client.CoreV1Api.delete_namespaced_service_account.assert_called_with( name="test-cluster-oauth-proxy", namespace="test-namespace" ) client.CoreV1Api.delete_namespaced_service.assert_called_with( name="test-cluster-oauth", namespace="test-namespace" ) - client.NetworkingV1Api.delete_namespaced_ingress.assert_called_with( - name="test-cluster-ingress", namespace="test-namespace" - ) client.RbacAuthorizationV1Api.delete_cluster_role_binding.assert_called_with( name="test-cluster-rb" ) @@ -2751,7 +2767,6 @@ def test_create_openshift_oauth(mocker: MockerFixture): create_namespaced_service_account = MagicMock() create_cluster_role_binding = MagicMock() create_namespaced_service = MagicMock() - create_namespaced_ingress = MagicMock() mocker.patch.object( client.CoreV1Api, "create_namespaced_service_account", @@ -2765,35 +2780,17 @@ def test_create_openshift_oauth(mocker: MockerFixture): mocker.patch.object( client.CoreV1Api, "create_namespaced_service", create_namespaced_service ) - mocker.patch.object( - client.NetworkingV1Api, "create_namespaced_ingress", create_namespaced_ingress - ) - mocker.patch( - "codeflare_sdk.utils.openshift_oauth._get_api_host", return_value="foo.com" - ) + mock_routes_api(mocker) create_openshift_oauth_objects("foo", "bar") create_ns_sa_args = create_namespaced_service_account.call_args create_crb_args = create_cluster_role_binding.call_args create_ns_serv_args = create_namespaced_service.call_args - create_ns_ingress_args = create_namespaced_ingress.call_args assert ( create_ns_sa_args.kwargs["namespace"] == create_ns_serv_args.kwargs["namespace"] ) - assert ( - create_ns_serv_args.kwargs["namespace"] - == create_ns_ingress_args.kwargs["namespace"] - ) assert isinstance(create_ns_sa_args.kwargs["body"], client.V1ServiceAccount) assert isinstance(create_crb_args.kwargs["body"], client.V1ClusterRoleBinding) assert isinstance(create_ns_serv_args.kwargs["body"], client.V1Service) - assert isinstance(create_ns_ingress_args.kwargs["body"], client.V1Ingress) - assert ( - create_ns_serv_args.kwargs["body"].spec.ports[0].name - == create_ns_ingress_args.kwargs["body"] - .spec.rules[0] - .http.paths[0] - .backend.service.port.name - ) def test_replace_openshift_oauth(mocker: MockerFixture): @@ -2807,9 +2804,6 @@ def test_replace_openshift_oauth(mocker: MockerFixture): create_namespaced_service = MagicMock( side_effect=client.ApiException(reason="Conflict") ) - create_namespaced_ingress = MagicMock( - side_effect=client.ApiException(reason="Conflict") - ) mocker.patch.object( client.CoreV1Api, "create_namespaced_service_account", @@ -2823,16 +2817,10 @@ def test_replace_openshift_oauth(mocker: MockerFixture): mocker.patch.object( client.CoreV1Api, "create_namespaced_service", create_namespaced_service ) - mocker.patch.object( - client.NetworkingV1Api, "create_namespaced_ingress", create_namespaced_ingress - ) - mocker.patch( - "codeflare_sdk.utils.openshift_oauth._get_api_host", return_value="foo.com" - ) + mocker.patch.object(dynamic.ResourceList, "get", return_value=True) replace_namespaced_service_account = MagicMock() replace_cluster_role_binding = MagicMock() replace_namespaced_service = MagicMock() - replace_namespaced_ingress = MagicMock() mocker.patch.object( client.CoreV1Api, "replace_namespaced_service_account", @@ -2846,21 +2834,15 @@ def test_replace_openshift_oauth(mocker: MockerFixture): mocker.patch.object( client.CoreV1Api, "replace_namespaced_service", replace_namespaced_service ) - mocker.patch.object( - client.NetworkingV1Api, "replace_namespaced_ingress", replace_namespaced_ingress - ) + mock_routes_api(mocker) create_openshift_oauth_objects("foo", "bar") replace_namespaced_service_account.assert_called_once() replace_cluster_role_binding.assert_called_once() replace_namespaced_service.assert_called_once() - replace_namespaced_ingress.assert_called_once() def test_gen_app_wrapper_with_oauth(mocker: MockerFixture): mocker.patch("kubernetes.client.ApisApi.get_api_versions") - mocker.patch( - "codeflare_sdk.utils.generate_yaml._get_api_host", return_value="foo.com" - ) mocker.patch( "codeflare_sdk.cluster.cluster.get_current_namespace", return_value="opendatahub",