From 26184a20787b1cff5e68c41304982373446bde2b Mon Sep 17 00:00:00 2001
From: Nathan Hertz <nhertz@nrao.edu>
Date: Fri, 17 Sep 2021 15:08:53 -0400
Subject: [PATCH] WS-641: CARTA 2.0

---
 .../carta_envoy/carta_envoy/connect.py        | 65 ++++++++-----------
 .../carta_envoy/carta_envoy/launchers.py      | 61 +++++------------
 .../pexable/carta_envoy/test/test_connect.py  | 39 ++++++-----
 .../carta_envoy/test/test_launchers.py        | 32 ++++-----
 4 files changed, 83 insertions(+), 114 deletions(-)

diff --git a/apps/cli/executables/pexable/carta_envoy/carta_envoy/connect.py b/apps/cli/executables/pexable/carta_envoy/carta_envoy/connect.py
index 867405bf0..3df90ef6a 100644
--- a/apps/cli/executables/pexable/carta_envoy/carta_envoy/connect.py
+++ b/apps/cli/executables/pexable/carta_envoy/carta_envoy/connect.py
@@ -34,8 +34,7 @@ class RedisConnect:
         :return: dict of settings
         """
         return {
-            "front_end_id": generate_random_str(),
-            "back_end_id": generate_random_str(),
+            "session_id": generate_random_str(),
             "wrapper_id": generate_random_str(),
             "system_name": generate_random_str(),
         }
@@ -47,10 +46,9 @@ class RedisConnect:
         :return: URL of current CARTA session
         """
         self.logger.info("Generating CARTA url...")
-        front_end_id = self.generated_ids["front_end_id"]
-        back_end_id = self.generated_ids["back_end_id"]
+        session_id = self.generated_ids["session_id"]
         proxy = self.settings["reverse_proxy"]
-        carta_url = f"https://{proxy}/{front_end_id}/?socketUrl=wss://{proxy}/{back_end_id}/"
+        carta_url = f"https://{proxy}/{session_id}/"
         if self.settings["single_image"]:
             carta_url = carta_url + "&file=" + self.settings["image_name"]
 
@@ -90,26 +88,22 @@ class RedisConnect:
         :return: settings as key-value pairs
         """
         self.logger.info("Preparing Redis server...")
-        front_end_port = self.get_available_port()
-        back_end_port = self.get_available_port()
+        session_port = self.get_available_port()
         wrapper_port = self.get_available_port()
         carta_url = self.generate_carta_url()
         wrapper_url = self.generate_wrapper_url()
         self.conn = self.connect_to_redis()
         redis_values = self.get_redis_values(
             self.settings["reverse_proxy"],
-            self.generated_ids["front_end_id"],
-            self.generated_ids["back_end_id"],
+            self.generated_ids["session_id"],
             self.generated_ids["wrapper_id"],
-            front_end_port,
-            back_end_port,
+            session_port,
             wrapper_port,
         )
         self.set_redis_values(redis_values, int(self.settings["timeout"]))
 
         return {
-            "front_end_port": front_end_port,
-            "back_end_port": back_end_port,
+            "session_port": session_port,
             "wrapper_port": wrapper_port,
             "carta_url": carta_url,
             "wrapper_url": wrapper_url,
@@ -135,41 +129,38 @@ class RedisConnect:
     def get_redis_values(
         self,
         reverse_proxy_host: str,
-        front_end_id: str,
-        back_end_id: str,
+        session_id: str,
         carta_wrapper_id: str,
-        front_end_port: int,
-        back_end_port: int,
+        session_port: int,
         wrapper_port: int,
     ) -> Dict[str, str]:
         """
-        Get Redis key/value pairs for CARTA front end and back end setup
+        Get Redis key/value pairs for CARTA session setup
 
         :param reverse_proxy_host: Hostname for the reverse proxy
-        :param front_end_id: Random string ID for CARTA front end
-        :param back_end_id: Random string ID for CARTA back end
+        :param session_id: Random string ID for CARTA session
+        :param session_port: Port to run the CARTA session with
         :param carta_wrapper_id: Random string ID for CARTA wrapper
-        :param front_end_port: Port to run the CARTA front end with
-        :param back_end_port: Port to run the CARTA back end with
         :param wrapper_port: Port to run the CARTA wrapper with
         :return: Dictionary of values
         """
         self.logger.info("Determining values for Redis...")
-        service_name = generate_random_str()
-        front_end = f"carta-front-{service_name}"
-        back_end = f"carta-back-{service_name}"
-        carta_wrapper = f"carta-wrapper-{service_name}"
+        front_end = f"carta-front-{session_id}"
+        back_end = f"carta-back-{session_id}"
+        carta_wrapper = f"carta-wrapper-{carta_wrapper_id}"
         hostname = socket.getfqdn()
         values = {
             # FRONT END
-            f"traefik/http/routers/{front_end}/rule": f"Host(`{reverse_proxy_host}`) && PathPrefix(`/{front_end_id}/`)",
+            f"traefik/http/routers/{front_end}/rule": f"Host(`{reverse_proxy_host}`) && PathPrefix(`/{session_id}/`)",
             f"traefik/http/routers/{front_end}/service": front_end,
-            f"traefik/http/services/{front_end}/loadbalancer/servers/0/url": f"http://{hostname}:{front_end_port}/",
+            f"traefik/http/services/{front_end}/loadbalancer/servers/0/url": f"http://{hostname}:{session_port}/",
             f"traefik/http/routers/{front_end}/middlewares/0": "stripPrefixFE@file",
             # BACK END
-            f"traefik/http/routers/{back_end}/rule": f"Host(`{reverse_proxy_host}`) && PathPrefix(`/{back_end_id}/`)",
+            # Both sets of rules are very similar; small difference is with the missing / at end of PathPrefix
+            # This is needed to successfully connect to the web socket because CARTA says so
+            f"traefik/http/routers/{back_end}/rule": f"Host(`{reverse_proxy_host}`) && PathPrefix(`/{session_id}`)",
             f"traefik/http/routers/{back_end}/service": back_end,
-            f"traefik/http/services/{back_end}/loadbalancer/servers/0/url": f"http://{hostname}:{back_end_port}/",
+            f"traefik/http/services/{back_end}/loadbalancer/servers/0/url": f"http://{hostname}:{session_port}/",
             f"traefik/http/routers/{back_end}/middlewares/0": "upgradeWSHeader@file",
             f"traefik/http/routers/{back_end}/middlewares/1": "stripPrefixFE@file",
             # WRAPPER
@@ -179,11 +170,11 @@ class RedisConnect:
             f"traefik/http/routers/{carta_wrapper}/middlewares/0": "stripPrefixFE@file",
         }
 
-        unique_values = self.check_for_duplicate_values(values, front_end_port, back_end_port, wrapper_port)
+        unique_values = self.check_for_duplicate_values(values, session_port, wrapper_port)
         self.redis_values = unique_values
         return unique_values
 
-    def check_for_duplicate_values(self, redis_values: dict, front_port: int, back_port: int, wrapper_port: int):
+    def check_for_duplicate_values(self, redis_values: dict, session_port: int, wrapper_port: int):
         self.logger.info("Checking for duplicate values on server...")
         for key in redis_values:
             if self.conn.get(key):
@@ -191,11 +182,9 @@ class RedisConnect:
                 self.generated_ids = self.generate_ids()
                 new_values = self.get_redis_values(
                     self.settings["reverse_proxy"],
-                    self.generated_ids["front_end_id"],
-                    self.generated_ids["back_end_id"],
+                    self.generated_ids["session_id"],
                     self.generated_ids["wrapper_id"],
-                    front_port,
-                    back_port,
+                    session_port,
                     wrapper_port,
                 )
                 return new_values
@@ -235,7 +224,9 @@ class ArchiveConnect:
 
         :param url: URL generated to allow user access to this running CARTA instance
         """
-        send_archive_msg_url = f"{self.settings['workflow_url']}/workflows/carta/requests/{self.settings['wf_request_id']}/send-url-to-aat"
+        send_archive_msg_url = (
+            f"{self.settings['workflow_url']}/workflows/carta/requests/{self.settings['wf_request_id']}/send-url-to-aat"
+        )
         payload = {"carta_url": url}
         self.logger.info("Sending REST call to workflow service for AAT messaging.")
         requests.post(send_archive_msg_url, json=payload)
diff --git a/apps/cli/executables/pexable/carta_envoy/carta_envoy/launchers.py b/apps/cli/executables/pexable/carta_envoy/carta_envoy/launchers.py
index 599303e54..b0a272dc1 100644
--- a/apps/cli/executables/pexable/carta_envoy/carta_envoy/launchers.py
+++ b/apps/cli/executables/pexable/carta_envoy/carta_envoy/launchers.py
@@ -2,7 +2,6 @@
 import logging
 import math
 import os
-import re
 import signal
 import subprocess
 import sys
@@ -111,8 +110,7 @@ class CartaLauncher:
             self.settings["carta_path"],
             int(self.settings["timeout"]),
             Path(self.settings["data_location"]),
-            prepare["front_end_port"],
-            prepare["back_end_port"],
+            prepare["session_port"],
             prepare["wrapper_port"],
             prepare["carta_url"],
             prepare["wrapper_url"],
@@ -124,8 +122,7 @@ class CartaLauncher:
         path_to_carta: str,
         timeout_minutes: int,
         file_browser_path: Path,
-        front_end_port: int,
-        back_end_port: int,
+        session_port: int,
         wrapper_port: int,
         carta_url: str,
         wrapper_url: str,
@@ -137,8 +134,7 @@ class CartaLauncher:
         :param path_to_carta: Path to CARTA executable
         :param timeout_minutes: Number of minutes to execute CARTA for before stopping it
         :param file_browser_path: Path to use for CARTA file browser
-        :param front_end_port: Port to run the CARTA front end with
-        :param back_end_port: Port to run the CARTA back end with
+        :param session_port: Port to run the CARTA session with
         :param wrapper_port: Port to run the CARTA wrapper with
         :param carta_url: URL used to access CARTA from a web browser
         :param wrapper_url: URL used to access CARTA wrapper from a web browser
@@ -148,36 +144,23 @@ class CartaLauncher:
 
         self.logger.info(f"file_browser_path: {file_browser_path}")
         if not file_browser_path.exists():
-            self.logger.error(
-                f"ERROR: Invalid file browser path. {file_browser_path}: Directory does not exist."
-            )
+            self.logger.error(f"ERROR: Invalid file browser path. {file_browser_path}: Directory does not exist.")
             return
         elif not file_browser_path.is_dir():
-            self.logger.error(
-                f"ERROR: Invalid file browser path. {file_browser_path}: Path is not a directory."
-            )
+            self.logger.error(f"ERROR: Invalid file browser path. {file_browser_path}: Path is not a directory.")
             return
 
         if self.settings["useCarta"]:
             self.logger.info(f"Running CARTA with a timeout of {timeout_minutes} minute(s)...")
-            if re.search(r"CARTA.AppImage", path_to_carta):
-                # Command uses AppImage, include --remote option
-                carta_command = [
-                    path_to_carta,
-                    "--remote",
-                    f"--port={str(back_end_port)}",
-                    f"--fport={str(front_end_port)}",
-                    f"--folder={file_browser_path}",
-                    f"--root={file_browser_path}",
-                ]
-            else:
-                carta_command = [
-                    path_to_carta,
-                    f"--port={str(back_end_port)}",
-                    f"--fport={str(front_end_port)}",
-                    f"--folder={file_browser_path}",
-                    f"--root={file_browser_path}",
-                ]
+            carta_command = [
+                path_to_carta,
+                "--no_browser",  # 2.0 setting: don't automatically try to open browser
+                "--debug_no_auth",  # No tokenized authorization
+                "--no_runtime_config",  # Don't send runtime config object to CARTA front-end (fixes a bug)
+                f"--port={session_port!s}",
+                f"--top_level_folder={file_browser_path!s}",
+                f"{file_browser_path!s}",
+            ]
             try:
                 CARTA_PROCESS = subprocess.Popen(
                     carta_command,
@@ -190,9 +173,7 @@ class CartaLauncher:
                 self.teardown()
                 sys.exit(f"ERROR: Failed to launch CARTA: {err}")
             else:
-                CartaWrapperLauncher.deploy_wrapper_html(
-                    file_browser_path, carta_url, session_timeout_date
-                )
+                CartaWrapperLauncher.deploy_wrapper_html(file_browser_path, carta_url, session_timeout_date)
 
                 # CARTA is running and accessible, so send CARTA URL to AAT system or notify user
                 self.notify_ready(wrapper_url)
@@ -200,14 +181,10 @@ class CartaLauncher:
                 # Activate timeout handler
                 signal.signal(signal.SIGALRM, self.signal)
                 signal.alarm(60 * timeout_minutes)
-                self.logger.info(
-                    f"To access CARTA, enter this URL into a web browser: {wrapper_url}"
-                )
+                self.logger.info(f"To access CARTA, enter this URL into a web browser: {wrapper_url}")
                 self.logger.info("Press Ctrl+C to quit.")
 
-                wrapper_server_thread = Thread(
-                    target=CartaWrapperLauncher.run, args=[wrapper_port], daemon=True
-                )
+                wrapper_server_thread = Thread(target=CartaWrapperLauncher.run, args=[wrapper_port], daemon=True)
                 wrapper_server_thread.run()
 
                 # Loop until process ends or timeout elapses, whichever happens first
@@ -271,9 +248,7 @@ class CartaLauncher:
             timeout_hours, timeout_remainder = math.modf(timeout_minutes / 60)
             timeout_hours = int(timeout_hours)
             timeout_minutes = int(timeout_remainder * 60)
-            logger.info(
-                f"Timeout longer than 1 hour. Splitting into {timeout_hours}:{timeout_minutes}"
-            )
+            logger.info(f"Timeout longer than 1 hour. Splitting into {timeout_hours}:{timeout_minutes}")
             timeout_date = pendulum.now().add(hours=timeout_hours, minutes=timeout_minutes)
         else:
             timeout_date = pendulum.now().add(minutes=timeout_minutes)
diff --git a/apps/cli/executables/pexable/carta_envoy/test/test_connect.py b/apps/cli/executables/pexable/carta_envoy/test/test_connect.py
index 8ff1959c3..694504e2a 100644
--- a/apps/cli/executables/pexable/carta_envoy/test/test_connect.py
+++ b/apps/cli/executables/pexable/carta_envoy/test/test_connect.py
@@ -3,12 +3,18 @@ Tests for carta_envoy.connect
 """
 from unittest.mock import patch
 
-# pylint: disable=E0401, C0103, C0301, C0415, E1101, E1120, R0201, R0903, W0621
-
 import pytest
-from carta_envoy.connect import ArchiveConnect, NotificationConnect, RedisConnect, WorkflowConnect
+from carta_envoy.connect import (
+    ArchiveConnect,
+    NotificationConnect,
+    RedisConnect,
+    WorkflowConnect,
+)
 from carta_envoy.utilities import generate_random_str
 
+# pylint: disable=E0401, C0103, C0301, C0415, E1101, E1120, R0201, R0903, W0621
+
+
 test_settings = {
     "timeout": 1,
     "carta_path": "/fake/path/to/nowhere",
@@ -31,6 +37,7 @@ workflow_connect = WorkflowConnect(settings=test_settings)
 
 CARTA_POST = "carta_envoy.connect.requests.post"
 
+
 @pytest.fixture
 def redis():
     """
@@ -57,10 +64,10 @@ class TestRedisConnect:
     @pytest.mark.skip("not yet implemented")
     def test_generate_url(self):
         """
-       TODO
+        TODO
 
-       :return:
-       """
+        :return:
+        """
 
     def test_get_available_port(self):
         """
@@ -81,8 +88,8 @@ class TestRedisConnect:
         """
         TODO
 
-      :return:
-      """
+        :return:
+        """
 
     def test_connect_to_redis_error(self):
         """
@@ -98,8 +105,8 @@ class TestRedisConnect:
         Tests that filled out Redis values are given correctly
         """
         random_id = generate_random_str()
-        front_end_port = 7777
-        back_end_port = 6464
+        session_port = 7777
+        wrapper_port = 6464
         reverse_proxy_host = "test"
         front_end = f"carta-front-{random_id}"
         back_end = f"carta-back-{random_id}"
@@ -117,7 +124,7 @@ class TestRedisConnect:
         assert (
             value
             in redis_connect.get_redis_values(
-                reverse_proxy_host, random_id, random_id, front_end_port, back_end_port
+                reverse_proxy_host, random_id, random_id, session_port, wrapper_port
             ).keys()
             for value in values
         )
@@ -129,17 +136,13 @@ class TestRedisConnect:
         :param redis: Custom fixture that provides a sandbox Redis server
         """
         redis_connect.conn = redis
-        redis_connect.generated_ids = {
-            "front_end_id": "1234abcd",
-            "back_end_id": "5678efgh",
-            "wrapper_id": "9876jklm",
-        }
+        redis_connect.generated_ids = {"session_id": "5678efgh", "wrapper_id": "9876jklm"}
         redis.set("duplicate", "value")
         with patch("carta_envoy.connect.RedisConnect.get_redis_values") as values:
-            redis_connect.check_for_duplicate_values({"duplicate": "value"}, 9897, 6543, 1234)
+            redis_connect.check_for_duplicate_values({"duplicate": "value"}, 9897, 6543)
             assert values.call_count == 1
             values.call_count = 0
-            redis_connect.check_for_duplicate_values({"not_duplicate": "value"}, 9897, 6543, 1234)
+            redis_connect.check_for_duplicate_values({"not_duplicate": "value"}, 9897, 6543)
             assert values.call_count == 0
 
 
diff --git a/apps/cli/executables/pexable/carta_envoy/test/test_launchers.py b/apps/cli/executables/pexable/carta_envoy/test/test_launchers.py
index b5533b6c0..e8880327f 100644
--- a/apps/cli/executables/pexable/carta_envoy/test/test_launchers.py
+++ b/apps/cli/executables/pexable/carta_envoy/test/test_launchers.py
@@ -33,8 +33,7 @@ launcher = CartaLauncher(settings=test_settings)
 
 SUBPROCESS_COMMAND_PATCH = "carta_envoy.launchers.subprocess.Popen"
 
-BACK_END_PORT = 7777
-FRONT_END_PORT = 6464
+SESSION_PORT = 7777
 WRAPPER_PORT = 1010
 
 
@@ -93,8 +92,7 @@ class TestCartaLauncher:
                 path_to_carta="fake_carta_path",
                 timeout_minutes=1,
                 file_browser_path=Path(),
-                front_end_port=FRONT_END_PORT,
-                back_end_port=BACK_END_PORT,
+                session_port=SESSION_PORT,
                 wrapper_port=WRAPPER_PORT,
                 carta_url="carta_url",
                 wrapper_url="wrapper_url",
@@ -118,8 +116,7 @@ class TestCartaLauncher:
                     path_to_carta="fake_carta_path",
                     timeout_minutes=1,
                     file_browser_path=Path(),
-                    front_end_port=FRONT_END_PORT,
-                    back_end_port=BACK_END_PORT,
+                    session_port=SESSION_PORT,
                     wrapper_port=WRAPPER_PORT,
                     carta_url="carta_url",
                     wrapper_url="wrapper_url",
@@ -129,10 +126,12 @@ class TestCartaLauncher:
                 mock_subprocess.assert_called_with(
                     [
                         "fake_carta_path",
-                        f"--port={BACK_END_PORT}",
-                        f"--fport={FRONT_END_PORT}",
-                        "--folder=.",
-                        "--root=.",
+                        "--no_browser",
+                        "--debug_no_auth",
+                        "--no_runtime_config",
+                        f"--port={SESSION_PORT!s}",
+                        "--top_level_folder=.",
+                        ".",
                     ],
                     preexec_fn=None,
                     stdin=-1,
@@ -162,8 +161,7 @@ class TestCartaLauncher:
                     path_to_carta="fake_carta_path",
                     timeout_minutes=1,
                     file_browser_path=Path(),
-                    front_end_port=FRONT_END_PORT,
-                    back_end_port=BACK_END_PORT,
+                    session_port=SESSION_PORT,
                     wrapper_port=WRAPPER_PORT,
                     carta_url="carta_url",
                     wrapper_url="wrapper_url",
@@ -173,10 +171,12 @@ class TestCartaLauncher:
                 mock_subprocess.assert_called_with(
                     [
                         "fake_carta_path",
-                        f"--port={BACK_END_PORT}",
-                        f"--fport={FRONT_END_PORT}",
-                        "--folder=.",
-                        "--root=.",
+                        "--no_browser",
+                        "--debug_no_auth",
+                        "--no_runtime_config",
+                        f"--port={SESSION_PORT!s}",
+                        "--top_level_folder=.",
+                        ".",
                     ],
                     preexec_fn=None,
                     stdin=-1,
-- 
GitLab