Fix http protocol shown for https services with acm cert (#3709)

* Fix http protocol shown for https services with acm cert

* Fix tests

* Disallow http services on gateways with ACM cert

Author: r4victor

src/dstack/_internal/server/services/services/__init__.py

@@ -157,15 +157,29 @@ async def _register_service_in_gateway(
             f"'{gateway.name}' does not have the SGLang router configured."
         )
 
-    service_https = _get_service_https(run_spec, gateway_configuration)
-    router = _build_service_router_config(gateway_configuration, run_spec.configuration)
-    service_protocol = "https" if service_https else "http"
+    configure_service_https = _should_configure_service_https_on_gateway(
+        run_spec, gateway_configuration
+    )
+    show_service_https = _should_show_service_https(run_spec, gateway_configuration)
+    service_protocol = "https" if show_service_https else "http"
+
+    if (
+        not show_service_https
+        and gateway_configuration.certificate is not None
+        and gateway_configuration.certificate.type == "acm"
+    ):
+        # SSL termination is done globally at load balancer so cannot runs only some services via http.
+        raise ServerClientError(
+            "Cannot run HTTP service on gateway with ACM certificates configured"
+        )
 
-    if service_https and gateway_configuration.certificate is None:
+    if show_service_https and gateway_configuration.certificate is None:
         raise ServerClientError(
             "Cannot run HTTPS service on gateway with no SSL certificates configured"
         )
 
+    router = _build_service_router_config(gateway_configuration, run_spec.configuration)
+
     gateway_https = _get_gateway_https(gateway_configuration)
     gateway_protocol = "https" if gateway_https else "http"
 
@@ -195,7 +209,7 @@ async def _register_service_in_gateway(
                 project=run_model.project.name,
                 run_name=run_model.run_name,
                 domain=domain,
-                service_https=service_https,
+                service_https=configure_service_https,
                 gateway_https=gateway_https,
                 auth=run_spec.configuration.auth,
                 client_max_body_size=settings.DEFAULT_SERVICE_CLIENT_MAX_BODY_SIZE,
@@ -432,7 +446,13 @@ async def unregister_replica(session: AsyncSession, job_model: JobModel):
     )
 
 
-def _get_service_https(run_spec: RunSpec, configuration: GatewayConfiguration) -> bool:
+def _should_configure_service_https_on_gateway(
+    run_spec: RunSpec, configuration: GatewayConfiguration
+) -> bool:
+    """
+    Returns `True` if the gateway needs to serve the service with HTTPS.
+    May be `False` for HTTPS services, e.g. SSL termination is done on a load balancer.
+    """
     assert run_spec.configuration.type == "service"
     https = run_spec.configuration.https
     if https is None:
@@ -450,6 +470,21 @@ def _get_service_https(run_spec: RunSpec, configuration: GatewayConfiguration) -
     return True
 
 
+def _should_show_service_https(run_spec: RunSpec, configuration: GatewayConfiguration) -> bool:
+    """
+    Returns `True` if the service needs to be accessed via https://.
+    """
+    assert run_spec.configuration.type == "service"
+    https = run_spec.configuration.https
+    if https is None:
+        https = SERVICE_HTTPS_DEFAULT
+    if https == "auto":
+        if configuration.certificate is None:
+            return False
+        return True
+    return https
+
+
 def _get_gateway_https(configuration: GatewayConfiguration) -> bool:
     if configuration.certificate is not None and configuration.certificate.type == "acm":
         return False

src/tests/_internal/server/services/services/test_services.py

@@ -14,8 +14,9 @@
 )
 from dstack._internal.core.models.runs import RunSpec
 from dstack._internal.server.services.services import (
-    _get_service_https,
     _register_service_in_server,
+    _should_configure_service_https_on_gateway,
+    _should_show_service_https,
 )
 from dstack._internal.server.testing.common import get_run_spec
 
@@ -54,33 +55,69 @@ def test_accepts_auto(self) -> None:
         assert conf.https == "auto"
 
 
-class TestGetServiceHttps:
+class TestShouldConfigureServiceHttpsOnGateway:
     def test_auto_resolves_to_true_with_lets_encrypt_gateway(self) -> None:
         run_spec = _service_run_spec(https="auto")
         gw = _gateway_config(certificate=LetsEncryptGatewayCertificate())
-        assert _get_service_https(run_spec, gw) is True
+        assert _should_configure_service_https_on_gateway(run_spec, gw) is True
 
     def test_auto_resolves_to_false_when_gateway_has_no_certificate(self) -> None:
         run_spec = _service_run_spec(https="auto")
         gw = _gateway_config(certificate=None)
-        assert _get_service_https(run_spec, gw) is False
+        assert _should_configure_service_https_on_gateway(run_spec, gw) is False
 
     def test_auto_resolves_to_false_with_acm_gateway(self) -> None:
         run_spec = _service_run_spec(https="auto")
         gw = _gateway_config(
             certificate=ACMGatewayCertificate(arn="arn:aws:acm:us-east-1:123:cert/abc")
         )
-        assert _get_service_https(run_spec, gw) is False
+        assert _should_configure_service_https_on_gateway(run_spec, gw) is False
+
+    def test_true_enables_https_when_gateway_has_no_certificate(self) -> None:
+        run_spec = _service_run_spec(https=True)
+        gw = _gateway_config(certificate=None)
+        assert _should_configure_service_https_on_gateway(run_spec, gw) is True
+
+    def test_false_disables_https_regardless_of_gateway_certificate(self) -> None:
+        run_spec = _service_run_spec(https=False)
+        gw = _gateway_config(certificate=LetsEncryptGatewayCertificate())
+        assert _should_configure_service_https_on_gateway(run_spec, gw) is False
+
+    def test_true_does_not_configure_https_on_acm_gateway(self) -> None:
+        run_spec = _service_run_spec(https=True)
+        gw = _gateway_config(
+            certificate=ACMGatewayCertificate(arn="arn:aws:acm:us-east-1:123:cert/abc")
+        )
+        assert _should_configure_service_https_on_gateway(run_spec, gw) is False
+
+
+class TestShouldShowServiceHttps:
+    def test_auto_resolves_to_true_with_lets_encrypt_gateway(self) -> None:
+        run_spec = _service_run_spec(https="auto")
+        gw = _gateway_config(certificate=LetsEncryptGatewayCertificate())
+        assert _should_show_service_https(run_spec, gw) is True
+
+    def test_auto_resolves_to_false_when_gateway_has_no_certificate(self) -> None:
+        run_spec = _service_run_spec(https="auto")
+        gw = _gateway_config(certificate=None)
+        assert _should_show_service_https(run_spec, gw) is False
+
+    def test_auto_resolves_to_true_with_acm_gateway(self) -> None:
+        run_spec = _service_run_spec(https="auto")
+        gw = _gateway_config(
+            certificate=ACMGatewayCertificate(arn="arn:aws:acm:us-east-1:123:cert/abc")
+        )
+        assert _should_show_service_https(run_spec, gw) is True
 
     def test_true_enables_https_regardless_of_gateway_certificate(self) -> None:
         run_spec = _service_run_spec(https=True)
         gw = _gateway_config(certificate=None)
-        assert _get_service_https(run_spec, gw) is True
+        assert _should_show_service_https(run_spec, gw) is True
 
     def test_false_disables_https_regardless_of_gateway_certificate(self) -> None:
         run_spec = _service_run_spec(https=False)
         gw = _gateway_config(certificate=LetsEncryptGatewayCertificate())
-        assert _get_service_https(run_spec, gw) is False
+        assert _should_show_service_https(run_spec, gw) is False
 
 
 class TestRegisterServiceInServerHttps: