Merge pull request #756 from ministryofjustice/ag--fix-load-config-ra…

…ce-condition

Fix for kubernetes race condition on configuration

Dockerfile

@@ -20,7 +20,7 @@ RUN apk add --no-cache \
         build-base=0.5-r1 \
         ca-certificates=20190108-r0 \
         libffi-dev=3.2.1-r6 \
-        python3-dev=3.7.3-r0 \
+        python3-dev=3.7.4-r0 \
         libressl-dev=2.7.5-r0 \
         libstdc++=8.3.0-r0 \
         postgresql-dev \

controlpanel/api/kubernetes.py

@@ -1,9 +1,37 @@
+from copy import deepcopy
 import inspect
+import kubernetes
+import os
 
 from crequest.middleware import CrequestMiddleware
 from django.conf import settings
 
-from controlpanel.kubeapi.views import kubernetes, load_kube_config
+# This patch fixes incorrect base64 padding in the Kubernetes Python client.
+# Hopefully it will be fixed in the next release.
+from controlpanel.kubeapi import oidc_patch
+
+
+def get_config():
+    """
+    Load and returns a kubernetes Configuration instance
+    """
+
+    if "KUBERNETES_SERVICE_HOST" in os.environ:
+        kubernetes.config.load_incluster_config()
+    else:
+        kubernetes.config.load_kube_config()
+
+    config = kubernetes.client.Configuration()
+
+    # A deepcopy of the configuration is used to avoid a race condition
+    # caused by subsequent calls to Configuration() reusing a singleton
+    # datastructure
+    #
+    # See: https://github.com/kubernetes-client/python/issues/932
+    config.api_key_prefix = deepcopy(config.api_key_prefix)
+    config.api_key = deepcopy(config.api_key)
+
+    return config
 
 
 class KubernetesClient:
@@ -33,17 +61,16 @@ def __init__(self, id_token=None, use_cpanel_creds=False):
                 "the k8s API unless stricly necessary."
             )
 
-        load_kube_config()
-        config = kubernetes.client.Configuration()
+        config = get_config()
 
         if settings.ENABLED["k8s_rbac"] and not use_cpanel_creds:
             if id_token is None:
                 request = CrequestMiddleware.get_request()
                 if request and request.user and request.user.is_authenticated:
-                    id_token = request.session.get('oidc_id_token')
+                    id_token = request.session.get("oidc_id_token")
 
-            config.api_key_prefix['authorization'] = 'Bearer'
-            config.api_key['authorization'] = id_token
+            config.api_key_prefix["authorization"] = "Bearer"
+            config.api_key["authorization"] = id_token
 
         self.api_client = kubernetes.client.ApiClient(config)
 

controlpanel/frontend/views/app.py

@@ -66,8 +66,7 @@ def get_context_data(self, **kwargs):
         context = super().get_context_data(**kwargs)
         app = self.get_object()
 
-        # Temporarily hiding App URL until we fix the `KubernetesClient` bug causing the 401
-        context["app_url"] = None # cluster.App(app).url
+        context["app_url"] = cluster.App(app).url
         context["admin_options"] = User.objects.filter(
             auth0_id__isnull=False,
         ).exclude(

controlpanel/kubeapi/views.py

@@ -1,30 +1,14 @@
-from functools import wraps
-import os
 from urllib.parse import urljoin
 
-from django.conf import settings
 from django.core import exceptions
 from django.views.decorators.csrf import csrf_exempt
 from djproxy.views import HttpProxy
-import kubernetes
 
+from controlpanel import api
 from controlpanel.jwt import JWT
-# This patch fixes incorrect base64 padding in the Kubernetes Python client.
-# Hopefully it will be fixed in the next release.
-from controlpanel.kubeapi import oidc_patch
-from controlpanel.kubeapi.permissions import K8sPermissions
-
-
-def load_kube_config():
-    """
-    Load Kubernetes config. Avoid running at import time.
-    """
 
-    if 'KUBERNETES_SERVICE_HOST' in os.environ:
-        kubernetes.config.load_incluster_config()
+from controlpanel.kubeapi.permissions import K8sPermissions
 
-    else:
-        kubernetes.config.load_kube_config()
 
 
 class KubeAPIAuthMiddleware(object):
@@ -45,20 +29,20 @@ class KubeAPIProxy(HttpProxy):
 
     @property
     def base_url(self):
-        return kubernetes.client.Configuration().host
+        return self.kubernetes_config.host
 
     # Without this, we get SSL: CERTIFICATE_VERIFY_FAILED
     @property
     def verify_ssl(self):
-        return kubernetes.client.Configuration().ssl_ca_cert
+        return self.kubernetes_config.ssl_ca_cert
 
     @property
     def proxy_url(self):
         return urljoin(self.base_url, self.kwargs.get('url', ''))
 
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
-        load_kube_config()
+        self.kubernetes_config = api.kubernetes.get_config()
         self.proxy_middleware.append(
             "controlpanel.kubeapi.views.KubeAPIAuthMiddleware",
         )

tests/kubeapi/test_permissions.py

@@ -5,18 +5,20 @@
 from rest_framework import status
 
 
-TEST_K8S_API_URL = 'https://k8s.example.com'
-
 
 @pytest.yield_fixture(autouse=True)
-def k8s():
-    with patch('controlpanel.kubeapi.views.kubernetes') as k8s:
-        config = k8s.client.Configuration.return_value
-        config.host = TEST_K8S_API_URL
-        config.api_key = {
-            "authorization": "Bearer test-token",
-        }
-        yield k8s
+def k8s_get_config():
+    with patch('controlpanel.kubeapi.views.api.kubernetes.get_config') as get_config:
+        config = MagicMock("k8s config")
+
+        config.host = "http://api.k8s.localhost"
+        config.ssl_ca_cert = "test ssl_ca_cert"
+
+        config.api_key_prefix = {"authorization": "Bearer"}
+        config.api_key = {"authorization": "test-token"}
+
+        get_config.return_value = config
+        yield get_config
 
 
 @pytest.yield_fixture(autouse=True)