Skip to content

Remove creation of OAuth resources/logic and remove openshift_oauth option #480

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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