Merge pull request #2646 from ecordell/kubernetes-ca-fix

ConfigProviders abstract over path construction

conf/init/certs_install.sh

@@ -23,5 +23,13 @@ if [ -f /conf/stack/extra_ca_certs ]; then
   cat /conf/stack/extra_ca_certs >> /venv/lib/python2.7/site-packages/requests/cacert.pem
 fi
 
+## Add extra trusted certificates (prefixed)
+for f in $(ls -p /conf/stack/extra_ca* | grep -v ':$' | grep -v '/$')
+do
+ echo "Installing extra cert $f"
+ cp "$f" /usr/local/share/ca-certificates/
+ cat "$f" >> /venv/lib/python2.7/site-packages/requests/cacert.pem
+done
+
 # Update all CA certificates.
 update-ca-certificates

endpoints/api/superuser.py

@@ -852,7 +852,7 @@ class SuperUserCustomCertificates(ApiResource):
       cert_views = []
       for extra_cert_path in extra_certs_found:
         try:
-          cert_full_path = os.path.join(EXTRA_CA_DIRECTORY, extra_cert_path)
+          cert_full_path = config_provider.get_volume_path(EXTRA_CA_DIRECTORY, extra_cert_path)
           with config_provider.get_volume_file(cert_full_path) as f:
             certificate = load_certificate(f.read())
             cert_views.append({
@@ -900,7 +900,7 @@ class SuperUserCustomCertificate(ApiResource):
         abort(400)
 
       logger.debug('Saving custom certificate %s', certpath)
-      cert_full_path = os.path.join(EXTRA_CA_DIRECTORY, certpath)
+      cert_full_path = config_provider.get_volume_path(EXTRA_CA_DIRECTORY, certpath) 
       config_provider.save_volume_file(cert_full_path, uploaded_file)
       logger.debug('Saved custom certificate %s', certpath)
 
@@ -934,7 +934,7 @@ class SuperUserCustomCertificate(ApiResource):
   @verify_not_prod
   def delete(self, certpath):
     if SuperUserPermission().can():
-      cert_full_path = os.path.join(EXTRA_CA_DIRECTORY, certpath)
+      cert_full_path = config_provider.get_volume_path(EXTRA_CA_DIRECTORY, certpath)
       config_provider.remove_volume_file(cert_full_path)
       return '', 204
 

util/config/provider/baseprovider.py

@@ -109,6 +109,11 @@ class BaseProvider(object):
         indicating that this container requires a restart.
     """
     raise NotImplementedError
+  
+  def get_volume_path(self, directory, filename):
+    """ Helper for constructing file paths, which may differ between providers. For example,
+    kubernetes can't have subfolders in configmaps """
+    raise NotImplementedError
 
   def _get_license_file(self):
     """ Returns the contents of the license file. """

util/config/provider/fileprovider.py

@@ -110,3 +110,6 @@ class FileConfigProvider(BaseProvider):
         return True
 
     return False
+  
+  def get_volume_path(self, directory, filename):
+    return os.path.join(directory, filename)

util/config/provider/k8sprovider.py

@@ -54,6 +54,19 @@ class KubernetesConfigProvider(FileConfigProvider):
       self._update_secret_file(filename, contents)
     except IOError as ioe:
       raise CannotWriteConfigException(str(ioe))
+    
+  def volume_file_exists(self, filename):
+    secret = self._lookup_secret()
+    return filename in secret
+    
+
+  def list_volume_directory(self, path):
+    secret = self._lookup_secret()
+    
+    paths = []
+    for filename in secret:
+      if filename.startswith(path):
+        paths.append(filename[len(path) + 1:])
 
   def remove_volume_file(self, filename):
     super(KubernetesConfigProvider, self).remove_volume_file(filename)
@@ -130,3 +143,6 @@ class KubernetesConfigProvider(FileConfigProvider):
 
     request = Request(method, url, data=data, headers=headers)
     return session.send(request.prepare(), verify=False, timeout=2)
+
+  def get_volume_path(self, directory, filename):
+    return "_".join([directory.rstrip('/'), filename])

util/config/provider/test/test_fileprovider.py

@@ -0,0 +1,29 @@
+import pytest
+
+from util.config.provider import FileConfigProvider 
+
+from test.fixtures import *
+
+
+class TestFileConfigProvider(FileConfigProvider):
+  def __init__(self):
+    self.yaml_filename = 'yaml_filename'
+    self._service_token = 'service_token'
+    self.config_volume = 'config_volume' 
+    self.py_filename = 'py_filename'
+    self.yaml_path = os.path.join(self.config_volume, self.yaml_filename)
+    self.py_path = os.path.join(self.config_volume, self.py_filename)
+
+
+@pytest.mark.parametrize('directory,filename,expected', [
+  ("directory", "file", "directory/file"),
+  ("directory/dir", "file", "directory/dir/file"),
+  ("directory/dir/", "file", "directory/dir/file"),
+  ("directory", "file/test", "directory/file/test"),
+])
+def test_get_volume_path(directory, filename, expected):
+  provider = TestFileConfigProvider()
+
+  assert expected == provider.get_volume_path(directory, filename)
+
+

util/config/provider/test/test_k8sprovider.py

@@ -0,0 +1,25 @@
+import pytest
+
+from util.config.provider import KubernetesConfigProvider
+
+from test.fixtures import *
+
+
+class TestKubernetesConfigProvider(KubernetesConfigProvider):
+  def __init__(self):
+    self.yaml_filename = 'yaml_filename'
+    self._service_token = 'service_token'
+
+
+@pytest.mark.parametrize('directory,filename,expected', [
+  ("directory", "file", "directory_file"),
+  ("directory/dir", "file", "directory/dir_file"),
+  ("directory/dir/", "file", "directory/dir_file"),
+  ("directory", "file/test", "directory_file/test"),
+])
+def test_get_volume_path(directory, filename, expected):
+  provider = TestKubernetesConfigProvider()
+  
+  assert expected == provider.get_volume_path(directory, filename)
+
+

util/config/provider/testprovider.py

@@ -1,5 +1,6 @@
 import json
 import io
+import os
 from datetime import datetime, timedelta
 
 from util.config.provider.baseprovider import BaseProvider
@@ -88,3 +89,7 @@ class TestConfigProvider(BaseProvider):
   def reset_for_test(self):
     self._config['SUPER_USERS'] = ['devtable']
     self.files = {}
+    
+  def get_volume_path(self, directory, filename):
+    return os.path.join(directory, filename)
+