Skip to content

Commit 8a5ea19

Browse files
use route instead of ingress for oauth endpoint
Signed-off-by: Kevin <kpostlet@redhat.com>
1 parent 0feab0f commit 8a5ea19

File tree

5 files changed

+72
-123
lines changed

5 files changed

+72
-123
lines changed

poetry.lock

Lines changed: 0 additions & 10 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/codeflare_sdk/utils/generate_yaml.py

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,6 @@
2929
from base64 import b64encode
3030
from urllib3.util import parse_url
3131

32-
from .kube_api_helpers import _get_api_host
33-
3432

3533
def read_template(template):
3634
with open(template, "r") as stream:
@@ -557,10 +555,6 @@ def enable_openshift_oauth(user_yaml, cluster_name, namespace):
557555
tls_secret_name = f"{cluster_name}-proxy-tls-secret"
558556
tls_volume_name = "proxy-tls-secret"
559557
port_name = "oauth-proxy"
560-
host = _get_api_host(k8_client)
561-
host = host.replace(
562-
"api.", f"{gen_dashboard_ingress_name(cluster_name)}-{namespace}.apps."
563-
)
564558
oauth_sidecar = _create_oauth_sidecar_object(
565559
namespace,
566560
tls_mount_location,

src/codeflare_sdk/utils/kube_api_helpers.py

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,3 @@ def _kube_api_error_handling(
4747
elif e.reason == "Conflict":
4848
raise FileExistsError(exists_msg)
4949
raise e
50-
51-
52-
def _get_api_host(api_client: client.ApiClient): # pragma: no cover
53-
return parse_url(api_client.configuration.host).host

src/codeflare_sdk/utils/openshift_oauth.py

Lines changed: 48 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -1,47 +1,51 @@
11
from urllib3.util import parse_url
2-
from .generate_yaml import gen_dashboard_ingress_name
3-
from .kube_api_helpers import _get_api_host
4-
from base64 import b64decode
2+
import yaml
53

64
from ..cluster.auth import config_check, api_config_handler
75

86
from kubernetes import client
7+
from kubernetes import dynamic
8+
9+
10+
def _route_api_getter():
11+
return dynamic.DynamicClient(
12+
api_config_handler() or client.ApiClient()
13+
).resources.get(api_version="route.openshift.io/v1", kind="Route")
914

1015

1116
def create_openshift_oauth_objects(cluster_name, namespace):
1217
config_check()
13-
api_client = api_config_handler() or client.ApiClient()
1418
oauth_port = 8443
1519
oauth_sa_name = f"{cluster_name}-oauth-proxy"
1620
tls_secret_name = _gen_tls_secret_name(cluster_name)
1721
service_name = f"{cluster_name}-oauth"
1822
port_name = "oauth-proxy"
19-
host = _get_api_host(api_client)
20-
21-
# replace "^api" with the expected host
22-
host = f"{gen_dashboard_ingress_name(cluster_name)}-{namespace}.apps" + host.lstrip(
23-
"api"
24-
)
2523

26-
_create_or_replace_oauth_sa(namespace, oauth_sa_name, host)
24+
_create_or_replace_oauth_sa(namespace, oauth_sa_name, cluster_name)
2725
_create_or_replace_oauth_service_obj(
2826
cluster_name, namespace, oauth_port, tls_secret_name, service_name, port_name
2927
)
30-
_create_or_replace_oauth_ingress_object(
31-
cluster_name, namespace, service_name, port_name, host
28+
_create_or_replace_oauth_route_object(
29+
cluster_name,
30+
namespace,
31+
service_name,
32+
port_name,
3233
)
3334
_create_or_replace_oauth_rb(cluster_name, namespace, oauth_sa_name)
3435

3536

36-
def _create_or_replace_oauth_sa(namespace, oauth_sa_name, host):
37+
def _create_or_replace_oauth_sa(namespace, oauth_sa_name, cluster_name):
3738
oauth_sa = client.V1ServiceAccount(
3839
api_version="v1",
3940
kind="ServiceAccount",
4041
metadata=client.V1ObjectMeta(
4142
name=oauth_sa_name,
4243
namespace=namespace,
4344
annotations={
44-
"serviceaccounts.openshift.io/oauth-redirecturi.first": f"https://{host}"
45+
"serviceaccounts.openshift.io/oauth-redirectreference.first": '{"kind":"OAuthRedirectReference","apiVersion":"v1","reference":{"kind":"Route","name":"'
46+
+ "ray-dashboard-"
47+
+ cluster_name
48+
+ '"}}'
4549
},
4650
),
4751
)
@@ -98,15 +102,14 @@ def delete_openshift_oauth_objects(cluster_name, namespace):
98102
# for an existing cluster before calling this => the objects should never be deleted twice
99103
oauth_sa_name = f"{cluster_name}-oauth-proxy"
100104
service_name = f"{cluster_name}-oauth"
105+
v1_routes = _route_api_getter()
101106
client.CoreV1Api(api_config_handler()).delete_namespaced_service_account(
102107
name=oauth_sa_name, namespace=namespace
103108
)
104109
client.CoreV1Api(api_config_handler()).delete_namespaced_service(
105110
name=service_name, namespace=namespace
106111
)
107-
client.NetworkingV1Api(api_config_handler()).delete_namespaced_ingress(
108-
name=f"{cluster_name}-ingress", namespace=namespace
109-
)
112+
v1_routes.delete(name=f"ray-dashboard-{cluster_name}", namespace=namespace)
110113
client.RbacAuthorizationV1Api(api_config_handler()).delete_cluster_role_binding(
111114
name=f"{cluster_name}-rb"
112115
)
@@ -161,52 +164,36 @@ def _create_or_replace_oauth_service_obj(
161164
raise e
162165

163166

164-
def _create_or_replace_oauth_ingress_object(
167+
def _create_or_replace_oauth_route_object(
165168
cluster_name: str,
166169
namespace: str,
167170
service_name: str,
168171
port_name: str,
169-
host: str,
170-
) -> client.V1Ingress:
171-
ingress = client.V1Ingress(
172-
api_version="networking.k8s.io/v1",
173-
kind="Ingress",
174-
metadata=client.V1ObjectMeta(
175-
annotations={"route.openshift.io/termination": "passthrough"},
176-
name=f"{cluster_name}-ingress",
177-
namespace=namespace,
178-
),
179-
spec=client.V1IngressSpec(
180-
rules=[
181-
client.V1IngressRule(
182-
host=host,
183-
http=client.V1HTTPIngressRuleValue(
184-
paths=[
185-
client.V1HTTPIngressPath(
186-
backend=client.V1IngressBackend(
187-
service=client.V1IngressServiceBackend(
188-
name=service_name,
189-
port=client.V1ServiceBackendPort(
190-
name=port_name
191-
),
192-
)
193-
),
194-
path_type="ImplementationSpecific",
195-
)
196-
]
197-
),
198-
)
199-
]
200-
),
201-
)
172+
):
173+
route = f"""
174+
apiVersion: route.openshift.io/v1
175+
kind: Route
176+
metadata:
177+
name: ray-dashboard-{cluster_name}
178+
namespace: {namespace}
179+
spec:
180+
port:
181+
targetPort: {port_name}
182+
tls:
183+
termination: passthrough
184+
to:
185+
kind: Service
186+
name: {service_name}
187+
"""
188+
route_data = yaml.safe_load(route)
189+
v1_routes = _route_api_getter()
202190
try:
203-
client.NetworkingV1Api(api_config_handler()).create_namespaced_ingress(
204-
namespace=namespace, body=ingress
191+
existing_route = v1_routes.get(
192+
name=f"ray-dashboard-{cluster_name}", namespace=namespace
205193
)
206-
except client.ApiException as e:
207-
if e.reason == "Conflict":
208-
client.NetworkingV1Api(api_config_handler()).replace_namespaced_ingress(
209-
namespace=namespace, body=ingress, name=f"{cluster_name}-ingress"
210-
)
211-
else:
212-
raise e
194+
route_data["metadata"]["resourceVersion"] = existing_route["metadata"][
195+
"resourceVersion"
196+
]
197+
v1_routes.replace(body=route_data)
198+
except dynamic.client.ApiException:
199+
v1_routes.create(body=route_data)

tests/unit_test.py

Lines changed: 24 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
aw_dir = os.path.expanduser("~/.codeflare/appwrapper/")
2828
sys.path.append(str(parent) + "/src")
2929

30-
from kubernetes import client, config
30+
from kubernetes import client, config, dynamic
3131
from codeflare_sdk.cluster.awload import AWManager
3232
from codeflare_sdk.cluster.cluster import (
3333
Cluster,
@@ -91,6 +91,7 @@
9191
read_template,
9292
enable_local_interactive,
9393
)
94+
import codeflare_sdk.utils.openshift_oauth as sdk_oauth
9495

9596
import openshift
9697
from openshift.selector import Selector
@@ -110,6 +111,24 @@
110111
fake_res = openshift.Result("fake")
111112

112113

114+
def mock_routes_api(mocker):
115+
mocker.patch.object(
116+
sdk_oauth,
117+
"_route_api_getter",
118+
return_value=MagicMock(
119+
resources=MagicMock(
120+
get=MagicMock(
121+
return_value=MagicMock(
122+
create=MagicMock(),
123+
replace=MagicMock(),
124+
delete=MagicMock(),
125+
)
126+
)
127+
)
128+
),
129+
)
130+
131+
113132
def arg_side_effect(*args):
114133
fake_res.high_level_operation = args
115134
return fake_res
@@ -536,17 +555,14 @@ def test_delete_openshift_oauth_objects(mocker):
536555
mocker.patch.object(client.CoreV1Api, "delete_namespaced_service")
537556
mocker.patch.object(client.NetworkingV1Api, "delete_namespaced_ingress")
538557
mocker.patch.object(client.RbacAuthorizationV1Api, "delete_cluster_role_binding")
558+
mock_routes_api(mocker)
539559
delete_openshift_oauth_objects("test-cluster", "test-namespace")
540-
541560
client.CoreV1Api.delete_namespaced_service_account.assert_called_with(
542561
name="test-cluster-oauth-proxy", namespace="test-namespace"
543562
)
544563
client.CoreV1Api.delete_namespaced_service.assert_called_with(
545564
name="test-cluster-oauth", namespace="test-namespace"
546565
)
547-
client.NetworkingV1Api.delete_namespaced_ingress.assert_called_with(
548-
name="test-cluster-ingress", namespace="test-namespace"
549-
)
550566
client.RbacAuthorizationV1Api.delete_cluster_role_binding.assert_called_with(
551567
name="test-cluster-rb"
552568
)
@@ -2751,7 +2767,6 @@ def test_create_openshift_oauth(mocker: MockerFixture):
27512767
create_namespaced_service_account = MagicMock()
27522768
create_cluster_role_binding = MagicMock()
27532769
create_namespaced_service = MagicMock()
2754-
create_namespaced_ingress = MagicMock()
27552770
mocker.patch.object(
27562771
client.CoreV1Api,
27572772
"create_namespaced_service_account",
@@ -2765,35 +2780,17 @@ def test_create_openshift_oauth(mocker: MockerFixture):
27652780
mocker.patch.object(
27662781
client.CoreV1Api, "create_namespaced_service", create_namespaced_service
27672782
)
2768-
mocker.patch.object(
2769-
client.NetworkingV1Api, "create_namespaced_ingress", create_namespaced_ingress
2770-
)
2771-
mocker.patch(
2772-
"codeflare_sdk.utils.openshift_oauth._get_api_host", return_value="foo.com"
2773-
)
2783+
mock_routes_api(mocker)
27742784
create_openshift_oauth_objects("foo", "bar")
27752785
create_ns_sa_args = create_namespaced_service_account.call_args
27762786
create_crb_args = create_cluster_role_binding.call_args
27772787
create_ns_serv_args = create_namespaced_service.call_args
2778-
create_ns_ingress_args = create_namespaced_ingress.call_args
27792788
assert (
27802789
create_ns_sa_args.kwargs["namespace"] == create_ns_serv_args.kwargs["namespace"]
27812790
)
2782-
assert (
2783-
create_ns_serv_args.kwargs["namespace"]
2784-
== create_ns_ingress_args.kwargs["namespace"]
2785-
)
27862791
assert isinstance(create_ns_sa_args.kwargs["body"], client.V1ServiceAccount)
27872792
assert isinstance(create_crb_args.kwargs["body"], client.V1ClusterRoleBinding)
27882793
assert isinstance(create_ns_serv_args.kwargs["body"], client.V1Service)
2789-
assert isinstance(create_ns_ingress_args.kwargs["body"], client.V1Ingress)
2790-
assert (
2791-
create_ns_serv_args.kwargs["body"].spec.ports[0].name
2792-
== create_ns_ingress_args.kwargs["body"]
2793-
.spec.rules[0]
2794-
.http.paths[0]
2795-
.backend.service.port.name
2796-
)
27972794

27982795

27992796
def test_replace_openshift_oauth(mocker: MockerFixture):
@@ -2807,9 +2804,6 @@ def test_replace_openshift_oauth(mocker: MockerFixture):
28072804
create_namespaced_service = MagicMock(
28082805
side_effect=client.ApiException(reason="Conflict")
28092806
)
2810-
create_namespaced_ingress = MagicMock(
2811-
side_effect=client.ApiException(reason="Conflict")
2812-
)
28132807
mocker.patch.object(
28142808
client.CoreV1Api,
28152809
"create_namespaced_service_account",
@@ -2823,16 +2817,10 @@ def test_replace_openshift_oauth(mocker: MockerFixture):
28232817
mocker.patch.object(
28242818
client.CoreV1Api, "create_namespaced_service", create_namespaced_service
28252819
)
2826-
mocker.patch.object(
2827-
client.NetworkingV1Api, "create_namespaced_ingress", create_namespaced_ingress
2828-
)
2829-
mocker.patch(
2830-
"codeflare_sdk.utils.openshift_oauth._get_api_host", return_value="foo.com"
2831-
)
2820+
mocker.patch.object(dynamic.ResourceList, "get", return_value=True)
28322821
replace_namespaced_service_account = MagicMock()
28332822
replace_cluster_role_binding = MagicMock()
28342823
replace_namespaced_service = MagicMock()
2835-
replace_namespaced_ingress = MagicMock()
28362824
mocker.patch.object(
28372825
client.CoreV1Api,
28382826
"replace_namespaced_service_account",
@@ -2846,21 +2834,15 @@ def test_replace_openshift_oauth(mocker: MockerFixture):
28462834
mocker.patch.object(
28472835
client.CoreV1Api, "replace_namespaced_service", replace_namespaced_service
28482836
)
2849-
mocker.patch.object(
2850-
client.NetworkingV1Api, "replace_namespaced_ingress", replace_namespaced_ingress
2851-
)
2837+
mock_routes_api(mocker)
28522838
create_openshift_oauth_objects("foo", "bar")
28532839
replace_namespaced_service_account.assert_called_once()
28542840
replace_cluster_role_binding.assert_called_once()
28552841
replace_namespaced_service.assert_called_once()
2856-
replace_namespaced_ingress.assert_called_once()
28572842

28582843

28592844
def test_gen_app_wrapper_with_oauth(mocker: MockerFixture):
28602845
mocker.patch("kubernetes.client.ApisApi.get_api_versions")
2861-
mocker.patch(
2862-
"codeflare_sdk.utils.generate_yaml._get_api_host", return_value="foo.com"
2863-
)
28642846
mocker.patch(
28652847
"codeflare_sdk.cluster.cluster.get_current_namespace",
28662848
return_value="opendatahub",

0 commit comments

Comments
 (0)