Skip to content

Commit

Permalink
Remove creation of OAuth resources/logic and remove openshift_oauth o…
Browse files Browse the repository at this point in the history
…ption (#480)

* Remove creation of OAuth resources/logic and add annotation

* Remove openshift_oauth configuration

* Add verify_tls to ClusterConfiguration
  • Loading branch information
ChristianZaccaria authored Apr 3, 2024
1 parent 1497434 commit 403cca6
Show file tree
Hide file tree
Showing 7 changed files with 46 additions and 341 deletions.
48 changes: 19 additions & 29 deletions src/codeflare_sdk/cluster/cluster.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,7 @@
)
from ..utils.kube_api_helpers import _kube_api_error_handling
from ..utils.generate_yaml import is_openshift_cluster
from ..utils.openshift_oauth import (
create_openshift_oauth_objects,
delete_openshift_oauth_objects,
)

from .config import ClusterConfiguration
from .model import (
AppWrapper,
Expand Down Expand Up @@ -86,14 +83,16 @@ def _client_headers(self):

@property
def _client_verify_tls(self):
return not self.config.openshift_oauth
if not is_openshift_cluster or not self.config.verify_tls:
return False
return True

@property
def job_client(self):
k8client = api_config_handler() or client.ApiClient()
if self._job_submission_client:
return self._job_submission_client
if self.config.openshift_oauth:
if is_openshift_cluster():
print(k8client.configuration.get_api_key_with_prefix("authorization"))
self._job_submission_client = JobSubmissionClient(
self.cluster_dashboard_uri(),
Expand Down Expand Up @@ -191,6 +190,7 @@ def create_app_wrapper(self):
ingress_domain = self.config.ingress_domain
ingress_options = self.config.ingress_options
write_to_file = self.config.write_to_file
verify_tls = self.config.verify_tls
return generate_appwrapper(
name=name,
namespace=namespace,
Expand All @@ -213,10 +213,10 @@ def create_app_wrapper(self):
image_pull_secrets=image_pull_secrets,
dispatch_priority=dispatch_priority,
priority_val=priority_val,
openshift_oauth=self.config.openshift_oauth,
ingress_domain=ingress_domain,
ingress_options=ingress_options,
write_to_file=write_to_file,
verify_tls=verify_tls,
)

# creates a new cluster with the provided or default spec
Expand All @@ -226,10 +226,6 @@ def up(self):
the MCAD queue.
"""
namespace = self.config.namespace
if self.config.openshift_oauth:
create_openshift_oauth_objects(
cluster_name=self.config.name, namespace=namespace
)

try:
config_check()
Expand Down Expand Up @@ -281,11 +277,6 @@ def down(self):
except Exception as e: # pragma: no cover
return _kube_api_error_handling(e)

if self.config.openshift_oauth:
delete_openshift_oauth_objects(
cluster_name=self.config.name, namespace=namespace
)

def status(
self, print_to_console: bool = True
) -> Tuple[CodeFlareClusterStatus, bool]:
Expand Down Expand Up @@ -500,26 +491,21 @@ def torchx_config(
return to_return

def from_k8_cluster_object(
rc, mcad=True, ingress_domain=None, ingress_options={}, write_to_file=False
rc,
mcad=True,
ingress_domain=None,
ingress_options={},
write_to_file=False,
verify_tls=True,
):
config_check()
openshift_oauth = False
if (
rc["metadata"]["annotations"]["sdk.codeflare.dev/local_interactive"]
== "True"
):
local_interactive = True
else:
local_interactive = False
if "codeflare.dev/oauth" in rc["metadata"]["annotations"]:
openshift_oauth = (
rc["metadata"]["annotations"]["codeflare.dev/oauth"] == "True"
)
else:
for container in rc["spec"]["headGroupSpec"]["template"]["spec"][
"containers"
]:
openshift_oauth = "oauth-proxy" in container["name"]
machine_types = (
rc["metadata"]["labels"]["orderedinstance"].split("_")
if "orderedinstance" in rc["metadata"]["labels"]
Expand Down Expand Up @@ -570,7 +556,7 @@ def from_k8_cluster_object(
ingress_domain=ingress_domain,
ingress_options=ingress_options,
write_to_file=write_to_file,
openshift_oauth=openshift_oauth,
verify_tls=verify_tls,
)
return Cluster(cluster_config)

Expand Down Expand Up @@ -655,7 +641,10 @@ def get_current_namespace(): # pragma: no cover


def get_cluster(
cluster_name: str, namespace: str = "default", write_to_file: bool = False
cluster_name: str,
namespace: str = "default",
write_to_file: bool = False,
verify_tls: bool = True,
):
try:
config_check()
Expand Down Expand Up @@ -729,6 +718,7 @@ def get_cluster(
ingress_domain=ingress_domain,
ingress_options=ingress_options,
write_to_file=write_to_file,
verify_tls=verify_tls,
)
raise FileNotFoundError(
f"Cluster {cluster_name} is not found in {namespace} namespace"
Expand Down
8 changes: 7 additions & 1 deletion src/codeflare_sdk/cluster/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,13 @@ class ClusterConfiguration:
local_interactive: bool = False
image_pull_secrets: list = field(default_factory=list)
dispatch_priority: str = None
openshift_oauth: bool = False # NOTE: to use the user must have permission to create a RoleBinding for system:auth-delegator
ingress_options: dict = field(default_factory=dict)
ingress_domain: str = None
write_to_file: bool = False
verify_tls: bool = True

def __post_init__(self):
if not self.verify_tls:
print(
"Warning: TLS verification has been disabled - Endpoint checks will be bypassed"
)
23 changes: 17 additions & 6 deletions src/codeflare_sdk/utils/generate_yaml.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,20 @@ def update_names(yaml, item, appwrapper_name, cluster_name, namespace):
lower_meta["labels"]["workload.codeflare.dev/appwrapper"] = appwrapper_name
lower_meta["name"] = cluster_name
lower_meta["namespace"] = namespace
lower_spec = item.get("generictemplate", {}).get("spec")
if is_openshift_cluster():
cookie_secret_env_var = {
"name": "COOKIE_SECRET",
"valueFrom": {
"secretKeyRef": {
"key": "cookie_secret",
"name": f"{cluster_name}-oauth-config",
}
},
}
lower_spec["headGroupSpec"]["template"]["spec"]["containers"][0]["env"].append(
cookie_secret_env_var
)


def update_labels(yaml, instascale, instance_types):
Expand Down Expand Up @@ -585,9 +599,6 @@ def enable_openshift_oauth(user_yaml, cluster_name, namespace):
)
# allows for setting value of Cluster object when initializing object from an existing AppWrapper on cluster
user_yaml["metadata"]["annotations"] = user_yaml["metadata"].get("annotations", {})
user_yaml["metadata"]["annotations"][
"codeflare-sdk-use-oauth"
] = "true" # if the user gets an
ray_headgroup_pod = user_yaml["spec"]["resources"]["GenericItems"][0][
"generictemplate"
]["spec"]["headGroupSpec"]["template"]["spec"]
Expand Down Expand Up @@ -620,7 +631,7 @@ def _create_oauth_sidecar_object(
"--upstream=http://localhost:8265",
f"--tls-cert={tls_mount_location}/tls.crt",
f"--tls-key={tls_mount_location}/tls.key",
f"--cookie-secret={b64encode(urandom(64)).decode('utf-8')}", # create random string for encrypting cookie
"--cookie-secret=$(COOKIE_SECRET)",
f'--openshift-delegate-urls={{"/":{{"resource":"pods","namespace":"{namespace}","verb":"get"}}}}',
],
image="registry.redhat.io/openshift4/ose-oauth-proxy@sha256:1ea6a01bf3e63cdcf125c6064cbd4a4a270deaf0f157b3eabb78f60556840366",
Expand Down Expand Up @@ -696,10 +707,10 @@ def generate_appwrapper(
image_pull_secrets: list,
dispatch_priority: str,
priority_val: int,
openshift_oauth: bool,
ingress_domain: str,
ingress_options: dict,
write_to_file: bool,
verify_tls: bool,
):
user_yaml = read_template(template)
appwrapper_name, cluster_name = gen_names(name)
Expand Down Expand Up @@ -757,7 +768,7 @@ def generate_appwrapper(

delete_route_or_ingress(resources["resources"])

if openshift_oauth:
if is_openshift_cluster():
enable_openshift_oauth(user_yaml, cluster_name, namespace)

directory_path = os.path.expanduser("~/.codeflare/appwrapper/")
Expand Down
Loading

0 comments on commit 403cca6

Please sign in to comment.