Merge ~hloeung/content-cache-charm:master into content-cache-charm:master

[Haw Loeung]

Allow overriding secrets of origin headers with a dict

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[Stuart Bishop (stub)]

Looks good. Should be good to land with modifications, or hit me up for a re-review if you think it is necessary.

review: Approve

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision f6ed3c5572f0ac798390362e681ced5a31da6094

diff --git a/config.yaml b/config.yaml
index 2fa03a3..3e949e6 100644
--- a/config.yaml
+++ b/config.yaml
@@ -18,5 +18,14 @@ options:
       YAML-formatted virtual hosts/sites. e.g.
       site1.local:
         backends:
-        - 91.189.88.149:80
-        - 91.189.88.152:80
+          - 91.189.88.149:80
+          - 91.189.88.152:80
+        origin-headers:
+          - X-Origin-Key: ${secret}
+  sites_secrets:
+    default: ""
+    type: string
+    description: |
+      YAML-formatted dictionary of secrets/keys. e.g.
+      site1.local:
+        X-Origin-Key: mysecretkey
diff --git a/reactive/content_cache.py b/reactive/content_cache.py
index 51da197..b7c1663 100644
--- a/reactive/content_cache.py
+++ b/reactive/content_cache.py
@@ -70,16 +70,18 @@ def configure_nginx():
         status.blocked('list of sites provided has no backends or seems invalid')
         reactive.clear_flag('content_cache.active')
         return
+    sites_secrets = secrets_from_config(config.get('sites_secrets'))
 
     changed = False
-    for site in sites.keys():
-        cache_port = sites[site]['cache_port']
-        backend_port = sites[site]['backend_port']
+    for site, site_conf in sites.items():
+        cache_port = site_conf['cache_port']
+        backend_port = site_conf['backend_port']
         backend = 'http://localhost:{}'.format(backend_port)
         # Per site secret HMAC key, if it exists. We pass this through to the
         # caching layer to activate the bit to restrict access.
-        signed_url_hmac_key = sites[site].get('signed-url-hmac-key')
-        origin_headers = sites[site].get('origin-headers')
+        signed_url_hmac_key = site_conf.get('signed-url-hmac-key')
+        secrets = sites_secrets.get(site)
+        origin_headers = map_origin_headers_to_secrets(site_conf.get('origin-headers'), secrets)
         if ngx_conf.write_site(site, ngx_conf.render(site, cache_port, backend, signed_url_hmac_key, origin_headers)):
             hookenv.log('Wrote out new configs for site: {}'.format(site))
             changed = True
@@ -215,3 +217,26 @@ def sites_from_config(sites_yaml):
         site_conf['cache_port'] = cache_port
         site_conf['backend_port'] = backend_port
     return conf
+
+
+def secrets_from_config(secrets_yaml):
+    secrets = ''
+    if not secrets_yaml:
+        return {}
+    try:
+        secrets = yaml.safe_load(secrets_yaml)
+    except yaml.YAMLError:
+        return {}
+    if isinstance(secrets, dict):
+        return secrets
+    else:
+        return {}
+
+
+def map_origin_headers_to_secrets(origin_headers, secrets):
+    if origin_headers:
+        for l in origin_headers:
+            for header, value in l.items():
+                if value == '${secret}':
+                    l[header] = secrets.get(header)
+    return origin_headers
diff --git a/tests/unit/files/config_test_secrets.txt b/tests/unit/files/config_test_secrets.txt
new file mode 100644
index 0000000..7210a90
--- /dev/null
+++ b/tests/unit/files/config_test_secrets.txt
@@ -0,0 +1,2 @@
+site1.local:
+  X-Origin-Key: Sae6oob2aethuosh
diff --git a/tests/unit/files/nginx_config_rendered_test_output-site1.local-secrets.txt b/tests/unit/files/nginx_config_rendered_test_output-site1.local-secrets.txt
new file mode 100644
index 0000000..48089e2
--- /dev/null
+++ b/tests/unit/files/nginx_config_rendered_test_output-site1.local-secrets.txt
@@ -0,0 +1,22 @@
+proxy_cache_path /var/lib/nginx/proxy/site1.local use_temp_path=off levels=1:2 keys_zone=site1-cache:10m max_size=1g;
+
+server {
+    server_name site1.local;
+    listen 6080;
+
+    location / {
+        proxy_pass http://localhost:8080;
+        proxy_set_header Host "site1.local";
+        proxy_cache site1-cache;
+        proxy_cache_background_update on;
+        proxy_cache_lock on;
+        proxy_cache_min_uses 5;
+        proxy_cache_revalidate on;
+        proxy_cache_use_stale error timeout updating http_500 http_502 http_503 http_504;
+        proxy_cache_valid 200 1d;
+        proxy_set_header X-Origin-Key Sae6oob2aethuosh;
+    }
+
+    access_log /var/log/nginx/site1.local-access.log;
+    error_log /var/log/nginx/site1.local-error.log;
+}
diff --git a/tests/unit/test_content_cache.py b/tests/unit/test_content_cache.py
index db15a60..596d4b3 100644
--- a/tests/unit/test_content_cache.py
+++ b/tests/unit/test_content_cache.py
@@ -151,6 +151,39 @@ class TestCharm(unittest.TestCase):
                     current = f.read()
                 self.assertEqual(expected, current)
 
+    @mock.patch('reactive.content_cache.service_start_or_restart')
+    def test_configure_nginx_sites_secrets(self, service_start_or_restart):
+        with open('tests/unit/files/config_test_secrets.txt', 'r', encoding='utf-8') as f:
+            secrets = f.read()
+        config = '''
+site1.local:
+    backends:
+      - 127.0.1.10:80
+      - 127.0.1.11:80
+      - 127.0.1.12:80
+    origin-headers:
+      - X-Origin-Key: ${secret}
+'''
+        self.mock_config.return_value = {
+            'sites': config,
+            'sites_secrets': secrets,
+        }
+
+        with mock.patch('lib.nginx.NginxConf.sites_path', new_callable=mock.PropertyMock) as mock_site_path:
+            mock_site_path.return_value = os.path.join(self.tmpdir, 'sites-available')
+            # sites-available and sites-enabled won't exist in our temp dir
+            os.mkdir(os.path.join(self.tmpdir, 'sites-available'))
+            os.mkdir(os.path.join(self.tmpdir, 'sites-enabled'))
+            content_cache.configure_nginx()
+            for site in ['site1.local']:
+                with open('tests/unit/files/nginx_config_rendered_test_output-{}-secrets.txt'.format(site),
+                          'r', encoding='utf-8') as f:
+                    expected = f.read()
+                with open(os.path.join(self.tmpdir, 'sites-available/{}.conf'.format(site)),
+                          'r', encoding='utf-8') as f:
+                    current = f.read()
+                self.assertEqual(expected, current)
+
     @mock.patch('charms.reactive.clear_flag')
     @mock.patch('charms.reactive.set_flag')
     def test_configure_nginx_sites_no_backend(self, set_flag, clear_flag):
@@ -297,3 +330,24 @@ site1.local:
         port: 80
 '''
         self.assertFalse(content_cache.sites_from_config(config_yaml))
+
+    def test_secrets_from_config(self):
+        secrets_yaml = '''
+site1.local:
+        X-Some-Header: myvalue
+'''
+        expected = {
+            'site1.local': {
+                'X-Some-Header': 'myvalue',
+            }
+        }
+        self.assertEqual(content_cache.secrets_from_config(secrets_yaml), expected)
+        self.assertEqual(content_cache.secrets_from_config(''), {})
+        self.assertEqual(content_cache.secrets_from_config('invalid YAML'), {})
+        self.assertEqual(content_cache.secrets_from_config('invalid\n\tYAML'), {})
+
+    def test_map_origin_headers_to_secrets(self):
+        origin_headers = [{'X-Origin-Key': '${secret}'}]
+        secrets = {'X-Origin-Key': 'Sae6oob2aethuosh'}
+        expected = [{'X-Origin-Key': 'Sae6oob2aethuosh'}]
+        self.assertEqual(content_cache.map_origin_headers_to_secrets(origin_headers, secrets), expected)