Merge ~woutervb/snap-store-proxy-charm:SN-484 into snap-store-proxy-charm:master

[Wouter van Bommel]

Determine TLS usage and update status display

Show on which url the service is configured. Take the TLS status from the registration bundle.

[Przemysław Suliga (suligap)]

lgtm + one comment

review: Approve

[Wouter van Bommel (woutervb)]

> lgtm + one comment
Thanks, changed

[Otto Co-Pilot (otto-copilot)]

Running landing tests failed
https://jenkins.ols.canonical.com/online-services/job/snap-store-proxy-charm/4/

diff --git a/Makefile b/Makefile
index 11342be..c09cee6 100644
--- a/Makefile
+++ b/Makefile
@@ -29,7 +29,7 @@ charm: $(CHARM_FILENAME)
 # Ensure that the building of the charm depends on the python files that make
 # up the charm
 $(CHARM_FILENAME): $(DEPS) $(CHARMCRAFT)
-	$(CHARMCRAFT) pack
+	$(CHARMCRAFT) pack --verbose
 
 $(ENV): $(OLS_WHEELS_DIR) requirements-dev.txt requirements.txt
 	@echo Starting a new virtualenv
diff --git a/src/charm.py b/src/charm.py
index 500e623..b75ae15 100755
--- a/src/charm.py
+++ b/src/charm.py
@@ -262,6 +262,14 @@ class SnapstoreProxyCharm(CharmBase):
                 # Incomplete bundle(?) so stop doing anything
                 return
 
+            # see if we are running on https
+            if data["domain"].startswith("https"):
+                self._stored.ssl = True
+            else:
+                self._stored.ssl = False
+
+            logger.debug(f"Status of tls is {self._stored.ssl}")
+
             # Now make 2 lists, option names and values where the indexes
             # correspond with eachother
             options = []
@@ -269,16 +277,24 @@ class SnapstoreProxyCharm(CharmBase):
             for entry in data.keys():
                 if entry == "domain":
                     options.append("proxy.domain")
-                    values.append(data[entry])
+                    # We only want the hostname to be registered
+                    values.append(urlparse(data[entry]).hostname)
+                    self._stored.domain = data[entry]
+                    logger.debug(
+                        f"Domain set from the registration bundle {self._stored.domain}"
+                    )
                 elif entry == "public_key":
                     options.append("proxy.key.public")
                     values.append(data[entry])
+                    logger.debug("Stored the public key from the registration bundle")
                 elif entry == "private_key":
                     options.append("proxy.key.private")
                     values.append(data[entry])
+                    logger.debug("Stored the private key from the registration bundle")
                 elif entry == "store_id":
                     options.append("internal.store.id")
                     values.append(data[entry])
+                    logger.debug("Stored the store id from the registration bundle")
             configure_proxy(options, values, force=True)
 
     def handle_registration(self, event):
@@ -359,9 +375,9 @@ class SnapstoreProxyCharm(CharmBase):
             return
 
         # We are in a proper state, let's open the port
-        open_port(ssl=False)
+        open_port(ssl=getattr(self._stored, "ssl", False))
 
-        self.unit.status = ActiveStatus()
+        self.unit.status = ActiveStatus(f"Running on: {self._stored.domain}")
 
     def _on_update_status(self, _):
 
diff --git a/src/helpers.py b/src/helpers.py
index 7b440e6..1a53ad2 100644
--- a/src/helpers.py
+++ b/src/helpers.py
@@ -12,6 +12,7 @@ logger = logging.getLogger(__name__)
 
 def configure_proxy(option, value, force=False):
     """Simple wrapper around calling snap settings"""
+    logger.info("Configuring the snap-store-proxy snap")
 
     if isinstance(option, list):
         # Make sure that we set multiple values in one go
@@ -19,12 +20,15 @@ def configure_proxy(option, value, force=False):
         for count, entry in enumerate(option):
             combined.append(f'{entry}="{value[count]}"')
         command = ["snap-store-proxy", "config"] + combined
+        logger.debug("Running proxy config for single item")
     else:
         logger.debug(f"Running config for option {option} with value {value}")
         command = ["snap-store-proxy", "config", f"{option}={value}"]
+        logger.debug("Running registration for list of items")
 
     if force:
         command.append("--force")
+        logger.debug("Forcing the options to be accepted")
 
     run(command)
 
diff --git a/tests/test_charm.py b/tests/test_charm.py
index 69f0ba6..9fdac99 100644
--- a/tests/test_charm.py
+++ b/tests/test_charm.py
@@ -308,12 +308,15 @@ class TestRegistrationBundle(CharmTest):
     #         == "Missing key store_assertion in the registration bundle"
     #     )
 
-    def test_has_complete_bundle(self):
+    @pytest.mark.parametrize(
+        "domain,ssl_status", [("http://non.ssl", False), ("https://on.ssl", True)]
+    )
+    def test_has_complete_bundle(self, domain, ssl_status):
         to_update = [
             {
                 "name": "registration_bundle",
                 "value": base64.b64encode(
-                    '{"domain": "sdfsd",'
+                    f'{{"domain": "{domain}",'
                     '"public_key": "10",'
                     '"store_id": "11",'
                     '"store_assertion": "0",'
@@ -331,11 +334,14 @@ class TestRegistrationBundle(CharmTest):
                 "internal.store.id",
                 "proxy.key.private",
             ],
-            ["sdfsd", "10", "11", "12"],
+            # For the domain we only need the part without the protocol
+            # and since the code uses urlparse we prevent using that here
+            [domain.rsplit("/")[-1], "10", "11", "12"],
             force=True,
         )
 
         assert getattr(self.harness.charm._stored, "registered", None)
+        assert getattr(self.harness.charm._stored, "ssl") == ssl_status
 
 
 class TestStatus(CharmTest):
@@ -389,11 +395,12 @@ class TestStatus(CharmTest):
         self.harness.charm._stored.username = "minion"
         self.harness.charm._stored.password = "minions password"
         self.harness.charm._stored.registered = True
+        self.harness.charm._stored.domain = "dummy.domain"
 
         with patch("charm.open_port"):
             self.harness.charm.evaluate_status()
 
-        assert self.harness.charm.unit.status.message == ""
+        assert self.harness.charm.unit.status.message == "Running on: dummy.domain"
 
 
 class TestEvents(CharmTest):
@@ -594,7 +601,7 @@ class TestRegistration(CharmTest):
         self.harness.charm._stored.registered = False
         self.harness.charm._stored.username = "[e-mail]"
         self.harness.charm._stored.password = "password"
-        self.harness.charm._stored.domain = "http://localhost"
+        self.harness.charm._stored.domain = "localhost"
         event = MagicMock()
 
         with patch("charm.register_store") as patched_registration:
@@ -622,7 +629,9 @@ class TestSanity(CharmTest):
             self.harness.update_config({"username": None})
 
         assert self.harness.charm._stored.username == "Some value"
-        assert self.harness.charm.unit.status == ActiveStatus("")
+        assert self.harness.charm.unit.status == ActiveStatus(
+            "Running on: http://localhost.localdomain"
+        )
 
     def test_empty_password_after_registration_and_active_status(self):
         self.harness.charm._stored.registered = True
@@ -635,21 +644,25 @@ class TestSanity(CharmTest):
             self.harness.update_config({"password": None})
 
         assert self.harness.charm._stored.password == "Some value"
-        assert self.harness.charm.unit.status == ActiveStatus("")
+        assert self.harness.charm.unit.status == ActiveStatus(
+            "Running on: http://localhost.localdomain"
+        )
 
     def test_update_domain_after_registration(self):
         self.harness.charm._stored.snap_install_source = "store"
         self.harness.charm._stored.registered = True
         self.harness.charm._stored.database_created = True
-        self.harness.charm._stored.domain = "Some value"
+        self.harness.charm._stored.domain = "http://Some.value"
         self.harness.charm._stored.db_uri = "postgresql://user@host/database"
 
         # Since valid values will be pushed on to the proxy, we need to stub that here
         with patch("charm.configure_proxy"), patch("charm.open_port"):
             self.harness.update_config({"domain": "http://my.super.domain"})
 
-        assert self.harness.charm._stored.domain == "Some value"
-        assert self.harness.charm.unit.status == ActiveStatus("")
+        assert self.harness.charm._stored.domain == "http://Some.value"
+        assert self.harness.charm.unit.status == ActiveStatus(
+            "Running on: http://Some.value"
+        )
 
 
 class TestInstallation(CharmTest):