Skip to content

Commit 4438c78

Browse files
committed
use route instead of ingress for oauth endpoint
Signed-off-by: Kevin <kpostlet@redhat.com>
1 parent f509ae1 commit 4438c78

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:
@@ -539,10 +537,6 @@ def enable_openshift_oauth(user_yaml, cluster_name, namespace):
539537
tls_secret_name = f"{cluster_name}-proxy-tls-secret"
540538
tls_volume_name = "proxy-tls-secret"
541539
port_name = "oauth-proxy"
542-
host = _get_api_host(k8_client)
543-
host = host.replace(
544-
"api.", f"{gen_dashboard_ingress_name(cluster_name)}-{namespace}.apps."
545-
)
546540
oauth_sidecar = _create_oauth_sidecar_object(
547541
namespace,
548542
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,
@@ -90,6 +90,7 @@
9090
read_template,
9191
enable_local_interactive,
9292
)
93+
import codeflare_sdk.utils.openshift_oauth as sdk_oauth
9394

9495
import openshift
9596
from openshift.selector import Selector
@@ -109,6 +110,24 @@
109110
fake_res = openshift.Result("fake")
110111

111112

113+
def mock_routes_api(mocker):
114+
mocker.patch.object(
115+
sdk_oauth,
116+
"_route_api_getter",
117+
return_value=MagicMock(
118+
resources=MagicMock(
119+
get=MagicMock(
120+
return_value=MagicMock(
121+
create=MagicMock(),
122+
replace=MagicMock(),
123+
delete=MagicMock(),
124+
)
125+
)
126+
)
127+
),
128+
)
129+
130+
112131
def arg_side_effect(*args):
113132
fake_res.high_level_operation = args
114133
return fake_res
@@ -535,17 +554,14 @@ def test_delete_openshift_oauth_objects(mocker):
535554
mocker.patch.object(client.CoreV1Api, "delete_namespaced_service")
536555
mocker.patch.object(client.NetworkingV1Api, "delete_namespaced_ingress")
537556
mocker.patch.object(client.RbacAuthorizationV1Api, "delete_cluster_role_binding")
557+
mock_routes_api(mocker)
538558
delete_openshift_oauth_objects("test-cluster", "test-namespace")
539-
540559
client.CoreV1Api.delete_namespaced_service_account.assert_called_with(
541560
name="test-cluster-oauth-proxy", namespace="test-namespace"
542561
)
543562
client.CoreV1Api.delete_namespaced_service.assert_called_with(
544563
name="test-cluster-oauth", namespace="test-namespace"
545564
)
546-
client.NetworkingV1Api.delete_namespaced_ingress.assert_called_with(
547-
name="test-cluster-ingress", namespace="test-namespace"
548-
)
549565
client.RbacAuthorizationV1Api.delete_cluster_role_binding.assert_called_with(
550566
name="test-cluster-rb"
551567
)
@@ -2675,7 +2691,6 @@ def test_create_openshift_oauth(mocker: MockerFixture):
26752691
create_namespaced_service_account = MagicMock()
26762692
create_cluster_role_binding = MagicMock()
26772693
create_namespaced_service = MagicMock()
2678-
create_namespaced_ingress = MagicMock()
26792694
mocker.patch.object(
26802695
client.CoreV1Api,
26812696
"create_namespaced_service_account",
@@ -2689,35 +2704,17 @@ def test_create_openshift_oauth(mocker: MockerFixture):
26892704
mocker.patch.object(
26902705
client.CoreV1Api, "create_namespaced_service", create_namespaced_service
26912706
)
2692-
mocker.patch.object(
2693-
client.NetworkingV1Api, "create_namespaced_ingress", create_namespaced_ingress
2694-
)
2695-
mocker.patch(
2696-
"codeflare_sdk.utils.openshift_oauth._get_api_host", return_value="foo.com"
2697-
)
2707+
mock_routes_api(mocker)
26982708
create_openshift_oauth_objects("foo", "bar")
26992709
create_ns_sa_args = create_namespaced_service_account.call_args
27002710
create_crb_args = create_cluster_role_binding.call_args
27012711
create_ns_serv_args = create_namespaced_service.call_args
2702-
create_ns_ingress_args = create_namespaced_ingress.call_args
27032712
assert (
27042713
create_ns_sa_args.kwargs["namespace"] == create_ns_serv_args.kwargs["namespace"]
27052714
)
2706-
assert (
2707-
create_ns_serv_args.kwargs["namespace"]
2708-
== create_ns_ingress_args.kwargs["namespace"]
2709-
)
27102715
assert isinstance(create_ns_sa_args.kwargs["body"], client.V1ServiceAccount)
27112716
assert isinstance(create_crb_args.kwargs["body"], client.V1ClusterRoleBinding)
27122717
assert isinstance(create_ns_serv_args.kwargs["body"], client.V1Service)
2713-
assert isinstance(create_ns_ingress_args.kwargs["body"], client.V1Ingress)
2714-
assert (
2715-
create_ns_serv_args.kwargs["body"].spec.ports[0].name
2716-
== create_ns_ingress_args.kwargs["body"]
2717-
.spec.rules[0]
2718-
.http.paths[0]
2719-
.backend.service.port.name
2720-
)
27212718

27222719

27232720
def test_replace_openshift_oauth(mocker: MockerFixture):
@@ -2731,9 +2728,6 @@ def test_replace_openshift_oauth(mocker: MockerFixture):
27312728
create_namespaced_service = MagicMock(
27322729
side_effect=client.ApiException(reason="Conflict")
27332730
)
2734-
create_namespaced_ingress = MagicMock(
2735-
side_effect=client.ApiException(reason="Conflict")
2736-
)
27372731
mocker.patch.object(
27382732
client.CoreV1Api,
27392733
"create_namespaced_service_account",
@@ -2747,16 +2741,10 @@ def test_replace_openshift_oauth(mocker: MockerFixture):
27472741
mocker.patch.object(
27482742
client.CoreV1Api, "create_namespaced_service", create_namespaced_service
27492743
)
2750-
mocker.patch.object(
2751-
client.NetworkingV1Api, "create_namespaced_ingress", create_namespaced_ingress
2752-
)
2753-
mocker.patch(
2754-
"codeflare_sdk.utils.openshift_oauth._get_api_host", return_value="foo.com"
2755-
)
2744+
mocker.patch.object(dynamic.ResourceList, "get", return_value=True)
27562745
replace_namespaced_service_account = MagicMock()
27572746
replace_cluster_role_binding = MagicMock()
27582747
replace_namespaced_service = MagicMock()
2759-
replace_namespaced_ingress = MagicMock()
27602748
mocker.patch.object(
27612749
client.CoreV1Api,
27622750
"replace_namespaced_service_account",
@@ -2770,21 +2758,15 @@ def test_replace_openshift_oauth(mocker: MockerFixture):
27702758
mocker.patch.object(
27712759
client.CoreV1Api, "replace_namespaced_service", replace_namespaced_service
27722760
)
2773-
mocker.patch.object(
2774-
client.NetworkingV1Api, "replace_namespaced_ingress", replace_namespaced_ingress
2775-
)
2761+
mock_routes_api(mocker)
27762762
create_openshift_oauth_objects("foo", "bar")
27772763
replace_namespaced_service_account.assert_called_once()
27782764
replace_cluster_role_binding.assert_called_once()
27792765
replace_namespaced_service.assert_called_once()
2780-
replace_namespaced_ingress.assert_called_once()
27812766

27822767

27832768
def test_gen_app_wrapper_with_oauth(mocker: MockerFixture):
27842769
mocker.patch("kubernetes.client.ApisApi.get_api_versions")
2785-
mocker.patch(
2786-
"codeflare_sdk.utils.generate_yaml._get_api_host", return_value="foo.com"
2787-
)
27882770
mocker.patch(
27892771
"codeflare_sdk.cluster.cluster.get_current_namespace",
27902772
return_value="opendatahub",

0 commit comments

Comments
 (0)