Support HTTPS calls to clusters #114

jlewitt1 · 2023-09-28T21:51:24Z

Adds option for starting up the Runhouse API server on the cluster with HTTPS. This makes it incredibly fast and easy to stand up a microservice with standard bearer token authentication (using a Runhouse token), allowing users to share Runhouse resources with collaborators, teams, customers, etc.

Highlights:

New server connection types:

ssh: Connects to the cluster via an SSH tunnel.
tls: Connects to the cluster via HTTPS (port 443) and enforces verification via TLS certificates. Only users with a valid cert will be able to make requests to the API server.
none: Does not use any port forwarding or enforce any authentication. Connects to the cluster via HTTP (port 80).
aws_ssm: Uses the AWS Systems Manager to
create an SSH tunnel to the cluster. Note: this is currently only relevant for SageMaker Clusters
paramiko: Uses Paramiko to create an SSH tunnel to the cluster. This is relevant if you are using a cluster which require existing credentials (e.g. a password)

The API server will be started by default on port 32300.

Validating resource access on the cluster:

To start will apply general auth based on the Runhouse token provided in the request. When enabled requesting user must have access to the cluster (as saved in Den)
When executing a function, user must have access to the underlying function resource

NGINX (optional):

The Runhouse API server (a Fast API app) will by default run on a higher, non-privileged port (32300). Nginx will run in front of uvicorn and serve as a reverse proxy to forward requests from port 80 (default for HTTP) or port 443 (default for HTTPS) to the API server's port.

TODOs:

Add lots more tests (testing certs, performance, server connection types, den auth, nginx)
Smooth over flow of setting up nginx on the cluster when relevant
Implement Paramiko server connection logic

…based on their RH token

…ster, for function calls validate read or write

dongreenberg · 2023-10-12T07:35:28Z

docs/auth_and_data_collection.rst

+===========================
+By default, Runhouse collects metadata from provisioned clusters and data relating to performance and error monitoring.
+This data will only be used by Runhouse to improve the product.
+No Personal Identifiable Information (PII) is collected and we will not sell or buy data about you.


I would remove this line, it's more complex than it seems.

dongreenberg · 2023-10-12T07:37:56Z

docs/auth_and_data_collection.rst

+Runhouse provides a couple of options to manage the connection to the Runhouse API server running on a cluster.
+
+
+API Server Connection


To me everything that follows this should be in the Cluster docs, and everything before we already cover in the data collection doc. What do you think of removing this file and just putting the info below into the Cluster docs?

Oh I see, you added the auth into here. I don't think they're really the same, so I would revert it (but still remove the line above), and just add the new auth info into the Cluster docs.

dongreenberg · 2023-10-12T07:43:54Z

runhouse/resources/hardware/cluster.py

 class Cluster(Resource):
    RESOURCE_TYPE = "cluster"
    REQUEST_TIMEOUT = 5  # seconds
+    DEFAULT_HOST = "127.0.0.1"


Worth noting that if the connection type is tls or none (i.e. the calls are over HTTP alone and not a tunnel, the server will need to start with a host of 0.0.0.0. So maybe we should just make that the default, and only use localhost when the user starts with connection of ssh.

dongreenberg · 2023-10-12T07:44:09Z

runhouse/resources/hardware/cluster.py

 class Cluster(Resource):
    RESOURCE_TYPE = "cluster"
    REQUEST_TIMEOUT = 5  # seconds
+    DEFAULT_HOST = "127.0.0.1"
+    DEFAULT_HTTP_PORT = 50052


Should we take the opportunity to change this now?

runhouse/resources/hardware/cluster.py

dongreenberg · 2023-10-12T19:34:23Z

runhouse/servers/http/http_server.py

+    @wraps(func)
+    async def wrapper(*args, **kwargs):
+        request: Request = kwargs.get("request")
+        is_https: bool = request.url.scheme == "https"


Won't this mean the uesr can bypass auth if they just send a regular http request?

runhouse/servers/http/http_server.py

dongreenberg · 2023-10-12T19:45:53Z

runhouse/servers/http/http_server.py

+    DEN_AUTH = False
    memory_exporter = None

+    # NOTE: This is a temp in-mem cache, we will move this out into the object store for future permissions support
+    AUTH_CACHE = {}
+


Don't love using the class methods this way... but I don't know why. I think globals may be a bit safer.

dongreenberg · 2023-10-12T19:46:35Z

runhouse/servers/http/http_server.py

+    @classmethod
+    def get(cls, key) -> dict:
+        """Get resources associated with a particular user"""
+        return cls.AUTH_CACHE.get(key, {})
+
+    @classmethod
+    def put(cls, key, value):
+        """Update server cache with a user's resources and access type"""
+        cls.AUTH_CACHE[key] = value


Confusing for these to collide with getting and putting resources

dongreenberg · 2023-10-12T19:50:25Z

runhouse/servers/http/http_server.py

+    @staticmethod
+    @app.get("/cert")
+    @validate_user
+    def get_cert(request: Request, message: Message):


Wait... is it a problem to be sending the cert over HTTP? Also, is it a problem to be sending the user's token over HTTP before they have the cert?

DenisYay · 2023-10-13T17:22:34Z

docs/auth_and_data_collection.rst

+
+Removing Collected Data
+------------------------------------
+If you would like us to remove your collected data, please contact us.


should we provide a link for contact (e.g. an email address or a contact form)?

DenisYay · 2023-10-13T17:26:15Z

docs/auth_and_data_collection.rst

+The below options can be specified with the ``server_connection_type`` parameter
+when :ref:`initializing a cluster <Cluster Factory Method>`:
+
+- ``ssh``: Connects to the cluster via port forwarding. The API server will be started with HTTP.


Do we need to say "on port 80", given our conversation earlier today?

DenisYay · 2023-10-13T17:26:40Z

docs/auth_and_data_collection.rst

+
+- ``ssh``: Connects to the cluster via port forwarding. The API server will be started with HTTP.
+- ``tls``: Connects to the cluster via port forwarding and enforces verification via TLS certificates. The API server
+  will be started with HTTPS. Only users with a valid cert will be able to make requests to the API server.


"... on port 443"
?

DenisYay · 2023-10-13T17:26:54Z

docs/auth_and_data_collection.rst

+- ``tls``: Connects to the cluster via port forwarding and enforces verification via TLS certificates. The API server
+  will be started with HTTPS. Only users with a valid cert will be able to make requests to the API server.
+- ``none``: Does not use any port forwarding or enforce any authentication. The API server will be started
+  with HTTP.


DenisYay · 2023-10-13T17:28:45Z

docs/auth_and_data_collection.rst

+to enable token authentication. Runhouse will handle adding the token to each subsequent request as an auth header with
+format: :code:`{"Authorization": "Bearer <token>"}`
+
+Enabling TLS and Den Auth for the API server makes it incredibly fast and easy to stand up a microservice with


Should we explicitly mention "Runhouse Den Auth" for folks who are dropping in just for this section / Google searchability?

DenisYay · 2023-10-13T17:31:12Z

docs/index.rst

@@ -82,7 +82,7 @@ Table of Contents

   debugging_logging
   troubleshooting
-   data_collection
+   auth_and_data_collection
   Source Code <https://github.com/run-house/runhouse>
   REST API Guide <https://api.run.house/docs>
   Dashboard <https://www.run.house/dashboard>


Runhouse Den Dashboard?

DenisYay · 2023-10-13T17:34:44Z

runhouse/main.py

+    restart_ray,
+    screen,
+    create_logfile=True,
+    host=None,


Do we need to add a "port" parameter here and elsewhere in this file, or would this be handled outside of this PR?

DenisYay · 2023-10-13T17:40:21Z

runhouse/resources/hardware/cluster.py


 logger = logging.getLogger(__name__)


+class ServerConnectionType(Enum):
+    """Enum to manage the type of connection Runhouse will make with the Runhouse API server started on the cluster.
+    ``ssh``: Use port forwarding to connect to the server.


"The API server will be started with HTTP."
?

DenisYay · 2023-10-13T17:43:16Z

runhouse/resources/hardware/cluster_factory.py

@@ -24,6 +28,15 @@ def cluster(
        host (str or List[str], optional): Hostname, IP address, or list of IP addresses for the BYO cluster.
        ssh_creds (dict, optional): Dictionary mapping SSH credentials.
            Example: ``ssh_creds={'ssh_user': '...', 'ssh_private_key':'<path_to_key>'}``
+        server_port (bool, optional): Port to use for the server. (Default: ``50052``).


flagging both lines for a potential change given our conversation earlier today.

…ttp server, using nginx as a reverse proxy

…art command needed for non ssh server connections

…d,den auth flow

# Conflicts: # runhouse/resources/hardware/cluster.py # runhouse/servers/http/http_client.py # runhouse/servers/http/http_utils.py

…put and waiting for successful Uvicorn start line. Much better visibility when server fails to start properly! - Print server logfile output to terminal on `runhouse restart`, even when screen is enabled. - Fix https function and cluster fixtures - Change check to not require auth or Ray status, just send a ping - Fix async bug with HTTP server

dongreenberg · 2023-10-25T11:25:07Z

docs/api/python/cluster.rst

+- ``tls``: Connects to the cluster via HTTPS (by default on port :code:`443`) and enforces verification via TLS
+  certificates. Only users with a valid cert will be able to make requests to the API server.


This is out of date. Should just say "Connects to the cluster via HTTPS (by default on port :code:443) using either a provided certificate, or creating a new self-signed certificate just for this cluster. You must open the needed ports in the firewall, such as via the open_ports argument in the OnDemandCluster, or manually in the compute itself or cloud console.

dongreenberg · 2023-10-25T11:29:35Z

docs/api/python/cluster.rst

+- ``none``: Does not use any port forwarding or enforce any authentication. Connects to the cluster with HTTP by
+  default on port :code:`80`.


Might be good to add: "This is useful when connecting to a cluster within a VPC, or creating a tunnel manually on the side with custom settings."

dongreenberg · 2023-10-25T11:30:24Z

docs/api/python/cluster.rst

+- ``paramiko``: Uses `Paramiko <https://www.paramiko.org/>`_ to create an SSH tunnel to the cluster. This
+  is relevant if you are using a cluster which require existing credentials (e.g. a password).


Should just say "using a cluster which requires a password to authenticate."

dongreenberg · 2023-10-25T11:32:49Z

docs/api/python/cluster.rst

+    The ``tls`` connection type is the most secure and is recommended for production use if you are not running inside
+    of a VPC.


Suggested change

The ``tls`` connection type is the most secure and is recommended for production use if you are not running inside

of a VPC.

The ``tls`` connection type is the most secure and is recommended for production use if you are not running inside

of a VPC. However, be mindful that you must secure the cluster with authentication (see below) if you open it to the public internet.

dongreenberg · 2023-10-25T11:44:23Z

docs/api/python/cluster.rst

+Runhouse allows you to authenticate users via their Runhouse token (generated when
+:ref:`logging in <Login/Logout>`) and saved to local Runhouse configs in path: :code:`~/.rh/config.yaml`.


Suggested change

Runhouse allows you to authenticate users via their Runhouse token (generated when

:ref:`logging in <Login/Logout>`) and saved to local Runhouse configs in path: :code:`~/.rh/config.yaml`.

If desired, Runhouse provides out-of-the-box authentication via users' Runhouse token (generated when

:ref:`logging in <Login/Logout>` and set locally at: :code:`~/.rh/config.yaml`). This is crucial if the cluster has ports open to the public internet, as would usually be the case when using the ``tls`` connection type. You may also set up your own authentication manually inside of your own code, but you should likely still enable Runhouse authentication to ensure that even your non-user-facing endpoints into the server are secured.

dongreenberg · 2023-10-25T12:34:28Z

runhouse/servers/http/http_server.py

+                    detail=f"Failed to validate Runhouse user: {load_resp_content(resp)}",
+                )
+
+        if use_den_auth or func_call:


This should just be if func_call, if use_den_auth is false we return above

And I don't see how we're validating auth into the cluster for the non-func_call case

dongreenberg · 2023-10-25T12:37:54Z

runhouse/servers/http/http_server.py

+def _load_current_cluster(kwargs) -> Union[str, None]:
+    current_cluster = _get_cluster_from(_current_cluster("config"))
+    if current_cluster:
+        return current_cluster.rns_address
+
+    # If no cluster config saved yet on the cluster try getting the cluster uri from the message object
+    # included in the request
+    message: Message = kwargs.get("message")
+    resource = json.loads(message.data)
+    return resource.get("name")
+


I don't think we need this anymore?

dongreenberg · 2023-10-25T12:39:19Z

runhouse/servers/http/http_server.py

@@ -225,7 +372,8 @@ def lookup_env_for_name(name, check_rns=False):

    @staticmethod
    @app.post("/resource")
-    def put_resource(message: Message):
+    @validate_user
+    def put_resource(request: Request, message: Message):


Why are we adding Request objects here?

dongreenberg · 2023-10-25T12:43:29Z

tests/conftest.py

+        .save()
+    )
+
+    c.restart_server()


Why do we restart the server here?

dongreenberg · 2023-10-25T12:45:04Z

tests/test_clusters/test_cluster.py

+
+    configs.set("token", "abcd123")
+
+    try:


We need to try calling a bunch of other cluster methods with a bad token too, not just a function call

…d new pytest fixture for testing sharing resources

…reate & share resources. update auth logic in object store

# Conflicts: # runhouse/resources/hardware/cluster.py

jlewitt1 marked this pull request as draft September 28, 2023 21:59

jlewitt1 force-pushed the cluster-auth branch 7 times, most recently from 7c1f6c9 to c9f6780 Compare October 1, 2023 18:58

jlewitt1 changed the title ~~[WIP] Support HTTPS calls to clusters~~ Support HTTPS calls to clusters Oct 3, 2023

jlewitt1 marked this pull request as ready for review October 4, 2023 07:34

jlewitt1 force-pushed the cluster-auth branch 7 times, most recently from 324cb6d to 54bdea7 Compare October 10, 2023 16:37

jlewitt1 force-pushed the cluster-auth branch 2 times, most recently from d498520 to 99d7ef3 Compare October 12, 2023 08:26

jlewitt1 added 2 commits October 12, 2023 11:38

add option to use HTTPS server on the cluster, which validates users …

b8f00d0

…based on their RH token

update den auth handling: for all routes validate write access to clu…

145de09

…ster, for function calls validate read or write

jlewitt1 force-pushed the cluster-auth branch from 99d7ef3 to 145de09 Compare October 12, 2023 08:39

linting + update tests

553cc31

dongreenberg reviewed Oct 13, 2023

View reviewed changes

DenisYay reviewed Oct 13, 2023

View reviewed changes

jlewitt1 added 2 commits October 15, 2023 23:40

PR updates: server connection type options, added args for starting h…

4bdadda

…ttp server, using nginx as a reverse proxy

more PR fixes. add nginx config manager. add more args to server rest…

ed5e19f

…art command needed for non ssh server connections

jlewitt1 force-pushed the cluster-auth branch 2 times, most recently from cd875d6 to d9bf096 Compare October 18, 2023 08:34

jlewitt1 force-pushed the cluster-auth branch from 94a0578 to 0255048 Compare October 18, 2023 14:05

jlewitt1 added 3 commits October 20, 2023 16:14

update check server, nginx configs, cluster factories, restart comman…

3b4b397

…d,den auth flow

update ufw rule for nginx and update client log streaming handler

6cdbae5

Merge remote-tracking branch 'origin/main' into cluster-auth

7e8dd5a

# Conflicts: # runhouse/resources/hardware/cluster.py # runhouse/servers/http/http_client.py # runhouse/servers/http/http_utils.py

jlewitt1 force-pushed the cluster-auth branch 2 times, most recently from 68bc94d to 8fbe7c5 Compare October 24, 2023 08:57

sagemaker updates, start adding performance tests, new pytest fixtures.

d41b822

jlewitt1 force-pushed the cluster-auth branch from 8fbe7c5 to d41b822 Compare October 24, 2023 09:03

jlewitt1 force-pushed the cluster-auth branch 2 times, most recently from 2a78242 to fc2cd96 Compare October 25, 2023 09:13

update cert data saved to cluster config

7bf8f60

jlewitt1 force-pushed the cluster-auth branch from fc2cd96 to 7bf8f60 Compare October 25, 2023 10:04

dongreenberg reviewed Oct 25, 2023

View reviewed changes

Bug fixes

d980b9f

jlewitt1 force-pushed the cluster-auth branch from dc05a72 to cdce735 Compare October 25, 2023 21:41

PR updates: docs, file structure, cluster factory

a586afd

jlewitt1 force-pushed the cluster-auth branch from cdce735 to a586afd Compare October 25, 2023 21:45

jlewitt1 added 2 commits October 26, 2023 16:16

Auth Cluster Cache (#125)

c44993c

update validator logic and fix object store function access check. Ad…

f7d0d01

…d new pytest fixture for testing sharing resources

jlewitt1 force-pushed the cluster-auth branch 5 times, most recently from ad5842c to 261e1fb Compare November 2, 2023 09:14

update sharing tests, add context manager for using test account to c…

ce76d5e

…reate & share resources. update auth logic in object store

jlewitt1 force-pushed the cluster-auth branch from 261e1fb to ce76d5e Compare November 2, 2023 15:08

Merge remote-tracking branch 'origin/main' into cluster-auth

aa24289

# Conflicts: # runhouse/resources/hardware/cluster.py

jlewitt1 merged commit f9adbcc into main Nov 2, 2023
5 checks passed

jlewitt1 deleted the cluster-auth branch November 4, 2023 18:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support HTTPS calls to clusters #114

Support HTTPS calls to clusters #114

jlewitt1 commented Sep 28, 2023 •

edited

Loading

dongreenberg Oct 12, 2023

dongreenberg Oct 12, 2023

dongreenberg Oct 12, 2023

dongreenberg Oct 12, 2023

dongreenberg Oct 12, 2023

dongreenberg Oct 12, 2023

dongreenberg Oct 12, 2023

dongreenberg Oct 12, 2023

dongreenberg Oct 12, 2023

DenisYay Oct 13, 2023

DenisYay Oct 13, 2023

DenisYay Oct 13, 2023

DenisYay Oct 13, 2023

DenisYay Oct 13, 2023

DenisYay Oct 13, 2023

DenisYay Oct 13, 2023

DenisYay Oct 13, 2023

DenisYay Oct 13, 2023

dongreenberg Oct 25, 2023

dongreenberg Oct 25, 2023

dongreenberg Oct 25, 2023

dongreenberg Oct 25, 2023

dongreenberg Oct 25, 2023

dongreenberg Oct 25, 2023

dongreenberg Oct 25, 2023

dongreenberg Oct 25, 2023

dongreenberg Oct 25, 2023

dongreenberg Oct 25, 2023

dongreenberg Oct 25, 2023

		Runhouse provides a couple of options to manage the connection to the Runhouse API server running on a cluster.


		API Server Connection

		- ``tls``: Connects to the cluster via HTTPS (by default on port :code:`443`) and enforces verification via TLS
		certificates. Only users with a valid cert will be able to make requests to the API server.

		- ``none``: Does not use any port forwarding or enforce any authentication. Connects to the cluster with HTTP by
		default on port :code:`80`.

		- ``paramiko``: Uses `Paramiko <https://www.paramiko.org/>`_ to create an SSH tunnel to the cluster. This
		is relevant if you are using a cluster which require existing credentials (e.g. a password).

		The ``tls`` connection type is the most secure and is recommended for production use if you are not running inside
		of a VPC.

		Runhouse allows you to authenticate users via their Runhouse token (generated when
		:ref:`logging in <Login/Logout>`) and saved to local Runhouse configs in path: :code:`~/.rh/config.yaml`.

Support HTTPS calls to clusters #114

Support HTTPS calls to clusters #114

Conversation

jlewitt1 commented Sep 28, 2023 • edited Loading

Highlights:

New server connection types:

Validating resource access on the cluster:

NGINX (optional):

TODOs:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlewitt1 commented Sep 28, 2023 •

edited

Loading