Merge ~mitchburton/landscape-charm:2059194-respect-juju-proxy-settings into landscape-charm:main

[Mitch Burton]

use JUJU_*_PROXY env vars on bootstrap

This change uses the juju proxy-related model configs to populate Landscape's proxy configuration on initial setup. After initial setup, the proper way to change them is through the Landscape UI.

Don't use `make build` to test this, as it'll be too late to update the model config (there's no "model-config-changed" hook, unfortunately) and this only happens on initial bootstrap anyways. Do this instead. It's nice to have an actual proxy to test with, too, but you can _only_ set juju-no-proxy if you don't have one.

  1. make clean
  2. charmcraft pack
  3. juju add-model landscape-charm-dev
  4. juju model-config juju-http-proxy=http://<proxy_ip>:<proxy_port> juju-https-proxy=http://<proxy_ip>:<proxy_port> juju-no-proxy=127.0.0.1,localhost,::1,10.76.244.0/24
  5. juju deploy ./bundle.yaml
  6. log into Landscape via haproxy's IP address and click on "settings" for the account. Ensure that the proxy settings appear there.

There's currently an in-review MR for landscape-server that makes the appserver process actually respect these settings.

[Ardavan Behnia (abehnia)]

Manually checked with (squid) and without proxy, left a small question inline.

review: Needs Information

[Mitch Burton (mitchburton)]

Response to inline.

[Ardavan Behnia (abehnia)]

LGTM, left a comment.

review: Approve

diff --git a/src/charm.py b/src/charm.py
index 9b4f2de..17c1413 100755
--- a/src/charm.py
+++ b/src/charm.py
@@ -16,7 +16,9 @@ import logging
 import os
 import subprocess
 from base64 import b64decode, b64encode, binascii
+from functools import cached_property
 from subprocess import CalledProcessError, check_call
+from typing import List
 
 import yaml
 
@@ -53,7 +55,6 @@ from settings_files import (
     generate_secret_token,
     merge_service_conf,
     prepend_default_settings,
-    SecretTokenMissing,
     update_db_conf,
     update_default_settings,
     update_service_conf,
@@ -104,6 +105,12 @@ OIDC_CONFIG_VALS = (
     "oidc_logout_url",
 )
 
+PROXY_ENV_MAPPING = {
+    "JUJU_CHARM_HTTP_PROXY": "--with-http-proxy",
+    "JUJU_CHARM_HTTPS_PROXY": "--with-https-proxy",
+    "JUJU_CHARM_NO_PROXY": "--with-no-proxy",
+}
+
 
 class LandscapeServerCharm(CharmBase):
     """Charm the service."""
@@ -485,14 +492,40 @@ class LandscapeServerCharm(CharmBase):
         self.unit.status = ActiveStatus("Unit is ready")
         self._update_ready_status()
 
+    @cached_property
+    def _proxy_settings(self) -> List[str]:
+        """Determines the current proxy settings from the juju-related environment
+        variables.
+
+        :returns: A list of proxy settings arguments suitable for passing to
+            `SCHEMA_SCRIPT`.
+        """
+        settings = []
+
+        for juju_env_var, schema_arg_name in PROXY_ENV_MAPPING.items():
+            value = os.environ.get(juju_env_var)
+
+            if value:
+                settings.append(schema_arg_name)
+                settings.append(value)
+
+        return settings
+
     def _migrate_schema_bootstrap(self):
         """
         Migrates schema along with the bootstrap command which ensures that the
-        databases along with the landscape user exists. In addition creates
-        admin if configured. Returns True on success
+        databases and the landscape user exists, and that proxy settings are set.
+        In addition, creates admin if configured.
+
+        :returns: True on success.
         """
+        call = [SCHEMA_SCRIPT, "--bootstrap"]
+
+        if self._proxy_settings:
+            call.extend(self._proxy_settings)
+
         try:
-            check_call([SCHEMA_SCRIPT, "--bootstrap"])
+            check_call(call)
             self._bootstrap_account()
             return True
         except CalledProcessError as e:
diff --git a/tests/test_charm.py b/tests/test_charm.py
index e44e8ab..62cf96c 100644
--- a/tests/test_charm.py
+++ b/tests/test_charm.py
@@ -75,7 +75,7 @@ class TestCharm(unittest.TestCase):
         mocks["check_call"].assert_any_call(
             ["add-apt-repository", "-y", ppa])
         mocks["check_call"].assert_any_call(
-            ["apt-mark", "hold", "landscape-hashids"])
+            ["apt-mark", "hold", "landscape-hashids", "landscape-server"])
         mocks["apt"].add_package.assert_called_once_with(
             ["landscape-server", "landscape-hashids"], update_cache=True,
         )
@@ -507,6 +507,9 @@ class TestCharm(unittest.TestCase):
                 "password": "testpass",
             },
         }
+        peer_relation_id = self.harness.add_relation("replicas", "landscape-server")
+        self.harness.update_relation_data(
+            peer_relation_id, "landscape-server", {"leader-ip": "test"})
 
         with patch("charm.check_call"):
             with patch(
@@ -838,6 +841,10 @@ class TestCharm(unittest.TestCase):
     def test_on_config_changed_no_smtp_change(self, _):
         self.harness.charm._update_ready_status = Mock()
         self.harness.charm._configure_smtp = Mock()
+        peer_relation_id = self.harness.add_relation("replicas", "landscape-server")
+        self.harness.update_relation_data(
+            peer_relation_id, "landscape-server", {"leader-ip": "test"})
+
         self.harness.update_config({"smtp_relay_host": ""})
 
         self.harness.charm._configure_smtp.assert_not_called()
@@ -847,6 +854,10 @@ class TestCharm(unittest.TestCase):
     def test_on_config_changed_smtp_change(self, _):
         self.harness.charm._update_ready_status = Mock()
         self.harness.charm._configure_smtp = Mock()
+        peer_relation_id = self.harness.add_relation("replicas", "landscape-server")
+        self.harness.update_relation_data(
+            peer_relation_id, "landscape-server", {"leader-ip": "test"})
+
         self.harness.update_config({"smtp_relay_host": "smtp.example.com"})
 
         self.harness.charm._configure_smtp.assert_called_once_with(
@@ -956,7 +967,10 @@ class TestCharm(unittest.TestCase):
         self.harness.charm._stored.running = False
         prev_status = self.harness.charm.unit.status
 
-        with patch("charm.apt", spec_set=apt) as apt_mock:
+        with (
+            patch("charm.apt", spec_set=apt) as apt_mock,
+            patch("charm.check_call")
+        ):
             pkg_mock = Mock()
             apt_mock.DebianPackage.from_apt_cache.return_value = pkg_mock
             self.harness.charm._upgrade(event)
@@ -987,7 +1001,10 @@ class TestCharm(unittest.TestCase):
         event = Mock(spec_set=ActionEvent)
         self.harness.charm._stored.running = False
 
-        with patch("charm.apt", spec_set=apt) as apt_mock:
+        with (
+            patch("charm.apt", spec_set=apt) as apt_mock,
+            patch("charm.check_call")
+        ):
             pkg_mock = Mock()
             apt_mock.DebianPackage.from_apt_cache.return_value = pkg_mock
             pkg_mock.ensure.side_effect = PackageNotFoundError("ouch")