Merge ~tshinn/snap-store-proxy-charm:self_signed_cert_support into snap-store-proxy-charm:main

[Tong Shinn]

Add support for configuring the proxy nginx with self signed certificate by setting both the "certificate" and "private_key" configurations to "selfsigned".

The reason for overloading the existing "certificate" and "private_key" configurations is that if a new configuration variable is created for enabling self-signed certs, the charm's behaviour becomes unclear when both the new variable and the existing variables are populated.

This change allows for easier local testing of the store proxy deployed using the charm.

The following juju config commands can be executed to generate and configure self signed certificate on the proxy:

juju config snap-store-proxy certificate="selfsigned"
juju config snap-store-proxy private_key="selfsigned"

Successful deployment looks like the following:

ubuntu@juju-b28c17-14:~$ snap-proxy status
Store URL: https://snap-store-proxy.local
Store DB: ok
Store ID: JFmKgUMENL7o6XsZ2OxtYkt3fkilpfkK
Status: approved
Connected Devices (updated daily): 0
Device Limit: 25
Internal Service Status:
  memcached: running
  nginx: running
  snapauth: running
  snapdevicegw: running
  snapdevicegw-local: running
  snapproxy: running
  snaprevs: running

ubuntu@juju-b28c17-14:~$ curl -k https://snap-store-proxy.local
snapcraft.io store API service - Copyright 2018-2022 Canonical.

[Przemysław Suliga (suligap)]

LGTM, but best if Wouter takes a look.

[Jonathan Hartley (tartley)]

This LGTM. Is this behavior of these config variables discoverable? Should it be documented in this project's README? There is an "Advanced options" section about config variables.

[Jonathan Hartley (tartley)]

review: Approve

[Jonathan Hartley (tartley)]

Oh, I approved this but did not mean to override the suggestion to get Wouter's input.

15e24e3... by Tong Shinn on 2023-01-24

Update README.md to include instructions for how to configure a self signed certificate.

[Wouter van Bommel (woutervb)]

1 remark, rest lgtm

review: Approve

ef4d614... by Tong Shinn on 2023-01-24

snap-proxy is an alias for snap-store-proxy so use the full file name to be safe.

diff --git a/README.md b/README.md
index 057c8e9..dbb1e49 100644
--- a/README.md
+++ b/README.md
@@ -48,6 +48,11 @@ There are some mandatory config options that have to be configured for the charm
 
       juju config snap-store-proxy registration_bundle=$(cat <path to file created with store-admin register command>)
 
+HTTPS can be enabled by setting the `certificate` and `private_key` configurations with the corresponding PEM formatted certficate/key. To use self signed certificate, set the configuration values to `selfsigned` as follows:
+
+      juju config snap-store-proxy certificate="selfsigned"
+      juju config snap-store-proxy private_key="selfsigned"
+
 ## Juju resource usage
 
 The charm supports 2 modes to install the snapstore proxy code itself. This code is distributed as a snap from the actual snapstore, which means that the unit to which this charm is deployed, will need internet access, or at least access to the actual snapstore. If this is undesired, it is possible to deploy this charm in complete offline mode, which means that the snaps(\*) will need to be added as a resource.
diff --git a/src/charm.py b/src/charm.py
index 3bdbca1..ac50311 100755
--- a/src/charm.py
+++ b/src/charm.py
@@ -20,7 +20,13 @@ from ops.main import main
 from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus, ModelError
 
 from exceptions import ConfigException
-from helpers import all_config_options, configure_proxy, default_values, open_port
+from helpers import (
+    all_config_options,
+    configure_proxy,
+    configure_self_signed_cert,
+    default_values,
+    open_port,
+)
 from optionvalidation import OptionValidation
 from resource_helpers import (
     create_database,
@@ -276,11 +282,18 @@ class SnapstoreProxyCharm(CharmBase):
     def handle_certificate(self):
         # If we are called, then we know that the config has changed, and that
         # self._stored["certificate|private_key"] contains the expected values
+        raw_certificate = getattr(self._stored, "certificate", None)
+        raw_key = getattr(self._stored, "private_key", None)
+
+        # If both the certificate and the private_key have been set to selfsigned,
+        # generate and configure self-signed cert and return early.
+        if raw_certificate == raw_key == "selfsigned":
+            configure_self_signed_cert()
+            return
 
         # See if the certificate matches the key, which is basically compare the
         # modules of the 2 certificate parts, they should be equal
         try:
-            raw_certificate = getattr(self._stored, "certificate", None)
             if raw_certificate is None:
                 return "Missing TLS certificate"
             cert = x509.load_pem_x509_certificate(raw_certificate.encode())
@@ -289,7 +302,6 @@ class SnapstoreProxyCharm(CharmBase):
             return "Problem loading the certificate, they should be PEM encoded strings"
 
         try:
-            raw_key = getattr(self._stored, "private_key", None)
             if raw_key is None:
                 self.errors.append("Missing TLS private key")
                 return
diff --git a/src/helpers.py b/src/helpers.py
index 9a82576..8b92f42 100644
--- a/src/helpers.py
+++ b/src/helpers.py
@@ -9,6 +9,8 @@ import yaml
 
 logger = logging.getLogger(__name__)
 
+PROXY_CLI = "snap-store-proxy"
+
 
 def configure_proxy(option, value, force=False):
     """Simple wrapper around calling snap settings"""
@@ -19,11 +21,11 @@ def configure_proxy(option, value, force=False):
         combined = []
         for count, entry in enumerate(option):
             combined.append(f'{entry}="{value[count]}"')
-        command = ["snap-store-proxy", "config"] + combined
+        command = [PROXY_CLI, "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}"]
+        command = [PROXY_CLI, "config", f"{option}={value}"]
         logger.debug("Running registration for list of items")
 
     if force:
@@ -33,6 +35,12 @@ def configure_proxy(option, value, force=False):
     run(command)
 
 
+def configure_self_signed_cert():
+    logger.info("Generating and configuring self signed certificate for nginx")
+    run(f'{PROXY_CLI} config proxy.tls.cert="" proxy.tls.key=""'.split())
+    run(f"{PROXY_CLI} import-certificate --selfsigned".split())
+
+
 def config_options():
     """Helper function to take the charm config.yaml and make it a dict.
 
diff --git a/tests/test_charm.py b/tests/test_charm.py
index 7c0ccd6..e36b9a3 100644
--- a/tests/test_charm.py
+++ b/tests/test_charm.py
@@ -760,3 +760,17 @@ class TestTLSCertificate(CharmTest):
             "Problem loading the certificate, they should be PEM encoded strings"
         ]
         assert len(harness.charm.errors) == 1
+
+    def test_self_signed_cert(self, harness):
+        self.setup_harness(harness)
+
+        domain = "https://test.domain.just.invented"
+        harness.charm._stored.domain = domain + ".some.added.test"
+        cert = key = "selfsigned"
+
+        with patch("charm.configure_self_signed_cert") as m_self_signed, patch(
+            "charm.open_port"
+        ):
+            harness.update_config({"certificate": cert, "private_key": key})
+
+        m_self_signed.assert_called_once()
diff --git a/tests/test_helpers.py b/tests/test_helpers.py
index b81b358..15fac97 100644
--- a/tests/test_helpers.py
+++ b/tests/test_helpers.py
@@ -2,15 +2,15 @@ from unittest import mock
 
 import helpers
 
+PROXY_CLI = "snap-store-proxy"
+
 
 def test_configure_proxy():
     with mock.patch("helpers.run") as mocked_run:
         helpers.configure_proxy("myvalue", "myoption")
 
     assert mocked_run.called
-    mocked_run.assert_called_once_with(
-        ["snap-store-proxy", "config", "myvalue=myoption"]
-    )
+    mocked_run.assert_called_once_with([PROXY_CLI, "config", "myvalue=myoption"])
 
 
 def test_configure_proxy_list():
@@ -21,7 +21,7 @@ def test_configure_proxy_list():
 
     assert mocked_run.called
     mocked_run.assert_called_once_with(
-        ["snap-store-proxy", "config", 'one="value_one"', 'two="value_two"']
+        [PROXY_CLI, "config", 'one="value_one"', 'two="value_two"']
     )
 
 
@@ -31,7 +31,7 @@ def test_configure_proxy_forced():
 
     assert mocked_run.called
     mocked_run.assert_called_once_with(
-        ["snap-store-proxy", "config", "myvalue=myoption", "--force"]
+        [PROXY_CLI, "config", "myvalue=myoption", "--force"]
     )
 
 
@@ -43,7 +43,20 @@ def test_configure_proxy_list_forced():
 
     assert mocked_run.called
     mocked_run.assert_called_once_with(
-        ["snap-store-proxy", "config", 'one="value_one"', 'two="value_two"', "--force"]
+        [PROXY_CLI, "config", 'one="value_one"', 'two="value_two"', "--force"]
+    )
+
+
+def test_configure_self_signed_cert():
+    with mock.patch("helpers.run") as mocked_run:
+        helpers.configure_self_signed_cert()
+
+    mocked_run.assert_has_calls(
+        [
+            mock.call(f'{PROXY_CLI} config proxy.tls.cert="" proxy.tls.key=""'.split()),
+            mock.call(f"{PROXY_CLI} import-certificate --selfsigned".split()),
+        ],
+        any_order=False,
     )